fix(auth): constrain ambiguous legacy grants
Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
parent
a2bd9b7a36
commit
88c2dff1b0
|
|
@ -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'));
|
||||
|
|
|
|||
|
|
@ -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": {}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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<Array<{ id: string; grant_origin: string }>>(`
|
||||
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();
|
||||
|
|
|
|||
|
|
@ -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')`,
|
||||
),
|
||||
}),
|
||||
);
|
||||
|
|
|
|||
|
|
@ -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({
|
||||
|
|
|
|||
|
|
@ -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<string, Set<string>>();
|
||||
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<string>();
|
||||
liveKeys.add(row.permissionKey);
|
||||
liveKeysByCompany.set(row.companyId, liveKeys);
|
||||
|
|
|
|||
|
|
@ -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<number> {
|
||||
if (input.grants.length === 0) return 0;
|
||||
|
|
|
|||
Loading…
Reference in New Issue