feat(passkeys): add management endpoints#2413
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Central YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds authenticated passkey management: GET /passkeys/ to list a user’s passkeys, PATCH /passkeys/{passkey_id} to update a passkey’s friendly name, and DELETE /passkeys/{passkey_id} to remove a passkey. Introduces PasskeyListItem and PasskeyUpdateParams types, API handlers (PasskeyList, PasskeyUpdate, PasskeyDelete), models.FindWebAuthnCredentialByIDAndUserID, router.Patch support, and a CORS change to allow PATCH. Handlers enforce authentication and ownership, use DB transactions with audit logging, and include comprehensive tests. Sequence Diagram(s)sequenceDiagram
actor Client
participant API as API Handler
participant DB as Database
participant Audit as Audit Logger
Client->>API: PATCH /passkeys/{id} (auth token, body: friendly_name)
API->>API: Validate authentication
API->>API: Parse & validate body
API->>DB: FindWebAuthnCredentialByIDAndUserID(id, userID)
DB-->>API: Credential or NotFound
alt Credential found and owned by user
API->>DB: Begin transaction
API->>DB: Update credential friendly_name
API->>Audit: Log PasskeyUpdatedAction (user, IP)
DB-->>API: Commit
API-->>Client: 200 OK (updated PasskeyListItem)
else Not found or not owned
API-->>Client: 404 Not Found
end
sequenceDiagram
actor Client
participant API as API Handler
participant DB as Database
participant Audit as Audit Logger
Client->>API: DELETE /passkeys/{id} (auth token)
API->>API: Validate authentication
API->>DB: FindWebAuthnCredentialByIDAndUserID(id, userID)
DB-->>API: Credential or NotFound
alt Credential found and owned by user
API->>DB: Begin transaction
API->>DB: Delete credential
API->>Audit: Log PasskeyDeletedAction (user, IP)
DB-->>API: Commit
API-->>Client: 204 No Content
else Not found or not owned
API-->>Client: 404 Not Found
end
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 |
492898e to
bff8a56
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
internal/api/passkey_manage.go (1)
108-145:⚠️ Potential issue | 🔴 CriticalRequire AAL2 before deleting an MFA-backed credential.
This still allows a single-factor session to delete a WebAuthn credential that may be serving as an MFA factor, which creates an MFA-downgrade path. The TODO acknowledges it, but this needs an enforced guard before merge; at minimum, reject delete when the user has MFA enabled and the current session is not AAL2.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/api/passkey_manage.go` around lines 108 - 145, Before deleting a passkey in PasskeyDelete, enforce AAL2 for MFA-protected credentials: after obtaining user := getUser(ctx) and before performing the delete transaction on cred, check the current session's authentication assurance level from the request context and whether the user has MFA enabled or the credential is an MFA factor (inspect cred metadata/flags). If the session is not AAL2 and the credential is an active MFA factor (or the user has MFA enabled and this cred is one of their MFA methods), return a forbidden/validation error (use apierrors.NewForbidden or NewValidationFailed) instead of proceeding; otherwise continue to the existing transactional delete. Ensure the check references PasskeyDelete, getUser, and cred so reviewers can locate it.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@internal/api/passkey_manage.go`:
- Around line 108-145: Before deleting a passkey in PasskeyDelete, enforce AAL2
for MFA-protected credentials: after obtaining user := getUser(ctx) and before
performing the delete transaction on cred, check the current session's
authentication assurance level from the request context and whether the user has
MFA enabled or the credential is an MFA factor (inspect cred metadata/flags). If
the session is not AAL2 and the credential is an active MFA factor (or the user
has MFA enabled and this cred is one of their MFA methods), return a
forbidden/validation error (use apierrors.NewForbidden or NewValidationFailed)
instead of proceeding; otherwise continue to the existing transactional delete.
Ensure the check references PasskeyDelete, getUser, and cred so reviewers can
locate it.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 34871219-76de-4bff-8ad1-dfd80179c51d
📒 Files selected for processing (1)
internal/api/passkey_manage.go
Adds endpoints to allow a user to manage their passkeys.