feat(lifecycle): wallet instance suspend/revoke with login gate and self-service (SID-AUTH-06) - #319
Open
leifj wants to merge 1 commit into
Open
feat(lifecycle): wallet instance suspend/revoke with login gate and self-service (SID-AUTH-06)#319leifj wants to merge 1 commit into
leifj wants to merge 1 commit into
Conversation
…elf-service (SID-AUTH-06) Implements #195 on the WalletInstance model instead of a parallel per-passkey status, superseding #196. - WalletLifecycleService: validated transitions, audit, and the cascade a deactivation implies. Suspension is reversible and only blocks; revocation is terminal; revoking the last non-revoked instance deactivates the wallet: private data, server-side credentials/presentations and pending challenges are erased, live sessions dropped. Data is never erased while an instance the user could still reactivate remains. The user record and passkeys stay so the refusal is attributable ("wallet revoked", not "user not found"). - Login gate in WebAuthnService.FinishLogin: the instance linked to the passkey must be active; a wallet with every instance revoked refuses every passkey. Both login handlers (legacy and AS) return 403 WALLET_SUSPENDED / WALLET_REVOKED. - Passkey link: WIA generate accepts an optional credential_id, recorded as WalletInstance.CredentialID (the field existed but nothing set it). - Self-service endpoints under /user/session/instances: list, change status of own instance, revoke-all. Admin status changes go through the same service when wired, so both paths share one cascade. - Engine session store wired into the lifecycle service (main.go). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The login handler currently returns a misleading “wallet deactivated / re-enroll” message for revocation cases where only a single instance is revoked, and the wallet erasure path silently falls back to default-tenant cleanup without warning when tenant membership lookup fails.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR implements SID-AUTH-06 wallet instance lifecycle management on top of the existing WalletInstance model, adding validated suspend/revoke transitions with a deactivation (erasure) cascade, plus login gating and self-service/admin APIs to manage instances.
Changes:
- Introduces
WalletLifecycleServiceto centralize lifecycle transitions, auditing, session dropping, and “last-instance revoked” wallet-data erasure. - Adds optional
credential_idplumbing (WIA generate → instance store) to link a passkey to a wallet instance for per-device login refusal. - Enforces lifecycle state at login (post-assertion verification) and exposes self-service instance management endpoints; wires admin status changes through the same lifecycle service when available.
File summaries
| File | Description |
|---|---|
| internal/storage/mongodb/wallet_instance.go | Persist credential_id on upsert without changing status for existing instances. |
| internal/storage/memory/wallet_instance.go | Mirror credential_id persistence behavior in memory store. |
| internal/storage/memory/wallet_instance_test.go | Adds regression test ensuring upsert records credential_id without reactivating status. |
| internal/service/wua_status_claims_test.go | Updates tests for signWIA signature change. |
| internal/service/wia.go | Extends WIA request to accept credential_id and records it onto wallet instances. |
| internal/service/webauthn.go | Adds SID-AUTH-06 login gate enforcing instance/wallet lifecycle at authentication. |
| internal/service/webauthn_lifecycle_test.go | Unit tests for login-gate lifecycle enforcement behavior. |
| internal/service/wallet_lifecycle.go | New lifecycle service implementing transitions, audit emission, session drop, and erasure cascade. |
| internal/service/wallet_lifecycle_test.go | Unit tests for suspend/revoke, revoke-all, ownership, terminal transitions, and erasure behavior. |
| internal/service/services.go | Registers WalletLifecycleService in the service container. |
| internal/server/providers.go | Wires self-service routes and injects lifecycle service into admin handlers when available. |
| internal/as/passkey.go | Maps lifecycle refusal errors to stable 403 codes in AS passkey finish handler. |
| internal/as/passkey_test.go | Tests 403 lifecycle refusal mapping for AS passkey finish handler. |
| internal/api/wia_handlers.go | Adds credential_id field to WIA generate API request. |
| internal/api/instance_handlers.go | New self-service endpoints for listing/updating/revoking instances via lifecycle service. |
| internal/api/instance_handlers_test.go | Tests self-service instance lifecycle endpoints, ownership behavior, and terminal states. |
| internal/api/handlers.go | Maps lifecycle refusal errors to 403 responses in standard login finish handler. |
| internal/api/admin_instance_handlers.go | Routes admin status changes through lifecycle service (with cascade) when configured. |
| internal/api/admin_instance_handlers_test.go | Tests admin lifecycle cascade behavior when lifecycle service is wired. |
| internal/api/admin_handlers.go | Adds lifecycle service wiring hook to admin handlers. |
| docs/API.md | Documents wallet instance lifecycle semantics and self-service endpoints. |
| cmd/server/main.go | Wires session store into lifecycle service so suspend/revoke drops live sessions. |
Review details
- Files reviewed: 22/22 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+285
to
+288
| case errors.Is(err, service.ErrWalletInstanceSuspended): | ||
| c.JSON(403, gin.H{"error": "WALLET_SUSPENDED", "message": "This wallet instance has been suspended"}) | ||
| case errors.Is(err, service.ErrWalletInstanceRevoked): | ||
| c.JSON(403, gin.H{"error": "WALLET_REVOKED", "message": "This wallet has been deactivated; a new enrollment is required"}) |
Comment on lines
+176
to
+179
| tenantIDs, err := s.store.UserTenants().GetUserTenants(ctx, userID) | ||
| if err != nil || len(tenantIDs) == 0 { | ||
| tenantIDs = []domain.TenantID{domain.DefaultTenantID} | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements #195 (SID-AUTH-06, tracking sirosfoundation/compliance#78) on top of the existing
WalletInstancemodel, and supersedes #196, which put a second, per-passkeydeactivatedstatus next to it.@smncd requesting your review: this changes login behaviour and adds an erasure cascade.
Why not #196
Main grew a first-class wallet instance model after #196 was written: instances keyed by instance-key thumbprint,
active/suspended/revokedwith validated transitions, admin endpoints, audit events, and enforcement at WIA generation. #196 would have added a disconnected passkey-level lifecycle beside it, had no reversible state, and wiped the user's private data on lockout, which is irreversible and shared across the user's devices.What this adds
WalletLifecycleService(internal/service/wallet_lifecycle.go): validated transitions, audit, and the cascade. Suspension is reversible and only blocks. Revocation is terminal. Revoking the last non-revoked instance deactivates the wallet: private data, server-side credentials/presentations and pending challenges are erased and live sessions dropped, so re-activation requires a full new enrollment. Nothing is erased while a suspended (reactivatable) instance remains. The user record and passkeys stay so login can be refused with a clear reason.FinishLogin: the instance linked to the passkey must be active; a wallet with every instance revoked refuses every passkey. Checked only after the assertion verified. Both login handlers return403 WALLET_SUSPENDED/WALLET_REVOKED.POST /wallet-provider/wia/generateaccepts optionalcredential_id, recorded asWalletInstance.CredentialID. That field was documented but nothing ever set it. Without it the gate still enforces whole-wallet deactivation; with it, per-device suspension also blocks that device's login./user/session/instances:GETlist,PUT {id}/status,POST revoke-all. Ownership is checked; other users' instances read as 404.main.go.Client follow-ups (not in this PR)
credential_idat WIA generation, and show the instance list / offer suspend and deactivate (wallet-frontend#148 needs re-scoping from the Implement wallet credential lifecycle deactivation and enforcement (SID-AUTH-06) #196 API to this one).Tests
Service: suspend blocks without erasing and can be reversed; revoking the last instance erases, an earlier one does not; revoke-all; ownership, tenant and terminal-state rules. Login gate: no instances, linked suspended/revoked, unlinked suspended (does not block other passkeys), all revoked. Handlers: self-service list/update/not-owned/bad status/revoke-all/terminal; admin cascade through the lifecycle service; AS and passkey 403 mapping; memory store records the passkey link without touching status. Full
go test ./...andgolangci-lintclean.🤖 Generated with Claude Code