Skip to content

Commit a617b47

Browse files
icecrasher321claude
andcommitted
fix(invitations): close the two accept-disclosure gaps Bugbot found
Membership notice ignored the join preview. `buildMembershipNotice` keyed off the invitation's sent `membershipIntent`, but acceptance resolves an internal invite to external when the invitee already belongs to another organization, or when the granted workspace changed organizations after the invite went out. The screen therefore promised "you'll join as a member, which uses one of their seats" to people who would neither join nor consume a seat. It now keys on the preview's `willJoinOrganization` — the same signal the migration notice already used — and falls back to the sent intent only when no preview could be computed. In-app accept skipped the disclosure entirely. `useAcceptMyInvitation` posted an empty body, so `disclosedWorkspaceIds` was absent and the server's consent guard was skipped, and the pending-invitations modal never showed which owned workspaces would move. Accepting from the workspace switcher (including the desktop path) could silently sweep personal workspaces into the organization, bypassing the consent model this PR adds on /invite. The list endpoint now returns each invitation's join preview, the modal renders the same membership/migration disclosure as /invite, and accept echoes `disclosedWorkspaceIds` so the guard applies on both paths. Both notices moved into lib/invitations/disclosure-copy.ts and are consumed by /invite and the modal, so the two accept surfaces cannot drift into disclosing different outcomes for the same invitation — which is how this gap arose. Co-Authored-By: Claude <noreply@anthropic.com>
1 parent 657ba06 commit a617b47

7 files changed

Lines changed: 394 additions & 100 deletions

File tree

apps/sim/app/api/invitations/route.ts

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
import { createLogger } from '@sim/logger'
22
import { NextResponse } from 'next/server'
3-
import type { InvitationDetails } from '@/lib/api/contracts/invitations'
3+
import type { MyInvitation } from '@/lib/api/contracts/invitations'
44
import { getSession } from '@/lib/auth'
55
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
6-
import { listPendingInvitationsForEmail } from '@/lib/invitations/core'
6+
import { getInvitationJoinPreview, listPendingInvitationsForEmail } from '@/lib/invitations/core'
77

88
const logger = createLogger('MyInvitationsAPI')
99

@@ -22,9 +22,30 @@ export const GET = withRouteHandler(async () => {
2222
try {
2323
const invitations = await listPendingInvitationsForEmail(session.user.email)
2424

25+
/**
26+
* Each row carries what accepting it will actually do, so the in-app list
27+
* can disclose the workspace migration and echo `disclosedWorkspaceIds` on
28+
* accept — the same consent contract the emailed `/invite` page honours.
29+
* Disclosure-only, so a preview failure degrades to `null` (the client
30+
* shows a generic notice) rather than hiding the invitation.
31+
*/
32+
const previews = await Promise.all(
33+
invitations.map(async (inv) => {
34+
try {
35+
return await getInvitationJoinPreview(session.user.id, inv)
36+
} catch (previewError) {
37+
logger.warn('Failed to compute join preview for pending invitation', {
38+
invitationId: inv.id,
39+
error: previewError,
40+
})
41+
return null
42+
}
43+
})
44+
)
45+
2546
return NextResponse.json({
2647
invitations: invitations.map(
27-
(inv) =>
48+
(inv, index) =>
2849
({
2950
id: inv.id,
3051
kind: inv.kind,
@@ -43,7 +64,8 @@ export const GET = withRouteHandler(async () => {
4364
workspaceName: grant.workspaceName,
4465
permission: grant.permission,
4566
})),
46-
}) satisfies InvitationDetails
67+
joinPreview: previews[index],
68+
}) satisfies MyInvitation
4769
),
4870
})
4971
} catch (error) {

apps/sim/app/invite/[id]/invite.tsx

Lines changed: 18 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,13 @@ import { useQueryClient } from '@tanstack/react-query'
99
import { useParams, useRouter, useSearchParams } from 'next/navigation'
1010
import { ApiClientError } from '@/lib/api/client/errors'
1111
import { requestJson } from '@/lib/api/client/request'
12-
import {
13-
acceptInvitationContract,
14-
type InvitationJoinPreview,
15-
} from '@/lib/api/contracts/invitations'
12+
import { acceptInvitationContract } from '@/lib/api/contracts/invitations'
1613
import { client, useSession } from '@/lib/auth/auth-client'
14+
import {
15+
buildMembershipNotice,
16+
buildWorkspaceMigrationNotice,
17+
MAX_LISTED_WORKSPACE_NAMES,
18+
} from '@/lib/invitations/disclosure-copy'
1719
import { InviteLayout, InviteStatusCard } from '@/app/invite/components'
1820
import { useInvitationDetails } from '@/hooks/queries/invitations'
1921
import { organizationKeys } from '@/hooks/queries/organization'
@@ -172,53 +174,6 @@ function getInviteError(code: string): InviteError {
172174
)
173175
}
174176

175-
const MAX_LISTED_WORKSPACE_NAMES = 3
176-
177-
/**
178-
* Disclosure appended to the accept copy when accepting moves the invitee's
179-
* own workspaces into the organization — said where the decision happens, so
180-
* accepting never silently changes who controls their work.
181-
*/
182-
function buildWorkspaceMigrationNotice(
183-
joinPreview: InvitationJoinPreview | null,
184-
organizationLabel: string
185-
): string {
186-
if (!joinPreview?.willJoinOrganization || joinPreview.workspacesToMove.length === 0) {
187-
return ''
188-
}
189-
190-
const names = joinPreview.workspacesToMove
191-
const nameList = formatQuotedNameList(names, MAX_LISTED_WORKSPACE_NAMES)
192-
const single = names.length === 1
193-
194-
return ` Accepting also moves your ${single ? 'workspace' : 'workspaces'} ${nameList} into ${organizationLabel}: its admins get full access, and ${single ? 'it stays' : 'they stay'} with the organization if you leave.`
195-
}
196-
197-
/**
198-
* States what the invitee becomes, so the seat consequence is disclosed to the
199-
* person it applies to rather than only to the inviter. Said unconditionally
200-
* for membership invites — the migration notice is empty for an invitee who
201-
* owns no workspaces, and joining is the larger consequence either way.
202-
*/
203-
function buildMembershipNotice(
204-
membershipIntent: 'internal' | 'external' | undefined,
205-
organizationRole: string | undefined,
206-
organizationLabel: string,
207-
isOrganizationScoped: boolean
208-
): string {
209-
if (!isOrganizationScoped || !membershipIntent) return ''
210-
211-
if (membershipIntent === 'external') {
212-
return ` You'll join as an external collaborator: you get access to the ${
213-
organizationLabel === 'the organization' ? 'invited' : `${organizationLabel}`
214-
} workspaces only, you don't take one of their seats, and everything you own stays yours.`
215-
}
216-
217-
return ` You'll join ${organizationLabel} as ${
218-
organizationRole && isOrgAdminRole(organizationRole) ? 'an admin' : 'a member'
219-
}, which uses one of their seats.`
220-
}
221-
222177
function codeFromStatus(status: number): InviteErrorCode {
223178
if (status === 401) return 'unauthorized'
224179
if (status === 403) return 'forbidden'
@@ -547,20 +502,23 @@ export default function Invite() {
547502
* migration notice for membership invites — a missing preview must never
548503
* read as "nothing moves".
549504
*/
550-
const migrationNotice =
551-
joinPreviewUnavailable && invitation?.membershipIntent !== 'external'
552-
? ` If you own personal workspaces, accepting membership moves them into ${organizationLabel}: its admins get full access, and they stay with the organization if you leave.`
553-
: buildWorkspaceMigrationNotice(joinPreview, organizationLabel)
505+
const migrationNotice = buildWorkspaceMigrationNotice({
506+
joinPreview,
507+
joinPreviewUnavailable,
508+
membershipIntent: invitation?.membershipIntent,
509+
organizationLabel,
510+
})
554511
/**
555512
* Only disclosed when the invitation actually carries organization standing —
556513
* a personal-workspace invite has no seat or membership to explain.
557514
*/
558-
const membershipNotice = buildMembershipNotice(
559-
invitation?.membershipIntent,
560-
invitation?.role,
515+
const membershipNotice = buildMembershipNotice({
516+
joinPreview,
517+
membershipIntent: invitation?.membershipIntent,
518+
isOrganizationAdminRole: Boolean(invitation?.role && isOrgAdminRole(invitation.role)),
561519
organizationLabel,
562-
Boolean(invitation?.organizationId || joinPreview?.organizationName)
563-
)
520+
isOrganizationScoped: Boolean(invitation?.organizationId || joinPreview?.organizationName),
521+
})
564522

565523
return (
566524
<InviteLayout>

apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/workspace-header/components/pending-invitations/view-invitations-modal.tsx

Lines changed: 70 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,14 @@
22

33
import { Chip, ChipModal, ChipModalBody, ChipModalFooter, ChipModalHeader, toast } from '@sim/emcn'
44
import { createLogger } from '@sim/logger'
5+
import { isOrgAdminRole } from '@sim/platform-authz/workspace'
56
import { getErrorMessage } from '@sim/utils/errors'
67
import { useRouter } from 'next/navigation'
7-
import type { InvitationDetails } from '@/lib/api/contracts/invitations'
8+
import type { MyInvitation } from '@/lib/api/contracts/invitations'
9+
import {
10+
buildMembershipNotice,
11+
buildWorkspaceMigrationNotice,
12+
} from '@/lib/invitations/disclosure-copy'
813
import { getInvitationErrorMessage } from '@/lib/invitations/error-messages'
914
import {
1015
useAcceptMyInvitation,
@@ -19,7 +24,7 @@ const logger = createLogger('ViewInvitationsModal')
1924
* invites are labeled by the org (even when workspace grants ride along);
2025
* workspace invites by their workspace(s).
2126
*/
22-
function invitationLabel(inv: InvitationDetails): string {
27+
function invitationLabel(inv: MyInvitation): string {
2328
if (inv.kind === 'organization') {
2429
return inv.organizationName ?? 'Organization'
2530
}
@@ -32,12 +37,37 @@ function invitationLabel(inv: InvitationDetails): string {
3237
}
3338

3439
/** Secondary line: who invited, plus role (org) or permission (workspace). */
35-
function invitationSubLabel(inv: InvitationDetails): string {
40+
function invitationSubLabel(inv: MyInvitation): string {
3641
const invitedBy = inv.inviterName ? `Invited by ${inv.inviterName}` : 'Invited'
3742
const detail = inv.kind === 'organization' ? inv.role : inv.grants[0]?.permission
3843
return detail ? `${invitedBy} · ${detail}` : invitedBy
3944
}
4045

46+
/**
47+
* What accepting will do to the invitee's own account: the seat/membership
48+
* consequence and any workspaces that will move into the organization. Built
49+
* from the same copy as the emailed `/invite` page so the two accept surfaces
50+
* can never disclose different outcomes.
51+
*/
52+
function invitationDisclosure(inv: MyInvitation): string {
53+
const organizationLabel =
54+
inv.joinPreview?.organizationName ?? inv.organizationName ?? 'the organization'
55+
const membership = buildMembershipNotice({
56+
joinPreview: inv.joinPreview,
57+
membershipIntent: inv.membershipIntent,
58+
isOrganizationAdminRole: isOrgAdminRole(inv.role),
59+
organizationLabel,
60+
isOrganizationScoped: Boolean(inv.organizationId || inv.joinPreview?.organizationName),
61+
})
62+
const migration = buildWorkspaceMigrationNotice({
63+
joinPreview: inv.joinPreview,
64+
joinPreviewUnavailable: inv.joinPreview === null,
65+
membershipIntent: inv.membershipIntent,
66+
organizationLabel,
67+
})
68+
return `${membership}${migration}`.trim()
69+
}
70+
4171
interface ViewInvitationsModalProps {
4272
open: boolean
4373
onOpenChange: (open: boolean) => void
@@ -58,9 +88,12 @@ export function ViewInvitationsModal({ open, onOpenChange }: ViewInvitationsModa
5888

5989
const isBusy = acceptInvitation.isPending || declineInvitation.isPending
6090

61-
const handleAccept = async (inv: InvitationDetails) => {
91+
const handleAccept = async (inv: MyInvitation) => {
6292
try {
63-
const result = await acceptInvitation.mutateAsync({ invitationId: inv.id })
93+
const result = await acceptInvitation.mutateAsync({
94+
invitationId: inv.id,
95+
disclosedWorkspaceIds: inv.joinPreview?.workspaceIdsToMove,
96+
})
6497
toast.success(`Joined ${invitationLabel(inv)}`)
6598
onOpenChange(false)
6699
router.push(result.redirectPath)
@@ -75,7 +108,7 @@ export function ViewInvitationsModal({ open, onOpenChange }: ViewInvitationsModa
75108
}
76109
}
77110

78-
const handleDecline = async (inv: InvitationDetails) => {
111+
const handleDecline = async (inv: MyInvitation) => {
79112
try {
80113
await declineInvitation.mutateAsync({ invitationId: inv.id })
81114
} catch (error) {
@@ -93,32 +126,38 @@ export function ViewInvitationsModal({ open, onOpenChange }: ViewInvitationsModa
93126
{!invitations || invitations.length === 0 ? (
94127
<p className='px-2 text-[var(--text-muted)] text-sm'>No pending invitations.</p>
95128
) : (
96-
invitations.map((inv) => (
97-
<div key={inv.id} className='flex items-center gap-2 px-2'>
98-
<div className='min-w-0 flex-1'>
99-
<p className='truncate text-[var(--text-body)] text-sm'>{invitationLabel(inv)}</p>
100-
<p className='truncate text-[var(--text-muted)] text-caption'>
101-
{invitationSubLabel(inv)}
102-
</p>
129+
invitations.map((inv) => {
130+
const disclosure = invitationDisclosure(inv)
131+
return (
132+
<div key={inv.id} className='flex items-start gap-2 px-2'>
133+
<div className='min-w-0 flex-1'>
134+
<p className='truncate text-[var(--text-body)] text-sm'>{invitationLabel(inv)}</p>
135+
<p className='truncate text-[var(--text-muted)] text-caption'>
136+
{invitationSubLabel(inv)}
137+
</p>
138+
{disclosure ? (
139+
<p className='mt-1 text-[var(--text-muted)] text-caption'>{disclosure}</p>
140+
) : null}
141+
</div>
142+
<Chip
143+
variant='primary'
144+
disabled={isBusy}
145+
onClick={() => void handleAccept(inv)}
146+
className='flex-shrink-0'
147+
>
148+
Accept
149+
</Chip>
150+
<Chip
151+
disabled={isBusy}
152+
onClick={() => void handleDecline(inv)}
153+
aria-label={`Decline invitation to ${invitationLabel(inv)}`}
154+
className='flex-shrink-0'
155+
>
156+
Decline
157+
</Chip>
103158
</div>
104-
<Chip
105-
variant='primary'
106-
disabled={isBusy}
107-
onClick={() => void handleAccept(inv)}
108-
className='flex-shrink-0'
109-
>
110-
Accept
111-
</Chip>
112-
<Chip
113-
disabled={isBusy}
114-
onClick={() => void handleDecline(inv)}
115-
aria-label={`Decline invitation to ${invitationLabel(inv)}`}
116-
className='flex-shrink-0'
117-
>
118-
Decline
119-
</Chip>
120-
</div>
121-
))
159+
)
160+
})
122161
)}
123162
</ChipModalBody>
124163
<ChipModalFooter

apps/sim/hooks/queries/invitations.ts

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,9 @@ import {
77
batchWorkspaceInvitationsContract,
88
cancelInvitationContract,
99
getInvitationContract,
10-
type InvitationDetails,
1110
listMyInvitationsContract,
1211
listWorkspaceInvitationsContract,
12+
type MyInvitation,
1313
type PendingInvitationRow,
1414
rejectInvitationContract,
1515
removeWorkspaceMemberContract,
@@ -121,7 +121,7 @@ export function usePendingInvitations(workspaceId: string | undefined) {
121121

122122
export const MY_INVITATIONS_STALE_TIME = 30 * 1000
123123

124-
async function fetchMyPendingInvitations(signal?: AbortSignal): Promise<InvitationDetails[]> {
124+
async function fetchMyPendingInvitations(signal?: AbortSignal): Promise<MyInvitation[]> {
125125
const data = await requestJson(listMyInvitationsContract, { signal })
126126
return data.invitations
127127
}
@@ -157,8 +157,23 @@ export function useAcceptMyInvitation() {
157157
const queryClient = useQueryClient()
158158

159159
return useMutation({
160-
mutationFn: async ({ invitationId }: { invitationId: string }) =>
161-
requestJson(acceptInvitationContract, { params: { id: invitationId }, body: {} }),
160+
/**
161+
* `disclosedWorkspaceIds` echoes the migration the caller actually showed the
162+
* invitee, so acceptance rejects if the sweep set changed since. Omitting it
163+
* would silently skip that guard — the in-app path must supply it for the
164+
* same reason the emailed `/invite` page does.
165+
*/
166+
mutationFn: async ({
167+
invitationId,
168+
disclosedWorkspaceIds,
169+
}: {
170+
invitationId: string
171+
disclosedWorkspaceIds?: string[]
172+
}) =>
173+
requestJson(acceptInvitationContract, {
174+
params: { id: invitationId },
175+
body: { disclosedWorkspaceIds },
176+
}),
162177
onSuccess: () => {
163178
queryClient.invalidateQueries({ queryKey: workspaceKeys.lists() })
164179
queryClient.invalidateQueries({ queryKey: organizationKeys.all })

apps/sim/lib/api/contracts/invitations.ts

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -217,13 +217,27 @@ export const getInvitationContract = defineRouteContract({
217217
},
218218
})
219219

220+
/**
221+
* A pending invitation plus what accepting it will actually do. The preview
222+
* rides along so the in-app list can disclose the workspace migration and echo
223+
* `disclosedWorkspaceIds` on accept, exactly like the emailed `/invite` page —
224+
* an accept surface without it would sweep workspaces without consent. `null`
225+
* when the preview could not be computed; the client then shows the generic
226+
* notice rather than treating it as "nothing moves".
227+
*/
228+
export const myInvitationSchema = invitationDetailsSchema.extend({
229+
joinPreview: invitationJoinPreviewSchema.nullable(),
230+
})
231+
232+
export type MyInvitation = z.output<typeof myInvitationSchema>
233+
220234
export const listMyInvitationsContract = defineRouteContract({
221235
method: 'GET',
222236
path: '/api/invitations',
223237
response: {
224238
mode: 'json',
225239
schema: z.object({
226-
invitations: z.array(invitationDetailsSchema),
240+
invitations: z.array(myInvitationSchema),
227241
}),
228242
},
229243
})

0 commit comments

Comments
 (0)