feat: wallet instance lifecycle - list, suspend/revoke, deactivate (SID-AUTH-06) - #135
Conversation
…ID-AUTH-06) Client half of go-wallet-backend#319 (issue go-wallet-backend#195). - generateWIA sends the logged-in passkey's credential_id so the backend links the wallet instance to the passkey and a suspended or revoked instance also refuses that passkey at login. - WalletInstance type; BackendApiClient.listWalletInstances / setWalletInstanceStatus / revokeAllWalletInstances (+ a PUT primitive). - SirosWallet.listWalletInstances(), setWalletInstanceStatus(), deactivateWallet(reason:) - the last revokes every instance server-side and then forgets the local account, since the vault it decrypts no longer exists. - SirosError.walletLifecycleRefusal reads WALLET_SUSPENDED / WALLET_REVOKED off a 403 backendApi error without adding an enum case, so exhaustive switches keep compiling. Older backends ignore credential_id and answer 404 on the new endpoints, surfaced as the usual backendApi error; nothing needs to ship in lockstep. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
There was a problem hiding this comment.
🟡 Changes recommended
The new lifecycle API methods include response-handling behaviors that can mask malformed backend responses and return partially populated models, which should be corrected before approval.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Implements the client-side of wallet instance lifecycle management (SID-AUTH-06) in the Swift SDK, including backend API support, wallet facade methods, and lifecycle-based login refusal parsing.
Changes:
- Add
WalletInstanceplusBackendApiClientendpoints for listing instances, updating instance status (PUT), and revoking all instances (deactivate). - Link WIA generation to the active passkey via
credential_idso backend instance suspension/revocation can refuse login for that passkey. - Add
SirosError.walletLifecycleRefusalhelper and targeted unit tests for lifecycle refusal parsing and new backend calls.
File summaries
| File | Description |
|---|---|
| Tests/SirosCredentialsTests/SirosErrorLifecycleTests.swift | Adds unit tests for parsing lifecycle refusal codes from 403 backend errors. |
| Tests/SirosAuthTests/BackendApiClientTests.swift | Adds tests for generateWIA(credentialId:), listing instances, PUT status updates, and revoke-all behavior. |
| Sources/SirosWallet/SirosWallet+Issuance.swift | Passes sessionStore.credentialId into WIA generation to link instance ↔ passkey. |
| Sources/SirosWallet/SirosWallet.swift | Exposes facade methods for listing instances, setting instance status, and deactivating the wallet. |
| Sources/SirosCredentials/SirosError.swift | Adds WalletLifecycleRefusal and a helper to extract refusal codes from 403 backend errors without adding a new enum case. |
| Sources/SirosAuth/WalletInstance.swift | Introduces a public WalletInstance model and JSON decoding initializer for backend responses. |
| Sources/SirosAuth/BackendApiClient.swift | Adds lifecycle endpoints, extends WIA generation to include credential_id, and introduces a PUT HTTP primitive. |
| CHANGELOG.md | Documents the new lifecycle APIs and behavior in the Unreleased section. |
Review details
- Files reviewed: 8/8 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…tus reply, facade tests Copilot review on #135: a response without the instances array is a malformed reply, not an empty wallet; a status update decodes the whole WalletInstance when the backend returns one; and the facade's not-logged-in and forwarding paths are now covered without touching the Keychain. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
There was a problem hiding this comment.
🟡 Changes recommended
deactivateWallet may not actually log out/clear session state when an active account exists, and revokeAllWalletInstances can silently misreport malformed responses by defaulting the revoked count to 0.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
Sources/SirosAuth/BackendApiClient.swift:337
revokeAllWalletInstances(reason:)returns0when the backend response is missing/invalid for therevokedfield, which can silently misreport the outcome (0 is a valid value). It’s safer to treat a missing count as a malformed response and surface aSirosError.backendApiparse error instead of defaulting.
- Files reviewed: 9/9 changed files
- Comments generated: 1
- Review effort level: Lite
…malformed reply Copilot on #135: AccountRegistry.removeAccount clears the active id, so forgetAccount's check after removal never matched and the active account was forgotten without logging out - deactivateWallet inherited that. Decide before removing. And a revoke-all reply without the count is an error, not zero revoked. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
There was a problem hiding this comment.
🟢 Approval recommended
The new APIs are covered by focused unit tests, error handling for malformed responses is exercised, and the account/logout edge case in deactivation is addressed.
Review details
- Files reviewed: 9/9 changed files
- Comments generated: 0 new
- Review effort level: Lite
…ce keeps a two-member init (Sonar S1075, S107) Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
There was a problem hiding this comment.
🔵 Needs a closer look
It introduces new auth/lifecycle behaviors and backend write calls (PUT/POST) that affect login eligibility and account deactivation, warranting final human review despite good test coverage.
Review details
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
Tests/SirosWalletTests/SirosWalletSignRequestTests.swift:382
Recorderis marked@unchecked Sendableand its@SendablehttpFnmutatescalls/responseswithout any synchronization. If the client ever issues requests concurrently (or tests are run with concurrency checking), this can introduce data races; additionally,removeFirst()will trap if the response queue is exhausted, crashing the test instead of failing cleanly.
- Files reviewed: 9/9 changed files
- Comments generated: 0 new
- Review effort level: Lite
Client half of sirosfoundation/go-wallet-backend#319 (design in sirosfoundation/go-wallet-backend#195; Android twin: siros-sdk-kotlin PR to follow). Tracked on the shared rollout page: https://claude.ai/code/artifact/49eb6eed-38ed-41b8-8168-3b5a300c33da
What changes
BackendApiClient.generateWIAgainscredentialId, andensureWalletInstanceAttestationpassessessionStore.credentialId. The backend records it asWalletInstance.CredentialID, so suspending or revoking this installation's instance also refuses login with its passkey. Without it, only whole-wallet deactivation gates login.WalletInstancetype (SirosAuth), andBackendApiClient.listWalletInstances(),setWalletInstanceStatus(instanceId:status:reason:),revokeAllWalletInstances(reason:)overGET/PUT /user/session/instances…andPOST …/revoke-all. Adds the client's firstPUTprimitive.SirosWallet.listWalletInstances(),setWalletInstanceStatus(...),deactivateWallet(reason:). Deactivate revokes every instance server-side, then forgets the cached account and logs out, since the vault it decrypts no longer exists.SirosError.walletLifecycleRefusalreadsWALLET_SUSPENDED/WALLET_REVOKEDoff a403 .backendApierror. A helper rather than a new enum case, so apps' exhaustiveswitches keep compiling. Neither refusal is retryable; apps should say so instead of retrying.Compatibility
Old backends ignore
credential_idand answer 404 on the new endpoints (surfaced asSirosError.backendApi). New backends work with old SDKs. No lockstep release needed.Tests
BackendApiClientTests:credential_idpresent only when given; list decodes the backend shape and skips unknown statuses; PUT body and path; revoke-all count.SirosErrorLifecycleTests: the helper reads both codes and returns nil for other 403s, non-403s, non-JSON bodies and other error cases. Linuxswift buildand the two suites pass; macOSswift testand the iOS build run in CI. No facade test:accountRegistryis Keychain-backed and private.Sample-app UI is out of scope here; the SDK exposes the calls.
🤖 Generated with Claude Code