Repository navigation
feat: resolve pre-registered SSO human identities - #487
Conversation
Refs: #486 Assisted-by: Codex Signed-off-by: sjungwon03 <sjungwon03@gmail.com>
sjungwon03-ai
left a comment
There was a problem hiding this comment.
Reviewed exact head 8f1c898 against the byte-identical published diff, migration loader/runner and startup composition, existing IAM snapshot boundaries, issue plan, ADR and SSO contracts.
The production blast radius is an additive empty binding table, its trusted deployment manifest and a new internal reader. No gateway authentication path is connected. C-collated composite issuer/subject keys preserve exact spelling and cross-issuer separation; the foreign key restricts principal deletion and multiple bindings may target one principal. Historical SQL bytes remain unchanged. Fresh joined reads return only persisted active humans and deny missing, inactive and service state without creating principals or policies. Caller-supplied principal/kind/active fields cannot override stored state.
Inputs and required result fields are captured once and validated before returning an immutable primitive ID. Required properties are own, opaque strings are bounded/non-NUL/well-formed, parameters are frozen and parameterized, duplicate/mismatched/malformed results fail privately, and thrown input/driver/result getters expose fixed errors without raw claims or causes. Returned identity confers no authorization: later management still requires verified JWT identity, current IAM and mandatory decision audit. Post-read races and stricter reader-versus-storage length limits are disclosed.
Evidence includes the initial three missing-table/manifest failures, then 54 behavioral reader failures from the deny-only skeleton, followed by all 92 focused cases green. Sixty-two new cases cover success, denial, invalid/malformed state, exact identities, getter/mutation behavior, storage constraints, and populated 010-to-011 upgrade without backfill or checksum changes. The existing startup/migration fixture now expects 011; no production startup error was weakened. Full local npm run check passes 6193 tests with one existing PostgreSQL skip, strict types, lint, document checks and all three unchanged schema pins. Merge remains gated on both exact-head check jobs.
The contributor's Q4/Q5 choices are recorded without inventing the remaining token profile. JWT verification, public HTTP management and audited binding provisioning remain explicitly pending; the new reader alone does not enable login. No actionable blocking findings; approve this bounded foundation. This is a Codex-operated account review, not independent human review or complete release certification.
Reviewed by Codex operating as sjungwon03-ai under the contributor's explicit authorization. Assisted-by: Codex.
Closes #486
Persisted IAM principals lacked an SSO identity mapping. Following the contributor's confirmed pre-registration and API-specific JWT choices, add migration 011 and an internal resolveHumanPrincipal reader. One parameterized joined query returns only an existing active human ID for an exact issuer/subject binding; missing mappings, inactive humans and service principals return undefined. Multiple bindings can name one principal. No automatic account creation, email fallback or identity normalization occurs.
Capture own required inputs and persisted result fields once, freeze projected SQL parameters, validate complete results and sanitize input/driver/getter/malformed/duplicate/mismatched failures without claims or causes. Reload binding/kind/active state each operation. C-collated composite keys and a restricting foreign key preserve exact identities. Historical migrations and all three OpenRouter pins remain byte-identical. This foundation does not verify JWTs, authenticate public requests, write bindings, grant policy rights or call audit/secrets/inference/usage ports.
TDD: before production changes, three storage tests failed for the absent table and old migration manifest. After the reader tests were written, the initial deny-only typed skeleton produced 54 expected behavioral failures and four denial passes: active human resolution returned undefined, invalid inputs/results did not raise their required errors, and the relation was still absent. The minimal migration/reader made the initial 67 focused cases green. Full validation revealed an existing gateway startup assertion pinned to migration 010; update its expected history and the migration-runner fixture to include 011, and verify an existing populated 010 database upgrades without backfill or historical checksum changes. All 92 focused cases pass, including 62 new cases. Full npm run check passes strict types, lint, 6193 tests with one existing PostgreSQL skip, planning/contracts/fixture scans and offline schema integrity.
Remaining scope: local opaque-string limits (issuer 1–2048, subject/principal ID 1–256 UTF-16 units, no NUL/ill-formed Unicode) are read bounds, not a JWT issuer-trust/access-token profile. SQL character-count guards are coarser than UTF-16 reader bounds. State changes after the joined read are not revalidated. Signature, issuer/audience/algorithm/key/JWKS/rotation/token-type/expiry/skew validation, public HTTP authentication/errors/audit, and audited administrator binding provisioning remain separate work. Current IAM and mandatory management decision audit still apply; no production login path is enabled here. Catalog publication/refresh, complete #116 and unresolved #7 remain open. Additive rollout requires shipping migration 011; rollback must deliberately handle its bindings before table removal.
Plan: 486-sso-identity-bindings. Contract: sso-identity-bindings.
Assisted-by: Codex