feat(auth): add admin MFA factor listing and deletion - #1357
AndroidPoet wants to merge 1 commit into
Conversation
Adds `auth.admin.mfa` with `listFactors(forUser:)` and `deleteFactor(id:forUser:)`, covering the two capabilities sdk-compliance.yaml tracked as not_implemented: `auth.admin.list_mfa_factors` and `auth.admin.delete_mfa_factor`.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (4)
Included review availability: Your plan provides up to 4 included reviews per hour; 3 remain after this review. 📝 SummarySummary by CodeRabbit
WalkthroughThe change adds the public Sequence Diagram(s)sequenceDiagram
participant Application
participant AuthAdminMFA
participant Dependencies
participant APIClient
participant MFAAPI
Application->>AuthAdminMFA: listFactors or deleteFactor
AuthAdminMFA->>Dependencies: resolve client configuration
AuthAdminMFA->>APIClient: build authenticated request
APIClient->>MFAAPI: GET or DELETE admin factor endpoint
MFAAPI-->>APIClient: factor response
APIClient-->>AuthAdminMFA: decoded factors or completion
AuthAdminMFA-->>Application: return result
Priority: ⬇️ Low Change: Feature Merge Risk: ⚪ Minimal · up to The admin MFA list and delete APIs follow the established endpoint contract, with no actionable merge-blocking issue identified. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Adds
auth.admin.mfawithlistFactors(forUser:)anddeleteFactor(id:forUser:), over the two admin MFA endpoints Auth already serves (GETandDELETE /admin/users/{user_id}/factors). A backend can now see which factors a user has enrolled and remove one, for example when the user has lost their authenticator and cannot sign in to unenroll it themselves.Marks
auth.admin.list_mfa_factorsandauth.admin.delete_mfa_factorimplemented insdk-compliance.yaml.Shape
id:and comes first, the owning user isforUser:, the same asdeletePasskey(id:forUser:)there. The admin passkey methods onmainstill takeuserId:, so this PR is ahead of them until refactor!: align the public API with the Swift API Design Guidelines #1355 lands, and refactor!: align the public API with the Swift API Design Guidelines #1355 has nothing to rename here.deleteFactorreturns nothing, likedeleteUser,deleteClientanddeletePasskey. Auth echoes the deleted factor back, but the caller already has it, and not decoding the body means a delete that succeeded on the server can never surface as a decoding error.idis aStringbecauseFactor.idis one, sofactor.idpasses straight through, as it does forAuthMFA.unenroll.listFactorsreturns[Factor], the modelauth.mfa.listFactors()already decodes. Its optionalfriendlyNamematters here, since Auth omitsfriendly_namewhen it is empty.AuthAdmin.mfaandAuthAdminMFAgo in the top-levelsupporting_symbolslist rather than under either capability, because both capabilities hang off them. That is what the capability matrix docs ask for with shared symbols, and the new-symbol check fails without them.Worth knowing
Deleting a factor also downgrades that user's sessions to AAL1 on the server (
DowngradeSessionsToAAL1inadminUserDeleteFactor), so a signed-in user loses AAL2 immediately. The doc comment says so.Testing
AuthAdminMFATestscovers request snapshots for both calls, a populated list, an empty list, and a factor without a friendly name.I also ran both methods against a local Auth (v2.185.0) with a throwaway test that is not part of this PR:
friendlyName == nil.AuthErrorwith status 404 (Factor not found).[]. Worth checking because the handler sendsuser.Factorsdirectly, and a nil Go slice would encode asnull.The capability matrix checks from
validate-sdk-compliance-swift.yml(at the pinnedcapability-matrix-v1.6.0) pass locally againstmain, with symbol graphs dumped and normalized the same way CI does.Review first
Namespace or flat. This follows the JS client (
auth.admin.mfa.listFactors) and mirrorsauth.mfaon the user side, next to the existingauth.admin.oauth. Admin passkeys went flat onAuthAdmininstead, so if you would rather haveadmin.listFactors(forUser:)to match them, it is a small move.