-
Notifications
You must be signed in to change notification settings - Fork 584
fix(auth): Fix invitation flow for enterprise workspaces #3940
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
fix(auth): Fix invitation flow for enterprise workspaces #3940
Conversation
- Add dedicated invitation acceptance API endpoint (/api/auth/accept-invitation) - Create invitation validation route (/api/auth/invitation) - Implement PostAuthInvitationHandler component for post-login invitation processing - Add join success page with proper invitation handling - Refactor auth actions to support invitation acceptance and workspace switching - Move join route to app-level routing with enhanced invitation logic - Update middleware to handle invitation-related redirects - Improve WorkOS auth integration for enterprise invitation flow - Remove legacy join route in favor of new implementation Resolves ENG-2062: Invitations not functioning in enterprise workspace
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
📝 WalkthroughWalkthroughAdds end-to-end invitation handling: new API endpoints and helpers to process/accept invitations, a client PostAuth handler injected into the APIs page, updated join route and success page, middleware redirect for /auth/join → /join, and auth action/hook changes to integrate invitation flows. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Browser
participant Middleware
participant JoinRoute as /join (GET)
participant Auth as Auth Service
participant Org as Org Service
participant Success as /join/success
User->>Browser: Open /auth/join?invitation_token=...
Browser->>Middleware: Request /auth/join?...
Middleware-->>Browser: 307 Redirect to /join?...
Browser->>JoinRoute: GET /join?invitation_token=...
JoinRoute->>Auth: updateSession + getCurrentUser
alt Unauthenticated
JoinRoute->>Auth: findUser(by invitation email)
alt User exists
JoinRoute-->>Browser: Redirect to /auth/sign-in?invitation_token&email
else No user
JoinRoute-->>Browser: Redirect to /auth/sign-up?invitation_token&email
end
else Authenticated
JoinRoute->>Auth: getInvitation(token)
alt Invalid/missing/non-pending
JoinRoute-->>Browser: Redirect /apis?invitation_token=...
else Valid
JoinRoute->>Auth: acceptInvitation
JoinRoute->>Org: switchOrg(orgId) and set session cookie
JoinRoute->>Auth: getOrg(orgId)
JoinRoute-->>Browser: Redirect to /join/success?from_invite=true&org_name=...
end
end
sequenceDiagram
autonumber
actor User
participant Browser
participant ApisPage as /apis page
participant Handler as PostAuthInvitationHandler
participant API as /api/auth/invitation
participant Lib as processPostAuthInvitation
participant Auth as Auth Service
User->>Browser: Land on /apis?invitation_token=...
Browser->>ApisPage: Render
ApisPage->>Handler: Mount handler
Handler->>API: POST { invitationToken } (with cookies)
API->>Lib: processPostAuthInvitation(token, userId)
Lib->>Auth: getInvitation(token), acceptInvitation, accept+switch org
Lib-->>API: { success, organizationId }
API-->>Handler: { success }
alt success
Handler->>Browser: Remove token from URL, router.replace, reload
else failure
Handler->>Handler: Retry (limited) or call onComplete(false)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks (2 passed, 1 warning)❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal). Please share your feedback with us on this Discord post. ✨ Finishing Touches
🧪 Generate unit tests
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 |
@CodeRabbit review |
✅ Actions performedReview triggered.
|
@@ -13,6 +13,14 @@ export default async function middleware(req: NextRequest, _evt: NextFetchEvent) | |||
return NextResponse.redirect("https://app.unkey.com/gateway-new"); | |||
} | |||
|
|||
// Redirect /auth/join to /join to bypass auth layout |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a temp redirect... because I was a bit worried that updating the workOS redirects would cause some users to have a bad time if they had an outstanding invite.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 28
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
apps/dashboard/middleware.ts (1)
28-36
: Tighten publicPaths: remove redundant /auth/join entryBecause the early redirect always intercepts /auth/join, keeping it in publicPaths is unnecessary and can mask regressions if the redirect is reordered later.
Apply:
const publicPaths = [ "/auth/sign-in", "/auth/sign-up", "/auth/sso-callback", "/auth/oauth-sign-in", - "/auth/join", "/join", "/join/success",
apps/dashboard/lib/auth/workos.ts (1)
507-514
: Don’t silently swallow provider errors in getInvitationList.Returning an empty list hides operational issues and breaks alerting.
- } catch (error) { - console.error("Failed to get organization invitations list:", { - error: error instanceof Error ? error.message : "Unknown error", - }); - return { - data: [], - metadata: {}, - }; - } + } catch (error) { + throw this.handleError(error); + }apps/dashboard/app/auth/actions.ts (1)
361-394
: Security: validate invitation–organization consistency and make accept step idempotentexport async function acceptInvitationAndJoin( invitationId: string, organizationId: string, ): Promise<{ success: boolean; error?: string }> { try { + // Retrieve and validate invitation belongs to target org + const invitation = await auth.getInvitation(invitationId); + if (invitation?.organizationId !== organizationId) { + throw new Error("Invitation does not belong to the specified organization"); + } + + // Accept invitation idempotently + try { + await auth.acceptInvitation(invitationId); + } catch (e) { + const msg = e instanceof Error ? e.message : String(e); + if (!/already\s+accepted/i.test(msg)) throw e; + } // Switch organization and get the new session token const { newToken, expiresAt } = await auth.switchOrg(organizationId); @@ console.error("Failed to accept invitation and join organization:", { error: error instanceof Error ? error.message : "Unknown error", + invitationId, + organizationId, });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (12)
apps/dashboard/app/(app)/apis/page.tsx
(2 hunks)apps/dashboard/app/api/auth/accept-invitation/route.ts
(1 hunks)apps/dashboard/app/api/auth/invitation/route.ts
(1 hunks)apps/dashboard/app/auth/actions.ts
(4 hunks)apps/dashboard/app/auth/hooks/useSignIn.ts
(1 hunks)apps/dashboard/app/auth/join/route.ts
(0 hunks)apps/dashboard/app/join/route.ts
(1 hunks)apps/dashboard/app/join/success/page.tsx
(1 hunks)apps/dashboard/components/auth/post-auth-invitation-handler.tsx
(1 hunks)apps/dashboard/lib/auth.ts
(1 hunks)apps/dashboard/lib/auth/workos.ts
(6 hunks)apps/dashboard/middleware.ts
(2 hunks)
💤 Files with no reviewable changes (1)
- apps/dashboard/app/auth/join/route.ts
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-08-18T10:28:47.391Z
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#3797
File: apps/dashboard/app/(app)/projects/[projectId]/deployments/components/control-cloud/index.tsx:1-4
Timestamp: 2025-08-18T10:28:47.391Z
Learning: In Next.js App Router, components that use React hooks don't need their own "use client" directive if they are rendered within a client component that already has the directive. The client boundary propagates to child components.
Applied to files:
apps/dashboard/app/(app)/apis/page.tsx
📚 Learning: 2025-05-15T16:26:08.666Z
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#3242
File: apps/dashboard/app/(app)/apis/[apiId]/_overview/components/table/components/override-indicator.tsx:50-65
Timestamp: 2025-05-15T16:26:08.666Z
Learning: In the Unkey dashboard, Next.js router (router.push) should be used for client-side navigation instead of window.location.href to preserve client state and enable smoother transitions between pages.
Applied to files:
apps/dashboard/middleware.ts
apps/dashboard/app/join/success/page.tsx
🔇 Additional comments (7)
apps/dashboard/middleware.ts (1)
16-23
: Good redirect; preserves invite params and bypasses auth layoutThe 307 from /auth/join → /join with full query preservation is correct and happens before public-path checks.
apps/dashboard/lib/auth/workos.ts (3)
616-621
: Invitation-aware verification: LGTM.Passing invitationToken through to WorkOS during magic-auth verification is appropriate and keeps tokens out of logs.
188-191
: Structured, token-free logging: good.Logs now avoid leaking tokens/PII and standardize messages.
Also applies to: 210-213, 721-723
948-965
: No change needed—invitation tokens aren’t exposed to clients. transformInvitationData’stoken
field is only used internally in the WorkOS service; no API route serializes the full Invitation object to the client.Likely an incorrect or invalid review comment.
apps/dashboard/app/(app)/apis/page.tsx (1)
16-16
: LGTM: Safe, idempotent post-auth hook placement.Rendering
PostAuthInvitationHandler
first is appropriate; assuming it no-ops withoutinvitation_token
, this won’t affect normal visits.If the handler performs network calls on mount, consider guarding behind a cheap
has("invitation_token")
check to avoid unnecessary work.apps/dashboard/app/auth/actions.ts (2)
166-168
: LGTM on structured error logging.Consistent key’ed logging improves observability.
228-230
: LGTM on rate-limit error handling.Logs the message and returns a permissive fallback, matching prior behavior.
@CodeRabbit resolve |
✅ Actions performedComments resolved. |
Thank you for following the naming conventions for pull request titles! 🙏 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 13
♻️ Duplicate comments (11)
apps/dashboard/lib/auth.ts (2)
75-85
: Avoid redirecting auth in library helper (use non-redirecting get-auth).getCurrentUser() can redirect, which is unsafe in library/API contexts. Use the non-redirecting base auth to assert session presence.
try { - // Verify we have a valid session before proceeding - const currentUser = await getCurrentUser(); - if (!currentUser) { - throw new Error("User not authenticated - cannot accept invitation"); - } + // Verify session without redirect side-effects + const { userId } = await getBaseAuth(); + if (!userId) { + throw new Error("User not authenticated"); + }
123-125
: Harden validation: generic client messages and case-insensitive email match.Prevents token enumeration and false negatives due to case/whitespace.
- if (!invitation) { - return { success: false, error: "Invitation not found" }; - } + if (!invitation) { + return { success: false, error: "Invalid or expired invitation" }; + } @@ - if (state !== "pending") { - return { success: false, error: `Invitation is ${state}` }; - } + if (state !== "pending") { + return { success: false, error: "Invalid or expired invitation" }; + } @@ - if (!organizationId) { - return { success: false, error: "No organization ID in invitation" }; - } + if (!organizationId) { + return { success: false, error: "Invalid or expired invitation" }; + } @@ - if (user.email !== invitationEmail) { - return { success: false, error: "Email mismatch" }; - } + const normalize = (e?: string | null) => (e ?? "").trim().toLowerCase(); + if (normalize(user.email) !== normalize(invitationEmail)) { + return { success: false, error: "Invalid or expired invitation" }; + }Also applies to: 129-135, 143-145
apps/dashboard/app/api/auth/accept-invitation/route.ts (1)
1-6
: Don’t import/use getCurrentUser() in API route; use non-redirecting auth.Avoid HTML redirects on XHR; check auth via base get-auth.
-import { acceptInvitationAndSwitchOrg, getCurrentUser } from "@/lib/auth"; +import { acceptInvitationAndSwitchOrg } from "@/lib/auth"; +import { getAuth as getBaseAuth } from "@/lib/auth/get-auth";apps/dashboard/app/auth/actions.ts (3)
117-125
: Only swallow “already accepted”; rethrow other accept failures.Don’t mask real errors.
- } catch (acceptError) { - // Log but don't fail - invitation might already be accepted - console.warn("Could not accept invitation (might already be accepted):", { - invitationId: invitation.id, - error: acceptError instanceof Error ? acceptError.message : "Unknown error", - }); - } + } catch (acceptError) { + const msg = acceptError instanceof Error ? acceptError.message : String(acceptError); + if (!/already\s+accepted/i.test(msg)) { + throw acceptError; + } + console.warn("Invitation already accepted; continuing:", { invitationId: invitation.id }); + }
355-361
: Add orgId context to switchOrg error log.Improves operability and supportability.
- console.error("Organization switch failed:", { - error: error instanceof Error ? error.message : "Unknown error", - }); + console.error("Organization switch failed:", { + error: error instanceof Error ? error.message : "Unknown error", + orgId, + });
71-90
: redirectUrl never updated; success page URL is dropped.Assign the built URL and avoid returning empty cookies.
- const redirectUrl = "/apis"; + let redirectUrl = "/apis"; try { const org = await auth.getOrg(invitation.organizationId); if (org?.name) { const params = new URLSearchParams({ from_invite: "true", org_name: org.name, }); - `/join/success?${params.toString()}`; + redirectUrl = `/join/success?${params.toString()}`; } } catch (error) { // Don't fail the redirect if we can't get org name console.warn("Could not fetch organization name for success page:", error); } return { success: true, redirectTo: redirectUrl, - cookies: [], };apps/dashboard/app/join/route.ts (5)
8-8
: Good: caching disabled for a side-effectful route.
Correctly uses export const dynamic = "force-dynamic".
81-99
: Bug: session cookie likely dropped on redirect; attach it to the redirect response.
Set the cookie on the NextResponse returned to the browser (or pass the response into your cookie helper).Apply this diff here:
- // Set the session cookie securely on the server side - await setSessionCookie({ token: newToken, expiresAt }); - - // Redirect to success page with organization context - const JOIN_SUCCESS_URL = new URL("/join/success", request.url); + // Redirect to success page with organization context + const JOIN_SUCCESS_URL = new URL("/join/success", request.url); @@ - return NextResponse.redirect(JOIN_SUCCESS_URL); + const res = NextResponse.redirect(JOIN_SUCCESS_URL); + await setSessionCookie({ token: newToken, expiresAt, response: res }); + return res;And update cookies helper to accept an optional response:
+import type { NextResponse } from "next/server"; export async function setSessionCookie(params: { token: string; expiresAt: Date; + response?: NextResponse; }): Promise<void> { - const { token, expiresAt } = params; - - await setCookie({ - name: UNKEY_SESSION_COOKIE, - value: token, - options: { - httpOnly: true, - secure: true, - sameSite: "strict", - path: "/", - maxAge: Math.floor((expiresAt.getTime() - Date.now()) / 1000), - }, - }); + const { token, expiresAt, response } = params; + const options = { + httpOnly: true, + secure: true, + sameSite: "strict" as const, + path: "/", + maxAge: Math.floor((expiresAt.getTime() - Date.now()) / 1000), + }; + if (response) { + response.cookies.set(UNKEY_SESSION_COOKIE, token, options); + return; + } + await setCookie({ name: UNKEY_SESSION_COOKIE, value: token, options }); }
56-58
: Avoid mutating the shared DASHBOARD_URL; clone before setting search params.
Prevents state bleed across branches and repeated mutations.- DASHBOARD_URL.searchParams.set("invitation_token", invitationToken); - return NextResponse.redirect(DASHBOARD_URL); + const dash = new URL(DASHBOARD_URL); + dash.searchParams.set("invitation_token", invitationToken); + return NextResponse.redirect(dash); @@ - DASHBOARD_URL.searchParams.set("invitation_token", invitationToken); - return NextResponse.redirect(DASHBOARD_URL); + const dash = new URL(DASHBOARD_URL); + dash.searchParams.set("invitation_token", invitationToken); + return NextResponse.redirect(dash);Also applies to: 105-107
62-64
: Normalize email compare and preserve token on mismatch/missing org.
Case-insensitive email checks; keep invitation_token so the post-auth handler can recover.- if (user.email !== invitationEmail) { - return NextResponse.redirect(DASHBOARD_URL); - } + if (user.email?.trim().toLowerCase() !== invitationEmail.trim().toLowerCase()) { + const dash = new URL(DASHBOARD_URL); + dash.searchParams.set("invitation_token", invitationToken); + return NextResponse.redirect(dash); + } @@ - if (!organizationId) { - return NextResponse.redirect(DASHBOARD_URL); - } + if (!organizationId) { + const dash = new URL(DASHBOARD_URL); + dash.searchParams.set("invitation_token", invitationToken); + return NextResponse.redirect(dash); + }Also applies to: 66-68
71-76
: Invite acceptance + org switch can leave a partial state; confirm idempotency or use an atomic helper.
If switchOrg fails after accept, the invite may be consumed.Option if available on the provider:
- // Accept invitation first - await auth.acceptInvitation(invitationId); - - // Switch organization and get the new session token - const { newToken, expiresAt } = await auth.switchOrg(organizationId); + // Accept + switch atomically + const { newToken, expiresAt } = await auth.acceptInvitationAndSwitchOrg({ + invitationId, + organizationId, + });If you keep the current flow, please confirm:
- acceptInvitation is idempotent, and
- failed switchOrg does not consume the invite irreversibly.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (5)
apps/dashboard/app/api/auth/accept-invitation/route.ts
(1 hunks)apps/dashboard/app/api/auth/invitation/route.ts
(1 hunks)apps/dashboard/app/auth/actions.ts
(4 hunks)apps/dashboard/app/join/route.ts
(1 hunks)apps/dashboard/lib/auth.ts
(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-05-05T17:55:59.607Z
Learnt from: mcstepp
PR: unkeyed/unkey#3215
File: apps/dashboard/lib/auth/sessions.ts:47-51
Timestamp: 2025-05-05T17:55:59.607Z
Learning: Local auth cookies in apps/dashboard/lib/auth/sessions.ts intentionally omit HttpOnly and Secure flags to allow easier debugging during local development. This is by design as these cookies are only used in local development environments, not production.
Applied to files:
apps/dashboard/app/api/auth/accept-invitation/route.ts
📚 Learning: 2025-09-09T19:25:09.941Z
Learnt from: perkinsjr
PR: unkeyed/unkey#3940
File: apps/dashboard/app/api/auth/accept-invitation/route.ts:7-16
Timestamp: 2025-09-09T19:25:09.941Z
Learning: WorkOS is configured in production to only allow URLs "https://app.unkey.com" to modify tokens, providing CSRF protection at the provider level rather than requiring application-level CSRF checks.
Applied to files:
apps/dashboard/app/api/auth/accept-invitation/route.ts
🧬 Code graph analysis (5)
apps/dashboard/app/join/route.ts (5)
apps/dashboard/lib/auth/types.ts (4)
SIGN_IN_URL
(6-6)SIGN_UP_URL
(7-7)AuthenticatedUser
(23-30)Invitation
(149-161)apps/dashboard/lib/auth/sessions.ts (1)
updateSession
(22-161)apps/dashboard/lib/auth.ts (1)
getCurrentUser
(53-61)apps/dashboard/lib/auth/server.ts (1)
auth
(67-67)apps/dashboard/lib/auth/cookies.ts (1)
setSessionCookie
(105-122)
apps/dashboard/app/api/auth/invitation/route.ts (3)
apps/dashboard/app/api/auth/accept-invitation/route.ts (1)
POST
(7-114)apps/dashboard/lib/auth.ts (2)
getAuth
(29-40)processPostAuthInvitation
(115-157)apps/dashboard/lib/auth/server.ts (1)
auth
(67-67)
apps/dashboard/app/api/auth/accept-invitation/route.ts (4)
apps/dashboard/lib/auth/sessions.ts (1)
updateSession
(22-161)apps/dashboard/lib/auth.ts (2)
getCurrentUser
(53-61)acceptInvitationAndSwitchOrg
(75-102)apps/dashboard/lib/auth/types.ts (1)
Invitation
(149-161)apps/dashboard/lib/auth/server.ts (1)
auth
(67-67)
apps/dashboard/app/auth/actions.ts (3)
apps/dashboard/lib/auth/server.ts (1)
auth
(67-67)apps/dashboard/lib/auth/cookies.ts (2)
setCookies
(42-47)setSessionCookie
(105-122)apps/dashboard/lib/auth/workos.ts (1)
completeOrgSelection
(751-789)
apps/dashboard/lib/auth.ts (4)
apps/dashboard/lib/auth/server.ts (1)
auth
(67-67)apps/dashboard/app/auth/actions.ts (1)
switchOrg
(346-363)apps/dashboard/lib/auth/workos.ts (1)
switchOrg
(285-328)apps/dashboard/lib/trpc/routers/user/switchOrg.ts (1)
switchOrg
(5-24)
⏰ 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: Test API / API Test Local
- GitHub Check: Test Go API Local / Test
- GitHub Check: Build / Build
🔇 Additional comments (2)
apps/dashboard/lib/auth.ts (1)
151-156
: Good: generic error to callers while logging details.This avoids leaking internals in API responses. LGTM.
apps/dashboard/app/auth/actions.ts (1)
106-166
: Verify flow duplication and switching behavior.This block accepts the invite but does not switch org. Given you also run post-auth processing via /api/auth/invitation, confirm users won’t double-accept or end up in the wrong org. If needed, invoke switchOrg here after acceptance.
Would you like me to wire switchOrg here and de-duplicate acceptance across the flow?
@CodeRabbit resolve |
✅ Actions performedComments resolved. |
What does this PR do?
Resolves ENG-2062: Invitations not functioning in enterprise workspace
Type of change
How should this be tested?
IF TESTING LOCALLY
** New User Scenario**
** Existing User Scenarios**
Logged In User
Logged out User
IF TESTING PREVIEW
You have to copy the link out of the email and change the URL to match the preview.
Checklist
Required
pnpm build
pnpm fmt
console.logs
git pull origin main
Appreciated
Summary by CodeRabbit
New Features
Improvements