diff --git a/server/src/__tests__/access-routes-permissions-upgrade.test.ts b/server/src/__tests__/access-routes-permissions-upgrade.test.ts index 7f9d440cfe..df764563ca 100644 --- a/server/src/__tests__/access-routes-permissions-upgrade.test.ts +++ b/server/src/__tests__/access-routes-permissions-upgrade.test.ts @@ -19,6 +19,8 @@ import { getEmbeddedPostgresTestSupport, startEmbeddedPostgresTestDatabase, } from "./helpers/embedded-postgres.js"; +import { ownerHasRequiredGrant } from "../security/board-key-owner-authority.js"; +import { grantsForHumanRole } from "../services/company-member-roles.js"; vi.hoisted(() => { process.env.PAPERCLIP_HOME = "/tmp/paperclip-test-home"; @@ -127,7 +129,7 @@ describeEmbeddedPostgres("access routes permissions upgrade compatibility", () = expect(unchanged.membershipRole).toBe("owner"); }, 10_000); - it("keeps custom grants when the role-only member route changes a member role", async () => { + it("retires former role defaults but keeps custom grants when the role-only route demotes a member", async () => { const { company, owner } = await createCompanyWithOwner(db); const member = await db .insert(companyMemberships) @@ -141,14 +143,31 @@ describeEmbeddedPostgres("access routes permissions upgrade compatibility", () = .returning() .then((rows) => rows[0]!); const customScope = { projectIds: ["project-1"] }; - await db.insert(principalPermissionGrants).values({ - companyId: company.id, - principalType: "user", - principalId: member.principalId, - permissionKey: "tasks:assign_scope", - scope: customScope, - grantedByUserId: owner.principalId, - }); + await db.insert(principalPermissionGrants).values([ + ...grantsForHumanRole("admin").map((grant) => ({ + companyId: company.id, + principalType: "user" as const, + principalId: member.principalId, + permissionKey: grant.permissionKey, + scope: grant.scope, + grantedByUserId: owner.principalId, + })), + { + companyId: company.id, + principalType: "user" as const, + principalId: member.principalId, + permissionKey: "tasks:assign_scope" as const, + scope: customScope, + grantedByUserId: owner.principalId, + }, + ]); + + await expect(ownerHasRequiredGrant( + db, + member.principalId, + [company.id], + "tools:manage", + )).resolves.toBe(true); const res = await request(await createApp(db, company.id, owner.principalId)) .patch(`/api/companies/${company.id}/members/${member.id}`) @@ -166,13 +185,25 @@ describeEmbeddedPostgres("access routes permissions upgrade compatibility", () = eq(principalPermissionGrants.principalType, "user"), eq(principalPermissionGrants.principalId, member.principalId), ), - ); - expect(grants).toHaveLength(1); - expect(grants[0]).toMatchObject({ - permissionKey: "tasks:assign_scope", - scope: customScope, - grantedByUserId: owner.principalId, - }); + ); + expect(grants).toHaveLength(2); + expect(grants).toEqual(expect.arrayContaining([ + expect.objectContaining({ + permissionKey: "tasks:assign", + scope: null, + }), + expect.objectContaining({ + permissionKey: "tasks:assign_scope", + scope: customScope, + grantedByUserId: owner.principalId, + }), + ])); + await expect(ownerHasRequiredGrant( + db, + member.principalId, + [company.id], + "tools:manage", + )).resolves.toBe(false); }); it("sweeps personal connection access when the member route suspends a user", async () => { diff --git a/server/src/services/access.ts b/server/src/services/access.ts index 73eaf0d49d..19fd2e23dc 100644 --- a/server/src/services/access.ts +++ b/server/src/services/access.ts @@ -18,6 +18,7 @@ import type { PermissionKey, PrincipalType } from "@paperclipai/shared"; import { conflict } from "../errors.js"; import { assertAssignableAgent } from "./agent-assignability.js"; import { authorizationService, type AuthorizationActor, type AuthorizationResource } from "./authorization.js"; +import { grantsForHumanRole, normalizeHumanRole } from "./company-member-roles.js"; import { ensureHumanRoleDefaultGrants } from "./principal-access-compatibility.js"; type MembershipRow = typeof companyMemberships.$inferSelect; @@ -1112,6 +1113,34 @@ export function accessService(db: Db) { await sweepMemberConnectionAccess(tx, companyId, existing.principalId, now); } + if ( + existing.principalType === "user" + && data.membershipRole !== undefined + && nextMembershipRole !== existing.membershipRole + ) { + const previousDefaultKeys = new Set( + grantsForHumanRole(normalizeHumanRole(existing.membershipRole, "operator")) + .map((grant) => grant.permissionKey), + ); + const nextDefaultKeys = new Set( + grantsForHumanRole(normalizeHumanRole(nextMembershipRole, "operator")) + .map((grant) => grant.permissionKey), + ); + const retiredDefaultKeys = [...previousDefaultKeys] + .filter((permissionKey) => !nextDefaultKeys.has(permissionKey)); + + if (retiredDefaultKeys.length > 0) { + await tx + .delete(principalPermissionGrants) + .where(and( + eq(principalPermissionGrants.companyId, companyId), + eq(principalPermissionGrants.principalType, "user"), + eq(principalPermissionGrants.principalId, existing.principalId), + inArray(principalPermissionGrants.permissionKey, retiredDefaultKeys), + )); + } + } + return tx .update(companyMemberships) .set({