mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-11 05:11:49 +01:00
fix(access): make owner membership updates atomic
Guard last-owner membership changes inside the same transaction that applies role/status and grant updates. Lock active owner rows before checking the owner count so concurrent downgrades cannot leave a company without an owner. Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
1 parent
802252f377
commit
ac955be548
1 file changed
+146
-61
+146
-61
@@ -9,7 +9,7 @@ import path from "node:path";
|
||||
import { fileURLToPath } from "node:url";
|
||||
import { Router } from "express";
|
||||
import type { Request } from "express";
|
||||
import { and, desc, eq, gt, inArray, isNotNull, isNull, lte } from "drizzle-orm";
|
||||
import { and, desc, eq, gt, inArray, isNotNull, isNull, lte, sql } from "drizzle-orm";
|
||||
import type { Db } from "@paperclipai/db";
|
||||
import {
|
||||
assets,
|
||||
@@ -38,7 +38,7 @@ import {
|
||||
updateUserCompanyAccessSchema,
|
||||
PERMISSION_KEYS
|
||||
} from "@paperclipai/shared";
|
||||
import type { DeploymentExposure, DeploymentMode } from "@paperclipai/shared";
|
||||
import type { DeploymentExposure, DeploymentMode, PermissionKey } from "@paperclipai/shared";
|
||||
import {
|
||||
forbidden,
|
||||
conflict,
|
||||
@@ -82,6 +82,11 @@ const INVITE_TOKEN_SUFFIX_LENGTH = 8;
|
||||
const INVITE_TOKEN_MAX_RETRIES = 5;
|
||||
const COMPANY_INVITE_TTL_MS = 72 * 60 * 60 * 1000;
|
||||
|
||||
type MemberGrantPayload = {
|
||||
permissionKey: PermissionKey;
|
||||
scope?: Record<string, unknown> | null;
|
||||
};
|
||||
|
||||
function createInviteToken() {
|
||||
const bytes = randomBytes(INVITE_TOKEN_SUFFIX_LENGTH);
|
||||
let suffix = "";
|
||||
@@ -3618,39 +3623,69 @@ export function accessRoutes(
|
||||
const memberId = req.params.memberId as string;
|
||||
await assertCompanyPermission(req, companyId, "users:manage_permissions");
|
||||
|
||||
const existing = await access.getMemberById(companyId, memberId);
|
||||
if (!existing) throw notFound("Member not found");
|
||||
const updated = await 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 nextMembershipRole =
|
||||
req.body.membershipRole !== undefined
|
||||
? req.body.membershipRole
|
||||
: existing.membershipRole;
|
||||
const nextStatus = req.body.status ?? existing.status;
|
||||
|
||||
if (
|
||||
existing.principalType === "user" &&
|
||||
existing.status === "active" &&
|
||||
existing.membershipRole === "owner" &&
|
||||
(nextStatus !== "active" || nextMembershipRole !== "owner")
|
||||
) {
|
||||
const activeOwnerCount = await db
|
||||
.select({ id: companyMemberships.id })
|
||||
const existing = await tx
|
||||
.select()
|
||||
.from(companyMemberships)
|
||||
.where(
|
||||
and(
|
||||
eq(companyMemberships.companyId, companyId),
|
||||
eq(companyMemberships.principalType, "user"),
|
||||
eq(companyMemberships.status, "active"),
|
||||
eq(companyMemberships.membershipRole, "owner"),
|
||||
eq(companyMemberships.id, memberId),
|
||||
),
|
||||
)
|
||||
.then((rows) => rows.length);
|
||||
if (activeOwnerCount <= 1) {
|
||||
throw conflict("Cannot remove the last active owner");
|
||||
}
|
||||
}
|
||||
.then((rows) => rows[0] ?? null);
|
||||
if (!existing) return null;
|
||||
|
||||
const updated = await access.updateMember(companyId, memberId, req.body);
|
||||
const nextMembershipRole =
|
||||
req.body.membershipRole !== undefined
|
||||
? req.body.membershipRole
|
||||
: existing.membershipRole;
|
||||
const nextStatus = req.body.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);
|
||||
});
|
||||
if (!updated) throw notFound("Member not found");
|
||||
|
||||
await logActivity(db, {
|
||||
@@ -3682,48 +3717,98 @@ export function accessRoutes(
|
||||
const memberId = req.params.memberId as string;
|
||||
await assertCompanyPermission(req, companyId, "users:manage_permissions");
|
||||
|
||||
const existing = await access.getMemberById(companyId, memberId);
|
||||
if (!existing) throw notFound("Member not found");
|
||||
const updated = await 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 nextMembershipRole =
|
||||
req.body.membershipRole !== undefined
|
||||
? req.body.membershipRole
|
||||
: existing.membershipRole;
|
||||
const nextStatus = req.body.status ?? existing.status;
|
||||
|
||||
if (
|
||||
existing.principalType === "user" &&
|
||||
existing.status === "active" &&
|
||||
existing.membershipRole === "owner" &&
|
||||
(nextStatus !== "active" || nextMembershipRole !== "owner")
|
||||
) {
|
||||
const activeOwnerCount = await db
|
||||
.select({ id: companyMemberships.id })
|
||||
const existing = await tx
|
||||
.select()
|
||||
.from(companyMemberships)
|
||||
.where(
|
||||
and(
|
||||
eq(companyMemberships.companyId, companyId),
|
||||
eq(companyMemberships.principalType, "user"),
|
||||
eq(companyMemberships.status, "active"),
|
||||
eq(companyMemberships.membershipRole, "owner"),
|
||||
eq(companyMemberships.id, memberId),
|
||||
),
|
||||
)
|
||||
.then((rows) => rows.length);
|
||||
if (activeOwnerCount <= 1) {
|
||||
throw conflict("Cannot remove the last active owner");
|
||||
}
|
||||
}
|
||||
.then((rows) => rows[0] ?? null);
|
||||
if (!existing) return null;
|
||||
|
||||
const updated = await access.updateMemberAndPermissions(
|
||||
companyId,
|
||||
memberId,
|
||||
{
|
||||
membershipRole: req.body.membershipRole,
|
||||
status: req.body.status,
|
||||
grants: req.body.grants ?? [],
|
||||
},
|
||||
req.actor.userId ?? null,
|
||||
);
|
||||
const nextMembershipRole =
|
||||
req.body.membershipRole !== undefined
|
||||
? req.body.membershipRole
|
||||
: existing.membershipRole;
|
||||
const nextStatus = req.body.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 updatedMember = await tx
|
||||
.update(companyMemberships)
|
||||
.set({
|
||||
membershipRole: nextMembershipRole,
|
||||
status: nextStatus,
|
||||
updatedAt: now,
|
||||
})
|
||||
.where(eq(companyMemberships.id, existing.id))
|
||||
.returning()
|
||||
.then((rows) => rows[0] ?? existing);
|
||||
|
||||
await tx
|
||||
.delete(principalPermissionGrants)
|
||||
.where(
|
||||
and(
|
||||
eq(principalPermissionGrants.companyId, companyId),
|
||||
eq(principalPermissionGrants.principalType, existing.principalType),
|
||||
eq(principalPermissionGrants.principalId, existing.principalId),
|
||||
),
|
||||
);
|
||||
|
||||
const grants = (req.body.grants ?? []) as MemberGrantPayload[];
|
||||
if (grants.length > 0) {
|
||||
await tx.insert(principalPermissionGrants).values(
|
||||
grants.map((grant) => ({
|
||||
companyId,
|
||||
principalType: existing.principalType,
|
||||
principalId: existing.principalId,
|
||||
permissionKey: grant.permissionKey,
|
||||
scope: grant.scope ?? null,
|
||||
grantedByUserId: req.actor.userId ?? null,
|
||||
createdAt: now,
|
||||
updatedAt: now,
|
||||
})),
|
||||
);
|
||||
}
|
||||
|
||||
return updatedMember;
|
||||
});
|
||||
if (!updated) throw notFound("Member not found");
|
||||
|
||||
await logActivity(db, {
|
||||
|
||||
Reference in new issue
Block a user