Repository navigation
feat: implement WebAuthn/Passkey authentication - #680
2witstudios wants to merge 17 commits into
Conversation
… and API routes Introduces a complete integration framework enabling AI agents to use third-party service tools (e.g., Google Calendar) via OAuth connections with PKCE, signed state, and credential encryption. Includes provider/connection/grant/audit repositories, OpenAPI spec importer, AI SDK tool converter, and comprehensive test coverage (280 tests). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ive scoping) - Pass connectionId/toolName from saga to audit log instead of empty strings - Make audit log driveId nullable to avoid FK violation in dashboard context - Handle driveId in OAuth callback to create drive-scoped connections - Look up userDriveRole in global assistant instead of passing null - Add provider drive-scope check on drive connection creation Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
# Conflicts: # apps/web/src/app/api/ai/global/[id]/messages/route.ts # packages/db/drizzle/meta/0071_snapshot.json # packages/db/drizzle/meta/_journal.json
…r merge Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ive scoping)
- P1: Pass null agentId for global assistant audit entries instead of
invalid 'global-assistant' FK (integration-tool-resolver, types, ai-sdk,
rate-limiter)
- P1: Gate drive integration resolution on isMember before resolving
drive-scoped tools (global assistant messages route)
- P1: Validate connection ownership (userId or driveId membership) before
creating agent integration grants
- P2: Use ParameterRef { $param } for OpenAPI query params instead of
literal string interpolation that resolveValue treats as static
- P2: Emit bodyTemplate for non-object request bodies so buildHttpRequest
sends array/primitive JSON payloads
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Security:
- Validate returnUrl as relative path in OAuth schemas and callback redirect
- Validate clientId presence before building OAuth URLs (drive + user routes)
- Normalize provider slug for env var names (replace non-alphanumeric with _)
- Guard against empty update body in user connection PATCH
- Load connection with provider relation in drive connection GET
Robustness:
- Wrap logAudit in catch-block try-catch to prevent masking original errors
- Fix TOCTOU race in config-repository with onConflictDoNothing upsert
- Detect body/param name collisions in OpenAPI converter, fallback to wrapped body
- Add audit logging for drive connection deletion
Code quality:
- Replace CommonJS require('crypto') with ESM import in oauth-state test
- Rename parseIntegrationToolName return field to connectionShortId for clarity
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
# Conflicts: # packages/db/drizzle/meta/0073_snapshot.json # packages/db/drizzle/meta/_journal.json
# Conflicts: # packages/db/drizzle/meta/0083_snapshot.json # packages/db/drizzle/meta/_journal.json
Add complete magic link authentication system for frictionless passwordless login:
**Core Service (packages/lib/src/auth/magic-link-service.ts)**
- createMagicLinkToken(): Generates ps_magic_* tokens with 5-minute expiry
- verifyMagicLinkToken(): Validates tokens with timing-safe hash comparison
- Zero-trust Result pattern ({ ok: true/false, data/error })
- Handles both existing users and new signups
- Colocated TDD test file with Riteway assertions
**API Routes**
- POST /api/auth/magic-link/send: Request magic link with CSRF validation
- GET /api/auth/magic-link/verify: Verify token and create session
- Rate limiting (3 requests per 15 minutes per email)
- Session fixation prevention (revokes existing sessions)
- Anti-enumeration (same response for all emails)
**Frontend**
- MagicLinkForm component with email input and confirmation state
- Sign-in page updated with magic link as primary option
- Support for switching to password login
- Error handling for expired/used/invalid links
**Email Template**
- MagicLinkEmail React Email template
- CTA button with plaintext URL fallback
- 5-minute expiry warning and security note
Closes #571, #572
Epic: #587
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The verifyMagicLinkToken function determines isNewUser by checking if emailVerified is null. Test was failing because the test user was created without emailVerified set. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Security fixes: - Fix TOCTOU race in token verification with atomic UPDATE WHERE usedAt IS NULL - Handle concurrent user creation with unique constraint catch and re-query - Add Secure flag to CSRF cookie in production - Mask email addresses in logs to prevent PII exposure Code quality: - Fix test cleanup to track dynamically created users - Remove unused 'spacing' import from MagicLinkEmail - Add type annotations to .json() calls in MagicLinkForm - Extract SocialSignInButtons component to reduce duplication - Improve resend link UX with auto-submit Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Fix Zod v4 email schema syntax: use object-based message parameter - Handle malformed JSON with early try/catch returning 400 - Replace fragile document.querySelector with stable useRef in MagicLinkForm Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Adds passwordless authentication via WebAuthn passkeys (TouchID, FaceID, Windows Hello, security keys). This is the second phase of the passwordless authentication initiative, building on the magic link implementation. Features: - Passkey registration flow with proper WebAuthn ceremony - Passkey authentication flow for passwordless login - PasskeyManager component in account settings - PasskeyLoginButton component for sign-in page - Counter replay protection for security - Session fixation prevention (revoke existing sessions) - Rate limiting on registration and authentication - Conditional UI support for browser autofill Technical implementation: - Uses @simplewebauthn/server and @simplewebauthn/browser - Zero-trust Result pattern matching existing auth patterns - Challenge stored as hash with 5-minute expiry - CSRF protection on all endpoints - Atomic counter updates for replay detection Database: - New passkeys table with proper indexes - Stores credential ID, public key, counter, device type - Tracks backup status and last used timestamp API Routes: - POST /api/auth/passkey/register/options - Get registration options - POST /api/auth/passkey/register - Verify and store passkey - POST /api/auth/passkey/authenticate/options - Get auth options - POST /api/auth/passkey/authenticate - Verify and create session - GET /api/auth/passkey - List user passkeys - PATCH/DELETE /api/auth/passkey/[id] - Manage passkeys Related: #586, #568, #569, #570 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
📝 WalkthroughWalkthroughAdds end-to-end WebAuthn (passkey) support: DB schema and migrations, server passkey service with generation/verification and management, API routes, client components/hooks, rate-limits, monitoring events, and dependency bumps for simplewebauthn. Changes
Sequence DiagramssequenceDiagram
actor User
participant Browser as Client (Browser)
participant WebAPI as /api/auth/passkey/register
participant Service as Passkey Service
participant DB as Database
participant WebAuthn as WebAuthn Server
User->>Browser: Navigate to Register Passkey
Browser->>WebAPI: POST /api/auth/passkey/register/options
WebAPI->>Service: generateRegistrationOptions(userId)
Service->>DB: Check user & passkey count
Service->>WebAuthn: Generate registration options
WebAuthn-->>Service: Challenge & options
Service->>DB: Store hashed challenge with expiry
Service-->>WebAPI: options, challengeId
WebAPI-->>Browser: Registration options
Browser->>Browser: navigator.credentials.create()
User->>Browser: Complete biometric/platform auth
Browser->>WebAPI: POST /api/auth/passkey/register (response, challenge)
WebAPI->>Service: verifyRegistration(userId, response, challenge)
Service->>WebAuthn: Verify response signature
WebAuthn-->>Service: Verification result
Service->>DB: Store passkey (credentialId, publicKey, counter)
Service-->>WebAPI: passkeyId, success
WebAPI-->>Browser: Registration success
sequenceDiagram
actor User
participant Browser as Client (Browser)
participant WebAPI as /api/auth/passkey/authenticate
participant Service as Passkey Service
participant DB as Database
participant WebAuthn as WebAuthn Server
User->>Browser: Click "Sign in with Passkey"
Browser->>WebAPI: POST /api/auth/passkey/authenticate/options (email?, csrfToken)
WebAPI->>Service: generateAuthenticationOptions(email?)
Service->>DB: Fetch user passkeys (if email provided)
Service->>WebAuthn: Generate authentication challenge
WebAuthn-->>Service: Challenge & options
Service->>DB: Store hashed challenge with expiry
Service-->>WebAPI: options, challengeId
WebAPI-->>Browser: Authentication options
Browser->>Browser: navigator.credentials.get()
User->>Browser: Complete biometric/platform auth
Browser->>WebAPI: POST /api/auth/passkey/authenticate (response, challenge)
WebAPI->>Service: verifyAuthentication(response, challenge)
Service->>WebAuthn: Verify assertion signature
WebAuthn-->>Service: Verification result + userId
Service->>DB: Validate counter, mark challenge used, update passkey
WebAPI->>DB: Revoke old sessions & create new session
WebAPI-->>Browser: Session cookie + CSRF cookie -> redirect
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 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}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In `@apps/web/src/app/api/auth/passkey/authenticate/options/route.ts`:
- Around line 45-54: The code currently calls req.json() which can throw on
malformed JSON and bubble up to the outer catch producing a 500; change the flow
in the route handler to catch parse errors separately: wrap the await req.json()
in a try/catch (or use a safe JSON parse helper) and if parsing fails return
NextResponse.json({ error: 'Invalid JSON' }, { status: 400 }) before running
optionsSchema.safeParse; keep the subsequent Zod validation using optionsSchema
and return the existing 400 with validation.error.issues when safeParse fails so
malformed JSON yields 400 instead of 500.
In `@apps/web/src/components/auth/PasskeyLoginButton.tsx`:
- Around line 99-107: The cookie parsing truncates CSRF tokens when the value
contains '='; update the logic in the PasskeyLoginButton component (and the
useConditionalPasskeyUI hook) to extract the cookie value by locating the first
'=' and taking the substring after it (or use a robust cookie parser) instead of
using csrfCookie.split('=')[1]; ensure you store the full decoded token to
localStorage under 'csrfToken'.
- Around line 164-261: The startConditionalUI callback in
useConditionalPasskeyUI is unstable because onSuccess is included in its
dependency array and the caller passes an inline arrow function, causing
startConditionalUI to be recreated each render; fix by storing onSuccess in a
ref (e.g., const onSuccessRef = useRef(onSuccess); update onSuccessRef.current
inside an effect when onSuccess changes) and then reference onSuccessRef.current
inside startConditionalUI instead of onSuccess so you can remove onSuccess from
the useCallback deps; update imports to include useRef and ensure
startConditionalUI only depends on stable values (isAvailable, csrfToken) and
calls onSuccessRef.current(redirectUrl) if present.
In `@packages/lib/src/auth/passkey-service.ts`:
- Around line 483-503: The current update in the passkey counter logic is not
atomic because the WHERE only matches passkeys.id and the post-update check
compares newCounter to passkey.counter after the update; change the .where(...)
in the db.update(passkeys).set(...) call to include an atomic guard like
sql`${passkeys.counter} < ${newCounter}` (use your project's sql/raw helper) so
the UPDATE only succeeds if stored counter is less than
verification.authenticationInfo.newCounter, then use counterUpdateResult (the
returned rows) to detect whether the update applied and return
COUNTER_REPLAY_DETECTED if no row was returned; update references: passkeys,
db.update(passkeys).set, verification.authenticationInfo.newCounter,
counterUpdateResult.
- Around line 369-381: The insert uses verificationTokens with userId: 'system'
which violates the users FK and CUID2 format; fix by introducing a dedicated
table (e.g., webauthn_challenges) that stores id (challengeId), tokenHash
(challengeHash), tokenPrefix (options.challenge.substring(0,12)), type
('webauthn_auth'), and expiresAt without a user FK, then change the else branch
that currently inserts into verificationTokens to insert into
webauthn_challenges instead; alternatively, if you prefer to keep a single
table, make verificationTokens.userId nullable in the schema and update the
insert to set userId: null for anonymous challenges and adjust any code that
assumes userId is non-null (search for verificationTokens usage and
verification/cleanup logic).
🧹 Nitpick comments (14)
apps/web/src/app/api/auth/passkey/authenticate/options/route.ts (1)
60-63: Minor: partial email logged on CSRF failure.Logging
email.substring(0, 3) + '***'still leaks a prefix of the user's email. For very short emails, this could be nearly the full address. Consider logging only a boolean (hasEmail) instead, consistent with the success log on Line 87.Proposed fix
loggers.auth.warn('Login CSRF validation failed for passkey auth', { ip: clientIP, - email: email ? email.substring(0, 3) + '***' : undefined, + hasEmail: !!email, });packages/lib/src/monitoring/activity-tracker.ts (1)
101-103: Consider formatting the event union for readability.The event union on Line 103 is quite long. A multi-line format would improve scanability as more events are added.
Suggested formatting
export function trackAuthEvent( userId: string | undefined, - event: 'login' | 'logout' | 'signup' | 'refresh' | 'failed_login' | 'failed_oauth' | 'email_verified' | 'magic_link_login' | 'passkey_login' | 'passkey_registered' | 'passkey_deleted', + event: + | 'login' + | 'logout' + | 'signup' + | 'refresh' + | 'failed_login' + | 'failed_oauth' + | 'email_verified' + | 'magic_link_login' + | 'passkey_login' + | 'passkey_registered' + | 'passkey_deleted', metadata?: LogInput ): void {apps/web/src/hooks/useLoginCSRF.ts (1)
41-43:refreshTokenis a redundant wrapper aroundfetchToken.
refreshTokenjust callsfetchTokenwith no additional logic. You can simplify by returningfetchTokendirectly asrefreshToken.Simplified return
- const refreshToken = useCallback(async () => { - await fetchToken(); - }, [fetchToken]); - return { csrfToken, isLoading, error, - refreshToken, + refreshToken: fetchToken, };apps/web/src/app/api/auth/passkey/register/route.ts (1)
17-21:z.any()violates the "never useanytypes" coding guideline.While the WebAuthn response object is validated downstream by
simplewebauthn, usingz.any()weakens the type-safety contract at the API boundary. Consider usingz.record(z.unknown())orz.unknown()to avoid introducinganyinto the type system while still deferring structural validation to the library.♻️ Suggested change
const verifySchema = z.object({ - response: z.any(), // WebAuthn response - validated by simplewebauthn + response: z.record(z.string(), z.unknown()), // WebAuthn response - validated by simplewebauthn expectedChallenge: z.string().min(1), name: z.string().max(255).optional(), });As per coding guidelines: "Never use
anytypes - always use proper TypeScript types."apps/web/src/app/api/auth/passkey/authenticate/route.ts (2)
19-23: Samez.any()issue — preferz.record(z.string(), z.unknown()).Same guideline violation as in the register route. See the comment on
apps/web/src/app/api/auth/passkey/register/route.tsfor the suggested pattern.As per coding guidelines: "Never use
anytypes - always use proper TypeScript types."
166-173: Consider omittinguserIdfrom the response body.Returning the internal
userIdin a public-facing authentication response is generally unnecessary—the client can obtain it from a subsequent authenticated request (e.g.,/api/auth/me). This avoids leaking internal identifiers in login responses, even though it's the user's own ID.apps/web/src/hooks/useCSRFToken.ts (1)
41-43:refreshTokenis a trivial wrapper aroundfetchToken.Since
refreshTokenonly delegates tofetchTokenwith no additional logic, you can exportfetchTokendirectly (renamed) or just alias it in the return:♻️ Optional simplification
- const refreshToken = useCallback(async () => { - await fetchToken(); - }, [fetchToken]); - return { csrfToken, isLoading, error, - refreshToken, + refreshToken: fetchToken, };packages/db/drizzle/0086_loving_whiplash.sql (1)
1-52: Large migration bundles many unrelated schema changes.This migration includes passkeys, ai_provider_consents, drive_invitations removal, column additions to sessions/pages/channel_messages/ai_usage_logs, and type enum changes — all in a single file. This is expected from Drizzle's migration generator on a feature branch with accumulated changes, but be aware that rollback complexity increases. Ensure the migration has been tested against a staging database before production deployment.
apps/web/src/app/auth/signin/page.tsx (1)
22-22: Unused'passkey'variant inAuthMethod.The
'passkey'value was added to theAuthMethodunion type, but no code path ever setsauthMethodto'passkey'. If this is reserved for a future UI toggle, consider adding a brief comment. Otherwise, it's dead code.apps/web/src/app/settings/account/page.tsx (1)
23-24: Consider consolidating thelucide-reactimports.Line 14 already imports many icons from
lucide-react. The additionalSmartphoneandFingerprinton Line 24 could be merged into that existing import for consistency.apps/web/src/components/settings/PasskeyManager.tsx (2)
39-43: Fetcher uses plainfetch— consider usingfetchWithAuthfor consistent credential/retry handling.The
GET /api/auth/passkeyendpoint is authenticated. While same-originfetchincludes cookies by default, usingfetchWithAuth(imported from@/lib/auth/auth-fetch) would provide automatic 401 retry with token refresh and CSRF handling consistent with other authenticated fetches in the settings page (seeapps/web/src/app/settings/account/page.tsxLine 27 which usesfetchWithAuthfor its fetcher).Proposed fix
+import { fetchWithAuth } from '@/lib/auth/auth-fetch'; + const fetcher = async (url: string) => { - const res = await fetch(url); + const res = await fetchWithAuth(url); if (!res.ok) throw new Error('Failed to fetch'); return res.json(); };
5-5: Use the boundmutatefromuseSWRinstead of the globalmutate.The global
mutateis imported fromswron Line 5, but theuseSWRcall on Line 60 also returns a boundmutatescoped to the'/api/auth/passkey'key. Using the bound version is more idiomatic and avoids repeating the cache key string.Proposed fix
-import useSWR, { mutate } from 'swr'; +import useSWR from 'swr';- const { data, error, isLoading } = useSWR<{ passkeys: Passkey[] }>( + const { data, error, isLoading, mutate } = useSWR<{ passkeys: Passkey[] }>( '/api/auth/passkey', fetcher );Then all
mutate('/api/auth/passkey')calls become justmutate().Also applies to: 60-63
packages/lib/src/auth/passkey-service.test.ts (1)
441-575: Consider adding tests forCOUNTER_REPLAY_DETECTEDandUSER_SUSPENDEDduring authentication.The
verifyAuthenticationtests cover the happy path andCREDENTIAL_NOT_FOUND, but don't exercise:
- Counter replay detection (when
newCounter <= passkey.counter)- User suspension check during authentication
These are important security paths. You could test counter replay by either inserting a passkey with a high initial counter or by adjusting the mock's
newCounterfor a specific test.Would you like me to open an issue to track adding these edge-case tests?
packages/lib/src/auth/passkey-service.ts (1)
40-54:z.any()used forresponsefields violates the "never useanytypes" coding guideline.Lines 42 and 52 use
z.any()for the WebAuthn response objects. While SimpleWebAuthn validates these internally, this bypasses compile-time type safety at the validation boundary. Consider usingz.record(z.unknown())orz.custom<RegistrationResponseJSON>()/z.custom<AuthenticationResponseJSON>()as a typed passthrough that still satisfies the no-anyrule.Proposed fix
const verifyRegSchema = z.object({ userId: z.string().min(1), - response: z.any(), // RegistrationResponseJSON validated by simplewebauthn + response: z.custom<RegistrationResponseJSON>((val) => val != null && typeof val === 'object', { + message: 'Invalid registration response', + }), expectedChallenge: z.string().min(1), name: z.string().max(PASSKEY_CONFIG.maxNameLength).optional(), });const verifyAuthSchema = z.object({ - response: z.any(), // AuthenticationResponseJSON validated by simplewebauthn + response: z.custom<AuthenticationResponseJSON>((val) => val != null && typeof val === 'object', { + message: 'Invalid authentication response', + }), expectedChallenge: z.string().min(1), });As per coding guidelines: "Never use
anytypes - always use proper TypeScript types"
| // Parse and validate request body | ||
| const body = await req.json(); | ||
| const validation = optionsSchema.safeParse(body); | ||
|
|
||
| if (!validation.success) { | ||
| return NextResponse.json( | ||
| { error: 'Invalid request body', details: validation.error.issues }, | ||
| { status: 400 } | ||
| ); | ||
| } |
There was a problem hiding this comment.
req.json() can throw on malformed JSON, returning 500 instead of 400.
If the request body is not valid JSON, req.json() throws before Zod validation runs. The outer catch block returns a generic 500. Consider catching the parse error separately to return a 400.
Proposed fix
// Parse and validate request body
- const body = await req.json();
+ let body: unknown;
+ try {
+ body = await req.json();
+ } catch {
+ return NextResponse.json(
+ { error: 'Invalid JSON body' },
+ { status: 400 }
+ );
+ }
const validation = optionsSchema.safeParse(body);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Parse and validate request body | |
| const body = await req.json(); | |
| const validation = optionsSchema.safeParse(body); | |
| if (!validation.success) { | |
| return NextResponse.json( | |
| { error: 'Invalid request body', details: validation.error.issues }, | |
| { status: 400 } | |
| ); | |
| } | |
| // Parse and validate request body | |
| let body: unknown; | |
| try { | |
| body = await req.json(); | |
| } catch { | |
| return NextResponse.json( | |
| { error: 'Invalid JSON body' }, | |
| { status: 400 } | |
| ); | |
| } | |
| const validation = optionsSchema.safeParse(body); | |
| if (!validation.success) { | |
| return NextResponse.json( | |
| { error: 'Invalid request body', details: validation.error.issues }, | |
| { status: 400 } | |
| ); | |
| } |
🤖 Prompt for AI Agents
In `@apps/web/src/app/api/auth/passkey/authenticate/options/route.ts` around lines
45 - 54, The code currently calls req.json() which can throw on malformed JSON
and bubble up to the outer catch producing a 500; change the flow in the route
handler to catch parse errors separately: wrap the await req.json() in a
try/catch (or use a safe JSON parse helper) and if parsing fails return
NextResponse.json({ error: 'Invalid JSON' }, { status: 400 }) before running
optionsSchema.safeParse; keep the subsequent Zod validation using optionsSchema
and return the existing 400 with validation.error.issues when safeParse fails so
malformed JSON yields 400 instead of 500.
| // Read the CSRF token from cookie (set by server) | ||
| const csrfCookie = document.cookie | ||
| .split('; ') | ||
| .find(row => row.startsWith('csrf_token=')); | ||
|
|
||
| if (csrfCookie) { | ||
| const newCsrfToken = csrfCookie.split('=')[1]; | ||
| localStorage.setItem('csrfToken', newCsrfToken); | ||
| } |
There was a problem hiding this comment.
Cookie value parsing breaks if the CSRF token contains = characters.
csrfCookie.split('=')[1] only captures the portion before the first = in the value. If the token is base64-encoded (which commonly includes = padding), the value will be truncated.
Proposed fix
const csrfCookie = document.cookie
.split('; ')
.find(row => row.startsWith('csrf_token='));
if (csrfCookie) {
- const newCsrfToken = csrfCookie.split('=')[1];
+ const newCsrfToken = csrfCookie.substring('csrf_token='.length);
localStorage.setItem('csrfToken', newCsrfToken);
}The same issue exists at Line 241 in useConditionalPasskeyUI.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Read the CSRF token from cookie (set by server) | |
| const csrfCookie = document.cookie | |
| .split('; ') | |
| .find(row => row.startsWith('csrf_token=')); | |
| if (csrfCookie) { | |
| const newCsrfToken = csrfCookie.split('=')[1]; | |
| localStorage.setItem('csrfToken', newCsrfToken); | |
| } | |
| // Read the CSRF token from cookie (set by server) | |
| const csrfCookie = document.cookie | |
| .split('; ') | |
| .find(row => row.startsWith('csrf_token=')); | |
| if (csrfCookie) { | |
| const newCsrfToken = csrfCookie.substring('csrf_token='.length); | |
| localStorage.setItem('csrfToken', newCsrfToken); | |
| } |
🤖 Prompt for AI Agents
In `@apps/web/src/components/auth/PasskeyLoginButton.tsx` around lines 99 - 107,
The cookie parsing truncates CSRF tokens when the value contains '='; update the
logic in the PasskeyLoginButton component (and the useConditionalPasskeyUI hook)
to extract the cookie value by locating the first '=' and taking the substring
after it (or use a robust cookie parser) instead of using
csrfCookie.split('=')[1]; ensure you store the full decoded token to
localStorage under 'csrfToken'.
| export function useConditionalPasskeyUI( | ||
| csrfToken: string, | ||
| onSuccess?: (redirectUrl: string) => void | ||
| ) { | ||
| const [isAvailable, setIsAvailable] = useState(false); | ||
| const [isAuthenticating, setIsAuthenticating] = useState(false); | ||
|
|
||
| useEffect(() => { | ||
| const checkAvailability = async () => { | ||
| if (typeof window === 'undefined') return; | ||
|
|
||
| // Check if conditional mediation is available | ||
| const available = await ( | ||
| window.PublicKeyCredential?.isConditionalMediationAvailable?.() ?? | ||
| Promise.resolve(false) | ||
| ); | ||
|
|
||
| setIsAvailable(available); | ||
| }; | ||
|
|
||
| checkAvailability(); | ||
| }, []); | ||
|
|
||
| const startConditionalUI = useCallback(async () => { | ||
| if (!isAvailable || !csrfToken) return; | ||
|
|
||
| try { | ||
| // Get authentication options for conditional UI | ||
| const optionsRes = await fetch('/api/auth/passkey/authenticate/options', { | ||
| method: 'POST', | ||
| headers: { | ||
| 'Content-Type': 'application/json', | ||
| }, | ||
| body: JSON.stringify({ | ||
| csrfToken, | ||
| }), | ||
| }); | ||
|
|
||
| if (!optionsRes.ok) return; | ||
|
|
||
| const { options } = await optionsRes.json(); | ||
|
|
||
| setIsAuthenticating(true); | ||
|
|
||
| // Start conditional UI authentication | ||
| const authResponse = await startAuthentication({ | ||
| optionsJSON: options, | ||
| useBrowserAutofill: true, | ||
| }); | ||
|
|
||
| // Verify authentication | ||
| const verifyRes = await fetch('/api/auth/passkey/authenticate', { | ||
| method: 'POST', | ||
| headers: { | ||
| 'Content-Type': 'application/json', | ||
| }, | ||
| body: JSON.stringify({ | ||
| response: authResponse, | ||
| expectedChallenge: options.challenge, | ||
| csrfToken, | ||
| }), | ||
| }); | ||
|
|
||
| if (!verifyRes.ok) { | ||
| const error = await verifyRes.json(); | ||
| toast.error(error.error || 'Authentication failed'); | ||
| return; | ||
| } | ||
|
|
||
| const { redirectUrl } = await verifyRes.json(); | ||
|
|
||
| // Read the CSRF token from cookie | ||
| const csrfCookie = document.cookie | ||
| .split('; ') | ||
| .find(row => row.startsWith('csrf_token=')); | ||
|
|
||
| if (csrfCookie) { | ||
| const newCsrfToken = csrfCookie.split('=')[1]; | ||
| localStorage.setItem('csrfToken', newCsrfToken); | ||
| } | ||
|
|
||
| toast.success('Signed in successfully'); | ||
|
|
||
| if (onSuccess) { | ||
| onSuccess(redirectUrl); | ||
| } else { | ||
| window.location.href = redirectUrl; | ||
| } | ||
| } catch (err) { | ||
| // Conditional UI was cancelled or failed - this is expected behavior | ||
| // Don't show error toast for AbortError (user cancelled) | ||
| if (err instanceof Error && err.name !== 'AbortError') { | ||
| console.debug('Conditional UI authentication failed:', err.message); | ||
| } | ||
| } finally { | ||
| setIsAuthenticating(false); | ||
| } | ||
| }, [isAvailable, csrfToken, onSuccess]); |
There was a problem hiding this comment.
Unstable onSuccess callback reference will cause startConditionalUI to restart on every render.
useConditionalPasskeyUI includes onSuccess in the dependency array of the useCallback for startConditionalUI (Line 261). The caller in signin/page.tsx (Line 112) passes an inline arrow function (redirectUrl) => router.replace(redirectUrl), which creates a new reference on every render. This causes startConditionalUI to get a new identity each render, which in turn re-fires the useEffect in the sign-in page, potentially starting multiple overlapping WebAuthn conditional UI ceremonies.
Use a ref to hold onSuccess so it doesn't affect the callback's memoization:
Proposed fix
export function useConditionalPasskeyUI(
csrfToken: string,
onSuccess?: (redirectUrl: string) => void
) {
const [isAvailable, setIsAvailable] = useState(false);
const [isAuthenticating, setIsAuthenticating] = useState(false);
+ const onSuccessRef = useRef(onSuccess);
+ useEffect(() => { onSuccessRef.current = onSuccess; }, [onSuccess]);
// ... availability check unchanged ...
const startConditionalUI = useCallback(async () => {
// ... existing logic ...
- if (onSuccess) {
- onSuccess(redirectUrl);
+ if (onSuccessRef.current) {
+ onSuccessRef.current(redirectUrl);
} else {
window.location.href = redirectUrl;
}
// ...
- }, [isAvailable, csrfToken, onSuccess]);
+ }, [isAvailable, csrfToken]);Don't forget to add useRef to the React import on Line 3.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export function useConditionalPasskeyUI( | |
| csrfToken: string, | |
| onSuccess?: (redirectUrl: string) => void | |
| ) { | |
| const [isAvailable, setIsAvailable] = useState(false); | |
| const [isAuthenticating, setIsAuthenticating] = useState(false); | |
| useEffect(() => { | |
| const checkAvailability = async () => { | |
| if (typeof window === 'undefined') return; | |
| // Check if conditional mediation is available | |
| const available = await ( | |
| window.PublicKeyCredential?.isConditionalMediationAvailable?.() ?? | |
| Promise.resolve(false) | |
| ); | |
| setIsAvailable(available); | |
| }; | |
| checkAvailability(); | |
| }, []); | |
| const startConditionalUI = useCallback(async () => { | |
| if (!isAvailable || !csrfToken) return; | |
| try { | |
| // Get authentication options for conditional UI | |
| const optionsRes = await fetch('/api/auth/passkey/authenticate/options', { | |
| method: 'POST', | |
| headers: { | |
| 'Content-Type': 'application/json', | |
| }, | |
| body: JSON.stringify({ | |
| csrfToken, | |
| }), | |
| }); | |
| if (!optionsRes.ok) return; | |
| const { options } = await optionsRes.json(); | |
| setIsAuthenticating(true); | |
| // Start conditional UI authentication | |
| const authResponse = await startAuthentication({ | |
| optionsJSON: options, | |
| useBrowserAutofill: true, | |
| }); | |
| // Verify authentication | |
| const verifyRes = await fetch('/api/auth/passkey/authenticate', { | |
| method: 'POST', | |
| headers: { | |
| 'Content-Type': 'application/json', | |
| }, | |
| body: JSON.stringify({ | |
| response: authResponse, | |
| expectedChallenge: options.challenge, | |
| csrfToken, | |
| }), | |
| }); | |
| if (!verifyRes.ok) { | |
| const error = await verifyRes.json(); | |
| toast.error(error.error || 'Authentication failed'); | |
| return; | |
| } | |
| const { redirectUrl } = await verifyRes.json(); | |
| // Read the CSRF token from cookie | |
| const csrfCookie = document.cookie | |
| .split('; ') | |
| .find(row => row.startsWith('csrf_token=')); | |
| if (csrfCookie) { | |
| const newCsrfToken = csrfCookie.split('=')[1]; | |
| localStorage.setItem('csrfToken', newCsrfToken); | |
| } | |
| toast.success('Signed in successfully'); | |
| if (onSuccess) { | |
| onSuccess(redirectUrl); | |
| } else { | |
| window.location.href = redirectUrl; | |
| } | |
| } catch (err) { | |
| // Conditional UI was cancelled or failed - this is expected behavior | |
| // Don't show error toast for AbortError (user cancelled) | |
| if (err instanceof Error && err.name !== 'AbortError') { | |
| console.debug('Conditional UI authentication failed:', err.message); | |
| } | |
| } finally { | |
| setIsAuthenticating(false); | |
| } | |
| }, [isAvailable, csrfToken, onSuccess]); | |
| export function useConditionalPasskeyUI( | |
| csrfToken: string, | |
| onSuccess?: (redirectUrl: string) => void | |
| ) { | |
| const [isAvailable, setIsAvailable] = useState(false); | |
| const [isAuthenticating, setIsAuthenticating] = useState(false); | |
| const onSuccessRef = useRef(onSuccess); | |
| useEffect(() => { | |
| onSuccessRef.current = onSuccess; | |
| }, [onSuccess]); | |
| useEffect(() => { | |
| const checkAvailability = async () => { | |
| if (typeof window === 'undefined') return; | |
| // Check if conditional mediation is available | |
| const available = await ( | |
| window.PublicKeyCredential?.isConditionalMediationAvailable?.() ?? | |
| Promise.resolve(false) | |
| ); | |
| setIsAvailable(available); | |
| }; | |
| checkAvailability(); | |
| }, []); | |
| const startConditionalUI = useCallback(async () => { | |
| if (!isAvailable || !csrfToken) return; | |
| try { | |
| // Get authentication options for conditional UI | |
| const optionsRes = await fetch('/api/auth/passkey/authenticate/options', { | |
| method: 'POST', | |
| headers: { | |
| 'Content-Type': 'application/json', | |
| }, | |
| body: JSON.stringify({ | |
| csrfToken, | |
| }), | |
| }); | |
| if (!optionsRes.ok) return; | |
| const { options } = await optionsRes.json(); | |
| setIsAuthenticating(true); | |
| // Start conditional UI authentication | |
| const authResponse = await startAuthentication({ | |
| optionsJSON: options, | |
| useBrowserAutofill: true, | |
| }); | |
| // Verify authentication | |
| const verifyRes = await fetch('/api/auth/passkey/authenticate', { | |
| method: 'POST', | |
| headers: { | |
| 'Content-Type': 'application/json', | |
| }, | |
| body: JSON.stringify({ | |
| response: authResponse, | |
| expectedChallenge: options.challenge, | |
| csrfToken, | |
| }), | |
| }); | |
| if (!verifyRes.ok) { | |
| const error = await verifyRes.json(); | |
| toast.error(error.error || 'Authentication failed'); | |
| return; | |
| } | |
| const { redirectUrl } = await verifyRes.json(); | |
| // Read the CSRF token from cookie | |
| const csrfCookie = document.cookie | |
| .split('; ') | |
| .find(row => row.startsWith('csrf_token=')); | |
| if (csrfCookie) { | |
| const newCsrfToken = csrfCookie.split('=')[1]; | |
| localStorage.setItem('csrfToken', newCsrfToken); | |
| } | |
| toast.success('Signed in successfully'); | |
| if (onSuccessRef.current) { | |
| onSuccessRef.current(redirectUrl); | |
| } else { | |
| window.location.href = redirectUrl; | |
| } | |
| } catch (err) { | |
| // Conditional UI was cancelled or failed - this is expected behavior | |
| // Don't show error toast for AbortError (user cancelled) | |
| if (err instanceof Error && err.name !== 'AbortError') { | |
| console.debug('Conditional UI authentication failed:', err.message); | |
| } | |
| } finally { | |
| setIsAuthenticating(false); | |
| } | |
| }, [isAvailable, csrfToken]); | |
| } |
🤖 Prompt for AI Agents
In `@apps/web/src/components/auth/PasskeyLoginButton.tsx` around lines 164 - 261,
The startConditionalUI callback in useConditionalPasskeyUI is unstable because
onSuccess is included in its dependency array and the caller passes an inline
arrow function, causing startConditionalUI to be recreated each render; fix by
storing onSuccess in a ref (e.g., const onSuccessRef = useRef(onSuccess); update
onSuccessRef.current inside an effect when onSuccess changes) and then reference
onSuccessRef.current inside startConditionalUI instead of onSuccess so you can
remove onSuccess from the useCallback deps; update imports to include useRef and
ensure startConditionalUI only depends on stable values (isAvailable, csrfToken)
and calls onSuccessRef.current(redirectUrl) if present.
| } else { | ||
| // For conditional UI (no specific user), we need to track the challenge differently | ||
| // We'll create a temporary system entry and validate against it | ||
| // The actual user will be determined by the credential ID during verification | ||
| await db.insert(verificationTokens).values({ | ||
| id: challengeId, | ||
| userId: 'system', // Placeholder for anonymous auth challenges | ||
| tokenHash: challengeHash, | ||
| tokenPrefix: options.challenge.substring(0, 12), | ||
| type: 'webauthn_auth', | ||
| expiresAt, | ||
| }); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat packages/db/src/schema/auth.tsRepository: 2witstudios/PageSpace
Length of output: 12066
🏁 Script executed:
rg -n "userId.*system" --type=ts packages/lib/src/auth/passkey-service.ts -B5 -A5Repository: 2witstudios/PageSpace
Length of output: 631
🏁 Script executed:
rg -rn "'system'" packages/db/src --type=ts | head -30Repository: 2witstudios/PageSpace
Length of output: 190
🏁 Script executed:
rg -rn "cuid|CUID" packages/db/src/schema/auth.ts --type=tsRepository: 2witstudios/PageSpace
Length of output: 110
🏁 Script executed:
fd -e ts -e sql -e js -g '*migration*' -g '*seed*' packages/db/Repository: 2witstudios/PageSpace
Length of output: 117
🏁 Script executed:
rg -rn "webauthn_auth" --type=tsRepository: 2witstudios/PageSpace
Length of output: 355
🏁 Script executed:
cat -n packages/lib/src/auth/passkey-service.ts | sed -n '360,390p'Repository: 2witstudios/PageSpace
Length of output: 1351
🏁 Script executed:
rg -rn "verificationTokens.*insert\|db\.insert.*verificationTokens" --type=ts -B3 -A3Repository: 2witstudios/PageSpace
Length of output: 47
🏁 Script executed:
find packages/db -name '*migration*' -o -name '*seed*' 2>/dev/null | head -20Repository: 2witstudios/PageSpace
Length of output: 47
🏁 Script executed:
rg -rn "system.*user\|'system'" --type=ts packages/db/src -A2 -B2Repository: 2witstudios/PageSpace
Length of output: 47
🏁 Script executed:
grep -rn "type: 'webauthn_auth'\|type: 'n'" packages/lib/src/auth/ --include="*.ts"Repository: 2witstudios/PageSpace
Length of output: 212
🏁 Script executed:
cat -n packages/lib/src/auth/passkey-service.ts | sed -n '1,50p'Repository: 2witstudios/PageSpace
Length of output: 1990
🏁 Script executed:
cat -n packages/lib/src/auth/passkey-service.ts | sed -n '340,400p'Repository: 2witstudios/PageSpace
Length of output: 2614
🏁 Script executed:
rg -rn "verificationTokens\|webauthn" packages/lib/src/auth/passkey-service.ts | head -20Repository: 2witstudios/PageSpace
Length of output: 47
Foreign key violation: userId: 'system' will fail at the database level.
The verificationTokens.userId column is a NOT NULL field with a foreign key constraint referencing users.id. Inserting the literal string 'system' will fail because:
- No user with
id = 'system'exists in the database 'system'does not conform to the CUID2 ID format used throughout the schema
This error will occur at runtime when the else branch executes (when !challengeUserId).
Fix this by:
- Make
userIdnullable onverificationTokensif anonymous challenges are needed, or - Create a dedicated table for anonymous webauthn challenges (e.g.,
webauthn_challengeswithout a user FK), or - Require a userId for all challenges and handle conditional UI differently on the client side
🤖 Prompt for AI Agents
In `@packages/lib/src/auth/passkey-service.ts` around lines 369 - 381, The insert
uses verificationTokens with userId: 'system' which violates the users FK and
CUID2 format; fix by introducing a dedicated table (e.g., webauthn_challenges)
that stores id (challengeId), tokenHash (challengeHash), tokenPrefix
(options.challenge.substring(0,12)), type ('webauthn_auth'), and expiresAt
without a user FK, then change the else branch that currently inserts into
verificationTokens to insert into webauthn_challenges instead; alternatively, if
you prefer to keep a single table, make verificationTokens.userId nullable in
the schema and update the insert to set userId: null for anonymous challenges
and adjust any code that assumes userId is non-null (search for
verificationTokens usage and verification/cleanup logic).
| // Atomic counter update with replay protection | ||
| const newCounter = verification.authenticationInfo.newCounter; | ||
| const counterUpdateResult = await db | ||
| .update(passkeys) | ||
| .set({ | ||
| counter: newCounter, | ||
| lastUsedAt: new Date(), | ||
| }) | ||
| .where( | ||
| and( | ||
| eq(passkeys.id, passkey.id), | ||
| // Only update if new counter is greater (prevents replay) | ||
| // Use raw SQL for the < comparison | ||
| ) | ||
| ) | ||
| .returning(); | ||
|
|
||
| // Check counter wasn't replayed (simplified check - newCounter should be > old counter) | ||
| if (newCounter <= passkey.counter) { | ||
| return { ok: false, error: { code: 'COUNTER_REPLAY_DETECTED' } }; | ||
| } |
There was a problem hiding this comment.
Critical: Counter replay protection is not atomic — check happens after unconditional update.
The comment on Line 483 says "Atomic counter update with replay protection," and Lines 494-496 contain a TODO-like comment about using raw SQL for the < comparison, but the condition was never implemented. The .where() clause (Lines 491-496) only matches on passkeys.id — it doesn't enforce counter < newCounter. The replay check on Lines 500-503 runs after the counter is already overwritten, so:
- TOCTOU race: Two concurrent requests can both update the counter before either checks the old value.
- Data corruption on replay: If a replayed counter is detected at Line 501, the damage is already done — the counter has been updated to the attacker's value.
This directly contradicts the PR's stated security guarantee of "counter replay protection via atomic database updates."
Proposed fix — add a counter guard to the WHERE clause
// Atomic counter update with replay protection
const newCounter = verification.authenticationInfo.newCounter;
+
+ // Reject replay before updating
+ if (newCounter <= passkey.counter) {
+ return { ok: false, error: { code: 'COUNTER_REPLAY_DETECTED' } };
+ }
+
const counterUpdateResult = await db
.update(passkeys)
.set({
counter: newCounter,
lastUsedAt: new Date(),
})
.where(
and(
eq(passkeys.id, passkey.id),
- // Only update if new counter is greater (prevents replay)
- // Use raw SQL for the < comparison
)
)
.returning();
- // Check counter wasn't replayed (simplified check - newCounter should be > old counter)
- if (newCounter <= passkey.counter) {
- return { ok: false, error: { code: 'COUNTER_REPLAY_DETECTED' } };
+ if (counterUpdateResult.length === 0) {
+ // Concurrent update or unexpected state
+ return { ok: false, error: { code: 'COUNTER_REPLAY_DETECTED' } };
}For full atomicity against concurrent requests, you should additionally use Drizzle's sql operator or raw SQL to add a counter < ${newCounter} condition in the WHERE clause, e.g.:
import { sql } from '@pagespace/db'; // or drizzle-orm
.where(
and(
eq(passkeys.id, passkey.id),
sql`${passkeys.counter} < ${newCounter}`
)
)This ensures the update only succeeds if the stored counter is still less than the new one, preventing concurrent replay attacks.
🤖 Prompt for AI Agents
In `@packages/lib/src/auth/passkey-service.ts` around lines 483 - 503, The current
update in the passkey counter logic is not atomic because the WHERE only matches
passkeys.id and the post-update check compares newCounter to passkey.counter
after the update; change the .where(...) in the db.update(passkeys).set(...)
call to include an atomic guard like sql`${passkeys.counter} < ${newCounter}`
(use your project's sql/raw helper) so the UPDATE only succeeds if stored
counter is less than verification.authenticationInfo.newCounter, then use
counterUpdateResult (the returned rows) to detect whether the update applied and
return COUNTER_REPLAY_DETECTED if no row was returned; update references:
passkeys, db.update(passkeys).set, verification.authenticationInfo.newCounter,
counterUpdateResult.
- Change storedChallenge check from !== null to !== undefined (Drizzle findFirst returns undefined, not null) - Wrap assertion in if block to satisfy TypeScript strict null checks Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 participant
Summary
Implements WebAuthn/Passkey authentication for passwordless login, the second phase of the passwordless authentication initiative building on the magic link implementation (PR #635).
Features:
Security:
Technical:
@simplewebauthn/serverand@simplewebauthn/browserpasskeystable with proper indexesTest plan
Related: #586, #568, #569, #570
🤖 Generated with Claude Code
Summary by CodeRabbit