mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-10-26 13:16:24 +00:00
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 <willem.jiang@gmail.com>
This commit is contained in:
parent
eecfebd419
commit
62a5b6cc26
@ -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.
|
||||
|
||||
@ -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`).
|
||||
|
||||
|
||||
@ -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 ─────────────────────────────────────────────────────
|
||||
|
||||
|
||||
@ -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
|
||||
- **Verified email requirement** — SSO users must have verified emails by default
|
||||
|
||||
@ -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")
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user