From d07005cbaddca0b8f606bd87b3b4d336abfd4b35 Mon Sep 17 00:00:00 2001 From: Don Beckham Date: Sun, 26 Jul 2026 16:39:40 -0500 Subject: [PATCH] Restore the Starlette upgrade lost in a merge MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Starlette/FastAPI upgrade landed on main and was then silently undone by the next merge: a feature branch cut before it, merged with a stale view of origin/main, resolved requirements.txt and app/main.py back to the older content without reporting a conflict. The changelog entries survived because they were reconstructed by hand at the time; the code did not, and the loss was invisible in a green test run because the new TemplateResponse signature works on both Starlette versions. Re-applies the original change unchanged: fastapi 0.140.0, an explicit starlette 1.3.1 pin, the render()/error_page() helpers, and the canonical-host regression test. Nothing here is new — see the original for the reasoning and the advisories it clears. Verified against main as it now stands, so the upgrade is exercised alongside the trusted-proxy and groups-claim changes rather than on its own. --- app/main.py | 41 +++++++++++++++++------------------- requirements.txt | 5 ++++- tests/backend/test_public.py | 10 +++++++++ 3 files changed, 33 insertions(+), 23 deletions(-) diff --git a/app/main.py b/app/main.py index 4a94649..2904b96 100644 --- a/app/main.py +++ b/app/main.py @@ -97,7 +97,6 @@ def expiry_from_hours(hours: int) -> "object | None": def base_context(request: Request) -> dict: user = get_current_user(request) return { - "request": request, "settings": settings, "user": user, "expiry_options": settings.expiry_options, @@ -105,6 +104,17 @@ def base_context(request: Request) -> dict: } +def render(request: Request, template: str, extra: dict | None = None, status_code: int = 200): + """Render a template with the shared base context merged in.""" + return templates.TemplateResponse( + request, template, {**base_context(request), **(extra or {})}, status_code=status_code, + ) + + +def error_page(request: Request, code: int, message: str): + return render(request, "error.html", {"code": code, "message": message}, status_code=code) + + def record_event(db: Session, request: Request, file: FileModel, *, anonymous: bool, user=None, client_fp: str | None = None, fp_data=None) -> UploadEvent: info = fingerprint.collect(request, client_fp, fp_data) @@ -139,7 +149,7 @@ def landing(request: Request): user = get_current_user(request) if user: return RedirectResponse(url="/app", status_code=302) - return templates.TemplateResponse("landing.html", base_context(request)) + return render(request, "landing.html") @app.post("/api/anon-upload") @@ -214,11 +224,7 @@ async def auth_callback(request: Request): token = await oauth.authentik.authorize_access_token(request) except Exception as exc: # noqa: BLE001 - surface a friendly error page log.warning("OIDC callback failed: %s", exc) - return templates.TemplateResponse( - "error.html", - {**base_context(request), "code": 400, "message": "Sign-in failed. Please try again."}, - status_code=400, - ) + return error_page(request, 400, "Sign-in failed. Please try again.") userinfo = token.get("userinfo") if not userinfo: userinfo = await oauth.authentik.userinfo(token=token) @@ -250,17 +256,14 @@ def app_home(request: Request, db: Session = Depends(get_db)): if not user: return RedirectResponse(url="/login", status_code=302) if not user.in_required_group: - return templates.TemplateResponse( - "not_authorized.html", base_context(request), status_code=403, - ) + return render(request, "not_authorized.html", status_code=403) files = db.execute( select(FileModel) .where(FileModel.owner_sub == user.sub, FileModel.deleted.is_(False)) .order_by(FileModel.created_at.desc()) ).scalars().all() rows = [_file_row(f) for f in files] - ctx = {**base_context(request), "files": rows, "usage": humanize_size(storage.disk_usage_bytes())} - return templates.TemplateResponse("app.html", ctx) + return render(request, "app.html", {"files": rows, "usage": humanize_size(storage.disk_usage_bytes())}) @app.post("/api/files") @@ -365,23 +368,17 @@ def email_share(token: str, request: Request, to: str = Form(...), db: Session = def share_page(token: str, request: Request, db: Session = Depends(get_db)): link = db.get(ShareLink, token) if not link or link.file.deleted: - return templates.TemplateResponse( - "error.html", {**base_context(request), "code": 404, "message": "This link doesn't exist."}, - status_code=404) + return error_page(request, 404, "This link doesn't exist.") if link.is_expired: - return templates.TemplateResponse( - "error.html", {**base_context(request), "code": 410, "message": "This link has expired."}, - status_code=410) - ctx = { - **base_context(request), + return error_page(request, 410, "This link has expired.") + return render(request, "share.html", { "file": link.file, "size_h": humanize_size(link.file.size_bytes), "token": token, "download_url": f"{settings.base_url.rstrip('/')}/d/{token}", "this_url": share_url(token), "expires_at": link.expires_at, - } - return templates.TemplateResponse("share.html", ctx) + }) @app.get("/d/{token}") diff --git a/requirements.txt b/requirements.txt index 9546c53..66d7460 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,5 +1,8 @@ # Beckham Share — Python dependencies (pinned for reproducible builds) -fastapi==0.115.6 +fastapi==0.140.0 +# FastAPI's ASGI core. Pinned explicitly rather than left to resolution because +# it terminates every request and has its own advisory stream. +starlette==1.3.1 uvicorn[standard]==0.34.0 sqlalchemy==2.0.36 psycopg2-binary==2.9.10 diff --git a/tests/backend/test_public.py b/tests/backend/test_public.py index f5f497e..ee60714 100644 --- a/tests/backend/test_public.py +++ b/tests/backend/test_public.py @@ -20,3 +20,13 @@ def test_landing_redirects_logged_in_member(client, member): r = client.get("/", follow_redirects=False) assert r.status_code == 302 assert r.headers["location"] == "/app" + + +def test_unrecognized_host_lands_on_the_canonical_origin(client): + # The app answers on several hostnames, but the authenticated flow is pinned + # to BASE_URL so the session cookie and the OIDC redirect URI stay on one + # origin. Whatever Host arrives, the redirect target is rebuilt from the + # canonical host rather than echoed back. + r = client.get("/app", headers={"host": "elsewhere.example.com"}, follow_redirects=False) + assert r.status_code == 302 + assert r.headers["location"].startswith("https://share.beckham.ai/app")