From 1eb35b3cab41013a46102b3eba73e51610f22297 Mon Sep 17 00:00:00 2001 From: Alexandr Basiuk Date: Sun, 17 May 2026 18:58:56 +0300 Subject: [PATCH 1/6] test(oidc): add failing tests for enterprise-sso-004 OIDC SSO login Covers /api/v1/auth/config, /auth/oauth/oidc/login, and /auth/oauth/oidc/callback behaviors: disabled-by-default 503s, discovery + authorize redirect, callback upsert (new + existing user by email), no-email rejection, token-endpoint failure propagation. respx mocks the OIDC provider; no live Authelia needed. Implementation follows in the next commit. --- backend/tests/api/test_oidc.py | 285 +++++++++++++++++++++++++++++++++ 1 file changed, 285 insertions(+) create mode 100644 backend/tests/api/test_oidc.py diff --git a/backend/tests/api/test_oidc.py b/backend/tests/api/test_oidc.py new file mode 100644 index 0000000..f6a7c2f --- /dev/null +++ b/backend/tests/api/test_oidc.py @@ -0,0 +1,285 @@ +"""Tests for generic OIDC SSO login (Authelia, Keycloak, Authentik, etc.). + +The flow mirrors the existing Google OAuth router in app/api/v1/oauth_stub.py +but is provider-agnostic — endpoints are discovered from +{OIDC_ISSUER_URL}/.well-known/openid-configuration and configured via: + + OIDC_ISSUER_URL → e.g. https://auth.example.com + OIDC_CLIENT_ID + OIDC_CLIENT_SECRET + OIDC_REDIRECT_URI → backend callback + OIDC_SCOPES → defaults to "openid email profile" + OIDC_PROVIDER_NAME → display name shown on the login button + +When any of the first three are missing both endpoints return 503 so the SPA +can fall back to email/password. +""" +from urllib.parse import parse_qs, urlparse + +import pytest +import respx +from httpx import Response +from sqlalchemy import select + +from app.core.config import settings +from app.models.user import User + + +# --------------------------------------------------------------------------- +# Helpers +# --------------------------------------------------------------------------- + +ISSUER = "https://auth.example.com" +DISCOVERY_URL = f"{ISSUER}/.well-known/openid-configuration" +AUTH_URL = f"{ISSUER}/api/oidc/authorize" +TOKEN_URL = f"{ISSUER}/api/oidc/token" +USERINFO_URL = f"{ISSUER}/api/oidc/userinfo" + + +def _discovery_doc() -> dict: + return { + "issuer": ISSUER, + "authorization_endpoint": AUTH_URL, + "token_endpoint": TOKEN_URL, + "userinfo_endpoint": USERINFO_URL, + } + + +@pytest.fixture +def oidc_enabled(monkeypatch): + """Configure OIDC settings so _oidc_enabled() returns True.""" + monkeypatch.setattr(settings, "oidc_issuer_url", ISSUER) + monkeypatch.setattr(settings, "oidc_client_id", "test-client-id") + monkeypatch.setattr(settings, "oidc_client_secret", "test-client-secret") + monkeypatch.setattr( + settings, + "oidc_redirect_uri", + "http://localhost:8000/api/v1/auth/oauth/oidc/callback", + ) + monkeypatch.setattr(settings, "oidc_scopes", "openid email profile") + monkeypatch.setattr(settings, "oidc_provider_name", "Authelia") + # Discovery cache must be reset between configs since it keys on issuer. + from app.api.v1 import oidc as oidc_module + + oidc_module._discovery_cache.clear() + yield + oidc_module._discovery_cache.clear() + + +@pytest.fixture +def oidc_disabled(monkeypatch): + """Force all OIDC settings to None so the feature is off.""" + monkeypatch.setattr(settings, "oidc_issuer_url", None) + monkeypatch.setattr(settings, "oidc_client_id", None) + monkeypatch.setattr(settings, "oidc_client_secret", None) + + +# --------------------------------------------------------------------------- +# /api/v1/auth/config +# --------------------------------------------------------------------------- + + +async def test_auth_config_when_oidc_disabled(client, oidc_disabled, monkeypatch): + """Config endpoint reports OIDC off when issuer not set.""" + monkeypatch.setattr(settings, "google_client_id", None) + monkeypatch.setattr(settings, "google_client_secret", None) + + resp = await client.get("/api/v1/auth/config") + + assert resp.status_code == 200 + body = resp.json() + assert body["oidc_enabled"] is False + assert body["google_enabled"] is False + + +async def test_auth_config_when_oidc_enabled(client, oidc_enabled): + """Config endpoint reports OIDC on and surfaces the display name.""" + resp = await client.get("/api/v1/auth/config") + + assert resp.status_code == 200 + body = resp.json() + assert body["oidc_enabled"] is True + assert body["oidc_provider_name"] == "Authelia" + + +# --------------------------------------------------------------------------- +# /api/v1/auth/oauth/oidc/login +# --------------------------------------------------------------------------- + + +async def test_oidc_login_returns_503_when_not_configured(client, oidc_disabled): + """Login endpoint is unavailable until OIDC creds are provisioned.""" + resp = await client.get( + "/api/v1/auth/oauth/oidc/login", follow_redirects=False + ) + assert resp.status_code == 503 + + +@respx.mock +async def test_oidc_login_redirects_to_authorization_endpoint( + client, oidc_enabled +): + """Login fetches discovery and 302s to the provider's authorize URL with + the expected OAuth2/OIDC query params.""" + respx.get(DISCOVERY_URL).mock(return_value=Response(200, json=_discovery_doc())) + + resp = await client.get( + "/api/v1/auth/oauth/oidc/login", follow_redirects=False + ) + + assert resp.status_code in (302, 307) + location = resp.headers["location"] + parsed = urlparse(location) + assert f"{parsed.scheme}://{parsed.netloc}{parsed.path}" == AUTH_URL + + qs = parse_qs(parsed.query) + assert qs["client_id"] == ["test-client-id"] + assert qs["redirect_uri"] == [ + "http://localhost:8000/api/v1/auth/oauth/oidc/callback" + ] + assert qs["response_type"] == ["code"] + assert "openid" in qs["scope"][0] + + +# --------------------------------------------------------------------------- +# /api/v1/auth/oauth/oidc/callback +# --------------------------------------------------------------------------- + + +async def test_oidc_callback_returns_503_when_not_configured( + client, oidc_disabled +): + resp = await client.get( + "/api/v1/auth/oauth/oidc/callback?code=any", follow_redirects=False + ) + assert resp.status_code == 503 + + +@respx.mock +async def test_oidc_callback_creates_new_user_and_returns_tokens( + client, db, oidc_enabled, monkeypatch +): + """Happy path: code → tokens → userinfo → upsert → 302 with app JWTs in + URL fragment. New user gets auth_provider=oidc + a personal workspace.""" + monkeypatch.setattr(settings, "frontend_url", "http://localhost:5173") + + # Clean slate — conftest's `db` fixture truncates `users`, so the email + # we use below is guaranteed not to exist. + new_email = "new-oidc-user@example.com" + + respx.get(DISCOVERY_URL).mock(return_value=Response(200, json=_discovery_doc())) + respx.post(TOKEN_URL).mock( + return_value=Response( + 200, + json={ + "access_token": "provider-access-token", + "id_token": "provider-id-token", + "token_type": "Bearer", + "expires_in": 3600, + }, + ) + ) + respx.get(USERINFO_URL).mock( + return_value=Response( + 200, + json={ + "sub": "auth0|abc123", + "email": new_email, + "name": "New OIDC User", + }, + ) + ) + + resp = await client.get( + "/api/v1/auth/oauth/oidc/callback?code=test-code", + follow_redirects=False, + ) + + assert resp.status_code in (302, 307) + location = resp.headers["location"] + assert location.startswith("http://localhost:5173/auth/callback#") + fragment = location.split("#", 1)[1] + frag_params = parse_qs(fragment) + assert "access_token" in frag_params + assert "refresh_token" in frag_params + assert frag_params["access_token"][0] # non-empty + assert frag_params["refresh_token"][0] + + # User row created with oidc provider tag. + result = await db.execute(select(User).where(User.email == new_email)) + user = result.scalar_one() + assert user.auth_provider == "oidc" + assert user.name == "New OIDC User" + + +@respx.mock +async def test_oidc_callback_reuses_existing_user_by_email( + client, db, user, oidc_enabled, monkeypatch +): + """If a user with the OIDC-returned email already exists, callback returns + tokens for that user instead of erroring on the unique-email constraint.""" + monkeypatch.setattr(settings, "frontend_url", "http://localhost:5173") + + respx.get(DISCOVERY_URL).mock(return_value=Response(200, json=_discovery_doc())) + respx.post(TOKEN_URL).mock( + return_value=Response(200, json={"access_token": "pa"}) + ) + respx.get(USERINFO_URL).mock( + return_value=Response( + 200, json={"sub": "x", "email": user.email, "name": user.name} + ) + ) + + resp = await client.get( + "/api/v1/auth/oauth/oidc/callback?code=test-code", + follow_redirects=False, + ) + + assert resp.status_code in (302, 307) + + # No duplicate row. + result = await db.execute(select(User).where(User.email == user.email)) + rows = result.scalars().all() + assert len(rows) == 1 + assert rows[0].id == user.id + + +@respx.mock +async def test_oidc_callback_rejects_userinfo_without_email( + client, oidc_enabled +): + """Provider must return an email claim — anything else is rejected with + 400 rather than creating a userless row.""" + respx.get(DISCOVERY_URL).mock(return_value=Response(200, json=_discovery_doc())) + respx.post(TOKEN_URL).mock( + return_value=Response(200, json={"access_token": "pa"}) + ) + respx.get(USERINFO_URL).mock( + return_value=Response(200, json={"sub": "x"}) # no email + ) + + resp = await client.get( + "/api/v1/auth/oauth/oidc/callback?code=test-code", + follow_redirects=False, + ) + + assert resp.status_code == 400 + + +@respx.mock +async def test_oidc_callback_propagates_token_endpoint_failure( + client, oidc_enabled +): + """If the token exchange fails the user sees a 400 rather than a 500 — + callback must surface provider errors instead of crashing.""" + respx.get(DISCOVERY_URL).mock(return_value=Response(200, json=_discovery_doc())) + respx.post(TOKEN_URL).mock( + return_value=Response(400, json={"error": "invalid_grant"}) + ) + + resp = await client.get( + "/api/v1/auth/oauth/oidc/callback?code=bad-code", + follow_redirects=False, + ) + + assert resp.status_code == 400 From e3ca24235c2ea2a4125258ec91a4a519ef5fd0c5 Mon Sep 17 00:00:00 2001 From: Alexandr Basiuk Date: Sun, 17 May 2026 19:02:06 +0300 Subject: [PATCH 2/6] =?UTF-8?q?feat(auth):=20generic=20OIDC=20SSO=20(Authe?= =?UTF-8?q?lia,=20Keycloak,=20Authentik,=20Okta,=20=E2=80=A6)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes GitHub issue #18 (enterprise-sso-004). Backend: - New `app/api/v1/oidc.py` router with GET /api/v1/auth/oauth/oidc/login → 302 to provider's authorize endpoint GET /api/v1/auth/oauth/oidc/callback → token exchange + userinfo + upsert + 302 to /auth/callback#tokens Endpoints are discovered from {issuer}/.well-known/openid-configuration and cached per-process. Falls back to 503 when OIDC_* env vars are unset so the SPA can hide the button. - Mirrors the Google OAuth pattern in oauth_stub.py for consistency: same fragment-based token delivery, same email-keyed user upsert, same personal-workspace bootstrap on first login. New users land with auth_provider="oidc". - Settings: OIDC_ISSUER_URL, OIDC_CLIENT_ID, OIDC_CLIENT_SECRET, OIDC_REDIRECT_URI, OIDC_SCOPES, OIDC_PROVIDER_NAME (display name). - New GET /api/v1/auth/config (public) returns {google_enabled, oidc_enabled, oidc_provider_name} so the SPA renders only the SSO buttons whose backends are actually configured. Frontend: - AuthPage fetches /auth/config on mount and conditionally renders the Google and/or OIDC buttons. The OIDC button label uses oidc_provider_name ("Continue with Authelia", "Continue with Keycloak"…). - The "or" divider only renders when at least one SSO option is available. Out of scope (separate tasks): - OIDC group/role mapping into ArchFlow roles - CSRF state cookie (matches existing Google flow — track as a security hardening pass across both providers later) --- .env.example | 12 ++ backend/app/api/v1/auth.py | 19 +++ backend/app/api/v1/oidc.py | 145 ++++++++++++++++++++++ backend/app/core/config.py | 13 ++ backend/app/main.py | 2 + backend/tests/api/test_oidc.py | 1 - frontend/src/components/auth/AuthPage.tsx | 69 +++++++--- 7 files changed, 245 insertions(+), 16 deletions(-) create mode 100644 backend/app/api/v1/oidc.py diff --git a/.env.example b/.env.example index fc6c55a..8c967eb 100644 --- a/.env.example +++ b/.env.example @@ -28,6 +28,18 @@ GOOGLE_CLIENT_SECRET= GOOGLE_REDIRECT_URI=http://localhost:8000/api/v1/auth/oauth/google/callback FRONTEND_URL=http://localhost:5173 +# Generic OIDC SSO (Authelia, Keycloak, Authentik, Okta, etc.) +# Leave OIDC_ISSUER_URL / OIDC_CLIENT_ID / OIDC_CLIENT_SECRET blank to hide +# the SSO button. Endpoints are auto-discovered from +# {OIDC_ISSUER_URL}/.well-known/openid-configuration. +OIDC_ISSUER_URL= +OIDC_CLIENT_ID= +OIDC_CLIENT_SECRET= +OIDC_REDIRECT_URI=http://localhost:8000/api/v1/auth/oauth/oidc/callback +OIDC_SCOPES=openid email profile +# Display name shown on the login button ("Continue with "). +OIDC_PROVIDER_NAME=SSO + # ============================================================================= # REQUIRED: Agent platform encryption key # ============================================================================= diff --git a/backend/app/api/v1/auth.py b/backend/app/api/v1/auth.py index 6e1ae64..e66a074 100644 --- a/backend/app/api/v1/auth.py +++ b/backend/app/api/v1/auth.py @@ -3,6 +3,7 @@ from sqlalchemy.ext.asyncio import AsyncSession from app.api.deps import get_current_user +from app.core.config import settings from app.core.database import get_db from app.core.security import ( create_access_token, @@ -18,6 +19,24 @@ router = APIRouter(prefix="/auth", tags=["auth"]) +@router.get("/config") +async def auth_config(): + """Public — tells the SPA which SSO buttons to render. + + Read on the login page so we can hide buttons whose backend creds are + blank instead of showing buttons that 503. + """ + return { + "google_enabled": bool(settings.google_client_id and settings.google_client_secret), + "oidc_enabled": bool( + settings.oidc_issuer_url + and settings.oidc_client_id + and settings.oidc_client_secret + ), + "oidc_provider_name": settings.oidc_provider_name, + } + + @router.post("/register", response_model=TokenResponse, status_code=201) async def register(data: RegisterRequest, db: AsyncSession = Depends(get_db)): existing = await db.execute(select(User).where(User.email == data.email)) diff --git a/backend/app/api/v1/oidc.py b/backend/app/api/v1/oidc.py new file mode 100644 index 0000000..ed56503 --- /dev/null +++ b/backend/app/api/v1/oidc.py @@ -0,0 +1,145 @@ +"""Generic OIDC SSO — Authorization Code flow. + + GET /api/v1/auth/oauth/oidc/login + → 302 to the provider's authorization endpoint (or 503 if not configured). + GET /api/v1/auth/oauth/oidc/callback?code=... + → exchange code → userinfo → upsert user → issue app JWTs + → 302 to frontend /auth/callback with tokens in URL fragment. + +Works with any OIDC-compliant provider (Authelia, Keycloak, Authentik, Okta, +Google, etc.). Endpoints are discovered from +``{OIDC_ISSUER_URL}/.well-known/openid-configuration``; we cache the document +in-process per issuer so we don't hammer the IdP on every login click. + +Configured via OIDC_* env vars in app/core/config.py. When any of issuer_url, +client_id, or client_secret is missing both endpoints return 503 so the SPA +can fall back to email/password. + +Mirrors the Google OAuth pattern in oauth_stub.py (same user upsert, same +fragment-based token delivery) — chose composition over abstraction to keep +each provider's quirks contained. +""" +from urllib.parse import urlencode + +import httpx +from fastapi import APIRouter, Depends, HTTPException, Query +from fastapi.responses import RedirectResponse +from sqlalchemy import select +from sqlalchemy.ext.asyncio import AsyncSession + +from app.core.config import settings +from app.core.database import get_db +from app.core.security import create_access_token, create_refresh_token, hash_password +from app.models.user import User +from app.services import workspace_service + +router = APIRouter(prefix="/auth/oauth", tags=["oauth"]) + +# Discovery doc cache keyed by issuer URL. OIDC discovery responses are stable +# and the IdP signals rotation via the keys endpoint, not this doc — caching +# for the process lifetime is the standard pattern. Cleared by tests. +_discovery_cache: dict[str, dict] = {} + + +def _oidc_enabled() -> bool: + return bool( + settings.oidc_issuer_url + and settings.oidc_client_id + and settings.oidc_client_secret + ) + + +async def _get_discovery(client: httpx.AsyncClient) -> dict: + issuer = settings.oidc_issuer_url + cached = _discovery_cache.get(issuer) + if cached is not None: + return cached + url = f"{issuer.rstrip('/')}/.well-known/openid-configuration" + resp = await client.get(url) + if resp.status_code != 200: + raise HTTPException(502, f"OIDC discovery failed: {resp.status_code}") + doc = resp.json() + _discovery_cache[issuer] = doc + return doc + + +@router.get("/oidc/login") +async def oidc_login(): + if not _oidc_enabled(): + raise HTTPException(503, "OIDC not configured") + async with httpx.AsyncClient(timeout=10) as client: + disc = await _get_discovery(client) + qs = urlencode({ + "client_id": settings.oidc_client_id, + "redirect_uri": settings.oidc_redirect_uri, + "response_type": "code", + "scope": settings.oidc_scopes, + }) + return RedirectResponse(f"{disc['authorization_endpoint']}?{qs}") + + +@router.get("/oidc/callback") +async def oidc_callback( + code: str = Query(...), + db: AsyncSession = Depends(get_db), +): + if not _oidc_enabled(): + raise HTTPException(503, "OIDC not configured") + + async with httpx.AsyncClient(timeout=10) as client: + disc = await _get_discovery(client) + + token_resp = await client.post( + disc["token_endpoint"], + data={ + "code": code, + "client_id": settings.oidc_client_id, + "client_secret": settings.oidc_client_secret, + "redirect_uri": settings.oidc_redirect_uri, + "grant_type": "authorization_code", + }, + ) + if token_resp.status_code != 200: + raise HTTPException(400, f"OIDC token exchange failed: {token_resp.text}") + provider_access = token_resp.json().get("access_token") + if not provider_access: + raise HTTPException(400, "OIDC token response missing access_token") + + ui_resp = await client.get( + disc["userinfo_endpoint"], + headers={"Authorization": f"Bearer {provider_access}"}, + ) + if ui_resp.status_code != 200: + raise HTTPException(400, "OIDC userinfo fetch failed") + info = ui_resp.json() + + email = info.get("email") + if not email: + raise HTTPException(400, "OIDC account returned no email claim") + name = info.get("name") or email.split("@")[0].title() + + existing = ( + await db.execute(select(User).where(User.email == email)) + ).scalar_one_or_none() + + if existing is None: + user = User( + email=email, + name=name, + # Random hash no one can log in with — they must keep using SSO. + password_hash=hash_password("oidc-only:" + email), + auth_provider="oidc", + ) + db.add(user) + await db.flush() + await db.refresh(user) + await workspace_service.create_personal_workspace(db, user) + else: + user = existing + + # Fragment-based delivery so tokens never show up in server access logs. + frag = urlencode({ + "access_token": create_access_token(str(user.id)), + "refresh_token": create_refresh_token(str(user.id)), + }) + return RedirectResponse(f"{settings.frontend_url}/auth/callback#{frag}") diff --git a/backend/app/core/config.py b/backend/app/core/config.py index 6780bb4..de51908 100644 --- a/backend/app/core/config.py +++ b/backend/app/core/config.py @@ -35,6 +35,19 @@ class Settings(BaseSettings): google_redirect_uri: str = "http://localhost:8000/api/v1/auth/oauth/google/callback" frontend_url: str = "http://localhost:5173" + # Generic OIDC SSO (opt-in — works with Authelia, Keycloak, Authentik, + # Okta, etc.). Endpoints are discovered from + # {OIDC_ISSUER_URL}/.well-known/openid-configuration at request time. + # Leave issuer/client_id/secret blank to hide the SSO button. + oidc_issuer_url: str | None = None + oidc_client_id: str | None = None + oidc_client_secret: str | None = None + oidc_redirect_uri: str = "http://localhost:8000/api/v1/auth/oauth/oidc/callback" + oidc_scopes: str = "openid email profile" + # Display name shown on the "Continue with …" button. Defaults to a + # generic label; set to "Authelia", "Keycloak", "Okta", etc. to brand it. + oidc_provider_name: str = "SSO" + # Agent platform — Fernet key for encrypting workspace LLM provider keys + Langfuse keys. # Must be a 32-byte url-safe base64-encoded string (44 chars). # Generate: python -c "from cryptography.fernet import Fernet; print(Fernet.generate_key().decode())" # noqa: E501 diff --git a/backend/app/main.py b/backend/app/main.py index 824a39d..3152402 100644 --- a/backend/app/main.py +++ b/backend/app/main.py @@ -23,6 +23,7 @@ from app.api.v1.notifications import router as notifications_router from app.api.v1.oauth_stub import router as oauth_router from app.api.v1.objects import router as objects_router +from app.api.v1.oidc import router as oidc_router from app.api.v1.packs import router as packs_router from app.api.v1.repos import router as repos_router from app.api.v1.teams import router as teams_router @@ -96,6 +97,7 @@ def create_app() -> FastAPI: app.include_router(technologies_router, prefix="/api/v1") app.include_router(diagram_access_router, prefix="/api/v1") app.include_router(oauth_router, prefix="/api/v1") + app.include_router(oidc_router, prefix="/api/v1") app.include_router(invites_router, prefix="/api/v1") app.include_router(my_invites_router, prefix="/api/v1") app.include_router(versions_router, prefix="/api/v1") diff --git a/backend/tests/api/test_oidc.py b/backend/tests/api/test_oidc.py index f6a7c2f..9367388 100644 --- a/backend/tests/api/test_oidc.py +++ b/backend/tests/api/test_oidc.py @@ -24,7 +24,6 @@ from app.core.config import settings from app.models.user import User - # --------------------------------------------------------------------------- # Helpers # --------------------------------------------------------------------------- diff --git a/frontend/src/components/auth/AuthPage.tsx b/frontend/src/components/auth/AuthPage.tsx index 94a0705..46eb5a3 100644 --- a/frontend/src/components/auth/AuthPage.tsx +++ b/frontend/src/components/auth/AuthPage.tsx @@ -1,7 +1,13 @@ -import { useState } from 'react' +import { useEffect, useState } from 'react' import axios from 'axios' import { useAuthStore } from '../../stores/auth-store' +type AuthConfig = { + google_enabled: boolean + oidc_enabled: boolean + oidc_provider_name: string +} + export function AuthPage() { const [isLogin, setIsLogin] = useState(true) const [email, setEmail] = useState('') @@ -9,8 +15,18 @@ export function AuthPage() { const [password, setPassword] = useState('') const [error, setError] = useState('') const [loading, setLoading] = useState(false) + const [config, setConfig] = useState(null) const { setTokens } = useAuthStore() + useEffect(() => { + // Fire-and-forget — if the call fails (offline, backend down) the SSO + // buttons just stay hidden. Email/password login still works. + axios + .get('/api/v1/auth/config') + .then((r) => setConfig(r.data)) + .catch(() => setConfig({ google_enabled: false, oidc_enabled: false, oidc_provider_name: 'SSO' })) + }, []) + const handleSubmit = async (e: React.FormEvent) => { e.preventDefault() setError('') @@ -39,6 +55,12 @@ export function AuthPage() { window.location.href = '/api/v1/auth/oauth/google/login' } + const handleOidcLogin = () => { + window.location.href = '/api/v1/auth/oauth/oidc/login' + } + + const showDivider = config?.google_enabled || config?.oidc_enabled + return (
@@ -87,20 +109,37 @@ export function AuthPage() { -
-
- or -
-
- - + {showDivider && ( +
+
+ or +
+
+ )} + + {config?.google_enabled && ( + + )} + + {config?.oidc_enabled && ( + + )}

{isLogin ? "Don't have an account?" : 'Already have an account?'}{' '} From 95a031a35f31b2ae44adc5492853b49b78fd5318 Mon Sep 17 00:00:00 2001 From: Alexandr Basiuk Date: Sun, 17 May 2026 19:10:24 +0300 Subject: [PATCH 3/6] docs(readme): mark AI agents, per-diagram export, and OIDC SSO as shipped AI agents (#14) and per-diagram export (#15) already landed on main; OIDC SSO ships in this PR. --- README.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 96e4221..74c2078 100644 --- a/README.md +++ b/README.md @@ -329,10 +329,11 @@ make dev-infra && make db-upgrade - [x] API keys + webhooks - [x] Packs, pinned, search - [x] Per-user undo & redo (Phase 1) +- [x] AI agents (supervisor + GitHub repo researcher, Langfuse tracing) +- [x] Per-diagram export — Mermaid / PlantUML / Structurizr DSL / JSON +- [x] SSO (OIDC — Authelia, Keycloak, Authentik, Okta, …) - [ ] Per-user undo — stale-detection (Phase 2) - [ ] Import from Structurizr DSL -- [ ] Export to Mermaid / PlantUML -- [ ] SSO (OIDC) - [ ] Deployment diagrams (C4 L4) See [`docs/architecture/`](docs/architecture/) for ADRs and ongoing design. From 5408da29e9e14999342d8794e8e228391b45fed4 Mon Sep 17 00:00:00 2001 From: Alexandr Basiuk Date: Sun, 17 May 2026 19:47:46 +0300 Subject: [PATCH 4/6] test(oidc): scope respx to issuer base_url and drop db fixture from HTTP tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CI hung indefinitely on the previous version (33+ min on a job that normally finishes in ~2 min). Two issues, both about fixture/transport interaction: 1. `@respx.mock` with no base_url intercepts ALL httpx clients, including the test client's ASGITransport. Outbound requests to the fake provider were matched; the inbound test-client request to /api/v1/auth/... was silently swallowed and never reached FastAPI, hanging the test. Fix: scope each block with `respx.mock(base_url=ISSUER, ...)` so only provider URLs are intercepted. 2. Tests took both `client` AND `db` fixtures. Holding the fixture's AsyncSession open while the ASGI handler opens its own via `get_db` deadlocks (or reads stale state) because Postgres row locks from the handler's commit can't be observed by the long-lived fixture session. No other test in the suite mixes these two — that's the convention. Fix: drop `db` from HTTP tests, open a fresh `async_session()` after the response to verify DB state. Use uuid-suffixed emails so tests don't depend on table truncation order. Same 8 behaviors covered as before. --- backend/tests/api/test_oidc.py | 182 ++++++++++++++++++--------------- 1 file changed, 101 insertions(+), 81 deletions(-) diff --git a/backend/tests/api/test_oidc.py b/backend/tests/api/test_oidc.py index 9367388..0c8975a 100644 --- a/backend/tests/api/test_oidc.py +++ b/backend/tests/api/test_oidc.py @@ -13,7 +13,18 @@ When any of the first three are missing both endpoints return 503 so the SPA can fall back to email/password. + +Implementation notes +-------------------- +* respx is scoped to ``base_url=ISSUER`` so it ONLY intercepts outbound calls + to the fake provider. Without that scope, respx swallows the test client's + ASGI requests too and the tests hang indefinitely on CI. +* The HTTP-level tests do NOT take the ``db`` fixture. Holding the fixture's + session open while the ASGI handler opens its own via ``get_db`` causes the + in-test verification query to either deadlock or read stale data. Instead + we open a fresh ``async_session`` after the call to assert on DB state. """ +import uuid from urllib.parse import parse_qs, urlparse import pytest @@ -22,6 +33,7 @@ from sqlalchemy import select from app.core.config import settings +from app.core.database import async_session from app.models.user import User # --------------------------------------------------------------------------- @@ -29,7 +41,7 @@ # --------------------------------------------------------------------------- ISSUER = "https://auth.example.com" -DISCOVERY_URL = f"{ISSUER}/.well-known/openid-configuration" +DISCOVERY_PATH = "/.well-known/openid-configuration" AUTH_URL = f"{ISSUER}/api/oidc/authorize" TOKEN_URL = f"{ISSUER}/api/oidc/token" USERINFO_URL = f"{ISSUER}/api/oidc/userinfo" @@ -57,7 +69,9 @@ def oidc_enabled(monkeypatch): ) monkeypatch.setattr(settings, "oidc_scopes", "openid email profile") monkeypatch.setattr(settings, "oidc_provider_name", "Authelia") - # Discovery cache must be reset between configs since it keys on issuer. + monkeypatch.setattr(settings, "frontend_url", "http://localhost:5173") + # Discovery cache is keyed on issuer; clear it so each test sees a fresh + # fetch path. from app.api.v1 import oidc as oidc_module oidc_module._discovery_cache.clear() @@ -73,6 +87,30 @@ def oidc_disabled(monkeypatch): monkeypatch.setattr(settings, "oidc_client_secret", None) +def _mock_provider(*, token_status: int = 200, userinfo: dict | None = None): + """Register the three OIDC endpoints on the active respx router. Caller + must already be inside a ``with respx.mock(base_url=ISSUER)`` block.""" + respx.get(DISCOVERY_PATH).mock( + return_value=Response(200, json=_discovery_doc()) + ) + if token_status == 200: + respx.post(TOKEN_URL).mock( + return_value=Response(200, json={"access_token": "provider-access"}) + ) + else: + respx.post(TOKEN_URL).mock( + return_value=Response(token_status, json={"error": "invalid_grant"}) + ) + if userinfo is not None: + respx.get(USERINFO_URL).mock(return_value=Response(200, json=userinfo)) + + +async def _fetch_user_by_email(email: str) -> User | None: + async with async_session() as session: + result = await session.execute(select(User).where(User.email == email)) + return result.scalar_one_or_none() + + # --------------------------------------------------------------------------- # /api/v1/auth/config # --------------------------------------------------------------------------- @@ -114,17 +152,19 @@ async def test_oidc_login_returns_503_when_not_configured(client, oidc_disabled) assert resp.status_code == 503 -@respx.mock async def test_oidc_login_redirects_to_authorization_endpoint( client, oidc_enabled ): """Login fetches discovery and 302s to the provider's authorize URL with the expected OAuth2/OIDC query params.""" - respx.get(DISCOVERY_URL).mock(return_value=Response(200, json=_discovery_doc())) + with respx.mock(base_url=ISSUER, assert_all_called=False): + respx.get(DISCOVERY_PATH).mock( + return_value=Response(200, json=_discovery_doc()) + ) - resp = await client.get( - "/api/v1/auth/oauth/oidc/login", follow_redirects=False - ) + resp = await client.get( + "/api/v1/auth/oauth/oidc/login", follow_redirects=False + ) assert resp.status_code in (302, 307) location = resp.headers["location"] @@ -154,131 +194,111 @@ async def test_oidc_callback_returns_503_when_not_configured( assert resp.status_code == 503 -@respx.mock async def test_oidc_callback_creates_new_user_and_returns_tokens( - client, db, oidc_enabled, monkeypatch + client, oidc_enabled ): """Happy path: code → tokens → userinfo → upsert → 302 with app JWTs in URL fragment. New user gets auth_provider=oidc + a personal workspace.""" - monkeypatch.setattr(settings, "frontend_url", "http://localhost:5173") + # Unique email per test run so we don't depend on table-truncation order. + new_email = f"oidc-new-{uuid.uuid4().hex[:10]}@example.com" - # Clean slate — conftest's `db` fixture truncates `users`, so the email - # we use below is guaranteed not to exist. - new_email = "new-oidc-user@example.com" - - respx.get(DISCOVERY_URL).mock(return_value=Response(200, json=_discovery_doc())) - respx.post(TOKEN_URL).mock( - return_value=Response( - 200, - json={ - "access_token": "provider-access-token", - "id_token": "provider-id-token", - "token_type": "Bearer", - "expires_in": 3600, - }, - ) - ) - respx.get(USERINFO_URL).mock( - return_value=Response( - 200, - json={ + with respx.mock(base_url=ISSUER, assert_all_called=False): + _mock_provider( + userinfo={ "sub": "auth0|abc123", "email": new_email, "name": "New OIDC User", - }, + } ) - ) - resp = await client.get( - "/api/v1/auth/oauth/oidc/callback?code=test-code", - follow_redirects=False, - ) + resp = await client.get( + "/api/v1/auth/oauth/oidc/callback?code=test-code", + follow_redirects=False, + ) assert resp.status_code in (302, 307) location = resp.headers["location"] assert location.startswith("http://localhost:5173/auth/callback#") fragment = location.split("#", 1)[1] frag_params = parse_qs(fragment) - assert "access_token" in frag_params - assert "refresh_token" in frag_params assert frag_params["access_token"][0] # non-empty assert frag_params["refresh_token"][0] - # User row created with oidc provider tag. - result = await db.execute(select(User).where(User.email == new_email)) - user = result.scalar_one() + # User row created with oidc provider tag (verified via a fresh session + # because the ASGI handler runs its own). + user = await _fetch_user_by_email(new_email) + assert user is not None assert user.auth_provider == "oidc" assert user.name == "New OIDC User" -@respx.mock async def test_oidc_callback_reuses_existing_user_by_email( - client, db, user, oidc_enabled, monkeypatch + client, oidc_enabled ): """If a user with the OIDC-returned email already exists, callback returns tokens for that user instead of erroring on the unique-email constraint.""" - monkeypatch.setattr(settings, "frontend_url", "http://localhost:5173") + # Seed an existing user via a fresh session, then verify the callback + # doesn't try to insert a duplicate. + existing_email = f"oidc-existing-{uuid.uuid4().hex[:10]}@example.com" + async with async_session() as session: + u = User( + email=existing_email, + name="Existing User", + password_hash="x", + auth_provider="local", + ) + session.add(u) + await session.commit() + existing_id = u.id - respx.get(DISCOVERY_URL).mock(return_value=Response(200, json=_discovery_doc())) - respx.post(TOKEN_URL).mock( - return_value=Response(200, json={"access_token": "pa"}) - ) - respx.get(USERINFO_URL).mock( - return_value=Response( - 200, json={"sub": "x", "email": user.email, "name": user.name} + with respx.mock(base_url=ISSUER, assert_all_called=False): + _mock_provider( + userinfo={"sub": "x", "email": existing_email, "name": "Existing User"} ) - ) - resp = await client.get( - "/api/v1/auth/oauth/oidc/callback?code=test-code", - follow_redirects=False, - ) + resp = await client.get( + "/api/v1/auth/oauth/oidc/callback?code=test-code", + follow_redirects=False, + ) assert resp.status_code in (302, 307) - # No duplicate row. - result = await db.execute(select(User).where(User.email == user.email)) - rows = result.scalars().all() + async with async_session() as session: + result = await session.execute( + select(User).where(User.email == existing_email) + ) + rows = result.scalars().all() assert len(rows) == 1 - assert rows[0].id == user.id + assert rows[0].id == existing_id -@respx.mock async def test_oidc_callback_rejects_userinfo_without_email( client, oidc_enabled ): """Provider must return an email claim — anything else is rejected with 400 rather than creating a userless row.""" - respx.get(DISCOVERY_URL).mock(return_value=Response(200, json=_discovery_doc())) - respx.post(TOKEN_URL).mock( - return_value=Response(200, json={"access_token": "pa"}) - ) - respx.get(USERINFO_URL).mock( - return_value=Response(200, json={"sub": "x"}) # no email - ) + with respx.mock(base_url=ISSUER, assert_all_called=False): + _mock_provider(userinfo={"sub": "x"}) # no email - resp = await client.get( - "/api/v1/auth/oauth/oidc/callback?code=test-code", - follow_redirects=False, - ) + resp = await client.get( + "/api/v1/auth/oauth/oidc/callback?code=test-code", + follow_redirects=False, + ) assert resp.status_code == 400 -@respx.mock async def test_oidc_callback_propagates_token_endpoint_failure( client, oidc_enabled ): """If the token exchange fails the user sees a 400 rather than a 500 — callback must surface provider errors instead of crashing.""" - respx.get(DISCOVERY_URL).mock(return_value=Response(200, json=_discovery_doc())) - respx.post(TOKEN_URL).mock( - return_value=Response(400, json={"error": "invalid_grant"}) - ) + with respx.mock(base_url=ISSUER, assert_all_called=False): + _mock_provider(token_status=400) - resp = await client.get( - "/api/v1/auth/oauth/oidc/callback?code=bad-code", - follow_redirects=False, - ) + resp = await client.get( + "/api/v1/auth/oauth/oidc/callback?code=bad-code", + follow_redirects=False, + ) assert resp.status_code == 400 From a21a0ea402d6bd28a4604fbcae1137808101d3ce Mon Sep 17 00:00:00 2001 From: Alexandr Basiuk Date: Sun, 17 May 2026 19:53:31 +0300 Subject: [PATCH 5/6] test(oidc): register routes on the scoped respx router, not the global one MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previous attempt scoped respx with `respx.mock(base_url=ISSUER) as router` but kept adding routes via module-level `respx.get(...)` — which writes to the global default router, a different instance. The scoped router stayed empty, so every outbound request raised AllMockedAssertionError ("not mocked!"). Capture the router from the context manager and call `router.get/post(...)` on it. --- backend/tests/api/test_oidc.py | 41 ++++++++++++++++++++-------------- 1 file changed, 24 insertions(+), 17 deletions(-) diff --git a/backend/tests/api/test_oidc.py b/backend/tests/api/test_oidc.py index 0c8975a..6926ec7 100644 --- a/backend/tests/api/test_oidc.py +++ b/backend/tests/api/test_oidc.py @@ -87,22 +87,27 @@ def oidc_disabled(monkeypatch): monkeypatch.setattr(settings, "oidc_client_secret", None) -def _mock_provider(*, token_status: int = 200, userinfo: dict | None = None): - """Register the three OIDC endpoints on the active respx router. Caller - must already be inside a ``with respx.mock(base_url=ISSUER)`` block.""" - respx.get(DISCOVERY_PATH).mock( +def _mock_provider(router, *, token_status: int = 200, userinfo: dict | None = None): + """Register the three OIDC endpoints on the passed respx router. + + Routes must be added to the scoped router instance returned by + ``respx.mock(base_url=...)`` — the module-level ``respx.get(...)`` adds + to the global default router, which is a different object and won't + intercept the scoped requests. + """ + router.get(DISCOVERY_PATH).mock( return_value=Response(200, json=_discovery_doc()) ) if token_status == 200: - respx.post(TOKEN_URL).mock( + router.post(TOKEN_URL).mock( return_value=Response(200, json={"access_token": "provider-access"}) ) else: - respx.post(TOKEN_URL).mock( + router.post(TOKEN_URL).mock( return_value=Response(token_status, json={"error": "invalid_grant"}) ) if userinfo is not None: - respx.get(USERINFO_URL).mock(return_value=Response(200, json=userinfo)) + router.get(USERINFO_URL).mock(return_value=Response(200, json=userinfo)) async def _fetch_user_by_email(email: str) -> User | None: @@ -157,8 +162,8 @@ async def test_oidc_login_redirects_to_authorization_endpoint( ): """Login fetches discovery and 302s to the provider's authorize URL with the expected OAuth2/OIDC query params.""" - with respx.mock(base_url=ISSUER, assert_all_called=False): - respx.get(DISCOVERY_PATH).mock( + with respx.mock(base_url=ISSUER, assert_all_called=False) as router: + router.get(DISCOVERY_PATH).mock( return_value=Response(200, json=_discovery_doc()) ) @@ -202,13 +207,14 @@ async def test_oidc_callback_creates_new_user_and_returns_tokens( # Unique email per test run so we don't depend on table-truncation order. new_email = f"oidc-new-{uuid.uuid4().hex[:10]}@example.com" - with respx.mock(base_url=ISSUER, assert_all_called=False): + with respx.mock(base_url=ISSUER, assert_all_called=False) as router: _mock_provider( + router, userinfo={ "sub": "auth0|abc123", "email": new_email, "name": "New OIDC User", - } + }, ) resp = await client.get( @@ -251,9 +257,10 @@ async def test_oidc_callback_reuses_existing_user_by_email( await session.commit() existing_id = u.id - with respx.mock(base_url=ISSUER, assert_all_called=False): + with respx.mock(base_url=ISSUER, assert_all_called=False) as router: _mock_provider( - userinfo={"sub": "x", "email": existing_email, "name": "Existing User"} + router, + userinfo={"sub": "x", "email": existing_email, "name": "Existing User"}, ) resp = await client.get( @@ -277,8 +284,8 @@ async def test_oidc_callback_rejects_userinfo_without_email( ): """Provider must return an email claim — anything else is rejected with 400 rather than creating a userless row.""" - with respx.mock(base_url=ISSUER, assert_all_called=False): - _mock_provider(userinfo={"sub": "x"}) # no email + with respx.mock(base_url=ISSUER, assert_all_called=False) as router: + _mock_provider(router, userinfo={"sub": "x"}) # no email resp = await client.get( "/api/v1/auth/oauth/oidc/callback?code=test-code", @@ -293,8 +300,8 @@ async def test_oidc_callback_propagates_token_endpoint_failure( ): """If the token exchange fails the user sees a 400 rather than a 500 — callback must surface provider errors instead of crashing.""" - with respx.mock(base_url=ISSUER, assert_all_called=False): - _mock_provider(token_status=400) + with respx.mock(base_url=ISSUER, assert_all_called=False) as router: + _mock_provider(router, token_status=400) resp = await client.get( "/api/v1/auth/oauth/oidc/callback?code=bad-code", From f9d61c77e3c9395cb6eaba2b6d68a67bbe87f440 Mon Sep 17 00:00:00 2001 From: Alexandr Basiuk Date: Sun, 17 May 2026 20:19:53 +0300 Subject: [PATCH 6/6] fix(auth): harden OIDC + Google flows per PR #21 review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three fixes, all applied to both providers for parity: 1. Don't leak provider error bodies into HTTP responses `HTTPException(400, f"... {token_resp.text}")` echoed the provider's error body straight back to the browser — could include client_id, missing scopes, and other config hints. Log full response server-side at WARNING, return a generic " token exchange failed". 2. Require email_verified before upsert The user-upsert keys on email alone. Without verifying that the IdP actually vouches for the address, an attacker with control of any OIDC IdP (or an unverified-email Google account) could claim a victim's email and take over a pre-existing local account on first SSO login. Default-deny: missing claim is treated the same as explicit false. * OIDC uses the spec field `email_verified`. * Google's userinfo uses `verified_email` (Google's own naming). 3. Validate discovery doc has required endpoints Previously a 200 response missing authorization_endpoint / token_endpoint / userinfo_endpoint would throw KeyError → 500. Now raises 502 at discovery time with the list of missing fields. Tests: - Existing happy paths now send `email_verified: true` in userinfo. - New: `test_oidc_callback_rejects_unverified_email` - New: `test_oidc_discovery_doc_missing_endpoints_returns_502` Cross-provider hardening (state cookie / PKCE, strict cross-provider collision check on existing-email upsert) stays as a follow-up PR — same scope decision as the original. --- backend/app/api/v1/oauth_stub.py | 20 +++++++++++- backend/app/api/v1/oidc.py | 40 +++++++++++++++++++++++- backend/tests/api/test_oidc.py | 52 +++++++++++++++++++++++++++++++- 3 files changed, 109 insertions(+), 3 deletions(-) diff --git a/backend/app/api/v1/oauth_stub.py b/backend/app/api/v1/oauth_stub.py index 37d45aa..4abffa2 100644 --- a/backend/app/api/v1/oauth_stub.py +++ b/backend/app/api/v1/oauth_stub.py @@ -10,6 +10,7 @@ GOOGLE_CLIENT_SECRET, GOOGLE_REDIRECT_URI, FRONTEND_URL). When any is missing both endpoints return 503 so the SPA can fall back to email/password. """ +import logging from urllib.parse import urlencode import httpx @@ -24,6 +25,8 @@ from app.models.user import User from app.services import workspace_service +logger = logging.getLogger(__name__) + router = APIRouter(prefix="/auth/oauth", tags=["oauth"]) GOOGLE_AUTH_URL = "https://accounts.google.com/o/oauth2/v2/auth" @@ -67,7 +70,15 @@ async def callback( "grant_type": "authorization_code", }) if token_resp.status_code != 200: - raise HTTPException(400, f"Google token exchange failed: {token_resp.text}") + # Generic error to the client; full provider response logged + # server-side. Same hardening as the OIDC flow — don't leak + # client_id / scope details into the browser. + logger.warning( + "Google token exchange failed: status=%s body=%s", + token_resp.status_code, + token_resp.text, + ) + raise HTTPException(400, "Google token exchange failed") google_access = token_resp.json().get("access_token") ui_resp = await client.get( @@ -81,6 +92,13 @@ async def callback( email = info.get("email") if not email: raise HTTPException(400, "Google account returned no email") + # Google's userinfo always includes verified_email for Google-hosted + # accounts, but Workspace admins can let users add unverified addresses. + # Without this check, an attacker with an unverified Google-side email + # could claim an arbitrary address and take over an existing local + # account in the upsert below. + if not info.get("verified_email", False): + raise HTTPException(400, "Google email is not verified") name = info.get("name") or email.split("@")[0].title() existing = ( diff --git a/backend/app/api/v1/oidc.py b/backend/app/api/v1/oidc.py index ed56503..edf0143 100644 --- a/backend/app/api/v1/oidc.py +++ b/backend/app/api/v1/oidc.py @@ -19,6 +19,7 @@ fragment-based token delivery) — chose composition over abstraction to keep each provider's quirks contained. """ +import logging from urllib.parse import urlencode import httpx @@ -33,6 +34,8 @@ from app.models.user import User from app.services import workspace_service +logger = logging.getLogger(__name__) + router = APIRouter(prefix="/auth/oauth", tags=["oauth"]) # Discovery doc cache keyed by issuer URL. OIDC discovery responses are stable @@ -40,6 +43,15 @@ # for the process lifetime is the standard pattern. Cleared by tests. _discovery_cache: dict[str, dict] = {} +# Endpoints we require from the discovery document. If any is missing the +# provider isn't usable — fail at discovery time with 502 instead of throwing +# KeyError later when we try to dereference it. +_REQUIRED_DISCOVERY_KEYS = ( + "authorization_endpoint", + "token_endpoint", + "userinfo_endpoint", +) + def _oidc_enabled() -> bool: return bool( @@ -59,6 +71,11 @@ async def _get_discovery(client: httpx.AsyncClient) -> dict: if resp.status_code != 200: raise HTTPException(502, f"OIDC discovery failed: {resp.status_code}") doc = resp.json() + missing = [k for k in _REQUIRED_DISCOVERY_KEYS if not doc.get(k)] + if missing: + raise HTTPException( + 502, f"OIDC discovery doc missing required endpoint(s): {', '.join(missing)}" + ) _discovery_cache[issuer] = doc return doc @@ -100,7 +117,15 @@ async def oidc_callback( }, ) if token_resp.status_code != 200: - raise HTTPException(400, f"OIDC token exchange failed: {token_resp.text}") + # Log the provider's response server-side for operators; return a + # generic message to the user so we don't leak provider config + # (client_id, missing scopes, etc.) into a browser response. + logger.warning( + "OIDC token exchange failed: status=%s body=%s", + token_resp.status_code, + token_resp.text, + ) + raise HTTPException(400, "OIDC token exchange failed") provider_access = token_resp.json().get("access_token") if not provider_access: raise HTTPException(400, "OIDC token response missing access_token") @@ -110,12 +135,25 @@ async def oidc_callback( headers={"Authorization": f"Bearer {provider_access}"}, ) if ui_resp.status_code != 200: + logger.warning( + "OIDC userinfo fetch failed: status=%s body=%s", + ui_resp.status_code, + ui_resp.text, + ) raise HTTPException(400, "OIDC userinfo fetch failed") info = ui_resp.json() email = info.get("email") if not email: raise HTTPException(400, "OIDC account returned no email claim") + # Reject if the IdP can't vouch that the user actually controls this + # address. Without this check, an attacker with control of any OIDC IdP + # (or a user with an unverified email on a public IdP) could claim an + # arbitrary email and take over an existing local account in the upsert + # below. Default-deny: if the provider doesn't send the claim at all, + # treat it as not-verified. + if not info.get("email_verified", False): + raise HTTPException(400, "OIDC email is not verified by the provider") name = info.get("name") or email.split("@")[0].title() existing = ( diff --git a/backend/tests/api/test_oidc.py b/backend/tests/api/test_oidc.py index 6926ec7..c22aa20 100644 --- a/backend/tests/api/test_oidc.py +++ b/backend/tests/api/test_oidc.py @@ -213,6 +213,7 @@ async def test_oidc_callback_creates_new_user_and_returns_tokens( userinfo={ "sub": "auth0|abc123", "email": new_email, + "email_verified": True, "name": "New OIDC User", }, ) @@ -260,7 +261,12 @@ async def test_oidc_callback_reuses_existing_user_by_email( with respx.mock(base_url=ISSUER, assert_all_called=False) as router: _mock_provider( router, - userinfo={"sub": "x", "email": existing_email, "name": "Existing User"}, + userinfo={ + "sub": "x", + "email": existing_email, + "email_verified": True, + "name": "Existing User", + }, ) resp = await client.get( @@ -309,3 +315,47 @@ async def test_oidc_callback_propagates_token_endpoint_failure( ) assert resp.status_code == 400 + + +async def test_oidc_callback_rejects_unverified_email(client, oidc_enabled): + """email_verified=false must be rejected — otherwise an attacker with + control of any IdP could claim someone else's email and take over a + pre-existing local account in the upsert path. Default-deny: missing + claim is treated the same as explicit false.""" + with respx.mock(base_url=ISSUER, assert_all_called=False) as router: + _mock_provider( + router, + userinfo={ + "sub": "x", + "email": "victim@example.com", + "email_verified": False, + "name": "Attacker", + }, + ) + + resp = await client.get( + "/api/v1/auth/oauth/oidc/callback?code=test-code", + follow_redirects=False, + ) + + assert resp.status_code == 400 + # Sanity: no user row created. + user = await _fetch_user_by_email("victim@example.com") + assert user is None + + +async def test_oidc_discovery_doc_missing_endpoints_returns_502( + client, oidc_enabled +): + """If the IdP's discovery doc is missing a required endpoint we must + fail at discovery (502) rather than throw KeyError downstream.""" + bad_doc = {"issuer": ISSUER} # no *_endpoint fields + + with respx.mock(base_url=ISSUER, assert_all_called=False) as router: + router.get(DISCOVERY_PATH).mock(return_value=Response(200, json=bad_doc)) + + resp = await client.get( + "/api/v1/auth/oauth/oidc/login", follow_redirects=False + ) + + assert resp.status_code == 502