Skip to content

Commit 639eaf6

Browse files
claude[bot]claudematt-aitken
authored
fix(webapp): don't apply an invite's role to an existing org member (#4409)
<!-- ccr-slack-attribution --> _Requested via [Slack thread](https://triggerdotdev.slack.com/archives/C097ZHVKZFA/p1785249693523749)_ ## Summary Accepting an old invitation could change the role of someone who was already in the organization. A long-pending invite can carry a lower role than the member has since been promoted to, so accepting it was a silent demotion. When the accepting user was the organization's only Owner, the role layer refused that demotion, and the refusal (an expected, protective outcome) was logged as an error. An invitation now only sets a role on a membership the accept actually created, and people who are already in an organization are skipped when invitations are sent. ## How `acceptInvite` already skipped the `OrgMember` create when it found an existing membership, but the `rbac.setUserRole` call below it was gated only on `invite.rbacRoleId`. It now also tracks whether this accept created the membership. A create that loses the unique-constraint race counts as pre-existing, since whichever flow won it owns that membership's role. Skipping existing members outright would regress one case: a member with no RBAC role at all would never receive the invitation's role. `ensureOrgMember` handles that with `healMissingRoleAssignment`, which fills in a null role but never overwrites a real one, so `assignInviteRbacRole` takes the same gate. An established role is never touched; an absent one is filled in. `assignInviteRbacRole` branches on the result's machine-readable `code` instead of logging every refusal at `error`. `last_owner` goes to `logger.info`, matching the two directory-sync role paths; everything else, including a refusal that carries no code, goes to `logger.warn`. The helper is best-effort and never throws, so no outcome it produces warrants `error`. No string matching on the error text is involved. `inviteMembers` resolves the organization's members by email and skips those addresses before creating invites. The invite table's `@@unique([organizationId, email])` only dedupes *pending invites*, so it could never catch this. ## Invite surfaces Skipping addresses means a batch can now come back empty, and neither caller handled that: - The dashboard action built its redirect from `invites[0].organization`, so a batch where every address was skipped threw a `TypeError` that reached the admin as a raw error string. It also reported the submitted count rather than the created one. It now names what it skipped ("No invitations sent: 1 already a member of this organization") and counts what it actually created. - The invites API derived `alreadyInvited` as "everything not created", so an existing member was reported as though they had already been invited. `inviteMembers` now returns the two groups separately and the endpoint reports `alreadyMembers` alongside `alreadyInvited`. ## Testing `apps/webapp/test/member.server.test.ts` passes 16/16 locally, up from 12. Getting there needed a harness fix. The `~/db.server` mock did not export `Prisma`, so any code reaching `PrismaNamespace.PrismaClientKnownRequestError` threw before it could branch, leaving every duplicate-key path in `member.server.ts` unreachable from tests. The mock now re-exports the real `Prisma`, and there is a case covering the pending-invite skip. New cases: the invite role is applied when the accept creates the membership; it is not applied when the member already has a role; it is applied when an existing member has no role assigned; the organization is still joined when the assignment is refused with `last_owner`; and `inviteMembers` reports members separately from pending invites. Forcing the gate off fails exactly the "already has a role" case, so the coverage is load-bearing. `pnpm run typecheck --filter webapp` and `oxfmt --check` both pass. ## Changelog Accepting an old invitation could change the role of someone who was already in the organization. An invitation now leaves an existing member's role untouched, people who are already in an organization are no longer sent invitations to it, and the invite form says which addresses it skipped instead of failing with an unhelpful error. --- ## ✅ Checklist - [x] I have followed every step in the [contributing guide](https://github.com/triggerdotdev/trigger.dev/blob/main/CONTRIBUTING.md) - [x] The PR title follows the convention. - [x] I ran and tested the code works. ## Screenshots No visual changes. The invite form's toast copy changes, as described above. --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Matt Aitken <matt@mattaitken.com>
1 parent 8d321f8 commit 639eaf6

5 files changed

Lines changed: 333 additions & 40 deletions

File tree

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
area: webapp
3+
type: fix
4+
---
5+
6+
Accepting an old invitation could change the role of someone who was already in the organization. An invitation now leaves an existing member's role untouched, people who are already in an organization are no longer sent invitations to it, and the invite form now says which addresses it skipped instead of failing with an unhelpful error.

apps/webapp/app/models/member.server.ts

Lines changed: 48 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,17 @@ export async function inviteMembers({
100100
throw new Error("User does not have access to this organization");
101101
}
102102

103+
const uniqueEmails = new Set(emails);
104+
105+
const existingMembers = await prisma.orgMember.findMany({
106+
where: {
107+
organizationId: org.id,
108+
user: { email: { in: [...uniqueEmails] } },
109+
},
110+
select: { user: { select: { email: true } } },
111+
});
112+
const existingMemberEmails = new Set(existingMembers.map((member) => member.user.email));
113+
103114
// Create one invite per unique email and return ONLY the invites actually
104115
// created by this call. A P2002 means the email is already invited to this org
105116
// (unique org+email) — skip it so one duplicate can't fail the batch, and
@@ -108,8 +119,15 @@ export async function inviteMembers({
108119
const created: Prisma.OrgMemberInviteGetPayload<{
109120
include: { organization: true; inviter: true };
110121
}>[] = [];
122+
const alreadyMembers: string[] = [];
123+
const alreadyInvited: string[] = [];
124+
125+
for (const email of uniqueEmails) {
126+
if (existingMemberEmails.has(email)) {
127+
alreadyMembers.push(email);
128+
continue;
129+
}
111130

112-
for (const email of new Set(emails)) {
113131
try {
114132
const invite = await prisma.orgMemberInvite.create({
115133
data: {
@@ -131,13 +149,14 @@ export async function inviteMembers({
131149
error instanceof PrismaNamespace.PrismaClientKnownRequestError &&
132150
error.code === "P2002"
133151
) {
152+
alreadyInvited.push(email);
134153
continue;
135154
}
136155
throw error;
137156
}
138157
}
139158

140-
return created;
159+
return { created, alreadyMembers, alreadyInvited };
141160
}
142161

143162
export async function getInviteFromToken({ token }: { token: string }) {
@@ -264,24 +283,41 @@ async function assignInviteRbacRole({
264283
userId,
265284
organizationId,
266285
rbacRoleId,
286+
onlyWhenUnassigned,
267287
}: {
268288
userId: string;
269289
organizationId: string;
270290
rbacRoleId: string;
291+
onlyWhenUnassigned: boolean;
271292
}) {
272293
try {
294+
if (onlyWhenUnassigned) {
295+
const currentRole = await rbac.getUserRole({ userId, organizationId });
296+
if (currentRole !== null) {
297+
return;
298+
}
299+
}
300+
273301
const roleResult = await rbac.setUserRole({
274302
userId,
275303
organizationId,
276304
roleId: rbacRoleId,
277305
});
278306
if (!roleResult.ok) {
279-
logger.error("acceptInvite: skipped RBAC role assignment", {
280-
organizationId,
281-
userId,
282-
rbacRoleId,
283-
reason: roleResult.error,
284-
});
307+
if (roleResult.code === "last_owner") {
308+
logger.info("acceptInvite: kept last Owner, skipped RBAC role assignment", {
309+
organizationId,
310+
userId,
311+
rbacRoleId,
312+
});
313+
} else {
314+
logger.warn("acceptInvite: skipped RBAC role assignment", {
315+
organizationId,
316+
userId,
317+
rbacRoleId,
318+
reason: roleResult.error,
319+
});
320+
}
285321
}
286322
} catch (error) {
287323
logger.error("acceptInvite: RBAC role assignment threw", {
@@ -415,6 +451,8 @@ export async function acceptInvite({
415451
},
416452
});
417453

454+
let membershipCreated = false;
455+
418456
if (!member) {
419457
try {
420458
member = await prisma.orgMember.create({
@@ -424,6 +462,7 @@ export async function acceptInvite({
424462
role: invite.role,
425463
},
426464
});
465+
membershipCreated = true;
427466
} catch (error) {
428467
if (
429468
error instanceof PrismaNamespace.PrismaClientKnownRequestError &&
@@ -473,16 +512,12 @@ export async function acceptInvite({
473512

474513
const remainingInvites = await getUsersInvites({ email: user.email });
475514

476-
// If the invite carried an explicit RBAC role, assign it. Best-effort: the
477-
// invite is already consumed and membership created above, so a failure here
478-
// — a returned {ok:false} or a thrown error from the plugin — must not block
479-
// joining the org. Swallow and log either way; without the catch a plugin
480-
// throw escapes and turns the whole invite-accept into a 400.
481515
if (invite.rbacRoleId) {
482516
await assignInviteRbacRole({
483517
userId: user.id,
484518
organizationId: invite.organization.id,
485519
rbacRoleId: invite.rbacRoleId,
520+
onlyWhenUnassigned: !membershipCreated,
486521
});
487522
}
488523

apps/webapp/app/routes/_app.orgs.$organizationSlug.invite/route.tsx

Lines changed: 31 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ import { $replica } from "~/db.server";
2828
import { env } from "~/env.server";
2929
import { useOrganization } from "~/hooks/useOrganizations";
3030
import { inviteMembers } from "~/models/member.server";
31-
import { redirectWithSuccessMessage } from "~/models/message.server";
31+
import { redirectWithErrorMessage, redirectWithSuccessMessage } from "~/models/message.server";
3232
import { resolveOrgIdFromSlug } from "~/models/organization.server";
3333
import { TeamPresenter } from "~/presenters/TeamPresenter.server";
3434
import { scheduleEmail } from "~/services/scheduleEmail.server";
@@ -127,6 +127,20 @@ const schema = z.object({
127127
rbacRoleId: z.string().optional(),
128128
});
129129

130+
function describeSkippedInvites(alreadyMembers: string[], alreadyInvited: string[]) {
131+
const parts: string[] = [];
132+
133+
if (alreadyMembers.length > 0) {
134+
parts.push(simplur`${alreadyMembers.length} already [a member|members] of this organization`);
135+
}
136+
137+
if (alreadyInvited.length > 0) {
138+
parts.push(simplur`${alreadyInvited.length} already invited`);
139+
}
140+
141+
return parts.join(" and ");
142+
}
143+
130144
export const action = dashboardAction(
131145
{
132146
params: Params,
@@ -201,7 +215,11 @@ export const action = dashboardAction(
201215
}
202216

203217
try {
204-
const invites = await inviteMembers({
218+
const {
219+
created: invites,
220+
alreadyMembers,
221+
alreadyInvited,
222+
} = await inviteMembers({
205223
slug: organizationSlug,
206224
emails: submission.value.emails,
207225
userId,
@@ -224,10 +242,19 @@ export const action = dashboardAction(
224242
}
225243
}
226244

245+
const teamPath = organizationTeamPath({ slug: organizationSlug });
246+
const skipped = describeSkippedInvites(alreadyMembers, alreadyInvited);
247+
248+
if (invites.length === 0) {
249+
return redirectWithErrorMessage(teamPath, request, `No invitations sent: ${skipped}.`);
250+
}
251+
227252
return redirectWithSuccessMessage(
228-
organizationTeamPath(invites[0].organization),
253+
teamPath,
229254
request,
230-
simplur`${submission.value.emails.length} member[|s] invited`
255+
skipped
256+
? simplur`${invites.length} member[|s] invited. Skipped ${skipped}.`
257+
: simplur`${invites.length} member[|s] invited`
231258
);
232259
} catch (error: any) {
233260
return json({ errors: { body: error.message } }, { status: 400 });

apps/webapp/app/routes/api.v1.orgs.$orgParam.invites.ts

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -57,9 +57,7 @@ export const action = createActionPATApiRoute(
5757
return json({ error: "Membership is managed by Directory Sync" }, { status: 403 });
5858
}
5959

60-
// Returns only the invites created by this call; already-invited emails are
61-
// skipped (re-sending is the dashboard's dedicated resend flow, not this).
62-
const created = await inviteMembers({
60+
const { created, alreadyMembers, alreadyInvited } = await inviteMembers({
6361
slug: organization.slug,
6462
emails: body.emails,
6563
userId: authentication.userId,
@@ -85,13 +83,11 @@ export const action = createActionPATApiRoute(
8583
// Report per-email outcome so callers aren't misled by an empty list on
8684
// re-invite. 201 when something was created, 200 when everything already
8785
// existed.
88-
const createdEmails = new Set(created.map((invite) => invite.email));
89-
const alreadyInvited = [...new Set(body.emails)].filter((email) => !createdEmails.has(email));
90-
9186
return json(
9287
{
9388
invited: created.map((invite) => ({ id: invite.id, email: invite.email })),
9489
alreadyInvited,
90+
alreadyMembers,
9591
},
9692
{ status: created.length > 0 ? 201 : 200 }
9793
);

0 commit comments

Comments
 (0)