feat: reassign team member roles#1746
Conversation
Admins can set any member (including themselves) to User or Admin. Only the super-admin can transfer the super-admin role, becoming an admin himself, so there is always exactly one super-admin per org. The affected user is notified by email. Adds PUT /settings/team/:id and an inline role selector in the Teams settings screen. Tested manually end-to-end: admin promote/demote including self-demotion, super-admin transfer and transfer back, removal from the org, and the failure path (a failed request shows a warning instead of a false success toast). Service authorization rules also verified with a scripted runtime check covering eight scenarios, including blocking an ex-super-admin from taking the role back. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
| where: { | ||
| userId_organizationId: { | ||
| userId: fromUserId, | ||
| organizationId: orgId, | ||
| }, | ||
| }, | ||
| data: { | ||
| role: Role.ADMIN, | ||
| }, | ||
| }); | ||
| } |
There was a problem hiding this comment.
Bug: The transferSuperAdminRole function performs two separate database updates without a transaction. A failure between the updates can leave an organization with two SUPERADMINs.
Severity: HIGH
Suggested Fix
Wrap the two userOrganization.update calls within a single database transaction. Inject the PrismaTransaction service into the OrganizationRepository and use its $transaction method to ensure that both updates either succeed together or fail together, maintaining data consistency.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location:
libraries/nestjs-libraries/src/database/prisma/organizations/organization.repository.ts#L391-L419
Potential issue: The `transferSuperAdminRole` function updates user roles with two
sequential, non-atomic database calls: one to promote `toUserId` to `SUPERADMIN` and
another to demote `fromUserId` to `ADMIN`. If the first call succeeds but the second one
fails due to a database error or timeout, the process halts midway. This results in the
organization having two users with the `SUPERADMIN` role, violating the application's
invariant that only one super admin should exist per organization. The database schema
does not enforce this constraint, so the inconsistent state will persist.
Did we get this right? 👍 / 👎 to inform future reviews.
There was a problem hiding this comment.
Valid observation, accepted risk. The window is two back-to-back keyed updates on the same connection, and the order is deliberately promote-first so a mid-failure can never leave the org with zero super admins — the worst case is a brief second super admin, visible in the team list and fixable in the DB. This codebase intentionally avoids $transaction (one of the reasons #1668 was rejected), so we're not introducing it here.
Adds the five new UI strings to the en locale and generates all other locales via lingo.dev. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
What kind of change does this PR introduce?
Feature.
Why was this change needed?
Team roles were fixed at invite time — there was no way to change a member's role afterwards, or to hand over ownership of an org. This PR lets:
The affected user is notified by email. All rules are enforced server-side (the requester's role is re-read from the DB per request, so a stale session can't act on a role it no longer has); the UI derives the viewer's role from the freshly fetched team list so the dropdown options update immediately after a transfer. A failed request shows a warning toast instead of a false success.
Adds
PUT /settings/team/:id(DTO → Controller → Service → Repository) and an inline role selector in the Teams settings screen. No schema change in this PR.Other information:
Depends on #1745 — this PR is stacked on
feat/persist-last-selected-organd must not merge before it. Cross-org role transfer surfaced that pre-existing org-selection bug (it had no visible effect before this feature). When #1745 merges, this PR retargets tomainautomatically.Tested manually end-to-end: admin promote/demote including self-demotion, super-admin transfer and transfer back, removal from the org, and the failure path. Service authorization rules also verified with a scripted runtime check covering eight scenarios, including blocking an ex-super-admin from taking the role back.
Checklist:
🤖 Generated with Claude Code