mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-11 05:11:49 +01:00
fix(access): guard member service owner updates
This commit is contained in:
1 parent
6c10078422
commit
8b341f120f
2 files changed
+201
-23
No files matched your search
@@ -0,0 +1,99 @@
|
||||
import { randomUUID } from "node:crypto";
|
||||
import { eq } from "drizzle-orm";
|
||||
import { afterAll, afterEach, beforeAll, describe, expect, it } from "vitest";
|
||||
import {
|
||||
companies,
|
||||
companyMemberships,
|
||||
createDb,
|
||||
principalPermissionGrants,
|
||||
} from "@paperclipai/db";
|
||||
import {
|
||||
getEmbeddedPostgresTestSupport,
|
||||
startEmbeddedPostgresTestDatabase,
|
||||
} from "./helpers/embedded-postgres.js";
|
||||
import { accessService } from "../services/access.js";
|
||||
|
||||
const embeddedPostgresSupport = await getEmbeddedPostgresTestSupport();
|
||||
const describeEmbeddedPostgres = embeddedPostgresSupport.supported ? describe : describe.skip;
|
||||
|
||||
async function createCompanyWithOwner(db: ReturnType<typeof createDb>) {
|
||||
const company = await db
|
||||
.insert(companies)
|
||||
.values({
|
||||
name: `Access Service ${randomUUID()}`,
|
||||
issuePrefix: `AS${randomUUID().slice(0, 6).toUpperCase()}`,
|
||||
})
|
||||
.returning()
|
||||
.then((rows) => rows[0]!);
|
||||
|
||||
const owner = await db
|
||||
.insert(companyMemberships)
|
||||
.values({
|
||||
companyId: company.id,
|
||||
principalType: "user",
|
||||
principalId: `owner-${randomUUID()}`,
|
||||
status: "active",
|
||||
membershipRole: "owner",
|
||||
})
|
||||
.returning()
|
||||
.then((rows) => rows[0]!);
|
||||
|
||||
return { company, owner };
|
||||
}
|
||||
|
||||
describeEmbeddedPostgres("access service", () => {
|
||||
let db!: ReturnType<typeof createDb>;
|
||||
let tempDb: Awaited<ReturnType<typeof startEmbeddedPostgresTestDatabase>> | null = null;
|
||||
|
||||
beforeAll(async () => {
|
||||
tempDb = await startEmbeddedPostgresTestDatabase("paperclip-access-service-");
|
||||
db = createDb(tempDb.connectionString);
|
||||
}, 20_000);
|
||||
|
||||
afterEach(async () => {
|
||||
await db.delete(principalPermissionGrants);
|
||||
await db.delete(companyMemberships);
|
||||
await db.delete(companies);
|
||||
});
|
||||
|
||||
afterAll(async () => {
|
||||
await tempDb?.cleanup();
|
||||
});
|
||||
|
||||
it("rejects combined access updates that would demote the last active owner", async () => {
|
||||
const { company, owner } = await createCompanyWithOwner(db);
|
||||
const access = accessService(db);
|
||||
|
||||
await expect(
|
||||
access.updateMemberAndPermissions(
|
||||
company.id,
|
||||
owner.id,
|
||||
{ membershipRole: "admin", grants: [] },
|
||||
"admin-user",
|
||||
),
|
||||
).rejects.toThrow("Cannot remove the last active owner");
|
||||
|
||||
const unchanged = await db
|
||||
.select()
|
||||
.from(companyMemberships)
|
||||
.where(eq(companyMemberships.id, owner.id))
|
||||
.then((rows) => rows[0]!);
|
||||
expect(unchanged.membershipRole).toBe("owner");
|
||||
});
|
||||
|
||||
it("rejects role-only updates that would suspend the last active owner", async () => {
|
||||
const { company, owner } = await createCompanyWithOwner(db);
|
||||
const access = accessService(db);
|
||||
|
||||
await expect(
|
||||
access.updateMember(company.id, owner.id, { status: "suspended" }),
|
||||
).rejects.toThrow("Cannot remove the last active owner");
|
||||
|
||||
const unchanged = await db
|
||||
.select()
|
||||
.from(companyMemberships)
|
||||
.where(eq(companyMemberships.id, owner.id))
|
||||
.then((rows) => rows[0]!);
|
||||
expect(unchanged.status).toBe("active");
|
||||
});
|
||||
});
|
||||
+102
-23
@@ -6,6 +6,7 @@ import {
|
||||
principalPermissionGrants,
|
||||
} from "@paperclipai/db";
|
||||
import type { PermissionKey, PrincipalType } from "@paperclipai/shared";
|
||||
import { conflict } from "../errors.js";
|
||||
|
||||
type MembershipRow = typeof companyMemberships.$inferSelect;
|
||||
type GrantInput = {
|
||||
@@ -153,15 +154,52 @@ export function accessService(db: Db) {
|
||||
},
|
||||
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) => {
|
||||
await tx.execute(sql`
|
||||
select ${companyMemberships.id}
|
||||
from ${companyMemberships}
|
||||
where ${companyMemberships.companyId} = ${companyId}
|
||||
and ${companyMemberships.principalType} = 'user'
|
||||
and ${companyMemberships.status} = 'active'
|
||||
and ${companyMemberships.membershipRole} = 'owner'
|
||||
for update
|
||||
`);
|
||||
|
||||
const existing = await tx
|
||||
.select()
|
||||
.from(companyMemberships)
|
||||
.where(and(eq(companyMemberships.companyId, companyId), eq(companyMemberships.id, memberId)))
|
||||
.then((rows) => rows[0] ?? null);
|
||||
if (!existing) return null;
|
||||
|
||||
const nextMembershipRole =
|
||||
data.membershipRole !== undefined ? data.membershipRole : existing.membershipRole;
|
||||
const nextStatus = data.status ?? existing.status;
|
||||
|
||||
if (
|
||||
existing.principalType === "user" &&
|
||||
existing.status === "active" &&
|
||||
existing.membershipRole === "owner" &&
|
||||
(nextStatus !== "active" || nextMembershipRole !== "owner")
|
||||
) {
|
||||
const activeOwnerCount = await tx
|
||||
.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 now = new Date();
|
||||
const updated = await tx
|
||||
.update(companyMemberships)
|
||||
.set({
|
||||
@@ -429,21 +467,62 @@ export function accessService(db: Db) {
|
||||
status?: "pending" | "active" | "suspended";
|
||||
},
|
||||
) {
|
||||
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;
|
||||
return db
|
||||
.update(companyMemberships)
|
||||
.set({
|
||||
membershipRole: nextMembershipRole,
|
||||
status: nextStatus,
|
||||
updatedAt: new Date(),
|
||||
})
|
||||
.where(eq(companyMemberships.id, existing.id))
|
||||
.returning()
|
||||
.then((rows) => rows[0] ?? existing);
|
||||
return db.transaction(async (tx) => {
|
||||
await tx.execute(sql`
|
||||
select ${companyMemberships.id}
|
||||
from ${companyMemberships}
|
||||
where ${companyMemberships.companyId} = ${companyId}
|
||||
and ${companyMemberships.principalType} = 'user'
|
||||
and ${companyMemberships.status} = 'active'
|
||||
and ${companyMemberships.membershipRole} = 'owner'
|
||||
for update
|
||||
`);
|
||||
|
||||
const existing = await tx
|
||||
.select()
|
||||
.from(companyMemberships)
|
||||
.where(and(eq(companyMemberships.companyId, companyId), eq(companyMemberships.id, memberId)))
|
||||
.then((rows) => rows[0] ?? null);
|
||||
if (!existing) return null;
|
||||
|
||||
const nextMembershipRole =
|
||||
data.membershipRole !== undefined ? data.membershipRole : existing.membershipRole;
|
||||
const nextStatus = data.status ?? existing.status;
|
||||
|
||||
if (
|
||||
existing.principalType === "user" &&
|
||||
existing.status === "active" &&
|
||||
existing.membershipRole === "owner" &&
|
||||
(nextStatus !== "active" || nextMembershipRole !== "owner")
|
||||
) {
|
||||
const activeOwnerCount = await tx
|
||||
.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");
|
||||
}
|
||||
}
|
||||
|
||||
return tx
|
||||
.update(companyMemberships)
|
||||
.set({
|
||||
membershipRole: nextMembershipRole,
|
||||
status: nextStatus,
|
||||
updatedAt: new Date(),
|
||||
})
|
||||
.where(eq(companyMemberships.id, existing.id))
|
||||
.returning()
|
||||
.then((rows) => rows[0] ?? existing);
|
||||
});
|
||||
}
|
||||
|
||||
return {
|
||||
|
||||
Reference in new issue
Block a user