From 9b930999ad1f4f8ac7315c3ccef1eb9eb9b78bbd Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 28 Jul 2026 15:49:46 +0000 Subject: [PATCH 1/3] fix(webapp): don't apply an invite's role to an existing org member MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `acceptInvite` looked up an existing OrgMember and skipped creating one when it found one, but still passed the invite's `rbacRoleId` to `rbac.setUserRole` regardless. For a user who was already a member that is a role *change*, not an initial assignment, so accepting a long-pending invite that carries a lesser role attempted to demote them — including demoting an org's sole Owner, which the role layer then correctly refuses. - `acceptInvite` now tracks whether this accept created the membership and only applies the invite's role in that case, following the rule `ensureOrgMember` already documents. A create that loses the unique-constraint race counts as pre-existing, since whichever flow won it owns that membership's role. - `assignInviteRbacRole` classifies a refusal instead of logging every one at `error`: a `last_owner` code logs at `info`, matching the directory-sync role paths, and anything else — including a refusal that carries no code — logs at `warn`. The helper is best-effort and never throws, so it was logging an expected outcome at an inappropriate severity. - `inviteMembers` skips emails that already belong to a member of the org. The invite table's unique constraint only dedupes pending invites, so nothing previously stopped an existing member being invited again. Skipped addresses are reported the same way an already-pending invite is: left out of the returned list. Adds coverage for all three in apps/webapp/test/member.server.test.ts. --- ...invite-role-change-for-existing-members.md | 6 + apps/webapp/app/models/member.server.ts | 80 +++++++-- apps/webapp/test/member.server.test.ts | 168 +++++++++++++++++- 3 files changed, 239 insertions(+), 15 deletions(-) create mode 100644 .server-changes/fix-invite-role-change-for-existing-members.md diff --git a/.server-changes/fix-invite-role-change-for-existing-members.md b/.server-changes/fix-invite-role-change-for-existing-members.md new file mode 100644 index 00000000000..fec4dd25362 --- /dev/null +++ b/.server-changes/fix-invite-role-change-for-existing-members.md @@ -0,0 +1,6 @@ +--- +area: webapp +type: fix +--- + +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, and people who are already in an organization are no longer sent invitations to it. diff --git a/apps/webapp/app/models/member.server.ts b/apps/webapp/app/models/member.server.ts index b786ce7e352..3cf656f5c73 100644 --- a/apps/webapp/app/models/member.server.ts +++ b/apps/webapp/app/models/member.server.ts @@ -100,6 +100,24 @@ export async function inviteMembers({ throw new Error("User does not have access to this organization"); } + const uniqueEmails = new Set(emails); + + // Emails that already belong to a member of this org. The unique constraint + // on the invite table dedupes against *pending invites* only, so nothing else + // stops an existing member being invited again — and accepting such an invite + // is a role change on an established membership, not a join (see + // `acceptInvite`). Skip those emails and report them exactly like an + // already-pending one: left out of the returned list, without failing the + // rest of the batch. + const existingMembers = await prisma.orgMember.findMany({ + where: { + organizationId: org.id, + user: { email: { in: [...uniqueEmails] } }, + }, + select: { user: { select: { email: true } } }, + }); + const existingMemberEmails = new Set(existingMembers.map((member) => member.user.email)); + // Create one invite per unique email and return ONLY the invites actually // created by this call. A P2002 means the email is already invited to this org // (unique org+email) — skip it so one duplicate can't fail the batch, and @@ -109,7 +127,11 @@ export async function inviteMembers({ include: { organization: true; inviter: true }; }>[] = []; - for (const email of new Set(emails)) { + for (const email of uniqueEmails) { + if (existingMemberEmails.has(email)) { + continue; + } + try { const invite = await prisma.orgMemberInvite.create({ data: { @@ -276,12 +298,29 @@ async function assignInviteRbacRole({ roleId: rbacRoleId, }); if (!roleResult.ok) { - logger.error("acceptInvite: skipped RBAC role assignment", { - organizationId, - userId, - rbacRoleId, - reason: roleResult.error, - }); + // An org must keep at least one Owner, so the plugin refuses a change + // that would remove the last one. That's expected, not a failure — the + // same outcome the directory-sync role paths log at info. + if (roleResult.code === "last_owner") { + logger.info("acceptInvite: kept last Owner, skipped RBAC role assignment", { + organizationId, + userId, + rbacRoleId, + }); + } else { + // Any other refusal (plan gating, no plugin installed, a validation + // conflict) is a documented `{ok:false}` outcome of this contract, and + // this helper is best-effort and never throws — the membership stands + // either way. Log it and move on rather than treating it as an error. + // Unrecognised codes land here too, so a refusal that carries no code + // is still reported at a level that matches its impact. + logger.warn("acceptInvite: skipped RBAC role assignment", { + organizationId, + userId, + rbacRoleId, + reason: roleResult.error, + }); + } } } catch (error) { logger.error("acceptInvite: RBAC role assignment threw", { @@ -415,6 +454,13 @@ export async function acceptInvite({ }, }); + // Whether this accept is what made the user a member. Only a membership + // created here gets the invite's RBAC role applied further down — for anyone + // who was already a member, applying it would be a role change rather than an + // initial assignment. A create that lost the P2002 race counts as existing + // too: whoever won it owns the role for that membership. + let membershipCreated = false; + if (!member) { try { member = await prisma.orgMember.create({ @@ -424,6 +470,7 @@ export async function acceptInvite({ role: invite.role, }, }); + membershipCreated = true; } catch (error) { if ( error instanceof PrismaNamespace.PrismaClientKnownRequestError && @@ -473,12 +520,19 @@ export async function acceptInvite({ const remainingInvites = await getUsersInvites({ email: user.email }); - // If the invite carried an explicit RBAC role, assign it. Best-effort: the - // invite is already consumed and membership created above, so a failure here - // — a returned {ok:false} or a thrown error from the plugin — must not block - // joining the org. Swallow and log either way; without the catch a plugin - // throw escapes and turns the whole invite-accept into a 400. - if (invite.rbacRoleId) { + // If the invite carried an explicit RBAC role, assign it — but only to a + // membership this accept actually created. For someone who was already a + // member this is a role *change*, not an initial assignment: we do NOT touch + // the existing role to avoid demoting a member who has since been promoted + // (a long-pending invite can carry a lesser role than they now hold), the + // same rule `ensureOrgMember` follows. + // + // Best-effort for a new member: the invite is already consumed and membership + // created above, so a failure here — a returned {ok:false} or a thrown error + // from the plugin — must not block joining the org. Swallow and log either + // way; without the catch a plugin throw escapes and turns the whole + // invite-accept into a 400. + if (invite.rbacRoleId && membershipCreated) { await assignInviteRbacRole({ userId: user.id, organizationId: invite.organization.id, diff --git a/apps/webapp/test/member.server.test.ts b/apps/webapp/test/member.server.test.ts index 8d4a9128642..62eb777601e 100644 --- a/apps/webapp/test/member.server.test.ts +++ b/apps/webapp/test/member.server.test.ts @@ -6,12 +6,28 @@ const prismaHolder = vi.hoisted(() => ({ client: null as PrismaClient | null, })); +type SetUserRoleParams = { userId: string; organizationId: string; roleId: string }; +type SetUserRoleResult = { ok: true } | { ok: false; error: string; code?: "last_owner" }; + +const rbacHolder = vi.hoisted(() => ({ + setUserRoleCalls: [] as SetUserRoleParams[], + setUserRoleResult: { ok: true } as SetUserRoleResult, +})); + vi.mock("~/services/rbac.server", () => ({ rbac: { - setUserRole: async () => ({ ok: true as const }), + setUserRole: async (params: SetUserRoleParams) => { + rbacHolder.setUserRoleCalls.push(params); + return rbacHolder.setUserRoleResult; + }, }, })); +function resetRbac(result: SetUserRoleResult = { ok: true }) { + rbacHolder.setUserRoleCalls.length = 0; + rbacHolder.setUserRoleResult = result; +} + vi.mock("~/db.server", () => ({ get prisma() { if (!prismaHolder.client) { @@ -39,7 +55,7 @@ function randomHex(len = 12): string { async function seedInviteFixture( prisma: PrismaClient, - opts: { activeProjectCount: number; deletedProjectCount?: number } + opts: { activeProjectCount: number; deletedProjectCount?: number; rbacRoleId?: string } ) { const suffix = randomHex(8); const inviter = await prisma.user.create({ @@ -99,6 +115,7 @@ async function seedInviteFixture( organizationId: organization.id, inviterId: inviter.id, role: "MEMBER", + rbacRoleId: opts.rbacRoleId ?? null, }, }); @@ -253,6 +270,153 @@ describe("acceptInvite", () => { expect(member).toBeNull(); } ); + + postgresTest( + "applies the invite RBAC role when it creates the membership", + { timeout: 60_000 }, + async ({ prisma }) => { + prismaHolder.client = prisma; + resetRbac(); + const { acceptInvite } = await import("../app/models/member.server"); + + const { invitee, organization, invite } = await seedInviteFixture(prisma, { + activeProjectCount: 1, + rbacRoleId: "role_admin", + }); + + await acceptInvite({ + inviteId: invite.id, + organizationId: organization.id, + user: { id: invitee.id, email: invitee.email }, + }); + + expect(rbacHolder.setUserRoleCalls).toEqual([ + { userId: invitee.id, organizationId: organization.id, roleId: "role_admin" }, + ]); + } + ); + + postgresTest( + "does not apply the invite RBAC role to a user who is already a member", + { timeout: 60_000 }, + async ({ prisma }) => { + prismaHolder.client = prisma; + resetRbac(); + const { acceptInvite } = await import("../app/models/member.server"); + + const { invitee, organization, invite } = await seedInviteFixture(prisma, { + activeProjectCount: 1, + rbacRoleId: "role_admin", + }); + + // The user is already in the org, so accepting the invite must not touch + // their role — it could be a demotion. + await prisma.orgMember.create({ + data: { + organizationId: organization.id, + userId: invitee.id, + role: "MEMBER", + }, + }); + + await acceptInvite({ + inviteId: invite.id, + organizationId: organization.id, + user: { id: invitee.id, email: invitee.email }, + }); + + expect(rbacHolder.setUserRoleCalls).toEqual([]); + + // The invite is still consumed and the membership left intact. + const remainingInvite = await prisma.orgMemberInvite.findFirst({ + where: { id: invite.id }, + }); + expect(remainingInvite).toBeNull(); + + const member = await prisma.orgMember.findFirst({ + where: { userId: invitee.id, organizationId: organization.id }, + }); + expect(member).not.toBeNull(); + } + ); + + postgresTest( + "still joins the org when the RBAC role assignment is refused", + { timeout: 60_000 }, + async ({ prisma }) => { + prismaHolder.client = prisma; + resetRbac({ + ok: false, + error: "An organisation must have at least one Owner", + code: "last_owner", + }); + const { acceptInvite } = await import("../app/models/member.server"); + + const { invitee, organization, invite } = await seedInviteFixture(prisma, { + activeProjectCount: 1, + rbacRoleId: "role_admin", + }); + + const { organization: joinedOrg } = await acceptInvite({ + inviteId: invite.id, + organizationId: organization.id, + user: { id: invitee.id, email: invitee.email }, + }); + + expect(joinedOrg.id).toBe(organization.id); + expect(rbacHolder.setUserRoleCalls).toHaveLength(1); + + const member = await prisma.orgMember.findFirst({ + where: { userId: invitee.id, organizationId: organization.id }, + }); + expect(member).not.toBeNull(); + + resetRbac(); + } + ); +}); + +describe("inviteMembers", () => { + postgresTest( + "skips emails that already belong to a member of the org", + { timeout: 60_000 }, + async ({ prisma }) => { + prismaHolder.client = prisma; + const { inviteMembers } = await import("../app/models/member.server"); + + const { inviter, invitee, organization, invite } = await seedInviteFixture(prisma, { + activeProjectCount: 0, + }); + + // Drop the seeded pending invite so the skip can only come from the + // membership check, not from the pending-invite dedupe. + await prisma.orgMemberInvite.delete({ where: { id: invite.id } }); + + await prisma.orgMember.create({ + data: { + organizationId: organization.id, + userId: invitee.id, + role: "MEMBER", + }, + }); + + const newcomerEmail = `newcomer-${randomHex(8)}@test.local`; + + const created = await inviteMembers({ + slug: organization.slug, + emails: [invitee.email, newcomerEmail], + userId: inviter.id, + }); + + expect(created.map((row) => row.email)).toEqual([newcomerEmail]); + + const invites = await prisma.orgMemberInvite.findMany({ + where: { organizationId: organization.id }, + select: { email: true }, + }); + expect(invites.map((pending) => pending.email)).toEqual([newcomerEmail]); + } + ); }); describe("provisionMemberDevelopmentEnvironments", () => { From 3ad14936c991c544ecadbba44e82d31e98887519 Mon Sep 17 00:00:00 2001 From: Matt Aitken Date: Tue, 28 Jul 2026 19:20:10 +0100 Subject: [PATCH 2/3] chore(webapp): strip explanatory comments from the invite role fix --- apps/webapp/app/models/member.server.ts | 33 ------------------------- apps/webapp/test/member.server.test.ts | 5 ---- 2 files changed, 38 deletions(-) diff --git a/apps/webapp/app/models/member.server.ts b/apps/webapp/app/models/member.server.ts index 3cf656f5c73..9bdf7b8991d 100644 --- a/apps/webapp/app/models/member.server.ts +++ b/apps/webapp/app/models/member.server.ts @@ -102,13 +102,6 @@ export async function inviteMembers({ const uniqueEmails = new Set(emails); - // Emails that already belong to a member of this org. The unique constraint - // on the invite table dedupes against *pending invites* only, so nothing else - // stops an existing member being invited again — and accepting such an invite - // is a role change on an established membership, not a join (see - // `acceptInvite`). Skip those emails and report them exactly like an - // already-pending one: left out of the returned list, without failing the - // rest of the batch. const existingMembers = await prisma.orgMember.findMany({ where: { organizationId: org.id, @@ -298,9 +291,6 @@ async function assignInviteRbacRole({ roleId: rbacRoleId, }); if (!roleResult.ok) { - // An org must keep at least one Owner, so the plugin refuses a change - // that would remove the last one. That's expected, not a failure — the - // same outcome the directory-sync role paths log at info. if (roleResult.code === "last_owner") { logger.info("acceptInvite: kept last Owner, skipped RBAC role assignment", { organizationId, @@ -308,12 +298,6 @@ async function assignInviteRbacRole({ rbacRoleId, }); } else { - // Any other refusal (plan gating, no plugin installed, a validation - // conflict) is a documented `{ok:false}` outcome of this contract, and - // this helper is best-effort and never throws — the membership stands - // either way. Log it and move on rather than treating it as an error. - // Unrecognised codes land here too, so a refusal that carries no code - // is still reported at a level that matches its impact. logger.warn("acceptInvite: skipped RBAC role assignment", { organizationId, userId, @@ -454,11 +438,6 @@ export async function acceptInvite({ }, }); - // Whether this accept is what made the user a member. Only a membership - // created here gets the invite's RBAC role applied further down — for anyone - // who was already a member, applying it would be a role change rather than an - // initial assignment. A create that lost the P2002 race counts as existing - // too: whoever won it owns the role for that membership. let membershipCreated = false; if (!member) { @@ -520,18 +499,6 @@ export async function acceptInvite({ const remainingInvites = await getUsersInvites({ email: user.email }); - // If the invite carried an explicit RBAC role, assign it — but only to a - // membership this accept actually created. For someone who was already a - // member this is a role *change*, not an initial assignment: we do NOT touch - // the existing role to avoid demoting a member who has since been promoted - // (a long-pending invite can carry a lesser role than they now hold), the - // same rule `ensureOrgMember` follows. - // - // Best-effort for a new member: the invite is already consumed and membership - // created above, so a failure here — a returned {ok:false} or a thrown error - // from the plugin — must not block joining the org. Swallow and log either - // way; without the catch a plugin throw escapes and turns the whole - // invite-accept into a 400. if (invite.rbacRoleId && membershipCreated) { await assignInviteRbacRole({ userId: user.id, diff --git a/apps/webapp/test/member.server.test.ts b/apps/webapp/test/member.server.test.ts index 62eb777601e..5d79bf09f9b 100644 --- a/apps/webapp/test/member.server.test.ts +++ b/apps/webapp/test/member.server.test.ts @@ -309,8 +309,6 @@ describe("acceptInvite", () => { rbacRoleId: "role_admin", }); - // The user is already in the org, so accepting the invite must not touch - // their role — it could be a demotion. await prisma.orgMember.create({ data: { organizationId: organization.id, @@ -327,7 +325,6 @@ describe("acceptInvite", () => { expect(rbacHolder.setUserRoleCalls).toEqual([]); - // The invite is still consumed and the membership left intact. const remainingInvite = await prisma.orgMemberInvite.findFirst({ where: { id: invite.id }, }); @@ -388,8 +385,6 @@ describe("inviteMembers", () => { activeProjectCount: 0, }); - // Drop the seeded pending invite so the skip can only come from the - // membership check, not from the pending-invite dedupe. await prisma.orgMemberInvite.delete({ where: { id: invite.id } }); await prisma.orgMember.create({ From 9b60a421511754ea4094ff6396e3dbe59493c934 Mon Sep 17 00:00:00 2001 From: Matt Aitken Date: Tue, 28 Jul 2026 19:20:45 +0100 Subject: [PATCH 3/3] fix(webapp): report skipped invites instead of failing, and heal a member with no role The invite form crashed when every address in a batch was skipped, and reported the submitted count rather than the created one. It now names what it skipped and why. An invitation still leaves an established role alone, but an existing member with no role assigned gets the invitation role, matching how ensureOrgMember completes an interrupted assignment. The invites API reports skipped members separately from skipped pending invites. --- ...invite-role-change-for-existing-members.md | 2 +- apps/webapp/app/models/member.server.ts | 18 ++- .../route.tsx | 35 +++++- .../routes/api.v1.orgs.$orgParam.invites.ts | 8 +- apps/webapp/test/member.server.test.ts | 114 ++++++++++++++---- 5 files changed, 142 insertions(+), 35 deletions(-) diff --git a/.server-changes/fix-invite-role-change-for-existing-members.md b/.server-changes/fix-invite-role-change-for-existing-members.md index fec4dd25362..ad4fb18b622 100644 --- a/.server-changes/fix-invite-role-change-for-existing-members.md +++ b/.server-changes/fix-invite-role-change-for-existing-members.md @@ -3,4 +3,4 @@ area: webapp type: fix --- -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, and people who are already in an organization are no longer sent invitations to it. +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. diff --git a/apps/webapp/app/models/member.server.ts b/apps/webapp/app/models/member.server.ts index 9bdf7b8991d..4167738413b 100644 --- a/apps/webapp/app/models/member.server.ts +++ b/apps/webapp/app/models/member.server.ts @@ -119,9 +119,12 @@ export async function inviteMembers({ const created: Prisma.OrgMemberInviteGetPayload<{ include: { organization: true; inviter: true }; }>[] = []; + const alreadyMembers: string[] = []; + const alreadyInvited: string[] = []; for (const email of uniqueEmails) { if (existingMemberEmails.has(email)) { + alreadyMembers.push(email); continue; } @@ -146,13 +149,14 @@ export async function inviteMembers({ error instanceof PrismaNamespace.PrismaClientKnownRequestError && error.code === "P2002" ) { + alreadyInvited.push(email); continue; } throw error; } } - return created; + return { created, alreadyMembers, alreadyInvited }; } export async function getInviteFromToken({ token }: { token: string }) { @@ -279,12 +283,21 @@ async function assignInviteRbacRole({ userId, organizationId, rbacRoleId, + onlyWhenUnassigned, }: { userId: string; organizationId: string; rbacRoleId: string; + onlyWhenUnassigned: boolean; }) { try { + if (onlyWhenUnassigned) { + const currentRole = await rbac.getUserRole({ userId, organizationId }); + if (currentRole !== null) { + return; + } + } + const roleResult = await rbac.setUserRole({ userId, organizationId, @@ -499,11 +512,12 @@ export async function acceptInvite({ const remainingInvites = await getUsersInvites({ email: user.email }); - if (invite.rbacRoleId && membershipCreated) { + if (invite.rbacRoleId) { await assignInviteRbacRole({ userId: user.id, organizationId: invite.organization.id, rbacRoleId: invite.rbacRoleId, + onlyWhenUnassigned: !membershipCreated, }); } diff --git a/apps/webapp/app/routes/_app.orgs.$organizationSlug.invite/route.tsx b/apps/webapp/app/routes/_app.orgs.$organizationSlug.invite/route.tsx index 6a5a0e7a010..b6fe0b09c68 100644 --- a/apps/webapp/app/routes/_app.orgs.$organizationSlug.invite/route.tsx +++ b/apps/webapp/app/routes/_app.orgs.$organizationSlug.invite/route.tsx @@ -28,7 +28,7 @@ import { $replica } from "~/db.server"; import { env } from "~/env.server"; import { useOrganization } from "~/hooks/useOrganizations"; import { inviteMembers } from "~/models/member.server"; -import { redirectWithSuccessMessage } from "~/models/message.server"; +import { redirectWithErrorMessage, redirectWithSuccessMessage } from "~/models/message.server"; import { resolveOrgIdFromSlug } from "~/models/organization.server"; import { TeamPresenter } from "~/presenters/TeamPresenter.server"; import { scheduleEmail } from "~/services/scheduleEmail.server"; @@ -127,6 +127,20 @@ const schema = z.object({ rbacRoleId: z.string().optional(), }); +function describeSkippedInvites(alreadyMembers: string[], alreadyInvited: string[]) { + const parts: string[] = []; + + if (alreadyMembers.length > 0) { + parts.push(simplur`${alreadyMembers.length} already [a member|members] of this organization`); + } + + if (alreadyInvited.length > 0) { + parts.push(simplur`${alreadyInvited.length} already invited`); + } + + return parts.join(" and "); +} + export const action = dashboardAction( { params: Params, @@ -201,7 +215,11 @@ export const action = dashboardAction( } try { - const invites = await inviteMembers({ + const { + created: invites, + alreadyMembers, + alreadyInvited, + } = await inviteMembers({ slug: organizationSlug, emails: submission.value.emails, userId, @@ -224,10 +242,19 @@ export const action = dashboardAction( } } + const teamPath = organizationTeamPath({ slug: organizationSlug }); + const skipped = describeSkippedInvites(alreadyMembers, alreadyInvited); + + if (invites.length === 0) { + return redirectWithErrorMessage(teamPath, request, `No invitations sent: ${skipped}.`); + } + return redirectWithSuccessMessage( - organizationTeamPath(invites[0].organization), + teamPath, request, - simplur`${submission.value.emails.length} member[|s] invited` + skipped + ? simplur`${invites.length} member[|s] invited. Skipped ${skipped}.` + : simplur`${invites.length} member[|s] invited` ); } catch (error: any) { return json({ errors: { body: error.message } }, { status: 400 }); diff --git a/apps/webapp/app/routes/api.v1.orgs.$orgParam.invites.ts b/apps/webapp/app/routes/api.v1.orgs.$orgParam.invites.ts index 76aeeb171b7..36e56e060eb 100644 --- a/apps/webapp/app/routes/api.v1.orgs.$orgParam.invites.ts +++ b/apps/webapp/app/routes/api.v1.orgs.$orgParam.invites.ts @@ -57,9 +57,7 @@ export const action = createActionPATApiRoute( return json({ error: "Membership is managed by Directory Sync" }, { status: 403 }); } - // Returns only the invites created by this call; already-invited emails are - // skipped (re-sending is the dashboard's dedicated resend flow, not this). - const created = await inviteMembers({ + const { created, alreadyMembers, alreadyInvited } = await inviteMembers({ slug: organization.slug, emails: body.emails, userId: authentication.userId, @@ -85,13 +83,11 @@ export const action = createActionPATApiRoute( // Report per-email outcome so callers aren't misled by an empty list on // re-invite. 201 when something was created, 200 when everything already // existed. - const createdEmails = new Set(created.map((invite) => invite.email)); - const alreadyInvited = [...new Set(body.emails)].filter((email) => !createdEmails.has(email)); - return json( { invited: created.map((invite) => ({ id: invite.id, email: invite.email })), alreadyInvited, + alreadyMembers, }, { status: created.length > 0 ? 201 : 200 } ); diff --git a/apps/webapp/test/member.server.test.ts b/apps/webapp/test/member.server.test.ts index 5d79bf09f9b..722c49aff18 100644 --- a/apps/webapp/test/member.server.test.ts +++ b/apps/webapp/test/member.server.test.ts @@ -1,5 +1,5 @@ import { randomBytes } from "node:crypto"; -import { describe, expect, vi } from "vitest"; +import { beforeEach, describe, expect, vi } from "vitest"; import type { PrismaClient } from "@trigger.dev/database"; const prismaHolder = vi.hoisted(() => ({ @@ -8,14 +8,17 @@ const prismaHolder = vi.hoisted(() => ({ type SetUserRoleParams = { userId: string; organizationId: string; roleId: string }; type SetUserRoleResult = { ok: true } | { ok: false; error: string; code?: "last_owner" }; +type CurrentRole = { id: string } | null; const rbacHolder = vi.hoisted(() => ({ setUserRoleCalls: [] as SetUserRoleParams[], setUserRoleResult: { ok: true } as SetUserRoleResult, + currentRole: null as CurrentRole, })); vi.mock("~/services/rbac.server", () => ({ rbac: { + getUserRole: async () => rbacHolder.currentRole, setUserRole: async (params: SetUserRoleParams) => { rbacHolder.setUserRoleCalls.push(params); return rbacHolder.setUserRoleResult; @@ -23,25 +26,35 @@ vi.mock("~/services/rbac.server", () => ({ }, })); -function resetRbac(result: SetUserRoleResult = { ok: true }) { +function resetRbac(result: SetUserRoleResult = { ok: true }, currentRole: CurrentRole = null) { rbacHolder.setUserRoleCalls.length = 0; rbacHolder.setUserRoleResult = result; + rbacHolder.currentRole = currentRole; } -vi.mock("~/db.server", () => ({ - get prisma() { - if (!prismaHolder.client) { - throw new Error("test prisma not set"); - } - return prismaHolder.client; - }, - get $replica() { - if (!prismaHolder.client) { - throw new Error("test prisma not set"); - } - return prismaHolder.client; - }, -})); +beforeEach(() => { + resetRbac(); +}); + +vi.mock("~/db.server", async () => { + const { Prisma } = await import("@trigger.dev/database"); + + return { + Prisma, + get prisma() { + if (!prismaHolder.client) { + throw new Error("test prisma not set"); + } + return prismaHolder.client; + }, + get $replica() { + if (!prismaHolder.client) { + throw new Error("test prisma not set"); + } + return prismaHolder.client; + }, + }; +}); import { postgresTest } from "@internal/testcontainers"; @@ -276,7 +289,6 @@ describe("acceptInvite", () => { { timeout: 60_000 }, async ({ prisma }) => { prismaHolder.client = prisma; - resetRbac(); const { acceptInvite } = await import("../app/models/member.server"); const { invitee, organization, invite } = await seedInviteFixture(prisma, { @@ -297,11 +309,11 @@ describe("acceptInvite", () => { ); postgresTest( - "does not apply the invite RBAC role to a user who is already a member", + "does not apply the invite RBAC role to a member who already has a role", { timeout: 60_000 }, async ({ prisma }) => { prismaHolder.client = prisma; - resetRbac(); + resetRbac({ ok: true }, { id: "role_owner" }); const { acceptInvite } = await import("../app/models/member.server"); const { invitee, organization, invite } = await seedInviteFixture(prisma, { @@ -337,6 +349,39 @@ describe("acceptInvite", () => { } ); + postgresTest( + "applies the invite RBAC role to an existing member who has no role assigned", + { timeout: 60_000 }, + async ({ prisma }) => { + prismaHolder.client = prisma; + resetRbac({ ok: true }, null); + const { acceptInvite } = await import("../app/models/member.server"); + + const { invitee, organization, invite } = await seedInviteFixture(prisma, { + activeProjectCount: 1, + rbacRoleId: "role_admin", + }); + + await prisma.orgMember.create({ + data: { + organizationId: organization.id, + userId: invitee.id, + role: "MEMBER", + }, + }); + + await acceptInvite({ + inviteId: invite.id, + organizationId: organization.id, + user: { id: invitee.id, email: invitee.email }, + }); + + expect(rbacHolder.setUserRoleCalls).toEqual([ + { userId: invitee.id, organizationId: organization.id, roleId: "role_admin" }, + ]); + } + ); + postgresTest( "still joins the org when the RBAC role assignment is refused", { timeout: 60_000 }, @@ -367,8 +412,6 @@ describe("acceptInvite", () => { where: { userId: invitee.id, organizationId: organization.id }, }); expect(member).not.toBeNull(); - - resetRbac(); } ); }); @@ -397,13 +440,15 @@ describe("inviteMembers", () => { const newcomerEmail = `newcomer-${randomHex(8)}@test.local`; - const created = await inviteMembers({ + const { created, alreadyMembers, alreadyInvited } = await inviteMembers({ slug: organization.slug, emails: [invitee.email, newcomerEmail], userId: inviter.id, }); expect(created.map((row) => row.email)).toEqual([newcomerEmail]); + expect(alreadyMembers).toEqual([invitee.email]); + expect(alreadyInvited).toEqual([]); const invites = await prisma.orgMemberInvite.findMany({ where: { organizationId: organization.id }, @@ -412,6 +457,31 @@ describe("inviteMembers", () => { expect(invites.map((pending) => pending.email)).toEqual([newcomerEmail]); } ); + + postgresTest( + "reports an email with a pending invitation as already invited", + { timeout: 60_000 }, + async ({ prisma }) => { + prismaHolder.client = prisma; + const { inviteMembers } = await import("../app/models/member.server"); + + const { inviter, organization, invite } = await seedInviteFixture(prisma, { + activeProjectCount: 0, + }); + + const newcomerEmail = `newcomer-${randomHex(8)}@test.local`; + + const { created, alreadyMembers, alreadyInvited } = await inviteMembers({ + slug: organization.slug, + emails: [invite.email, newcomerEmail], + userId: inviter.id, + }); + + expect(created.map((row) => row.email)).toEqual([newcomerEmail]); + expect(alreadyInvited).toEqual([invite.email]); + expect(alreadyMembers).toEqual([]); + } + ); }); describe("provisionMemberDevelopmentEnvironments", () => {