feat(auth)!: migrate WebAuthn to Passkey.#2161
feat(auth)!: migrate WebAuthn to Passkey.#2161HappyDIY wants to merge 1 commit intoOpenListTeam:mainfrom
Conversation
… management - Migrate traditional WebAuthn to Passkey to support cloud synchronization and username-less authentication - Rename all backend WebAuthn APIs to Passkey for improved standardization - Enable Passkey by default to align with current industry adoption trends - Display user device IP and User-Agent (UA) in Passkey management page for better device identification - Add upgrade flow to migrate legacy WebAuthn credentials to new Passkey format - Prevent creation of new Passkeys until legacy credentials are upgraded or removed to avoid post-upgrade incompatibility
|
For Reviewers: |
There was a problem hiding this comment.
Pull request overview
This PR performs a breaking migration from legacy WebAuthn naming/behavior to a Passkey-first flow, including API renames, credential metadata tracking, and legacy-credential upgrade constraints.
Changes:
- Renamed
/authn/webauthn_*endpoints to/authn/passkey_*and added a legacy-status endpoint. - Updated authn credential persistence to a record-based JSON format (resident-key + creator IP/UA) with legacy fallback parsing.
- Enforced “legacy credential must be upgraded/deleted before creating a new passkey” and required resident keys for new registrations.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
server/router.go |
Renames WebAuthn routes to Passkey routes and adds passkey_legacy_status. |
server/handles/webauthn.go |
Updates login/registration logic for discoverable credentials, legacy handling, and stores creator IP/UA. |
internal/model/user.go |
Introduces WebAuthnCredentialRecord format and legacy parsing fallback helpers. |
internal/db/user.go |
Writes/reads new authn record format; adds legacy-credential detection helper. |
internal/conf/const.go |
Removes the WebauthnLoginEnabled setting key. |
internal/bootstrap/patch/v3_32_0/update_authn.go |
Updates a historical comment associated with authn patching. |
internal/bootstrap/data/setting.go |
Removes initialization of the WebauthnLoginEnabled setting item. |
drivers/cloudreve_v4/util.go |
Updates a driver error message from “webauthn” to “passkey”. |
Comments suppressed due to low confidence (1)
server/handles/webauthn.go:168
- BeginAuthnRegistration also calls common.ErrorResp on BeginRegistration/json.Marshal failures but does not return, which can lead to sending a success response with nil/partial data or further panics. Return immediately after each error response (and ensure options/sessionData are not used when err != nil).
options, sessionData, err := authnInstance.BeginRegistration(
user,
webauthn.WithResidentKeyRequirement(protocol.ResidentKeyRequirementRequired),
)
if err != nil {
common.ErrorResp(c, err, 400)
}
val, err := json.Marshal(sessionData)
if err != nil {
common.ErrorResp(c, err, 400)
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| authnInstance, err := authn.NewAuthnInstance(c) | ||
| if err != nil { | ||
| common.ErrorResp(c, err, 400) | ||
| } |
There was a problem hiding this comment.
BeginAuthnRegistration calls common.ErrorResp when authn.NewAuthnInstance fails but does not return, so execution continues with a nil/invalid authnInstance and can panic. Add an early return after sending the error response.
| var credentials []webauthn.Credential | ||
| err = json.Unmarshal([]byte(u.Authn), &credentials) | ||
| if err != nil { | ||
| fmt.Println(err) |
There was a problem hiding this comment.
Avoid fmt.Println(err) inside WebAuthnCredentialRecords; it writes to stdout and can leak/produce noisy logs in production. Use the project's logger (e.g., utils.Log) at an appropriate level, or simply return an empty slice when parsing fails (possibly after metrics/logging at a higher layer).
| fmt.Println(err) |
|
|
||
| // UpdateAuthnForOldVersion updates users' authn | ||
| // First published: bdfc159 fix: webauthn logspam (#6181) by itsHenry | ||
| // First published: bdfc159 fix: passkey logspam (#6181) by itsHenry |
There was a problem hiding this comment.
This comment references a specific historical commit hash (bdfc159). Renaming the original commit message from "webauthn" to "passkey" can make it harder to trace the exact upstream change. Consider keeping the original message or mentioning both terms (e.g., "webauthn/passkey") for accuracy.
| // First published: bdfc159 fix: passkey logspam (#6181) by itsHenry | |
| // First published: bdfc159 fix: webauthn/passkey logspam (#6181) by itsHenry |
| } else { // client-side discoverable login | ||
| requireUsername, err = db.HasLegacyAuthnCredentials() | ||
| if err == nil && !requireUsername { | ||
| options, sessionData, err = authnInstance.BeginDiscoverableLogin() | ||
| } |
There was a problem hiding this comment.
BeginAuthnLogin calls db.HasLegacyAuthnCredentials() on the unauthenticated login path when username is empty. That function loads and parses authn data for all users, which is expensive and can become a DoS vector under login traffic. Consider replacing this with a cached flag/TTL cache, a cheaper query, or moving the legacy check to a user-scoped flow rather than a full-table scan per request.
| func LegacyAuthnStatus(c *gin.Context) { | ||
| hasLegacy, err := db.HasLegacyAuthnCredentials() | ||
| if err != nil { | ||
| common.ErrorResp(c, err, 400) | ||
| return | ||
| } | ||
| common.SuccessResp(c, gin.H{"has_legacy": hasLegacy}) |
There was a problem hiding this comment.
LegacyAuthnStatus is exposed without auth and triggers db.HasLegacyAuthnCredentials(), which currently scans all users and parses authn JSON. This can be very expensive if polled by the UI; consider caching the result, rate-limiting, or restricting the endpoint to authenticated/admin users if possible.
| var users []model.User | ||
| err := db.Select("id", "authn").Where("authn <> '' AND authn <> '[]'").Find(&users).Error | ||
| if err != nil { | ||
| return false, err | ||
| } | ||
| for i := range users { | ||
| if users[i].HasLegacyWebAuthnCredential() { | ||
| return true, nil | ||
| } | ||
| } |
There was a problem hiding this comment.
HasLegacyAuthnCredentials loads every user with a non-empty authn field into memory and JSON-parses each record. This is O(N) over the user table and will be called during passkey login/status, so it can become a serious performance bottleneck. Consider a cached result, incremental tracking (e.g., maintain a counter/flag on update), or iterating rows/limiting to the first legacy match instead of loading all users.
| var users []model.User | |
| err := db.Select("id", "authn").Where("authn <> '' AND authn <> '[]'").Find(&users).Error | |
| if err != nil { | |
| return false, err | |
| } | |
| for i := range users { | |
| if users[i].HasLegacyWebAuthnCredential() { | |
| return true, nil | |
| } | |
| } | |
| rows, err := db.Model(&model.User{}). | |
| Select("id", "authn"). | |
| Where("authn <> '' AND authn <> '[]'"). | |
| Rows() | |
| if err != nil { | |
| return false, err | |
| } | |
| defer rows.Close() | |
| for rows.Next() { | |
| var u model.User | |
| if err := db.ScanRows(rows, &u); err != nil { | |
| return false, err | |
| } | |
| if u.HasLegacyWebAuthnCredential() { | |
| return true, nil | |
| } | |
| } | |
| if err := rows.Err(); err != nil { | |
| return false, err | |
| } |
| func (u *User) WebAuthnCredentialRecords() []WebAuthnCredentialRecord { | ||
| var records []WebAuthnCredentialRecord | ||
| err := json.Unmarshal([]byte(u.Authn), &records) | ||
| if err == nil { | ||
| recordParsed := false | ||
| for i := range records { | ||
| if len(records[i].Credential.ID) > 0 { | ||
| recordParsed = true | ||
| if records[i].ResidentKey == "" { | ||
| records[i].ResidentKey = string(protocol.ResidentKeyRequirementDiscouraged) | ||
| } | ||
| } | ||
| } | ||
| if recordParsed || u.Authn == "[]" || u.Authn == "" { | ||
| return records | ||
| } | ||
| } |
There was a problem hiding this comment.
WebAuthnCredentialRecords attempts to json.Unmarshal(u.Authn) even when Authn is an empty string; this falls through to the legacy unmarshal and returns nil after printing an error. Treat empty Authn the same as "[]" (return an empty slice) so callers don't have to handle nil and to avoid noisy parse errors during upgrades.
| credentials := user.WebAuthnCredentials() | ||
| res := make([]WebAuthnCredentials, 0, len(credentials)) | ||
| records := user.WebAuthnCredentialRecords() | ||
| res := make([]PasskeyCredentials, 0, len(credentials)) | ||
| for _, v := range credentials { | ||
| credential := WebAuthnCredentials{ | ||
| var creatorIP string | ||
| var creatorUA string | ||
| var isLegacy bool | ||
| for i := range records { | ||
| if string(records[i].Credential.ID) == string(v.ID) { | ||
| creatorIP = records[i].CreatorIP | ||
| creatorUA = records[i].CreatorUA | ||
| isLegacy = records[i].ResidentKey == string(protocol.ResidentKeyRequirementDiscouraged) | ||
| break | ||
| } |
There was a problem hiding this comment.
GetAuthnCredentials builds passkey metadata by doing an inner loop over records for every credential and compares IDs via string conversions. This is O(n^2) and allocates; consider building a map from credential ID to record (and use bytes.Equal/base64 keying) to simplify and speed up the lookup.
|
@copilot open a new pull request to apply changes based on the comments in this thread |
feat(passkey)!: 迁移 WebAuthn 至 Passkey 并默认启用，统一后端 API 命名
… management
Description / 描述
本次 PR 对现有 WebAuthn 认证体系进行整体升级，统一迁移至 Passkey 方案，并对相关接口与管理页面进行增强。
主要包括：
认证体系升级
接口命名标准化
webauthn相关 API 重命名为passkey默认策略调整
管理页面增强
旧凭证升级机制
Motivation and Context / 背景
随着主流科技公司全面推广 Passkey 体系，传统 WebAuthn 命名与使用方式已逐步过时。
本次升级解决以下问题：
How Has This Been Tested? / 测试
Checklist / 检查清单
我已阅读 CONTRIBUTING 文档。
go fmtor prettier.我已使用
go fmt或 prettier 格式化提交的代码。我已为此 PR 添加了适当的标签（如无权限或需要的标签不存在，请在描述中说明，管理员将后续处理）。
我已在适当情况下使用"Request review"功能请求相关代码作者进行审查。
我已相应更新了相关仓库（若适用）。