diff --git a/.gitignore b/.gitignore index 2e4bf9a1..5b52e1f4 100644 --- a/.gitignore +++ b/.gitignore @@ -82,6 +82,7 @@ celerybeat-schedule # dotenv .env +!tests/mock_app_store/*/.env # virtualenv .venv/ diff --git a/config.toml b/config.toml index cc54a6ee..acaff8ef 100644 --- a/config.toml +++ b/config.toml @@ -73,6 +73,9 @@ enabled = false enabled = false send_interval_seconds = 300 +[oidc] +enabled = false + [management] api_url = "https://ptlfunctionapp.azurewebsites.net/api/management" diff --git a/docker-compose.yml b/docker-compose.yml index 8f957f8b..93713c7b 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -66,6 +66,7 @@ services: - FREESHARD_TRAEFIK__DISABLE_SSL=${DISABLE_SSL:-false} - FREESHARD_PATH_ROOT_HOST=${FREESHARD_DIR:?} - FREESHARD_APPS__LIFECYCLE__PAUSE_ENABLED=${PAUSE_ENABLED:-false} + - FREESHARD_OIDC__ENABLED=${OIDC_ENABLED:-false} depends_on: postgres: condition: service_healthy diff --git a/migrations/shard-core-0004-oidc.sql b/migrations/shard-core-0004-oidc.sql new file mode 100644 index 00000000..aaf13951 --- /dev/null +++ b/migrations/shard-core-0004-oidc.sql @@ -0,0 +1,50 @@ +-- shard-core-0004-oidc +-- depends: shard-core-0003-app-status-message + +CREATE TABLE IF NOT EXISTS oidc_clients ( + client_id TEXT PRIMARY KEY, + client_secret TEXT, + app_name TEXT UNIQUE NOT NULL + REFERENCES installed_apps (name) ON DELETE CASCADE, + 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() +); + +-- terminal_id is ON DELETE SET NULL, deliberately not CASCADE: un-pairing must +-- leave the rows in place. A deleted token row is indistinguishable from an +-- unknown one, which loses both the ability to deny a presented token and the +-- refresh-token reuse detection that get_token_by_refresh_hash relies on. +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 BIGINT NOT NULL REFERENCES users (id) ON DELETE CASCADE, + terminal_id TEXT REFERENCES terminals (id) ON DELETE SET NULL, + sid TEXT NOT NULL, + nonce TEXT, + code_challenge TEXT, + code_challenge_method TEXT, + auth_time BIGINT NOT NULL, + expires_at TIMESTAMPTZ NOT NULL, + redeemed BOOLEAN NOT NULL DEFAULT FALSE +); + +CREATE INDEX IF NOT EXISTS oidc_codes_terminal_id_idx ON oidc_codes (terminal_id); + +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 BIGINT NOT NULL REFERENCES users (id) ON DELETE CASCADE, + terminal_id TEXT REFERENCES terminals (id) ON DELETE SET NULL, + sid TEXT NOT NULL, + scope TEXT, + issued_at BIGINT NOT NULL, + expires_in BIGINT NOT NULL, + revoked BOOLEAN NOT NULL DEFAULT FALSE +); + +CREATE INDEX IF NOT EXISTS oidc_tokens_terminal_id_idx ON oidc_tokens (terminal_id); diff --git a/pyproject.toml b/pyproject.toml index a00dbc50..d201b3d1 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/data_model/oidc.py b/shard_core/data_model/oidc.py new file mode 100644 index 00000000..c6c8ff47 --- /dev/null +++ b/shard_core/data_model/oidc.py @@ -0,0 +1,48 @@ +from datetime import datetime + +from pydantic import BaseModel + + +class OidcClient(BaseModel): + """A row of oidc_clients — the OIDC client registered for an installed app.""" + + client_id: str + client_secret: str | None = None + app_name: str + redirect_uris: list[str] + scope: str + token_endpoint_auth_method: str + created: datetime | None = None + + +class OidcCode(BaseModel): + """A row of oidc_codes. The code itself is only ever stored as a digest.""" + + code_hash: str + client_id: str + redirect_uri: str | None = None + scope: str | None = None + user_sub: int + terminal_id: str | None = None + sid: str + nonce: str | None = None + code_challenge: str | None = None + code_challenge_method: str | None = None + auth_time: int + expires_at: datetime + redeemed: bool = False + + +class OidcToken(BaseModel): + """A row of oidc_tokens. Both tokens are only ever stored as digests.""" + + access_token_hash: str + refresh_token_hash: str | None = None + client_id: str + user_sub: int + terminal_id: str | None = None + sid: str + scope: str | None = None + issued_at: int + expires_in: int + revoked: bool = False diff --git a/shard_core/database/connection.py b/shard_core/database/connection.py index c8fef09c..12de6950 100644 --- a/shard_core/database/connection.py +++ b/shard_core/database/connection.py @@ -53,3 +53,9 @@ def get_connection_pool() -> AsyncConnectionPool: async def db_conn() -> AsyncGenerator[AsyncConnection, None]: async with get_connection_pool().connection() as conn: yield conn + + +async def with_conn(fn, *args): + """Run a single conn-first database function on its own pooled connection.""" + async with db_conn() as conn: + return await fn(conn, *args) diff --git a/shard_core/database/oidc.py b/shard_core/database/oidc.py new file mode 100644 index 00000000..f8ebb56e --- /dev/null +++ b/shard_core/database/oidc.py @@ -0,0 +1,146 @@ +from typing import LiteralString + +from psycopg import AsyncConnection +from psycopg.rows import class_row +from psycopg.types.json import Jsonb + +from shard_core.data_model.oidc import OidcClient, OidcCode, OidcToken + +# --- clients --------------------------------------------------------------- + + +async def upsert_client(conn: AsyncConnection, client: dict) -> OidcClient: + 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=class_row(OidcClient)) 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) -> OidcClient | None: + sql: LiteralString = "SELECT * FROM oidc_clients WHERE client_id = %s" + async with conn.cursor(row_factory=class_row(OidcClient)) 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, terminal_id, sid, 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, + %(terminal_id)s, %(sid)s, + %(nonce)s, %(code_challenge)s, %(code_challenge_method)s, + %(auth_time)s, %(expires_at)s)""" + await conn.execute(sql, code) + + +async def redeem_code( + conn: AsyncConnection, code_hash: str, client_id: str +) -> OidcCode | 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=class_row(OidcCode)) 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 +) -> OidcCode | None: + sql: LiteralString = ( + "SELECT * FROM oidc_codes WHERE code_hash = %s AND client_id = %s" + ) + async with conn.cursor(row_factory=class_row(OidcCode)) as cur: + await cur.execute(sql, (code_hash, client_id)) + return await cur.fetchone() + + +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, terminal_id, sid, + scope, issued_at, expires_in) + VALUES (%(access_token_hash)s, %(refresh_token_hash)s, %(client_id)s, %(user_sub)s, + %(terminal_id)s, %(sid)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 +) -> OidcToken | None: + sql: LiteralString = ( + "SELECT * FROM oidc_tokens WHERE access_token_hash = %s AND NOT revoked" + ) + async with conn.cursor(row_factory=class_row(OidcToken)) 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 +) -> OidcToken | None: + # 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=class_row(OidcToken)) 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,)) + + +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)) + + +async def revoke_for_terminal(conn: AsyncConnection, terminal_id: str): + """Kill everything issued to one device, on un-pair. + + Must run before the terminals row is deleted — the FK is ON DELETE SET NULL, + so afterwards nothing matches terminal_id any more. + """ + revoke_tokens: LiteralString = ( + "UPDATE oidc_tokens SET revoked = TRUE WHERE terminal_id = %s" + ) + burn_codes: LiteralString = ( + "UPDATE oidc_codes SET redeemed = TRUE WHERE terminal_id = %s AND NOT redeemed" + ) + await conn.execute(revoke_tokens, (terminal_id,)) + await conn.execute(burn_codes, (terminal_id,)) diff --git a/shard_core/service/oidc_provider.py b/shard_core/service/oidc_provider.py new file mode 100644 index 00000000..389c8218 --- /dev/null +++ b/shard_core/service/oidc_provider.py @@ -0,0 +1,536 @@ +"""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 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 +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 +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, with_conn +from shard_core.data_model.oidc import OidcClient, OidcCode, OidcToken +from shard_core.data_model.user import User + +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 +BRIDGE_TIMEOUT = 10 # seconds a storage hook waits on the event loop; +# matches the connection pool's own timeout — waiting longer than the pool +# will spend acquiring a connection cannot help, and outliving the app's +# 20s lifespan shutdown would leave a worker thread on a loop that is gone + +_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): + """Run a coroutine on the main event loop from inside a worker thread. + + Authlib's server core is synchronous, so request handling runs in a thread + (asyncio.to_thread). Its storage hooks still need Postgres, and the psycopg + pool belongs to the loop that opened it — a connection cannot be driven from + another thread, and asyncio.run() here would create a second loop the pool + knows nothing about. So the coroutine goes back to the original loop and + this thread blocks on the result: loop -> thread -> same loop. + + This does not deadlock because the route awaits its to_thread call, leaving + the loop free to service what we submit. Block the loop on the thread + instead and it deadlocks immediately. + """ + loop = _state["loop"] + if loop is None or loop.is_closed(): + raise RuntimeError( + "OIDC provider has no running event loop — reach it through _get_server()" + ) + future = asyncio.run_coroutine_threadsafe(coro, loop) + try: + return future.result(BRIDGE_TIMEOUT) + except TimeoutError: + # the coroutine would otherwise keep running and commit under a request + # that has already failed + future.cancel() + raise + + +# --- models ------------------------------------------------------------------ + + +@dataclass +class ShardUser: + """The resource owner: a row from the users table.""" + + id: int + username: str + display_name: str + email: str | None = None + # set only on the /authorize path, where the terminal cookie is in hand; + # None when the user is reconstructed from a stored grant + terminal_id: str | None = None + + @classmethod + def from_user(cls, user: User | None): + if user is None or user.disabled: + return None + return cls( + id=user.id, + username=user.username, + display_name=user.display_name, + email=user.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_id_async(user_id: int) -> ShardUser | None: + async with db_conn() as conn: + return ShardUser.from_user(await db_users.get_by_id(conn, user_id)) + + +class AuthlibClient(OidcClient, ClientMixin): + """The stored client (data_model.oidc.OidcClient) plus what Authlib calls. + + Every method below exists for Authlib's grants, which look them up by name + on the client object. Nothing in this codebase calls them directly, so an + apparent lack of callers is expected rather than dead code. + """ + + grant_types: list[str] = ["authorization_code", "refresh_token"] + + @classmethod + def from_row(cls, row: OidcClient | None): + return cls(**row.model_dump()) if row is not None else None + + 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: int + terminal_id: str | None + sid: str + nonce: str | None + code_challenge: str | None + code_challenge_method: str | None + auth_time: int + + @classmethod + def from_row(cls, code: str, row: OidcCode | None): + if row is None: + return None + stored = row.model_dump() + fields = {k: stored[k] for k in cls.__dataclass_fields__ if k in stored} + 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) --------------------------- + + +# --- 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.id, + "terminal_id": request.user.terminal_id, + # opaque per-authorization session id: what back-channel + # logout targets. Deliberately not the terminal id — apps + # have no business learning shard-internal identifiers. + "sid": secrets.token_urlsafe(16), + "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): + # 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): + # 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)) + + +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 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 + + def authenticate_user(self, credential): + 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)) + + +class _TokenRecord: + def __init__(self, row: OidcToken): + self.row = row + + @property + def terminal_id(self): + return self.row.terminal_id + + @property + def sid(self): + return self.row.sid + + 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) + claims = {k: v for k, v in claims.items() if v is not None} + claims["sid"] = authorization_code.sid + return claims + + 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): + # request.user is rebuilt from the stored grant and has no terminal, so the + # binding comes from the credential: the code on first issue, the previous + # token on rotation. + credential = getattr(request, "authorization_code", None) or getattr( + request, "refresh_token", None + ) + _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.id, + "terminal_id": credential.terminal_id, + "sid": credential.sid, + "scope": token.get("scope"), + "issued_at": int(time.time()), + "expires_in": token["expires_in"], + }, + ) + ) + + +def _query_client(client_id: str): + return AuthlibClient.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) + server.query_client = _query_client + server.save_token = _save_token + server.register_token_generator("default", _generate_bearer_token) + server.register_grant( + ShardAuthCodeGrant, + [S256CodeChallenge(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", +) -> OidcClient: + """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: + stored = await db_oidc.upsert_client(conn, row) + log.info(f"registered OIDC client for app {app_name}") + return stored + + +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_user(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"], + } diff --git a/shard_core/settings.py b/shard_core/settings.py index 10fb2ebe..6a6c08a1 100644 --- a/shard_core/settings.py +++ b/shard_core/settings.py @@ -82,6 +82,10 @@ class TelemetrySettings(BaseModel): send_interval_seconds: int = 300 +class OidcSettings(BaseModel): + enabled: bool = False # rollout kill-switch for the embedded OIDC provider + + class ManagementSettings(BaseModel): api_url: str @@ -121,6 +125,7 @@ class Settings(BaseSettings): traefik: TraefikSettings apps: AppsSettings telemetry: TelemetrySettings = TelemetrySettings() + oidc: OidcSettings = OidcSettings() management: ManagementSettings freeshard_controller: FreeshardControllerSettings log: LogSettings = LogSettings() diff --git a/shard_core/web/protected/terminals.py b/shard_core/web/protected/terminals.py index 66d0fcf4..ef41b024 100644 --- a/shard_core/web/protected/terminals.py +++ b/shard_core/web/protected/terminals.py @@ -5,6 +5,7 @@ from fastapi.responses import Response from shard_core.database.connection import db_conn +from shard_core.database import oidc as db_oidc from shard_core.database import terminals as db_terminals from shard_core.data_model.terminal import Terminal, InputTerminal from shard_core.service import pairing @@ -60,6 +61,7 @@ async def edit_terminal(id_: str, terminal: InputTerminal): ) async def delete_terminal_by_id(id_: str): async with db_conn() as conn: + await db_oidc.revoke_for_terminal(conn, id_) await db_terminals.remove(conn, id_) await on_terminals_update.send_async() 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..aa073828 --- /dev/null +++ b/shard_core/web/public/oidc.py @@ -0,0 +1,251 @@ +"""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, Depends, HTTPException, Request, status +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__) + +# traefik routes /core/* here and strips the prefix +CORE_PREFIX = "/core" + + +def _require_enabled(): + """404 rather than 403 — a shard without the rollout must not advertise an IdP.""" + if not settings().oidc.enabled: + raise HTTPException(status.HTTP_404_NOT_FOUND) + + +router = APIRouter( + prefix="/oidc", + tags=["/public/oidc"], + dependencies=[Depends(_require_enabled)], +) + + +# --- 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 + + +def _public_url(request: Request) -> str: + """The URL this request has from outside. + + Traefik terminates TLS and strips the `/core/` prefix, so the URL Starlette + sees is neither https nor prefixed. Scheme and host come from the forwarded + headers, the prefix from our own traefik config, and the shard's default + identity is the fallback when the headers are absent (direct access, tests). + + Read here by hand rather than through uvicorn's proxy-header middleware: + that middleware also rewrites request.client from X-Forwarded-For, and + /internal/call_peer identifies the calling app by request.client.host, so + enabling it process-wide would turn direct reach into app impersonation + (#188). Taking the headers for the URL alone leaves request.client meaning + the real socket peer. + """ + proto = request.headers.get("x-forwarded-proto") + host = request.headers.get("x-forwarded-host") + base = ( + f"{proto}://{host}{CORE_PREFIX}" + if proto and host + else request.app.state.oidc_public_base + ) + url = f"{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=_public_url(request), + # 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 + 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() + 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: + user = ShardUser.from_user(await db_users.get_by_id(conn, terminal.user_id)) + if user is not None: + # carried into the code and token rows, so un-pairing this device + # revokes what it authorized + user.terminal_id = terminal.id + return user + + +# --- 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(_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) + + +@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/config.toml b/tests/config.toml index d4794028..bb881a15 100644 --- a/tests/config.toml +++ b/tests/config.toml @@ -19,3 +19,6 @@ shard_core = "info" [telemetry] enabled = true + +[oidc] +enabled = true diff --git a/tests/conftest.py b/tests/conftest.py index c6eab9cf..a0ed34f8 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -4,12 +4,13 @@ import logging import re import shutil +import tempfile from contextlib import contextmanager from dataclasses import dataclass from datetime import datetime, timedelta from logging import LogRecord from pathlib import Path -from typing import List, AsyncGenerator +from typing import List, AsyncGenerator, Generator import psycopg import pytest @@ -39,9 +40,9 @@ from shard_core.settings import Settings, set_settings, reset_settings from shard_core.web.internal.call_peer import _get_app_for_ip_address from tests.util import ( + build_mock_app_store_zips, docker_network_portal, wait_until_all_apps_installed, - mock_app_store_path, ) pytest_plugins = ("pytest_asyncio",) @@ -177,14 +178,16 @@ async def db(): @pytest_asyncio.fixture -async def api_client(requests_mock, mocker) -> AsyncGenerator[AsyncClient]: +async def api_client( + requests_mock, mocker, mock_app_store_zips +) -> AsyncGenerator[AsyncClient]: # Modules that define some global state need to be reloaded importlib.reload(websocket) importlib.reload(app_installation.worker) importlib.reload(telemetry) # Mocks must be set up after modules are reloaded or else they will be overwritten - mock_app_store(mocker) + mock_app_store(mocker, mock_app_store_zips) async def noop(): pass @@ -368,9 +371,16 @@ def peer_mock_requests(mocker): _get_app_for_ip_address.cache_clear() -def mock_app_store(mocker): +@pytest.fixture(scope="session") +def mock_app_store_zips() -> Generator[Path]: + """The mock app store packed into zips, the shape the store serves them in.""" + with tempfile.TemporaryDirectory() as target_root: + yield build_mock_app_store_zips(Path(target_root)) + + +def mock_app_store(mocker, zips_path: Path): async def mock_download_app_zip(name: str, _=None) -> Path: - source_zip = mock_app_store_path() / name / f"{name}.zip" + source_zip = zips_path / name / f"{name}.zip" target_zip = get_installed_apps_path() / name / f"{name}.zip" target_zip.parent.mkdir(parents=True, exist_ok=True) shutil.copy(source_zip, target_zip.parent) @@ -383,7 +393,7 @@ async def mock_download_app_zip(name: str, _=None) -> Path: ) async def mock_app_exists_in_store(name: str) -> bool: - source_zip = mock_app_store_path() / name / f"{name}.zip" + source_zip = zips_path / name / f"{name}.zip" return source_zip.exists() mocker.patch( diff --git a/tests/mock_app_store/always_on/always_on.zip b/tests/mock_app_store/always_on/always_on.zip deleted file mode 100644 index 137f32d2..00000000 Binary files a/tests/mock_app_store/always_on/always_on.zip and /dev/null differ diff --git a/tests/mock_app_store/always_on/docker-compose.yml.template b/tests/mock_app_store/always_on/docker-compose.yml.template index e84ee8d3..1d43f561 100644 --- a/tests/mock_app_store/always_on/docker-compose.yml.template +++ b/tests/mock_app_store/always_on/docker-compose.yml.template @@ -1,5 +1,7 @@ +version: '3.5' + networks: - shard: + portal: external: true services: @@ -7,7 +9,5 @@ services: restart: always image: nginx:alpine container_name: always_on - ports: - - 80:80 networks: - portal diff --git a/tests/mock_app_store/filebrowser/docker-compose.yml.template b/tests/mock_app_store/filebrowser/docker-compose.yml.template index 8bf9b458..4e0f75ac 100644 --- a/tests/mock_app_store/filebrowser/docker-compose.yml.template +++ b/tests/mock_app_store/filebrowser/docker-compose.yml.template @@ -1,3 +1,5 @@ +version: '3.5' + networks: portal: external: true @@ -7,8 +9,6 @@ services: restart: always image: portalapps.azurecr.io/ptl-apps/filebrowser:master container_name: filebrowser - ports: - - 80:80 volumes: - "{{ fs.app_data }}:/data" - "{{ fs.shared }}:/data/shared" diff --git a/tests/mock_app_store/filebrowser/filebrowser.zip b/tests/mock_app_store/filebrowser/filebrowser.zip deleted file mode 100644 index 4d00791c..00000000 Binary files a/tests/mock_app_store/filebrowser/filebrowser.zip and /dev/null differ diff --git a/tests/mock_app_store/immich/.env b/tests/mock_app_store/immich/.env new file mode 100644 index 00000000..1f657065 --- /dev/null +++ b/tests/mock_app_store/immich/.env @@ -0,0 +1,19 @@ +# You can find documentation for all the supported env variables at https://immich.app/docs/install/environment-variables + +# The location where your uploaded files are stored +UPLOAD_LOCATION=./library + +# The Immich version to use. You can pin this to a specific version like "v1.71.0" +IMMICH_VERSION=v2.2.3 + +# Connection secrets for postgres and typesense. You should change these to random passwords +# TYPESENSE_API_KEY=some-random-text +DB_PASSWORD=postgres + +# The values below this line do not need to be changed +################################################################################### +DB_HOSTNAME=immich_postgres +DB_USERNAME=postgres +DB_DATABASE_NAME=immich + +REDIS_HOSTNAME=immich_redis diff --git a/tests/mock_app_store/immich/docker-compose.yml.template b/tests/mock_app_store/immich/docker-compose.yml.template index 5b6f1d3f..de65570f 100644 --- a/tests/mock_app_store/immich/docker-compose.yml.template +++ b/tests/mock_app_store/immich/docker-compose.yml.template @@ -7,8 +7,6 @@ services: restart: always image: nginx:alpine container_name: immich - ports: - - 80:80 volumes: - "{{ fs.app_data }}:/data" - "{{ fs.shared }}:/data/shared" diff --git a/tests/mock_app_store/immich/immich.zip b/tests/mock_app_store/immich/immich.zip deleted file mode 100644 index 93b31857..00000000 Binary files a/tests/mock_app_store/immich/immich.zip and /dev/null differ diff --git a/tests/mock_app_store/large_app/app_meta.json b/tests/mock_app_store/large_app/app_meta.json deleted file mode 100644 index fac37e32..00000000 --- a/tests/mock_app_store/large_app/app_meta.json +++ /dev/null @@ -1,59 +0,0 @@ -{ - "v": "1.0", - "app_version": "0.1.0", - "name": "large_app", - "icon": "icon.svg", - "entrypoints": [ - { - "container_name": "large_app", - "container_port": 80, - "entrypoint_port": "http" - }, - { - "container_name": "large_app", - "container_port": 1883, - "entrypoint_port": "mqtt" - } - ], - "paths": { - "": { - "access": "private", - "headers": { - "X-Ptl-Client-Id": "{{ auth.client_id }}", - "X-Ptl-Client-Name": "{{ auth.client_name }}", - "X-Ptl-Client-Type": "{{ auth.client_type }}", - "X-Ptl-ID": "{{ portal.id }}", - "X-Ptl-Foo": "bar" - } - }, - "/pub": { - "access": "public", - "headers": { - "X-Ptl-Client-Id": "{{ auth.client_id }}", - "X-Ptl-Client-Name": "{{ auth.client_name }}", - "X-Ptl-Client-Type": "{{ auth.client_type }}", - "X-Ptl-ID": "{{ portal.id }}", - "X-Ptl-Foo": "baz" - } - }, - "/peer": { - "access": "peer", - "headers": { - "X-Ptl-Client-Id": "{{ auth.client_id }}", - "X-Ptl-Client-Name": "{{ auth.client_name }}", - "X-Ptl-Client-Type": "{{ auth.client_type }}", - "X-Ptl-ID": "{{ portal.id }}", - "X-Ptl-Foo": "foo" - } - } - }, - "minimum_portal_size": "m", - "lifecycle": { - "always_on": false, - "idle_time_for_shutdown": 3600 - }, - "store_info": { - "description_short": "an app for mocking", - "is_featured": true - } -} diff --git a/tests/mock_app_store/large_app/docker-compose.yml.template b/tests/mock_app_store/large_app/docker-compose.yml.template deleted file mode 100644 index a4a3bba3..00000000 --- a/tests/mock_app_store/large_app/docker-compose.yml.template +++ /dev/null @@ -1,17 +0,0 @@ -networks: - portal: - external: true - -services: - large_app: - restart: always - image: nginx:alpine - container_name: large_app - ports: - - 80:80 - volumes: - - "{{ fs.app_data }}:/data" - - "{{ fs.shared }}:/data/shared" - - "/var/run/docker.sock:/var/run/docker.sock:ro" - networks: - - portal diff --git a/tests/mock_app_store/large_app/icon.svg b/tests/mock_app_store/large_app/icon.svg deleted file mode 100644 index 5e78eccf..00000000 --- a/tests/mock_app_store/large_app/icon.svg +++ /dev/null @@ -1,147 +0,0 @@ - -image/svg+xml - - - - - \ No newline at end of file diff --git a/tests/mock_app_store/large_app/large_app.zip b/tests/mock_app_store/large_app/large_app.zip deleted file mode 100644 index f191af4b..00000000 Binary files a/tests/mock_app_store/large_app/large_app.zip and /dev/null differ diff --git a/tests/mock_app_store/mock_app/docker-compose.yml.template b/tests/mock_app_store/mock_app/docker-compose.yml.template index 8d071b08..74c5a5bc 100644 --- a/tests/mock_app_store/mock_app/docker-compose.yml.template +++ b/tests/mock_app_store/mock_app/docker-compose.yml.template @@ -1,3 +1,5 @@ +version: '3.5' + networks: portal: external: true @@ -7,8 +9,6 @@ services: restart: always image: nginx:alpine container_name: mock_app - ports: - - 80:80 volumes: - "{{ fs.app_data }}:/data" - "{{ fs.shared }}:/data/shared" diff --git a/tests/mock_app_store/mock_app/mock_app.zip b/tests/mock_app_store/mock_app/mock_app.zip deleted file mode 100644 index 3202c374..00000000 Binary files a/tests/mock_app_store/mock_app/mock_app.zip and /dev/null differ diff --git a/tests/mock_app_store/paperless-ngx/docker-compose.yml.template b/tests/mock_app_store/paperless-ngx/docker-compose.yml.template index 3ff6d6d5..a1535e34 100644 --- a/tests/mock_app_store/paperless-ngx/docker-compose.yml.template +++ b/tests/mock_app_store/paperless-ngx/docker-compose.yml.template @@ -7,8 +7,6 @@ services: restart: always image: nginx:alpine container_name: paperless-ngx - ports: - - 80:80 volumes: - "{{ fs.app_data }}:/data" - "{{ fs.shared }}:/data/shared" diff --git a/tests/mock_app_store/paperless-ngx/paperless-ngx.zip b/tests/mock_app_store/paperless-ngx/paperless-ngx.zip deleted file mode 100644 index c696ccec..00000000 Binary files a/tests/mock_app_store/paperless-ngx/paperless-ngx.zip and /dev/null differ diff --git a/tests/mock_app_store/pause_cycle/pause_cycle.zip b/tests/mock_app_store/pause_cycle/pause_cycle.zip deleted file mode 100644 index 5b77c864..00000000 Binary files a/tests/mock_app_store/pause_cycle/pause_cycle.zip and /dev/null differ diff --git a/tests/mock_app_store/quick_stop/docker-compose.yml.template b/tests/mock_app_store/quick_stop/docker-compose.yml.template index a07f76e3..156e3ae8 100644 --- a/tests/mock_app_store/quick_stop/docker-compose.yml.template +++ b/tests/mock_app_store/quick_stop/docker-compose.yml.template @@ -7,7 +7,5 @@ services: restart: always image: nginx:alpine container_name: quick_stop - ports: - - 80:80 networks: - portal diff --git a/tests/mock_app_store/quick_stop/quick_stop.zip b/tests/mock_app_store/quick_stop/quick_stop.zip deleted file mode 100644 index c564805b..00000000 Binary files a/tests/mock_app_store/quick_stop/quick_stop.zip and /dev/null differ diff --git a/tests/test_app_install_crash_recovery.py b/tests/test_app_install_crash_recovery.py index 4ff560d2..4fdcea7b 100644 --- a/tests/test_app_install_crash_recovery.py +++ b/tests/test_app_install_crash_recovery.py @@ -13,7 +13,7 @@ from shard_core.service.app_installation import worker from shard_core.service.app_tools import get_installed_apps_path from tests.conftest import mock_app_store -from tests.util import docker_network_portal, mock_app_store_path, retry_async +from tests.util import docker_network_portal, retry_async pytest_plugins = ("pytest_asyncio",) pytestmark = pytest.mark.asyncio @@ -151,7 +151,7 @@ def _reload_stateful_modules(): ], ) async def test_unrecoverable_stranded_row_boots_and_settles_to_error( - requests_mock, mocker, status + requests_mock, mocker, status, mock_app_store_zips ): """A row with no recoverable source boots the shard and ends in ERROR. @@ -160,7 +160,7 @@ async def test_unrecoverable_stranded_row_boots_and_settles_to_error( reconcile picks the wrong branch, giving it teeth. """ _reload_stateful_modules() - mock_app_store(mocker) + mock_app_store(mocker, mock_app_store_zips) async def noop(): pass @@ -180,12 +180,14 @@ async def app_row_is_error(): @pytest.mark.parametrize("status", [Status.INSTALLATION_QUEUED, Status.INSTALLING]) -async def test_reconciled_install_completes_on_boot(requests_mock, mocker, status): +async def test_reconciled_install_completes_on_boot( + requests_mock, mocker, status, mock_app_store_zips +): """A stranded install whose zip is on disk is re-queued by reconcile and the worker drives it to STOPPED — proving reconcile's task_type and status reset line up with the worker's status assertion end to end.""" _reload_stateful_modules() - mock_app_store(mocker) + mock_app_store(mocker, mock_app_store_zips) async def noop(): pass @@ -193,7 +195,7 @@ async def noop(): mocker.patch("shard_core.service.app_installation.login_docker_registries", noop) await _seed_via_own_connection(status, InstallationReason.CUSTOM) - src = mock_app_store_path() / APP_NAME / f"{APP_NAME}.zip" + src = mock_app_store_zips / APP_NAME / f"{APP_NAME}.zip" dst = get_installed_apps_path() / APP_NAME / f"{APP_NAME}.zip" dst.parent.mkdir(parents=True, exist_ok=True) shutil.copy(src, dst) diff --git a/tests/test_app_installation.py b/tests/test_app_installation.py index bdd08677..1cfde6be 100644 --- a/tests/test_app_installation.py +++ b/tests/test_app_installation.py @@ -1,5 +1,6 @@ import io import zipfile +from pathlib import Path import docker import pytest @@ -13,7 +14,6 @@ from shard_core.service.app_tools import get_installed_apps_path from tests.util import ( wait_until_app_installed, - mock_app_store_path, wait_until_app_uninstalled, ) @@ -146,26 +146,26 @@ async def _upload_custom_app(api_client: AsyncClient, filename: str, content: by ) -def _mock_app_zip_bytes() -> bytes: - return (mock_app_store_path() / "mock_app" / "mock_app.zip").read_bytes() +def _mock_app_zip_bytes(zips_path: Path) -> bytes: + return (zips_path / "mock_app" / "mock_app.zip").read_bytes() -def _nested_mock_app_zip_bytes() -> bytes: +def _nested_mock_app_zip_bytes(zips_path: Path) -> bytes: """The mock_app zip, but with all files inside a single top-level directory.""" buffer = io.BytesIO() - with zipfile.ZipFile(io.BytesIO(_mock_app_zip_bytes())) as source: + with zipfile.ZipFile(io.BytesIO(_mock_app_zip_bytes(zips_path))) as source: with zipfile.ZipFile(buffer, "w") as target: for name in source.namelist(): target.writestr(f"mock_app/{name}", source.read(name)) return buffer.getvalue() -async def test_install_custom_app(api_client: AsyncClient): +async def test_install_custom_app(api_client: AsyncClient, mock_app_store_zips): app_name = "mock_app" docker_client = docker.from_env() response = await _upload_custom_app( - api_client, f"{app_name}.zip", _mock_app_zip_bytes() + api_client, f"{app_name}.zip", _mock_app_zip_bytes(mock_app_store_zips) ) assert response.status_code == status.HTTP_201_CREATED @@ -177,9 +177,13 @@ async def test_install_custom_app(api_client: AsyncClient): assert len(response) == 4 -async def test_install_custom_app_uses_name_from_app_meta(api_client: AsyncClient): +async def test_install_custom_app_uses_name_from_app_meta( + api_client: AsyncClient, mock_app_store_zips +): response = await _upload_custom_app( - api_client, "some-unrelated-filename.zip", _mock_app_zip_bytes() + api_client, + "some-unrelated-filename.zip", + _mock_app_zip_bytes(mock_app_store_zips), ) assert response.status_code == status.HTTP_201_CREATED @@ -189,12 +193,14 @@ async def test_install_custom_app_uses_name_from_app_meta(api_client: AsyncClien assert response.status_code == status.HTTP_404_NOT_FOUND -async def test_install_custom_app_with_single_top_level_dir(api_client: AsyncClient): +async def test_install_custom_app_with_single_top_level_dir( + api_client: AsyncClient, mock_app_store_zips +): app_name = "mock_app" docker_client = docker.from_env() response = await _upload_custom_app( - api_client, f"{app_name}.zip", _nested_mock_app_zip_bytes() + api_client, f"{app_name}.zip", _nested_mock_app_zip_bytes(mock_app_store_zips) ) assert response.status_code == status.HTTP_201_CREATED diff --git a/tests/test_app_lifecycle.py b/tests/test_app_lifecycle.py index 18bcb325..a8e92f79 100644 --- a/tests/test_app_lifecycle.py +++ b/tests/test_app_lifecycle.py @@ -115,6 +115,4 @@ async def assert_app_running(): await retry_async(assert_app_running, timeout=30, retry_errors=[AssertionError]) -# todo: test_large_app_does_not_start - # todo: test app with size comparison diff --git a/tests/test_app_meta_migration.py b/tests/test_app_meta_migration.py index a97f818c..9890396c 100644 --- a/tests/test_app_meta_migration.py +++ b/tests/test_app_meta_migration.py @@ -1,5 +1,4 @@ import json -import zipfile from pathlib import Path from shard_core.data_model.app_meta import AppMeta @@ -76,18 +75,17 @@ def test_migrate_1_2_to_1_3_missing_lifecycle_gets_defaults(): def test_all_mock_app_store_metas_migrate(): """Every app_meta.json fixture (the app-repository stand-ins) must load through the migration chain without regeneration.""" - zips = sorted(MOCK_APP_STORE.glob("*/*.zip")) - assert zips, "expected mock app store zips" - for zip_path in zips: - with zipfile.ZipFile(zip_path) as zf: - values = json.loads(zf.read("app_meta.json")) + metas = sorted(MOCK_APP_STORE.glob("*/app_meta.json")) + assert metas, "expected mock app store app_meta.json files" + for meta_path in metas: + values = json.loads(meta_path.read_text()) old_lifecycle = values.get("lifecycle", {}) app_meta = AppMeta.model_validate(values) - assert app_meta.v == "1.3", zip_path + assert app_meta.v == "1.3", meta_path if old_lifecycle.get("always_on"): - assert app_meta.lifecycle.always_on is True, zip_path + assert app_meta.lifecycle.always_on is True, meta_path elif "idle_time_for_shutdown" in old_lifecycle: assert ( app_meta.lifecycle.idle_for_pause == old_lifecycle["idle_time_for_shutdown"] - ), zip_path + ), meta_path diff --git a/tests/test_app_zip.py b/tests/test_app_zip.py index f50bb43c..321ec738 100644 --- a/tests/test_app_zip.py +++ b/tests/test_app_zip.py @@ -9,7 +9,6 @@ validate_app_zip, ) from shard_core.service.app_installation.exceptions import InvalidAppZip -from tests.util import mock_app_store_path def _app_meta(**overrides) -> str: @@ -203,8 +202,8 @@ def test_extract_rejects_path_traversal(tmp_path): assert not (tmp_path / "installed_apps" / "escaped.txt").exists() -def test_extract_mock_app_store_zip(tmp_path): - zip_file = mock_app_store_path() / "mock_app" / "mock_app.zip" +def test_extract_mock_app_store_zip(tmp_path, mock_app_store_zips): + zip_file = mock_app_store_zips / "mock_app" / "mock_app.zip" target_dir = tmp_path / "installed_apps" / "mock_app" target_dir.mkdir(parents=True) diff --git a/tests/test_oidc.py b/tests/test_oidc.py new file mode 100644 index 00000000..ad690b17 --- /dev/null +++ b/tests/test_oidc.py @@ -0,0 +1,672 @@ +"""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, unquote, urlparse + +import pytest +from httpx import AsyncClient +from joserfc import jwt as joserfc_jwt +from joserfc.jwk import KeySet + +from psycopg.rows import dict_row + +from shard_core.data_model.oidc import OidcClient +from shard_core.database.connection import db_conn +from shard_core.database import oidc as db_oidc +from shard_core.database import terminals as db_terminals +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) -> OidcClient: + kwargs = {"scope": scope} if scope else {} + await _ensure_app_row("testapp") + return await oidc_provider.register_client( + "testapp", [REDIRECT_URI], public_client=public_client, **kwargs + ) + + +async def _ensure_app_row(name: str): + """oidc_clients.app_name is an FK, so the app has to exist before its client.""" + async with db_conn() as conn: + await conn.execute( + "INSERT INTO installed_apps (name) VALUES (%s) ON CONFLICT DO NOTHING", + (name,), + ) + + +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(): + 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 disco["code_challenge_methods_supported"] == ["S256"] + 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"] == str(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"] == 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"} + ) + 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 + + # ...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 ----------------------------------------------------------------------- + + +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.model_copy( + update={"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, + "terminal_id": None, + "sid": secrets.token_urlsafe(16), + "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() + 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.""" + 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 + + +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 ----------------------------------------------------------------- + + +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" + + +@pytest.mark.config_override({"oidc": {"enabled": False}}) +async def test_provider_absent_when_disabled(app_client: AsyncClient): + await pair_new_terminal(app_client) + for path in (DISCOVERY, JWKS, AUTHORIZE, USERINFO): + r = await app_client.get(path) + assert r.status_code == 404, f"{path} answered {r.status_code}" + r = await app_client.post(TOKEN, data={"grant_type": "authorization_code"}) + assert r.status_code == 404, r.text + + +# --- terminal binding + revocation on un-pair ----------------------------------- + + +async def rows(sql: str, *args) -> list[dict]: + async with db_conn() as conn: + async with conn.cursor(row_factory=dict_row) as cur: + await cur.execute(sql, args) + return await cur.fetchall() + + +async def terminal_id(name: str) -> str: + async with db_conn() as conn: + return (await db_terminals.get_by_name(conn, name))["id"] + + +async def unpair(client: AsyncClient, id_: str): + r = await client.delete(f"protected/terminals/id/{id_}") + assert r.status_code == 204, r.text + + +async def test_code_and_token_are_bound_to_the_authorizing_terminal( + app_client: AsyncClient, +): + await pair_new_terminal(app_client, name="laptop") + tid = await terminal_id("laptop") + oidc_client = await make_client() + + await run_code_flow(app_client, oidc_client) + + code_rows = await rows("SELECT * FROM oidc_codes") + assert len(code_rows) == 1 + assert code_rows[0]["terminal_id"] == tid + assert code_rows[0]["sid"], "every authorization must get a session id" + + token_rows = await rows("SELECT * FROM oidc_tokens") + assert len(token_rows) == 1 + assert token_rows[0]["terminal_id"] == tid + assert token_rows[0]["sid"] == code_rows[0]["sid"] + + +async def test_sid_is_opaque_and_not_the_terminal_id(app_client: AsyncClient): + await pair_new_terminal(app_client, name="laptop") + tid = await terminal_id("laptop") + await run_code_flow(app_client, await make_client()) + + sid = (await rows("SELECT * FROM oidc_codes"))[0]["sid"] + assert sid != tid, "sid must not leak the shard-internal terminal id to apps" + + +async def test_id_token_carries_sid(app_client: AsyncClient): + await pair_new_terminal(app_client) + tok, _ = await run_code_flow(app_client, await make_client()) + + keyset = await jwks_keyset(app_client) + claims = joserfc_jwt.decode(tok["id_token"], keyset).claims + assert claims["sid"] == (await rows("SELECT * FROM oidc_codes"))[0]["sid"] + + +async def test_refresh_preserves_the_terminal_binding(app_client: AsyncClient): + await pair_new_terminal(app_client, name="laptop") + tid = await terminal_id("laptop") + oidc_client = await make_client() + tok, _ = await run_code_flow(app_client, oidc_client) + sid = (await rows("SELECT * FROM oidc_codes"))[0]["sid"] + + r = await app_client.post( + TOKEN, + data={"grant_type": "refresh_token", "refresh_token": tok["refresh_token"]}, + auth=(oidc_client.client_id, oidc_client.client_secret), + ) + assert r.status_code == 200, r.text + + fresh = await rows( + "SELECT * FROM oidc_tokens WHERE access_token_hash = %s", + oidc_provider.hash_secret(r.json()["access_token"]), + ) + assert fresh[0]["terminal_id"] == tid, "rotation must not drop the device binding" + assert fresh[0]["sid"] == sid + + +async def test_unpair_revokes_only_that_terminals_tokens(app_client: AsyncClient): + oidc_client = await make_client() + + await pair_new_terminal(app_client, name="laptop") + laptop_tok, _ = await run_code_flow(app_client, oidc_client) + laptop = await terminal_id("laptop") + + await pair_new_terminal(app_client, name="desktop") + desktop_tok, _ = await run_code_flow(app_client, oidc_client) + desktop = await terminal_id("desktop") + + await unpair(app_client, laptop) + + revoked = await rows("SELECT * FROM oidc_tokens WHERE sid IS NOT NULL") + by_token = {r["access_token_hash"]: r["revoked"] for r in revoked} + assert by_token[oidc_provider.hash_secret(laptop_tok["access_token"])] is True + assert by_token[oidc_provider.hash_secret(desktop_tok["access_token"])] is False + + r = await app_client.get( + USERINFO, headers={"Authorization": f"Bearer {laptop_tok['access_token']}"} + ) + assert r.status_code == 401, "un-paired device's access token must stop working" + + r = await app_client.get( + USERINFO, headers={"Authorization": f"Bearer {desktop_tok['access_token']}"} + ) + assert r.status_code == 200, "other devices must stay signed in" + assert desktop != laptop + + +async def test_unpair_burns_codes_not_yet_redeemed(app_client: AsyncClient): + oidc_client = await make_client() + await pair_new_terminal(app_client, name="laptop") + verifier = secrets.token_urlsafe(32) + code = await get_code(app_client, oidc_client, verifier) + laptop = await terminal_id("laptop") + + await pair_new_terminal(app_client, name="desktop") + await unpair(app_client, laptop) + + r = await exchange_code(app_client, oidc_client, code, verifier) + assert r.status_code == 400, "a code issued to an un-paired device must not redeem" + + +async def test_unpair_leaves_a_terminal_without_grants_alone(app_client: AsyncClient): + await pair_new_terminal(app_client, name="laptop") + laptop = await terminal_id("laptop") + await pair_new_terminal(app_client, name="desktop") + + await unpair(app_client, laptop) + assert await rows("SELECT * FROM oidc_tokens") == [] diff --git a/tests/util.py b/tests/util.py index 358b7b9d..15e3db1a 100644 --- a/tests/util.py +++ b/tests/util.py @@ -1,5 +1,7 @@ import asyncio +import subprocess as std_subprocess import time +import zipfile from contextlib import asynccontextmanager from pathlib import Path from typing import Callable @@ -102,6 +104,39 @@ def mock_app_store_path(): return Path(__file__).parent / "mock_app_store" +def _git_tracked_mock_app_store_files() -> set[Path]: + store = mock_app_store_path() + listing = std_subprocess.run( + ["git", "ls-files", "-z"], + cwd=store, + capture_output=True, + text=True, + check=True, + ) + return {store / name for name in listing.stdout.split("\0") if name} + + +def build_mock_app_store_zips(target_root: Path) -> Path: + """Pack every mock app directory into //.zip. + + Fixture files that are not committed would be absent from a fresh checkout, + so the packed app would differ between a developer machine and CI. + """ + tracked = _git_tracked_mock_app_store_files() + for app_dir in sorted(p for p in mock_app_store_path().iterdir() if p.is_dir()): + sources = sorted(p for p in app_dir.rglob("*") if p.is_file()) + untracked = [str(p.relative_to(app_dir)) for p in sources if p not in tracked] + assert ( + not untracked + ), f"{app_dir.name} has uncommitted fixture files: {untracked}" + zip_path = target_root / app_dir.name / f"{app_dir.name}.zip" + zip_path.parent.mkdir(parents=True) + with zipfile.ZipFile(zip_path, "w") as zip_file: + for source in sources: + zip_file.write(source, source.relative_to(app_dir)) + return target_root + + def verify_signature_auth(request: PreparedRequest, pubkey: PublicKey) -> VerifyResult: class KR(HTTPSignatureKeyResolver): def resolve_private_key(self, key_id: str): diff --git a/uv.lock b/uv.lock index 33cddfbe..ce9aa49d 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" },