From 802252f37720fc8340c9c562cd4338745e1c816c Mon Sep 17 00:00:00 2001 From: Dotta Date: Wed, 15 Apr 2026 22:48:48 -0500 Subject: [PATCH] fix(access): make profile and member updates atomic --- packages/shared/src/index.ts | 2 + packages/shared/src/validators/access.ts | 10 ++++ packages/shared/src/validators/index.ts | 2 + server/src/__tests__/auth-routes.test.ts | 23 ++++++++ server/src/routes/access.ts | 75 ++++++++++++++++++++++++ server/src/routes/auth.ts | 2 +- server/src/services/access.ts | 59 +++++++++++++++++++ ui/src/api/access.ts | 13 ++++ ui/src/pages/CompanyAccess.test.tsx | 51 ++++++++++++++++ ui/src/pages/CompanyAccess.tsx | 4 +- 10 files changed, 237 insertions(+), 4 deletions(-) diff --git a/packages/shared/src/index.ts b/packages/shared/src/index.ts index fcaf8bc1499..d1569b67ab8 100644 --- a/packages/shared/src/index.ts +++ b/packages/shared/src/index.ts @@ -591,6 +591,7 @@ export { authSessionSchema, updateCurrentUserProfileSchema, updateCompanyMemberSchema, + updateCompanyMemberWithPermissionsSchema, updateMemberPermissionsSchema, searchAdminUsersQuerySchema, updateUserCompanyAccessSchema, @@ -611,6 +612,7 @@ export { type AuthSession, type UpdateCurrentUserProfile, type UpdateCompanyMember, + type UpdateCompanyMemberWithPermissions, type UpdateMemberPermissions, type SearchAdminUsersQuery, type UpdateUserCompanyAccess, diff --git a/packages/shared/src/validators/access.ts b/packages/shared/src/validators/access.ts index 187ef865fe5..8aaed6babcb 100644 --- a/packages/shared/src/validators/access.ts +++ b/packages/shared/src/validators/access.ts @@ -106,6 +106,16 @@ export const updateCompanyMemberSchema = z.object({ export type UpdateCompanyMember = z.infer; +export const updateCompanyMemberWithPermissionsSchema = z.object({ + membershipRole: z.enum(HUMAN_COMPANY_MEMBERSHIP_ROLES).optional().nullable(), + status: z.enum(MEMBERSHIP_STATUSES).optional(), + grants: updateMemberPermissionsSchema.shape.grants, +}).refine((value) => value.membershipRole !== undefined || value.status !== undefined, { + message: "membershipRole or status is required", +}); + +export type UpdateCompanyMemberWithPermissions = z.infer; + export const updateUserCompanyAccessSchema = z.object({ companyIds: z.array(z.string().uuid()).default([]), }); diff --git a/packages/shared/src/validators/index.ts b/packages/shared/src/validators/index.ts index 5dcd5e37456..8e1662f4280 100644 --- a/packages/shared/src/validators/index.ts +++ b/packages/shared/src/validators/index.ts @@ -263,6 +263,7 @@ export { authSessionSchema, updateCurrentUserProfileSchema, updateCompanyMemberSchema, + updateCompanyMemberWithPermissionsSchema, updateMemberPermissionsSchema, searchAdminUsersQuerySchema, updateUserCompanyAccessSchema, @@ -279,6 +280,7 @@ export { type AuthSession, type UpdateCurrentUserProfile, type UpdateCompanyMember, + type UpdateCompanyMemberWithPermissions, type UpdateMemberPermissions, type SearchAdminUsersQuery, type UpdateUserCompanyAccess, diff --git a/server/src/__tests__/auth-routes.test.ts b/server/src/__tests__/auth-routes.test.ts index 7ef45f149ef..88033d661da 100644 --- a/server/src/__tests__/auth-routes.test.ts +++ b/server/src/__tests__/auth-routes.test.ts @@ -110,6 +110,29 @@ describe("auth routes", () => { }); }); + it("preserves the existing avatar when updating only the profile name", async () => { + const app = await createApp( + { + type: "board", + userId: "user-1", + source: "local_implicit", + }, + baseUser, + ); + + const res = await request(app) + .patch("/api/auth/profile") + .send({ name: "Board Operator" }); + + expect(res.status).toBe(200); + expect(res.body).toMatchObject({ + id: "user-1", + name: "Board Operator", + email: "jane@example.com", + image: "https://example.com/jane.png", + }); + }); + it("accepts Paperclip asset paths for avatars", async () => { const app = await createApp( { diff --git a/server/src/routes/access.ts b/server/src/routes/access.ts index 4830e501118..1159aea783f 100644 --- a/server/src/routes/access.ts +++ b/server/src/routes/access.ts @@ -32,6 +32,7 @@ import { listJoinRequestsQuerySchema, resolveCliAuthChallengeSchema, searchAdminUsersQuerySchema, + updateCompanyMemberWithPermissionsSchema, updateCompanyMemberSchema, updateMemberPermissionsSchema, updateUserCompanyAccessSchema, @@ -3673,6 +3674,80 @@ export function accessRoutes( } ); + router.patch( + "/companies/:companyId/members/:memberId/role-and-grants", + validate(updateCompanyMemberWithPermissionsSchema), + async (req, res) => { + const companyId = req.params.companyId as string; + const memberId = req.params.memberId as string; + await assertCompanyPermission(req, companyId, "users:manage_permissions"); + + const existing = await access.getMemberById(companyId, memberId); + if (!existing) throw notFound("Member not found"); + + const nextMembershipRole = + req.body.membershipRole !== undefined + ? req.body.membershipRole + : existing.membershipRole; + const nextStatus = req.body.status ?? existing.status; + + if ( + existing.principalType === "user" && + existing.status === "active" && + existing.membershipRole === "owner" && + (nextStatus !== "active" || nextMembershipRole !== "owner") + ) { + const activeOwnerCount = await db + .select({ id: companyMemberships.id }) + .from(companyMemberships) + .where( + and( + eq(companyMemberships.companyId, companyId), + eq(companyMemberships.principalType, "user"), + eq(companyMemberships.status, "active"), + eq(companyMemberships.membershipRole, "owner"), + ), + ) + .then((rows) => rows.length); + if (activeOwnerCount <= 1) { + throw conflict("Cannot remove the last active owner"); + } + } + + const updated = await access.updateMemberAndPermissions( + companyId, + memberId, + { + membershipRole: req.body.membershipRole, + status: req.body.status, + grants: req.body.grants ?? [], + }, + req.actor.userId ?? null, + ); + if (!updated) throw notFound("Member not found"); + + await logActivity(db, { + companyId, + actorType: "user", + actorId: req.actor.userId ?? "board", + action: "company_member.access_updated", + entityType: "company_membership", + entityId: memberId, + details: { + membershipRole: updated.membershipRole, + status: updated.status, + grantCount: req.body.grants?.length ?? 0, + }, + }); + + const member = (await loadCompanyMemberRecords(db, companyId)).find( + (entry) => entry.id === memberId, + ); + if (!member) throw notFound("Member not found"); + res.json(member); + } + ); + router.patch( "/companies/:companyId/members/:memberId/permissions", validate(updateMemberPermissionsSchema), diff --git a/server/src/routes/auth.ts b/server/src/routes/auth.ts index e7b0ad4c3bd..ee94341c8fd 100644 --- a/server/src/routes/auth.ts +++ b/server/src/routes/auth.ts @@ -72,7 +72,7 @@ export function authRoutes(db: Db) { .update(authUsers) .set({ name: patch.name, - image: patch.image ?? null, + ...(patch.image !== undefined ? { image: patch.image } : {}), updatedAt: now, }) .where(eq(authUsers.id, req.actor.userId)) diff --git a/server/src/services/access.ts b/server/src/services/access.ts index 5b7833cf952..dd5f74af2b3 100644 --- a/server/src/services/access.ts +++ b/server/src/services/access.ts @@ -143,6 +143,64 @@ export function accessService(db: Db) { return member; } + async function updateMemberAndPermissions( + companyId: string, + memberId: string, + data: { + membershipRole?: string | null; + status?: "pending" | "active" | "suspended"; + grants: GrantInput[]; + }, + grantedByUserId: string | null, + ) { + const existing = await getMemberById(companyId, memberId); + if (!existing) return null; + + const nextMembershipRole = + data.membershipRole !== undefined ? data.membershipRole : existing.membershipRole; + const nextStatus = data.status ?? existing.status; + const now = new Date(); + + return db.transaction(async (tx) => { + const updated = await tx + .update(companyMemberships) + .set({ + membershipRole: nextMembershipRole, + status: nextStatus, + updatedAt: now, + }) + .where(eq(companyMemberships.id, existing.id)) + .returning() + .then((rows) => rows[0] ?? existing); + + await tx + .delete(principalPermissionGrants) + .where( + and( + eq(principalPermissionGrants.companyId, companyId), + eq(principalPermissionGrants.principalType, existing.principalType), + eq(principalPermissionGrants.principalId, existing.principalId), + ), + ); + if (data.grants.length > 0) { + await tx.insert(principalPermissionGrants).values( + data.grants.map((grant) => ({ + companyId, + principalType: existing.principalType, + principalId: existing.principalId, + permissionKey: grant.permissionKey, + scope: grant.scope ?? null, + grantedByUserId, + createdAt: now, + updatedAt: now, + })), + ); + } + + return updated; + }); + } + async function promoteInstanceAdmin(userId: string) { const existing = await db .select() @@ -399,6 +457,7 @@ export function accessService(db: Db) { listActiveUserMemberships, copyActiveUserMemberships, setMemberPermissions, + updateMemberAndPermissions, promoteInstanceAdmin, demoteInstanceAdmin, listUserCompanyAccess, diff --git a/ui/src/api/access.ts b/ui/src/api/access.ts index 1bfa65eae1a..917a049fa0f 100644 --- a/ui/src/api/access.ts +++ b/ui/src/api/access.ts @@ -328,6 +328,19 @@ export const accessApi = { }, ) => api.patch(`/companies/${companyId}/members/${memberId}/permissions`, input), + updateMemberAccess: ( + companyId: string, + memberId: string, + input: { + membershipRole?: HumanCompanyRole | null; + status?: "pending" | "active" | "suspended"; + grants: Array<{ + permissionKey: PermissionKey; + scope?: Record | null; + }>; + }, + ) => api.patch(`/companies/${companyId}/members/${memberId}/role-and-grants`, input), + approveJoinRequest: (companyId: string, requestId: string) => api.post(`/companies/${companyId}/join-requests/${requestId}/approve`, {}), diff --git a/ui/src/pages/CompanyAccess.test.tsx b/ui/src/pages/CompanyAccess.test.tsx index 6fe1e474442..4d6af17cf0a 100644 --- a/ui/src/pages/CompanyAccess.test.tsx +++ b/ui/src/pages/CompanyAccess.test.tsx @@ -8,6 +8,7 @@ import { CompanyAccess } from "./CompanyAccess"; const listMembersMock = vi.hoisted(() => vi.fn()); const listJoinRequestsMock = vi.hoisted(() => vi.fn()); +const updateMemberAccessMock = vi.hoisted(() => vi.fn()); vi.mock("@/api/access", () => ({ accessApi: { @@ -15,6 +16,8 @@ vi.mock("@/api/access", () => ({ listJoinRequests: (companyId: string, status: string) => listJoinRequestsMock(companyId, status), updateMember: vi.fn(), updateMemberPermissions: vi.fn(), + updateMemberAccess: (companyId: string, memberId: string, input: unknown) => + updateMemberAccessMock(companyId, memberId, input), approveJoinRequest: vi.fn(), rejectJoinRequest: vi.fn(), }, @@ -109,6 +112,7 @@ describe("CompanyAccess", () => { }, }, ]); + updateMemberAccessMock.mockResolvedValue({}); }); afterEach(() => { @@ -165,4 +169,51 @@ describe("CompanyAccess", () => { root.unmount(); }); }); + + it("saves member role, status, and grants in one request", async () => { + const root = createRoot(container); + const queryClient = new QueryClient({ + defaultOptions: { queries: { retry: false } }, + }); + + await act(async () => { + root.render( + + + , + ); + }); + await flushReact(); + await flushReact(); + + const editButton = Array.from(container.querySelectorAll("button")).find( + (button) => button.textContent === "Edit", + ); + expect(editButton).toBeTruthy(); + + await act(async () => { + editButton?.dispatchEvent(new MouseEvent("click", { bubbles: true })); + }); + await flushReact(); + + const saveButton = Array.from(document.body.querySelectorAll("button")).find( + (button) => button.textContent === "Save access", + ); + expect(saveButton).toBeTruthy(); + + await act(async () => { + saveButton?.dispatchEvent(new MouseEvent("click", { bubbles: true })); + }); + await flushReact(); + + expect(updateMemberAccessMock).toHaveBeenCalledWith("company-1", "member-1", { + membershipRole: "owner", + status: "active", + grants: [], + }); + + await act(async () => { + root.unmount(); + }); + }); }); diff --git a/ui/src/pages/CompanyAccess.tsx b/ui/src/pages/CompanyAccess.tsx index 80f829df003..9c65e45e161 100644 --- a/ui/src/pages/CompanyAccess.tsx +++ b/ui/src/pages/CompanyAccess.tsx @@ -84,11 +84,9 @@ export function CompanyAccess() { const updateMemberMutation = useMutation({ mutationFn: async (input: { memberId: string; membershipRole: CompanyMember["membershipRole"]; status: CompanyMember["status"]; grants: PermissionKey[] }) => { - await accessApi.updateMember(selectedCompanyId!, input.memberId, { + return accessApi.updateMemberAccess(selectedCompanyId!, input.memberId, { membershipRole: input.membershipRole, status: input.status, - }); - return accessApi.updateMemberPermissions(selectedCompanyId!, input.memberId, { grants: input.grants.map((permissionKey) => ({ permissionKey })), }); },