fix(auth): require legacy grant review on demotion

Co-Authored-By: Paperclip <noreply@paperclip.ing>

server/src/__tests__/access-routes-permissions-upgrade.test.ts

@@ -261,7 +261,7 @@ describeEmbeddedPostgres("access routes permissions upgrade compatibility", () =
     )).resolves.toBe(false);
   });
 
-  it("preserves ambiguous legacy grants while limiting them to the current role", async () => {
+  it("requires legacy permission review before a role-only demotion", async () => {
     const { company, owner } = await createCompanyWithOwner(db);
     const member = await db.insert(companyMemberships).values({
       companyId: company.id,
@@ -288,7 +288,10 @@ describeEmbeddedPostgres("access routes permissions upgrade compatibility", () =
     const res = await request(await createApp(db, company.id, owner.principalId))
       .patch(`/api/companies/${company.id}/members/${member.id}`)
       .send({ membershipRole: "operator" });
-    expect(res.status, JSON.stringify(res.body)).toBe(200);
+    expect(res.status, JSON.stringify(res.body)).toBe(409);
+    expect(res.body).toMatchObject({
+      error: "Review this member's legacy permissions before changing their role",
+    });
 
     const preserved = await db
       .select()
@@ -297,12 +300,28 @@ describeEmbeddedPostgres("access routes permissions upgrade compatibility", () =
     expect(preserved).toEqual(expect.arrayContaining([
       expect.objectContaining({ permissionKey: "tools:admin", grantOrigin: "legacy_unknown" }),
     ]));
+
+    const reviewed = await request(await createApp(db, company.id, owner.principalId))
+      .patch(`/api/companies/${company.id}/members/${member.id}/role-and-grants`)
+      .send({
+        membershipRole: "operator",
+        grants: [{ permissionKey: "tools:admin", scope: null }],
+      });
+    expect(reviewed.status, JSON.stringify(reviewed.body)).toBe(200);
+    expect(reviewed.body.membershipRole).toBe("operator");
+    const reviewedGrants = await db
+      .select()
+      .from(principalPermissionGrants)
+      .where(eq(principalPermissionGrants.principalId, member.principalId));
+    expect(reviewedGrants).toEqual([
+      expect.objectContaining({ permissionKey: "tools:admin", grantOrigin: "explicit" }),
+    ]);
     await expect(ownerHasRequiredGrant(
       db,
       member.principalId,
       [company.id],
       "tools:manage",
-    )).resolves.toBe(false);
+    )).resolves.toBe(true);
   });
 
   it("sweeps personal connection access when the member route suspends a user", async () => {

server/src/__tests__/authorization-service.test.ts

@@ -227,7 +227,7 @@ describeEmbeddedPostgres("authorization service", () => {
     expect(decision.explanation).toContain("Allowed by explicit grant tasks:assign");
   });
 
-  it("limits ambiguous legacy user grants to the active membership role", async () => {
+  it("preserves ambiguous historical explicit grants across a direct role change", async () => {
     const company = await createCompany(db, "LegacyGrantRoleCeiling");
     const userId = `user-${randomUUID()}`;
     await db.insert(companyMemberships).values({
@@ -252,7 +252,7 @@ describeEmbeddedPostgres("authorization service", () => {
       principalId: userId,
       action: "tools:admin",
       permissionKey: "tools:admin",
-    })).resolves.toMatchObject({ allowed: true, reason: "allow_role_default" });
+    })).resolves.toMatchObject({ allowed: true, reason: "allow_explicit_grant" });
 
     await db
       .update(companyMemberships)
@@ -269,7 +269,7 @@ describeEmbeddedPostgres("authorization service", () => {
       principalId: userId,
       action: "tools:admin",
       permissionKey: "tools:admin",
-    })).resolves.toMatchObject({ allowed: false, reason: "deny_missing_grant" });
+    })).resolves.toMatchObject({ allowed: true, reason: "allow_explicit_grant" });
     await expect(db.select().from(principalPermissionGrants).where(and(
       eq(principalPermissionGrants.companyId, company.id),
       eq(principalPermissionGrants.principalId, userId),

server/src/security/board-key-owner-authority.ts

@@ -10,7 +10,6 @@ import type {
   BoardPermissionKey,
   PermissionKey,
 } from "@paperclipai/shared";
-import { grantsForHumanRole, normalizeHumanRole } from "../services/company-member-roles.js";
 
 export function isBoardKeyWriteAction(action: BoardPermissionKey) {
   return /:(?:write|manage|control|operate|run|decide|create|import_export)$/.test(action);
@@ -164,13 +163,6 @@ export async function ownerHasRequiredGrant(
     ));
   const liveKeysByCompany = new Map<string, Set<string>>();
   for (const row of rows) {
-    if (row.grantOrigin === "legacy_unknown") {
-      const currentRolePermissionKeys = new Set(
-        grantsForHumanRole(normalizeHumanRole(membershipRoleByCompany.get(row.companyId)))
-          .map((grant) => grant.permissionKey),
-      );
-      if (!currentRolePermissionKeys.has(row.permissionKey as PermissionKey)) continue;
-    }
     const liveKeys = liveKeysByCompany.get(row.companyId) ?? new Set<string>();
     liveKeys.add(row.permissionKey);
     liveKeysByCompany.set(row.companyId, liveKeys);

server/src/services/access.ts

@@ -1131,6 +1131,22 @@ export function accessService(db: Db) {
           .filter((permissionKey) => !nextDefaultKeys.has(permissionKey));
 
         if (retiredDefaultKeys.length > 0) {
+          const ambiguousLegacyGrants = await tx
+            .select({ id: principalPermissionGrants.id })
+            .from(principalPermissionGrants)
+            .where(and(
+              eq(principalPermissionGrants.companyId, companyId),
+              eq(principalPermissionGrants.principalType, "user"),
+              eq(principalPermissionGrants.principalId, existing.principalId),
+              inArray(principalPermissionGrants.permissionKey, retiredDefaultKeys),
+              eq(principalPermissionGrants.grantOrigin, "legacy_unknown"),
+            ));
+          if (ambiguousLegacyGrants.length > 0) {
+            throw conflict(
+              "Review this member's legacy permissions before changing their role",
+            );
+          }
+
           await tx
             .delete(principalPermissionGrants)
             .where(and(

server/src/services/authorization.ts

@@ -697,18 +697,6 @@ export function authorizationService(db: Db | DbTransaction) {
       });
     }
 
-    const legacyGrantMatchesCurrentRole = grant.grantOrigin !== "legacy_unknown"
-      || input.principalType !== "user"
-      || grantsForHumanRole(normalizeHumanRole(membership.membershipRole, "operator"))
-        .some((defaultGrant) => defaultGrant.permissionKey === input.permissionKey);
-    if (!legacyGrantMatchesCurrentRole) {
-      return deny({
-        action: input.action,
-        reason: "deny_missing_grant",
-        explanation: `Missing permission: ${input.permissionKey}.`,
-      });
-    }
-
     if (
       !(await scopeAllows(db, input.companyId, grant.scope, input.scope, {
         requireStructuredScope: input.permissionKey === "tasks:assign_scope",
@@ -729,10 +717,8 @@ export function authorizationService(db: Db | DbTransaction) {
 
     return allow({
       action: input.action,
-      reason: grant.grantOrigin === "legacy_unknown" ? "allow_role_default" : "allow_explicit_grant",
-      explanation: grant.grantOrigin === "legacy_unknown"
-        ? `Allowed by the ${membership.membershipRole ?? "operator"} membership role.`
-        : `Allowed by explicit grant ${input.permissionKey}.`,
+      reason: "allow_explicit_grant",
+      explanation: `Allowed by explicit grant ${input.permissionKey}.`,
       grant: {
         principalType: input.principalType,
         principalId: input.principalId,