add WebAuthn passkey support for passwordless login#10
Conversation
Adds full passkey (WebAuthn) flow as a passwordless login alternative. Users can register passkeys in Settings and use them to sign in directly, bypassing TOTP since passkeys are inherently multi-factor. Backend: migration, model, WebAuthn service with challenge storage, 6 new API endpoints, py_webauthn dependency. Frontend: @simplewebauthn/browser, passkey login button on Login page, passkey management (register/list/delete) on Settings page. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Caution Review failedThe pull request is closed. ℹ️ Recent review infoConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughAdds WebAuthn/passkey support across backend and frontend: new PasskeyCredential and WebAuthnChallenge models and migrations, WebAuthnService, auth API endpoints for register/authenticate/list/delete, config and dependency updates, and frontend UI + flows for passkey registration, login, and management. Changes
Sequence DiagramssequenceDiagram
actor User
participant Browser as Browser/Frontend
participant API as Backend API
participant Service as WebAuthnService
participant DB as Database
User->>Browser: Click "Register Passkey"
Browser->>API: POST /auth/passkeys/register/options
API->>Service: generate_registration_options_for_user(user)
Service->>DB: Query existing credentials
Service->>DB: Create WebAuthnChallenge (key=reg:<user_id>) with TTL
Service-->>API: Return registration options JSON
API-->>Browser: Return options
Browser->>Browser: startRegistration() (authenticator)
Browser->>API: POST /auth/passkeys/register/verify (credential + name)
API->>Service: verify_registration(user, credential_json, name)
Service->>DB: Pop & validate challenge
Service->>DB: Create PasskeyCredential record
Service-->>API: Return PasskeyResponse
API-->>Browser: Return passkey details
sequenceDiagram
actor User
participant Browser as Browser/Frontend
participant API as Backend API
participant Service as WebAuthnService
participant DB as Database
User->>Browser: Click "Sign in with passkey"
Browser->>API: POST /auth/passkeys/authenticate/options
API->>Service: generate_authentication_options_for_login()
Service->>DB: Create WebAuthnChallenge (key=auth:<challenge_id>) with TTL
Service-->>API: Return options JSON + challenge_id
API-->>Browser: Return options + challenge_id
Browser->>Browser: startAuthentication() (authenticator)
Browser->>API: POST /auth/passkeys/authenticate/verify (credential + challenge_id)
API->>Service: verify_authentication(credential_json, challenge_id)
Service->>DB: Pop & validate challenge
Service->>DB: Find PasskeyCredential by credential_id
Service->>DB: Update sign_count and last_used_at
Service-->>API: Return authenticated User / tokens
API-->>Browser: Return LoginResponse (tokens)
Browser->>Browser: Store tokens and refresh user data
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (4)
backend/app/services/webauthn.py (2)
6-20: Unused import:PublicKeyCredentialCreationOptions.
PublicKeyCredentialCreationOptions(line 17) is imported but never referenced in this file.Proposed fix
from webauthn.helpers.structs import ( PublicKeyCredentialDescriptor, AuthenticatorTransport, UserVerificationRequirement, - PublicKeyCredentialCreationOptions, AuthenticatorSelectionCriteria, ResidentKeyRequirement, )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/services/webauthn.py` around lines 6 - 20, Remove the unused import PublicKeyCredentialCreationOptions from the webauthn.helpers.structs import list in this module; locate the import block that currently brings in PublicKeyCredentialDescriptor, AuthenticatorTransport, UserVerificationRequirement, PublicKeyCredentialCreationOptions, AuthenticatorSelectionCriteria, ResidentKeyRequirement and delete only PublicKeyCredentialCreationOptions so the remaining symbols (e.g., PublicKeyCredentialDescriptor, AuthenticatorTransport, UserVerificationRequirement, AuthenticatorSelectionCriteria, ResidentKeyRequirement) remain imported.
104-113:if verification.credential_device_typeis always truthy after successful verification.
credential_device_typeis always a non-empty string ("single_device"or"multi_device") on a successfulVerifiedRegistration. This condition never gates anything. If the intent was to conditionally extract transports, the innerparsed.get("response", {}).get("transports")check already handles that. Consider removing the outerifto avoid confusion.Proposed simplification
transports_json = None - if verification.credential_device_type: - # Store transports from the credential response if available - try: - parsed = json.loads(credential_json) - raw_transports = parsed.get("response", {}).get("transports") - if raw_transports: - transports_json = json.dumps(raw_transports) - except (json.JSONDecodeError, AttributeError): - pass + # Store transports from the credential response if available + try: + parsed = json.loads(credential_json) + raw_transports = parsed.get("response", {}).get("transports") + if raw_transports: + transports_json = json.dumps(raw_transports) + except (json.JSONDecodeError, AttributeError): + pass🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/services/webauthn.py` around lines 104 - 113, The outer conditional using verification.credential_device_type is redundant because credential_device_type is always a non-empty string after a successful VerifiedRegistration; remove the if guard and always attempt to parse credential_json to extract transports into transports_json (keep the existing json.loads(credential_json), the parsed.get("response", {}).get("transports") check, and the exception handling for json.JSONDecodeError/AttributeError) so that the logic centers on presence of response.transports rather than credential_device_type.backend/app/config.py (1)
23-26: Ensure production overrides for WebAuthn settings.The defaults target local development. In production,
webauthn_rp_idmust match the serving domain andwebauthn_originmust match the exact origin (scheme + host + port) of the frontend, or all WebAuthn ceremonies will fail. This mirrors the existingsecret_keypattern but is worth calling out since a misconfigured RP ID silently breaks passkey registration/authentication with no recovery path.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/config.py` around lines 23 - 26, The WebAuthn defaults (webauthn_rp_id, webauthn_rp_name, webauthn_origin) are currently hardcoded for localhost; change them to be overridable in production the same way as secret_key by reading from environment/config so the RP ID equals the serving domain and webauthn_origin equals the exact frontend origin (scheme+host(+port)). Update the Config initialization that defines webauthn_rp_id, webauthn_rp_name, and webauthn_origin to prefer environment variables (or a provided production config) and fall back to the existing localhost defaults, and ensure deployment docs/ENV examples include the correct values for production.backend/app/api/auth.py (1)
227-231: Consider rate-limiting the unauthenticated passkey endpoints.
passkey_authenticate_optionsandpasskey_authenticate_verifyare open to unauthenticated callers. Without rate limiting, an attacker could flood the in-memory challenge store or brute-force assertions. This is typically handled at the reverse-proxy/middleware layer, but worth flagging.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/api/auth.py` around lines 227 - 231, The unauthenticated endpoints passkey_authenticate_options and passkey_authenticate_verify can be abused; add rate-limiting to them by applying the project's existing rate-limiter middleware/decorator (or add one if missing) to the router.post handlers for passkey_authenticate_options and passkey_authenticate_verify; implement a conservative limit (e.g., X requests per minute per IP or per client token) that returns a 429 on exceed, ensure the limiter keys by remote IP (or IP+endpoint) and is checked before calling WebAuthnService.generate_authentication_options_for_login or the corresponding verify function, and include brief logging for exceeded limits to aid monitoring.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/app/api/auth.py`:
- Around line 211-217: The current except clause in passkey_register_verify (and
similarly in passkey_authenticate_verify) catches a bare Exception which masks
unexpected errors and leaks internal messages; change the handler to only catch
the expected ValueError (i.e., use "except ValueError as e") and re-raise the
HTTPException using exception chaining (raise HTTPException(status_code=...,
detail=str(e)) from e) so unexpected exceptions (DB/serialization bugs)
propagate as 500s instead of being returned to the client; update both places
where WebAuthnService.verify_registration and the corresponding
verify_authentication call are wrapped.
- Around line 237-243: The catch-all Exception in the WebAuthn authentication
block leaks internal errors; change the try/except around
WebAuthnService.verify_authentication(request.credential, request.challenge_id)
to only catch ValueError and raise HTTPException(status.HTTP_401_UNAUTHORIZED,
detail=str(e)) for that case, and for any other unexpected Exception re-raise or
convert to a generic HTTPException with a non-sensitive message like
"Authentication failed" (do not include str(e)); ensure you update the except
clauses so only ValueError exposes the original message and other exceptions
return the generic message.
- Around line 253-264: The delete_passkey endpoint currently accepts passkey_id
as str which lets Tortoise raise on invalid UUIDs; change passkey_id's type to
UUID in the delete_passkey function signature (so FastAPI validates and returns
422 for malformed IDs) or explicitly validate/convert passkey_id to uuid.UUID
before calling PasskeyCredential.filter (catch ValueError and raise
HTTPException(status_code=422, detail="Invalid passkey_id")); update references
to passkey_id in the function body and keep the existing
PasskeyCredential.filter(id=passkey_id, user=user) logic.
In `@backend/app/models/__init__.py`:
- Line 9: The __all__ list is not alphabetically sorted (RUF022); update the
__all__ variable in models/__init__.py so the exported names ("User", "Team",
"TeamMembership", "TeamRole", "ApiKey", "Log", "LogLevel", "RecoveryCode",
"TotpAttempt", "PasskeyCredential") are in sorted order (case-sensitive Python
string order) while keeping the same symbols and semantics; edit the __all__
assignment to a sorted list so the linter passes.
In `@backend/app/services/webauthn.py`:
- Around line 28-54: The current in-memory challenge store (_challenges) used by
_store_challenge, _pop_challenge and _cleanup_expired with _CHALLENGE_TTL will
not work across multiple workers/processes; replace it with a shared persistence
layer (e.g., Redis or the existing DB) so challenges survive cross-worker
verification. Implement a small adapter module (e.g., webauthn_store) that
exposes the same functions (_store_challenge(key, challenge),
_pop_challenge(key), and optional cleanup) but uses Redis (SET with TTL and
GET/DEL or Lua GETDEL) or a DB table with expiry checks instead of the dict;
update callers to import these functions and remove reliance on the module-level
_challenges dict and _cleanup_expired logic. Ensure TTL uses _CHALLENGE_TTL and
that pop is atomic (use Redis GETDEL or transaction) to avoid race conditions.
In `@backend/migrations/007_passkey_credentials_table.sql`:
- Around line 8-9: The gen_random_uuid() dependency (pgcrypto) must be created
in an earlier migration that first uses it (migration 004) rather than in
migration 007; update migration 004 to include "CREATE EXTENSION IF NOT EXISTS
pgcrypto;" at its top (or in the very first foundational migration) so
gen_random_uuid() is available when migration 004 runs, and remove any pgcrypto
guard or creation from migration 007 (leave id UUID PRIMARY KEY DEFAULT
gen_random_uuid() unchanged in the Passkey credentials table creation).
---
Nitpick comments:
In `@backend/app/api/auth.py`:
- Around line 227-231: The unauthenticated endpoints
passkey_authenticate_options and passkey_authenticate_verify can be abused; add
rate-limiting to them by applying the project's existing rate-limiter
middleware/decorator (or add one if missing) to the router.post handlers for
passkey_authenticate_options and passkey_authenticate_verify; implement a
conservative limit (e.g., X requests per minute per IP or per client token) that
returns a 429 on exceed, ensure the limiter keys by remote IP (or IP+endpoint)
and is checked before calling
WebAuthnService.generate_authentication_options_for_login or the corresponding
verify function, and include brief logging for exceeded limits to aid
monitoring.
In `@backend/app/config.py`:
- Around line 23-26: The WebAuthn defaults (webauthn_rp_id, webauthn_rp_name,
webauthn_origin) are currently hardcoded for localhost; change them to be
overridable in production the same way as secret_key by reading from
environment/config so the RP ID equals the serving domain and webauthn_origin
equals the exact frontend origin (scheme+host(+port)). Update the Config
initialization that defines webauthn_rp_id, webauthn_rp_name, and
webauthn_origin to prefer environment variables (or a provided production
config) and fall back to the existing localhost defaults, and ensure deployment
docs/ENV examples include the correct values for production.
In `@backend/app/services/webauthn.py`:
- Around line 6-20: Remove the unused import PublicKeyCredentialCreationOptions
from the webauthn.helpers.structs import list in this module; locate the import
block that currently brings in PublicKeyCredentialDescriptor,
AuthenticatorTransport, UserVerificationRequirement,
PublicKeyCredentialCreationOptions, AuthenticatorSelectionCriteria,
ResidentKeyRequirement and delete only PublicKeyCredentialCreationOptions so the
remaining symbols (e.g., PublicKeyCredentialDescriptor, AuthenticatorTransport,
UserVerificationRequirement, AuthenticatorSelectionCriteria,
ResidentKeyRequirement) remain imported.
- Around line 104-113: The outer conditional using
verification.credential_device_type is redundant because credential_device_type
is always a non-empty string after a successful VerifiedRegistration; remove the
if guard and always attempt to parse credential_json to extract transports into
transports_json (keep the existing json.loads(credential_json), the
parsed.get("response", {}).get("transports") check, and the exception handling
for json.JSONDecodeError/AttributeError) so that the logic centers on presence
of response.transports rather than credential_device_type.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
frontend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (15)
backend/app/api/auth.pybackend/app/config.pybackend/app/models/__init__.pybackend/app/models/passkey_credential.pybackend/app/models/user.pybackend/app/schemas/__init__.pybackend/app/schemas/auth.pybackend/app/services/webauthn.pybackend/migrations/007_passkey_credentials_table.sqlbackend/requirements.txtfrontend/package.jsonfrontend/src/api/client.tsfrontend/src/stores/auth.tsfrontend/src/views/Login.vuefrontend/src/views/Settings.vue
The PyPI package is 'webauthn', not 'py_webauthn'. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
backend/requirements.txt (1)
13-13: Consider tightening the version constraint for better dependency stability.The
webauthnpackage name is correct and resolves on PyPI. The latest stable release is 2.7.1 (Feb 11, 2026), with multiple releases published since2.1.0. The library has maintained backward compatibility across the range—all changes from 2.1.0 through 2.7.1 are additive (new optional parameters, new helper functions) or environmental (Python 3.9+ requirement in 2.3.0), with no breaking changes to the public API.That said, an open-ended
>=2.1.0constraint allows pip to resolve any future version, including a potential3.xwith different API design. For clarity and to avoid future surprises, consider adding a compatible upper bound:📦 Suggested constraint tightening
-webauthn>=2.1.0 +webauthn>=2.1.0,<3.0.0🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/requirements.txt` at line 13, The requirements entry for the webauthn dependency currently uses an open-ended constraint ("webauthn>=2.1.0"); tighten it to prevent accidental upgrades to a future major (potentially breaking) release by changing that line to include an upper-bound, e.g. update "webauthn>=2.1.0" to "webauthn>=2.1.0,<3.0.0" (or pin to a specific tested release like "webauthn==2.7.1") so the project stays on the 2.x compatible range.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@backend/requirements.txt`:
- Line 13: The requirements entry for the webauthn dependency currently uses an
open-ended constraint ("webauthn>=2.1.0"); tighten it to prevent accidental
upgrades to a future major (potentially breaking) release by changing that line
to include an upper-bound, e.g. update "webauthn>=2.1.0" to
"webauthn>=2.1.0,<3.0.0" (or pin to a specific tested release like
"webauthn==2.7.1") so the project stays on the 2.x compatible range.
The in-memory dict breaks with multiple workers since a challenge stored in one process can't be retrieved by another. Store challenges in a new webauthn_challenges table instead, matching the pattern used by totp_attempts. Also narrow except clauses to ValueError only so unexpected errors propagate as 500s. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Adds full passkey (WebAuthn) flow as a passwordless login alternative. Users can register passkeys in Settings and use them to sign in directly, bypassing TOTP since passkeys are inherently multi-factor.
Backend: migration, model, WebAuthn service with challenge storage, 6 new API endpoints, py_webauthn dependency.
Frontend: @simplewebauthn/browser, passkey login button on Login page, passkey management (register/list/delete) on Settings page.
Summary by CodeRabbit
New Features
Bug Fixes