♻️ Upgrade to next-auth v5 #1547
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
WalkthroughThis update revises the authentication flow across the application. It updates the NextAuth declarations by adding a required Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant LC as Login Component
participant NA as NextAuth (auth function)
U->>LC: Enter credentials and submit
LC->>NA: call signIn(redirectTo)
NA->>LC: Return authentication result
LC->>U: Redirect based on result
sequenceDiagram
participant U as User
participant AR as API Route
participant PH as PostHog Wrapper
U->>AR: Send authentication API request
AR->>PH: withPosthog(handler) wraps auth()
PH->>AR: Invoke handler (auth())
AR->>U: Return response
PH->>PH: Flush PostHog events
Possibly related PRs
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 15
🔭 Outside diff range comments (2)
apps/web/src/auth.ts (1)
198-200: Ensure compatibility ofreplaceAllmethod and fix regex inisEmailBlocked.The use of
replaceAllat line 200 may not be supported in all environments, as it was introduced in ES2021. Consider usingreplaceinstead. Additionally, the regex/[*]/gcan be simplified to/\*/g.Apply this diff to fix the method and regex:
- .replace(/[.+?^${}()|[\]\\]/g, "\\$&") - .replaceAll(/[*]/g, ".*") + .replace(/[.+?^${}()|[\]\\]/g, "\\$&") + .replace(/\*/g, ".*")apps/web/src/app/api/user/verify-email-change/route.ts (1)
57-57: Update URL parameter fromcallbackUrltoredirectTo.Next-auth v5 uses
redirectToinstead ofcallbackUrlfor redirection parameters.- new URL(`/login?callbackUrl=${request.url}`, request.url), + new URL(`/login?redirectTo=${request.url}`, request.url),
🧹 Nitpick comments (16)
apps/web/src/auth.ts (1)
192-210: Avoid code duplication by importingisEmailBlockedfrom a shared module.The
isEmailBlockedfunction is defined here but is also imported in other files (e.g., innext-auth.ts). To maintain consistency and reduce duplication, consider movingisEmailBlockedto a shared module and importing it where needed.apps/web/src/auth/providers/guest.ts (1)
8-13: Consider adding rate limiting for guest authentication.While the implementation is functional, it could benefit from rate limiting to prevent abuse.
Consider implementing rate limiting using a library like
express-rate-limitor similar:async authorize() { + // Add rate limiting logic here + const rateLimit = await checkRateLimit(req.ip); + if (!rateLimit.success) { + throw new Error('Too many guest accounts created. Please try again later.'); + } return { id: `user-${randomid()}`, email: null, }; },apps/web/src/auth/get-optional-providers.ts (1)
7-11: LGTM! Consider documenting provider order.The implementation effectively aggregates optional providers and filters out undefined ones. Consider adding a comment explaining if the order of providers is significant for UI presentation.
export function getOptionalProviders() { + // Note: Provider order determines the order of login buttons in the UI return [OIDCProvider(), GoogleProvider(), MicrosoftProvider()].filter( Boolean, ) as Provider[]; }apps/web/src/auth/providers/microsoft.ts (1)
3-17: Consider adding error logging for missing configuration.The function correctly checks for required environment variables but silently returns undefined when configuration is missing. Consider adding error logging to help with debugging configuration issues.
export function MicrosoftProvider() { + const missingVars = [ + ["MICROSOFT_TENANT_ID", process.env.MICROSOFT_TENANT_ID], + ["MICROSOFT_CLIENT_ID", process.env.MICROSOFT_CLIENT_ID], + ["MICROSOFT_CLIENT_SECRET", process.env.MICROSOFT_CLIENT_SECRET], + ].filter(([_, value]) => !value).map(([name]) => name); + + if (missingVars.length > 0) { + console.warn( + `Microsoft provider disabled due to missing environment variables: ${missingVars.join(", ")}`, + ); + return; + } + if ( process.env.MICROSOFT_TENANT_ID && process.env.MICROSOFT_CLIENT_ID && process.env.MICROSOFT_CLIENT_SECRET ) {apps/web/src/auth/is-email-blocked.ts (1)
1-19: Add input validation and optimize performance.The implementation correctly handles email patterns but could benefit from the following improvements:
- Input validation for the email parameter
- Caching of compiled regex patterns
- Memoization of environment variable parsing
+const getAllowedEmailPatterns = (() => { + let patterns: RegExp[] | undefined; + return () => { + if (!patterns && process.env.ALLOWED_EMAILS) { + patterns = process.env.ALLOWED_EMAILS.split(",").map( + (allowedEmail) => + new RegExp( + `^${allowedEmail + .replace(/[.+?^${}()|[\]\\]/g, "\\$&") + .replaceAll(/[*]/g, ".*")}$`, + ), + ); + } + return patterns; + }; +})(); + export const isEmailBlocked = (email: string) => { + if (!email || typeof email !== "string") { + return true; + } + if (process.env.ALLOWED_EMAILS) { - const allowedEmails = process.env.ALLOWED_EMAILS.split(","); - // Check whether the email matches enough of the patterns specified in ALLOWED_EMAILS - const isAllowed = allowedEmails.some((allowedEmail) => { - const regex = new RegExp( - `^${allowedEmail - .replace(/[.+?^${}()|[\]\\]/g, "\\$&") - .replaceAll(/[*]/g, ".*")}$`, - ); - return regex.test(email); - }); + const patterns = getAllowedEmailPatterns(); + const isAllowed = patterns?.some((pattern) => pattern.test(email)) ?? false; if (!isAllowed) { return true; } } return false; };apps/web/src/app/[locale]/(auth)/login/components/login-with-oidc.tsx (1)
8-12: Consider renaming the prop for consistency.The prop name
callbackUrlis inconsistent with the newredirectToparameter used in thesignInoptions. Consider renaming the prop to match the new parameter name for better maintainability.export async function LoginWithOIDC({ name, - callbackUrl, + redirectTo, }: { name: string; - callbackUrl?: string; + redirectTo?: string; }) { return ( <Button onClick={() => { signIn("oidc", { - redirectTo: callbackUrl, + redirectTo, });Also applies to: 18-18
packages/posthog/src/server/index.ts (1)
18-24: Consider handling null posthog client explicitly.The
posthogApiHandlerfunction should handle the case whereposthogis null more explicitly to avoid unnecessarywaitUntilcalls.export function posthogApiHandler() { + if (!posthog) return; try { - waitUntil(Promise.all([posthog?.shutdown()])); + waitUntil(Promise.all([posthog.shutdown()])); } catch (error) { console.error("Failed to flush PostHog events:", error); } }apps/web/src/auth/providers/email.ts (2)
12-14: Add return type to generateVerificationToken.For better type safety and documentation, consider adding an explicit return type to the function.
- generateVerificationToken() { + generateVerificationToken(): string { return generateOtp(); },
8-10: Document the "none" server value.The comment about the required "none" value could be more descriptive to explain why it's needed and what it does.
export const EmailProvider = NodemailerProvider({ - server: "none", // This value is required even though we don't need it + server: "none", // Required by NodemailerProvider but not used since we handle email sending ourselvesapps/web/src/trpc/server/create-ssr-helper.ts (1)
39-41: Consider adding error logging before redirect.Error information might be useful for debugging authentication issues.
} catch (error) { + console.error("SSR helper creation failed:", error); return redirect("/login"); }apps/web/src/app/[locale]/layout.tsx (1)
33-33: Consider adding error boundary for auth failures.While the migration to
auth()is correct, adding error handling would improve reliability.- const session = await auth(); + try { + const session = await auth(); + } catch (error) { + console.error("Authentication failed:", error); + // Return a minimal session state or redirect as needed + }apps/web/src/auth/providers/registration-token.ts (1)
39-42: Add logging for failed registration attempts.For security monitoring, consider logging failed registration attempts.
if (user) { return user; +} else { + console.warn(`Failed registration attempt for email: ${payload.email}`); }apps/web/src/auth/adapters/prisma.ts (2)
13-13: Consider keeping the client parameter for better flexibility.While using a direct prisma import simplifies the code, it reduces flexibility for testing and dependency injection. Consider keeping the client parameter or using a dependency injection pattern.
-import { prisma } from "@rallly/database"; +import type { ExtendedPrismaClient } from "@rallly/database"; -export function CustomPrismaAdapter(options: { +export function CustomPrismaAdapter( + client: ExtendedPrismaClient, + options: { migrateData: (userId: string) => Promise<void>; -}) { +}) { - const adapter = PrismaAdapter(prisma); + const adapter = PrismaAdapter(client);Also applies to: 14-14, 16-20
40-40: Usesatisfiesinstead ofastype assertion.Using
satisfiesprovides better type safety thanasby validating the object shape at compile time.- } as Adapter; + } satisfies Adapter;apps/web/src/app/[locale]/(auth)/login/page.tsx (1)
72-73: Add fallback for provider name.Consider adding a more robust fallback for provider name to handle undefined options.
- name={provider.options?.name || provider.name} + name={provider.options?.name ?? provider.name ?? provider.id}apps/web/src/app/[locale]/(admin)/settings/profile/delete-account-dialog.tsx (1)
41-41: Consider making the redirect path configurable.While the parameter rename from
callbackUrltoredirectTois correct for next-auth v5, consider making the redirect path configurable rather than hardcoding it to "/login". This would make the component more reusable and maintainable.- redirectTo: "/login", + redirectTo: process.env.NEXT_PUBLIC_AUTH_REDIRECT_PATH ?? "/login",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (31)
apps/web/declarations/next-auth.d.ts(1 hunks)apps/web/package.json(2 hunks)apps/web/src/app/[locale]/(admin)/settings/profile/delete-account-dialog.tsx(1 hunks)apps/web/src/app/[locale]/(auth)/login/components/login-email-form.tsx(1 hunks)apps/web/src/app/[locale]/(auth)/login/components/login-with-oidc.tsx(1 hunks)apps/web/src/app/[locale]/(auth)/login/components/sso-provider.tsx(2 hunks)apps/web/src/app/[locale]/(auth)/login/page.tsx(3 hunks)apps/web/src/app/[locale]/layout.tsx(2 hunks)apps/web/src/app/api/auth/[...nextauth]/route.ts(1 hunks)apps/web/src/app/api/notifications/unsubscribe/route.ts(2 hunks)apps/web/src/app/api/stripe/checkout/route.ts(2 hunks)apps/web/src/app/api/stripe/portal/route.ts(2 hunks)apps/web/src/app/api/trpc/[trpc]/route.ts(2 hunks)apps/web/src/app/api/user/verify-email-change/route.ts(2 hunks)apps/web/src/auth.ts(3 hunks)apps/web/src/auth/adapters/prisma.ts(2 hunks)apps/web/src/auth/get-optional-providers.ts(1 hunks)apps/web/src/auth/is-email-blocked.ts(1 hunks)apps/web/src/auth/providers/email.ts(1 hunks)apps/web/src/auth/providers/google.ts(1 hunks)apps/web/src/auth/providers/guest.ts(1 hunks)apps/web/src/auth/providers/microsoft.ts(1 hunks)apps/web/src/auth/providers/oidc.ts(1 hunks)apps/web/src/auth/providers/registration-token.ts(1 hunks)apps/web/src/features/quick-create/lib/get-guest-polls.ts(1 hunks)apps/web/src/middleware.ts(2 hunks)apps/web/src/next-auth.confg.ts(1 hunks)apps/web/src/next-auth.ts(1 hunks)apps/web/src/pages/api/auth/[...nextauth].ts(0 hunks)apps/web/src/trpc/server/create-ssr-helper.ts(1 hunks)packages/posthog/src/server/index.ts(2 hunks)
💤 Files with no reviewable changes (1)
- apps/web/src/pages/api/auth/[...nextauth].ts
🧰 Additional context used
📓 Learnings (1)
apps/web/src/auth/is-email-blocked.ts (1)
Learnt from: lukevella
PR: lukevella/rallly#1189
File: apps/web/src/utils/auth.ts:202-202
Timestamp: 2024-11-12T09:08:14.221Z
Learning: The `isEmailBlocked()` function in `apps/web/src/utils/auth.ts` is not asynchronous.
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Unit tests
- GitHub Check: Integration tests
- GitHub Check: Type check
- GitHub Check: Linting
🔇 Additional comments (14)
apps/web/src/middleware.ts (1)
25-68: Middleware logic correctly updated to useauthfrom NextAuth v5.The middleware function has been appropriately updated to use
authinstead ofwithAuth, aligning with NextAuth v5 changes. The authentication flow and user data handling are correctly implemented.apps/web/src/app/api/auth/[...nextauth]/route.ts (1)
1-6: LGTM! Good integration of analytics with auth handlers.The implementation correctly wraps the NextAuth handlers with PostHog analytics while maintaining the core authentication functionality.
apps/web/src/features/quick-create/lib/get-guest-polls.ts (1)
3-3: LGTM! Correctly updated to next-auth v5.The changes correctly update the session retrieval method to use the new
auth()function from next-auth v5.Also applies to: 6-6
apps/web/declarations/next-auth.d.ts (1)
22-24: LGTM!The addition of the required
idfield to theUserinterface is correct and aligns with next-auth v5 requirements. The change maintains consistency with the existingSessioninterface which already includes theidfield.apps/web/src/auth/providers/oidc.ts (1)
23-23:⚠️ Potential issueReview security implications of
allowDangerousEmailAccountLinking.Setting
allowDangerousEmailAccountLinking: truecould potentially lead to account hijacking if the OIDC provider's email verification process is not robust.Consider implementing additional safeguards or documenting the security implications in the codebase.
apps/web/src/trpc/server/create-ssr-helper.ts (1)
12-12: LGTM! Correct usage of next-auth v5'sauth()function.The migration from
getServerSession()toauth()is implemented correctly.apps/web/src/app/api/trpc/[trpc]/route.ts (1)
7-7: LGTM! Correctly migrated to next-auth v5.The changes follow the next-auth v5 upgrade guide by replacing
getServerSessionwith the newauth()function.Let's verify that we're using the latest version of next-auth:
Also applies to: 18-18
✅ Verification successful
🌐 Web query:
What is the latest version of next-auth/auth.js?💡 Result:
NextAuth.js (now part of Auth.js) has two relevant versions as of February 2025: 1. **Latest stable release**: `v4.24.11` - Published December 6, 2024[9][10] 2. **Upcoming major release**: `v5.x-beta` - Currently in public beta testing with features like: - App Router-first architecture[4][6] - Edge compatibility[4] - Simplified environment variable handling (`AUTH_*` prefix)[6] - Unified `auth()` method replacing older APIs[4][6] To install the beta: ```bash npm install next-auth@beta ``` The Auth.js project (encompassing NextAuth.js) now uses semantic versioning where: - `next-auth` refers specifically to Next.js integrations[2][8] - All framework integrations share a common `@auth/core` library[2][8] Key changes in v5 include stricter OAuth compliance and dropping support for Next.js <14[4]. Database schemas remain compatible with v4[4]. Citations: - 1: https://next-auth.js.org/getting-started/upgrade-v4 - 2: https://authjs.dev/contributors - 3: https://www.npmjs.com/package/next-auth/v/5.0.0-beta.19 - 4: https://authjs.dev/getting-started/migrating-to-v5 - 5: https://www.npmjs.com/package/next-auth?activeTab=versions - 6: https://authjs.dev/reference/nextjs - 7: https://next-auth.js.org/v3/contributors - 8: https://next-auth.js.org - 9: https://www.npmjs.com/package/next-auth - 10: https://github.com/nextauthjs/next-auth/actions/runs/9997732960
Verified: Migration to Next-Auth v5 is Correct
The web query confirms that next-auth has released a v5.x-beta version featuring the unifiedauth()function in place ofgetServerSession. The code's import ofauthfrom"@/next-auth"aligns with the migration guidelines for next-auth v5. Make sure the dependency in your package.json is set tonext-auth@betaas required.apps/web/src/app/api/user/verify-email-change/route.ts (1)
6-6: LGTM! Correctly migrated to next-auth v5.The changes follow the next-auth v5 upgrade guide by replacing
getServerSessionwith the newauth()function.Also applies to: 53-53
apps/web/src/app/api/notifications/unsubscribe/route.ts (1)
6-6: LGTM! Correctly migrated to next-auth v5.The changes follow the next-auth v5 upgrade guide by replacing
getServerSessionwith the newauth()function.Also applies to: 17-17
apps/web/src/app/[locale]/(auth)/login/components/sso-provider.tsx (2)
18-18: LGTM! Provider name updated to reflect Microsoft's rebranding.The change from "azure-ad" to "microsoft-entra-id" aligns with Microsoft's rebranding of Azure Active Directory to Microsoft Entra ID.
61-61: Verify the parameter rename across the codebase.The change from
callbackUrltoredirectTois part of the next-auth v5 upgrade. Let's verify this change is consistently applied across the codebase.✅ Verification successful
🏁 Script executed:
#!/bin/bash # Search for any remaining instances of callbackUrl in authentication flows rg -g '*.{ts,tsx}' 'callbackUrl.*signIn'Length of output: 42
Let's run an additional search for any remaining uses of the string
"callbackUrl"in our TypeScript/TSX files, not just in connection withsignIn. This helps us ensure that the parameter rename to"redirectTo"is consistently applied. For example, we can search for occurrences with object notation (using"callbackUrl:") and also any loose occurrences.Once we have the results from the above search, we can confirm if any instances remain or if the rename is consistently applied.
🏁 Script executed:
#!/bin/bash echo "Searching for 'callbackUrl:' usage:" rg -g '*.{ts,tsx}' 'callbackUrl:' echo "-----------------------------" echo "Searching for any occurrence of 'callbackUrl':" rg -g '*.{ts,tsx}' 'callbackUrl'Length of output: 2521
Parameter rename verified across signIn flows.
The search results confirm that all areas where the next-auth parameter is passed to signIn now use the newredirectToproperty (e.g., insso-provider.tsx,login-email-form.tsx, andlogin-with-oidc.tsx). Remaining instances of"callbackUrl"are used to read query parameters or construct URLs, which is the expected behavior.apps/web/src/app/api/stripe/portal/route.ts (1)
8-8: LGTM! Updated to next-auth v5's session handling.The change from
getServerSessiontoauth()aligns with next-auth v5's new session handling approach. Error handling remains robust.Also applies to: 35-35
apps/web/src/app/api/stripe/checkout/route.ts (1)
8-8: LGTM! Consistent update to next-auth v5's session handling.The change from
getServerSessiontoauth()maintains consistency with next-auth v5's new session handling approach.Also applies to: 17-17
apps/web/src/app/[locale]/(auth)/login/components/login-email-form.tsx (1)
56-56: LGTM: Parameter renamed to match next-auth v5 API.The change from
callbackUrltoredirectToaligns with the next-auth v5 API changes.
apps/web/src/app/[locale]/(auth)/login/components/login-email-form.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/web/src/app/[locale]/(auth)/login/actions.ts (1)
7-14: Simplify the database query by removing unnecessary select clause.Since the email is already known from the input parameter, selecting it again from the database is redundant.
const user = await prisma.user.findUnique({ where: { email, }, - select: { - email: true, - }, });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (8)
apps/web/src/app/[locale]/(auth)/login/actions.ts(1 hunks)apps/web/src/auth.ts(0 hunks)apps/web/src/auth/providers/registration-token.ts(1 hunks)apps/web/src/middleware.ts(2 hunks)apps/web/src/next-auth.config.ts(1 hunks)apps/web/src/next-auth.ts(1 hunks)apps/web/src/trpc/routers/auth.ts(1 hunks)package.json(1 hunks)
💤 Files with no reviewable changes (1)
- apps/web/src/auth.ts
✅ Files skipped from review due to trivial changes (1)
- apps/web/src/trpc/routers/auth.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/web/src/auth/providers/registration-token.ts
- apps/web/src/next-auth.ts
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Linting
- GitHub Check: Integration tests
- GitHub Check: Type check
🔇 Additional comments (13)
apps/web/src/middleware.ts (6)
4-9: LGTM! But fix the typo in the import path.The initialization follows the next-auth v5 pattern correctly.
Apply this diff to correct the import statement:
- import { nextAuthConfig } from "@/next-auth.confg"; + import { nextAuthConfig } from "@/next-auth.config";
25-28: LGTM! Correctly updated to next-auth v5 middleware pattern.The middleware function signature has been properly updated to use the new next-auth v5 pattern.
29-35: LGTM! Correctly updated session access pattern.The session access has been properly updated to use
req.auth?.user?.emailinstead of the deprecatedreq.nextauth.token?.email.
37-47: Update redirection parameter toredirectTo.According to the PR objectives, redirection parameters are being renamed from
callbackUrltoredirectToin sign-in/sign-out flows.Apply this diff to update the parameter:
- newUrl.searchParams.set("callbackUrl", newUrl.pathname); + newUrl.searchParams.set("redirectTo", newUrl.pathname);
49-58: LGTM! Correctly updated locale access pattern.The locale access has been properly updated to use
req.auth?.user?.localeinstead of the deprecatedreq.nextauth.token?.locale.
63-65: LGTM! Correctly updated user ID access pattern for PostHog.The user ID access has been properly updated to use
req.auth?.user?.idfor PostHog analytics integration.apps/web/src/app/[locale]/(auth)/login/actions.ts (1)
16-24: LGTM! Secure cookie configuration.The cookie configuration is secure with:
httpOnlyto prevent XSSsecurein productionsameSite: "lax"to prevent CSRF- 15-minute expiration for verification window
package.json (1)
61-62: Verify the empty dependencies object.Since this PR upgrades to next-auth v5, consider whether the dependencies should be defined here or if they are already defined in workspace-specific package.json files.
Let's check the workspace-specific package.json files for the next-auth dependency:
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Description: Check for next-auth dependency in workspace package.json files # Search for next-auth in package.json files fd 'package.json' apps packages -x grep -l 'next-auth' # For each found file, show the next-auth version fd 'package.json' apps packages -x jq -r '.dependencies["next-auth"] // .devDependencies["next-auth"]'Length of output: 257
Workspace packages provide the next-auth dependency.
The shell script output shows that in at least one workspace (e.g.,
apps/web/package.json), the next-auth dependency is defined with version^5.0.0-beta.25. This indicates that the empty"dependencies": {}entry in the root package.json is intentional and correct for managing dependencies in a monorepo setup.apps/web/src/next-auth.config.ts (5)
1-3: LGTM! Proper type imports and environment configuration.The imports are well-structured, ensuring type safety with NextAuthConfig and proper environment variable handling.
9-9: Verify SECRET_PASSWORD environment variable.Ensure that SECRET_PASSWORD is properly configured in your environment variables and has sufficient entropy for production use.
Let me help you verify the environment variable configuration:
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Check for environment variable validation rg "SECRET_PASSWORD" --type tsLength of output: 461
SECRET_PASSWORD environment variable configuration is verified.
- The schema in
apps/web/src/env.tsenforces a minimum length of 32 characters usingz.string().min(32), ensuring adequate entropy.- The variable is consistently referenced across the project (e.g.,
next-auth.config.tsand session configuration).
5-11: Verify the empty providers array implementation.The providers array is currently empty. According to the PR summary, various providers (email, Google, OIDC, Microsoft) should be configured.
Let me help you verify if the providers are defined elsewhere:
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Search for auth provider configurations rg -l "(?i)(email|google|oidc|microsoft)provider" --type tsLength of output: 424
Providers Configuration Verification
The search results confirm that provider configurations (Email, Google, OIDC, Microsoft) are defined separately in files under
apps/web/src/auth/providers/and managed via theget-optional-providers.tsmodule. The empty providers array innext-auth.config.tsappears intentional—likely to delegate provider loading dynamically rather than hardcoding them in the configuration object.
- Providers are defined in separate files (
providers/email.ts,providers/google.ts,providers/oidc.ts,providers/microsoft.ts).- The presence of
apps/web/src/auth/get-optional-providers.tssuggests that providers are incorporated at runtime.- This design aligns with a modular and conditional configuration approach.
6-8: Verify NextAuth v5 session configuration.Let's confirm that the JWT strategy configuration is correct for NextAuth v5.
✅ Verification successful
🌐 Web query:
What are the session strategy options in NextAuth v5? Is JWT the default strategy?💡 Result:
NextAuth.js v5 offers two session strategies: **JSON Web Token (JWT)** and **database sessions**. The default strategy depends on whether you've configured a database adapter: 1. **JWT (default without an adapter)** - Stores session data in an encrypted JWT within a secure `HttpOnly` cookie[1][3][7]. - Automatically selected if no database adapter is configured[1][3][7]. - Supports serverless/edge environments without direct database access[3][7]. 2. **Database sessions (default with an adapter)** - Stores session data in your database and uses a session ID cookie[3][7]. - Requires a configured database adapter (e.g., Prisma, TypeORM)[3][7]. - Automatically enabled when using any database adapter[1][3][7]. ### Key Differences | Feature | JWT Strategy | Database Strategy | |----------------------|---------------------------------------|---------------------------------------| | Session Storage | Encrypted token in cookie | Database record + session ID cookie | | Server Requirements | Stateless (no DB queries) | Requires database connection | | Session Updates | Requires token regeneration | Direct DB updates | | Default Trigger | No adapter configured | Adapter configured | To explicitly set your strategy regardless of adapter configuration: ```javascript // auth.ts export const { auth } = NextAuth({ session: { strategy: "jwt" }, // Force JWT // OR session: { strategy: "database" }, // Force DB // ...other config }) ``` The encrypted JWT approach provides better performance for serverless architectures[3][7], while database sessions offer more control over session management[3][7]. Citations: - 1: https://next-auth.js.org/configuration/options - 2: https://github.com/nextauthjs/next-auth/issues/9122 - 3: https://authjs.dev/concepts/session-strategies - 4: https://github.com/nextauthjs/next-auth/discussions/8487 - 5: https://next-auth.js.org/deployment - 6: https://next-auth.js.org/configuration/nextjs - 7: https://authjs.dev/getting-started/migrating-to-v5 - 8: https://javascript.plainenglish.io/complete-authentication-guide-using-next-auth-v5-in-next-js-14-70e7630ab1c2?gi=f11fac1c6ffd - 9: https://www.youtube.com/watch?v=1MTyCvS05V4 - 10: https://github.com/joe-jngigi/next_auth_V5
JWT session strategy configuration for NextAuth v5 verified.
- NextAuth v5 supports both JWT and database session strategies.
- When no database adapter is configured, JWT is the default.
- Explicitly setting
session: { strategy: "jwt" }ensures the JWT strategy is used.
5-11: Consider additional NextAuth v5 configuration options.The current configuration might be missing some important NextAuth v5 options. Let's verify the required configuration.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/web/src/components/poll/language-selector.tsx(1 hunks)apps/web/src/next-auth.config.ts(1 hunks)apps/web/src/next-auth.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/src/next-auth.ts
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Unit tests
- GitHub Check: Integration tests
- GitHub Check: Linting
- GitHub Check: Type check
🔇 Additional comments (5)
apps/web/src/components/poll/language-selector.tsx (1)
21-21: Verify the button styling change.The removal of
variant="ghost"will change the button's appearance to use the default variant. Please ensure this aligns with the intended UI design.Let me help you verify if this change is consistent with other similar components:
apps/web/src/next-auth.config.ts (4)
7-9: LGTM! JWT strategy is appropriate.Using JWT as the session strategy is a good choice for Next.js applications, especially when dealing with serverless functions.
6-12: Verify provider configuration location.The
providersarray is empty. This is likely intentional if providers are configured elsewhere, but we should verify this to ensure authentication will work correctly.✅ Verification successful
🏁 Script executed:
#!/bin/bash # Search for auth provider configurations fd -t f "provider" apps/web/src/authLength of output: 119
🏁 Script executed:
#!/bin/bash # Display the content of the get-optional-providers file to inspect how providers are configured. cat apps/web/src/auth/get-optional-providers.tsLength of output: 419
Provider configuration verified.
The empty providers array in apps/web/src/next-auth.config.ts is intentional. The authentication providers are defined in apps/web/src/auth/get-optional-providers.ts, which aggregates OIDC, Google, and Microsoft providers. This confirms that the NextAuth configuration is correctly delegating provider setup elsewhere.
1-4: Verify next-auth package version and environment configuration.Let's ensure we're using the correct next-auth v5 package and that the environment variables are properly configured.
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Check package.json for next-auth version jq '.dependencies["next-auth"]' package.json # Check environment variable configuration cat apps/web/src/env.ts | grep -A 5 "SECRET_PASSWORD"Length of output: 601
🏁 Script executed:
#!/bin/bash # Check apps/web/package.json for next-auth dependency/version in both dependencies and devDependencies. jq '.dependencies["next-auth"] // .devDependencies["next-auth"]' apps/web/package.jsonLength of output: 105
NextAuth v5 Dependency and Environment Configuration Verified
- The next-auth dependency in apps/web/package.json is correctly set to "^5.0.0-beta.25", confirming it's using NextAuth v5.
- The SECRET_PASSWORD environment variable is properly defined and type-checked, ensuring it meets the minimum string length requirement.
1-23: Verify NextAuth v5 migration requirements.Let's ensure all required changes for the v5 upgrade are implemented correctly.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
apps/web/src/next-auth.config.ts (1)
30-38:⚠️ Potential issueAdd type safety and validation to session callback.
The current implementation uses unsafe type assertions without validation.
async session({ session, token }) { - session.user.id = token.sub as string; - session.user.email = token.email as string; - session.user.locale = token.locale; - session.user.timeFormat = token.timeFormat; - session.user.timeZone = token.timeZone; - session.user.weekStart = token.weekStart; + if (!token.sub) { + throw new Error("Missing required token.sub"); + } + session.user = { + ...session.user, + id: token.sub, + email: token.email ?? null, + locale: typeof token.locale === "string" ? token.locale : "en", + timeFormat: isTimeFormat(token.timeFormat) ? token.timeFormat : "12h", + timeZone: typeof token.timeZone === "string" ? token.timeZone : "UTC", + weekStart: typeof token.weekStart === "number" ? token.weekStart : 0, + }; return session; },Add this helper function at the top of the file:
function isTimeFormat(value: unknown): value is TimeFormat { return value === "12h" || value === "24h"; }
🧹 Nitpick comments (2)
apps/web/src/next-auth.config.ts (2)
7-17: Add type safety to public routes array.Consider adding a type definition for the routes array to ensure type safety and maintainability.
+type PublicRoute = '/login' | '/register' | '/invite/' | '/poll/' | '/auth/login' | '/quick-create' | '/new'; -const publicRoutes = [ +const publicRoutes: PublicRoute[] = [ "/login", "/register", "/invite/", "/poll/", "/auth/login", ];
39-54: Simplify redirect logic in authorized callback.The redirect URL construction could be simplified and made more maintainable.
async authorized({ request, auth }) { const { nextUrl } = request; const isLoggedIn = !!auth?.user?.email; const isPublicRoute = publicRoutes.some((route) => nextUrl.pathname.startsWith(route), ); if (isLoggedIn || isPublicRoute) { return true; } - const redirectUrl = new URL("/login", request.url); - if (nextUrl.pathname !== "/") { - const redirectPath = nextUrl.pathname + nextUrl.search; - redirectUrl.searchParams.set("redirectTo", redirectPath); - } + const redirectUrl = new URL("/login", request.url); + const redirectPath = nextUrl.pathname === "/" ? "" : nextUrl.pathname + nextUrl.search; + if (redirectPath) { + redirectUrl.searchParams.set("redirectTo", redirectPath); + } return NextResponse.redirect(redirectUrl); },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/web/.env.test(1 hunks)apps/web/declarations/next-auth.d.ts(1 hunks)apps/web/src/next-auth.config.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/declarations/next-auth.d.ts
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Unit tests
- GitHub Check: Linting
- GitHub Check: Type check
- GitHub Check: Integration tests
🔇 Additional comments (3)
apps/web/.env.test (1)
7-7: Update SMTP_HOST for Broader Interface Binding
The change from "localhost" to "0.0.0.0" allows the SMTP service in the test environment to accept connections from any network interface. This is appropriate for testing scenarios where services may run in containerized or multi-host setups. However, please ensure that this setting remains restricted to test environments and is not inadvertently used in production.apps/web/src/next-auth.config.ts (2)
1-6: LGTM!The imports are appropriate and well-organized for the NextAuth v5 configuration.
28-28: Verify authentication providers configuration.The providers array is empty. Ensure this is intentional and that providers are configured elsewhere.
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Search for NextAuth providers configuration rg -l "(?i)auth.*provider" --type tsLength of output: 550
Authentication Providers Configuration Confirmed
The empty providers array in apps/web/src/next-auth.config.ts is intentional. A search of the codebase shows that several NextAuth providers (Google, Microsoft, email, guest, OIDC, etc.) are defined separately under apps/web/src/auth/providers. These modules are likely imported and composed dynamically in the authentication flow (for example via apps/web/src/auth/get-optional-providers.ts or similar mechanisms), so maintaining an empty providers array in the NextAuth configuration is by design.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/web/src/app/[locale]/(auth)/auth/login/components/login-page.tsx (1)
29-47: Consider providing more specific error messages.While the error handling is good, consider differentiating between invalid and expired links to help users better understand the issue.
- setError( - t("loginMagicLinkError", { - defaultValue: - "This link is invalid or expired. Please request a new link.", - }), - ); + const errorType = data.url.includes("expired") ? "expired" : "invalid"; + setError( + t(`loginMagicLink${errorType}Error`, { + defaultValue: + errorType === "expired" + ? "This link has expired. Please request a new link." + : "This link is invalid. Please request a new link.", + }), + );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/web/i18next-scanner.config.js(1 hunks)apps/web/public/locales/en/app.json(1 hunks)apps/web/src/app/[locale]/(auth)/auth/login/components/login-page.tsx(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Unit tests
- GitHub Check: Integration tests
- GitHub Check: Linting
- GitHub Check: Type check
🔇 Additional comments (4)
apps/web/i18next-scanner.config.js (1)
4-4: LGTM! Verify the scope of excluded files.The updated exclusion pattern aligns with the next-auth v5 upgrade. However, let's verify that the wildcard pattern
!src/next-auth*.tsdoesn't accidentally exclude files that should be included in translation.✅ Verification successful
🏁 Script executed:
#!/bin/bash # Description: List all files that match the next-auth*.ts pattern to ensure we're not excluding files that need translation echo "Files that will be excluded from translation scanning:" fd -t f "next-auth.*\.ts$" src/ echo -e "\nAll TypeScript files in src/ for comparison:" fd -t f ".*\.ts$" src/Length of output: 466
I'll re-run the verification shell script using the correct relative path (apps/web/src) to accurately gather the necessary file information. This will help confirm that the wildcard exclusion ("!src/next-auth*.ts") is correctly scoped.
🏁 Script executed:
#!/bin/bash echo "Searching for files matching 'next-auth*.ts' pattern in apps/web/src:" fd -t f "next-auth.*\.ts$" apps/web/src echo -e "\nListing all TypeScript files in apps/web/src for comparison:" fd -t f ".*\.ts$" apps/web/srcLength of output: 4298
LGTM! Wildcard exclusion correctly targets next-auth files.
The updated exclusion pattern excludes only the two next-auth files (apps/web/src/next-auth.config.ts and apps/web/src/next-auth.ts) as expected, ensuring that no additional files are inadvertently omitted from translation scanning.
apps/web/src/app/[locale]/(auth)/auth/login/components/login-page.tsx (2)
7-7: LGTM! Good error handling setup.The addition of error state management and i18n support enhances the user experience by providing localized error messages.
Also applies to: 12-12, 20-21
86-86: LGTM! Good error message styling.The error message follows the design system's conventions using the
text-destructiveclass.apps/web/public/locales/en/app.json (1)
304-305: LGTM! Good localization support.The added keys support the enhanced error handling in the login page and maintain consistency in the localization system.
| const magicLinkFetch = useMutation({ | ||
| mutationFn: async () => { | ||
| const res = await fetch(magicLink); | ||
| const res = await fetch(magicLink + "test"); |
There was a problem hiding this comment.
Remove debugging code.
The "test" suffix appears to be debugging code that shouldn't be in production.
- const res = await fetch(magicLink + "test");
+ const res = await fetch(magicLink);📝 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.
| const res = await fetch(magicLink + "test"); | |
| const res = await fetch(magicLink); |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
apps/web/src/app/[locale]/(auth)/auth/login/components/login-page.tsx (2)
28-47: Consider handling navigation and errors sequentially.The current implementation might lead to a race condition where the error state is set but not displayed due to immediate navigation. Consider restructuring the flow:
onSuccess: async (data) => { - if (!data.url.includes("auth/error")) { + if (data.url.includes("auth/error")) { + setError( + t("loginMagicLinkError", { + defaultValue: + "This link is invalid or expired. Please request a new link.", + }), + ); + return; + } + // if login was successful, update the session const updatedSession = await session.update(); if (updatedSession?.user) { // identify the user in posthog posthog?.identify(updatedSession.user.id, { email: updatedSession.user.email, name: updatedSession.user.name, }); } router.push(data.url); - } else { - setError( - t("loginMagicLinkError", { - defaultValue: - "This link is invalid or expired. Please request a new link.", - }), - ); - } },
86-86: Enhance error message accessibility.Consider adding proper ARIA attributes to improve screen reader support for the error message.
- {error && <p className="text-destructive text-sm">{error}</p>} + {error && ( + <p + className="text-destructive text-sm" + role="alert" + aria-live="polite" + > + {error} + </p> + )}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/web/src/app/[locale]/(auth)/auth/login/components/login-page.tsx(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (4)
- GitHub Check: Unit tests
- GitHub Check: Type check
- GitHub Check: Integration tests
- GitHub Check: Linting
🔇 Additional comments (2)
apps/web/src/app/[locale]/(auth)/auth/login/components/login-page.tsx (2)
7-7: LGTM! Well-structured initialization of error handling.The addition of error state management and internationalization support is well implemented.
Also applies to: 12-12, 20-21
25-26: Verify URL redirection security.Ensure that the magic link URL and subsequent redirections are properly validated to prevent open redirect vulnerabilities.
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Search for URL validation in auth-related files rg -A 5 "magicLink|redirectTo|callbackUrl" --type tsLength of output: 15855
Security Verification: Magic Link URL Handling
After reviewing the code snippets along with the search results, here are our findings:
• The magic link is required to pass through a zod validation—ensuring it is a valid URL. In addition, upstream components (e.g. in apps/web/src/app/[locale]/(auth)/auth/login/page.tsx) parse and validate the magic link parameters before passing the URL into the LoginPage component.
• The LoginPage component itself merely uses the already‑validated magic link by fetching it and then checking backend response details (e.g. verifying the absence of “auth/error” in data.url). Any redirection behavior appears to be handled by upstream flows using safe construction (for example, with encodeURIComponent for any “redirectTo” parameters).
Based on these observations, the current implementation appears to validate and restrict the magic link URL sufficiently against open redirect vulnerabilities.
Summary by CodeRabbit
New Features
EmailProviderfor enhanced email verification processes.OIDCProviderandMicrosoftProviderfor OpenID Connect and Microsoft authentication.RegistrationTokenProviderfor user registration without email verification.Improvements