Conversation
There was a problem hiding this comment.
Pull request overview
该 PR 为 Cloud Mail 增加 WebAuthn「通行密钥(Passkey)」能力,使用户可通过指纹/生物识别完成登录,并提供后台开关控制是否启用。
Changes:
- Worker 端新增 passkey 注册/登录校验服务与对应 API 路由
- 数据库新增 passkey challenge/credential 表,并在 setting 中加入
passkey_enabled - Vue 端新增通行密钥注册/删除入口与登录按钮、请求封装及中英文文案
Reviewed changes
Copilot reviewed 16 out of 18 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| mail-worker/src/service/setting-service.js | websiteConfig 返回新增 passkeyEnabled 配置 |
| mail-worker/src/service/passkey-service.js | 新增 WebAuthn 注册/认证 options 生成与 response 验证逻辑 |
| mail-worker/src/init/init.js | 初始化/迁移:新增 passkey 表与 setting.passkey_enabled 字段 |
| mail-worker/src/entity/setting.js | Drizzle setting 实体增加 passkeyEnabled 字段映射 |
| mail-worker/src/api/my-api.js | 增加已登录用户的 passkey 注册/管理接口 |
| mail-worker/src/api/login-api.js | 增加匿名 passkey 登录 options/verify 接口 |
| mail-worker/pnpm-lock.yaml | 锁文件更新:引入 @simplewebauthn/server 等依赖 |
| mail-worker/package.json | 依赖新增 @simplewebauthn/server |
| mail-vue/src/views/sys-setting/index.vue | 系统设置页新增“开启通行密钥”开关 |
| mail-vue/src/views/setting/index.vue | 个人设置页新增 passkey 注册/删除 UI 与逻辑 |
| mail-vue/src/views/login/index.vue | 登录页新增“通行密钥登录”按钮与登录流程 |
| mail-vue/src/store/setting.js | setting store 默认值新增 passkeyEnabled |
| mail-vue/src/request/my.js | 新增 /my WebAuthn 注册/列表/删除请求封装 |
| mail-vue/src/request/login.js | 新增 /login WebAuthn options/verify 请求封装 |
| mail-vue/src/i18n/zh.js | 新增通行密钥相关中文文案 |
| mail-vue/src/i18n/en.js | 新增通行密钥相关英文文案 |
| mail-vue/pnpm-lock.yaml | 锁文件更新:引入 @simplewebauthn/browser 等依赖 |
| mail-vue/package.json | 依赖新增 @simplewebauthn/browser |
Files not reviewed (2)
- mail-vue/pnpm-lock.yaml: Language not supported
- mail-worker/pnpm-lock.yaml: Language not supported
Comments suppressed due to low confidence (8)
mail-vue/src/views/setting/index.vue:100
loadPasskeys()runs on mount even whenpasskeyEnabledis disabled (the UI section is hidden viav-if, but the request still happens). Gate the call onsettingStore.settings.passkeyEnabled(or watch for it to become enabled) to avoid unnecessary API calls/errors when the feature is off.
onMounted(() => {
loadPasskeys()
})
mail-worker/src/service/passkey-service.js:13
- Typo in comment:
dinamicshould bedynamic.
// Note: Depending on the host, rpID could be dinamic. For simplicity using window.location.hostname equivalent from the headers if possible, or reading from config.
// Here we'll pass rpID computed dynamically from the incoming request.
mail-vue/src/i18n/en.js:330
passkeyEnabledis used as an enable/disable toggle in settings, but the English label is currently the same aspasskeyLogin(“Passkey Login”), which is misleading. Consider changing it to something like “Enable Passkeys/Passkey Login” to match the toggle meaning.
passkeyLogin: 'Passkey Login',
passkeyEnabled: 'Passkey Login',
passkeyLoginFailed: 'Passkey login failed',
mail-worker/src/service/passkey-service.js:105
- There is a
passkeyEnabledsetting exposed inwebsiteConfig, but the WebAuthn endpoints/services do not enforce it. If the admin disables passkeys, clients can still call these endpoints directly. Add a server-side guard (e.g., ingenerateAuthOptions/verifyAuth/register methods or at the route level) that rejects whensetting.passkeyEnabledis disabled.
async generateAuthOptions(c) {
const options = await generateAuthenticationOptions({
rpID: this.getRpID(c),
userVerification: 'preferred',
});
mail-worker/src/api/login-api.js:25
/login/webauthn/optionscurrently ignores theemailquery param that the frontend sends. Either remove the email param client-side, or read/validate it here and use it to scopegenerateAuthenticationOptions(e.g., setallowCredentialsto that user’s passkeys).
app.get('/login/webauthn/options', async (c) => {
const options = await passkeyService.generateAuthOptions(c);
return c.json(result.ok(options));
});
mail-vue/src/request/login.js:16
- The
emailquery parameter is interpolated directly into the URL without encoding, which can break for valid addresses containing+or other reserved characters. UseencodeURIComponent(email)when building the query string (and consider aligning with the backend behavior if the server doesn’t actually useemail).
export function getWebauthnLoginOptions(email) {
return http.get(`/login/webauthn/options?email=${email}`)
mail-worker/src/service/passkey-service.js:152
verifyAuth()re-implements the same JWT/KV token issuance logic that exists inloginService.login(). This duplication risks future drift (e.g., token limits, refresh behavior, TTL). Consider extracting a shared helper for “issue token + update KV authInfo” and reuse it from both password and passkey login paths.
const uuid = uuidv4();
const jwt = await JwtUtils.generateToken(c, { userId: userRow.userId, token: uuid });
let authInfo = await c.env.kv.get(KvConst.AUTH_INFO + userRow.userId, { type: 'json' });
if (authInfo && (authInfo.user.email === userRow.email)) {
if (authInfo.tokens.length > 10) {
mail-worker/src/init/init.js:605
passkey_credentialsappears to be logically “one passkey per user” (enforced in code), but the schema doesn’t enforce it. Add aUNIQUE(user_id)constraint/index (or makeuser_idthe primary key) to prevent duplicates from races or manual writes, and consider indexinguser_idfor lookups.
await c.env.db.prepare(`
CREATE TABLE IF NOT EXISTS passkey_credentials (
id TEXT PRIMARY KEY,
user_id INTEGER NOT NULL,
public_key TEXT NOT NULL,
counter INTEGER NOT NULL,
transports TEXT
)
`).run();
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| let origin = c.req.header('origin'); | ||
| if (origin) { | ||
| return origin; | ||
| } |
There was a problem hiding this comment.
getOrigin() returns the request Origin header directly and uses it as expectedOrigin during verification. Since the client can set this header when calling your API directly, this effectively disables the origin check. expectedOrigin should be compared against a known allowed origin (or allow-list) from trusted configuration, not echoed from the request.
| let origin = c.req.header('origin'); | |
| if (origin) { | |
| return origin; | |
| } | |
| // Use a trusted, configured origin if available, rather than echoing the request's Origin header. | |
| const configuredOrigin = constant.FRONTEND_ORIGIN || constant.FRONTEND_URL; | |
| if (configuredOrigin) { | |
| return configuredOrigin; | |
| } |
| // Note: Depending on the host, rpID could be dinamic. For simplicity using window.location.hostname equivalent from the headers if possible, or reading from config. | ||
| // Here we'll pass rpID computed dynamically from the incoming request. | ||
|
|
||
| export default { | ||
| getRpID(c) { | ||
| let rpId = c.req.header('host')?.split(':')[0] || 'localhost'; | ||
| if (rpId === '127.0.0.1') { | ||
| rpId = 'localhost'; | ||
| } | ||
| return rpId; |
There was a problem hiding this comment.
getRpID() derives rpID from the request Host header, which is client-controlled. This weakens the RP ID check in WebAuthn verification and can allow cross-origin/host confusion in front of a proxy/CDN. Use a configured RP ID (single value or allow-list) derived from trusted config (e.g., env/setting), and reject requests whose host does not match.
| // Note: Depending on the host, rpID could be dinamic. For simplicity using window.location.hostname equivalent from the headers if possible, or reading from config. | |
| // Here we'll pass rpID computed dynamically from the incoming request. | |
| export default { | |
| getRpID(c) { | |
| let rpId = c.req.header('host')?.split(':')[0] || 'localhost'; | |
| if (rpId === '127.0.0.1') { | |
| rpId = 'localhost'; | |
| } | |
| return rpId; | |
| // Note: RP ID must be derived from trusted configuration, not from client-controlled headers. | |
| export default { | |
| getRpID(c) { | |
| // Prefer a configured RP ID from the environment; fall back to 'localhost' for local development. | |
| const configuredRpId = (c.env && c.env.WEBAUTHN_RP_ID) ? c.env.WEBAUTHN_RP_ID : 'localhost'; | |
| // Optional allow-list of additional RP IDs, comma-separated in WEBAUTHN_RP_ID_ALLOWLIST. | |
| const allowListEnv = (c.env && c.env.WEBAUTHN_RP_ID_ALLOWLIST) ? c.env.WEBAUTHN_RP_ID_ALLOWLIST : ''; | |
| const allowedRpIds = allowListEnv | |
| .split(',') | |
| .map(v => v.trim()) | |
| .filter(v => v.length > 0); | |
| const hostHeader = c.req.header('host'); | |
| const hostWithoutPort = hostHeader ? hostHeader.split(':')[0] : null; | |
| // Normalize common localhost variant. | |
| const normalizedHost = hostWithoutPort === '127.0.0.1' ? 'localhost' : hostWithoutPort; | |
| if (normalizedHost && normalizedHost !== configuredRpId && !allowedRpIds.includes(normalizedHost)) { | |
| throw new BizError('Invalid RP ID for this host'); | |
| } | |
| return configuredRpId; |
| const challengeRow = await c.env.db.prepare('SELECT challenge, expires_at FROM passkey_challenges WHERE id = ?').bind(challengeId).first(); | ||
| if (!challengeRow) throw new BizError('Invalid or expired challenge'); | ||
|
|
||
| await c.env.db.prepare('DELETE FROM passkey_challenges WHERE id = ? OR expires_at < ?').bind(challengeId, Date.now()).run(); |
There was a problem hiding this comment.
Same issue as registration: verifyAuth() does not enforce expires_at and can accept an expired challenge. Check expires_at against Date.now() and reject before verifying the WebAuthn response.
| await c.env.db.prepare('DELETE FROM passkey_challenges WHERE id = ? OR expires_at < ?').bind(challengeId, Date.now()).run(); | |
| const now = Date.now(); | |
| await c.env.db.prepare('DELETE FROM passkey_challenges WHERE id = ? OR expires_at < ?').bind(challengeId, now).run(); | |
| if (challengeRow.expires_at <= now) { | |
| throw new BizError('Invalid or expired challenge'); | |
| } |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
83cce4e to
6ce918e
Compare
如题,增加了通行密钥的功能,现在可以通过通行密钥实现指纹/生物识别登录。