feat(passkey): support multiple passkeys per user#4922
Closed
v1xingyue wants to merge 1 commit into
Closed
Conversation
Previously, each user could only register one Passkey credential. The database had a unique index on user_id, and the UI only showed a single passkey status. This change removes that limitation. Backend changes: - Remove uniqueIndex on PasskeyCredential.UserID, add plain index - UpsertPasskeyCredential now inserts new credentials instead of replacing the old one (update by credential_id if exists) - Add DeletePasskeyByCredentialID for per-credential removal - GetPasskeyByUserID now returns []*PasskeyCredential - Controller endpoints updated to handle credential lists: * Register: exclusions include all existing credentials * Delete: requires credential_id param * Status: returns list of credentials with metadata * Verify: supports multiple allowCredentials * Login/Verify: find and update the matched credential - Add DB migration to drop old unique index on user_id Frontend (default theme): - PasskeyCard shows a list of registered credentials - Each credential displays device type, last used, created at - Add 'Add Passkey' button to register additional credentials - Delete action now targets a specific credential_id Frontend (classic theme): - AccountManagement shows credential list - PersonalSetting delete API updated to use credential_id path Breaking change: - DELETE /api/user/passkey -> DELETE /api/user/passkey/:credential_id
Contributor
WalkthroughThis PR enables multiple passkeys per user by refactoring database constraints, backend credential handling, and frontend state management. The schema removes the unique constraint on user ID, model methods now return and operate on credential collections, and all passkey endpoints adapt to load full credential lists. Frontend components updated to display and delete individual credentials by credential ID instead of operating on a single passkey. ChangesMulti-Passkey Support
Sequence Diagram(s)sequenceDiagram
participant Frontend as Frontend UI
participant Hook as usePasskeyManagement Hook
participant API as Passkey API
participant Backend as Backend Controller
participant DB as Database (Model)
Frontend->>Hook: remove(credentialId)
Hook->>API: deletePasskey(credentialId)
API->>Backend: DELETE /api/user/passkey/:credentialId
Backend->>DB: DeletePasskeyByCredentialID(credentialId, userId)
DB-->>Backend: success
Backend-->>API: response
API-->>Hook: resolved
Hook->>Hook: refresh status
Hook-->>Frontend: updated credentials list
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
model/passkey.go (1)
5-5: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winUse
common.Marshal/common.Unmarshalinstead of directencoding/jsoncalls.Lines 49 and 68 use
json.Unmarshalandjson.Marshaldirectly. As per coding guidelines, all JSON operations must use wrapper functions fromcommon/json.go.♻️ Proposed fix
import ( "encoding/base64" - "encoding/json" "errors" "fmt"In
TransportList():- if err := json.Unmarshal([]byte(p.Transports), &transports); err != nil { + if err := common.Unmarshal([]byte(p.Transports), &transports); err != nil {In
SetTransports():- encoded, err := json.Marshal(stringList) + encoded, err := common.Marshal(stringList)As per coding guidelines: "All JSON marshal/unmarshal operations MUST use wrapper functions from
common/json.go."Also applies to: 49-49, 68-68
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@model/passkey.go` at line 5, Replace direct encoding/json calls in TransportList() and SetTransports(): where the code currently calls json.Unmarshal and json.Marshal, use the wrapper helpers common.Unmarshal and common.Marshal from common/json.go instead. Update the import to remove "encoding/json" and ensure the functions handle the returned error values the same way (propagate or log) so TransportList() and SetTransports() maintain existing error handling semantics.
🧹 Nitpick comments (1)
web/default/src/features/profile/components/passkey-card.tsx (1)
362-379: 💤 Low valueProps are destructured in the function signature.
The
PasskeyCredentialItemcomponent destructures its props directly in the function signature. Additionally, the typet: (key: string) => stringis overly narrow and doesn't match the actualreact-i18nexttfunction signature which supports interpolation parameters.As per coding guidelines: "Do not destructure component props; use
props.xxxdirectly instead for clarity".Suggested refactor
-function PasskeyCredentialItem({ - credential, - removing, - onRemove, - t, -}: { - credential: { +import type { TFunction } from 'i18next' + +interface PasskeyCredentialItemProps { + credential: { credential_id: string created_at: string last_used_at?: string | null backup_eligible?: boolean backup_state?: boolean attachment?: string } removing: boolean onRemove: () => void - t: (key: string) => string -}) { + t: TFunction +} + +function PasskeyCredentialItem(props: PasskeyCredentialItemProps) { const formattedLastUsed = - credential.last_used_at && !Number.isNaN(Date.parse(credential.last_used_at)) - ? dayjs(credential.last_used_at).fromNow() - : t('Not used yet') + props.credential.last_used_at && !Number.isNaN(Date.parse(props.credential.last_used_at)) + ? dayjs(props.credential.last_used_at).fromNow() + : props.t('Not used yet')🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@web/default/src/features/profile/components/passkey-card.tsx` around lines 362 - 379, PasskeyCredentialItem currently destructures props in the function signature and types the translator as t: (key: string) => string; change the component to accept a single props object (e.g. function PasskeyCredentialItem(props: PasskeyCredentialItemProps) { ... }) and reference props.credential, props.removing, props.onRemove, props.t inside the body; define an interface PasskeyCredentialItemProps that describes credential, removing, onRemove and a wider translator type such as t: (key: string, vars?: Record<string, any>) => string (or the appropriate i18next/TFunction type) to allow interpolation. Ensure all internal usages switch from destructured names to props.xxx and update any callers if necessary.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@model/passkey.go`:
- Line 5: Replace direct encoding/json calls in TransportList() and
SetTransports(): where the code currently calls json.Unmarshal and json.Marshal,
use the wrapper helpers common.Unmarshal and common.Marshal from common/json.go
instead. Update the import to remove "encoding/json" and ensure the functions
handle the returned error values the same way (propagate or log) so
TransportList() and SetTransports() maintain existing error handling semantics.
---
Nitpick comments:
In `@web/default/src/features/profile/components/passkey-card.tsx`:
- Around line 362-379: PasskeyCredentialItem currently destructures props in the
function signature and types the translator as t: (key: string) => string;
change the component to accept a single props object (e.g. function
PasskeyCredentialItem(props: PasskeyCredentialItemProps) { ... }) and reference
props.credential, props.removing, props.onRemove, props.t inside the body;
define an interface PasskeyCredentialItemProps that describes credential,
removing, onRemove and a wider translator type such as t: (key: string, vars?:
Record<string, any>) => string (or the appropriate i18next/TFunction type) to
allow interpolation. Ensure all internal usages switch from destructured names
to props.xxx and update any callers if necessary.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0fdaaa6a-7b2d-4a2b-a470-c5a1f0537e1f
📒 Files selected for processing (12)
controller/passkey.gocontroller/secure_verification.gomodel/main.gomodel/passkey.gorouter/api-router.goservice/passkey/user.goweb/classic/src/components/settings/PersonalSetting.jsxweb/classic/src/components/settings/personal/cards/AccountManagement.jsxweb/default/src/features/auth/passkey/api.tsweb/default/src/features/auth/passkey/hooks/use-passkey-management.tsweb/default/src/features/auth/passkey/types.tsweb/default/src/features/profile/components/passkey-card.tsx
Collaborator
|
暂不考虑允许多passkey |
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
2 participants
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.
Description
Previously, each user could only register one Passkey credential. The database had a unique index on
user_id, and the UI only showed a single passkey status. This PR removes that limitation and allows users to register multiple Passkeys (e.g., one on their phone, one on their laptop, one hardware key).Backend Changes
Database (
model/passkey.go):uniqueIndexonPasskeyCredential.UserID, replaced with plainindexGetPasskeyByUserIDnow returns[]*PasskeyCredentialUpsertPasskeyCredentialnow inserts new credentials instead of replacing the old one (updates bycredential_idif already exists)DeletePasskeyByCredentialIDfor per-credential removalDeletePasskeyByUserIDfor admin resetController (
controller/passkey.go):RegisterBegin: exclusions now include all existing credentialsRegisterFinish: inserts new credential without deleting old onesDelete: changed toDELETE /api/user/passkey/:credential_idfor targeted removalStatus: returns a list of credentials with metadata (created_at, last_used_at, backup info, attachment type)VerifyBegin/VerifyFinish: supports multipleallowCredentialsLoginFinish: finds and updates the matched credential'sLastUsedAtMigration (
model/main.go):migratePasskeyUserIDUniqueIndex()to safely drop the old unique index onuser_idacross SQLite / MySQL / PostgreSQLFrontend Changes
Default Theme:
PasskeyCardnow shows a list of registered credentialscredential_idClassic Theme:
AccountManagementshows credential list with per-item deletePersonalSettingupdated to call newDELETE /api/user/passkey/:credential_idAPIAPI Breaking Change
DELETE /api/user/passkeyDELETE /api/user/passkey/:credential_idChecklist
Related
Closes the limitation discussed in #3490 (Passkey 只能绑定一个设备).
Summary by CodeRabbit