fix(admin): the last-admin rule counts only admins who can sign in

Demoting an admin counted every admin, pending deletion included, so after
one admin deleted their account the other could step down and leave the
instance with no admin who can sign in or undo the deletion. Both the
demotion and an admin's own deletion now count the other admins not pending
deletion, inside the transaction that makes the change, under one roster
lock, so two at the same moment cannot each count the other.
This commit is contained in:
Marc-André Bombeck committed 2026-10-25 17:02:32 +01:00
1 parent 4137aa546e
commit b37e4f5df7
4 files changed
+126 -39

No files matched your search

@@ -1,15 +1,21 @@
import { describe, it, expect, vi, beforeEach } from "vitest";
import { NextRequest } from "next/server";
vi.mock("@/lib/db", () => ({
prisma: {
vi.mock("@/lib/db", () => {
const prisma = {
user: {
findUnique: vi.fn(),
count: vi.fn(),
update: vi.fn(),
},
},
}));
$queryRaw: vi.fn(async () => [{ locked: 1 }]),
$transaction: vi.fn(),
};
prisma.$transaction.mockImplementation(
async (fn: (tx: typeof prisma) => unknown) => fn(prisma),
);
return { prisma };
});
vi.mock("@/lib/api-handler", async () => {
const actual =
@@ -111,7 +117,8 @@ describe("PUT /api/admin/users/[id]", () => {
id: "u1",
role: "ADMIN",
} as never);
vi.mocked(prisma.user.count).mockResolvedValue(1);
// No other admin who can sign in.
vi.mocked(prisma.user.count).mockResolvedValue(0);
const res = await PUT(jsonReq({ role: "USER" }), params("u1"));
expect(res.status).toBe(400);
const body = (await res.json()) as { error: string };
@@ -119,12 +126,31 @@ describe("PUT /api/admin/users/[id]", () => {
expect(vi.mocked(prisma.user.update)).not.toHaveBeenCalled();
});
// An admin pending deletion cannot sign in and is erased when the window
// ends; counted, they let the last usable admin step down.
it("counts only the other admins who are not pending deletion, under the roster lock", async () => {
vi.mocked(prisma.user.findUnique).mockResolvedValue({
id: "u1",
role: "ADMIN",
} as never);
vi.mocked(prisma.user.count).mockResolvedValue(0);
await PUT(jsonReq({ role: "USER" }), params("u1"));
expect(vi.mocked(prisma.user.count).mock.calls[0]?.[0]).toEqual({
where: {
role: "ADMIN",
id: { not: "u1" },
deletionRequestedAt: null,
},
});
expect(prisma.$queryRaw).toHaveBeenCalled();
});
it("allows demoting an admin while another admin exists", async () => {
vi.mocked(prisma.user.findUnique).mockResolvedValue({
id: "u1",
role: "ADMIN",
} as never);
vi.mocked(prisma.user.count).mockResolvedValue(2);
vi.mocked(prisma.user.count).mockResolvedValue(1);
vi.mocked(prisma.user.update).mockResolvedValue({
id: "u1",
username: "alice",
+47 -33
View File
@@ -1,4 +1,8 @@
import { prisma } from "@/lib/db";
import {
assertAnotherAdminStays,
LastAdminError,
} from "@/lib/auth/admin-roster";
import { apiHandler, requireAdmin } from "@/lib/api-handler";
import { auditLog } from "@/lib/auth/audit";
import { apiSuccess, apiError, getClientIp } from "@/lib/api-response";
@@ -55,43 +59,53 @@ export const PUT = apiHandler(
});
if (!target) return apiError("User not found", 404);
// Prevent removing the last admin
if (parsed.data.role === "USER" && target.role === "ADMIN") {
const adminCount = await prisma.user.count({ where: { role: "ADMIN" } });
if (adminCount <= 1) {
const demoting = parsed.data.role === "USER" && target.role === "ADMIN";
let updatedUser;
try {
updatedUser = await prisma.$transaction(async (tx) => {
// Another admin who can sign in must stay: one pending deletion does
// not count, and the count holds the roster lock an admin's own
// deletion takes too.
if (demoting) await assertAnotherAdminStays(tx, id);
return tx.user.update({
where: { id },
data: {
...(parsed.data.username !== undefined && {
username: parsed.data.username,
}),
...(parsed.data.email !== undefined && {
email: parsed.data.email,
}),
...(parsed.data.role !== undefined && { role: parsed.data.role }),
...(parsed.data.mfaEnforced !== undefined && {
mfaEnforced: parsed.data.mfaEnforced,
}),
...(parsed.data.documentQuotaBytes !== undefined && {
documentQuotaBytes:
parsed.data.documentQuotaBytes === null
? null
: BigInt(parsed.data.documentQuotaBytes),
}),
},
select: {
id: true,
username: true,
email: true,
role: true,
createdAt: true,
mfaEnforced: true,
documentQuotaBytes: true,
},
});
});
} catch (error) {
if (error instanceof LastAdminError) {
return apiError("The last admin cannot be demoted", 400);
}
throw error;
}
const updatedUser = await prisma.user.update({
where: { id },
data: {
...(parsed.data.username !== undefined && {
username: parsed.data.username,
}),
...(parsed.data.email !== undefined && { email: parsed.data.email }),
...(parsed.data.role !== undefined && { role: parsed.data.role }),
...(parsed.data.mfaEnforced !== undefined && {
mfaEnforced: parsed.data.mfaEnforced,
}),
...(parsed.data.documentQuotaBytes !== undefined && {
documentQuotaBytes:
parsed.data.documentQuotaBytes === null
? null
: BigInt(parsed.data.documentQuotaBytes),
}),
},
select: {
id: true,
username: true,
email: true,
role: true,
createdAt: true,
mfaEnforced: true,
documentQuotaBytes: true,
},
});
await auditLog("admin.user.update", {
userId: user.id,
ipAddress: getClientIp(request),
+10
View File
@@ -22,6 +22,10 @@ import {
lockAccountForDeletion,
} from "@/lib/account-deletion/lifecycle";
import { ACTIVE_ACCOUNT } from "@/lib/account-deletion/active-account";
import {
assertAnotherAdminStays,
LastAdminError,
} from "@/lib/auth/admin-roster";
export const dynamic = "force-dynamic";
@@ -110,6 +114,9 @@ export const DELETE = apiHandler(async (request: NextRequest) => {
await deleteGuardianAccountWithLifecycle(
userId,
async (tx) => {
// Checked again under the roster lock: two admins deleting at the
// same moment must not each count the other.
if (user.role === "ADMIN") await assertAnotherAdminStays(tx, userId);
// The invariant has now passed, so this success event cannot survive
// a refusal. On an immediate erasure it is purged with the rest of the
// account's audit history; during a grace window it is the account's
@@ -145,6 +152,9 @@ export const DELETE = apiHandler(async (request: NextRequest) => {
{ erasing: purgeAt === null },
);
} catch (error) {
if (error instanceof LastAdminError) {
return apiError("The last admin account cannot be deleted", 400);
}
if (error instanceof LastManagedGuardianError) {
return apiError(
"Add another Guardian or delete the managed profile first",
+37
View File
@@ -0,0 +1,37 @@
/**
* The rule that an instance keeps an admin who can sign in.
*
* Two changes can take the last one away: an admin demoted, and an admin
* deleting their own account. Both count the other admins who are not pending
* deletion (one pending deletion cannot sign in and is erased when the window
* ends) under one transaction-scoped lock, so two of them at the same moment
* cannot each count the other and leave nobody.
*/
import type { Prisma } from "@/generated/prisma/client";
import { ACTIVE_ACCOUNT } from "@/lib/account-deletion/active-account";
/** Thrown inside the transaction to roll the change back. */
export class LastAdminError extends Error {
constructor() {
super("the instance would be left without an admin who can sign in");
this.name = "LastAdminError";
}
}
/**
* Hold the admin roster for the rest of `tx`, then throw `LastAdminError`
* unless an admin other than `leavingId` stays, not pending deletion.
*/
export async function assertAnotherAdminStays(
tx: Prisma.TransactionClient,
leavingId: string,
): Promise<void> {
await tx.$queryRaw`
SELECT 1 AS locked
FROM pg_advisory_xact_lock(hashtextextended('admin-roster', 0))
`;
const others = await tx.user.count({
where: { role: "ADMIN", id: { not: leavingId }, ...ACTIVE_ACCOUNT },
});
if (others === 0) throw new LastAdminError();
}