Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -278,10 +278,11 @@ const handler = createSmartRouteHandler({
// ========================== sign up user ==========================

let primaryEmailAuthEnabled = false;
let oldContactChannel = null;
if (userInfo.email) {
primaryEmailAuthEnabled = true;

const oldContactChannel = await getAuthContactChannelWithEmailNormalization(
oldContactChannel = await getAuthContactChannelWithEmailNormalization(
prisma,
{
tenancyId: outerInfo.tenancyId,
Expand Down Expand Up @@ -351,6 +352,44 @@ const handler = createSmartRouteHandler({


if (!tenancy.config.auth.allowSignUp) {
// 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) {
Copy link

Copilot AI Dec 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.
const existingUser = oldContactChannel.projectUser;
Comment on lines +358 to +359
Copy link

Copilot AI Dec 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Suggested change
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;

Copilot uses AI. Check for mistakes.

// 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,
};
}
Comment on lines +358 to +390
Copy link

Copilot AI Dec 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:

  1. The existing user's email to be verified (oldContactChannel.isVerified)
  2. 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:

  1. Creating an OAuth account with an unverified email matching an existing user
  2. 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.

Copilot uses AI. Check for mistakes.

// No existing user with this email, so throw SIGN_UP_NOT_ENABLED as expected
throw new KnownErrors.SignUpNotEnabled();
Comment on lines +355 to 393
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

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.

Suggested change
// 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.

}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@

import { it, localRedirectUrl, updateCookiesFromResponse } from "../../../../../../helpers";
import { Auth, InternalApiKey, Project, niceBackendFetch } from "../../../../../backend-helpers";
import { Auth, InternalApiKey, Project, backendContext, niceBackendFetch } from "../../../../../backend-helpers";

it("should return outer authorization code when inner callback url is valid", async ({ expect }) => {
const response = await Auth.OAuth.getAuthorizationCode();
Expand Down Expand Up @@ -241,3 +241,90 @@ it("should fail if an untrusted redirect URL is provided that is similar to a tr
`);
});

it("should link OAuth account to existing user when sign-ups are disabled but user exists with matching email", async ({ expect }) => {
// Test Case A: sign-ups disabled, existing user with matching email, no existing connected account → OAuth login succeeds, links account, and signs in.
await Project.createAndSwitch({ config: { sign_up_enabled: false, oauth_providers: [ { id: "spotify", type: "shared" } ] } });
await InternalApiKey.createAndSetProjectKeys();

// Create a user via the server API with the same email that will be returned by OAuth
const createUserResponse = await niceBackendFetch("/api/v1/users", {
method: "POST",
accessType: "server",
body: {
primary_email: backendContext.value.mailbox.emailAddress,
primary_email_verified: true,
},
Comment on lines +250 to +256
Copy link

Copilot AI Dec 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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:

  1. Add primary_email_auth_enabled: true to the test user creation
  2. Fix the route handler to look up users by email regardless of the usedForAuth status

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.

Copilot uses AI. Check for mistakes.
});
expect(createUserResponse.status).toBe(201);
const existingUserId = createUserResponse.body.id;

// Now attempt OAuth sign-in with the same email
// Since a user with this email already exists, it should link the OAuth account instead of throwing SIGN_UP_NOT_ENABLED
const getInnerCallbackUrlResponse = await Auth.OAuth.getInnerCallbackUrl();
const cookie = updateCookiesFromResponse("", getInnerCallbackUrlResponse.authorizeResponse);
const response = await niceBackendFetch(getInnerCallbackUrlResponse.innerCallbackUrl, {
redirect: "manual",
headers: {
cookie,
},
});

// The OAuth callback should succeed and return an authorization code
expect(response.status).toBe(303);
expect(response.headers.get("location")).toBeTruthy();
const outerCallbackUrl = new URL(response.headers.get("location")!);
expect(outerCallbackUrl.searchParams.get("code")).toBeTruthy();

// Exchange the authorization code for tokens
const projectKeys = backendContext.value.projectKeys;
if (projectKeys === "no-project") throw new Error("No project keys found");
const tokenResponse = await niceBackendFetch("/api/v1/auth/oauth/token", {
method: "POST",
accessType: "client",
body: {
client_id: projectKeys.projectId,
client_secret: projectKeys.publishableClientKey,
code: outerCallbackUrl.searchParams.get("code")!,
redirect_uri: localRedirectUrl,
grant_type: "authorization_code",
code_verifier: "some-code-challenge",
},
});

expect(tokenResponse.status).toBe(200);
expect(tokenResponse.body.user_id).toBe(existingUserId);
expect(tokenResponse.body.is_new_user).toBe(false);
});

it("should still fail with SIGN_UP_NOT_ENABLED when no user exists with that email", async ({ expect }) => {
// Test Case B: sign-ups disabled, NO user with that email → still returns SIGN_UP_NOT_ENABLED (unchanged behavior).
await Project.createAndSwitch({ config: { sign_up_enabled: false, oauth_providers: [ { id: "spotify", type: "shared" } ] } });
await InternalApiKey.createAndSetProjectKeys();

// Do NOT create a user first - attempt OAuth sign-in directly
const getInnerCallbackUrlResponse = await Auth.OAuth.getInnerCallbackUrl();
const cookie = updateCookiesFromResponse("", getInnerCallbackUrlResponse.authorizeResponse);
const response = await niceBackendFetch(getInnerCallbackUrlResponse.innerCallbackUrl, {
redirect: "manual",
headers: {
cookie,
},
});

// Should still throw SIGN_UP_NOT_ENABLED as before
expect(response).toMatchInlineSnapshot(`
NiceResponse {
"status": 400,
"body": {
"code": "SIGN_UP_NOT_ENABLED",
"error": "Creation of new accounts is not enabled for this project. Please ask the project owner to enable it.",
},
"headers": Headers {
"set-cookie": <deleting cookie 'stack-oauth-inner-<stripped cookie name key>' at path '/'>,
"x-stack-known-error": "SIGN_UP_NOT_ENABLED",
<some fields may have been hidden>,
},
}
`);
});