fix: restrict self-hosted registration#423
Conversation
Entire-Checkpoint: fc272bcc8747
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughAdds shared authentication error mapping and login-page handling for email sign-in failures, URL errors, and OTP callback normalization. Updates server authentication to restrict self-hosted registration based on existing users and team invitations. Wraps the Prisma adapter to perform serialized, transactional self-hosted user creation. Expands unit tests for registration policy, adapter behavior, invite validation, and missing-email rejection. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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 |
Deploying usesend with
|
| Latest commit: |
de12fab
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://d73e5b62.usesend.pages.dev |
| Branch Preview URL: | https://codex-self-hosted-invite-onl.usesend.pages.dev |
![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
🧹 Nitpick comments (2)
apps/web/src/app/login/login-page.tsx (2)
83-187: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffNo test coverage for the new email sign-in error handling.
onEmailSubmit's try/catch branching and the new inlineauthErrorMessagealert aren't covered by any test, unlikeauth-error.ts/auth.tswhich received dedicated unit tests in this PR.As per path instructions, "Add tests when changes impact logic, APIs, or behavior in apps/web".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/app/login/login-page.tsx` around lines 83 - 187, Add tests for the login page’s email sign-in flow, covering successful submission, a result containing an error, and the rejected `signIn` catch path, including status and form-error updates. Also test the rendered inline `authErrorMessage` alert for an authentication error query parameter, following the existing `auth-error.ts` and `auth.ts` testing conventions.Source: Path instructions
93-119: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRedundant fallback and duplicated error copy.
getAuthErrorMessageonly returnsnullwhen its input is falsy; sinceresult?.error ?? "SignInFailed"is always truthy, the?? "Unable to sign in. Please try again."on line 101 is dead code. The same literal is also hardcoded again in thecatchblock (line 115), duplicating whatauth-error.tsalready exports.♻️ Suggested consolidation
- message: - getAuthErrorMessage(result?.error ?? "SignInFailed") ?? - "Unable to sign in. Please try again.", + message: getAuthErrorMessage(result?.error ?? "SignInFailed"),} catch { setEmailStatus("idle"); emailForm.setError( "email", { type: "server", - message: "Unable to sign in. Please try again.", + message: getAuthErrorMessage("SignInFailed"), }, { shouldFocus: true }, ); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/app/login/login-page.tsx` around lines 93 - 119, Update the login error handling around getAuthErrorMessage to remove the unreachable fallback and reuse the existing exported generic authentication error message from auth-error.ts. Apply that shared message in both the result-error branch and the catch block, while preserving the existing server error type and focus behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/web/src/server/auth.ts`:
- Around line 177-208: Update the auth error mapping for the createUser flow so
SelfHostedRegistrationError thrown inside the transaction is recognized under
OAuthCreateAccount and surfaces as AccessDenied with the invitation-required
message. Preserve the advisory-lock race guard and existing error behavior for
unrelated authentication failures.
---
Nitpick comments:
In `@apps/web/src/app/login/login-page.tsx`:
- Around line 83-187: Add tests for the login page’s email sign-in flow,
covering successful submission, a result containing an error, and the rejected
`signIn` catch path, including status and form-error updates. Also test the
rendered inline `authErrorMessage` alert for an authentication error query
parameter, following the existing `auth-error.ts` and `auth.ts` testing
conventions.
- Around line 93-119: Update the login error handling around getAuthErrorMessage
to remove the unreachable fallback and reuse the existing exported generic
authentication error message from auth-error.ts. Apply that shared message in
both the result-error branch and the catch block, while preserving the existing
server error type and focus behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 49dcd58a-e8f1-468e-b5bf-69f971a65026
📒 Files selected for processing (5)
apps/web/src/app/login/auth-error.tsapps/web/src/app/login/auth-error.unit.test.tsapps/web/src/app/login/login-page.tsxapps/web/src/server/auth.tsapps/web/src/server/auth.unit.test.ts
| if (!user.email) { | ||
| throw new SelfHostedRegistrationError(); | ||
| } | ||
|
|
||
| return db.$transaction(async (tx) => { | ||
| await tx.$executeRaw`SELECT pg_advisory_xact_lock(${SELF_HOSTED_REGISTRATION_LOCK_ID})`; | ||
|
|
||
| const firstUser = await tx.user.findFirst({ | ||
| select: { id: true }, | ||
| }); | ||
|
|
||
| if (firstUser) { | ||
| const invite = await tx.teamInvite.findFirst({ | ||
| where: { email: user.email }, | ||
| select: { id: true }, | ||
| }); | ||
|
|
||
| if (!invite) { | ||
| throw new SelfHostedRegistrationError(); | ||
| } | ||
| } | ||
|
|
||
| return tx.user.create({ | ||
| data: { | ||
| name: user.name, | ||
| email: user.email, | ||
| emailVerified: user.emailVerified, | ||
| image: user.image, | ||
| }, | ||
| }); | ||
| }); | ||
| }, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
next-auth v4 adapter createUser throws error redirect query param error code name
💡 Result:
In NextAuth.js v4, if the createUser method in your adapter throws an error, the sign-in flow is interrupted, and the library attempts to redirect the user to an error page [1]. The error handling mechanism works as follows: 1. Error Redirection: When an error occurs during the sign-in/callback process, NextAuth v4 catches the exception [2]. If the error name matches specific known types (e.g., "CreateUserError"), it initiates a redirect [2]. 2. Query Parameter Construction: The redirect URL is appended with an error query parameter [2][3]. For instance, if an error occurs during user creation, it often results in a redirect to {baseUrl}/error?error=OAuthCreateAccount [2]. 3. Redirect Loops: A common issue reported in v4 is "infinite redirect loops," which often occur when the configured error page itself requires authentication, or when the callbackUrl handling is misconfigured in middleware [4][5][6]. If your error page is protected by middleware, the system may perpetually redirect to the sign-in/error page [6]. 4. Special Characters: If the error message thrown contains unencoded special characters, the redirect may fail due to invalid header content or malformed URLs [7]. It is best practice to log the technical error server-side and pass a generic, URL-safe string as the error code [3][7]. To mitigate these issues: - Verify that your custom error page (configured in pages.error) does not require authentication [4][6]. - Ensure your createUser implementation does not throw raw, unencoded strings that might break URL parsing [7]. - Check for "redirect loops" where the application continuously appends callbackUrl parameters to the URL [8][5]. This is frequently solved by properly configuring the pages options so NextAuth recognizes the sign-in and error pages as public routes [8][5][9]. Top results: [2], [1], [4], [9], [6], [7]
Citations:
- 1: https://authjs.dev/reference/nextjs
- 2: https://github.com/nextauthjs/next-auth/blob/v4/packages/next-auth/src/core/routes/callback.ts
- 3: Is it possible to deactivate the error message in the query string? nextauthjs/next-auth#2912
- 4: https://github.com/nextauthjs/next-auth/blob/main/packages/core/src/index.ts
- 5: next.js middleware redirect loop nextauthjs/next-auth#6871
- 6: https://next-auth.js.org/errors
- 7: Error page redirection fails if error parameter contains special characters nextauthjs/next-auth#3986
- 8: Unable to Trigger Custom Sign In Flow after User Creation via NextAuth V4 nextauthjs/next-auth#9397
- 9: https://next-auth.js.org/configuration/pages
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== apps/web/src/server/auth.ts =="
cat -n apps/web/src/server/auth.ts | sed -n '150,240p'
echo
echo "== search for getAuthErrorMessage and auth error handling =="
rg -n "getAuthErrorMessage|AccessDenied|AdapterError|CreateUserError|OAuthCreateAccount|Unable to sign in" apps/web/src -S
echo
echo "== relevant login page file =="
fd -i "login-page.tsx" apps/web/srcRepository: usesend/useSend
Length of output: 2014
🌐 Web query:
next-auth v4 callback.ts createUser adapter error query param AdapterError CreateUserError AccessDenied
💡 Result:
In NextAuth.js v4, encountering an AdapterError related to createUser typically indicates that your database adapter failed to persist a new user record during the authentication flow [1][2]. The AccessDenied error is often a consequence, occurring because the authentication process halts when the database operation fails [2][3]. To resolve these errors, follow these diagnostic steps: 1. Enable Debugging: Set debug: true in your NextAuth configuration [4][2]. This will provide detailed logs in your terminal, showing exactly what is being passed to the createUser adapter method and the precise database error (e.g., NOT NULL constraint failed, foreign key violation) [4][1][2]. 2. Verify Database Schema: A common cause for AdapterError: CreateUserError is a mismatch between your database schema and the data NextAuth attempts to write [4][5]. For example, if your database column requires a non-null ID but your adapter logic fails to generate or return one, the database will reject the operation [4]. Ensure that your database constraints (like primary keys or required fields) align with the expected data from your provider [4][5]. 3. Check Callback Logic: The AccessDenied error often triggers if your signIn callback returns false or throws an unhandled exception [2][3]. Review your signIn callback for any logic that might conditionally return false, preventing the user from being created in your database [6][3]. 4. Analyze Adapter Implementation: If you are using a custom adapter, ensure it correctly implements the createUser method and properly handles the user object passed to it [1][7]. If using a standard adapter (e.g., Prisma, Drizzle), ensure you are on a compatible version and that your schema migrations are up to date [4][5]. The error is usually a secondary symptom of a database-level rejection; focusing on the underlying database error message found in the [auth][debug] logs is the most effective way to identify the specific constraint or configuration issue [4][2].
Citations:
- 1: https://deepwiki.com/nextauthjs/next-auth/4.1-adapter-interface
- 2: https://github.com/nextauthjs/next-auth/blob/b4ef14ab/packages/core/src/errors.ts
- 3: https://next-auth.js.org/configuration/pages
- 4: ID not beeing created in the database nextauthjs/next-auth#11124
- 5: Prisma adapter fails when trying to create a user nextauthjs/next-auth#3828
- 6: https://stackoverflow.com/questions/79575613/nextauth-js-github-provider-returns-accessdenied-and-profile-is-undefined-duri
- 7: https://next-auth.js.org/tutorials/creating-a-database-adapter
🌐 Web query:
site:github.com/nextauthjs/next-auth v4 packages/next-auth/src/core/routes/callback.ts createUser error redirect error=
💡 Result:
In NextAuth.js v4, the error redirect to ?error=OAuthCreateAccount is triggered specifically when the adapter fails to create a user during an OAuth callback [1]. In the source file packages/next-auth/src/core/routes/callback.ts, the library wraps the user creation and account linking process in a try-catch block [1]. When an error occurs during this phase, it checks for specific error names [1]. If the caught error is named CreateUserError, the library performs a redirect to the configured error page with the query parameter error=OAuthCreateAccount [1]. Key points regarding this behavior: 1. Cause: This error typically occurs when the database adapter is unable to persist the new user record in your database [1]. This can be due to database constraints (e.g., unique email violation), connection issues, or other operational failures within the adapter's createUser method [1]. 2. Redirect Behavior: The handler intercepts the failure, logs the error internally, and issues a redirect to the error page [1]. This prevents the user from being left in an ambiguous state and directs them to the configured NextAuth error page [1]. 3. Related Events: Note that the createUser event (defined in events in authOptions) is only triggered upon successful user creation when using OAuth providers [2][3]. It is not triggered for Credentials provider sign-ins, nor is it triggered if the database adapter fails during the process [2][3]. 4. Debugging: If you encounter this error in production, check your server logs (which will contain the original error details logged by OAUTH_CALLBACK_HANDLER_ERROR) [1] and verify your database schema and adapter configuration [4]. Ensure that your database adapter is correctly initialized and that it has the necessary permissions to perform write operations [4]. If you are using a custom adapter or complex database logic, ensure that the error thrown by your adapter is correctly bubbling up or is being handled as expected by the NextAuth callback logic [1].
Citations:
- 1: https://github.com/nextauthjs/next-auth/blob/v4/packages/next-auth/src/core/routes/callback.ts
- 2: How is the createUser event fired? nextauthjs/next-auth#7411
- 3: createUser event not being triggered on credentials provider nextauthjs/next-auth#9852
- 4: [4.24.6] Redirect after OAuth SignUp not working nextauthjs/next-auth#10385
createUser needs to preserve the invitation error path
Throwing SelfHostedRegistrationError here surfaces as OAuthCreateAccount, not AccessDenied, so this race-condition guard will fall back to the generic sign-in error instead of the invitation-required copy. Add OAuthCreateAccount to the auth-error mapping or catch this earlier so the locked rejection still shows the intended message.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/web/src/server/auth.ts` around lines 177 - 208, Update the auth error
mapping for the createUser flow so SelfHostedRegistrationError thrown inside the
transaction is recognized under OAuthCreateAccount and surfaces as AccessDenied
with the invitation-required message. Preserve the advisory-lock race guard and
existing error behavior for unrelated authentication failures.
1 participant
Summary
Why
Self-hosted instances currently allow any reachable user to create an account. After bootstrap, account creation should be limited to users invited by an existing administrator.
The email form previously ignored the result returned by NextAuth and always displayed the OTP success state, even when the server rejected the sign-in.
Verification
pnpm --filter=web exec vitest run -c vitest.unit.config.ts src/server/auth.unit.test.tspnpm --filter=web exec eslint src/server/auth.ts src/server/auth.unit.test.ts src/app/login/login-page.tsx src/app/login/auth-error.ts --max-warnings 0pnpm --filter=web exec tsc --noEmit --pretty falseNo database migration is required.
Summary by CodeRabbit