● Increment 1 complete. Marking it done. #37

Merged

navicore merged 2 commits from remote into main

2026-05-29 13:35:27 +00:00

navicore commented

2026-05-29 13:12:31 +00:00

Owner

● What landed:

Piece: Config fields + per-realm TTL helpers
File: src/config.rs
Notes: admin_bind_address, admin_realm (default "master"), admin_access_token_lifetime_secs,
admin_refresh_token_lifetime_secs; access_token_lifetime_for(realm) / refresh_token_lifetime_for(realm)
────────────────────────────────────────
Piece: Forbidden 403 variant
File: src/server/error.rs
Notes: distinct from Unauthorized (401)
────────────────────────────────────────
Piece: Per-realm lifetimes wired
File: src/server/token.rs
Notes: all 3 grant paths (auth_code, refresh, client_credentials) + their expires_in
────────────────────────────────────────
Piece: State/router split
File: src/server/mod.rs
Notes: build_state + oidc_router are the real API now; build_router kept #[cfg(test)] so existing tests are
byte-for-byte untouched
────────────────────────────────────────
Piece: Admin module
File: src/server/admin/mod.rs
Notes: ANZ_ADMIN_GROUP constant, AdminContext extractor doing live-state authz (signature → issuer → kid → load
user fresh → realm + enabled + group check), admin_router with /admin/health (open) and /admin/whoami (authed)
────────────────────────────────────────
Piece: Dual-listener serve
File: src/cli/serve.rs
Notes: try_join! when admin_bind_address set; identical to before when unset

Posture verified by tests (125 passing, clippy clean with -D warnings):

admin acceptance — valid token + anz-admin group → 200
401 on missing token
401 on token signed by a non-admin realm (issuer + kid mismatch)
403 on valid admin-realm-signed token whose sub is in a different realm carrying anz-admin (the realm-binding
guard)
403 on no group
403 on disabled admin
per-realm TTL helpers fall back to global when override unset; admin realm gets short lifetimes while app
realms keep long ones

Nothing observable changes for current deployments — without admin_bind_address in config, no second listener
starts and no /admin/* exists. Today's behavior is byte-for-byte preserved.

Ready to start Increment 2 (shared DTO module + read-only admin endpoints: list
realms/users/clients/keys/sessions). That increment is still server-side only — no CLI changes — and it's where
the request/response types that the CLI backend will share get defined.

● What landed: Piece: Config fields + per-realm TTL helpers File: src/config.rs Notes: admin_bind_address, admin_realm (default "master"), admin_access_token_lifetime_secs, admin_refresh_token_lifetime_secs; access_token_lifetime_for(realm) / refresh_token_lifetime_for(realm) ──────────────────────────────────────── Piece: Forbidden 403 variant File: src/server/error.rs Notes: distinct from Unauthorized (401) ──────────────────────────────────────── Piece: Per-realm lifetimes wired File: src/server/token.rs Notes: all 3 grant paths (auth_code, refresh, client_credentials) + their expires_in ──────────────────────────────────────── Piece: State/router split File: src/server/mod.rs Notes: build_state + oidc_router are the real API now; build_router kept #[cfg(test)] so existing tests are byte-for-byte untouched ──────────────────────────────────────── Piece: Admin module File: src/server/admin/mod.rs Notes: ANZ_ADMIN_GROUP constant, AdminContext extractor doing live-state authz (signature → issuer → kid → load user fresh → realm + enabled + group check), admin_router with /admin/health (open) and /admin/whoami (authed) ──────────────────────────────────────── Piece: Dual-listener serve File: src/cli/serve.rs Notes: try_join! when admin_bind_address set; identical to before when unset Posture verified by tests (125 passing, clippy clean with -D warnings): - admin acceptance — valid token + anz-admin group → 200 - 401 on missing token - 401 on token signed by a non-admin realm (issuer + kid mismatch) - 403 on valid admin-realm-signed token whose sub is in a different realm carrying anz-admin (the realm-binding guard) - 403 on no group - 403 on disabled admin - per-realm TTL helpers fall back to global when override unset; admin realm gets short lifetimes while app realms keep long ones Nothing observable changes for current deployments — without admin_bind_address in config, no second listener starts and no /admin/* exists. Today's behavior is byte-for-byte preserved. Ready to start Increment 2 (shared DTO module + read-only admin endpoints: list realms/users/clients/keys/sessions). That increment is still server-side only — no CLI changes — and it's where the request/response types that the CLI backend will share get defined.

navicore added 1 commit

2026-05-29 13:12:31 +00:00

● Increment 1 complete. Marking it done.

CI / ci (pull_request) Successful in 3m38s

Details

314c035572

● What landed:

  Piece: Config fields + per-realm TTL helpers
  File: src/config.rs
  Notes: admin_bind_address, admin_realm (default "master"), admin_access_token_lifetime_secs,
    admin_refresh_token_lifetime_secs; access_token_lifetime_for(realm) / refresh_token_lifetime_for(realm)
  ────────────────────────────────────────
  Piece: Forbidden 403 variant
  File: src/server/error.rs
  Notes: distinct from Unauthorized (401)
  ────────────────────────────────────────
  Piece: Per-realm lifetimes wired
  File: src/server/token.rs
  Notes: all 3 grant paths (auth_code, refresh, client_credentials) + their expires_in
  ────────────────────────────────────────
  Piece: State/router split
  File: src/server/mod.rs
  Notes: build_state + oidc_router are the real API now; build_router kept #[cfg(test)] so existing tests are
    byte-for-byte untouched
  ────────────────────────────────────────
  Piece: Admin module
  File: src/server/admin/mod.rs
  Notes: ANZ_ADMIN_GROUP constant, AdminContext extractor doing live-state authz (signature → issuer → kid → load
    user fresh → realm + enabled + group check), admin_router with /admin/health (open) and /admin/whoami (authed)
  ────────────────────────────────────────
  Piece: Dual-listener serve
  File: src/cli/serve.rs
  Notes: try_join! when admin_bind_address set; identical to before when unset

  Posture verified by tests (125 passing, clippy clean with -D warnings):
  - admin acceptance — valid token + anz-admin group → 200
  - 401 on missing token
  - 401 on token signed by a non-admin realm (issuer + kid mismatch)
  - 403 on valid admin-realm-signed token whose sub is in a different realm carrying anz-admin (the realm-binding
  guard)
  - 403 on no group
  - 403 on disabled admin
  - per-realm TTL helpers fall back to global when override unset; admin realm gets short lifetimes while app
  realms keep long ones

  Nothing observable changes for current deployments — without admin_bind_address in config, no second listener
  starts and no /admin/* exists. Today's behavior is byte-for-byte preserved.

  Ready to start Increment 2 (shared DTO module + read-only admin endpoints: list
  realms/users/clients/keys/sessions). That increment is still server-side only — no CLI changes — and it's where
  the request/response types that the CLI backend will share get defined.

navicore commented

2026-05-29 13:19:55 +00:00

Author

Owner

Review: Increment 1 — admin API foundation

Verdict: solid, ready to merge. Tightly-scoped, well-tested foundation. It does exactly what it claims, and nothing observable changes for current deployments (no admin_bind_address → no second listener, no /admin/*). Verified locally: clippy clean with -D warnings, all 125 tests pass.

Verified against the code

realm in token.rs is the realm name (Path(realm)), so access_token_lifetime_for(realm) / refresh_token_lifetime_for(realm) get the right argument in all three grant paths, and expires_in now reflects the effective lifetime rather than the global one. Consistent across auth_code / refresh / client_credentials.
decode_access_token validates issuer (set_issuer) and matches kid, so the "reject tokens from any other realm" claim holds two ways over — wrong iss and wrong kid.
get_all_keys returns active + deactivated keys (no active filter), so a token signed by a since-rotated admin key still verifies until hard-delete — matching the comment.
Live-state authz in authorize_admin fails closed: realm-binding (realm_id == admin_realm.id), !disabled, and anz-admin group all checked against fresh DB state, not token claims. The 401 (bad/missing/foreign-signed token) vs 403 (valid token, not an admin) split is clean.

Test coverage is the strong point

The adversarial cases that matter are all present — notably whoami_rejects_admin_group_in_foreign_realm (admin-realm-signed token whose sub lives in another realm but carries anz-admin) exercises exactly the realm-binding guard a naive implementation would miss. Plus disabled, no-group, missing-token, and foreign-realm-signed.

Worth noting (none blocking — forward-looking)

No audit event on denied admin access. The design's Domain Events specify admin_access_denied should be audited on 403. The security boundary (the extractor) lands now, so logging denied 401/403 attempts on this high-value surface arguably belongs in Increment 1 rather than waiting for the operation endpoints. Currently only TraceLayer covers it.
Forbidden("admin realm not provisioned") returns 403 for a missing admin realm — a server misconfiguration conflated with an authz denial. Fail-closed is safe, so this is a nitpick, but 401/500 might read truer.
No rate limiting on the admin listener. The OIDC login path has per-IP limiting; the admin path doesn't. Token brute-force is cryptographically infeasible so risk is low, but worth a thought once write endpoints land.

## Review: Increment 1 — admin API foundation **Verdict: solid, ready to merge.** Tightly-scoped, well-tested foundation. It does exactly what it claims, and nothing observable changes for current deployments (no `admin_bind_address` → no second listener, no `/admin/*`). Verified locally: clippy clean with `-D warnings`, all 125 tests pass. ### Verified against the code - **`realm` in `token.rs` is the realm *name*** (`Path(realm)`), so `access_token_lifetime_for(realm)` / `refresh_token_lifetime_for(realm)` get the right argument in all three grant paths, and `expires_in` now reflects the effective lifetime rather than the global one. Consistent across auth_code / refresh / client_credentials. - **`decode_access_token` validates issuer (`set_issuer`) and matches `kid`**, so the "reject tokens from any other realm" claim holds two ways over — wrong `iss` *and* wrong `kid`. - **`get_all_keys` returns active + deactivated keys** (no active filter), so a token signed by a since-rotated admin key still verifies until hard-delete — matching the comment. - **Live-state authz in `authorize_admin` fails closed**: realm-binding (`realm_id == admin_realm.id`), `!disabled`, and `anz-admin` group all checked against fresh DB state, not token claims. The 401 (bad/missing/foreign-signed token) vs 403 (valid token, not an admin) split is clean. ### Test coverage is the strong point The adversarial cases that matter are all present — notably `whoami_rejects_admin_group_in_foreign_realm` (admin-realm-signed token whose `sub` lives in another realm but carries `anz-admin`) exercises exactly the realm-binding guard a naive implementation would miss. Plus disabled, no-group, missing-token, and foreign-realm-signed. ### Worth noting (none blocking — forward-looking) 1. **No audit event on denied admin access.** The design's Domain Events specify `admin_access_denied` should be audited on 403. The security boundary (the extractor) lands *now*, so logging denied 401/403 attempts on this high-value surface arguably belongs in Increment 1 rather than waiting for the operation endpoints. Currently only `TraceLayer` covers it. 2. **`Forbidden("admin realm not provisioned")` returns 403 for a missing admin realm** — a server misconfiguration conflated with an authz denial. Fail-closed is safe, so this is a nitpick, but 401/500 might read truer. 3. **No rate limiting on the admin listener.** The OIDC login path has per-IP limiting; the admin path doesn't. Token brute-force is cryptographically infeasible so risk is low, but worth a thought once write endpoints land.

navicore added 1 commit

2026-05-29 13:32:56 +00:00

● 126 passing, clippy clean. Two nits addressed:

CI / ci (pull_request) Successful in 1m44s

Details

5d2dd58656

Nit: #1 audit on denial
  Resolution: New AuditAction::AdminAccessDenied. Every 401/403 from the admin extractor emits an audit event with

    a reason in detail (no_bearer / invalid_token / foreign_realm / disabled / no_admin_group / unknown_subject)
    and the verified sub when we have it. Peer IP is pulled from ConnectInfo extensions when present (production),

    None otherwise (test harness with oneshot). New test (denials_are_audited_with_reason) writes to a real temp
    audit file and verifies the JSON shape — locks the doc's Domain Events contract.
  ────────────────────────────────────────
  Nit: #2 missing admin realm
  Resolution: Now returns AppError::Internal (500) with a tracing::error! log — server misconfig signaled honestly

    instead of conflated with authz denial. Not audited (it's a config bug, not an access attempt).

  Nit #3 (rate limiting) stays deferred to Increment 3 per the reviewer's own framing.

navicore merged commit c3217ed902 into main

2026-05-29 13:35:27 +00:00

navicore deleted branch remote

2026-05-29 13:35:28 +00:00

navicore referenced this pull request from a commit

2026-05-29 13:35:29 +00:00

Merge pull request '● Increment 1 complete. Marking it done.' (#37) from remote into main