fix(webapp): don't apply an invite's role to an existing org member - #4409
Merged
Conversation
|
matt-aitken
marked this pull request as ready for review
July 28, 2026 16:14
1stvamp
approved these changes
Jul 28, 2026
`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.
…mber 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.
matt-aitken
force-pushed
the
claude/fix-invite-role-demotion
branch
from
July 28, 2026 18:33
43ff2e0 to
9b60a42
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Requested via Slack thread
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
acceptInvitealready skipped theOrgMembercreate when it found an existing membership, but therbac.setUserRolecall below it was gated only oninvite.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.
ensureOrgMemberhandles that withhealMissingRoleAssignment, which fills in a null role but never overwrites a real one, soassignInviteRbacRoletakes the same gate. An established role is never touched; an absent one is filled in.assignInviteRbacRolebranches on the result's machine-readablecodeinstead of logging every refusal aterror.last_ownergoes tologger.info, matching the two directory-sync role paths; everything else, including a refusal that carries no code, goes tologger.warn. The helper is best-effort and never throws, so no outcome it produces warrantserror. No string matching on the error text is involved.inviteMembersresolves 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:
invites[0].organization, so a batch where every address was skipped threw aTypeErrorthat 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.alreadyInvitedas "everything not created", so an existing member was reported as though they had already been invited.inviteMembersnow returns the two groups separately and the endpoint reportsalreadyMembersalongsidealreadyInvited.Testing
apps/webapp/test/member.server.test.tspasses 16/16 locally, up from 12.Getting there needed a harness fix. The
~/db.servermock did not exportPrisma, so any code reachingPrismaNamespace.PrismaClientKnownRequestErrorthrew before it could branch, leaving every duplicate-key path inmember.server.tsunreachable from tests. The mock now re-exports the realPrisma, 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; andinviteMembersreports 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 webappandoxfmt --checkboth 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
Screenshots
No visual changes. The invite form's toast copy changes, as described above.