Prep for #112 and #113: security review pack, OIDC https tests and real-IdP check

.github/workflows/quality-gate.yml

@@ -240,6 +240,12 @@ jobs:
         working-directory: csv-table-hub-main
         run: npm ci
 
+      # #112: the frontend's own unit tests (vitest). No job ran them before, so a change to the one place the app
+      # builds HTML by hand (the print window in src/lib/export.ts) could lose its escaping without anything failing.
+      - name: Frontend unit tests (vitest)
+        working-directory: csv-table-hub-main
+        run: npm test
+
       - name: Install Chromium for Playwright
         run: python -m playwright install --with-deps chromium
 

Makefile

@@ -1,5 +1,5 @@
 .PHONY: test-free test-gate test-evals test-e2e test-all \
-        test-unit test-system test-egress-blocked check-bundle test-sit test-performance test-accessibility test-uat \
+        test-unit test-system test-egress-blocked check-bundle security-evidence test-sit test-performance test-accessibility test-uat \
         lint lint-diff health \
         eval-list eval-compare eval-summary select-tests \
         csv-load csv-list csv-demo
@@ -60,6 +60,16 @@ test-egress-blocked:
 check-bundle:
 	python3 scripts/check_bundle_egress.py csv-table-hub-main/.output
 
+# The tests behind docs/SECURITY_REVIEW_PACK.md (#112): authentication, tenant isolation, roles, upload handling and the
+# audit trail. Needs PostgreSQL. A passing run is evidence for the reviewer, not the review.
+security-evidence:
+	python3 -m pytest tests/system/test_accounts.py tests/system/test_password_reset.py tests/system/test_oidc.py \
+	    tests/system/test_oidc_https.py tests/system/test_check_oidc.py tests/system/test_tenant_isolation.py \
+	    tests/system/test_tenant_te_migrations.py tests/system/test_roles.py tests/system/test_runtime_role.py \
+	    tests/system/test_audit_append_only.py tests/system/test_preserved_uploads.py tests/system/test_staged_import.py \
+	    tests/system/test_versioned_datasets.py tests/system/test_reports_and_policy.py \
+	    tests/unit/test_security.py tests/unit/test_api_hardening.py tests/unit/test_sovereignty.py -v
+
 test-sit:
 	$(call run_category,tests/system_integration)
 

api/oidc.py

@@ -18,6 +18,9 @@
 from api.config import settings
 
 ALGORITHMS = ["RS256", "ES256"]
+# Seconds to wait for the IdP's key set. PyJWT's default is 30, which would hold a request thread that long for
+# every sign-in attempt while the IdP is down or unreachable.
+JWKS_TIMEOUT_SECONDS = 5
 
 
 def enabled() -> bool:
@@ -36,12 +39,15 @@ def check_config() -> None:
 
 @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
+    """Key source for ``url``: https:// is fetched; file:// is a JWKS file published inside an
     enclave. Plain http:// is refused, since keys fetched over it could be swapped.
+
+    An https:// key set is cached for PyJWT's lifespan (5 minutes), and an unknown key id triggers a
+    re-fetch, for IdP key rotation, but at most once per PyJWT's cooldown (30 seconds): so random key ids
+    can't be used to hammer the IdP, and a freshly rotated key can be refused for up to 30 seconds.
     """
     if url.startswith("https://"):
-        return jwt.PyJWKClient(url, cache_keys=True)
+        return jwt.PyJWKClient(url, cache_keys=True, timeout=JWKS_TIMEOUT_SECONDS)
     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}")
@@ -58,12 +64,20 @@ def _signing_key(id_token: str):
         raise jwt.InvalidTokenError(f"unknown key id {kid!r}") from None
 
 
+def verify(id_token: str) -> dict:
+    """The token's claims once its signature, issuer, audience and expiry check out; raises ``jwt.PyJWTError`` if not.
+
+    ``username_from`` hides the reason; this keeps it, for scripts/check_oidc.py.
+    """
+    key = _signing_key(id_token)
+    return jwt.decode(id_token, key, algorithms=ALGORITHMS, audience=settings.OIDC_AUDIENCE,
+                      issuer=settings.OIDC_ISSUER, options={"require": ["exp", "iat", "iss", "aud"]})
+
+
 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"]})
+        claims = verify(id_token)
     except jwt.PyJWTError:
         return None
     username = claims.get(settings.OIDC_USERNAME_CLAIM)

csv-table-hub-main/src/__tests__/export.test.ts

@@ -305,3 +305,67 @@ describe("downloadAuditReport", () => {
     expect(text).toContain("Imported");
   });
 });
+
+// ─── print window (PDF): the one place HTML is built by hand ──────────────────
+//
+// The print window is written with document.write(), not React, so nothing escapes for us, and
+// it shares the app's origin (and so its sessionStorage, which holds the sign-in token). File
+// names, cell values and reasons all come from uploaded CSVs. Security review pack, #112.
+
+describe("print window HTML (PDF) escapes everything user-supplied", () => {
+  const HOSTILE = [
+    `<script>alert(1)</script>`,
+    `"><img src=x onerror=alert(1)>`,
+    `'-alert(1)-'`,
+    `&lt;already&gt; & </td><td>`,
+  ];
+
+  let written: string;
+
+  beforeEach(() => {
+    written = "";
+    vi.useFakeTimers();
+    vi.spyOn(window, "open").mockImplementation(() => ({
+      document: { write: (html: string) => { written += html; }, close: vi.fn() },
+      focus: vi.fn(),
+      print: vi.fn(),
+      close: vi.fn(),
+    }) as unknown as Window);
+  });
+  afterEach(() => {
+    vi.useRealTimers();
+    vi.restoreAllMocks();
+  });
+
+  function assertNothingExecutable() {
+    expect(written).not.toMatch(/<script/i);
+    expect(written).not.toMatch(/<img/i);
+    expect(written).not.toMatch(/<[^>]+\sonerror\s*=/i);   // no tag carries an event handler
+    for (const text of HOSTILE) expect(written).not.toContain(text);
+    expect(written).toContain("&lt;script&gt;alert(1)&lt;/script&gt;");
+  }
+
+  it("error report: the file name (title), headers and every cell", () => {
+    const rows = HOSTILE.map((h, i) => ({
+      file: h, row_number: i, column: h, value: h, reason: h, plain_english: h,
+    }));
+    downloadErrorReport(HOSTILE[0], rows, "pdf");
+    expect(written).toContain("<table>");
+    assertNothingExecutable();
+  });
+
+  it("audit report: every cell", () => {
+    const rows = HOSTILE.map((h) => ({
+      file: h, status: h, imported_at: h, total_rows: h, inserted_rows: h,
+      duplicate_rows_skipped: h, failed_rows: h, table_name: h, header_mapping: h, cancellation_reason: h,
+    }));
+    downloadAuditReport(rows as AuditReportRow[], "pdf");
+    assertNothingExecutable();
+  });
+
+  it("the rest of the page is fixed markup: only table cells vary", () => {
+    downloadAuditReport([], "pdf");
+    expect(written).toMatch(/^<!DOCTYPE html><html><head>/);
+    expect(written).toContain("<title>Audit report</title>");
+  });
+});

docs/ENV_VARIABLES.md

@@ -47,7 +47,7 @@ Read by `api/config.py`. Set these in the shell before running
 | `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_JWKS_URL` | _(empty)_ | If OIDC is on | The IdP's signing keys: `https://…` (fetched, and refreshed when keys rotate: at most every 30 s; a private CA needs `SSL_CERT_FILE`, see `docs/OIDC_REAL_IDP_CHECK.md`) 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. |

docs/OIDC_REAL_IDP_CHECK.md

@@ -0,0 +1,86 @@
+# Checking OIDC sign-in against the real identity provider (#113)
+
+One sign-in against the deployment's real IdP (Keycloak or Entra ID) proves what no automated test can: that **its** certificate is trusted, **its** issuer and audience match the configuration, and **its** tokens carry a claim that names a local user. This is the procedure and the script for it. It needs access to the IdP, and a person who can sign in there.
+
+## What is already tested, and what is not
+
+| | Where | Status |
+|---|---|---|
+| Signature, issuer, audience, expiry and claim checks | `tests/system/test_oidc.py` (file:// key set) | Automated |
+| The `https://` key fetch (PyJWT's `PyJWKClient`), key rotation, an untrusted certificate, an IdP that is down or hangs, the sign-in through the API | `tests/system/test_oidc_https.py`, against a local TLS server | Automated (#113) |
+| This script's checks and their hints | `tests/system/test_check_oidc.py` | Automated |
+| **That the real IdP's certificate, issuer, audience, claims and key set work with this configuration** | This procedure | **To do, by a person with IdP access** |
+
+## 1. Gather the values
+
+| Setting | Keycloak | Entra ID (v2 endpoints) |
+|---|---|---|
+| `OIDC_ISSUER` | `https://<host>/realms/<realm>` | `https://login.microsoftonline.com/<tenant-id>/v2.0` |
+| `OIDC_JWKS_URL` | `<issuer>/protocol/openid-connect/certs` | `https://login.microsoftonline.com/<tenant-id>/discovery/v2.0/keys` |
+| `OIDC_AUDIENCE` | the client id | the application (client) id |
+| `OIDC_USERNAME_CLAIM` | `preferred_username` (default) | `preferred_username` (v2 tokens). v1 tokens use `upn` |
+
+The IdP publishes the right issuer and key URL at `<issuer>/.well-known/openid-configuration`; the script compares yours with it. The issuer must match the token's `iss` **exactly**, including any trailing slash.
+
+## 2. Get an ID token
+
+Use the device-code flow, so the token never goes through a browser address bar or a third-party site. **Don't paste an ID token into an online debugger, an issue or a chat: it is a credential.** Register a **test** client (public, device flow enabled, scope `openid`), then:
+
+```bash
+umask 077                                         # the token file is private to you
+CLIENT_ID=<client id>
+# Keycloak: DEVICE=<issuer>/protocol/openid-connect/auth/device   TOKEN=<issuer>/protocol/openid-connect/token
+# Entra:    DEVICE=https://login.microsoftonline.com/<tenant-id>/oauth2/v2.0/devicecode
+#           TOKEN=https://login.microsoftonline.com/<tenant-id>/oauth2/v2.0/token
+curl -s "$DEVICE" -d "client_id=$CLIENT_ID" -d "scope=openid" | tee /tmp/device.json | jq '{user_code, verification_uri}'
+# Sign in at the verification_uri as the user, enter the user_code, then:
+curl -s "$TOKEN" -d "client_id=$CLIENT_ID" -d "grant_type=urn:ietf:params:oauth:grant-type:device_code" \
+     -d "device_code=$(jq -r .device_code /tmp/device.json)" | jq -r .id_token > idtoken.txt
+rm /tmp/device.json
+```
+
+The user must **already exist** in the platform and be a **member of the tenant** (nobody is created from token claims), and their local username must equal the token's username claim, for example `alice@example.org` for Entra's `preferred_username`.
+
+## 3. Run the check
+
+On a machine that can reach the IdP, with the API's `OIDC_*` values and the token file:
+
+```bash
+export OIDC_ISSUER=... OIDC_AUDIENCE=... OIDC_JWKS_URL=...
+python scripts/check_oidc.py --token-file idtoken.txt                      # the IdP, its keys, the token
+python scripts/check_oidc.py --token-file idtoken.txt --api http://localhost:8000 --tenant <slug>   # and the real sign-in
+rm idtoken.txt
+```
+
+A private CA: set `SSL_CERT_FILE=/path/to/ca-bundle.pem` for the script **and** in the API's own environment, or the key fetch fails closed. Passing means every stage printed `OK`. The last line says whether the sign-in itself was checked. The script never prints the token or the session token.
+
+Run the same sign-in once through the real front end too, because the front end builds the authorization request.
+
+## 4. What to record on #113
+
+Paste the script's output (it holds no secrets), plus: which IdP and version, the date, and who ran it. For example:
+
+```
+OK    config     issuer https://…, audience …, keys https://…, username claim 'preferred_username'
+OK    discovery  issuer and key URL match what the IdP publishes
+OK    keys       2 signing key(s): …
+OK    token      alg RS256, kid …, expires in … s, preferred_username = '…'. Signature, issuer, audience and expiry verified
+OK    sign-in    200: signed in as '…' to tenant '…' with role '…'; …
+```
+
+Then close #113.
+
+## When a stage fails
+
+| Stage | Message | Usual cause |
+|---|---|---|
+| keys | `CERTIFICATE_VERIFY_FAILED` | The host doesn't trust the IdP's CA. Set `SSL_CERT_FILE` (above) |
+| keys | connection refused or timed out | A firewall or proxy between the API host and the IdP. The API waits 5 s for the key set, then refuses the sign-in |
+| discovery | issuer differs | Trailing slash, or the wrong tenant id or realm in `OIDC_ISSUER` |
+| token | `InvalidAudienceError` | `OIDC_AUDIENCE` isn't the client id the token was issued for |
+| token | `InvalidIssuerError` | The token's `iss` isn't exactly `OIDC_ISSUER` |
+| token | no username claim | Set `OIDC_USERNAME_CLAIM` to the claim that holds the username |
+| token | key id not in the key set | The IdP rotated its keys. A new key can be refused for up to 30 s after rotation (PyJWT re-fetches at most every 30 s, so random key ids can't be used to hammer the IdP). Retry |
+| token | `ImmatureSignatureError` | Clock skew between this host and the IdP |
+| sign-in | 401 | The token verifies but that user isn't a member of that tenant, or the user doesn't exist locally |
+| sign-in | 404 | The **API's** environment has no `OIDC_ISSUER`: the check passed here, but the API wasn't configured |