From 8052a1c18db6f9f2042b646e2068d1b724955aeb Mon Sep 17 00:00:00 2001 From: ClaydeCode Date: Fri, 10 Jul 2026 21:29:33 +0000 Subject: [PATCH 1/5] =?UTF-8?q?feat(oidc):=20phase-2a=20embedded=20OIDC=20?= =?UTF-8?q?provider=20=E2=80=94=20productionized=20from=20the=20passed=20s?= =?UTF-8?q?pike?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Five endpoints under /public/oidc (discovery, authorize, token, userinfo, jwks) on Authlib's framework-agnostic server classes: authorization-code + PKCE + refresh rotation, RS256 id_tokens signed with a key persisted in kv_store. First-party app clients only — no dynamic registration, no consent UI, no third-party exposure. The session is the existing terminal cookie: /authorize resolves it to the terminal's user (phase-1 users table), so every paired browser signs into an app with zero clicks. Anonymous browsers are redirected to the terminal UI with an oidc_rd return URL until the phase-3 login page exists. Authlib's core is synchronous; endpoints run it in asyncio.to_thread and the storage hooks bridge back to the main loop's psycopg pool via run_coroutine_threadsafe — single pool, unlike the spike's sync-psycopg sidestep. Hardening over the spike (per PoC verdict findings): access/refresh tokens and authorization codes stored as SHA-256 digests; scope saved from request.scope (client-allowed) not the raw request; null claims stripped from id_tokens (strict clients reject nonce:null); client_secret_basic AND _post accepted (Immich uses _post); token endpoint rate-limited in-process; authlib pinned <1.8 (grant param becomes mandatory there); FORWARDED_ALLOW_IPS=* so uvicorn trusts Traefik's X-Forwarded-Proto — Authlib refuses flows it sees as plain http. Client secrets deliberately stay plaintext: compose templates re-render them at every startup and they live in app compose envs on the same disk anyway. Phase 2a of the multi-user identity rollout; client registration at app install follows in phase 2b. Co-Authored-By: Claude Fable 5 --- Dockerfile | 3 + migrations/shard-core-0003-oidc.sql | 36 +++ pyproject.toml | 1 + shard_core/database/oidc.py | 109 +++++++ shard_core/service/oidc_provider.py | 459 ++++++++++++++++++++++++++++ shard_core/web/public/__init__.py | 3 +- shard_core/web/public/oidc.py | 205 +++++++++++++ tests/test_oidc.py | 436 ++++++++++++++++++++++++++ uv.lock | 27 ++ 9 files changed, 1278 insertions(+), 1 deletion(-) create mode 100644 migrations/shard-core-0003-oidc.sql create mode 100644 shard_core/database/oidc.py create mode 100644 shard_core/service/oidc_provider.py create mode 100644 shard_core/web/public/oidc.py create mode 100644 tests/test_oidc.py diff --git a/Dockerfile b/Dockerfile index 5952cac6..8340893d 100644 --- a/Dockerfile +++ b/Dockerfile @@ -47,6 +47,9 @@ ENV PATH="/app/.venv/bin:$PATH" HEALTHCHECK --start-period=5s --interval=30s --timeout=5s CMD curl -f http://localhost/public/health #ENV FLASK_APP=shard_core +# Trust X-Forwarded-* from the in-network Traefik so request URLs are https; +# Authlib refuses OAuth flows on plain http (InsecureTransportError). +ENV FORWARDED_ALLOW_IPS=* EXPOSE 80 ENTRYPOINT ["/usr/bin/tini", "--"] CMD ["fastapi", "run", "--port", "80", "shard_core/app.py"] diff --git a/migrations/shard-core-0003-oidc.sql b/migrations/shard-core-0003-oidc.sql new file mode 100644 index 00000000..82f1d250 --- /dev/null +++ b/migrations/shard-core-0003-oidc.sql @@ -0,0 +1,36 @@ +-- shard-core-0003-oidc +-- depends: shard-core-0002-users + +CREATE TABLE IF NOT EXISTS oidc_clients ( + client_id TEXT PRIMARY KEY, + client_secret TEXT, + app_name TEXT UNIQUE NOT NULL, + redirect_uris JSONB NOT NULL, + scope TEXT NOT NULL DEFAULT 'openid profile email', + token_endpoint_auth_method TEXT NOT NULL DEFAULT 'client_secret_basic', + created TIMESTAMPTZ NOT NULL DEFAULT now() +); + +CREATE TABLE IF NOT EXISTS oidc_codes ( + code_hash TEXT PRIMARY KEY, + client_id TEXT NOT NULL REFERENCES oidc_clients (client_id) ON DELETE CASCADE, + redirect_uri TEXT, + scope TEXT, + user_sub TEXT NOT NULL REFERENCES users (id) ON DELETE CASCADE, + nonce TEXT, + code_challenge TEXT, + code_challenge_method TEXT, + auth_time BIGINT NOT NULL, + expires_at TIMESTAMPTZ NOT NULL +); + +CREATE TABLE IF NOT EXISTS oidc_tokens ( + access_token_hash TEXT PRIMARY KEY, + refresh_token_hash TEXT UNIQUE, + client_id TEXT NOT NULL REFERENCES oidc_clients (client_id) ON DELETE CASCADE, + user_sub TEXT NOT NULL REFERENCES users (id) ON DELETE CASCADE, + scope TEXT, + issued_at BIGINT NOT NULL, + expires_in BIGINT NOT NULL, + revoked BOOLEAN NOT NULL DEFAULT FALSE +); diff --git a/pyproject.toml b/pyproject.toml index 9668b9bb..2f73dcab 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -42,6 +42,7 @@ dependencies = [ "python-multipart", "aiofiles", "httpx", + "authlib>=1.7.2,<1.8", ] [project.optional-dependencies] diff --git a/shard_core/database/oidc.py b/shard_core/database/oidc.py new file mode 100644 index 00000000..e1da01d6 --- /dev/null +++ b/shard_core/database/oidc.py @@ -0,0 +1,109 @@ +from typing import LiteralString + +from psycopg import AsyncConnection +from psycopg.rows import dict_row +from psycopg.types.json import Jsonb + +# --- clients --------------------------------------------------------------- + + +async def upsert_client(conn: AsyncConnection, client: dict) -> dict: + sql: LiteralString = """INSERT INTO oidc_clients + (client_id, client_secret, app_name, redirect_uris, scope, token_endpoint_auth_method) + VALUES (%(client_id)s, %(client_secret)s, %(app_name)s, %(redirect_uris)s, + %(scope)s, %(token_endpoint_auth_method)s) + ON CONFLICT (app_name) DO UPDATE SET + client_id = EXCLUDED.client_id, + client_secret = EXCLUDED.client_secret, + redirect_uris = EXCLUDED.redirect_uris, + scope = EXCLUDED.scope, + token_endpoint_auth_method = EXCLUDED.token_endpoint_auth_method + RETURNING *""" + async with conn.cursor(row_factory=dict_row) as cur: + await cur.execute( + sql, {**client, "redirect_uris": Jsonb(client["redirect_uris"])} + ) + return await cur.fetchone() + + +async def get_client(conn: AsyncConnection, client_id: str) -> dict | None: + sql: LiteralString = "SELECT * FROM oidc_clients WHERE client_id = %s" + async with conn.cursor(row_factory=dict_row) as cur: + await cur.execute(sql, (client_id,)) + return await cur.fetchone() + + +# --- authorization codes ------------------------------------------------------ + + +async def insert_code(conn: AsyncConnection, code: dict): + sql: LiteralString = """INSERT INTO oidc_codes + (code_hash, client_id, redirect_uri, scope, user_sub, nonce, + code_challenge, code_challenge_method, auth_time, expires_at) + VALUES (%(code_hash)s, %(client_id)s, %(redirect_uri)s, %(scope)s, %(user_sub)s, + %(nonce)s, %(code_challenge)s, %(code_challenge_method)s, + %(auth_time)s, %(expires_at)s)""" + await conn.execute(sql, code) + + +async def get_code( + conn: AsyncConnection, code_hash: str, client_id: str +) -> dict | None: + sql: LiteralString = ( + "SELECT * FROM oidc_codes WHERE code_hash = %s AND client_id = %s AND expires_at > now()" + ) + async with conn.cursor(row_factory=dict_row) as cur: + await cur.execute(sql, (code_hash, client_id)) + return await cur.fetchone() + + +async def delete_code(conn: AsyncConnection, code_hash: str): + sql: LiteralString = "DELETE FROM oidc_codes WHERE code_hash = %s" + await conn.execute(sql, (code_hash,)) + + +async def exists_nonce(conn: AsyncConnection, nonce: str, client_id: str) -> bool: + sql: LiteralString = "SELECT 1 FROM oidc_codes WHERE nonce = %s AND client_id = %s" + async with conn.cursor() as cur: + await cur.execute(sql, (nonce, client_id)) + return await cur.fetchone() is not None + + +# --- tokens -------------------------------------------------------------------- + + +async def insert_token(conn: AsyncConnection, token: dict): + sql: LiteralString = """INSERT INTO oidc_tokens + (access_token_hash, refresh_token_hash, client_id, user_sub, scope, issued_at, expires_in) + VALUES (%(access_token_hash)s, %(refresh_token_hash)s, %(client_id)s, %(user_sub)s, + %(scope)s, %(issued_at)s, %(expires_in)s)""" + await conn.execute(sql, token) + + +async def get_token_by_access_hash( + conn: AsyncConnection, access_token_hash: str +) -> dict | None: + sql: LiteralString = ( + "SELECT * FROM oidc_tokens WHERE access_token_hash = %s AND NOT revoked" + ) + async with conn.cursor(row_factory=dict_row) as cur: + await cur.execute(sql, (access_token_hash,)) + return await cur.fetchone() + + +async def get_token_by_refresh_hash( + conn: AsyncConnection, refresh_token_hash: str +) -> dict | None: + sql: LiteralString = ( + "SELECT * FROM oidc_tokens WHERE refresh_token_hash = %s AND NOT revoked" + ) + async with conn.cursor(row_factory=dict_row) as cur: + await cur.execute(sql, (refresh_token_hash,)) + return await cur.fetchone() + + +async def revoke_token(conn: AsyncConnection, access_token_hash: str): + sql: LiteralString = ( + "UPDATE oidc_tokens SET revoked = TRUE WHERE access_token_hash = %s" + ) + await conn.execute(sql, (access_token_hash,)) diff --git a/shard_core/service/oidc_provider.py b/shard_core/service/oidc_provider.py new file mode 100644 index 00000000..8c8c863f --- /dev/null +++ b/shard_core/service/oidc_provider.py @@ -0,0 +1,459 @@ +"""Embedded OIDC provider. + +Authorization server for first-party app clients: authorization-code + PKCE + +refresh, RS256 id_tokens, discovery, JWKS, userinfo. Users come from the +`users` table (phase 1); the OIDC `sub` is the user id, which for the owner is +the shard's default identity id. + +Built on Authlib's framework-agnostic server classes; see +`shard_core.web.public.oidc` for the FastAPI adapter. Authlib's server core is +synchronous and runs in a worker thread (asyncio.to_thread); its storage hooks +bridge back to the main event loop's connection pool via +asyncio.run_coroutine_threadsafe. + +Secrets at rest: client secrets, authorization codes, and access/refresh +tokens are high-entropy random strings and are stored as SHA-256 digests only. +""" + +import asyncio +import hashlib +import logging +import secrets +import time +from dataclasses import dataclass, field +from datetime import datetime, timedelta, timezone + +from authlib.oauth2.rfc6749 import ( + AuthorizationServer, + ClientMixin, + AuthorizationCodeMixin, + grants, +) +from authlib.oauth2.rfc7636 import CodeChallenge +from authlib.oidc.core import UserInfo +from authlib.oidc.core.grants import OpenIDCode +from joserfc.jwk import RSAKey + +from shard_core.database import kv_store +from shard_core.database import oidc as db_oidc +from shard_core.database import users as db_users +from shard_core.database.connection import db_conn + +log = logging.getLogger(__name__) + +STORE_KEY_OIDC_JWK = "oidc_provider_jwk" +ACCESS_TOKEN_EXPIRES_IN = 3600 +REFRESH_TOKEN_LIFETIME = 30 * 24 * 3600 +CODE_EXPIRES_IN = 300 +SUPPORTED_SCOPES = ["openid", "profile", "email"] +TOKEN_RATE_LIMIT = 30 # token requests per minute, enforced by the web layer + +_state: dict = {"issuer": None, "jwk": None, "loop": None} + + +def configure(issuer: str, jwk: dict, loop: asyncio.AbstractEventLoop): + _state["issuer"] = issuer + _state["jwk"] = jwk + _state["loop"] = loop + + +def hash_secret(value: str) -> str: + return hashlib.sha256(value.encode()).hexdigest() + + +def _run(coro): + """Bridge for Authlib's sync storage hooks (called inside asyncio.to_thread) + back to the main event loop, where the connection pool lives.""" + return asyncio.run_coroutine_threadsafe(coro, _state["loop"]).result() + + +# --- models ------------------------------------------------------------------ + + +@dataclass +class ShardUser: + """The resource owner: a row from the users table.""" + + sub: str + username: str + display_name: str + email: str | None = None + + @classmethod + def from_row(cls, row: dict | None): + if row is None or row["disabled"]: + return None + return cls( + sub=row["id"], + username=row["username"], + display_name=row["display_name"], + email=row["email"], + ) + + def get_user_id(self): + return self.sub + + +async def _user_from_sub_async(sub: str) -> ShardUser | None: + async with db_conn() as conn: + return ShardUser.from_row(await db_users.get_by_id(conn, sub)) + + +@dataclass +class OidcClient(ClientMixin): + client_id: str + client_secret: str | None + app_name: str + redirect_uris: list[str] + scope: str + token_endpoint_auth_method: str + grant_types: list[str] = field( + default_factory=lambda: ["authorization_code", "refresh_token"] + ) + + @classmethod + def from_row(cls, row: dict | None): + if row is None: + return None + return cls( + client_id=row["client_id"], + client_secret=row["client_secret"], + app_name=row["app_name"], + redirect_uris=row["redirect_uris"], + scope=row["scope"], + token_endpoint_auth_method=row["token_endpoint_auth_method"], + ) + + def get_client_id(self): + return self.client_id + + def get_default_redirect_uri(self): + return self.redirect_uris[0] + + def get_allowed_scope(self, scope): + if not scope: + return self.scope + allowed = set(self.scope.split()) + return " ".join(s for s in scope.split() if s in allowed) + + def check_redirect_uri(self, redirect_uri): + return redirect_uri in self.redirect_uris + + def check_client_secret(self, client_secret): + return self.client_secret is not None and secrets.compare_digest( + self.client_secret, client_secret + ) + + def check_endpoint_auth_method(self, method, endpoint): + if endpoint == "token": + if self.client_secret is None: + return method == "none" + # first-party confidential clients may use either secret method + # (Immich sends client_secret_post; others default to basic) + return method in ("client_secret_basic", "client_secret_post") + return True + + def check_response_type(self, response_type): + return response_type == "code" + + def check_grant_type(self, grant_type): + return grant_type in self.grant_types + + +@dataclass +class AuthorizationCode(AuthorizationCodeMixin): + code: str + client_id: str + redirect_uri: str | None + scope: str | None + user_sub: str + nonce: str | None + code_challenge: str | None + code_challenge_method: str | None + auth_time: int + + @classmethod + def from_row(cls, code: str, row: dict | None): + if row is None: + return None + fields = {k: row[k] for k in cls.__dataclass_fields__ if k in row} + return cls(code=code, **fields) + + def get_redirect_uri(self): + return self.redirect_uri + + def get_scope(self): + return self.scope + + def get_nonce(self): + return self.nonce + + def get_auth_time(self): + return self.auth_time + + def get_acr(self): + return None + + def get_amr(self): + return None + + +# --- key management ------------------------------------------------------------- + + +async def ensure_jwk() -> dict: + """RS256 keypair for id_token signing, persisted as a private JWK in kv_store.""" + async with db_conn() as conn: + try: + return await kv_store.get_value(conn, STORE_KEY_OIDC_JWK) + except KeyError: + key = RSAKey.generate_key(2048, private=True, auto_kid=True) + jwk = key.as_dict(private=True) + await kv_store.set_value(conn, STORE_KEY_OIDC_JWK, jwk) + log.info("generated OIDC provider signing key (kid=%s)", jwk.get("kid")) + return jwk + + +def public_jwks() -> dict: + public = RSAKey.import_key(_state["jwk"]).as_dict(private=False) + public.setdefault("use", "sig") + public.setdefault("alg", "RS256") + return {"keys": [public]} + + +# --- storage bridges (called from Authlib's sync hooks) --------------------------- + + +async def _with_conn(fn, *args): + async with db_conn() as conn: + return await fn(conn, *args) + + +# --- grants ------------------------------------------------------------------------ + + +class ShardAuthCodeGrant(grants.AuthorizationCodeGrant): + TOKEN_ENDPOINT_AUTH_METHODS = ["client_secret_basic", "client_secret_post", "none"] + + def save_authorization_code(self, code, request): + data = request.payload.data + _run( + _with_conn( + db_oidc.insert_code, + { + "code_hash": hash_secret(code), + "client_id": request.client.client_id, + "redirect_uri": request.payload.redirect_uri, + # request.scope (not payload.scope) is the scope resolved against + # the client's allowed scope — payload.scope would let a client + # escalate beyond its registered scope + "scope": request.scope, + "user_sub": request.user.sub, + "nonce": data.get("nonce"), + "code_challenge": data.get("code_challenge"), + "code_challenge_method": data.get("code_challenge_method"), + "auth_time": int(time.time()), + "expires_at": datetime.now(timezone.utc) + + timedelta(seconds=CODE_EXPIRES_IN), + }, + ) + ) + + def query_authorization_code(self, code, client): + row = _run(_with_conn(db_oidc.get_code, hash_secret(code), client.client_id)) + return AuthorizationCode.from_row(code, row) + + def delete_authorization_code(self, authorization_code): + _run(_with_conn(db_oidc.delete_code, hash_secret(authorization_code.code))) + + def authenticate_user(self, authorization_code): + return _run(_user_from_sub_async(authorization_code.user_sub)) + + +class ShardRefreshTokenGrant(grants.RefreshTokenGrant): + INCLUDE_NEW_REFRESH_TOKEN = True + + def authenticate_refresh_token(self, refresh_token): + row = _run( + _with_conn(db_oidc.get_token_by_refresh_hash, hash_secret(refresh_token)) + ) + if row and row["issued_at"] + REFRESH_TOKEN_LIFETIME > time.time(): + return _TokenRecord(row) + return None + + def authenticate_user(self, credential): + return _run(_user_from_sub_async(credential.row["user_sub"])) + + def revoke_old_credential(self, credential): + _run(_with_conn(db_oidc.revoke_token, credential.row["access_token_hash"])) + + +class _TokenRecord: + def __init__(self, row: dict): + self.row = row + + def check_client(self, client): + return self.row["client_id"] == client.client_id + + def get_scope(self): + return self.row["scope"] + + def get_expires_in(self): + return self.row["expires_in"] + + +class ShardOpenIDCode(OpenIDCode): + def exists_nonce(self, nonce, request): + return _run(_with_conn(db_oidc.exists_nonce, nonce, request.payload.client_id)) + + def get_authorization_code_claims(self, authorization_code): + # Base impl always emits "nonce", as null when the client sent none. + # Strict clients (oauth4webapi/Immich) reject nonce:null vs absent. + claims = super().get_authorization_code_claims(authorization_code) + return {k: v for k, v in claims.items() if v is not None} + + def resolve_client_private_key(self, client): + return _state["jwk"] + + def get_client_claims(self, client): + return {"iss": _state["issuer"], "aud": [client.get_client_id()]} + + def generate_user_info(self, user, scope): + info = UserInfo(sub=user.sub) + if "profile" in scope: + info["name"] = user.display_name + info["preferred_username"] = user.username + if "email" in scope and user.email: + info["email"] = user.email + info["email_verified"] = True + return info + + +# --- server ----------------------------------------------------------------------- + + +def _generate_bearer_token( + grant_type, + client, + user=None, + scope=None, + expires_in=None, + include_refresh_token=True, +): + token = { + "token_type": "Bearer", + "access_token": secrets.token_urlsafe(32), + "expires_in": expires_in or ACCESS_TOKEN_EXPIRES_IN, + "scope": scope, + } + if include_refresh_token: + token["refresh_token"] = secrets.token_urlsafe(32) + return token + + +def _save_token(token: dict, request): + _run( + _with_conn( + db_oidc.insert_token, + { + "access_token_hash": hash_secret(token["access_token"]), + "refresh_token_hash": ( + hash_secret(token["refresh_token"]) + if token.get("refresh_token") + else None + ), + "client_id": request.client.client_id, + "user_sub": request.user.sub, + "scope": token.get("scope"), + "issued_at": int(time.time()), + "expires_in": token["expires_in"], + }, + ) + ) + + +def _query_client(client_id: str): + return OidcClient.from_row(_run(_with_conn(db_oidc.get_client, client_id))) + + +def build_authorization_server(server_cls=AuthorizationServer) -> AuthorizationServer: + """server_cls lets the web layer pass its framework-adapter subclass.""" + server = server_cls(scopes_supported=SUPPORTED_SCOPES) + server.query_client = _query_client + server.save_token = _save_token + server.register_token_generator("default", _generate_bearer_token) + server.register_grant( + ShardAuthCodeGrant, + [CodeChallenge(required=True), ShardOpenIDCode(require_nonce=False)], + ) + server.register_grant(ShardRefreshTokenGrant) + return server + + +# --- async API for routes and app installation --------------------------------------- + + +async def register_client( + app_name: str, + redirect_uris: list[str], + public_client: bool = False, + scope: str = "openid profile email", +) -> dict: + """Create (or replace) the OIDC client for an installed app. + + The client secret is stored plaintext: docker-compose templates consume it + on every startup re-render, and it sits in the app's compose env on the + same disk anyway. Codes and tokens, by contrast, are stored hashed. + """ + client_secret = None if public_client else secrets.token_urlsafe(32) + row = { + "client_id": f"{app_name}-{secrets.token_urlsafe(6)}", + "client_secret": client_secret, + "app_name": app_name, + "redirect_uris": redirect_uris, + "scope": scope, + "token_endpoint_auth_method": ( + "none" if public_client else "client_secret_basic" + ), + } + async with db_conn() as conn: + await db_oidc.upsert_client(conn, row) + log.info(f"registered OIDC client for app {app_name}") + return row + + +async def userinfo_for_access_token(access_token: str) -> dict | None: + async with db_conn() as conn: + row = await db_oidc.get_token_by_access_hash(conn, hash_secret(access_token)) + if row is None or row["issued_at"] + row["expires_in"] < time.time(): + return None + user = ShardUser.from_row(await db_users.get_by_id(conn, row["user_sub"])) + if user is None: + return None + return dict( + ShardOpenIDCode(require_nonce=False).generate_user_info( + user, row["scope"] or "" + ) + ) + + +def discovery_document() -> dict: + issuer = _state["issuer"] + return { + "issuer": issuer, + "authorization_endpoint": f"{issuer}/authorize", + "token_endpoint": f"{issuer}/token", + "userinfo_endpoint": f"{issuer}/userinfo", + "jwks_uri": f"{issuer}/jwks", + "response_types_supported": ["code"], + "grant_types_supported": ["authorization_code", "refresh_token"], + "subject_types_supported": ["public"], + "id_token_signing_alg_values_supported": ["RS256"], + "scopes_supported": SUPPORTED_SCOPES, + "token_endpoint_auth_methods_supported": [ + "client_secret_basic", + "client_secret_post", + "none", + ], + "code_challenge_methods_supported": ["S256", "plain"], + } diff --git a/shard_core/web/public/__init__.py b/shard_core/web/public/__init__.py index 73f0f174..0158aec1 100644 --- a/shard_core/web/public/__init__.py +++ b/shard_core/web/public/__init__.py @@ -1,6 +1,6 @@ from fastapi import APIRouter -from . import health, meta, pair +from . import health, meta, oidc, pair router = APIRouter( prefix="/public", @@ -9,4 +9,5 @@ router.include_router(health.router) router.include_router(meta.router) +router.include_router(oidc.router) router.include_router(pair.router) diff --git a/shard_core/web/public/oidc.py b/shard_core/web/public/oidc.py new file mode 100644 index 00000000..51a7ea13 --- /dev/null +++ b/shard_core/web/public/oidc.py @@ -0,0 +1,205 @@ +"""FastAPI adapter for the embedded OIDC provider. + +Authlib's server core is synchronous; request handling runs in a worker thread +(asyncio.to_thread) while its storage hooks bridge back to this event loop's +connection pool. The OAuth2Request is pre-built in the async route (reading +the form body is async in Starlette) and passed through. + +The provider is initialized lazily on first request: the issuer is derived +from the default identity's domain (which does not exist yet at router import +time) and the signing key lives in the database. +""" + +import asyncio +import logging +import time +from collections import deque +from urllib.parse import quote + +from authlib.oauth2.rfc6749 import AuthorizationServer, OAuth2Request +from authlib.oauth2.rfc6749.requests import BasicOAuth2Payload +from fastapi import APIRouter, Cookie, Request +from fastapi.responses import JSONResponse, RedirectResponse, Response +from requests.structures import CaseInsensitiveDict + +from shard_core.database import users as db_users +from shard_core.database.connection import db_conn +from shard_core.service import identity, pairing +from shard_core.service.oidc_provider import ( + TOKEN_RATE_LIMIT, + ShardUser, + build_authorization_server, + configure, + discovery_document, + ensure_jwk, + public_jwks, + userinfo_for_access_token, +) +from shard_core.settings import settings + +log = logging.getLogger(__name__) + +router = APIRouter(prefix="/oidc", tags=["/public/oidc"]) + + +# --- request/response adapter --------------------------------------------------- + + +class StarletteOAuth2Request(OAuth2Request): + def __init__(self, method: str, url: str, headers, query: dict, form: dict): + super().__init__(method=method, uri=url, headers=headers) + merged = {**query, **form} + self.payload = BasicOAuth2Payload(merged) + self._args = query + self._form = form + + @property + def args(self): + return self._args + + @property + def form(self): + return self._form + + +async def _build_oauth2_request(request: Request) -> StarletteOAuth2Request: + form = {} + if request.method == "POST": + form = {k: v for k, v in (await request.form()).items()} + return StarletteOAuth2Request( + method=request.method, + url=str(request.url), + # Authlib looks headers up by canonical name ("Authorization"); + # Starlette normalizes to lowercase — bridge with a CI dict. + headers=CaseInsensitiveDict(request.headers), + query=dict(request.query_params), + form=form, + ) + + +class StarletteAuthorizationServer(AuthorizationServer): + def create_oauth2_request(self, request): + if isinstance(request, OAuth2Request): + return request + raise RuntimeError("expected a pre-built OAuth2Request") + + def handle_response(self, status, body, headers): + headers = dict(headers or []) + if isinstance(body, dict): + return JSONResponse(body, status_code=status, headers=headers) + if 300 <= status < 400 and "Location" in headers: + return RedirectResponse(headers["Location"], status_code=status) + return Response(content=body or "", status_code=status, headers=headers) + + def send_signal(self, name, *args, **kwargs): + pass + + +# --- lazy per-app initialization --------------------------------------------------- + +_init_lock = asyncio.Lock() + + +async def _get_server(request: Request) -> StarletteAuthorizationServer: + app = request.app + if not hasattr(app.state, "oidc_server"): + async with _init_lock: + if not hasattr(app.state, "oidc_server"): + i = await identity.get_default_identity() + protocol = "http" if settings().traefik.disable_ssl else "https" + # Traefik routes /core/* to shard_core with /core/ stripped + issuer = f"{protocol}://{i.domain}/core/public/oidc" + jwk = await ensure_jwk() + configure(issuer, jwk, asyncio.get_running_loop()) + _token_request_times.clear() + app.state.oidc_server = build_authorization_server( + StarletteAuthorizationServer + ) + log.info(f"OIDC provider initialized, issuer={issuer}") + return app.state.oidc_server + + +async def _session_user(authorization: str | None) -> ShardUser | None: + """Existing shard session (terminal JWT cookie) → the terminal's user.""" + if not authorization: + return None + try: + terminal = await pairing.verify_terminal_jwt(authorization) + except pairing.InvalidJwt: + return None + if terminal.user_id is None: + return None + async with db_conn() as conn: + return ShardUser.from_row(await db_users.get_by_id(conn, terminal.user_id)) + + +# --- token endpoint rate limit ------------------------------------------------------ + +_token_request_times: deque = deque() +_RATE_WINDOW = 60 + + +def _rate_limited() -> bool: + now = time.monotonic() + while _token_request_times and _token_request_times[0] < now - _RATE_WINDOW: + _token_request_times.popleft() + if len(_token_request_times) >= TOKEN_RATE_LIMIT: + return True + _token_request_times.append(now) + return False + + +# --- endpoints ------------------------------------------------------------------- + + +@router.get("/.well-known/openid-configuration") +async def openid_configuration(request: Request): + await _get_server(request) + return discovery_document() + + +@router.get("/jwks") +async def jwks(request: Request): + await _get_server(request) + return public_jwks() + + +@router.get("/authorize") +async def authorize(request: Request, authorization: str = Cookie(None)): + server = await _get_server(request) + user = await _session_user(authorization) + if user is None: + # No shard session: send the browser to the terminal's pairing/login UI. + rd = quote(str(request.url), safe="") + return RedirectResponse(f"/?oidc_rd={rd}", status_code=302) + oreq = await _build_oauth2_request(request) + return await asyncio.to_thread(server.create_authorization_response, oreq, user) + + +@router.post("/token") +async def token(request: Request): + server = await _get_server(request) + if _rate_limited(): + return JSONResponse({"error": "slow_down"}, status_code=429) + oreq = await _build_oauth2_request(request) + return await asyncio.to_thread(server.create_token_response, oreq) + + +@router.get("/userinfo") +async def userinfo(request: Request): + await _get_server(request) + auth = request.headers.get("authorization", "") + if not auth.lower().startswith("bearer "): + return JSONResponse( + {"error": "invalid_token"}, + status_code=401, + headers={"WWW-Authenticate": "Bearer"}, + ) + info = await userinfo_for_access_token(auth[7:]) + if info is None: + return JSONResponse( + {"error": "invalid_token"}, + status_code=401, + headers={"WWW-Authenticate": "Bearer"}, + ) + return info diff --git a/tests/test_oidc.py b/tests/test_oidc.py new file mode 100644 index 00000000..94a933b0 --- /dev/null +++ b/tests/test_oidc.py @@ -0,0 +1,436 @@ +"""Integration tests for the embedded OIDC provider. + +Flows run against the real app via app_client with a paired-terminal session +cookie — the same session mechanism browsers use. Storage assertions verify +that no plaintext secret/token material lands in the database. +""" + +import base64 +import hashlib +import secrets +from datetime import datetime, timedelta, timezone +from urllib.parse import parse_qs, urlparse + +from httpx import AsyncClient +from joserfc import jwt as joserfc_jwt +from joserfc.jwk import KeySet + +from shard_core.database.connection import db_conn +from shard_core.database import oidc as db_oidc +from shard_core.database import users as db_users +from shard_core.service import oidc_provider +from shard_core.service.identity import get_default_identity +from tests.util import pair_new_terminal + +AUTHORIZE = "public/oidc/authorize" +TOKEN = "public/oidc/token" +USERINFO = "public/oidc/userinfo" +JWKS = "public/oidc/jwks" +DISCOVERY = "public/oidc/.well-known/openid-configuration" + +REDIRECT_URI = "http://app.testserver/oauth/callback" + + +def s256(verifier: str) -> str: + digest = hashlib.sha256(verifier.encode()).digest() + return base64.urlsafe_b64encode(digest).rstrip(b"=").decode() + + +async def expected_issuer() -> str: + identity = await get_default_identity() + return f"https://{identity.domain}/core/public/oidc" + + +async def make_client(public_client: bool = False, scope: str = None) -> dict: + kwargs = {"scope": scope} if scope else {} + return await oidc_provider.register_client( + "testapp", [REDIRECT_URI], public_client=public_client, **kwargs + ) + + +async def authorize(client: AsyncClient, oidc_client: dict, verifier: str, **overrides): + params = { + "response_type": "code", + "client_id": oidc_client["client_id"], + "redirect_uri": REDIRECT_URI, + "scope": "openid profile email", + "state": secrets.token_urlsafe(8), + "nonce": secrets.token_urlsafe(8), + "code_challenge": s256(verifier), + "code_challenge_method": "S256", + **overrides, + } + params = {k: v for k, v in params.items() if v is not None} + return params, await client.get(AUTHORIZE, params=params) + + +async def get_code( + client: AsyncClient, oidc_client: dict, verifier: str, **overrides +) -> str: + params, r = await authorize(client, oidc_client, verifier, **overrides) + assert r.status_code == 302, r.text + location = urlparse(r.headers["location"]) + query = parse_qs(location.query) + registered = urlparse(params["redirect_uri"]) + assert (location.scheme, location.netloc, location.path) == ( + registered.scheme, + registered.netloc, + registered.path, + ) + assert query["state"] == [params["state"]] + return query["code"][0] + + +async def exchange_code( + client: AsyncClient, oidc_client: dict, code: str, verifier: str | None, **overrides +): + data = { + "grant_type": "authorization_code", + "code": code, + "redirect_uri": REDIRECT_URI, + **overrides, + } + if verifier is not None: + data["code_verifier"] = verifier + if oidc_client["client_secret"] is None: + data.setdefault("client_id", oidc_client["client_id"]) + auth = None + else: + auth = (oidc_client["client_id"], oidc_client["client_secret"]) + return await client.post(TOKEN, data=data, auth=auth) + + +async def run_code_flow( + client: AsyncClient, oidc_client: dict, scope: str = "openid profile email" +) -> tuple[dict, dict]: + verifier = secrets.token_urlsafe(32) + params, r = await authorize(client, oidc_client, verifier, scope=scope) + assert r.status_code == 302, r.text + code = parse_qs(urlparse(r.headers["location"]).query)["code"][0] + r = await exchange_code(client, oidc_client, code, verifier) + assert r.status_code == 200, r.text + return r.json(), params + + +async def jwks_keyset(client: AsyncClient) -> KeySet: + r = await client.get(JWKS) + assert r.status_code == 200 + return KeySet.import_key_set(r.json()) + + +async def get_owner() -> dict: + async with db_conn() as conn: + return await db_users.get_owner(conn) + + +# --- discovery + jwks --------------------------------------------------------- + + +async def test_discovery_document(app_client: AsyncClient): + r = await app_client.get(DISCOVERY) + assert r.status_code == 200 + disco = r.json() + + issuer = await expected_issuer() + assert disco["issuer"] == issuer + for endpoint in ( + "authorization_endpoint", + "token_endpoint", + "userinfo_endpoint", + "jwks_uri", + ): + assert disco[endpoint].startswith(issuer + "/"), f"{endpoint} not under issuer" + assert "code" in disco["response_types_supported"] + assert "public" in disco["subject_types_supported"] + assert "RS256" in disco["id_token_signing_alg_values_supported"] + assert "openid" in disco["scopes_supported"] + assert "S256" in disco["code_challenge_methods_supported"] + assert {"authorization_code", "refresh_token"} <= set( + disco["grant_types_supported"] + ) + assert {"client_secret_basic", "client_secret_post", "none"} <= set( + disco["token_endpoint_auth_methods_supported"] + ) + + +async def test_jwks_returns_rsa_sig_key_and_is_stable(app_client: AsyncClient): + r = await app_client.get(JWKS) + assert r.status_code == 200 + keys = r.json()["keys"] + assert len(keys) == 1 + key = keys[0] + assert key["kty"] == "RSA" + assert key["use"] == "sig" + assert key["alg"] == "RS256" + assert key.get("kid") + assert "n" in key and "e" in key + assert "d" not in key, "private key material exposed" + + r2 = await app_client.get(JWKS) + assert r2.json() == r.json(), "signing key must be persistent" + + +# --- code + PKCE flows ---------------------------------------------------------- + + +async def test_confidential_code_pkce_flow(app_client: AsyncClient): + await pair_new_terminal(app_client) + oidc_client = await make_client() + tok, params = await run_code_flow(app_client, oidc_client) + + assert tok["token_type"].lower() == "bearer" + assert all( + k in tok for k in ("access_token", "refresh_token", "id_token", "expires_in") + ) + + owner = await get_owner() + keyset = await jwks_keyset(app_client) + claims = joserfc_jwt.decode(tok["id_token"], keyset).claims + assert claims["iss"] == await expected_issuer() + assert claims["aud"] == [oidc_client["client_id"]] + assert claims["nonce"] == params["nonce"] + assert claims["sub"] == owner["id"] + assert claims["exp"] > claims["iat"] + + +async def test_id_token_omits_nonce_when_client_sent_none(app_client: AsyncClient): + """Strict clients (oauth4webapi) reject nonce:null — the claim must be absent.""" + await pair_new_terminal(app_client) + oidc_client = await make_client() + + verifier = secrets.token_urlsafe(32) + params, r = await authorize(app_client, oidc_client, verifier, nonce=None) + assert r.status_code == 302, r.text + code = parse_qs(urlparse(r.headers["location"]).query)["code"][0] + r = await exchange_code(app_client, oidc_client, code, verifier) + assert r.status_code == 200, r.text + + keyset = await jwks_keyset(app_client) + claims = joserfc_jwt.decode(r.json()["id_token"], keyset).claims + assert "nonce" not in claims + assert all(v is not None for v in claims.values()), f"null claim in {claims}" + + +async def test_public_client_pkce_flow(app_client: AsyncClient): + await pair_new_terminal(app_client) + oidc_client = await make_client(public_client=True) + assert oidc_client["client_secret"] is None + + tok, _ = await run_code_flow(app_client, oidc_client) + assert all(k in tok for k in ("access_token", "refresh_token", "id_token")) + + # PKCE is mandatory: a token request without code_verifier must fail + verifier = secrets.token_urlsafe(32) + code = await get_code(app_client, oidc_client, verifier) + r = await exchange_code(app_client, oidc_client, code, verifier=None) + assert r.status_code == 400, r.text + + +async def test_client_secret_post_accepted(app_client: AsyncClient): + """Immich authenticates with client_secret_post, not _basic.""" + await pair_new_terminal(app_client) + oidc_client = await make_client() + + verifier = secrets.token_urlsafe(32) + code = await get_code(app_client, oidc_client, verifier) + r = await app_client.post( + TOKEN, + data={ + "grant_type": "authorization_code", + "code": code, + "redirect_uri": REDIRECT_URI, + "code_verifier": verifier, + "client_id": oidc_client["client_id"], + "client_secret": oidc_client["client_secret"], + }, + ) + assert r.status_code == 200, r.text + + +# --- userinfo -------------------------------------------------------------------- + + +async def test_userinfo(app_client: AsyncClient): + await pair_new_terminal(app_client) + oidc_client = await make_client() + tok, _ = await run_code_flow(app_client, oidc_client) + + owner = await get_owner() + r = await app_client.get( + USERINFO, headers={"Authorization": f"Bearer {tok['access_token']}"} + ) + assert r.status_code == 200 + info = r.json() + assert info["sub"] == owner["id"] + assert info["email"] == owner["email"] + assert info["preferred_username"] == owner["username"] + + r = await app_client.get( + USERINFO, headers={"Authorization": "Bearer garbage-token"} + ) + assert r.status_code == 401 + assert r.headers["www-authenticate"] == "Bearer" + + r = await app_client.get(USERINFO) + assert r.status_code == 401 + + +# --- refresh rotation -------------------------------------------------------------- + + +async def test_refresh_rotation(app_client: AsyncClient): + await pair_new_terminal(app_client) + oidc_client = await make_client() + tok, _ = await run_code_flow(app_client, oidc_client) + auth = (oidc_client["client_id"], oidc_client["client_secret"]) + + r = await app_client.post( + TOKEN, + data={"grant_type": "refresh_token", "refresh_token": tok["refresh_token"]}, + auth=auth, + ) + assert r.status_code == 200, r.text + new_tok = r.json() + assert new_tok["access_token"] != tok["access_token"] + assert new_tok["refresh_token"] != tok["refresh_token"] + + r = await app_client.get( + USERINFO, headers={"Authorization": f"Bearer {new_tok['access_token']}"} + ) + assert r.status_code == 200 + + # old access token is revoked + r = await app_client.get( + USERINFO, headers={"Authorization": f"Bearer {tok['access_token']}"} + ) + assert r.status_code == 401 + + # rotated-out refresh token is unusable + r = await app_client.post( + TOKEN, + data={"grant_type": "refresh_token", "refresh_token": tok["refresh_token"]}, + auth=auth, + ) + assert r.status_code in (400, 401), r.text + + +# --- negatives ----------------------------------------------------------------------- + + +async def test_authorize_wrong_redirect_uri_is_not_open_redirect( + app_client: AsyncClient, +): + await pair_new_terminal(app_client) + oidc_client = await make_client() + _, r = await authorize( + app_client, + oidc_client, + secrets.token_urlsafe(32), + redirect_uri="http://evil.example/steal", + ) + assert not ( + 300 <= r.status_code < 400 + ), f"must not redirect, got {r.status_code} -> {r.headers.get('location')}" + assert r.status_code == 400, r.text + + +async def test_token_wrong_client_secret(app_client: AsyncClient): + await pair_new_terminal(app_client) + oidc_client = await make_client() + verifier = secrets.token_urlsafe(32) + code = await get_code(app_client, oidc_client, verifier) + bad = {**oidc_client, "client_secret": "wrong-" + secrets.token_urlsafe(16)} + r = await exchange_code(app_client, bad, code, verifier) + assert r.status_code == 401, r.text + + +async def test_token_unknown_code(app_client: AsyncClient): + await pair_new_terminal(app_client) + oidc_client = await make_client() + r = await exchange_code( + app_client, oidc_client, "no-such-code", secrets.token_urlsafe(32) + ) + assert r.status_code == 400, r.text + + +async def test_token_expired_code(app_client: AsyncClient): + await pair_new_terminal(app_client) + oidc_client = await make_client() + owner = await get_owner() + verifier = secrets.token_urlsafe(32) + code = "expired-" + secrets.token_urlsafe(16) + async with db_conn() as conn: + await db_oidc.insert_code( + conn, + { + "code_hash": oidc_provider.hash_secret(code), + "client_id": oidc_client["client_id"], + "redirect_uri": REDIRECT_URI, + "scope": "openid", + "user_sub": owner["id"], + "nonce": None, + "code_challenge": s256(verifier), + "code_challenge_method": "S256", + "auth_time": int(datetime.now(timezone.utc).timestamp()) - 600, + "expires_at": datetime.now(timezone.utc) - timedelta(seconds=1), + }, + ) + r = await exchange_code(app_client, oidc_client, code, verifier) + assert r.status_code == 400, r.text + + +async def test_code_reuse_rejected(app_client: AsyncClient): + await pair_new_terminal(app_client) + oidc_client = await make_client() + verifier = secrets.token_urlsafe(32) + code = await get_code(app_client, oidc_client, verifier) + r = await exchange_code(app_client, oidc_client, code, verifier) + assert r.status_code == 200, r.text + r = await exchange_code(app_client, oidc_client, code, verifier) + assert r.status_code == 400, r.text + + +async def test_anonymous_authorize_redirects_to_terminal_ui(app_client: AsyncClient): + oidc_client = await make_client() + _, r = await authorize(app_client, oidc_client, secrets.token_urlsafe(32)) + assert r.status_code == 302 + assert r.headers["location"].startswith("/?oidc_rd=") + + +async def test_scope_narrowing(app_client: AsyncClient): + """A client registered without "email" must not be granted it, even if requested.""" + await pair_new_terminal(app_client) + oidc_client = await make_client(scope="openid profile") + + tok, _ = await run_code_flow(app_client, oidc_client, scope="openid profile email") + granted = set((tok.get("scope") or "").split()) + assert "email" not in granted, f"scope escalation: granted {granted}" + assert {"openid", "profile"} <= granted + + +# --- storage hardening ----------------------------------------------------------------- + + +async def test_no_plaintext_tokens_in_database(app_client: AsyncClient): + await pair_new_terminal(app_client) + oidc_client = await make_client() + tok, _ = await run_code_flow(app_client, oidc_client) + + async with db_conn() as conn: + token_rows = await conn.execute("SELECT * FROM oidc_tokens") + all_values = [str(v) for row in await token_rows.fetchall() for v in row] + for secret_value in (tok["access_token"], tok["refresh_token"]): + assert not any(secret_value in v for v in all_values), "plaintext token stored" + + +async def test_token_endpoint_rate_limited(app_client: AsyncClient): + await pair_new_terminal(app_client) + oidc_client = await make_client() + + statuses = [] + for _ in range(oidc_provider.TOKEN_RATE_LIMIT + 5): + r = await exchange_code( + app_client, oidc_client, "no-such-code", secrets.token_urlsafe(32) + ) + statuses.append(r.status_code) + assert 429 in statuses, "token endpoint must rate-limit bursts" diff --git a/uv.lock b/uv.lock index d83fecb3..bcdff492 100644 --- a/uv.lock +++ b/uv.lock @@ -199,6 +199,19 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/64/b4/17d4b0b2a2dc85a6df63d1157e028ed19f90d4cd97c36717afef2bc2f395/attrs-26.1.0-py3-none-any.whl", hash = "sha256:c647aa4a12dfbad9333ca4e71fe62ddc36f4e63b2d260a37a8b83d2f043ac309", size = 67548, upload-time = "2026-03-19T14:22:23.645Z" }, ] +[[package]] +name = "authlib" +version = "1.7.2" +source = { registry = "https://pypi.org/simple" } +dependencies = [ + { name = "cryptography" }, + { name = "joserfc" }, +] +sdist = { url = "https://files.pythonhosted.org/packages/36/98/7d93f30d029643c0275dbc0bd6d5a6f670661ee6c9a94d93af7ab4887600/authlib-1.7.2.tar.gz", hash = "sha256:2cea25fefcd4e7173bdf1372c0afc265c8034b23a8cd5dcb6a9164b826c64231", size = 176511, upload-time = "2026-05-06T08:10:23.116Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/fb/95/adcb68e20c34162e9135f370d6e31737719c2b6f94bc953fe7ed1f10fe21/authlib-1.7.2-py2.py3-none-any.whl", hash = "sha256:3e1faedc9d87e7d56a164eca3ccb6ace0d61b94abe83e92242f8dc8bba9b4a9f", size = 259548, upload-time = "2026-05-06T08:10:21.436Z" }, +] + [[package]] name = "azure-core" version = "1.41.0" @@ -984,6 +997,18 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/62/a1/3d680cbfd5f4b8f15abc1d571870c5fc3e594bb582bc3b64ea099db13e56/jinja2-3.1.6-py3-none-any.whl", hash = "sha256:85ece4451f492d0c13c5dd7c13a64681a86afae63a5f347908daf103ce6d2f67", size = 134899, upload-time = "2025-03-05T20:05:00.369Z" }, ] +[[package]] +name = "joserfc" +version = "1.7.4" +source = { registry = "https://pypi.org/simple" } +dependencies = [ + { name = "cryptography" }, +] +sdist = { url = "https://files.pythonhosted.org/packages/c7/e0/27a6a081ae25420eda6768ceae05d7022a7f2447f420588843f2a44e4298/joserfc-1.7.4.tar.gz", hash = "sha256:b3bc561672ae541b17a9237053b48a03dacddd92d68047b3ecdfb4b5714a88ed", size = 234027, upload-time = "2026-07-19T15:43:02.739Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/f9/bf/249dcd99b3376375910b7fa922383b57792975c8758f50d44612e749226c/joserfc-1.7.4-py3-none-any.whl", hash = "sha256:32d46c2cd5e3203c13e87a6c61333cab310b1ba80cd54b4c4f386a848a122463", size = 71000, upload-time = "2026-07-19T15:43:01.299Z" }, +] + [[package]] name = "markdown-it-py" version = "4.2.0" @@ -1820,6 +1845,7 @@ dependencies = [ { name = "aiohttp" }, { name = "aiozipstream" }, { name = "asgi-lifespan" }, + { name = "authlib" }, { name = "azure-storage-blob" }, { name = "bitstring" }, { name = "blinker" }, @@ -1867,6 +1893,7 @@ requires-dist = [ { name = "aioresponses", marker = "extra == 'dev'" }, { name = "aiozipstream" }, { name = "asgi-lifespan", specifier = "==2.*" }, + { name = "authlib", specifier = ">=1.7.2,<1.8" }, { name = "azure-storage-blob" }, { name = "bitstring" }, { name = "blinker" }, From fcca2380bc546e3aae0a178ebec5cfbf8a235b7b Mon Sep 17 00:00:00 2001 From: ClaydeCode Date: Sat, 11 Jul 2026 10:58:53 +0000 Subject: [PATCH 2/5] fix(oidc): adapt provider to numeric user ids from phase-1 review user_sub columns are BIGINT FKs; ShardUser carries the numeric id and the OIDC sub claim is its stringified form. Co-Authored-By: Claude Fable 5 --- migrations/shard-core-0003-oidc.sql | 4 ++-- shard_core/service/oidc_provider.py | 26 +++++++++++++++----------- tests/test_oidc.py | 4 ++-- 3 files changed, 19 insertions(+), 15 deletions(-) diff --git a/migrations/shard-core-0003-oidc.sql b/migrations/shard-core-0003-oidc.sql index 82f1d250..6bfa810d 100644 --- a/migrations/shard-core-0003-oidc.sql +++ b/migrations/shard-core-0003-oidc.sql @@ -16,7 +16,7 @@ CREATE TABLE IF NOT EXISTS oidc_codes ( client_id TEXT NOT NULL REFERENCES oidc_clients (client_id) ON DELETE CASCADE, redirect_uri TEXT, scope TEXT, - user_sub TEXT NOT NULL REFERENCES users (id) ON DELETE CASCADE, + user_sub BIGINT NOT NULL REFERENCES users (id) ON DELETE CASCADE, nonce TEXT, code_challenge TEXT, code_challenge_method TEXT, @@ -28,7 +28,7 @@ CREATE TABLE IF NOT EXISTS oidc_tokens ( access_token_hash TEXT PRIMARY KEY, refresh_token_hash TEXT UNIQUE, client_id TEXT NOT NULL REFERENCES oidc_clients (client_id) ON DELETE CASCADE, - user_sub TEXT NOT NULL REFERENCES users (id) ON DELETE CASCADE, + user_sub BIGINT NOT NULL REFERENCES users (id) ON DELETE CASCADE, scope TEXT, issued_at BIGINT NOT NULL, expires_in BIGINT NOT NULL, diff --git a/shard_core/service/oidc_provider.py b/shard_core/service/oidc_provider.py index 8c8c863f..e946832c 100644 --- a/shard_core/service/oidc_provider.py +++ b/shard_core/service/oidc_provider.py @@ -2,8 +2,7 @@ Authorization server for first-party app clients: authorization-code + PKCE + refresh, RS256 id_tokens, discovery, JWKS, userinfo. Users come from the -`users` table (phase 1); the OIDC `sub` is the user id, which for the owner is -the shard's default identity id. +`users` table (phase 1); the OIDC `sub` is the stringified numeric user id. Built on Authlib's framework-agnostic server classes; see `shard_core.web.public.oidc` for the FastAPI adapter. Authlib's server core is @@ -74,7 +73,7 @@ def _run(coro): class ShardUser: """The resource owner: a row from the users table.""" - sub: str + id: int username: str display_name: str email: str | None = None @@ -84,19 +83,24 @@ def from_row(cls, row: dict | None): if row is None or row["disabled"]: return None return cls( - sub=row["id"], + id=row["id"], username=row["username"], display_name=row["display_name"], email=row["email"], ) + @property + def sub(self) -> str: + # OIDC subject claims are strings; the numeric user id is the identity + return str(self.id) + def get_user_id(self): return self.sub -async def _user_from_sub_async(sub: str) -> ShardUser | None: +async def _user_from_id_async(user_id: int) -> ShardUser | None: async with db_conn() as conn: - return ShardUser.from_row(await db_users.get_by_id(conn, sub)) + return ShardUser.from_row(await db_users.get_by_id(conn, user_id)) @dataclass @@ -166,7 +170,7 @@ class AuthorizationCode(AuthorizationCodeMixin): client_id: str redirect_uri: str | None scope: str | None - user_sub: str + user_sub: int nonce: str | None code_challenge: str | None code_challenge_method: str | None @@ -248,7 +252,7 @@ def save_authorization_code(self, code, request): # the client's allowed scope — payload.scope would let a client # escalate beyond its registered scope "scope": request.scope, - "user_sub": request.user.sub, + "user_sub": request.user.id, "nonce": data.get("nonce"), "code_challenge": data.get("code_challenge"), "code_challenge_method": data.get("code_challenge_method"), @@ -267,7 +271,7 @@ def delete_authorization_code(self, authorization_code): _run(_with_conn(db_oidc.delete_code, hash_secret(authorization_code.code))) def authenticate_user(self, authorization_code): - return _run(_user_from_sub_async(authorization_code.user_sub)) + return _run(_user_from_id_async(authorization_code.user_sub)) class ShardRefreshTokenGrant(grants.RefreshTokenGrant): @@ -282,7 +286,7 @@ def authenticate_refresh_token(self, refresh_token): return None def authenticate_user(self, credential): - return _run(_user_from_sub_async(credential.row["user_sub"])) + return _run(_user_from_id_async(credential.row["user_sub"])) def revoke_old_credential(self, credential): _run(_with_conn(db_oidc.revoke_token, credential.row["access_token_hash"])) @@ -363,7 +367,7 @@ def _save_token(token: dict, request): else None ), "client_id": request.client.client_id, - "user_sub": request.user.sub, + "user_sub": request.user.id, "scope": token.get("scope"), "issued_at": int(time.time()), "expires_in": token["expires_in"], diff --git a/tests/test_oidc.py b/tests/test_oidc.py index 94a933b0..fff82e98 100644 --- a/tests/test_oidc.py +++ b/tests/test_oidc.py @@ -189,7 +189,7 @@ async def test_confidential_code_pkce_flow(app_client: AsyncClient): assert claims["iss"] == await expected_issuer() assert claims["aud"] == [oidc_client["client_id"]] assert claims["nonce"] == params["nonce"] - assert claims["sub"] == owner["id"] + assert claims["sub"] == str(owner["id"]) assert claims["exp"] > claims["iat"] @@ -261,7 +261,7 @@ async def test_userinfo(app_client: AsyncClient): ) assert r.status_code == 200 info = r.json() - assert info["sub"] == owner["id"] + assert info["sub"] == str(owner["id"]) assert info["email"] == owner["email"] assert info["preferred_username"] == owner["username"] From d3357785ccabc32f9d321a9452f21f2c1e92c02d Mon Sep 17 00:00:00 2001 From: ClaydeCode Date: Sat, 11 Jul 2026 18:11:13 +0000 Subject: [PATCH 3/5] fix(oidc): ShardUser builds from User model (db module returns objects now) Co-Authored-By: Claude Fable 5 --- shard_core/service/oidc_provider.py | 17 +++++++++-------- shard_core/web/public/oidc.py | 2 +- tests/test_oidc.py | 12 ++++++------ 3 files changed, 16 insertions(+), 15 deletions(-) diff --git a/shard_core/service/oidc_provider.py b/shard_core/service/oidc_provider.py index e946832c..adb273e4 100644 --- a/shard_core/service/oidc_provider.py +++ b/shard_core/service/oidc_provider.py @@ -37,6 +37,7 @@ from shard_core.database import oidc as db_oidc from shard_core.database import users as db_users from shard_core.database.connection import db_conn +from shard_core.data_model.user import User log = logging.getLogger(__name__) @@ -79,14 +80,14 @@ class ShardUser: email: str | None = None @classmethod - def from_row(cls, row: dict | None): - if row is None or row["disabled"]: + def from_user(cls, user: User | None): + if user is None or user.disabled: return None return cls( - id=row["id"], - username=row["username"], - display_name=row["display_name"], - email=row["email"], + id=user.id, + username=user.username, + display_name=user.display_name, + email=user.email, ) @property @@ -100,7 +101,7 @@ def get_user_id(self): async def _user_from_id_async(user_id: int) -> ShardUser | None: async with db_conn() as conn: - return ShardUser.from_row(await db_users.get_by_id(conn, user_id)) + return ShardUser.from_user(await db_users.get_by_id(conn, user_id)) @dataclass @@ -431,7 +432,7 @@ async def userinfo_for_access_token(access_token: str) -> dict | None: row = await db_oidc.get_token_by_access_hash(conn, hash_secret(access_token)) if row is None or row["issued_at"] + row["expires_in"] < time.time(): return None - user = ShardUser.from_row(await db_users.get_by_id(conn, row["user_sub"])) + user = ShardUser.from_user(await db_users.get_by_id(conn, row["user_sub"])) if user is None: return None return dict( diff --git a/shard_core/web/public/oidc.py b/shard_core/web/public/oidc.py index 51a7ea13..bc70b904 100644 --- a/shard_core/web/public/oidc.py +++ b/shard_core/web/public/oidc.py @@ -130,7 +130,7 @@ async def _session_user(authorization: str | None) -> ShardUser | None: if terminal.user_id is None: return None async with db_conn() as conn: - return ShardUser.from_row(await db_users.get_by_id(conn, terminal.user_id)) + return ShardUser.from_user(await db_users.get_by_id(conn, terminal.user_id)) # --- token endpoint rate limit ------------------------------------------------------ diff --git a/tests/test_oidc.py b/tests/test_oidc.py index fff82e98..eb33dd04 100644 --- a/tests/test_oidc.py +++ b/tests/test_oidc.py @@ -118,7 +118,7 @@ async def jwks_keyset(client: AsyncClient) -> KeySet: return KeySet.import_key_set(r.json()) -async def get_owner() -> dict: +async def get_owner(): async with db_conn() as conn: return await db_users.get_owner(conn) @@ -189,7 +189,7 @@ async def test_confidential_code_pkce_flow(app_client: AsyncClient): assert claims["iss"] == await expected_issuer() assert claims["aud"] == [oidc_client["client_id"]] assert claims["nonce"] == params["nonce"] - assert claims["sub"] == str(owner["id"]) + assert claims["sub"] == str(owner.id) assert claims["exp"] > claims["iat"] @@ -261,9 +261,9 @@ async def test_userinfo(app_client: AsyncClient): ) assert r.status_code == 200 info = r.json() - assert info["sub"] == str(owner["id"]) - assert info["email"] == owner["email"] - assert info["preferred_username"] == owner["username"] + assert info["sub"] == str(owner.id) + assert info["email"] == owner.email + assert info["preferred_username"] == owner.username r = await app_client.get( USERINFO, headers={"Authorization": "Bearer garbage-token"} @@ -367,7 +367,7 @@ async def test_token_expired_code(app_client: AsyncClient): "client_id": oidc_client["client_id"], "redirect_uri": REDIRECT_URI, "scope": "openid", - "user_sub": owner["id"], + "user_sub": owner.id, "nonce": None, "code_challenge": s256(verifier), "code_challenge_method": "S256", From ba1ea678f58a194e9c91b5fa05812b2aa5568999 Mon Sep 17 00:00:00 2001 From: ClaydeCode Date: Sat, 11 Jul 2026 18:43:26 +0000 Subject: [PATCH 4/5] =?UTF-8?q?feat(oidc):=20security=20hardening=20from?= =?UTF-8?q?=20OIDC=20review=20=E2=80=94=20S256-only=20PKCE,=20atomic=20cod?= =?UTF-8?q?e=20redemption,=20reuse=20family=20revocation?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Applies the three findings from the 2026-07-11 security review (RFC 9700): - PKCE accepts S256 only. Authlib's default also allows 'plain', where the challenge travels as the cleartext verifier — useless against the authorization-request interception PKCE exists for. Discovery no longer advertises plain. - Authorization codes are consumed atomically (single UPDATE ... RETURNING on a redeemed flag), closing the TOCTOU race where two concurrent /token requests could both redeem one code. Any redemption attempt burns the code, including ones that later fail PKCE. Redeemed rows are kept until expiry: reuse of a redeemed code signals interception and revokes every token issued to that (client, user) grant. Side effect: the nonce-replay window now actually spans the code lifetime. - Replay of a rotated-out refresh token revokes the whole token family, so a thief who rotates first no longer keeps a live token while the legit client is silently rejected. Per-client rate-limit buckets (finding 4) deferred — the global in-process guard stays; noted as follow-up when it matters. Co-Authored-By: Claude Fable 5 --- migrations/shard-core-0003-oidc.sql | 3 +- shard_core/database/oidc.py | 33 ++++++++++---- shard_core/service/oidc_provider.py | 47 +++++++++++++++++--- tests/test_oidc.py | 69 ++++++++++++++++++++++++++++- 4 files changed, 136 insertions(+), 16 deletions(-) diff --git a/migrations/shard-core-0003-oidc.sql b/migrations/shard-core-0003-oidc.sql index 6bfa810d..5e91f6cd 100644 --- a/migrations/shard-core-0003-oidc.sql +++ b/migrations/shard-core-0003-oidc.sql @@ -21,7 +21,8 @@ CREATE TABLE IF NOT EXISTS oidc_codes ( code_challenge TEXT, code_challenge_method TEXT, auth_time BIGINT NOT NULL, - expires_at TIMESTAMPTZ NOT NULL + expires_at TIMESTAMPTZ NOT NULL, + redeemed BOOLEAN NOT NULL DEFAULT FALSE ); CREATE TABLE IF NOT EXISTS oidc_tokens ( diff --git a/shard_core/database/oidc.py b/shard_core/database/oidc.py index e1da01d6..6cf75e8e 100644 --- a/shard_core/database/oidc.py +++ b/shard_core/database/oidc.py @@ -46,22 +46,30 @@ async def insert_code(conn: AsyncConnection, code: dict): await conn.execute(sql, code) +async def redeem_code( + conn: AsyncConnection, code_hash: str, client_id: str +) -> dict | None: + """Atomically consume the code — the single UPDATE makes concurrent + redemptions of the same code impossible (only one caller gets the row).""" + sql: LiteralString = """UPDATE oidc_codes SET redeemed = TRUE + WHERE code_hash = %s AND client_id = %s AND NOT redeemed AND expires_at > now() + RETURNING *""" + async with conn.cursor(row_factory=dict_row) as cur: + await cur.execute(sql, (code_hash, client_id)) + return await cur.fetchone() + + async def get_code( conn: AsyncConnection, code_hash: str, client_id: str ) -> dict | None: sql: LiteralString = ( - "SELECT * FROM oidc_codes WHERE code_hash = %s AND client_id = %s AND expires_at > now()" + "SELECT * FROM oidc_codes WHERE code_hash = %s AND client_id = %s" ) async with conn.cursor(row_factory=dict_row) as cur: await cur.execute(sql, (code_hash, client_id)) return await cur.fetchone() -async def delete_code(conn: AsyncConnection, code_hash: str): - sql: LiteralString = "DELETE FROM oidc_codes WHERE code_hash = %s" - await conn.execute(sql, (code_hash,)) - - async def exists_nonce(conn: AsyncConnection, nonce: str, client_id: str) -> bool: sql: LiteralString = "SELECT 1 FROM oidc_codes WHERE nonce = %s AND client_id = %s" async with conn.cursor() as cur: @@ -94,9 +102,9 @@ async def get_token_by_access_hash( async def get_token_by_refresh_hash( conn: AsyncConnection, refresh_token_hash: str ) -> dict | None: - sql: LiteralString = ( - "SELECT * FROM oidc_tokens WHERE refresh_token_hash = %s AND NOT revoked" - ) + # revoked rows included on purpose — rotated-token replay must be + # distinguishable from an unknown token (reuse detection) + sql: LiteralString = "SELECT * FROM oidc_tokens WHERE refresh_token_hash = %s" async with conn.cursor(row_factory=dict_row) as cur: await cur.execute(sql, (refresh_token_hash,)) return await cur.fetchone() @@ -107,3 +115,10 @@ async def revoke_token(conn: AsyncConnection, access_token_hash: str): "UPDATE oidc_tokens SET revoked = TRUE WHERE access_token_hash = %s" ) await conn.execute(sql, (access_token_hash,)) + + +async def revoke_all_for_grant(conn: AsyncConnection, client_id: str, user_sub: int): + sql: LiteralString = ( + "UPDATE oidc_tokens SET revoked = TRUE WHERE client_id = %s AND user_sub = %s" + ) + await conn.execute(sql, (client_id, user_sub)) diff --git a/shard_core/service/oidc_provider.py b/shard_core/service/oidc_provider.py index adb273e4..57bb5036 100644 --- a/shard_core/service/oidc_provider.py +++ b/shard_core/service/oidc_provider.py @@ -265,11 +265,31 @@ def save_authorization_code(self, code, request): ) def query_authorization_code(self, code, client): - row = _run(_with_conn(db_oidc.get_code, hash_secret(code), client.client_id)) + # atomic burn-on-query: any redemption attempt (even one that later + # fails PKCE) consumes the code, and two concurrent requests can't + # both get it (RFC 9700 single-use) + row = _run(_with_conn(db_oidc.redeem_code, hash_secret(code), client.client_id)) + if row is None: + stale = _run( + _with_conn(db_oidc.get_code, hash_secret(code), client.client_id) + ) + if stale and stale["redeemed"]: + # reuse of a redeemed code signals interception — kill every + # token issued to this (client, user) grant + _run( + _with_conn( + db_oidc.revoke_all_for_grant, + client.client_id, + stale["user_sub"], + ) + ) + return None return AuthorizationCode.from_row(code, row) def delete_authorization_code(self, authorization_code): - _run(_with_conn(db_oidc.delete_code, hash_secret(authorization_code.code))) + # already consumed atomically in query_authorization_code; the row is + # kept (redeemed) for reuse detection until it expires + pass def authenticate_user(self, authorization_code): return _run(_user_from_id_async(authorization_code.user_sub)) @@ -282,7 +302,18 @@ def authenticate_refresh_token(self, refresh_token): row = _run( _with_conn(db_oidc.get_token_by_refresh_hash, hash_secret(refresh_token)) ) - if row and row["issued_at"] + REFRESH_TOKEN_LIFETIME > time.time(): + if row is None: + return None + if row["revoked"]: + # replay of a rotated-out refresh token — revoke the whole family + # so a thief who rotated first doesn't keep a live token + _run( + _with_conn( + db_oidc.revoke_all_for_grant, row["client_id"], row["user_sub"] + ) + ) + return None + if row["issued_at"] + REFRESH_TOKEN_LIFETIME > time.time(): return _TokenRecord(row) return None @@ -381,6 +412,12 @@ def _query_client(client_id: str): return OidcClient.from_row(_run(_with_conn(db_oidc.get_client, client_id))) +class S256CodeChallenge(CodeChallenge): + # 'plain' (Authlib's default second method) sends challenge == verifier in + # cleartext, defeating PKCE's interception protection (RFC 9700 wants S256) + SUPPORTED_CODE_CHALLENGE_METHOD = ["S256"] + + def build_authorization_server(server_cls=AuthorizationServer) -> AuthorizationServer: """server_cls lets the web layer pass its framework-adapter subclass.""" server = server_cls(scopes_supported=SUPPORTED_SCOPES) @@ -389,7 +426,7 @@ def build_authorization_server(server_cls=AuthorizationServer) -> AuthorizationS server.register_token_generator("default", _generate_bearer_token) server.register_grant( ShardAuthCodeGrant, - [CodeChallenge(required=True), ShardOpenIDCode(require_nonce=False)], + [S256CodeChallenge(required=True), ShardOpenIDCode(require_nonce=False)], ) server.register_grant(ShardRefreshTokenGrant) return server @@ -460,5 +497,5 @@ def discovery_document() -> dict: "client_secret_post", "none", ], - "code_challenge_methods_supported": ["S256", "plain"], + "code_challenge_methods_supported": ["S256"], } diff --git a/tests/test_oidc.py b/tests/test_oidc.py index eb33dd04..72855558 100644 --- a/tests/test_oidc.py +++ b/tests/test_oidc.py @@ -144,7 +144,7 @@ async def test_discovery_document(app_client: AsyncClient): assert "public" in disco["subject_types_supported"] assert "RS256" in disco["id_token_signing_alg_values_supported"] assert "openid" in disco["scopes_supported"] - assert "S256" in disco["code_challenge_methods_supported"] + assert disco["code_challenge_methods_supported"] == ["S256"] assert {"authorization_code", "refresh_token"} <= set( disco["grant_types_supported"] ) @@ -313,6 +313,12 @@ async def test_refresh_rotation(app_client: AsyncClient): ) assert r.status_code in (400, 401), r.text + # ...and its replay is treated as compromise: the whole family dies + r = await app_client.get( + USERINFO, headers={"Authorization": f"Bearer {new_tok['access_token']}"} + ) + assert r.status_code == 401, "family not revoked after refresh-token reuse" + # --- negatives ----------------------------------------------------------------------- @@ -408,6 +414,67 @@ async def test_scope_narrowing(app_client: AsyncClient): assert {"openid", "profile"} <= granted +async def test_plain_pkce_rejected(app_client: AsyncClient): + """RFC 9700: only S256 — 'plain' sends the verifier in cleartext.""" + await pair_new_terminal(app_client) + oidc_client = await make_client() + verifier = secrets.token_urlsafe(32) + params, r = await authorize( + app_client, + oidc_client, + verifier, + code_challenge=verifier, + code_challenge_method="plain", + ) + if 300 <= r.status_code < 400: + query = parse_qs(urlparse(r.headers["location"]).query) + assert "code" not in query, "code issued for plain PKCE" + assert "error" in query + else: + assert r.status_code == 400, r.text + + +async def test_failed_redemption_burns_code(app_client: AsyncClient): + """Any redemption attempt consumes the code — a PKCE-failing attempt + must not leave the code redeemable.""" + await pair_new_terminal(app_client) + oidc_client = await make_client() + verifier = secrets.token_urlsafe(32) + code = await get_code(app_client, oidc_client, verifier) + + r = await exchange_code(app_client, oidc_client, code, "wrong-" + verifier) + assert r.status_code == 400, r.text + + r = await exchange_code(app_client, oidc_client, code, verifier) + assert r.status_code == 400, "code survived a failed redemption attempt" + + +async def test_code_reuse_revokes_issued_tokens(app_client: AsyncClient): + """Reuse of a redeemed code signals interception — tokens issued to the + grant must be revoked (RFC 9700).""" + await pair_new_terminal(app_client) + oidc_client = await make_client() + verifier = secrets.token_urlsafe(32) + code = await get_code(app_client, oidc_client, verifier) + + r = await exchange_code(app_client, oidc_client, code, verifier) + assert r.status_code == 200, r.text + tok = r.json() + + r = await app_client.get( + USERINFO, headers={"Authorization": f"Bearer {tok['access_token']}"} + ) + assert r.status_code == 200 + + r = await exchange_code(app_client, oidc_client, code, verifier) + assert r.status_code == 400, r.text + + r = await app_client.get( + USERINFO, headers={"Authorization": f"Bearer {tok['access_token']}"} + ) + assert r.status_code == 401, "tokens not revoked after code reuse" + + # --- storage hardening ----------------------------------------------------------------- From 2a65b76d0c777aa748650e33b26fbf4c1d1d5e12 Mon Sep 17 00:00:00 2001 From: ClaydeCode Date: Mon, 27 Jul 2026 11:55:27 +0000 Subject: [PATCH 5/5] fix(oidc): build the request URL from the shard's domain instead of trusting proxy headers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Authlib refuses OAuth flows on plain http, and the URL Starlette sees inside the container is http — Traefik terminates TLS. The previous fix set FORWARDED_ALLOW_IPS=* so uvicorn's ProxyHeadersMiddleware would take the scheme from X-Forwarded-Proto. That trusts those headers from every peer, not just Traefik, and every installed app shares the portal network and can reach shard_core directly. The middleware also rewrites request.client from X-Forwarded-For, and /internal/call_peer identifies the calling app by request.client.host to decide which subdomain the shard's identity key signs a peer request for — so the wildcard turns a direct-reach path into app impersonation (see issue #188 for the underlying weakness). Narrowing the value to Traefik's address is not available: uvicorn reads the variable once at process start, core compose starts traefik only after shard_core is healthy, container IPs are dynamic, a hostname never matches (the middleware compares against the peer IP), and the portal subnet contains every app container. The OAuth2Request is built by hand here, so the URL handed to Authlib does not have to come from the request. Rebuild it from the default identity's domain — the same source the issuer already uses — and drop the env var. The provider no longer reads X-Forwarded-* at all, and request.client keeps meaning the real socket peer. This also fixes the anonymous-authorize return leg. Traefik strips the /core/ prefix, so str(request.url) lacked it and the oidc_rd URL pointed at https:///public/oidc/authorize, which matches the catch-all web-terminal router rather than shard_core — the browser would never get back to the authorize endpoint after pairing. Reverting _public_url in that line fails the extended test with the unprefixed URL. Co-Authored-By: Claude Opus 5 (1M context) --- Dockerfile | 3 --- shard_core/web/public/oidc.py | 19 ++++++++++++++++--- tests/test_oidc.py | 11 +++++++++-- 3 files changed, 25 insertions(+), 8 deletions(-) diff --git a/Dockerfile b/Dockerfile index 8340893d..5952cac6 100644 --- a/Dockerfile +++ b/Dockerfile @@ -47,9 +47,6 @@ ENV PATH="/app/.venv/bin:$PATH" HEALTHCHECK --start-period=5s --interval=30s --timeout=5s CMD curl -f http://localhost/public/health #ENV FLASK_APP=shard_core -# Trust X-Forwarded-* from the in-network Traefik so request URLs are https; -# Authlib refuses OAuth flows on plain http (InsecureTransportError). -ENV FORWARDED_ALLOW_IPS=* EXPOSE 80 ENTRYPOINT ["/usr/bin/tini", "--"] CMD ["fastapi", "run", "--port", "80", "shard_core/app.py"] diff --git a/shard_core/web/public/oidc.py b/shard_core/web/public/oidc.py index bc70b904..310d62fc 100644 --- a/shard_core/web/public/oidc.py +++ b/shard_core/web/public/oidc.py @@ -62,13 +62,25 @@ def form(self): return self._form +def _public_url(request: Request) -> str: + """The URL this request has from outside, rebuilt from the shard's own domain. + + Traefik terminates TLS and strips the `/core/` prefix, so the URL Starlette + sees is neither https nor prefixed. Rebuilding it from the default identity + keeps the provider off the X-Forwarded-* headers entirely — an app container + on the portal network can reach shard_core directly and set them at will. + """ + url = f"{request.app.state.oidc_public_base}{request.url.path}" + return f"{url}?{request.url.query}" if request.url.query else url + + async def _build_oauth2_request(request: Request) -> StarletteOAuth2Request: form = {} if request.method == "POST": form = {k: v for k, v in (await request.form()).items()} return StarletteOAuth2Request( method=request.method, - url=str(request.url), + url=_public_url(request), # Authlib looks headers up by canonical name ("Authorization"); # Starlette normalizes to lowercase — bridge with a CI dict. headers=CaseInsensitiveDict(request.headers), @@ -108,7 +120,8 @@ async def _get_server(request: Request) -> StarletteAuthorizationServer: i = await identity.get_default_identity() protocol = "http" if settings().traefik.disable_ssl else "https" # Traefik routes /core/* to shard_core with /core/ stripped - issuer = f"{protocol}://{i.domain}/core/public/oidc" + app.state.oidc_public_base = f"{protocol}://{i.domain}/core" + issuer = f"{app.state.oidc_public_base}/public/oidc" jwk = await ensure_jwk() configure(issuer, jwk, asyncio.get_running_loop()) _token_request_times.clear() @@ -170,7 +183,7 @@ async def authorize(request: Request, authorization: str = Cookie(None)): user = await _session_user(authorization) if user is None: # No shard session: send the browser to the terminal's pairing/login UI. - rd = quote(str(request.url), safe="") + rd = quote(_public_url(request), safe="") return RedirectResponse(f"/?oidc_rd={rd}", status_code=302) oreq = await _build_oauth2_request(request) return await asyncio.to_thread(server.create_authorization_response, oreq, user) diff --git a/tests/test_oidc.py b/tests/test_oidc.py index 72855558..b5c02448 100644 --- a/tests/test_oidc.py +++ b/tests/test_oidc.py @@ -9,7 +9,7 @@ import hashlib import secrets from datetime import datetime, timedelta, timezone -from urllib.parse import parse_qs, urlparse +from urllib.parse import parse_qs, unquote, urlparse from httpx import AsyncClient from joserfc import jwt as joserfc_jwt @@ -398,10 +398,17 @@ async def test_code_reuse_rejected(app_client: AsyncClient): async def test_anonymous_authorize_redirects_to_terminal_ui(app_client: AsyncClient): oidc_client = await make_client() - _, r = await authorize(app_client, oidc_client, secrets.token_urlsafe(32)) + params, r = await authorize(app_client, oidc_client, secrets.token_urlsafe(32)) assert r.status_code == 302 assert r.headers["location"].startswith("/?oidc_rd=") + # The return leg must be the URL the browser can reach: Traefik strips + # /core/, so an unprefixed path routes to the web-terminal, not here. + rd = unquote(r.headers["location"].removeprefix("/?oidc_rd=")) + identity = await get_default_identity() + assert rd.startswith(f"https://{identity.domain}/core/{AUTHORIZE}?") + assert f"client_id={params['client_id']}" in rd + async def test_scope_narrowing(app_client: AsyncClient): """A client registered without "email" must not be granted it, even if requested."""