From 62a5b6cc26ff5707acc7e533ecb85aa864e9dc3b Mon Sep 17 00:00:00 2001 From: jswSpace <1140534808@qq.com> Date: Wed, 14 Oct 2026 08:19:22 +0800 Subject: [PATCH] fix(auth): preserve OIDC authorization endpoint query parameters (#6831) * fix(auth): preserve OIDC authorization endpoint query parameters * docs(auth): keep updated guidance within size budget * docs(auth): clarify OIDC endpoint query configuration --------- Co-authored-by: jswSpace <198684035+jswSpace@users.noreply.github.com> Co-authored-by: Willem Jiang --- README.md | 2 + backend/app/gateway/AGENTS.md | 11 +++- backend/app/gateway/auth/oidc.py | 7 ++- backend/docs/SSO.md | 15 ++++- backend/tests/test_oidc_auth.py | 105 +++++++++++++++++++++++++++++++ 5 files changed, 135 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 7aa07d1b51..a952fd733c 100644 --- a/README.md +++ b/README.md @@ -543,6 +543,8 @@ When fine-grained authorization is enabled, Live Browser connections require `th Browser login uses `HttpOnly` session cookies. The login page offers a "keep me signed in" option that extends the browser session when the request is HTTPS (including trusted `X-Forwarded-Proto: https`) or localhost HTTP. The localhost exception uses the direct request `Host` and ignores forwarded host headers. Public HTTP deployments, including many temporary sandbox URLs, fall back to session cookies by default. DeerFlow never stores the password in browser storage; the UI may remember only the email address. +OIDC/SSO authorization endpoints may include provider query parameters such as `?prompt=login`. DeerFlow preserves those parameters when adding the login request, including for configured `authorization_endpoint` overrides. + Administrators can suspend and restore accounts from **Settings → Users** (admin-only; the section is hidden for ordinary users). The page lists each account's email, role, and status and exposes the same enable/disable operation as `PATCH /api/v1/admin/users/{id}`; the last remaining active admin cannot be disabled, and the Gateway's rejection detail is shown verbatim. A disabled account is rejected by every authentication surface, and its live sessions end on the next request with a login page that states the account has been disabled. Password login for a disabled account deliberately still answers "incorrect email or password" so the state is not disclosed to whoever holds the credentials. DeerFlow still uses `Forwarded` / `X-Forwarded-*` headers to recover the browser-facing scheme and origin behind a proxy. The bundled nginx sets `X-Forwarded-Proto`, but preserves an upstream HTTPS value and does not overwrite every forwarded header. Configure the outer trusted proxy to replace or strip client-supplied forwarding headers before traffic reaches DeerFlow. diff --git a/backend/app/gateway/AGENTS.md b/backend/app/gateway/AGENTS.md index c06e6e66e1..277423ef79 100644 --- a/backend/app/gateway/AGENTS.md +++ b/backend/app/gateway/AGENTS.md @@ -60,7 +60,16 @@ Durable MCP notifications use internal Agent runs. Keep their trusted delivery i CORS is same-origin by default when requests enter through nginx on port 2026. Split-origin or port-forwarded browser clients must opt in with `GATEWAY_CORS_ORIGINS` (exact origins); Gateway `CORSMiddleware` and `CSRFMiddleware` both read that variable so browser CORS and auth-origin checks stay aligned. Those clients also need `CORS_EXPOSED_HEADERS` (`csrf_middleware.py`): run-creating routes return the run's id in `Content-Location`, which is not CORS-safelisted, so JS cannot read it unless it is exposed — and the LangGraph SDK resolves run metadata from that header alone, so withholding it breaks `useStream`'s `onCreated` and thread-gated actions. -Browser auth sessions are owned by `app.gateway.auth.session_cookie`. Login accepts a `remember_me` form flag, but the Gateway never stores passwords. `SessionCookiePolicy` persists the `HttpOnly access_token` cookie only for HTTPS/trusted-forwarded HTTPS, direct-host localhost HTTP, or explicit operator opt-in for insecure persistence; public HTTP sandbox URLs degrade to session cookies. Session-creating handlers stamp the final `max_age` on `request.state`; CSRF cookie creation mirrors it so the double-submit pair expires together, including re-issue after password changes and OIDC callbacks. A small `HttpOnly` preference cookie preserves the remember choice across re-issues. Logout clears all auth cookies and suppresses CSRF re-issue on the logout response. +Browser sessions belong to `app.gateway.auth.session_cookie`. Login's `remember_me` +flag never stores passwords. `SessionCookiePolicy` persists `HttpOnly access_token` +only for HTTPS/trusted-forwarded HTTPS, direct-host localhost HTTP, or explicit +operator opt-in; public HTTP otherwise uses session cookies. Session creators set +final `max_age` on `request.state`; CSRF mirrors it across login, password changes +and OIDC callbacks. A small `HttpOnly` preference cookie preserves the choice. +Logout clears auth cookies and suppresses CSRF re-issue. + +OIDC redirects retain existing endpoint query bytes (RFC 6749 section 3.1). +`tests/test_oidc_auth.py` pins query, state-cookie, nonce and PKCE agreement. Personal Access Tokens (`app.gateway.auth.pat`, `Authorization: Bearer dfp_...`) run as their owning user: an invalid Bearer is a hard 401 with no cookie fallback, which keeps `CSRFMiddleware`'s Bearer skip safe (origin checks still run). Scopes narrow within the allowlisted threads/runs/projects routes (including `POST /api/threads/{id}/move`); every other authenticated route 403s PAT callers (admin included). PAT management and `/change-password` require session auth; only SHA-256 digests are stored (`0017`). diff --git a/backend/app/gateway/auth/oidc.py b/backend/app/gateway/auth/oidc.py index cd2cb5ff3d..626a2f3442 100644 --- a/backend/app/gateway/auth/oidc.py +++ b/backend/app/gateway/auth/oidc.py @@ -10,7 +10,7 @@ import logging import time from dataclasses import dataclass from typing import Any -from urllib.parse import urlencode +from urllib.parse import urlencode, urlsplit, urlunsplit import httpx import jwt @@ -169,7 +169,10 @@ class OIDCService: params["code_challenge"] = code_challenge params["code_challenge_method"] = "S256" - return f"{metadata.authorization_endpoint}?{urlencode(params)}" + endpoint = urlsplit(metadata.authorization_endpoint) + # OAuth endpoints may already have a query (RFC 6749 section 3.1). + query = f"{endpoint.query}&{urlencode(params)}" if endpoint.query else urlencode(params) + return urlunsplit(endpoint._replace(query=query)) # ── Token exchange ───────────────────────────────────────────────────── diff --git a/backend/docs/SSO.md b/backend/docs/SSO.md index dd92a335d5..7040afb142 100644 --- a/backend/docs/SSO.md +++ b/backend/docs/SSO.md @@ -137,12 +137,23 @@ providers: issuer: "https://provider.example.com" client_id: "..." client_secret: $SECRET - authorization_endpoint: "https://provider.example.com/oauth2/authorize" + authorization_endpoint: "https://provider.example.com/oauth2/authorize?prompt=login" token_endpoint: "https://provider.example.com/oauth2/token" userinfo_endpoint: "https://provider.example.com/oauth2/userinfo" jwks_uri: "https://provider.example.com/oauth2/jwks" ``` +Authorization endpoints may include provider query parameters, such as +`prompt=login` in the example above. DeerFlow preserves existing encoded query +values, blank values and repeated parameters when appending the login request +parameters. This applies to both discovered endpoints and configured overrides. + +Do not include DeerFlow-generated parameters (`response_type`, `client_id`, +`redirect_uri`, `scope`, `state`, `nonce`, `code_challenge` or +`code_challenge_method`) in the endpoint query. Appending does not replace +existing keys, and the identity provider decides how duplicate parameters are +resolved. + ## Local Keycloak Example This section walks through setting up a local Keycloak instance with Podman or Docker for development. @@ -277,4 +288,4 @@ The frontend handles the post-SSO flow at `/auth/callback`: - **UserInfo sub check** ensures the UserInfo response matches the ID token subject - **Reject alg=none** — ID tokens with algorithm "none" are always rejected - **No email auto-linking** — SSO accounts are always separate from email/password accounts. An email collision with an existing local account blocks the SSO login (409) rather than merging the two. -- **Verified email requirement** — SSO users must have verified emails by default \ No newline at end of file +- **Verified email requirement** — SSO users must have verified emails by default diff --git a/backend/tests/test_oidc_auth.py b/backend/tests/test_oidc_auth.py index 9d9f9021c8..4195a720d0 100644 --- a/backend/tests/test_oidc_auth.py +++ b/backend/tests/test_oidc_auth.py @@ -1,5 +1,6 @@ import time from unittest.mock import AsyncMock +from urllib.parse import parse_qs, urlsplit import jwt import pytest @@ -34,6 +35,110 @@ def _identity(**overrides): return OIDCIdentity(**values) +@pytest.mark.asyncio +@pytest.mark.parametrize( + "endpoint_query", + [ + "", + "?", + "?prompt=login", + "?login_hint=user%2Btag%40example.com&empty=&resource=first&resource=second", + ], +) +@pytest.mark.parametrize("with_pkce_and_nonce", [False, True]) +async def test_oidc_authorization_url_preserves_endpoint_query(endpoint_query, with_pkce_and_nonce): + service = OIDCService() + metadata = OIDCMetadata( + issuer="https://issuer.example.com", + authorization_endpoint=f"https://issuer.example.com/auth{endpoint_query}", + token_endpoint="https://issuer.example.com/token", + userinfo_endpoint=None, + jwks_uri="https://issuer.example.com/jwks", + ) + try: + url = service.build_authorization_url( + metadata=metadata, + client_id="deer-flow", + redirect_uri="https://app.example.com/api/auth/callback/test", + scopes=["openid", "email"], + state="state+with&reserved=characters", + nonce="test-nonce" if with_pkce_and_nonce else None, + code_challenge="test-challenge" if with_pkce_and_nonce else None, + ) + finally: + await service.close() + + parsed = urlsplit(url) + expected = parse_qs(endpoint_query.lstrip("?"), keep_blank_values=True) + expected.update( + response_type=["code"], + client_id=["deer-flow"], + redirect_uri=["https://app.example.com/api/auth/callback/test"], + scope=["openid email"], + state=["state+with&reserved=characters"], + ) + if with_pkce_and_nonce: + expected.update(nonce=["test-nonce"], code_challenge=["test-challenge"], code_challenge_method=["S256"]) + assert (parsed.scheme, parsed.netloc, parsed.path) == ("https", "issuer.example.com", "/auth") + assert parse_qs(parsed.query, keep_blank_values=True) == expected + if endpoint_query.lstrip("?"): + assert parsed.query.startswith(endpoint_query[1:] + "&") + + +@pytest.mark.asyncio +async def test_oidc_login_redirect_preserves_configured_endpoint_query(monkeypatch): + from types import SimpleNamespace + + import httpx + from fastapi import FastAPI + + import deerflow.config.app_config as app_config_module + from app.gateway.auth import oidc_state + from app.gateway.auth.config import AuthConfig + from app.gateway.routers import auth as auth_router + + service = OIDCService() + provider = _provider_config(authorization_endpoint="https://issuer.example.com/auth?prompt=login") + config = SimpleNamespace(auth=SimpleNamespace(oidc=SimpleNamespace(enabled=True, providers={"keycloak": provider}))) + monkeypatch.setattr(app_config_module, "get_app_config", lambda: config) + monkeypatch.setattr(auth_router, "_get_oidc_service", lambda: service) + monkeypatch.setattr(oidc_state, "get_auth_config", lambda: AuthConfig(jwt_secret="test-secret-for-oidc-query-parameters-min-32")) + + async def discovery_get(url): + assert url == "https://issuer.example.com/.well-known/openid-configuration" + return httpx.Response( + 200, + request=httpx.Request("GET", url), + json={ + "issuer": provider.issuer, + "authorization_endpoint": "https://issuer.example.com/auth", + "token_endpoint": "https://issuer.example.com/token", + "jwks_uri": "https://issuer.example.com/jwks", + }, + ) + + monkeypatch.setattr(service._http, "get", discovery_get) + app = FastAPI() + app.include_router(auth_router.router) + try: + async with httpx.AsyncClient(transport=httpx.ASGITransport(app), base_url="https://app.example.com") as client: + response = await client.get("/api/v1/auth/oauth/keycloak", follow_redirects=False) + finally: + await service.close() + + assert response.status_code == 302 + query = parse_qs(urlsplit(response.headers["location"]).query) + assert query["prompt"] == ["login"] + assert query["response_type"] == ["code"] + assert query["client_id"] == [provider.client_id] + assert query["code_challenge_method"] == ["S256"] + state = oidc_state._verify_state_signed(response.cookies["df_oidc_state_keycloak"]) + assert state is not None + assert query["state"] == [state.state] + assert query["nonce"] == [state.nonce] + assert query["code_challenge"] == [oidc_state.compute_code_challenge(state.code_verifier)] + + @pytest.mark.asyncio async def test_oidc_existing_local_account_blocks_sso_login_even_when_unverified(): local_user = User(email="user@example.com", password_hash="hash")