fix: allow OAuth sign-in with existing email when sign-ups disabled#1055
Conversation
|
Someone is attempting to deploy a commit to the Stack Auth Team on Vercel. A member of the Team first needs to authorize it. |
Preview Screenshots⏳ Preview screenshots are being captured... Workspace and dev browser links will appear here once the preview environment is ready. Generated by cmux preview system |
|
|
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughWhen sign-ups are disabled, the OAuth callback now uses a previously fetched contact channel to link an incoming OAuth account to an existing user (if email matches). It creates the OAuth account, an oauthAuthMethod, stores tokens, and returns the existing user's ID with Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🧰 Additional context used📓 Path-based instructions (7)**/*.{ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.{ts,tsx,css}📄 CodeRabbit inference engine (AGENTS.md)
Files:
apps/**/*.{ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
{.env*,**/*.{ts,tsx,js}}📄 CodeRabbit inference engine (AGENTS.md)
Files:
apps/**/*.tsx📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.test.{ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
apps/e2e/**/*.{ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
🧠 Learnings (3)📚 Learning: 2025-12-03T07:19:44.433ZApplied to files:
📚 Learning: 2025-12-04T18:03:49.984ZApplied to files:
📚 Learning: 2025-12-04T18:03:49.984ZApplied to files:
🧬 Code graph analysis (1)apps/backend/src/app/api/latest/auth/oauth/callback/[provider_id]/route.tsx (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). (1)
🔇 Additional comments (2)
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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
apps/backend/src/app/api/latest/auth/oauth/callback/[provider_id]/route.tsx (1)
362-389: Consider extracting shared account linking logic.This code block duplicates the
link_methodcase (lines 313-340). Both perform the same sequence: create OAuth account → create auth method → store tokens → return user ID.A helper function could reduce duplication:
async function linkOAuthAccountToUser(params: { prisma: PrismaClient, tenancyId: string, providerId: string, providerAccountId: string, email: string | undefined, projectUserId: string, afterCallbackRedirectUrl: string | undefined, storeTokens: (oauthAccountId: string) => Promise<void>, }) { /* ... */ }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/backend/src/app/api/latest/auth/oauth/callback/[provider_id]/route.tsx(2 hunks)apps/e2e/tests/backend/endpoints/api/v1/auth/oauth/callback.test.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Always add new E2E tests when changing the API or SDK interface
For blocking alerts and errors, never use toast; use alerts instead as they are less easily missed by the user
NEVER try-catch-all, NEVER void a promise, and NEVER .catch(console.error); use loading indicators and async callbacks instead, or use runAsynchronously/runAsynchronouslyWithAlert for error handling
Use ES6 maps instead of records wherever you can
Files:
apps/backend/src/app/api/latest/auth/oauth/callback/[provider_id]/route.tsxapps/e2e/tests/backend/endpoints/api/v1/auth/oauth/callback.test.ts
**/*.{ts,tsx,css}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,css}: Keep hover/click transitions snappy and fast; avoid fade-in delays on hover. Apply transitions after action completion instead, like smooth fade-out when hover ends
Use hover-exit transitions instead of hover-enter transitions; for example, use 'transition-colors hover:transition-none' instead of fade-in on hover
Files:
apps/backend/src/app/api/latest/auth/oauth/callback/[provider_id]/route.tsxapps/e2e/tests/backend/endpoints/api/v1/auth/oauth/callback.test.ts
apps/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
NEVER use Next.js dynamic functions if you can avoid them; prefer using client components and hooks like usePathname instead of await params to keep pages static
Files:
apps/backend/src/app/api/latest/auth/oauth/callback/[provider_id]/route.tsxapps/e2e/tests/backend/endpoints/api/v1/auth/oauth/callback.test.ts
{.env*,**/*.{ts,tsx,js}}
📄 CodeRabbit inference engine (AGENTS.md)
Prefix environment variables with STACK_ (or NEXT_PUBLIC_STACK_ if public) so changes are picked up by Turborepo and improves readability
Files:
apps/backend/src/app/api/latest/auth/oauth/callback/[provider_id]/route.tsxapps/e2e/tests/backend/endpoints/api/v1/auth/oauth/callback.test.ts
apps/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
Check existing apps for inspiration when implementing new apps or pages; update apps-frontend.tsx and apps-config.ts to add new apps
Files:
apps/backend/src/app/api/latest/auth/oauth/callback/[provider_id]/route.tsx
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Prefer .toMatchInlineSnapshot over other selectors in tests when possible; check snapshot-serializer.ts for formatting details
Files:
apps/e2e/tests/backend/endpoints/api/v1/auth/oauth/callback.test.ts
apps/e2e/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Always add new E2E tests when changing API or SDK interface; err on the side of creating too many tests due to the critical nature of the industry
Files:
apps/e2e/tests/backend/endpoints/api/v1/auth/oauth/callback.test.ts
🧠 Learnings (3)
📚 Learning: 2025-12-03T07:19:44.433Z
Learnt from: madster456
Repo: stack-auth/stack-auth PR: 1040
File: packages/stack-shared/src/interface/crud/oauth-providers.ts:62-87
Timestamp: 2025-12-03T07:19:44.433Z
Learning: In packages/stack-shared/src/interface/crud/oauth-providers.ts and similar CRUD files, the tag "Oauth" (not "OAuth") is the correct capitalization format as it's used by the documentation generation system and follows OpenAPI conventions.
Applied to files:
apps/backend/src/app/api/latest/auth/oauth/callback/[provider_id]/route.tsx
📚 Learning: 2025-12-04T18:03:49.961Z
Learnt from: CR
Repo: stack-auth/stack-auth PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-04T18:03:49.961Z
Learning: Applies to apps/e2e/**/*.{ts,tsx} : Always add new E2E tests when changing API or SDK interface; err on the side of creating too many tests due to the critical nature of the industry
Applied to files:
apps/e2e/tests/backend/endpoints/api/v1/auth/oauth/callback.test.ts
📚 Learning: 2025-12-04T18:03:49.961Z
Learnt from: CR
Repo: stack-auth/stack-auth PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-04T18:03:49.961Z
Learning: Applies to **/*.{ts,tsx} : Always add new E2E tests when changing the API or SDK interface
Applied to files:
apps/e2e/tests/backend/endpoints/api/v1/auth/oauth/callback.test.ts
🧬 Code graph analysis (2)
apps/backend/src/app/api/latest/auth/oauth/callback/[provider_id]/route.tsx (1)
apps/backend/src/lib/contact-channel.tsx (1)
getAuthContactChannelWithEmailNormalization(51-80)
apps/e2e/tests/backend/endpoints/api/v1/auth/oauth/callback.test.ts (6)
apps/e2e/tests/helpers.ts (3)
it(12-12)updateCookiesFromResponse(111-118)localRedirectUrl(187-187)packages/template/src/lib/stack-app/index.ts (3)
Project(76-76)InternalApiKey(20-20)Auth(96-96)packages/template/src/lib/stack-app/projects/index.ts (1)
Project(9-13)packages/template/src/lib/stack-app/internal-api-keys/index.ts (1)
InternalApiKey(24-34)apps/e2e/tests/backend/backend-helpers.ts (2)
niceBackendFetch(109-173)backendContext(35-57)packages/template/src/lib/stack-app/users/index.ts (1)
Auth(71-77)
⏰ 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)
- GitHub Check: Agent
- GitHub Check: CodeQL analysis (javascript-typescript)
- GitHub Check: Vercel Agent Review
🔇 Additional comments (2)
apps/e2e/tests/backend/endpoints/api/v1/auth/oauth/callback.test.ts (2)
3-3: LGTM!The
backendContextimport is correctly added to support accessingmailbox.emailAddressandprojectKeysin the new tests.
244-297: Tests look good; consider adding edge case coverage.The test correctly validates the new linking behavior. The setup is clear, and using
toMatchInlineSnapshotfor the error case aligns with guidelines.Consider adding tests for these edge cases to ensure security:
- User exists with unverified email (
primary_email_verified: false) — should this link succeed?- OAuth provider returns an unverified email — does the linking behavior match expectations?
These additional tests would help document and enforce the expected security behavior. Based on learnings, E2E tests should err on the side of creating too many tests due to the critical nature of authentication flows.
| // Before rejecting as a new sign-up, check if a user with this email already exists | ||
| // (reuse oldContactChannel from above to avoid duplicate database lookup) | ||
| // If a user with this email exists (even if email is not used for auth), link the OAuth account | ||
| if (oldContactChannel) { | ||
| const existingUser = oldContactChannel.projectUser; | ||
|
|
||
| // Create the OAuth account linked to the existing user | ||
| const newOAuthAccount = await createProjectUserOAuthAccount(prisma, { | ||
| tenancyId: outerInfo.tenancyId, | ||
| providerId: provider.id, | ||
| providerAccountId: userInfo.accountId, | ||
| email: userInfo.email, | ||
| projectUserId: existingUser.projectUserId, | ||
| }); | ||
|
|
||
| await prisma.authMethod.create({ | ||
| data: { | ||
| tenancyId: outerInfo.tenancyId, | ||
| projectUserId: existingUser.projectUserId, | ||
| oauthAuthMethod: { | ||
| create: { | ||
| projectUserId: existingUser.projectUserId, | ||
| configOAuthProviderId: provider.id, | ||
| providerAccountId: userInfo.accountId, | ||
| } | ||
| } | ||
| } | ||
| }); | ||
|
|
||
| await storeTokens(newOAuthAccount.id); | ||
| return { | ||
| id: existingUser.projectUserId, | ||
| newUser: false, | ||
| afterCallbackRedirectUrl, | ||
| }; | ||
| } | ||
|
|
||
| // No existing user with this email, so throw SIGN_UP_NOT_ENABLED as expected | ||
| throw new KnownErrors.SignUpNotEnabled(); |
There was a problem hiding this comment.
Missing email verification checks may allow unauthorized account linking.
The link_method merge strategy (lines 299-308) enforces that both oldContactChannel.isVerified and userInfo.emailVerified are true before linking. This new path bypasses those checks, potentially allowing an attacker to link an OAuth account to a user with an unverified email.
Consider adding similar verification logic:
if (oldContactChannel) {
+ // Require email verification to prevent unauthorized linking
+ if (!oldContactChannel.isVerified) {
+ throw new KnownErrors.SignUpNotEnabled();
+ }
+ if (!userInfo.emailVerified) {
+ throw new KnownErrors.SignUpNotEnabled();
+ }
+
const existingUser = oldContactChannel.projectUser;Alternatively, if intentionally allowing unverified linking for admin-created users, please document this security trade-off with a comment explaining the rationale.
📝 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.
| // Before rejecting as a new sign-up, check if a user with this email already exists | |
| // (reuse oldContactChannel from above to avoid duplicate database lookup) | |
| // If a user with this email exists (even if email is not used for auth), link the OAuth account | |
| if (oldContactChannel) { | |
| const existingUser = oldContactChannel.projectUser; | |
| // Create the OAuth account linked to the existing user | |
| const newOAuthAccount = await createProjectUserOAuthAccount(prisma, { | |
| tenancyId: outerInfo.tenancyId, | |
| providerId: provider.id, | |
| providerAccountId: userInfo.accountId, | |
| email: userInfo.email, | |
| projectUserId: existingUser.projectUserId, | |
| }); | |
| await prisma.authMethod.create({ | |
| data: { | |
| tenancyId: outerInfo.tenancyId, | |
| projectUserId: existingUser.projectUserId, | |
| oauthAuthMethod: { | |
| create: { | |
| projectUserId: existingUser.projectUserId, | |
| configOAuthProviderId: provider.id, | |
| providerAccountId: userInfo.accountId, | |
| } | |
| } | |
| } | |
| }); | |
| await storeTokens(newOAuthAccount.id); | |
| return { | |
| id: existingUser.projectUserId, | |
| newUser: false, | |
| afterCallbackRedirectUrl, | |
| }; | |
| } | |
| // No existing user with this email, so throw SIGN_UP_NOT_ENABLED as expected | |
| throw new KnownErrors.SignUpNotEnabled(); | |
| // Before rejecting as a new sign-up, check if a user with this email already exists | |
| // (reuse oldContactChannel from above to avoid duplicate database lookup) | |
| // If a user with this email exists (even if email is not used for auth), link the OAuth account | |
| if (oldContactChannel) { | |
| // Require email verification to prevent unauthorized linking | |
| if (!oldContactChannel.isVerified) { | |
| throw new KnownErrors.SignUpNotEnabled(); | |
| } | |
| if (!userInfo.emailVerified) { | |
| throw new KnownErrors.SignUpNotEnabled(); | |
| } | |
| const existingUser = oldContactChannel.projectUser; | |
| // Create the OAuth account linked to the existing user | |
| const newOAuthAccount = await createProjectUserOAuthAccount(prisma, { | |
| tenancyId: outerInfo.tenancyId, | |
| providerId: provider.id, | |
| providerAccountId: userInfo.accountId, | |
| email: userInfo.email, | |
| projectUserId: existingUser.projectUserId, | |
| }); | |
| await prisma.authMethod.create({ | |
| data: { | |
| tenancyId: outerInfo.tenancyId, | |
| projectUserId: existingUser.projectUserId, | |
| oauthAuthMethod: { | |
| create: { | |
| projectUserId: existingUser.projectUserId, | |
| configOAuthProviderId: provider.id, | |
| providerAccountId: userInfo.accountId, | |
| } | |
| } | |
| } | |
| }); | |
| await storeTokens(newOAuthAccount.id); | |
| return { | |
| id: existingUser.projectUserId, | |
| newUser: false, | |
| afterCallbackRedirectUrl, | |
| }; | |
| } | |
| // No existing user with this email, so throw SIGN_UP_NOT_ENABLED as expected | |
| throw new KnownErrors.SignUpNotEnabled(); |
🤖 Prompt for AI Agents
In apps/backend/src/app/api/latest/auth/oauth/callback/[provider_id]/route.tsx
around lines 355 to 393, the new OAuth-account-linking path skips the email
verification checks used by the link_method flow and may allow unauthorized
linking; update this block to enforce the same verification logic (require
oldContactChannel.isVerified && userInfo.emailVerified) before creating the
OAuth account and authMethod and throwing SignUpNotEnabled if the checks fail,
or if you intentionally permit linking for admin-created users, add a clear
comment documenting that exception and the exact conditions (e.g.,
createdByAdmin) under which unverified linking is allowed.
There was a problem hiding this comment.
Pull request overview
This PR aims to fix an issue where OAuth sign-in fails with SIGN_UP_NOT_ENABLED when a user with a matching email already exists in the system, even though it should link to the existing account instead.
Key Changes:
- Modified OAuth callback logic to check for existing users by email before rejecting sign-in when sign-ups are disabled
- Added new database lookup and account linking flow when existing user is found
- Added two test cases covering both the fix scenario and the existing error behavior
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
apps/backend/src/app/api/latest/auth/oauth/callback/[provider_id]/route.tsx |
Adds logic to link OAuth accounts to existing users with matching emails when sign-ups are disabled, including account creation and auth method setup |
apps/e2e/tests/backend/endpoints/api/v1/auth/oauth/callback.test.ts |
Adds test coverage for the new account linking behavior and verifies the existing error case still works correctly |
Critical Issues Found:
-
Bug: The implementation uses
getAuthContactChannelWithEmailNormalization()which only finds emails whereusedForAuth: "TRUE". Users created withoutprimary_email_auth_enabled: truewon't be found, causing the fix to not work as intended. -
Security: Missing email verification checks before linking accounts. Unlike the existing merge logic (lines 299-308), the new code doesn't verify that both the existing user's email and the OAuth provider's email are verified, creating a potential account takeover vulnerability.
-
Test Coverage: The test doesn't set
primary_email_auth_enabled: truewhen creating the user, so it will fail with the current implementation due to issue #1.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (oldContactChannel) { | ||
| const existingUser = oldContactChannel.projectUser; |
There was a problem hiding this comment.
The comment states "If a user with this email exists (even if email is not used for auth)" but this is not actually what the code does. Due to the bug in line 358, the code only checks for users where the email IS used for auth.
Once the bug in line 358 is fixed to properly query for any user with this email, this comment will be accurate. Until then, it's misleading.
| if (oldContactChannel) { | |
| const existingUser = oldContactChannel.projectUser; | |
| let contactChannelForEmail = oldContactChannel; | |
| if (!contactChannelForEmail && userInfo.email) { | |
| // Try to find any contact channel with this email, even if not used for auth | |
| contactChannelForEmail = await prisma.contactChannel.findFirst({ | |
| where: { | |
| tenancyId: outerInfo.tenancyId, | |
| type: 'EMAIL', | |
| value: userInfo.email, | |
| }, | |
| include: { | |
| projectUser: true, | |
| }, | |
| }); | |
| } | |
| if (contactChannelForEmail) { | |
| const existingUser = contactChannelForEmail.projectUser; |
| const createUserResponse = await niceBackendFetch("/api/v1/users", { | ||
| method: "POST", | ||
| accessType: "server", | ||
| body: { | ||
| primary_email: backendContext.value.mailbox.emailAddress, | ||
| primary_email_verified: true, | ||
| }, |
There was a problem hiding this comment.
This test will fail with the current implementation because primary_email_auth_enabled is not set to true. When creating a user without primary_email_auth_enabled: true, the email's usedForAuth field is set to null, and the OAuth callback logic won't find this user (see the bug in the route handler at line 358).
To fix, you need to either:
- Add
primary_email_auth_enabled: trueto the test user creation - Fix the route handler to look up users by email regardless of the
usedForAuthstatus
Option 2 is the correct fix since the PR description explicitly states it should work for users created "in the dashboard" who may not have auth enabled for that email.
| if (oldContactChannel) { | ||
| const existingUser = oldContactChannel.projectUser; | ||
|
|
||
| // Create the OAuth account linked to the existing user | ||
| const newOAuthAccount = await createProjectUserOAuthAccount(prisma, { | ||
| tenancyId: outerInfo.tenancyId, | ||
| providerId: provider.id, | ||
| providerAccountId: userInfo.accountId, | ||
| email: userInfo.email, | ||
| projectUserId: existingUser.projectUserId, | ||
| }); | ||
|
|
||
| await prisma.authMethod.create({ | ||
| data: { | ||
| tenancyId: outerInfo.tenancyId, | ||
| projectUserId: existingUser.projectUserId, | ||
| oauthAuthMethod: { | ||
| create: { | ||
| projectUserId: existingUser.projectUserId, | ||
| configOAuthProviderId: provider.id, | ||
| providerAccountId: userInfo.accountId, | ||
| } | ||
| } | ||
| } | ||
| }); | ||
|
|
||
| await storeTokens(newOAuthAccount.id); | ||
| return { | ||
| id: existingUser.projectUserId, | ||
| newUser: false, | ||
| afterCallbackRedirectUrl, | ||
| }; | ||
| } |
There was a problem hiding this comment.
Missing email verification checks before linking OAuth account to existing user. This creates a potential account takeover vulnerability.
The existing merge logic (lines 299-308) requires both:
- The existing user's email to be verified (
oldContactChannel.isVerified) - The OAuth provider's email to be verified (
userInfo.emailVerified)
The new code should enforce the same checks. An attacker could potentially take over an account by:
- Creating an OAuth account with an unverified email matching an existing user
- Triggering this code path to link to the victim's account
Add verification checks similar to the existing merge logic before allowing the account link.
| // Before rejecting as a new sign-up, check if a user with this email already exists | ||
| // (reuse oldContactChannel from above to avoid duplicate database lookup) | ||
| // If a user with this email exists (even if email is not used for auth), link the OAuth account | ||
| if (oldContactChannel) { |
There was a problem hiding this comment.
The condition if (oldContactChannel) will not work as intended for users created without primary_email_auth_enabled: true.
getAuthContactChannelWithEmailNormalization only returns contact channels where usedForAuth: "TRUE". When a user is created via the server API without setting primary_email_auth_enabled: true (which is the common case described in the PR), usedForAuth will be null, and oldContactChannel will be null even though a user with that email exists.
To fix this, you need to query for any contact channel with the email (regardless of usedForAuth), not just auth-enabled ones. Consider creating a new query function or modifying the lookup logic to check for any primary email contact channel first, then fall back to the auth-enabled check for the existing merge strategy logic.
Greptile OverviewGreptile SummaryThis PR fixes a bug in the OAuth authentication flow where users with existing accounts would receive The core change modifies the OAuth flow in The implementation reuses existing database queries for efficiency and maintains backward compatibility by preserving the Important Files Changed
Confidence score: 4/5
Sequence DiagramsequenceDiagram
participant User
participant Frontend
participant OAuthProvider as "OAuth Provider"
participant Backend as "OAuth Callback Handler"
participant Database
participant TokenEndpoint as "Token Endpoint"
User->>Frontend: "Click 'Sign in with OAuth'"
Frontend->>Backend: "GET /auth/oauth/authorize"
Backend->>Database: "Store outer OAuth info"
Backend->>Frontend: "Return authorization URL"
Frontend->>OAuthProvider: "Redirect to OAuth provider"
User->>OAuthProvider: "Authenticate with provider"
OAuthProvider->>Backend: "GET /auth/oauth/callback/{provider_id}"
Backend->>Database: "Retrieve outer OAuth info"
Backend->>OAuthProvider: "Exchange code for tokens"
OAuthProvider->>Backend: "Return user info & tokens"
alt Existing OAuth Account Found
Backend->>Database: "Find existing OAuth account"
Backend->>Database: "Store OAuth tokens"
Backend->>Frontend: "Redirect with auth code"
else Sign-ups Disabled & User Exists with Email
Backend->>Database: "Check for existing user with email"
Backend->>Database: "Create OAuth account for existing user"
Backend->>Database: "Create auth method"
Backend->>Database: "Store OAuth tokens"
Backend->>Frontend: "Redirect with auth code"
else Sign-ups Disabled & No User with Email
Backend->>Frontend: "Throw SIGN_UP_NOT_ENABLED error"
else Sign-ups Enabled
Backend->>Database: "Create new user"
Backend->>Database: "Create OAuth account"
Backend->>Database: "Create auth method"
Backend->>Database: "Store OAuth tokens"
Backend->>Frontend: "Redirect with auth code"
end
Frontend->>TokenEndpoint: "POST /auth/oauth/token"
TokenEndpoint->>Database: "Validate authorization code"
TokenEndpoint->>Frontend: "Return access & refresh tokens"
Frontend->>User: "User signed in successfully"
|
There was a problem hiding this comment.
Additional Comments (1)
-
apps/backend/src/app/api/latest/auth/oauth/callback/[provider_id]/route.tsx, line 358-390 (link)style: this section duplicates the OAuth account linking logic from lines 313-334. The duplication creates a maintenance burden and potential for inconsistencies
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
2 files reviewed, 1 comment
- Add logic to link OAuth accounts to existing users by email when sign-ups are disabled - Previously would throw SIGN_UP_NOT_ENABLED even if user with that email already existed - Now checks for existing user by email before rejecting as new sign-up - Reuses existing contact channel lookup to avoid duplicate database queries - Add comprehensive test coverage for both success and error cases - Preserves all existing behavior when no user exists with that email Fixes the issue where users created via dashboard couldn't sign in with OAuth when sign-ups were disabled, even though they already had an account.
38ce2a5 to
7d7dd79
Compare
Preview Screenshots⏳ Preview screenshots are being captured... Workspace and dev browser links will appear here once the preview environment is ready. Generated by cmux preview system |
Summary
When project sign-ups are disabled, signing in with Google (or another OAuth provider) using an email that already exists as a user currently fails with
SIGN_UP_NOT_ENABLED.This PR updates the OAuth flow so that, if a user with that email already exists, the login is treated as a sign-in + account link, not as a new sign-up.
Background / Bug
Current behavior:
Admin disables sign-ups in Auth Methods.
Admin enables Google SSO.
Admin creates a user in the dashboard with
user@example.comand marks them verified.The user tries to “Sign in with Google” using the same email.
OAuth completes, but Stack Auth returns:
SIGN_UP_NOT_ENABLEDEven though the user already exists, the OAuth flow treats this as a new sign-up attempt instead of linking to the existing account.
Expected behavior:
SIGN_UP_NOT_ENABLEDbe returned.Updated OAuth flow