♻️ Move auth #1422
♻️ Move auth #1422
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
|
Warning Rate limit exceeded@lukevella has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 19 minutes and 17 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughThe changes in this pull request primarily involve modifications to the authentication logic within the Changes
Possibly related PRs
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: 1
🧹 Outside diff range and nitpick comments (3)
apps/web/src/pages/api/trpc/[trpc].ts (1)
Line range hint
20-39: Consider documenting the authentication flow.The
createContextfunction plays a critical role in API authentication. Consider adding a brief comment explaining the session validation flow and guest user handling.createContext: async (opts) => { + // Validate user session and handle both registered and guest users + // Guest users are identified by null email in the session const session = await getServerSession(opts.req, opts.res); if (!session) {apps/web/src/auth.ts (2)
Line range hint
191-196: Remove duplicate locale assignment.The
session.user.localeis assigned twice in this code block.session.user.id = token.sub as string; session.user.locale = token.locale; session.user.timeFormat = token.timeFormat; session.user.timeZone = token.timeZone; - session.user.locale = token.locale; session.user.weekStart = token.weekStart;
Line range hint
198-234: Add error handling and simplify session logic.The database query and subsequent logic could benefit from:
- Error handling for the database operation
- Simplified assignment of user properties
Consider refactoring to:
async session({ session, token }) { if (token.sub?.startsWith("user-")) { return { ...session, user: { ...session.user, id: token.sub, locale: token.locale, timeFormat: token.timeFormat, timeZone: token.timeZone, weekStart: token.weekStart, }, }; } try { const user = await prisma.user.findUnique({ where: { id: token.sub as string }, select: { id: true, name: true, timeFormat: true, timeZone: true, locale: true, weekStart: true, email: true, image: true, }, }); const source = user ?? token; return { ...session, user: { ...session.user, id: user?.id ?? `user-${randomid()}`, name: user?.name ?? session.user.name, email: user?.email ?? session.user.email, image: user?.image ?? session.user.image, locale: source.locale, timeFormat: source.timeFormat, timeZone: source.timeZone, weekStart: source.weekStart, }, }; } catch (error) { console.error('Failed to fetch user data:', error); // Fallback to token data return { ...session, user: { ...session.user, id: token.sub || `user-${randomid()}`, locale: token.locale, timeFormat: token.timeFormat, timeZone: token.timeZone, weekStart: token.weekStart, }, }; } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (7)
apps/web/src/auth.ts(1 hunks)apps/web/src/pages/[locale]/auth/disable-notifications.tsx(1 hunks)apps/web/src/pages/api/auth/[...nextauth].ts(1 hunks)apps/web/src/pages/api/stripe/checkout.ts(1 hunks)apps/web/src/pages/api/stripe/portal.ts(1 hunks)apps/web/src/pages/api/trpc/[trpc].ts(1 hunks)apps/web/src/trpc/routers/auth.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- apps/web/src/pages/api/stripe/portal.ts
🔇 Additional comments (11)
apps/web/src/pages/api/auth/[...nextauth].ts (1)
4-4: LGTM! Import path change aligns with module restructuring.
The change to import from "@/auth" is consistent with the broader authentication module restructuring effort.
apps/web/src/pages/[locale]/auth/disable-notifications.tsx (1)
5-5: LGTM! Verify the new import path.
The import path change aligns with the PR objective of moving auth-related code and standardizing imports across files.
Let's verify the new import path and its interface:
✅ Verification successful
Let's verify the actual implementation of getServerSession in the found files.
Import path change is valid and correctly implemented
The verification confirms that:
- The new path
@/authexists atapps/web/src/auth.ts - It properly exports
getServerSessionwith the same interface - No other files are still using the old import path
@/utils/auth - The implementation maintains compatibility by using
next-authunder the hood
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the new auth module exports getServerSession
# and check for any potential breaking changes in the interface
# Test 1: Check if the new auth file exists
echo "Checking for the new auth file..."
fd --type f "auth.ts$" --exec echo "Found: {}"
# Test 2: Verify the exported function in the new location
echo -e "\nVerifying getServerSession export..."
ast-grep --pattern 'export { getServerSession }'
Length of output: 338
Script:
#!/bin/bash
# Check the content of auth files to verify getServerSession implementation
echo "Checking apps/web/src/auth.ts:"
rg -A 5 "getServerSession" "apps/web/src/auth.ts"
echo -e "\nChecking apps/web/src/trpc/routers/auth.ts:"
rg -A 5 "getServerSession" "apps/web/src/trpc/routers/auth.ts"
# Also check if there are any other files importing getServerSession from the old path
echo -e "\nChecking for imports from old path:"
rg "from ['\"]@/utils/auth['\"]" --type ts
Length of output: 1269
apps/web/src/pages/api/trpc/[trpc].ts (1)
7-7: Verify the new import path exists.
The import path change from "@/utils/auth" to "@/auth" appears to be part of a larger authentication module reorganization.
Let's verify the new import path exists:
✅ Verification successful
Let me generate another verification script to find the actual export of getServerSession since the previous results show the implementation but not the re-export.
Let me try one more search with a different approach to locate the export:
Based on the results, we can see that getServerSession is exported from ./apps/web/src/auth.ts, which matches the new import path @/auth (where @ is an alias for src). The previous results also showed us the implementation of getServerSession in this file. Therefore, I can now generate the final response:
The new import path is correct and verified
The getServerSession function is properly exported from @/auth (apps/web/src/auth.ts), confirming that the import path change is valid.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the new auth module exists and contains getServerSession
# Expected: Find the new location of getServerSession export
# Test 1: Look for the auth module
fd -t f "auth.ts" -X cat
# Test 2: Look for getServerSession export
ast-grep --pattern 'export { getServerSession }'
Length of output: 13004
Script:
#!/bin/bash
# Look for the re-export of getServerSession in auth/index.ts or similar files
fd -t f "index.ts" -p "auth" -X cat
# Also search for any direct exports of getServerSession
ast-grep --pattern 'export async function getServerSession'
Length of output: 187
Script:
#!/bin/bash
# Look for files that might contain the getServerSession export
fd -t f -e ts -X grep -l "export.*getServerSession" {}
# Look for auth-related files
fd -t f -e ts auth
Length of output: 277
apps/web/src/pages/api/stripe/checkout.ts (2)
Line range hint 1-116: Implementation is secure and well-structured.
The code demonstrates good practices:
- Strong input validation using Zod
- Proper session handling and authentication checks
- Secure redirect handling
- Comprehensive error management
- Well-structured Stripe integration with proper metadata
7-7: LGTM! Import path update aligns with auth module restructuring.
The change from @/utils/auth to @/auth is part of the broader authentication module reorganization.
Let's verify the auth module's availability and consistent usage:
✅ Verification successful
Import path change is correctly implemented and consistently used
The verification confirms:
- The auth module exists at
apps/web/src/auth.ts - No remaining imports from the old path
@/utils/auth - The new import path
@/authis consistently used across 6 files - The
getServerSessionfunction is imported from the new path in multiple API routes
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the auth module's existence and consistent import usage
# Test 1: Check if the auth module exists
fd --type f "auth.(ts|tsx|js|jsx)" --exec echo "Found auth module: {}"
# Test 2: Verify consistent import usage across the codebase
# Expect: No remaining imports from @/utils/auth
rg "from ['\"]@/utils/auth['\"]"
# Test 3: Verify new import pattern usage
rg "from ['\"]@/auth['\"]"
Length of output: 751
apps/web/src/trpc/routers/auth.ts (2)
5-5: LGTM! Import path update aligns with auth restructuring.
The change to import from "@/auth" is consistent with the broader standardization of auth-related imports across the codebase.
Line range hint 31-33: Verify the optional email blocking behavior.
The addition of optional chaining (?.) means the email blocking check will be skipped if isEmailBlocked is undefined. This could potentially allow registration of blocked emails if the function is not properly initialized.
Let's verify the implementation and usage of this function:
apps/web/src/auth.ts (4)
18-19: LGTM! Good architectural organization.
The move of auth-related utilities to a dedicated "@/auth" directory improves code organization and maintainability.
Line range hint 236-245: LGTM! Clean abstractions for session and auth route handling.
The exported functions provide well-typed wrappers around NextAuth functionality, making the auth implementation more maintainable and type-safe.
Line range hint 146-149: Verify JWT secret configuration in production.
The JWT configuration uses SECRET_PASSWORD from environment variables. Ensure that:
- A strong, unique secret is used in production
- The secret is properly rotated
- The secret is at least 32 characters long
Let's check the secret's usage across the codebase:
#!/bin/bash
# Search for SECRET_PASSWORD usage
rg "SECRET_PASSWORD" --type ts -C 2Line range hint 64-75: Consider adding rate limiting for guest user creation.
While the guest provider implementation is clean, it could benefit from additional security measures to prevent abuse. Consider:
- Adding rate limiting per IP
- Implementing a maximum limit for guest accounts
- Adding request validation middleware
Let's check if rate limiting is implemented elsewhere:
| res.status(200).end(); | ||
| res.setHeader('Content-Length', '0'); | ||
| res.setHeader("Content-Length", "0"); |
There was a problem hiding this comment.
Fix response header setting order.
The setHeader call occurs after end(), but headers cannot be modified after the response has been ended. This could lead to runtime errors.
Apply this fix:
if (req.method === "HEAD") {
+ res.setHeader("Content-Length", "0");
res.status(200).end();
- res.setHeader("Content-Length", "0");
} else {📝 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.
| res.status(200).end(); | |
| res.setHeader('Content-Length', '0'); | |
| res.setHeader("Content-Length", "0"); | |
| res.setHeader("Content-Length", "0"); | |
| res.status(200).end(); |
Summary by CodeRabbit
New Features
CredentialsProviderfor managing guest user authentication, generating unique IDs for each guest.Bug Fixes
Documentation
Chores