diff --git a/packages/db/src/migrations/0278_natural_nehzno.sql b/packages/db/src/migrations/0278_natural_nehzno.sql index aa40e8e32d..d6bdac17b4 100644 --- a/packages/db/src/migrations/0278_natural_nehzno.sql +++ b/packages/db/src/migrations/0278_natural_nehzno.sql @@ -1,2 +1,3 @@ -ALTER TABLE "principal_permission_grants" ADD COLUMN "grant_origin" text DEFAULT 'explicit' NOT NULL;--> statement-breakpoint -ALTER TABLE "principal_permission_grants" ADD CONSTRAINT "principal_permission_grants_origin_check" CHECK ("principal_permission_grants"."grant_origin" in ('explicit', 'role_default')); +ALTER TABLE "principal_permission_grants" ADD COLUMN "grant_origin" text DEFAULT 'legacy_unknown' NOT NULL;--> statement-breakpoint +ALTER TABLE "principal_permission_grants" ALTER COLUMN "grant_origin" SET DEFAULT 'explicit';--> statement-breakpoint +ALTER TABLE "principal_permission_grants" ADD CONSTRAINT "principal_permission_grants_origin_check" CHECK ("principal_permission_grants"."grant_origin" in ('explicit', 'role_default', 'legacy_unknown')); diff --git a/packages/db/src/migrations/meta/0278_snapshot.json b/packages/db/src/migrations/meta/0278_snapshot.json index cafb348634..5ea03aa093 100644 --- a/packages/db/src/migrations/meta/0278_snapshot.json +++ b/packages/db/src/migrations/meta/0278_snapshot.json @@ -35518,7 +35518,7 @@ "checkConstraints": { "principal_permission_grants_origin_check": { "name": "principal_permission_grants_origin_check", - "value": "\"principal_permission_grants\".\"grant_origin\" in ('explicit', 'role_default')" + "value": "\"principal_permission_grants\".\"grant_origin\" in ('explicit', 'role_default', 'legacy_unknown')" } }, "isRLSEnabled": false @@ -47824,4 +47824,4 @@ "schemas": {}, "tables": {} } -} \ No newline at end of file +} diff --git a/packages/db/src/principal-permission-grant-origin-migration.test.ts b/packages/db/src/principal-permission-grant-origin-migration.test.ts index dab1243f29..fbe68abfd0 100644 --- a/packages/db/src/principal-permission-grant-origin-migration.test.ts +++ b/packages/db/src/principal-permission-grant-origin-migration.test.ts @@ -27,7 +27,7 @@ async function migrationStatements() { } describeEmbeddedPostgres("principal permission grant origin migration", () => { - it("preserves ambiguous historical grants as explicit", async () => { + it("preserves ambiguous historical grants with unknown provenance", async () => { const database = await startEmbeddedPostgresTestDatabase("paperclip-grant-origin-"); cleanups.push(database.cleanup); const sql = postgres(database.connectionString, { max: 1, onnotice: () => {} }); @@ -95,6 +95,19 @@ describeEmbeddedPostgres("principal permission grant origin migration", () => { for (const statement of await migrationStatements()) { await sql.unsafe(statement); } + await sql.unsafe(` + INSERT INTO principal_permission_grants ( + id, company_id, principal_type, principal_id, permission_key, scope, granted_by_user_id + ) VALUES ( + '00000000-0000-4000-8000-000000000020', + '00000000-0000-4000-8000-000000000001', + 'user', + 'new-user', + 'tools:use', + NULL, + NULL + ) + `); const rows = await sql.unsafe>(` SELECT id, grant_origin @@ -102,16 +115,17 @@ describeEmbeddedPostgres("principal permission grant origin migration", () => { ORDER BY id `); expect(rows).toEqual([ - { id: "00000000-0000-4000-8000-000000000010", grant_origin: "explicit" }, - { id: "00000000-0000-4000-8000-000000000011", grant_origin: "explicit" }, - { id: "00000000-0000-4000-8000-000000000012", grant_origin: "explicit" }, - { id: "00000000-0000-4000-8000-000000000013", grant_origin: "explicit" }, - { id: "00000000-0000-4000-8000-000000000014", grant_origin: "explicit" }, - { id: "00000000-0000-4000-8000-000000000015", grant_origin: "explicit" }, - { id: "00000000-0000-4000-8000-000000000016", grant_origin: "explicit" }, - { id: "00000000-0000-4000-8000-000000000017", grant_origin: "explicit" }, - { id: "00000000-0000-4000-8000-000000000018", grant_origin: "explicit" }, - { id: "00000000-0000-4000-8000-000000000019", grant_origin: "explicit" }, + { id: "00000000-0000-4000-8000-000000000010", grant_origin: "legacy_unknown" }, + { id: "00000000-0000-4000-8000-000000000011", grant_origin: "legacy_unknown" }, + { id: "00000000-0000-4000-8000-000000000012", grant_origin: "legacy_unknown" }, + { id: "00000000-0000-4000-8000-000000000013", grant_origin: "legacy_unknown" }, + { id: "00000000-0000-4000-8000-000000000014", grant_origin: "legacy_unknown" }, + { id: "00000000-0000-4000-8000-000000000015", grant_origin: "legacy_unknown" }, + { id: "00000000-0000-4000-8000-000000000016", grant_origin: "legacy_unknown" }, + { id: "00000000-0000-4000-8000-000000000017", grant_origin: "legacy_unknown" }, + { id: "00000000-0000-4000-8000-000000000018", grant_origin: "legacy_unknown" }, + { id: "00000000-0000-4000-8000-000000000019", grant_origin: "legacy_unknown" }, + { id: "00000000-0000-4000-8000-000000000020", grant_origin: "explicit" }, ]); } finally { await sql.end(); diff --git a/packages/db/src/schema/principal_permission_grants.ts b/packages/db/src/schema/principal_permission_grants.ts index 0fba447212..f222510fa2 100644 --- a/packages/db/src/schema/principal_permission_grants.ts +++ b/packages/db/src/schema/principal_permission_grants.ts @@ -23,7 +23,7 @@ export const principalPermissionGrants = pgTable( grantOrigin: text("grant_origin") .notNull() .default("explicit") - .$type<"explicit" | "role_default">(), + .$type<"explicit" | "role_default" | "legacy_unknown">(), grantedByUserId: text("granted_by_user_id"), createdAt: timestamp("created_at", { withTimezone: true }).notNull().defaultNow(), updatedAt: timestamp("updated_at", { withTimezone: true }).notNull().defaultNow(), @@ -41,7 +41,7 @@ export const principalPermissionGrants = pgTable( ), grantOriginCheck: check( "principal_permission_grants_origin_check", - sql`${table.grantOrigin} in ('explicit', 'role_default')`, + sql`${table.grantOrigin} in ('explicit', 'role_default', 'legacy_unknown')`, ), }), ); diff --git a/server/src/__tests__/access-routes-permissions-upgrade.test.ts b/server/src/__tests__/access-routes-permissions-upgrade.test.ts index 4947b9def9..c05b9f2a4b 100644 --- a/server/src/__tests__/access-routes-permissions-upgrade.test.ts +++ b/server/src/__tests__/access-routes-permissions-upgrade.test.ts @@ -261,6 +261,50 @@ describeEmbeddedPostgres("access routes permissions upgrade compatibility", () = )).resolves.toBe(false); }); + it("preserves ambiguous legacy grants while limiting them to the current role", async () => { + const { company, owner } = await createCompanyWithOwner(db); + const member = await db.insert(companyMemberships).values({ + companyId: company.id, + principalType: "user", + principalId: `legacy-admin-${randomUUID()}`, + status: "active", + membershipRole: "admin", + }).returning().then((rows) => rows[0]!); + await db.insert(principalPermissionGrants).values({ + companyId: company.id, + principalType: "user", + principalId: member.principalId, + permissionKey: "tools:admin", + grantOrigin: "legacy_unknown", + }); + + 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}`) + .send({ membershipRole: "operator" }); + expect(res.status, JSON.stringify(res.body)).toBe(200); + + const preserved = await db + .select() + .from(principalPermissionGrants) + .where(eq(principalPermissionGrants.principalId, member.principalId)); + expect(preserved).toEqual(expect.arrayContaining([ + expect.objectContaining({ permissionKey: "tools:admin", grantOrigin: "legacy_unknown" }), + ])); + 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 () => { const { company, owner } = await createCompanyWithOwner(db); const member = await db.insert(companyMemberships).values({ diff --git a/server/src/security/board-key-owner-authority.ts b/server/src/security/board-key-owner-authority.ts index fd43a6b976..12c1c114ab 100644 --- a/server/src/security/board-key-owner-authority.ts +++ b/server/src/security/board-key-owner-authority.ts @@ -10,6 +10,7 @@ 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); @@ -135,6 +136,8 @@ export async function ownerHasRequiredGrant( .select({ companyId: principalPermissionGrants.companyId, permissionKey: principalPermissionGrants.permissionKey, + grantOrigin: principalPermissionGrants.grantOrigin, + membershipRole: companyMemberships.membershipRole, }) .from(principalPermissionGrants) .innerJoin(companyMemberships, and( @@ -151,6 +154,13 @@ export async function ownerHasRequiredGrant( )); const liveKeysByCompany = new Map>(); for (const row of rows) { + if (row.grantOrigin === "legacy_unknown") { + const currentRolePermissionKeys = new Set( + grantsForHumanRole(normalizeHumanRole(row.membershipRole)) + .map((grant) => grant.permissionKey), + ); + if (!currentRolePermissionKeys.has(row.permissionKey as PermissionKey)) continue; + } const liveKeys = liveKeysByCompany.get(row.companyId) ?? new Set(); liveKeys.add(row.permissionKey); liveKeysByCompany.set(row.companyId, liveKeys); diff --git a/server/src/services/principal-access-compatibility.ts b/server/src/services/principal-access-compatibility.ts index 680db45faa..cfca7dbbab 100644 --- a/server/src/services/principal-access-compatibility.ts +++ b/server/src/services/principal-access-compatibility.ts @@ -22,7 +22,7 @@ export async function insertMissingPrincipalGrants( principalId: string; grants: GrantInput[]; grantedByUserId: string | null; - grantOrigin?: "explicit" | "role_default"; + grantOrigin?: "explicit" | "role_default" | "legacy_unknown"; }, ): Promise { if (input.grants.length === 0) return 0;