fix(auth): allow invited placeholder users to complete signup from invite links#28938
fix(auth): allow invited placeholder users to complete signup from invite links#28938Adityakk9031 wants to merge 2 commits intocalcom:mainfrom
Conversation
|
Welcome to Cal.diy, @Adityakk9031! Thanks for opening this pull request. A few things to keep in mind:
A maintainer will review your PR soon. Thanks for contributing! |
Adityakk9031
force-pushed
the
fix/auth-invite-signup-28380
branch
from
4ad14d1 to
842a4b8
Compare
📝 WalkthroughWalkthroughThis change refines authentication and onboarding logic across multiple handlers. The signup handlers ( 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/web/app/api/auth/signup/handlers/calcomSignupHandler.ts (1)
97-109:⚠️ Potential issue | 🟠 MajorAccount takeover via invitation link: insufficient guard allows password reset on existing user accounts.
The new condition only rejects when
existingUser.invitedTois non-null and mismatched. A fully-registered independent user (with no pending invite) hasinvitedTo === null, so the request flows past this check into theprisma.user.upsertbelow (line 221), whoseupdatebranch rewritespassword,emailVerified,identityProvider, andorganizationIdon the existing record.Consequences:
- If a team admin invites an email that already belongs to an existing cal.com user, whoever opens the invite link can reset the victim's password and reassign their
organizationIdwithout logging in.- Even in the benign case, a legitimate user who already has an account and clicks the invite link will have their existing password silently replaced instead of being told to log in.
Tighten the guard to additionally require the existing user to look like a true placeholder (no password hash, no
emailVerified) before allowing the upsert to proceed—mirroring the signal already used innext-auth-options.ts. Otherwise return 409.🛡️ Suggested tightening
if (foundToken?.teamId) { const existingUser = await prisma.user.findUnique({ where: { email }, - select: { invitedTo: true }, + select: { invitedTo: true, emailVerified: true, password: { select: { hash: true } } }, }); - if ( - existingUser && - existingUser.invitedTo !== null && - existingUser.invitedTo !== foundToken.teamId - ) { - return NextResponse.json({ message: SIGNUP_ERROR_CODES.USER_ALREADY_EXISTS }, { status: 409 }); - } + if (existingUser) { + const isInvitedPlaceholder = !existingUser.password?.hash && !existingUser.emailVerified; + const invitedToMismatch = + existingUser.invitedTo !== null && existingUser.invitedTo !== foundToken.teamId; + if (invitedToMismatch || !isInvitedPlaceholder) { + return NextResponse.json( + { message: SIGNUP_ERROR_CODES.USER_ALREADY_EXISTS }, + { status: 409 } + ); + } + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/app/api/auth/signup/handlers/calcomSignupHandler.ts` around lines 97 - 109, The current guard around foundToken?.teamId is insufficient: locate the block using foundToken and the prisma.user.findUnique call (and the later prisma.user.upsert) and tighten it so you only allow the upsert/update path when the existing user is a placeholder invitation account — i.e., invitedTo equals foundToken.teamId AND the existing user has no password hash and emailVerified is falsy; if any of those conditions are not true, return a 409 (SIGNUP_ERROR_CODES.USER_ALREADY_EXISTS) and do not perform the upsert that would overwrite password, emailVerified, identityProvider, or organizationId.
♻️ Duplicate comments (1)
apps/web/app/api/auth/signup/handlers/selfHostedHandler.ts (1)
48-60:⚠️ Potential issue | 🟠 MajorSame relaxed-guard concern as
calcomSignupHandler.ts: existing real users can be overwritten via invite token.The condition now allows requests where
existingUser.invitedTo === nullto fall through into theprisma.user.upsertat Line 123, whoseupdatebranch replacespassword,emailVerified,identityProvider, andorganizationIdon any matching email.This means a team admin who invites an email belonging to an already-existing self-hosted user will cause that user's password and org association to be silently overwritten when the invite link is completed. If the invite URL leaks, it becomes an account takeover path.
Recommend gating the upsert on the user actually looking like a placeholder (no password hash and no
emailVerified), consistent with the signal used innext-auth-options.ts.🛡️ Suggested tightening
if (foundToken?.teamId) { const existingUser = await prisma.user.findUnique({ where: { email: userEmail }, - select: { invitedTo: true }, + select: { invitedTo: true, emailVerified: true, password: { select: { hash: true } } }, }); - if ( - existingUser && - existingUser.invitedTo !== null && - existingUser.invitedTo !== foundToken.teamId - ) { - return NextResponse.json({ message: SIGNUP_ERROR_CODES.USER_ALREADY_EXISTS }, { status: 409 }); - } + if (existingUser) { + const isInvitedPlaceholder = !existingUser.password?.hash && !existingUser.emailVerified; + const invitedToMismatch = + existingUser.invitedTo !== null && existingUser.invitedTo !== foundToken.teamId; + if (invitedToMismatch || !isInvitedPlaceholder) { + return NextResponse.json( + { message: SIGNUP_ERROR_CODES.USER_ALREADY_EXISTS }, + { status: 409 } + ); + } + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/app/api/auth/signup/handlers/selfHostedHandler.ts` around lines 48 - 60, The current guard allows an upsert to overwrite real accounts; modify the pre-upsert checks in selfHostedHandler.ts so we only allow the upsert for placeholder/invite-only users. When loading existingUser (variable existingUser), include and check the credential signals used in next-auth-options.ts (e.g., password hash field and emailVerified) and refuse the invite/upsert if the user has a password hash or emailVerified is true; keep the existing invitedTo mismatch check that returns 409. Update the logic that precedes the prisma.user.upsert call so the upsert path only runs when existingUser is either absent or explicitly a placeholder (no password hash and not emailVerified).
🧹 Nitpick comments (1)
apps/web/app/api/auth/signup/handlers/calcomSignupHandler.ts (1)
102-108: Nit: indentation is off for this block.Lines 102–108 are indented two spaces deeper than the surrounding
if (foundToken?.teamId)body.✏️ Proposed formatting fix
- if ( - existingUser && - existingUser.invitedTo !== null && - existingUser.invitedTo !== foundToken.teamId - ) { - return NextResponse.json({ message: SIGNUP_ERROR_CODES.USER_ALREADY_EXISTS }, { status: 409 }); - } + if ( + existingUser && + existingUser.invitedTo !== null && + existingUser.invitedTo !== foundToken.teamId + ) { + return NextResponse.json({ message: SIGNUP_ERROR_CODES.USER_ALREADY_EXISTS }, { status: 409 }); + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/web/app/api/auth/signup/handlers/calcomSignupHandler.ts` around lines 102 - 108, The if-block checking existingUser.invitedTo should be re-indented to match the surrounding if (foundToken?.teamId) body; locate the block that returns NextResponse.json({ message: SIGNUP_ERROR_CODES.USER_ALREADY_EXISTS }, { status: 409 }) and align its indentation with the other statements in calcomSignupHandler.ts so the "if (existingUser && existingUser.invitedTo !== null && existingUser.invitedTo !== foundToken.teamId) { ... }" lines use the same indentation level as the rest of the foundToken handling code.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@apps/web/app/api/auth/signup/handlers/calcomSignupHandler.ts`:
- Around line 97-109: The current guard around foundToken?.teamId is
insufficient: locate the block using foundToken and the prisma.user.findUnique
call (and the later prisma.user.upsert) and tighten it so you only allow the
upsert/update path when the existing user is a placeholder invitation account —
i.e., invitedTo equals foundToken.teamId AND the existing user has no password
hash and emailVerified is falsy; if any of those conditions are not true, return
a 409 (SIGNUP_ERROR_CODES.USER_ALREADY_EXISTS) and do not perform the upsert
that would overwrite password, emailVerified, identityProvider, or
organizationId.
---
Duplicate comments:
In `@apps/web/app/api/auth/signup/handlers/selfHostedHandler.ts`:
- Around line 48-60: The current guard allows an upsert to overwrite real
accounts; modify the pre-upsert checks in selfHostedHandler.ts so we only allow
the upsert for placeholder/invite-only users. When loading existingUser
(variable existingUser), include and check the credential signals used in
next-auth-options.ts (e.g., password hash field and emailVerified) and refuse
the invite/upsert if the user has a password hash or emailVerified is true; keep
the existing invitedTo mismatch check that returns 409. Update the logic that
precedes the prisma.user.upsert call so the upsert path only runs when
existingUser is either absent or explicitly a placeholder (no password hash and
not emailVerified).
---
Nitpick comments:
In `@apps/web/app/api/auth/signup/handlers/calcomSignupHandler.ts`:
- Around line 102-108: The if-block checking existingUser.invitedTo should be
re-indented to match the surrounding if (foundToken?.teamId) body; locate the
block that returns NextResponse.json({ message:
SIGNUP_ERROR_CODES.USER_ALREADY_EXISTS }, { status: 409 }) and align its
indentation with the other statements in calcomSignupHandler.ts so the "if
(existingUser && existingUser.invitedTo !== null && existingUser.invitedTo !==
foundToken.teamId) { ... }" lines use the same indentation level as the rest of
the foundToken handling code.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a13db0c1-b44f-4f7c-ac4d-49c2ce9af9e8
📒 Files selected for processing (4)
apps/web/app/api/auth/signup/handlers/calcomSignupHandler.tsapps/web/app/api/auth/signup/handlers/selfHostedHandler.tsapps/web/modules/onboarding/components/OnboardingCard.tsxpackages/features/auth/lib/next-auth-options.ts
|
This PR has been marked as stale due to inactivity. If you're still working on it or need any help, please let us know or update the PR to keep it active. |
|
Hi, In both invite-token signup handlers, an existing user is only treated as an allowed invited placeholder when Severity: action required | Category: correctness How to fix: Allow null invitedTo placeholders Agent prompt to fix - you can give this to your LLM of choice:
We noticed a couple of other issues in this PR as well - happy to share if helpful. Spotted by Qodo code review - free for open-source projects. |
|
still working |
|
|
3 participants
Linked Issues
Closes #28380
Follow-up to #28351
Summary
This PR fixes a regression where invited users could receive invite emails but fail account creation from the invite link.
Symptoms:
invalid_server_responseInternal Server ErrorRoot Cause
Invite-signup completion logic treated some valid invited placeholder records as conflicts:
invitedTowasnull, even for valid invited placeholders.usernameto be empty, but placeholder records can already have generated usernames.This caused signup paths to incorrectly fail instead of completing the invited account.
Changes
1) Invite token signup handlers
Updated conflict guard to reject only when
invitedTois explicitly set to a different team.Files:
apps/web/app/api/auth/signup/handlers/calcomSignupHandler.tsapps/web/app/api/auth/signup/handlers/selfHostedHandler.tsChange:
existingUser.invitedTo !== foundToken.teamIdexistingUser.invitedTo !== null && existingUser.invitedTo !== foundToken.teamId2) OAuth invited account completion
Relaxed invited placeholder detection in NextAuth login callback to rely on:
and no longer require missing username.
File:
packages/features/auth/lib/next-auth-options.tsBehavior After Fix