diff --git a/api/config.py b/api/config.py index 7491e3c..d776479 100644 --- a/api/config.py +++ b/api/config.py @@ -20,6 +20,13 @@ class Settings: # for the same number of minutes. LOGIN_MAX_FAILURES: int = int(os.environ.get("LOGIN_MAX_FAILURES", "10")) LOGIN_LOCKOUT_MINUTES: int = int(os.environ.get("LOGIN_LOCKOUT_MINUTES", "15")) + # #95: OIDC sign-in against an IdP the deployment controls. Off unless + # OIDC_ISSUER is set. OIDC_JWKS_URL may be https:// or, for an enclave with + # a published key file, file://. + OIDC_ISSUER: str = os.environ.get("OIDC_ISSUER", "") + OIDC_AUDIENCE: str = os.environ.get("OIDC_AUDIENCE", "") + OIDC_JWKS_URL: str = os.environ.get("OIDC_JWKS_URL", "") + OIDC_USERNAME_CLAIM: str = os.environ.get("OIDC_USERNAME_CLAIM", "preferred_username") # Schema holding the fixed T&E tables (deploy_all.sh dev creates te_dev). TE_SCHEMA: str = os.environ.get("TE_SCHEMA", "te_dev") diff --git a/api/main.py b/api/main.py index af18112..34ec2d9 100644 --- a/api/main.py +++ b/api/main.py @@ -15,7 +15,7 @@ from fastapi.middleware.cors import CORSMiddleware from fastapi.responses import JSONResponse -from api import db +from api import db, oidc from api.config import settings from api.ratelimit import rate_limit_middleware from api.routers import audit_routes, auth_routes, csv_routes, te_routes @@ -45,6 +45,7 @@ async def lifespan(app: FastAPI): # O-03: fail closed. A warning was not enough — an unset key left every # router open, and a warning in a container log is easy to miss. assert_auth_configured(settings.API_ENV, settings.API_KEY) + oidc.check_config() if not settings.API_KEY: logger.warning( "API_KEY is not set - every endpoint is unauthenticated. " diff --git a/api/oidc.py b/api/oidc.py new file mode 100644 index 0000000..8de4c39 --- /dev/null +++ b/api/oidc.py @@ -0,0 +1,70 @@ +"""OIDC ID-token verification against a deployment-controlled IdP (#95, BR-36). + +The frontend completes the IdP's authorization-code flow and posts the ID +token to /api/auth/oidc. Here the token's signature (from the IdP's JWKS), +issuer, audience and expiry are checked; a valid token names a local user, +who then gets an ordinary session for the tenant they chose. +""" + +from __future__ import annotations + +from functools import lru_cache +from pathlib import Path +from urllib.parse import urlparse +from urllib.request import url2pathname + +import jwt + +from api.config import settings + +ALGORITHMS = ["RS256", "ES256"] + + +def enabled() -> bool: + return bool(settings.OIDC_ISSUER) + + +def check_config() -> None: + """Fail closed at startup on a half-configured IdP (called from api/main.py).""" + if not enabled(): + return + if not settings.OIDC_AUDIENCE: + raise RuntimeError("OIDC_ISSUER is set but OIDC_AUDIENCE is empty; set it to this app's client id.") + if not settings.OIDC_JWKS_URL.startswith(("https://", "file://")): + raise RuntimeError("OIDC_JWKS_URL must be https:// or file:// when OIDC_ISSUER is set.") + + +@lru_cache(maxsize=4) +def _jwks(url: str): + """Key source for ``url``: https:// is fetched (and re-fetched on an unknown + key id, for IdP key rotation); file:// is a JWKS file published inside an + enclave. Plain http:// is refused, since keys fetched over it could be swapped. + """ + if url.startswith("https://"): + return jwt.PyJWKClient(url, cache_keys=True) + if url.startswith("file://"): + return jwt.PyJWKSet.from_json(Path(url2pathname(urlparse(url).path)).read_text(encoding="utf-8")) + raise ValueError(f"OIDC_JWKS_URL must be https:// or file://, got {url!r}") + + +def _signing_key(id_token: str): + source = _jwks(settings.OIDC_JWKS_URL) + if isinstance(source, jwt.PyJWKClient): + return source.get_signing_key_from_jwt(id_token).key + kid = jwt.get_unverified_header(id_token).get("kid") + try: + return source[kid].key + except KeyError: + raise jwt.InvalidTokenError(f"unknown key id {kid!r}") from None + + +def username_from(id_token: str) -> str | None: + """The verified token's username claim, or None if anything about it is wrong.""" + try: + key = _signing_key(id_token) + claims = jwt.decode(id_token, key, algorithms=ALGORITHMS, audience=settings.OIDC_AUDIENCE, + issuer=settings.OIDC_ISSUER, options={"require": ["exp", "iat", "iss", "aud"]}) + except jwt.PyJWTError: + return None + username = claims.get(settings.OIDC_USERNAME_CLAIM) + return username if isinstance(username, str) and username else None diff --git a/api/requirements.txt b/api/requirements.txt index eb6f42b..9f83eb2 100644 --- a/api/requirements.txt +++ b/api/requirements.txt @@ -6,3 +6,6 @@ pydantic>=2.13.4,<3.0.0 # BUG-032: schema migrations for csv_uploads.* — auto-runs at API startup # via alembic upgrade head (see api/db.py bootstrap and alembic/ directory). alembic>=1.19.1,<2.0.0 +# #95: OIDC ID-token verification (RS256 via cryptography). Only used when +# OIDC_ISSUER is set; air-gapped installs can rely on local accounts. +PyJWT[crypto]>=2.10,<3.0 diff --git a/api/routers/auth_routes.py b/api/routers/auth_routes.py index 280cdb2..a7af6ce 100644 --- a/api/routers/auth_routes.py +++ b/api/routers/auth_routes.py @@ -12,6 +12,7 @@ from api.auth import bearer_token, require_session from api.db import Conn +from api import oidc from api.services import accounts router = APIRouter(prefix="/api/auth", tags=["auth"]) @@ -34,6 +35,28 @@ def login(req: LoginRequest) -> dict: return session +class OidcLoginRequest(BaseModel): + tenant: str = Field(min_length=1, max_length=64) + idToken: str = Field(min_length=1, max_length=16384) + + +@router.post("/oidc") +def oidc_login(req: OidcLoginRequest) -> dict: + """Exchange an IdP ID token for a session (#95). 404 when OIDC isn't configured.""" + if not oidc.enabled(): + raise HTTPException(status_code=404, detail="OIDC sign-in is not configured") + username = oidc.username_from(req.idToken) + session = None + if username: + with Conn() as conn: + with conn.cursor() as cur: + session = accounts.login_trusted(cur, req.tenant, username) + conn.commit() + if session is None: + raise HTTPException(status_code=401, detail="Invalid token, or no access to that tenant") + return session + + @router.post("/logout") def logout(token: str = Depends(bearer_token)) -> dict: with Conn() as conn: diff --git a/api/services/accounts.py b/api/services/accounts.py index 740233a..2932e03 100644 --- a/api/services/accounts.py +++ b/api/services/accounts.py @@ -87,7 +87,25 @@ def login(cur, tenant_slug: str, username: str, password: str) -> dict | None: return None cur.execute(_q("UPDATE {c}.users SET failed_logins = 0, first_failed_at = NULL, " "locked_until = NULL WHERE id = %s"), (row[0],)) - user_id, tenant_id, role = row[0], row[3], row[4] + return _new_session(cur, row[0], row[3], username, tenant_slug, row[4]) + + +def login_trusted(cur, tenant_slug: str, username: str) -> dict | None: + """Session for a user whose identity an IdP has already proven (OIDC, #95). + + Authorisation stays local: the user must already exist, be active and be a + member of the tenant. Nobody is created from IdP claims. + """ + cur.execute(_q( + "SELECT u.id, t.id, m.role FROM {c}.users u " + "JOIN {c}.tenants t ON t.slug = %s " + "JOIN {c}.memberships m ON m.user_id = u.id AND m.tenant_id = t.id " + "WHERE u.username = %s AND u.is_active"), (tenant_slug, username)) + row = cur.fetchone() + return _new_session(cur, row[0], row[1], username, tenant_slug, row[2]) if row else None + + +def _new_session(cur, user_id: int, tenant_id: int, username: str, tenant_slug: str, role: str) -> dict: token = secrets.token_urlsafe(32) cur.execute(_q( "INSERT INTO {c}.sessions (token_hash, user_id, tenant_id, expires_at) " diff --git a/docs/ENV_VARIABLES.md b/docs/ENV_VARIABLES.md index 783a235..7a7acb1 100644 --- a/docs/ENV_VARIABLES.md +++ b/docs/ENV_VARIABLES.md @@ -43,6 +43,10 @@ Read by `api/config.py`. Set these in the shell before running | `SESSION_TTL_HOURS` | `12` | No | How long a sign-in session lasts before the user must sign in again. | | `LOGIN_MAX_FAILURES` | `10` | No | Failed sign-ins within the window that lock an account (#102). | | `LOGIN_LOCKOUT_MINUTES` | `15` | No | Length of the failure window and of the lock. | +| `OIDC_ISSUER` | _(empty)_ | No | Turns on OIDC sign-in (#95). Must match the ID token's `iss`. Leave empty for air-gapped installs that use local accounts. | +| `OIDC_AUDIENCE` | _(empty)_ | If OIDC is on | This app's client id; must match the token's `aud`. | +| `OIDC_JWKS_URL` | _(empty)_ | If OIDC is on | The IdP's signing keys: `https://…` (fetched, refreshed when keys rotate) or `file:///…` (a JWKS file published inside an enclave). `http://` is refused. | +| `OIDC_USERNAME_CLAIM` | `preferred_username` | No | Token claim that names the local user. The user must already exist and be a member of the tenant; nobody is created from claims. | | `TE_SCHEMA` | `te_dev` | No | Schema holding the 12 fixed T&E tables. Switch to `te_test`, `te_staging`, or `te_prod` per environment. | | `MAX_UPLOAD_BYTES` | `52428800` (50 MB) | No | Maximum file size accepted by the upload endpoint. | diff --git a/scripts/lint.sh b/scripts/lint.sh index f5790a5..0d15b62 100644 --- a/scripts/lint.sh +++ b/scripts/lint.sh @@ -23,6 +23,7 @@ PY_SRC=( api/services/accounts.py api/routers/auth_routes.py api/tenancy.py + api/oidc.py scripts/create_tenant_admin.py ) @@ -40,6 +41,7 @@ PY_TESTS=( tests/test_accounts.py tests/test_tenant_isolation.py tests/test_roles.py + tests/test_oidc.py ) # Collect scripts/*.py dynamically diff --git a/tests/test_oidc.py b/tests/test_oidc.py new file mode 100644 index 0000000..addd108 --- /dev/null +++ b/tests/test_oidc.py @@ -0,0 +1,165 @@ +"""OIDC sign-in against a deployment-controlled IdP (#95, BR-36). + +Tokens are signed with a locally generated RSA key whose JWKS is served from +a file:// URL, so nothing reaches the network. +""" + +from __future__ import annotations + +import json +import os +import tempfile +import time +import unittest +import uuid +from pathlib import Path + +import jwt +import pytest +from cryptography.hazmat.primitives.asymmetric import rsa +from fastapi.testclient import TestClient + +from api import oidc +from api.config import settings +from api.db import Conn +from api.main import app +from api.services import accounts + +_ISS, _AUD, _KID = "https://idp.example.test/realms/te", "te-migration", "test-key-1" + + +def _key(): + return rsa.generate_private_key(public_exponent=65537, key_size=2048) + + +_SIGNER, _OTHER = _key(), _key() + + +class _Oidc(unittest.TestCase): + @classmethod + def setUpClass(cls): + cls._orig = (settings.OIDC_ISSUER, settings.OIDC_AUDIENCE, settings.OIDC_JWKS_URL) + jwk = json.loads(jwt.algorithms.RSAAlgorithm.to_jwk(_SIGNER.public_key())) + jwk.update({"kid": _KID, "use": "sig", "alg": "RS256"}) + cls._dir = tempfile.TemporaryDirectory() + jwks = Path(cls._dir.name) / "jwks.json" + jwks.write_text(json.dumps({"keys": [jwk]})) + settings.OIDC_ISSUER, settings.OIDC_AUDIENCE = _ISS, _AUD + settings.OIDC_JWKS_URL = jwks.as_uri() + oidc._jwks.cache_clear() + + @classmethod + def tearDownClass(cls): + settings.OIDC_ISSUER, settings.OIDC_AUDIENCE, settings.OIDC_JWKS_URL = cls._orig + oidc._jwks.cache_clear() + cls._dir.cleanup() + + @staticmethod + def token(username="alice", key=None, kid=_KID, alg="RS256", **overrides) -> str: + now = int(time.time()) + claims = {"iss": _ISS, "aud": _AUD, "iat": now, "exp": now + 300, + "sub": "abc", "preferred_username": username} + claims.update(overrides) + claims = {k: v for k, v in claims.items() if v is not None} + return jwt.encode(claims, key or _SIGNER, algorithm=alg, headers={"kid": kid}) + + +@pytest.mark.unit +class TokenVerification(_Oidc): + def test_valid_token_yields_the_username(self): + self.assertEqual(oidc.username_from(self.token()), "alice") + + def test_rejected_tokens(self): + now = int(time.time()) + cases = { + "expired": self.token(exp=now - 10, iat=now - 400), + "wrong issuer": self.token(iss="https://evil.example/realms/te"), + "wrong audience": self.token(aud="some-other-app"), + "signed by another key": self.token(key=_OTHER), + "unknown key id": self.token(kid="not-published"), + "no expiry": self.token(exp=None), + "no username claim": self.token(username=None), + "unsigned (alg none)": jwt.encode( + {"iss": _ISS, "aud": _AUD, "exp": now + 300, "iat": now, "preferred_username": "alice"}, + None, algorithm="none", headers={"kid": _KID}), + "garbage": "not.a.token", + } + for name, tok in cases.items(): + self.assertIsNone(oidc.username_from(tok), name) + + +@pytest.mark.integration +class OidcSignIn(_Oidc): + @classmethod + def setUpClass(cls): + super().setUpClass() + cls.ctx = TestClient(app) + cls.client = cls.ctx.__enter__() + + @classmethod + def tearDownClass(cls): + cls.ctx.__exit__(None, None, None) + super().tearDownClass() + + def setUp(self): + tag = f"{os.getpid()}_{uuid.uuid4().hex[:8]}" + self.tenant, self.other, self.user = f"oi_{tag}", f"ox_{tag}", f"oidc_{tag}" + with Conn() as conn: + with conn.cursor() as cur: + tid = accounts.create_tenant(cur, self.tenant, "OIDC tenant") + accounts.create_tenant(cur, self.other, "Other") + self.user_id = accounts.create_user(cur, self.user, uuid.uuid4().hex) + accounts.add_membership(cur, self.user_id, tid, "reviewer") + conn.commit() + + def _post(self, tenant, tok): + return self.client.post("/api/auth/oidc", json={"tenant": tenant, "idToken": tok}) + + def test_valid_token_for_a_member_gets_a_session(self): + res = self._post(self.tenant, self.token(self.user)) + self.assertEqual(res.status_code, 200, res.text) + me = self.client.get("/api/auth/me", headers={"Authorization": f"Bearer {res.json()['token']}"}) + self.assertEqual((me.json()["username"], me.json()["role"]), (self.user, "reviewer")) + + def test_valid_token_but_no_membership_or_account_is_401(self): + self.assertEqual(self._post(self.other, self.token(self.user)).status_code, 401) + self.assertEqual(self._post(self.tenant, self.token("nobody_" + uuid.uuid4().hex)).status_code, 401) + + def test_invalid_token_is_401(self): + self.assertEqual(self._post(self.tenant, self.token(self.user, key=_OTHER)).status_code, 401) + + def test_inactive_user_is_401(self): + with Conn() as conn: + with conn.cursor() as cur: + cur.execute(f'UPDATE "{settings.CONTROL_SCHEMA}".users SET is_active = false WHERE id = %s', + (self.user_id,)) + conn.commit() + self.assertEqual(self._post(self.tenant, self.token(self.user)).status_code, 401) + + def test_route_is_404_when_oidc_is_not_configured(self): + issuer, settings.OIDC_ISSUER = settings.OIDC_ISSUER, "" + try: + self.assertEqual(self._post(self.tenant, self.token(self.user)).status_code, 404) + finally: + settings.OIDC_ISSUER = issuer + + +@pytest.mark.unit +class ConfigFailsClosed(unittest.TestCase): + def setUp(self): + self._orig = (settings.OIDC_ISSUER, settings.OIDC_AUDIENCE, settings.OIDC_JWKS_URL) + + def tearDown(self): + settings.OIDC_ISSUER, settings.OIDC_AUDIENCE, settings.OIDC_JWKS_URL = self._orig + + def test_off_by_default_and_complete_config_passes(self): + settings.OIDC_ISSUER = "" + oidc.check_config() + settings.OIDC_ISSUER, settings.OIDC_AUDIENCE, settings.OIDC_JWKS_URL = _ISS, _AUD, "https://idp/jwks" + oidc.check_config() + + def test_missing_audience_or_insecure_jwks_url_refuses_to_start(self): + for aud, url in (("", "https://idp/jwks"), (_AUD, "http://idp/jwks"), (_AUD, "")): + settings.OIDC_ISSUER, settings.OIDC_AUDIENCE, settings.OIDC_JWKS_URL = _ISS, aud, url + with self.assertRaises(RuntimeError, msg=(aud, url)): + oidc.check_config()