● Increment 3 complete. 147 tests pass (+13), clippy clean. Marking it done: #39
No reviewers
Labels
No labels
bug
dependencies
documentation
duplicate
enhancement
good first issue
help wanted
invalid
question
rust
wontfix
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
navicore/anz!39
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "increment-3"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
● What landed:
┌────────────────┬───────────────────────────────┬──────────────────────────────────────────────────────────┐
│ Piece │ File │ Notes │
├────────────────┼───────────────────────────────┼──────────────────────────────────────────────────────────┤
│ │ │ RealmCreated/Deleted/MfaRequirementChanged, │
│ 12 new audit │ src/audit.rs │ KeyRotated/Deactivated/Deleted, │
│ actions │ │ UserCreated/Deleted/GroupsUpdated, │
│ │ │ ClientCreated/Deleted, SessionsRevoked │
├────────────────┼───────────────────────────────┼──────────────────────────────────────────────────────────┤
│ │ │ Request DTOs (CreateRealm, SetMfaRequired, RotateKey, │
│ 9 new contract │ src/admin_api.rs │ CreateUser, UpdateGroups, CreateClient) + secret-once │
│ types │ │ response DTOs (ClientCreatedResponse, │
│ │ │ MfaEnrollmentResponse, RevokeSessionsResponse) │
├────────────────┼───────────────────────────────┼──────────────────────────────────────────────────────────┤
│ audit_detail() │ │ AdminContext::audit_detail() returns "source=admin-api │
│ helper │ src/server/admin/auth.rs │ admin_sub="; every write handler attributes the │
│ │ │ operator consistently │
├────────────────┼───────────────────────────────┼──────────────────────────────────────────────────────────┤
│ │ src/server/admin/{lists.rs → │ Deleted lists.rs; each new file holds both reads + │
│ Reorganization │ realms.rs, users.rs, │ writes for its resource. Mirrors │
│ │ clients.rs} │ cli/{realm,user,client}.rs — sets up Increment 4 where │
│ │ │ each cli/*.rs backs one resource module. │
├────────────────┼───────────────────────────────┼──────────────────────────────────────────────────────────┤
│ │ │ All 22 admin routes wired (2 probes + 20 resource). │
│ Lean router │ src/server/admin/mod.rs │ Module declarations + router + probes + AdminContext │
│ │ │ re-export, ~190 lines including the route-coverage test. │
└────────────────┴───────────────────────────────┴──────────────────────────────────────────────────────────┘
Routes added beyond Increment 2 (13 writes):
┌────────┬─────────────────────────────────────────────────┬─────────────────────────────────────────────────┐
│ Verb │ Path │ What │
├────────┼─────────────────────────────────────────────────┼─────────────────────────────────────────────────┤
│ POST │ /admin/realms │ create realm │
├────────┼─────────────────────────────────────────────────┼─────────────────────────────────────────────────┤
│ DELETE │ /admin/realms/{realm} │ delete realm (cascades) │
├────────┼─────────────────────────────────────────────────┼─────────────────────────────────────────────────┤
│ PUT │ /admin/realms/{realm}/mfa-required │ toggle MFA requirement │
├────────┼─────────────────────────────────────────────────┼─────────────────────────────────────────────────┤
│ POST │ /admin/realms/{realm}/keys │ rotate (add active key) │
├────────┼─────────────────────────────────────────────────┼─────────────────────────────────────────────────┤
│ POST │ /admin/realms/{realm}/keys/{kid}/deactivate │ deactivate (stays in JWKS) │
├────────┼─────────────────────────────────────────────────┼─────────────────────────────────────────────────┤
│ DELETE │ /admin/realms/{realm}/keys/{kid} │ hard-delete (verification fails immediately) │
├────────┼─────────────────────────────────────────────────┼─────────────────────────────────────────────────┤
│ POST │ /admin/realms/{realm}/users │ create user (server-side Argon2id) │
├────────┼─────────────────────────────────────────────────┼─────────────────────────────────────────────────┤
│ DELETE │ /admin/realms/{realm}/users/{username} │ remove user │
├────────┼─────────────────────────────────────────────────┼─────────────────────────────────────────────────┤
│ PUT │ /admin/realms/{realm}/users/{username}/groups │ replace group set │
├────────┼─────────────────────────────────────────────────┼─────────────────────────────────────────────────┤
│ POST │ /admin/realms/{realm}/users/{username}/disable │ disable + revoke sessions + revoke refresh │
│ │ │ tokens │
├────────┼─────────────────────────────────────────────────┼─────────────────────────────────────────────────┤
│ POST │ /admin/realms/{realm}/users/{username}/enable │ re-enable │
├────────┼─────────────────────────────────────────────────┼─────────────────────────────────────────────────┤
│ POST │ /admin/realms/{realm}/users/{username}/mfa │ enroll TOTP, returns secret+URI+recovery codes │
│ │ │ once │
├────────┼─────────────────────────────────────────────────┼─────────────────────────────────────────────────┤
│ DELETE │ /admin/realms/{realm}/users/{username}/mfa │ disable MFA (recovery path) │
├────────┼─────────────────────────────────────────────────┼─────────────────────────────────────────────────┤
│ DELETE │ /admin/realms/{realm}/users/{username}/sessions │ revoke all sessions, returns count │
├────────┼─────────────────────────────────────────────────┼─────────────────────────────────────────────────┤
│ POST │ /admin/realms/{realm}/clients │ create regular or service-account client; │
│ │ │ returns generated secret once if minted │
├────────┼─────────────────────────────────────────────────┼─────────────────────────────────────────────────┤
│ DELETE │ /admin/realms/{realm}/clients/{client_id} │ remove client │
└────────┴─────────────────────────────────────────────────┴─────────────────────────────────────────────────┘
Security posture, verified by tests:
afterward; subsequent fetches report has_secret: true only
recovery codes (existing crypto path reused)
iterates all of them and asserts 401 without bearer. Failing assertion names the offending METHOD path, so a
future copy-paste that forgets the extractor breaks loudly.
Audit attribution: every successful write emits an audit event with detail containing source=admin-api
admin_sub=<operator_uid> plus operation-specific fields (e.g. algorithm=EdDSA kid= for key rotation,
sessions=N refresh_tokens=N for user disable).
● What landed: ┌────────────────┬───────────────────────────────┬──────────────────────────────────────────────────────────┐ │ Piece │ File │ Notes │ ├────────────────┼───────────────────────────────┼──────────────────────────────────────────────────────────┤ │ │ │ RealmCreated/Deleted/MfaRequirementChanged, │ │ 12 new audit │ src/audit.rs │ KeyRotated/Deactivated/Deleted, │ │ actions │ │ UserCreated/Deleted/GroupsUpdated, │ │ │ │ ClientCreated/Deleted, SessionsRevoked │ ├────────────────┼───────────────────────────────┼──────────────────────────────────────────────────────────┤ │ │ │ Request DTOs (CreateRealm, SetMfaRequired, RotateKey, │ │ 9 new contract │ src/admin_api.rs │ CreateUser, UpdateGroups, CreateClient) + secret-once │ │ types │ │ response DTOs (ClientCreatedResponse, │ │ │ │ MfaEnrollmentResponse, RevokeSessionsResponse) │ ├────────────────┼───────────────────────────────┼──────────────────────────────────────────────────────────┤ │ audit_detail() │ │ AdminContext::audit_detail() returns "source=admin-api │ │ helper │ src/server/admin/auth.rs │ admin_sub=<id>"; every write handler attributes the │ │ │ │ operator consistently │ ├────────────────┼───────────────────────────────┼──────────────────────────────────────────────────────────┤ │ │ src/server/admin/{lists.rs → │ Deleted lists.rs; each new file holds both reads + │ │ Reorganization │ realms.rs, users.rs, │ writes for its resource. Mirrors │ │ │ clients.rs} │ cli/{realm,user,client}.rs — sets up Increment 4 where │ │ │ │ each cli/*.rs backs one resource module. │ ├────────────────┼───────────────────────────────┼──────────────────────────────────────────────────────────┤ │ │ │ All 22 admin routes wired (2 probes + 20 resource). │ │ Lean router │ src/server/admin/mod.rs │ Module declarations + router + probes + AdminContext │ │ │ │ re-export, ~190 lines including the route-coverage test. │ └────────────────┴───────────────────────────────┴──────────────────────────────────────────────────────────┘ Routes added beyond Increment 2 (13 writes): ┌────────┬─────────────────────────────────────────────────┬─────────────────────────────────────────────────┐ │ Verb │ Path │ What │ ├────────┼─────────────────────────────────────────────────┼─────────────────────────────────────────────────┤ │ POST │ /admin/realms │ create realm │ ├────────┼─────────────────────────────────────────────────┼─────────────────────────────────────────────────┤ │ DELETE │ /admin/realms/{realm} │ delete realm (cascades) │ ├────────┼─────────────────────────────────────────────────┼─────────────────────────────────────────────────┤ │ PUT │ /admin/realms/{realm}/mfa-required │ toggle MFA requirement │ ├────────┼─────────────────────────────────────────────────┼─────────────────────────────────────────────────┤ │ POST │ /admin/realms/{realm}/keys │ rotate (add active key) │ ├────────┼─────────────────────────────────────────────────┼─────────────────────────────────────────────────┤ │ POST │ /admin/realms/{realm}/keys/{kid}/deactivate │ deactivate (stays in JWKS) │ ├────────┼─────────────────────────────────────────────────┼─────────────────────────────────────────────────┤ │ DELETE │ /admin/realms/{realm}/keys/{kid} │ hard-delete (verification fails immediately) │ ├────────┼─────────────────────────────────────────────────┼─────────────────────────────────────────────────┤ │ POST │ /admin/realms/{realm}/users │ create user (server-side Argon2id) │ ├────────┼─────────────────────────────────────────────────┼─────────────────────────────────────────────────┤ │ DELETE │ /admin/realms/{realm}/users/{username} │ remove user │ ├────────┼─────────────────────────────────────────────────┼─────────────────────────────────────────────────┤ │ PUT │ /admin/realms/{realm}/users/{username}/groups │ replace group set │ ├────────┼─────────────────────────────────────────────────┼─────────────────────────────────────────────────┤ │ POST │ /admin/realms/{realm}/users/{username}/disable │ disable + revoke sessions + revoke refresh │ │ │ │ tokens │ ├────────┼─────────────────────────────────────────────────┼─────────────────────────────────────────────────┤ │ POST │ /admin/realms/{realm}/users/{username}/enable │ re-enable │ ├────────┼─────────────────────────────────────────────────┼─────────────────────────────────────────────────┤ │ POST │ /admin/realms/{realm}/users/{username}/mfa │ enroll TOTP, returns secret+URI+recovery codes │ │ │ │ once │ ├────────┼─────────────────────────────────────────────────┼─────────────────────────────────────────────────┤ │ DELETE │ /admin/realms/{realm}/users/{username}/mfa │ disable MFA (recovery path) │ ├────────┼─────────────────────────────────────────────────┼─────────────────────────────────────────────────┤ │ DELETE │ /admin/realms/{realm}/users/{username}/sessions │ revoke all sessions, returns count │ ├────────┼─────────────────────────────────────────────────┼─────────────────────────────────────────────────┤ │ POST │ /admin/realms/{realm}/clients │ create regular or service-account client; │ │ │ │ returns generated secret once if minted │ ├────────┼─────────────────────────────────────────────────┼─────────────────────────────────────────────────┤ │ DELETE │ /admin/realms/{realm}/clients/{client_id} │ remove client │ └────────┴─────────────────────────────────────────────────┴─────────────────────────────────────────────────┘ Security posture, verified by tests: - password_hash never appears even on user-create response; literal password doesn't echo back - client_secret raw value appears only in POST /admin/realms/.../clients response when generated, never afterward; subsequent fetches report has_secret: true only - MFA enrollment returns plaintext secret + URI + recovery codes once; storage uses encrypted secret + hashed recovery codes (existing crypto path reused) - Service-account validation: requires audience AND non-empty scopes (400 otherwise), mirrors local CLI - Regular client validation: rejects empty / duplicate redirect_uris (400) - All 22 admin routes (2 probes + 20 resource) go through AdminContext — every_admin_route_requires_auth iterates all of them and asserts 401 without bearer. Failing assertion names the offending METHOD path, so a future copy-paste that forgets the extractor breaks loudly. Audit attribution: every successful write emits an audit event with detail containing source=admin-api admin_sub=<operator_uid> plus operation-specific fields (e.g. algorithm=EdDSA kid=<kid> for key rotation, sessions=N refresh_tokens=N for user disable).Review: Increment 3 — admin write endpoints
Verdict: solid, ready to merge. The 13 write routes mirror the local CLI faithfully, every write is audited with operator attribution, and the secret-handling contract holds. Verified locally: clippy clean with
-D warnings, 147 tests pass. My Increment 2 note is resolved (every_admin_route_requires_authiterates all 22 routes and asserts 401 without a bearer, naming the offending METHOD+path on failure).Verified against the code
cli/client.rs:88-90): 32 random bytes →URL_SAFE_NO_PAD→hex(SHA256(raw))stored as the hash.verify_client_secret(token.rs:496) hashes the presented secret the same way and compares constant-time. So an API-created client (regular or service account) authenticates correctly at/token. Raw secret appears once in the create response;has_secretthereafter.password_hashabsent even on user-create (test also asserts the literal password doesn't echo); MFA secret/URI/recovery codes returned once while storage uses the encrypted-secret + hashed-recovery-code path; signing-key responses stay metadata-only.disable_userdoes the full job — sets the flag, revokes sessions, revokes refresh tokens, and auditssessions=N refresh_tokens=N. Idempotent when already disabled.audit_detail()stampssource=admin-api admin_sub=<uid>on every write, plus operation-specific fields (kid=,algorithm=,count=,groups=, …).user_mfa::enrollis transactional (delete-then-insert), so re-enrolling an already-enrolled user replaces cleanly rather than erroring.Worth addressing
AppError'sFrom<rusqlite::Error>/From<anyhow::Error>both map toInternal→ 500 (and atracing::error!). So creating a realm/user/client with a name that already exists hits the UNIQUE constraint and surfaces as500 internal server error— a client-caused error reported as a server fault, and it pollutes the error log. Recommend mapping the constraint violation (or a pre-existence check) to 409 Conflict before Increment 4's CLI lands, so the CLI can render "already exists" instead of "internal server error". This is the one I'd actually want fixed. (Note: MFA enroll is exempt — it's an idempotent upsert.)anz-adminfrom themselves. It's recoverable via the local break-glass CLI — which is the design's intent — but a guard (refuse to deleteconfig.admin_realm, or refuse to remove the lastanz-adminmember) would turn a one-command outage into a clear 4xx. At minimum worth a deliberate "we accept this" decision.Minor / cosmetic
create_client+configure_service_accountare two non-transactional statements; a failure on the second leaves a created-but-unconfigured client. Matches the CLI pattern, but a partial create is more surprising over a remote API — a transaction would make it atomic.rotate_keybuilds the responsecreated_atfromUtc::now()rather than the stored row, so the DTO timestamp can drift from storage by microseconds. Purely cosmetic.