bruno/ifcurl
1
0
Fork 0
mirror of https://github.com/brunopostle/ifcurl.git synced 2026-07-12 02:08:13 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions

Document picker: forward OAuth2 access token to Forgejo API

Browse source 
The select-documents picker queried the Forgejo API with only the
browser's session cookie (credentials: 'include'), so an OpenCDE client
that authenticated via OAuth2 saw only public repositories.

Thread the access token presented on the select-documents request
through to the picker UI (query param) and have its JS send it as
Authorization: Bearer on every Forgejo API call, falling back to cookie
auth when no token is supplied. Add a no-referrer policy so the token in
the page URL does not leak via Referer on API fetches or the callback
redirect.

Closes ifcurl-i5c.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Bruno Postle 2026-06-07 23:56:30 +01:00
parent 3a395cca41
commit fee3008b60
3 changed files with 75 additions and 6 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

2
.beads/issues.jsonl
View file

@ -3,7 +3,7 @@
{"id":"ifcurl-p60","title":"Viewer: no element click-to-identify — can't get GlobalId or properties","description":"There is no way to click on a mesh in the viewer and see which IFC element it represents. Reviewers can see geometry but cannot reference specific elements — they can't discover a GlobalId to put in a selector URL, can't see the element name/type/Psets, and can't say 'this specific wall' in a BCF or Forgejo comment. This breaks the collaboration workflow at its foundation: every useful feedback comment needs to identify specific elements. Need: click or hover on a surface to highlight it and show a properties panel (name, type, GlobalId, key Psets). The GlobalId should be copyable into the selector field or URL. The ThatOpen components library provides the tools for this (OBC.IfcRelationsIndexer, element property queries).","status":"closed","priority":1,"issue_type":"feature","assignee":"Bruno Postle","owner":"bruno@postle.net","created_at":"2026-04-24T11:02:33Z","created_by":"Bruno Postle","updated_at":"2026-04-24T11:47:23Z","started_at":"2026-04-24T11:14:57Z","closed_at":"2026-04-24T11:47:23Z","close_reason":"Closed","dependency_count":0,"dependent_count":0,"comment_count":0}
{"id":"ifcurl-4l7","title":"viewer.html: scale= URLs don't load in orthographic mode","description":"applyCameraParam() reads the camera= and fov= parameters from the URL and applies them, but never reads scale= or switches the camera to orthographic (parallel) projection. URLs like ?camera=...\u0026scale=50 will load in perspective mode instead of orthographic. The OBC OrthoPerspectiveCamera can switch modes, but the code path to set parallel projection and apply the scale value is missing from applyCameraParam.","status":"closed","priority":1,"issue_type":"bug","assignee":"Bruno Postle","owner":"bruno@postle.net","created_at":"2026-04-24T09:04:51Z","created_by":"Bruno Postle","updated_at":"2026-04-24T10:56:29Z","started_at":"2026-04-24T10:50:19Z","closed_at":"2026-04-24T10:56:29Z","close_reason":"Closed","dependency_count":0,"dependent_count":0,"comment_count":0}
{"id":"ifcurl-362","title":"Add missing server-config files: gitconfig-ifcmerge and gitattributes","description":"forgejo/README.md documents deploying two files that don't exist in the repo: forgejo/server-config/gitconfig-ifcmerge and forgejo/server-config/gitattributes. Both are required for the ifcmerge git merge driver to work in bare Forgejo repos. The README even includes 'sudo cp forgejo/server-config/gitattributes /etc/gitattributes' and 'sudo git config --system include.path .../gitconfig-ifcmerge', but the files themselves are absent. Anyone following the deployment instructions will hit a 'file not found' error. Need to create both files with the content described in the README.","status":"closed","priority":1,"issue_type":"bug","assignee":"Bruno Postle","owner":"bruno@postle.net","created_at":"2026-04-24T09:04:31Z","created_by":"Bruno Postle","updated_at":"2026-04-24T10:51:47Z","started_at":"2026-04-24T10:50:15Z","closed_at":"2026-04-24T10:51:47Z","close_reason":"Files already exist: forgejo/server-config/gitconfig-ifcmerge and gitattributes are both present and correct. Missed in initial review.","dependency_count":0,"dependent_count":0,"comment_count":0}
{"id":"ifcurl-i5c","title":"Document picker: private repos not visible — picker needs to use OAuth2 access token","description":"The select-documents UI at /documents/1.0/select-documents/ui calls the Forgejo repos API without authentication, so only public repos appear in the dropdown. In a real OpenCDE integration the client would have obtained an access_token via the OAuth2 flow before invoking the document picker. The picker should accept the access_token (e.g. passed in the callback_url context or as a query param) and forward it as Authorization: Bearer to the Forgejo API so private repos are visible.","status":"open","priority":2,"issue_type":"bug","owner":"bruno@postle.net","created_at":"2026-06-07T22:41:36Z","created_by":"Bruno Postle","updated_at":"2026-06-07T22:41:36Z","dependency_count":0,"dependent_count":0,"comment_count":0}
{"id":"ifcurl-i5c","title":"Document picker: private repos not visible — picker needs to use OAuth2 access token","description":"The select-documents UI at /documents/1.0/select-documents/ui calls the Forgejo repos API without authentication, so only public repos appear in the dropdown. In a real OpenCDE integration the client would have obtained an access_token via the OAuth2 flow before invoking the document picker. The picker should accept the access_token (e.g. passed in the callback_url context or as a query param) and forward it as Authorization: Bearer to the Forgejo API so private repos are visible.","status":"in_progress","priority":2,"issue_type":"bug","assignee":"Bruno Postle","owner":"bruno@postle.net","created_at":"2026-06-07T22:41:36Z","created_by":"Bruno Postle","updated_at":"2026-06-07T22:48:07Z","started_at":"2026-06-07T22:48:07Z","dependency_count":0,"dependent_count":0,"comment_count":0}
{"id":"ifcurl-e9g","title":"Security review: viewer.js and viewer HTML","description":"Perform a security review of the browser-side viewer code. Files to cover: forgejo/custom/public/assets/viewer.js and forgejo/custom/public/assets/viewer.html (or equivalent). Areas to examine: ifc:// URL parsing and handling of untrusted URL components before passing to API calls or displaying in the UI, postMessage or cross-origin communication patterns, innerHTML or other unsafe DOM sinks fed from URL parameters or API responses, camera/selector parameter injection into API URLs, BCF file parsing (Zip content, XML parsing) for injection or path traversal, any eval() or dynamic script execution, handling of Forgejo API responses in the metadata/query panels, content security policy considerations, handling of the ifc:// protocol in OS-registered handler (ifcurl-open/register). Produce a prioritised list of findings with recommended fixes.","status":"closed","priority":2,"issue_type":"task","assignee":"Bruno Postle","owner":"bruno@postle.net","created_at":"2026-06-07T11:51:09Z","created_by":"Bruno Postle","updated_at":"2026-06-07T13:50:24Z","started_at":"2026-06-07T11:51:34Z","closed_at":"2026-06-07T13:50:24Z","close_reason":"Two findings fixed: stored XSS in populateMetaPanel via unescaped IFC model data (added hesc() helper), sessionStorage CSS injection in viewer.html snapshot overlay (validated data:image/jpeg;base64, prefix). No other high-confidence findings.","dependency_count":0,"dependent_count":0,"comment_count":0}
{"id":"ifcurl-b99","title":"Security review: Python API service (service.py, bcf_api.py, documents_api.py)","description":"Perform a security review of the core Python API service code. Files to cover: ifcurl/service.py (rate limiting, SSRF protection, caching, all endpoints), ifcurl/bcf_api.py (BCF 3.0 REST routes, Forgejo proxy, GUID encoding), ifcurl/documents_api.py (OpenCDE Documents API, select-documents picker), ifcurl/foundation_api.py (OAuth2 proxy routes). Areas to examine: SSRF protection gaps (is _ssrf_check applied consistently to all endpoints that fetch remote content?), sandbox escape paths (does sandboxed rendering fully isolate arbitrary IFC input?), injection risks in Forgejo API proxying (header forwarding, response handling), authentication bypass in BCF routes, document_id encoding/decoding robustness, input validation on all Pydantic models, any path traversal in file or cache operations, tier-4 cache poisoning via mutable/immutable ref misclassification. Produce a prioritised list of findings with recommended fixes.","status":"closed","priority":2,"issue_type":"task","assignee":"Bruno Postle","owner":"bruno@postle.net","created_at":"2026-06-07T11:51:06Z","created_by":"Bruno Postle","updated_at":"2026-06-07T13:50:05Z","started_at":"2026-06-07T11:51:32Z","closed_at":"2026-06-07T13:50:05Z","close_reason":"Three findings fixed: hostname SSRF bypass in _is_private_ip (now resolves DNS, fail-safe on error), SSRF+token exfiltration in BCF snapshot endpoint (now validates URL via _ssrf_check, drops caller token), X-Forwarded-Host in download URLs noted as medium/deployment concern.","dependency_count":0,"dependent_count":0,"comment_count":0}
{"id":"ifcurl-2yd","title":"Security review: OpenCDE CDE routes (OAuth2 proxy + document picker)","description":"Perform a security review of the CDE-related code added in foundation_api.py and the select-documents additions to documents_api.py. Areas to examine:\\n- OAuth2 proxy: does proxying client_secret server-side introduce SSRF or credential leak risks? Are Forgejo error responses safely forwarded without leaking internal detail?\\n- Picker HTML: is callback_url validated before embedding? Could a crafted callback_url cause open redirect to an attacker-controlled host? Is the JSON embedding in \u003cscript\u003e safe against XSS (\u003c/script\u003e injection test exists but review the escaping logic)?\\n- document_id encoding: can a malformed document_id returned from the picker cause decode_document_id to crash or expose unexpected data?\\n- Rate limiting: do the new /foundation/ and /select-documents endpoints inherit the rate-limit middleware?\\n- Auth forwarding: the document picker UI calls Forgejo API with credentials:include (session cookie). Could a malicious callback_url cause the picker to exfiltrate the session cookie?\\n- Input validation: are all Pydantic models tight enough to reject unexpected fields?\\nProduce a prioritised list of findings with recommended fixes.","status":"closed","priority":2,"issue_type":"task","assignee":"Bruno Postle","owner":"bruno@postle.net","created_at":"2026-06-07T11:21:23Z","created_by":"Bruno Postle","updated_at":"2026-06-07T11:48:36Z","started_at":"2026-06-07T11:28:58Z","closed_at":"2026-06-07T11:48:36Z","close_reason":"One confirmed HIGH finding: DOM XSS in renderBreadcrumb via unescaped single quote in onclick — fixed by replacing innerHTML construction with DOM API calls. Three other candidates (callback_url XSS, X-Forwarded-Host injection, Forgejo error disclosure) were false positives.","dependency_count":0,"dependent_count":0,"comment_count":0}

46
ifcurl/documents_api.py
View file

@ -196,6 +196,21 @@ class SelectDocumentsRequest(BaseModel):
callback: _Callback
def _bearer_token(request: Request) -> str | None:
"""Extract the OAuth2 access token from the request's Authorization header.
Accepts both ``Bearer <token>`` and Forgejo's ``token <token>`` forms and
returns the bare token, or ``None`` when no Authorization header is present.
"""
header = request.headers.get("authorization")
if not header:
return None
scheme, _, value = header.partition(" ")
if scheme.lower() in ("bearer", "token") and value:
return value
return header # opaque value with no recognised scheme — forward verbatim
@router.post("/select-documents")
def select_documents(body: SelectDocumentsRequest, request: Request) -> JSONResponse:
"""Return a URL for the document-picker UI.
@ -204,6 +219,10 @@ def select_documents(body: SelectDocumentsRequest, request: Request) -> JSONResp
redirects the user's browser to ``select_documents_url``; the picker lets
the user choose an IFC file from a Forgejo repository, then redirects back
to ``callback.url`` with ``document_ids[]`` query parameters appended.
The OAuth2 access token presented on this request (``Authorization:
Bearer``) is threaded through to the picker so it can query the Forgejo API
on the user's behalf — without it the picker only sees public repositories.
"""
host = request.headers.get("x-forwarded-host") or request.headers.get("host", "localhost")
proto = request.headers.get("x-forwarded-proto", "https")
@ -212,20 +231,27 @@ def select_documents(body: SelectDocumentsRequest, request: Request) -> JSONResp
f"{base}/documents/1.0/select-documents/ui"
f"?callback_url={quote(body.callback.url, safe='')}"
)
token = _bearer_token(request)
if token:
picker_url += f"&access_token={quote(token, safe='')}"
return JSONResponse({"select_documents_url": picker_url})
@router.get("/select-documents/ui", response_class=HTMLResponse)
def select_documents_ui(callback_url: str) -> HTMLResponse:
def select_documents_ui(callback_url: str, access_token: str | None = None) -> HTMLResponse:
"""Serve the document-picker HTML page.
The page uses the Forgejo REST API to let the user browse repositories and
select an IFC file. On selection it redirects to ``callback_url`` with
``document_ids[]`` appended.
When ``access_token`` is supplied (forwarded from the select-documents
flow) the picker sends it as ``Authorization: Bearer`` on every Forgejo API
call so private repositories the user can access are listed.
"""
if not callback_url:
raise HTTPException(status_code=400, detail="callback_url is required")
return HTMLResponse(_picker_html(callback_url))
return HTMLResponse(_picker_html(callback_url, access_token))
# ---------------------------------------------------------------------------
@ -237,6 +263,8 @@ _PICKER_TEMPLATE = """\
<html lang="en">
<head>
<meta charset="utf-8">
<!-- access_token may ride in this page's URL; keep it out of Referer headers -->
<meta name="referrer" content="no-referrer">
<title>Select IFC Document</title>
<style>
*, *::before, *::after { box-sizing: border-box; }
@ -269,13 +297,19 @@ _PICKER_TEMPLATE = """\
<script>
const CALLBACK_URL = __CALLBACK_URL__;
const FORGEJO_BASE = __FORGEJO_BASE__;
const ACCESS_TOKEN = __ACCESS_TOKEN__;
let currentOwner = '', currentRepo = '', currentPath = '';
function setStatus(msg) { document.getElementById('status').textContent = msg; }
function setError(msg) { document.getElementById('error').textContent = msg; }
async function apiFetch(path) {
const r = await fetch(FORGEJO_BASE + path, {credentials: 'include'});
// Prefer the OAuth2 access token (so private repos are visible); fall back to
// the browser's Forgejo session cookie when no token was supplied.
const opts = ACCESS_TOKEN
? {headers: {Authorization: 'Bearer ' + ACCESS_TOKEN}}
: {credentials: 'include'};
const r = await fetch(FORGEJO_BASE + path, opts);
if (!r.ok) throw new Error(r.status + ' ' + r.statusText);
return r.json();
}
@ -387,14 +421,16 @@ loadRepos();
"""
def _picker_html(callback_url: str) -> str:
def _js(v: str) -> str:
def _picker_html(callback_url: str, access_token: str | None = None) -> str:
def _js(v: str | None) -> str:
# json.dumps is valid JSON but </script> inside <script> breaks HTML
# parsing; <\/ is a legal JSON escape that browsers handle correctly.
# json.dumps(None) yields "null" — the JS treats that as "no token".
return json.dumps(v).replace("</", "<\\/")
return (
_PICKER_TEMPLATE
.replace("__CALLBACK_URL__", _js(callback_url))
.replace("__FORGEJO_BASE__", _js(_FORGEJO_URL))
.replace("__ACCESS_TOKEN__", _js(access_token))
)

33
tests/test_documents_api_picker.py
View file

@ -73,6 +73,25 @@ class TestSelectDocuments:
r = client.post("/documents/1.0/select-documents", json={})
assert r.status_code == 422
def test_access_token_forwarded_to_picker_url(self):
r = self._post(headers={"Authorization": "Bearer secret-tok-123"})
url = r.json()["select_documents_url"]
params = parse_qs(urlparse(url).query)
assert params["access_token"][0] == "secret-tok-123"
def test_token_scheme_forwarded_to_picker_url(self):
# Forgejo's own "token <tok>" form is also accepted
r = self._post(headers={"Authorization": "token abc-789"})
url = r.json()["select_documents_url"]
params = parse_qs(urlparse(url).query)
assert params["access_token"][0] == "abc-789"
def test_no_access_token_when_unauthenticated(self):
r = self._post()
url = r.json()["select_documents_url"]
params = parse_qs(urlparse(url).query)
assert "access_token" not in params
# ---------------------------------------------------------------------------
# GET /documents/1.0/select-documents/ui
@ -123,6 +142,20 @@ class TestSelectDocumentsUi:
# The raw </script> should not appear unescaped outside a JSON string
assert "</script><script>" not in r.text
def test_access_token_embedded_in_html(self):
r = client.get(
"/documents/1.0/select-documents/ui",
params={"callback_url": CALLBACK, "access_token": "tok-xyz"},
)
assert r.status_code == 200
assert "tok-xyz" in r.text
assert "Bearer" in r.text
def test_access_token_null_when_absent(self):
r = self._get()
# JS receives the literal null so it falls back to cookie auth
assert "const ACCESS_TOKEN = null;" in r.text
# ---------------------------------------------------------------------------
# document_id encoding round-trip (JS parity)