feat: integrate Cloudflare Turnstile CAPTCHA for authentication security#4396
feat: integrate Cloudflare Turnstile CAPTCHA for authentication security#4396
Conversation
This commit implements comprehensive CAPTCHA protection using Cloudflare Turnstile across all authentication flows to enhance security against automated attacks and bot abuse. Key Changes: - Add Turnstile verification component with dark theme and loading states - Integrate CAPTCHA challenges into sign-in and sign-up flows - Update authentication hooks (useSignIn, useSignUp) to handle CAPTCHA tokens - Modify auth actions to validate Turnstile tokens on the server side - Enhance WorkOS and local authentication providers with CAPTCHA support - Update auth types to include CAPTCHA token handling - Add environment configuration for Turnstile site key - Improve loading states and user experience during verification - Add proper error handling for CAPTCHA failures - Update invitation acceptance flows with CAPTCHA protection Technical Implementation: - Uses @marsidev/react-turnstile for React integration - Implements token validation in server-side auth actions - Adds proper TypeScript types for CAPTCHA-enabled auth flows - Maintains backward compatibility with existing auth flows - Includes cumulative layout shift (CLS) optimizations for better UX Security Benefits: - Protects against automated sign-up abuse - Prevents credential stuffing attacks on sign-in - Reduces spam account creation - Maintains legitimate user accessibility with minimal friction Files Modified: - Authentication components and hooks (29 files) - Server-side auth actions and routes - Type definitions and environment configuration - Package dependencies and build configuration
perkinsjr
requested review from
Flo4604,
chronark and
ogzhanolguncu
as code owners
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
📝 WalkthroughWalkthroughAdds Cloudflare Turnstile verification to auth flows (component, actions, hooks, types, env and dependency), introduces optional bypassRadar flags in auth providers, and removes many console.error/console.warn logs across server and client code while keeping core control flow unchanged. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI as SignIn/SignUp UI
participant Hook as useSignIn/useSignUp
participant Actions as auth/actions
participant Server as Auth Provider
participant Turnstile as Cloudflare Turnstile
User->>UI: submit email
UI->>Hook: handleSignInViaEmail / handleSignUpViaEmail
Hook->>Actions: signIn/signUp(email, client metadata)
Actions->>Server: signInViaEmail / signUpViaEmail (radar check)
alt Server returns RADAR_CHALLENGE_REQUIRED
Server-->>Actions: PendingTurnstileResponse
Actions-->>Hook: PendingTurnstileResponse
Hook-->>UI: render TurnstileChallenge
User->>Turnstile: solve widget
Turnstile-->>UI: token
UI->>Hook: handleTurnstileVerification(token, challengeParams)
Hook->>Actions: verifyTurnstileAndRetry(token, challengeParams)
Actions->>Server: signInViaEmail/signUpViaEmail (bypassRadar: true)
Server-->>Actions: success
Actions-->>Hook: success
Hook-->>UI: proceed (navigation/state)
else Server allows / success
Server-->>Actions: success
Actions-->>Hook: success
Hook-->>UI: proceed
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Areas requiring extra attention:
Pre-merge checks and finishing touches❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
💤 Files with no reviewable changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
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 |
|
Thank you for following the naming conventions for pull request titles! 🙏 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (13)
apps/dashboard/lib/auth/sessions.ts (1)
154-167: Consider minimal observability for swallowed session errorsThe underscore-prefixed
_refreshError,_validationError, and_errormake it explicit that errors are ignored, but with all failures collapsing to{ session: null, headers }, diagnosing intermittent auth issues may be harder. Consider wiring these branches into your structured logging or metrics system (even at a low verbosity level) so you can distinguish refresh failures from validation errors without reintroducing noisy console logs.apps/dashboard/app/auth/sign-in/[[...sign-in]]/page.tsx (1)
88-92: Consider preserving error logging for debugging.While the error is properly handled via
setError()to show the user a message, completely suppressing the error in the catch block makes debugging production issues more difficult. Consider preserving some minimal logging (even if just during development) to help trace automatic org selection failures.- .catch((_err) => { + .catch((err) => { + if (process.env.NODE_ENV === 'development') { + console.error('Auto org selection failed:', err); + } setError("Failed to automatically sign in. Please select your workspace.");apps/dashboard/components/auth/turnstile-challenge.tsx (2)
57-60: Preserve original error details for debugging.The
onErrorcallback receives an error from Turnstile but discards it, creating a generic error instead. This loses potentially valuable debugging information about why Turnstile verification failed.onError={(_error) => { setIsWidgetLoading(false); - onError(new Error("Turnstile verification failed")); + onError(_error || new Error("Turnstile verification failed")); }}
40-47: Add accessibility attributes to loading overlay.The loading overlay should include ARIA attributes to properly communicate the loading state to screen readers.
{isWidgetLoading && ( - <div className="absolute inset-0 flex items-center justify-center bg-gray-800/90 backdrop-blur-sm rounded border border-gray-600 z-10"> + <div + className="absolute inset-0 flex items-center justify-center bg-gray-800/90 backdrop-blur-sm rounded border border-gray-600 z-10" + role="status" + aria-live="polite" + aria-label="Loading verification widget" + > <div className="flex items-center space-x-2"> <div className="animate-spin h-4 w-4 border-2 border-gray-400 border-t-white rounded-full" /> <span className="text-sm text-gray-300">Loading verification...</span>apps/dashboard/app/auth/actions.ts (1)
86-213: Error handling inverifyAuthCodenow fully user-facing, but inner errors are completely silentThe outer
try/catchnow always maps unexpected failures to anUNKNOWN_ERRORresponse, and all innercatch (_error)blocks (invitation/org lookup and auto-selection) silently swallow errors and fall back to the original result.Behavior-wise this is fine for UX (you don’t block login on secondary failures), but you lose observability for real bugs in invitation/org flows. Consider routing these
_errors through a structured logger (at debug/info level) rather than dropping them entirely, e.g.:- } catch (_error) { - // Fall through to return the original result if auto-selection fails - } + } catch (error) { + // Fall through to return the original result if auto-selection fails + logger.debug?.("auto-org-selection failed", { error }); + }Same pattern could apply to the other invitation/org-related
catch (_error)blocks.apps/dashboard/app/auth/hooks/useSignIn.ts (2)
88-118:isAuthErrorResponsetype guard is now used withEmailAuthResult—consider widening its parameter type
handleSignInViaEmailnow calls:const result = await signInViaEmail(email); // EmailAuthResult ... if (isAuthErrorResponse(result)) { ... }but
isAuthErrorResponseis declared as:function isAuthErrorResponse(result: VerificationResult): result is AuthErrorResponse;Given
EmailAuthResultandVerificationResultare different unions, this can be awkward for TypeScript and may require casts.You could make the guard reusable across both flows by widening its parameter type, e.g.:
-function isAuthErrorResponse(result: VerificationResult): result is AuthErrorResponse { +function isAuthErrorResponse( + result: EmailAuthResult | VerificationResult, +): result is AuthErrorResponse {(or just
unknownplus a runtime check) so it cleanly supports both email auth and verification flows.
59-83: Silencing errors incheckAuthStatusis acceptable here but may hide intermittent issuesThe outer
trynow hascatch (_err) { /* Ignore auth status check errors */ }, so any failure readingorgsorPENDING_SESSION_COOKIEjust results inloadingeventually becomingfalse.Given this is only used to drive UX hints (org selector and “pending auth” indicator), failing open is reasonable. If you see odd behavior around org selection in the wild, consider emitting a low-level log here rather than fully ignoring.
apps/dashboard/app/auth/sign-up/email-signup.tsx (1)
105-124: EnsurehandleTurnstileVerificationhas access to the user’s name data for sign‑up retriesOn Turnstile success you call:
const result = await handleTurnstileVerification(token, turnstileChallenge); if (result?.success) { setTurnstileChallenge(null); setVerification(true); } else { toast.error("Verification failed. Please try again."); }But
verifyTurnstileAndRetry’s sign‑up path expectsuserData(firstName/lastName) whenchallengeParams.action === "sign-up".If the
useSignUphook’shandleTurnstileVerificationisn’t already pulling first/last name from its own state/context and passing them through asuserData, sign-ups that go through a Turnstile challenge may fail with “Invalid challenge parameters” or lose name information.Either:
- Pass the names explicitly here, e.g.:
await handleTurnstileVerification(token, turnstileChallenge, { firstName, lastName });or
- Confirm that
useSignUpmaintains and forwardsuserDatainternally.apps/dashboard/app/auth/sign-in/email-signin.tsx (1)
58-73: Turnstile success flow is solid; consider surfacing a small error on Turnstile failureOn success:
- You guard against a missing
turnstileChallenge.- Call
handleTurnstileVerification, then clear the challenge and mark"email"aslastUsed.isTurnstileLoadingensures the Turnstile UI can show a loading state.On error,
handleTurnstileErrorjust clears the challenge:const handleTurnstileError = () => { setTurnstileChallenge(null); };Functionally this is fine—the user is dropped back to the email form—but you might consider a subtle message (toast or inline) so users know why the challenge disappeared, e.g., “Verification failed, please try again.”
Not strictly required, but could improve UX.
Also applies to: 79-98
apps/dashboard/app/auth/hooks/useSignUp.ts (2)
33-42: Turnstile retry flow wiring is correct and safely guarded by userData checks
handleSignUpViaEmailupdatesuserDatabefore callingsignUpViaEmail, which ensuresuserDatais present when a Radar challenge occurs.handleTurnstileVerificationthen validatesfirstName/lastNameand delegates toverifyTurnstileAndRetrywith{ email, challengeParams }from the server response and{ firstName, lastName }from local state, which matches the expected signature inapps/dashboard/app/auth/actions.tsand the WorkOS provider’ssignUpViaEmailwithbypassRadar: true. This is aligned with the intended “only bypass Radar after passing Turnstile” pattern.Also applies to: 44-63
85-99: Resend flow is sound; consider whether the detailed error message should be user‑facingValidating
userData.emailbefore callingresendAuthCodeand returning anEmailAuthResultis consistent with the provider’sresendAuthCodecontract. Wrapping failures in a newErrorthat includes the email and original error message is reasonable, but if this message is surfaced directly to end‑users, you may want to switch to a more generic string to avoid leaking internal error details (and the full email) outside the UI context.apps/dashboard/lib/auth/workos.ts (2)
174-210: Session refresh and org switch logic are straightforward; double‑check TTL alignmentBoth
refreshSessionandswitchOrgnow:
- Load the sealed session once via
userManagement.loadSealedSession.- Call
session.refresh(...)and requireauthenticated && session && sealedSessionto proceed.- Compute
expiresAtas “now + 7 days” usingsetDate.- Throw explicit errors when the refresh result is malformed or unauthenticated.
This is clear and defensive. The only thing worth confirming is that this 7‑day
expiresAtmatches the cookie TTL configured ingetAuthCookieOptions; otherwise you could get subtle mismatches between server‑side session validity and browser cookie lifetime.Also applies to: 317-351
169-171: Silent fallbacks on provider errors are reasonable but reduce debuggabilityChanging several methods to swallow errors and return safe defaults:
validateSession→{ isValid: false, shouldRefresh: false }on error.getUser/findUser→nullon provider failure.getInvitationList→[]+ empty metadata on failure.getInvitation→nullon failure.getSignOutUrl→nullon failure.This is acceptable from a UX perspective (fail softly instead of crashing), especially in read‑only/listing operations. The tradeoff is reduced visibility into upstream WorkOS issues. If debugging becomes harder in practice, consider adding non‑console logging (e.g., structured logs or Sentry) in these catch blocks later without changing the public behavior.
Also applies to: 225-227, 244-246, 529-534, 546-548, 879-880
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (28)
apps/dashboard/app/(app)/api/auth/refresh/route.ts(0 hunks)apps/dashboard/app/api/auth/accept-invitation/route.ts(2 hunks)apps/dashboard/app/api/auth/invitation/route.ts(1 hunks)apps/dashboard/app/auth/actions.ts(12 hunks)apps/dashboard/app/auth/hooks/useSignIn.ts(6 hunks)apps/dashboard/app/auth/hooks/useSignUp.ts(2 hunks)apps/dashboard/app/auth/sign-in/[[...sign-in]]/page.tsx(3 hunks)apps/dashboard/app/auth/sign-in/email-code.tsx(2 hunks)apps/dashboard/app/auth/sign-in/email-signin.tsx(3 hunks)apps/dashboard/app/auth/sign-in/oauth-signin.tsx(1 hunks)apps/dashboard/app/auth/sign-up/[[...sign-up]]/page.tsx(1 hunks)apps/dashboard/app/auth/sign-up/email-code.tsx(4 hunks)apps/dashboard/app/auth/sign-up/email-signup.tsx(6 hunks)apps/dashboard/app/auth/sign-up/oauth-signup.tsx(1 hunks)apps/dashboard/components/auth/post-auth-invitation-handler.tsx(0 hunks)apps/dashboard/components/auth/turnstile-challenge.tsx(1 hunks)apps/dashboard/lib/auth/base-provider.ts(4 hunks)apps/dashboard/lib/auth/cookies.ts(1 hunks)apps/dashboard/lib/auth/get-auth.ts(1 hunks)apps/dashboard/lib/auth/local.ts(2 hunks)apps/dashboard/lib/auth/sessions.ts(4 hunks)apps/dashboard/lib/auth/types.ts(3 hunks)apps/dashboard/lib/auth/workos.ts(11 hunks)apps/dashboard/lib/env.ts(1 hunks)apps/dashboard/package.json(1 hunks)apps/engineering/content/design/components/search/llm-search.examples.tsx(1 hunks)apps/engineering/content/design/components/toaster.example.tsx(2 hunks)turbo.json(1 hunks)
💤 Files with no reviewable changes (2)
- apps/dashboard/components/auth/post-auth-invitation-handler.tsx
- apps/dashboard/app/(app)/api/auth/refresh/route.ts
🧰 Additional context used
🧠 Learnings (12)
📓 Common learnings
Learnt from: Flo4604
Repo: unkeyed/unkey PR: 4190
File: go/internal/services/keys/verifier.go:51-53
Timestamp: 2025-10-30T15:10:52.743Z
Learning: PR #4190 for unkeyed/unkey is focused solely on database schema and query changes for identity-based credits. It adds IdentityCredits and KeyCredits fields to structs and queries, but does not implement the priority enforcement logic in the usagelimiter. The logic implementation is intentionally deferred to a later PR in the stack.
Learnt from: mcstepp
Repo: unkeyed/unkey PR: 4355
File: apps/dashboard/lib/auth/workos.ts:104-144
Timestamp: 2025-11-20T19:10:23.081Z
Learning: In `apps/dashboard/lib/auth/workos.ts`, the `checkRadar()` method intentionally has different default behaviors for different failure modes: (1) When Radar API fails (network error or non-OK HTTP response), it defaults to `{ action: "allow" }` to fail-open and avoid blocking signups when the service is down. (2) When Radar API succeeds (OK response) but `data.verdict` is missing, it defaults to `"block"` via `data.verdict || "block"` to fail-closed as a conservative approach when the service is operational but returns unexpected data. This design prevents both service outages from blocking legitimate users and malformed responses from being treated permissively.
📚 Learning: 2024-10-25T23:53:41.716Z
Learnt from: Srayash
Repo: unkeyed/unkey PR: 2568
File: apps/dashboard/app/auth/sign-up/oauth-signup.tsx:25-25
Timestamp: 2024-10-25T23:53:41.716Z
Learning: In the React component `OAuthSignUp` (`apps/dashboard/app/auth/sign-up/oauth-signup.tsx`), adding a `useEffect` cleanup function to reset the `isLoading` state causes a "something went wrong" popup to appear before redirecting when a user clicks on signup.
Applied to files:
apps/dashboard/app/auth/sign-in/email-code.tsxapps/dashboard/app/auth/sign-up/[[...sign-up]]/page.tsxapps/dashboard/app/auth/sign-in/oauth-signin.tsxapps/dashboard/app/auth/sign-in/[[...sign-in]]/page.tsxapps/dashboard/app/auth/sign-up/oauth-signup.tsxapps/dashboard/app/auth/sign-in/email-signin.tsxapps/dashboard/app/auth/hooks/useSignUp.tsapps/dashboard/app/auth/sign-up/email-signup.tsxapps/dashboard/app/auth/sign-up/email-code.tsx
📚 Learning: 2025-11-20T19:10:23.081Z
Learnt from: mcstepp
Repo: unkeyed/unkey PR: 4355
File: apps/dashboard/lib/auth/workos.ts:104-144
Timestamp: 2025-11-20T19:10:23.081Z
Learning: In `apps/dashboard/lib/auth/workos.ts`, the `checkRadar()` method intentionally has different default behaviors for different failure modes: (1) When Radar API fails (network error or non-OK HTTP response), it defaults to `{ action: "allow" }` to fail-open and avoid blocking signups when the service is down. (2) When Radar API succeeds (OK response) but `data.verdict` is missing, it defaults to `"block"` via `data.verdict || "block"` to fail-closed as a conservative approach when the service is operational but returns unexpected data. This design prevents both service outages from blocking legitimate users and malformed responses from being treated permissively.
Applied to files:
apps/dashboard/app/auth/sign-up/[[...sign-up]]/page.tsxapps/dashboard/lib/auth/types.tsapps/dashboard/app/auth/sign-up/oauth-signup.tsxapps/dashboard/app/auth/actions.tsapps/dashboard/lib/auth/local.tsapps/dashboard/lib/auth/workos.tsapps/dashboard/app/auth/hooks/useSignIn.tsapps/dashboard/app/auth/hooks/useSignUp.tsapps/dashboard/lib/auth/base-provider.ts
📚 Learning: 2024-10-23T16:19:42.049Z
Learnt from: p6l-richard
Repo: unkeyed/unkey PR: 2085
File: apps/www/components/glossary/search.tsx:41-57
Timestamp: 2024-10-23T16:19:42.049Z
Learning: For the `FilterableCommand` component in `apps/www/components/glossary/search.tsx`, adding error handling and loading states to the results list is not necessary.
Applied to files:
apps/engineering/content/design/components/search/llm-search.examples.tsx
📚 Learning: 2024-10-23T16:21:47.395Z
Learnt from: p6l-richard
Repo: unkeyed/unkey PR: 2085
File: apps/www/components/glossary/search.tsx:16-20
Timestamp: 2024-10-23T16:21:47.395Z
Learning: For the `FilterableCommand` component in `apps/www/components/glossary/search.tsx`, refactoring type definitions into an interface is not necessary at this time.
Applied to files:
apps/engineering/content/design/components/search/llm-search.examples.tsx
📚 Learning: 2025-04-30T15:25:33.917Z
Learnt from: mcstepp
Repo: unkeyed/unkey PR: 3210
File: apps/dashboard/app/new/page.tsx:3-3
Timestamp: 2025-04-30T15:25:33.917Z
Learning: There are two different `getAuth` functions in the Unkey codebase with different purposes:
1. `@/lib/auth/get-auth` - Base function without redirects, used in special cases on the dashboard where redirect control is needed (like `/new` page) and within tRPC context
2. `@/lib/auth` - Helper function with redirects, used in most dashboard cases (approximately 98%)
Applied to files:
apps/dashboard/lib/auth/get-auth.ts
📚 Learning: 2025-05-05T17:55:59.607Z
Learnt from: mcstepp
Repo: unkeyed/unkey PR: 3215
File: apps/dashboard/lib/auth/sessions.ts:47-51
Timestamp: 2025-05-05T17:55:59.607Z
Learning: Local auth cookies in apps/dashboard/lib/auth/sessions.ts intentionally omit HttpOnly and Secure flags to allow easier debugging during local development. This is by design as these cookies are only used in local development environments, not production.
Applied to files:
apps/dashboard/lib/auth/sessions.tsapps/dashboard/lib/auth/local.tsapps/dashboard/lib/auth/cookies.ts
📚 Learning: 2025-06-02T11:09:58.791Z
Learnt from: ogzhanolguncu
Repo: unkeyed/unkey PR: 3292
File: apps/dashboard/lib/trpc/routers/key/create.ts:11-14
Timestamp: 2025-06-02T11:09:58.791Z
Learning: In the unkey codebase, TypeScript and the env() function implementation already provide sufficient validation for environment variables, so additional runtime error handling for missing env vars is not needed.
Applied to files:
apps/dashboard/app/auth/actions.tsapps/dashboard/lib/env.ts
📚 Learning: 2025-09-24T18:57:34.843Z
Learnt from: mcstepp
Repo: unkeyed/unkey PR: 4010
File: QUICKSTART-DEPLOY.md:17-17
Timestamp: 2025-09-24T18:57:34.843Z
Learning: In the Unkey deployment platform, API key environment variables use component-specific naming but share the same secret value: UNKEY_API_KEY for the ctrl service (validator), API_KEY for the CLI client, and CTRL_API_KEY for the dashboard client. The ctrl service acts as the source of truth for validation.
Applied to files:
apps/dashboard/app/auth/actions.ts
📚 Learning: 2025-08-08T16:07:48.307Z
Learnt from: imeyer
Repo: unkeyed/unkey PR: 3755
File: .github/actions/setup-node/action.yaml:37-40
Timestamp: 2025-08-08T16:07:48.307Z
Learning: Repo unkeyed/unkey — pnpm immutable installs are enforced by setting the CI environment variable; any truthy value (e.g., "1" or "true") is acceptable. Do not require the literal string "true". Applies to .github/actions/setup-node/action.yaml and all workflows using pnpm install.
Applied to files:
apps/dashboard/app/auth/actions.ts
📚 Learning: 2024-10-23T12:05:31.121Z
Learnt from: chronark
Repo: unkeyed/unkey PR: 2544
File: apps/api/src/pkg/env.ts:4-6
Timestamp: 2024-10-23T12:05:31.121Z
Learning: The `cloudflareRatelimiter` type definition in `apps/api/src/pkg/env.ts` should not have its interface changed; it should keep the `limit` method returning `Promise<{ success: boolean }>` without additional error properties.
Applied to files:
apps/dashboard/app/auth/actions.ts
📚 Learning: 2025-09-01T16:43:57.850Z
Learnt from: perkinsjr
Repo: unkeyed/unkey PR: 3898
File: apps/dashboard/app/(app)/settings/general/page.tsx:15-18
Timestamp: 2025-09-01T16:43:57.850Z
Learning: In the Unkey dashboard, orgId guards are intentionally duplicated across pages rather than extracted to helpers because each page needs different additional context (user details, DB connections, subscriptions, etc.). The orgId check serves as both authentication and handling edge cases where users sign up but don't have organization/workspace setup completed.
Applied to files:
apps/dashboard/app/auth/hooks/useSignIn.ts
🧬 Code graph analysis (8)
apps/dashboard/lib/auth/sessions.ts (2)
apps/dashboard/lib/auth/types.ts (1)
UNKEY_SESSION_COOKIE(4-4)apps/dashboard/lib/auth/cookies.ts (1)
getCookieOptionsAsString(142-178)
apps/dashboard/app/auth/sign-in/email-signin.tsx (3)
apps/dashboard/app/auth/hooks/useSignIn.ts (1)
useSignIn(46-231)apps/dashboard/lib/auth/types.ts (1)
PendingTurnstileResponse(94-103)apps/dashboard/components/auth/turnstile-challenge.tsx (1)
TurnstileChallenge(13-71)
apps/dashboard/app/auth/actions.ts (2)
apps/dashboard/lib/env.ts (1)
env(3-55)apps/dashboard/lib/auth/types.ts (2)
PendingTurnstileResponse(94-103)EmailAuthResult(106-106)
apps/dashboard/lib/auth/local.ts (1)
apps/dashboard/lib/auth/types.ts (1)
UserData(196-200)
apps/dashboard/lib/auth/workos.ts (1)
apps/dashboard/lib/auth/types.ts (2)
UserData(196-200)EmailAuthResult(106-106)
apps/dashboard/app/auth/hooks/useSignIn.ts (1)
apps/dashboard/lib/auth/types.ts (3)
EmailAuthResult(106-106)PendingTurnstileResponse(94-103)errorMessages(219-240)
apps/dashboard/app/auth/hooks/useSignUp.ts (3)
apps/dashboard/lib/auth/types.ts (4)
EmailAuthResult(106-106)PendingTurnstileResponse(94-103)UserData(196-200)VerificationResult(107-110)apps/dashboard/app/auth/actions.ts (5)
signUpViaEmail(76-79)verifyTurnstileAndRetry(444-489)verifyAuthCode(86-214)verifyEmail(216-244)resendAuthCode(246-285)apps/dashboard/lib/auth/workos.ts (4)
signUpViaEmail(578-647)verifyAuthCode(714-770)verifyEmail(772-827)resendAuthCode(705-712)
apps/dashboard/lib/auth/base-provider.ts (1)
apps/dashboard/lib/auth/types.ts (2)
VerificationResult(107-110)UserData(196-200)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Test Packages / Test
- GitHub Check: Test Dashboard / Test Dashboard
- GitHub Check: autofix
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (38)
apps/engineering/content/design/components/search/llm-search.examples.tsx (1)
117-117: LGTM! Console logging removed as intended.The change from console logging to a no-op callback aligns with the PR objectives to remove console logs across the codebase. For this example component that focuses on demonstrating placeholder customization, the no-op callback is appropriate—the loading state behavior is still demonstrated through the
useSearchStatehook, and the underscore prefix correctly indicates the intentionally unused parameter.apps/engineering/content/design/components/toaster.example.tsx (1)
155-155: Inconsistency between code snippet and implementation.The
customCodeSnippet(lines 116, 129, 133) still displaysconsole.logstatements, but the actual rendered component now uses empty functions. This mismatch will confuse developers who copy the snippet expecting console output.For documentation examples,
console.logis often appropriate to demonstrate that callbacks execute. Consider either:
- Reverting to
console.login both the snippet and implementation for consistency and educational value- Updating the
customCodeSnippetto match the empty function implementationApply this diff to align the snippet with the implementation:
action: { label: "Undo", - onClick: () => console.log("Undo clicked"), + onClick: () => {}, },action: { label: "Delete", - onClick: () => console.log("Delete confirmed"), + onClick: () => {}, }, cancel: { label: "Cancel", - onClick: () => console.log("Delete cancelled"), + onClick: () => {}, },Also applies to: 168-168, 172-172
⛔ Skipped due to learnings
Learnt from: MichaelUnkey Repo: unkeyed/unkey PR: 3425 File: apps/engineering/content/design/components/filter/control-cloud.examples.tsx:73-83 Timestamp: 2025-07-02T14:13:01.711Z Learning: In apps/engineering/content/design/components/, when the `RenderComponentWithSnippet` component does not render code snippets correctly, use the `customCodeSnippet` prop to manually provide the correct JSX code as a string. This manual approach is necessary due to technical limitations in the automatic rendering mechanism.apps/dashboard/app/auth/sign-in/oauth-signin.tsx (1)
38-42: Confirm error tracking strategy for OAuth failures.Verification found no structured error tracking infrastructure (Sentry, Datadog, logging service, etc.) in the codebase. OAuth failures will be silently caught with only a generic toast message and no observability for debugging or monitoring.
While the removal of
console.error()in client-side code is reasonable, losing all visibility into OAuth failures is problematic for a critical authentication flow. Before merging:
- Confirm your team has an alternative error tracking/monitoring strategy for auth flows
- If not, consider either adding structured error tracking (e.g., Sentry) or restoring error logging with additional context for the error handling service
- This change appears to be part of systematic logging cleanup; consider separating it into a dedicated PR for better change isolation and review focus
apps/dashboard/lib/auth/cookies.ts (1)
124-126: ExpandedsetLastUsedOrgCookieparam object looks goodThis is a pure signature/formatting tweak; call shape and behavior are unchanged, and the explicit object type improves readability without side effects.
apps/dashboard/lib/auth/sessions.ts (2)
49-51: Local auth Set-Cookie fallback remains correctOnly the string interpolation is reflowed; the emitted cookie (
Path=/; SameSite=Strict; Max-Age=10y) is unchanged and still matches the intentional choice to omit HttpOnly/Secure for local-only auth cookies. Based on learnings, this continues to align with the local-debug design.
114-118: Refreshed-session Set-Cookie construction stays consistent with defaultsUsing
getCookieOptionsAsString({ expiresAt: refreshedSession.expiresAt })inside the template keeps the refreshed session cookie aligned with your centralized defaults (path, SameSite, HttpOnly/Secure, etc.) in both the primary and fallback paths; the changes here are formatting-only and maintain correct behavior.Also applies to: 132-136
apps/dashboard/app/auth/sign-up/oauth-signup.tsx (1)
20-37: Catch block cleanup aligns with existing UXRenaming the error param to
_errand relying solely on the toast keeps the user-facing behavior while avoiding noisy console logs and unused-variable warnings. No issues from a flow or correctness perspective.apps/dashboard/app/api/auth/invitation/route.ts (1)
51-55: Generic 500 response without logging is fine hereCatching as
_errorand returning a generic 500 withno-storekeeps the API contract intact and avoids leaking internals, which is consistent with the rest of the auth routes in this PR.apps/dashboard/app/api/auth/accept-invitation/route.ts (1)
41-46: Error-handling updates are consistent with route behaviorSwitching to
_errorand dropping console logging in both catches keeps the existing response semantics (400 for bad tokens, 500 for unexpected failures) while matching the quieter logging pattern used elsewhere in this PR.Also applies to: 104-106
apps/dashboard/lib/auth/get-auth.ts (1)
14-34: Graceful fallback to unauthenticated state remains intactCatching as
_errorand returning nulluserId/orgId/roleon failure preserves the existing behavior of treating errors as “no auth” for this non-redirecting helper, which is appropriate for its usage.Based on learnings, this aligns with the intended role of
@/lib/auth/get-authas the base, non-redirecting auth helper.apps/dashboard/package.json (1)
103-104: @marsidev/react-turnstile@^1.0.2 is compatible with React 18 and Next.js 14The package declares peerDependencies supporting React 18 and React DOM 18, and version 1.3.1 is the latest release. The package includes SSR/Next.js support, so ^1.0.2 is well-suited for your setup. Ensure all usages stay in
"use client"components and that the Turnstile site key env is properly configured.apps/dashboard/app/auth/sign-up/[[...sign-up]]/page.tsx (1)
41-42: LGTM! Accurate error message.The error message now correctly reflects that this is a sign-up failure, not a sign-in failure, improving debugging clarity.
apps/dashboard/app/auth/sign-in/[[...sign-in]]/page.tsx (2)
53-55: Silent error suppression acceptable here.Ignoring cookie read errors is reasonable since the application continues to function normally without the last-used org preference.
125-127: Silent error suppression acceptable for auto sign-in.Since this is an automatic convenience feature and users can manually sign in if it fails, silently ignoring errors here is acceptable. The loading state is properly reset in the finally block.
apps/dashboard/lib/env.ts (1)
52-53: LGTM! Proper separation of public and secret keys.The environment variables follow Next.js conventions correctly:
NEXT_PUBLIC_CLOUDFLARE_TURNSTILE_SITE_KEYis client-accessible for the Turnstile widgetCLOUDFLARE_TURNSTILE_SECRET_KEYremains server-side only for verificationBoth are appropriately marked optional to support environments where Turnstile is not configured.
apps/dashboard/app/auth/sign-up/email-code.tsx (3)
102-111: Excellent form handling improvements.The conversion to a proper
<form>element with submit handling and loading guards prevents race conditions and improves accessibility. The form can now be submitted via Enter key, and concurrent submissions are properly prevented.
116-122: Good race condition prevention.Adding the
isLoadingcheck inonCompleteand thedisabledprop prevents multiple concurrent verification attempts, improving UX and reducing unnecessary API calls.
155-155: Fixed class name formatting issue.Removing the trailing space in the conditional class ensures proper CSS class application when the slot is active.
apps/dashboard/lib/auth/types.ts (3)
93-103: Verify optional fields inchallengeParams.The
challengeParamsstructure hasipAddressanduserAgentas optional, whileauthMethodandactionare required. Ensure this is intentional and that the verification flow can handle missing IP address or user agent information gracefully.If these fields are always expected to be present during Radar challenges, consider making them required to catch configuration issues earlier.
Please verify:
- Are there legitimate scenarios where
ipAddressoruserAgentmight be unavailable during a Radar challenge?- Does the Turnstile verification flow handle missing optional fields correctly?
106-106: LGTM! Consistent type union expansion.Adding
PendingTurnstileResponseto theEmailAuthResultunion follows the established pattern for handling special authentication states like org selection and email verification.
216-216: LGTM! Clear error messaging for Turnstile challenges.The new error code and user-facing message clearly communicate that a verification challenge is required, maintaining consistency with other auth error messages in the codebase.
Also applies to: 238-239
apps/dashboard/lib/auth/local.ts (1)
367-376: LGTM! API compatibility maintained.Adding the
bypassRadarparameter to both methods maintains API compatibility with the WorkOS provider. It's appropriate that the local development provider ignores this parameter since it's a no-op implementation that always succeeds.Also applies to: 378-386
turbo.json (1)
41-42: No issues found with removal of Clerk environment variables.Verification confirms that
CLERK_WEBHOOK_SECRETandCLERK_SECRET_KEYare defined in the Zod schema (apps/dashboard/lib/env.ts:19-20) as optional fields but are never actually used in the codebase. Their removal fromturbo.jsonis safe and will not break any functionality.apps/dashboard/app/auth/actions.ts (3)
246-285: RatelimitonErrorfail-open behavior looks intentional and consistentFalling back to
{ success: true, ... }fromonErrorkeeps resend flow available when the ratelimit backend fails, which matches a fail-open design for non-security-critical rate limiting. The_errrename also avoids unused-variable noise without changing semantics.No change needed here.
Also applies to: 257-265
441-489: Turnstile retry flow andbypassRadarusage look correct; confirmactionvalues are stableThe
verifyTurnstileAndRetryhelper correctly:
- Verifies the Turnstile token server-side.
- On success, retries the original operation with
bypassRadar: trueso Radar isn’t re-run after a human challenge.- Distinguishes between
"sign-up"(withuserData) vs"sign-in"actions.Two follow-ups to consider:
- Ensure the
challengeParams.actionvalues ("sign-up"/"sign-in") are treated as constants throughout the stack (ideally centralised as string literals or an enum) to avoid typos.- If you ever add more actions, this function will currently fall back to
UNKNOWN_ERROR; documenting/typing allowedactionvalues inPendingTurnstileResponse["challengeParams"]would help catch that at compile time.Overall flow looks good.
42-73: Turnstile verification likely broken due to JSON payload instead of form-encoded bodyCloudflare Turnstile’s
siteverifyendpoint expectssecretandresponseas URL-encoded form data; sending JSON here risks every verification failing and permanently blocking challenged logins/signups.Consider switching to
application/x-www-form-urlencodedwithURLSearchParams:- try { - const response = await fetch("https://challenges.cloudflare.com/turnstile/v0/siteverify", { - method: "POST", - headers: { - "Content-Type": "application/json", - }, - body: JSON.stringify({ - secret: secretKey, - response: token, - }), - }); + try { + const response = await fetch("https://challenges.cloudflare.com/turnstile/v0/siteverify", { + method: "POST", + headers: { + "Content-Type": "application/x-www-form-urlencoded", + }, + body: new URLSearchParams({ + secret: secretKey, + response: token, + }), + });This keeps the rest of the logic (fail-closed on missing key / non-OK / exceptions) intact.
⛔ Skipped due to learnings
Learnt from: mcstepp Repo: unkeyed/unkey PR: 4355 File: apps/dashboard/lib/auth/workos.ts:104-144 Timestamp: 2025-11-20T19:10:23.081Z Learning: In `apps/dashboard/lib/auth/workos.ts`, the `checkRadar()` method intentionally has different default behaviors for different failure modes: (1) When Radar API fails (network error or non-OK HTTP response), it defaults to `{ action: "allow" }` to fail-open and avoid blocking signups when the service is down. (2) When Radar API succeeds (OK response) but `data.verdict` is missing, it defaults to `"block"` via `data.verdict || "block"` to fail-closed as a conservative approach when the service is operational but returns unexpected data. This design prevents both service outages from blocking legitimate users and malformed responses from being treated permissively.apps/dashboard/app/auth/hooks/useSignIn.ts (2)
37-44: Turnstile challenge type guard is precise and safe
isPendingTurnstileChallengecheckssuccess === false, the specificRADAR_CHALLENGE_REQUIREDcode, and requiredchallengeParamsfields. This safely narrowsEmailAuthResulttoPendingTurnstileResponsewithout overlapping normal error responses.Nice, clear guard.
185-218: Turnstile verification handler behavior is consistent with other auth flows
handleTurnstileVerification:
- Clears existing errors, calls
verifyTurnstileAndRetry, and setssetIsVerifying(true)on success.- Reuses the same error handling as
handleSignInViaEmail, includingACCOUNT_NOT_FOUND→setAccountNotFound(true)and generic fallback toUNKNOWN_ERROR.- Propagates unexpected exceptions so the caller can handle them.
This keeps Turnstile sign-in behavior aligned with the non-challenge path and centralizes the Radar/Turnstile specifics in the server actions.
Looks good to me.
apps/dashboard/app/auth/sign-up/email-signup.tsx (3)
36-43: Form validation and disabled submit behavior are solid and consistent
isValidEmail+isFormValidgate the submit button, preventing obviously bad input.missingFields+validationErrorgive clear, accessible feedback (role="alert",aria-live="polite").- Button
disabled={isLoading || !isFormValid}matches this and avoids double submissions.This is a nice improvement over relying solely on server-side validation.
Also applies to: 65-71, 213-216
80-103: Sign-up Turnstile challenge handling looks correctThe sign-up flow:
- Calls
handleSignUpViaEmail(...).- If
isPendingTurnstileChallenge(result), stores the challenge and flips offisLoading.- If
result?.success, directly advances to verification.This cleanly separates the Turnstile step while keeping existing behavior for non-challenged users. The catch block correctly maps thrown errors to
AuthErrorCodewhen possible and falls back toUNKNOWN_ERROR.No change needed here.
126-147: Turnstile error and retry UX are clearWhen a Turnstile error occurs:
- You clear the challenge and show a toast (“Verification failed. Please try again.”).
- You present a “Try different information” button to restart with a fresh form.
This gives users a straightforward escape hatch from a failed challenge without leaving them stuck.
Looks good.
apps/dashboard/lib/auth/base-provider.ts (2)
56-62:bypassRadarwiring in email flows is appropriate—ensure all providers implement itAdding optional
bypassRadar?: booleantosignInViaEmailandsignUpViaEmailmatches the Turnstile retry flow, where you only set it after a successful human verification.Please double-check that all concrete providers (
workos,local, etc.):
- Update their method signatures to match this abstract class.
- Respect
bypassRadaronly in the intended contexts (i.e., skip Radar checks when it’s true, not by default).This will keep TS happy and ensure consistent behavior across providers.
Also applies to: 100-106
282-310: Removingconsole.errorfromhandleErroris fine; response semantics unchanged
handleErrorstill:
- Maps known
AuthErrorCodemessages to the correspondingerrorMessagesentry.- Falls back to an
UNKNOWN_ERRORwith either the originalerror.messageor the generic message.Dropping
console.errorkeeps logs cleaner and avoids noisy server output, especially for expected auth failures. As long as provider implementations log unexpected errors where appropriate, this is a good simplification.apps/dashboard/app/auth/sign-in/email-signin.tsx (2)
14-31: Email validation, form handling, and Turnstile challenge branching look good
currentEmail+isValidEmail→isFormValidcleanly control the disabled state of the submit button.handleSubmitstill usesFormDatafor the actual email value, which is fine and consistent.- When a Turnstile challenge is returned, you store it, drop the loading spinner, and early-return, so the user clearly transitions into the challenge step.
This is a solid integration that doesn’t disturb the existing LastUsed behavior.
Also applies to: 32-56
100-122: Disabled button styling and loading state behavior remain consistent
- The new button class adds disabled styling (
opacity-50,disabled:cursor-not-allowed, etc.), which matches how sign-up behaves.disabled={isLoading || !isFormValid}ensures we don’t send extra requests and don’t even try with obviously invalid emails.- The
clientReady && isLoadingcheck retains the pre-hydration safeguard for showing the spinner.No issues here.
apps/dashboard/app/auth/hooks/useSignUp.ts (2)
22-31: Type guard and returned API surface for Turnstile challenges look consistent
isPendingTurnstileChallengecorrectly narrowsEmailAuthResulttoPendingTurnstileResponseby checkingsuccess,code === RADAR_CHALLENGE_REQUIRED, and the presence ofchallengeParams. Exposing it (plushandleTurnstileVerification) from the hook gives the UI a clean way to branch on the Turnstile path, and the return type ofEmailAuthResultmatches the union defined in@/lib/auth/types.Also applies to: 102-112
65-79: Code and email verification handlers match server expectations and validate inputs
handleCodeVerification’s check thatuserData.emailexists before callingverifyAuthCode({ email, code, invitationToken })aligns with the WorkOS provider’sverifyAuthCodesignature (apps/dashboard/app/auth/workos.ts:713-769).handleEmailVerificationsimply delegating toverifyEmail(code)keeps the client-side surface minimal while letting the server action pull the pending token from cookies as needed. No issues here.Also applies to: 81-83
apps/dashboard/lib/auth/workos.ts (1)
104-141: Radar + Turnstile integration andbypassRadarflow are coherent and match prior design
checkRadarstill:
- Returns
{ action: "allow" }on non‑OK HTTP responses or exceptions, and- Defaults to
"block"whendata.verdictis falsy viadata.verdict || "block",
which preserves the fail‑open vs fail‑closed behavior described in the earlier learning for Radar. Returning aDecisionobject withreasonattached is a nice improvement for observability. Based on learnings, this keeps the intended safety profile intact.In
signUpViaEmailandsignInViaEmail:
bypassRadar?: booleanis only honored to skip Radar once the client has gone through the Turnstile flow; initial calls hit Radar as before.- On
radarDecision.action === "challenge", both methods now return a structuredRADAR_CHALLENGE_REQUIREDresponse withchallengeParams { ipAddress, userAgent, authMethod, action }, matchingPendingTurnstileResponse(apps/dashboard/lib/auth/types.ts:93-102).- On
radarDecision.action === "block", they still hard‑fail withAuthErrorCode.RADAR_BLOCKED, keeping blocked requests distinct from challenge flows.- After Turnstile verification,
verifyTurnstileAndRetrycalls back intosignUpViaEmail/signInViaEmailwithbypassRadar: true(apps/dashboard/app/auth/actions.ts:443-488), so we don’t loop on Radar.Overall this wiring correctly gates OTP sending on either Radar “allow” or a successful Turnstile challenge.
Also applies to: 578-647, 649-689
mcstepp
left a comment
mcstepp
left a comment
There was a problem hiding this comment.
fine, remove all the error logging, idc 😭
looks good, i tested sign-up with VPN and playwright to trigger some challenges.
|
|
Graphite Automations"Post a GIF when PR approved" took an action on this PR • (11/24/25)1 gif was posted to this PR based on Andreas Thomas's automation. |
3 participants
What does this PR do?
This commit implements CAPTCHA protection using Cloudflare Turnstile across all authentication flows that use Radar to allow "Challenged" requests to show they are indeed a human before sending the OTP.
Type of change
How should this be tested?
Testing is a bit of a challenge. I have been testing this by:
I was unable to trigger sign in because Radar would approve it due to the nature of cred stuffing being the motivator for radar blocks, and we don't use passwords.
Checklist
Required
pnpm buildpnpm fmtmake fmton/godirectoryconsole.logsgit pull origin mainAppreciated