db: the owner account cannot be demoted or deleted
Two guards on user 1, the bootstrap account, so ownership survives whatever happens to the rows. ck_users_owner_is_super_admin — CHECK ((id <> 1) OR (role = 'Super Admin')). In the database rather than in application code because the point is that it holds against a stray UPDATE, a migration script or someone at a psql prompt, not just against the API. A row-level CHECK can say "if this row is user 1 then its role is Super Admin"; it cannot say "some row must be Super Admin", which would need to see other rows. So it pins the bootstrap account and nothing else — promoting and demoting everyone else stays free. deleteUser() refuses id 1, because a CHECK cannot stop a DELETE and removing the owner reaches the same end by another route: nobody who can open the vault, no identity for the agent sidecar to run as, a web origin restricted to a Super Admin that no longer exists, and the passkeys cascaded away so there is no signing back in. It throws rather than returning false — its eventual caller is a manage-users flow, where a silent false reads as "already gone". OWNER_USER_ID is exported from the schema and used by both, so the number appears once. Tested on a scratch database, and the first harness was wrong — a shell variable holding a command did not expand, every statement failed with "command not found", and the check reported them all as allowed. Re-run directly: insert user 1 as Super Admin allowed demote user 1 -> Member REJECTED by CHECK demote user 1 -> Admin REJECTED by CHECK insert user 1 as Member REJECTED by CHECK demote/promote users 2 and 3 allowed deleteUser(1) refused with the message above deleteUser(3) deleted Pushing twice showed the CHECK adds no diff churn — still only the two known pk_music_now_playing statements. Every row in officer_dev already satisfies it, so it will apply without touching data. NOT covered: nothing stops a second account also being Super Admin. The rule asked for was "user 1 is always Super Admin", not "only user 1 is", and the two are different constraints. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -1,6 +1,6 @@
|
||||
import { eq, and, lt, sql } from 'drizzle-orm';
|
||||
import { db } from '../db';
|
||||
import { users, passkeys, passkeyChallenges, tokenBlacklist } from '../schema';
|
||||
import { users, passkeys, passkeyChallenges, tokenBlacklist, OWNER_USER_ID } from '../schema';
|
||||
import type { UserSelect, UserInsert, PasskeySelect, PasskeyInsert } from '../types';
|
||||
|
||||
// ── Users ──
|
||||
@@ -43,11 +43,26 @@ export async function createUser(data: UserInsert): Promise<UserSelect> {
|
||||
}
|
||||
|
||||
export async function updateUser(id: number, data: Partial<Omit<UserSelect, 'id'>>): Promise<UserSelect | undefined> {
|
||||
const [user] = await db.update(users).set({ ...data, updatedAt: new Date() }).where(eq(users.id, id)).returning();
|
||||
const [user] = await db
|
||||
.update(users)
|
||||
.set({ ...data, updatedAt: new Date() })
|
||||
.where(eq(users.id, id))
|
||||
.returning();
|
||||
return user;
|
||||
}
|
||||
|
||||
// The owner cannot be deleted. ck_users_owner_is_super_admin pins user 1's role, but a row-level CHECK
|
||||
// cannot stop a DELETE, and deleting the owner is the same outcome by another route: a platform with
|
||||
// nobody who can reach the vault, no identity for the agent sidecar to run as, and a web origin
|
||||
// restricted to a Super Admin that no longer exists. Cascades would take the passkeys with it, so there
|
||||
// is no signing back in either.
|
||||
//
|
||||
// Throws rather than returning false. This has no callers today; the one it will get is a
|
||||
// manage-users flow, and a silent `false` there reads as "already gone".
|
||||
export async function deleteUser(id: number): Promise<boolean> {
|
||||
if (id === OWNER_USER_ID) {
|
||||
throw new Error(`Refusing to delete user ${OWNER_USER_ID}: the platform owner cannot be removed.`);
|
||||
}
|
||||
const result = await db.delete(users).where(eq(users.id, id)).returning({ id: users.id });
|
||||
return result.length > 0;
|
||||
}
|
||||
@@ -59,10 +74,16 @@ export async function getPasskeysByUserId(userId: number): Promise<PasskeySelect
|
||||
}
|
||||
|
||||
export async function getPasskeysByUserIdAndOrigin(userId: number, origin: string): Promise<PasskeySelect[]> {
|
||||
return db.select().from(passkeys).where(and(eq(passkeys.userId, userId), eq(passkeys.origin, origin)));
|
||||
return db
|
||||
.select()
|
||||
.from(passkeys)
|
||||
.where(and(eq(passkeys.userId, userId), eq(passkeys.origin, origin)));
|
||||
}
|
||||
|
||||
export async function getPasskeyByCredentialId(userId: number, credentialId: string): Promise<PasskeySelect | undefined> {
|
||||
export async function getPasskeyByCredentialId(
|
||||
userId: number,
|
||||
credentialId: string,
|
||||
): Promise<PasskeySelect | undefined> {
|
||||
const [passkey] = await db
|
||||
.select()
|
||||
.from(passkeys)
|
||||
@@ -75,7 +96,10 @@ export async function createPasskey(data: PasskeyInsert): Promise<PasskeySelect>
|
||||
return passkey!;
|
||||
}
|
||||
|
||||
export async function updatePasskey(id: number, data: Partial<Omit<PasskeySelect, 'id'>>): Promise<PasskeySelect | undefined> {
|
||||
export async function updatePasskey(
|
||||
id: number,
|
||||
data: Partial<Omit<PasskeySelect, 'id'>>,
|
||||
): Promise<PasskeySelect | undefined> {
|
||||
const [passkey] = await db.update(passkeys).set(data).where(eq(passkeys.id, id)).returning();
|
||||
return passkey;
|
||||
}
|
||||
@@ -86,7 +110,9 @@ export async function storeChallenge(userId: number, origin: string, challenge:
|
||||
const expiresAt = new Date(Date.now() + ttlMs);
|
||||
|
||||
// Upsert: delete existing challenge for this user+origin, then insert
|
||||
await db.delete(passkeyChallenges).where(and(eq(passkeyChallenges.userId, userId), eq(passkeyChallenges.origin, origin)));
|
||||
await db
|
||||
.delete(passkeyChallenges)
|
||||
.where(and(eq(passkeyChallenges.userId, userId), eq(passkeyChallenges.origin, origin)));
|
||||
await db.insert(passkeyChallenges).values({ userId, origin, challenge, expiresAt });
|
||||
}
|
||||
|
||||
|
||||
@@ -1,4 +1,10 @@
|
||||
import { pgTable, serial, text, integer, timestamp, index } from 'drizzle-orm/pg-core';
|
||||
import { pgTable, serial, text, integer, timestamp, index, check } from 'drizzle-orm/pg-core';
|
||||
import { sql } from 'drizzle-orm';
|
||||
|
||||
// The bootstrap account. `id` is a serial starting at 1 and bootstrap is gated on an empty user table,
|
||||
// so the first account created is always 1. Exported so the CHECK below and the delete guard in
|
||||
// queries/auth.ts state the number once between them.
|
||||
export const OWNER_USER_ID = 1;
|
||||
|
||||
// Roles, in descending authority. Exported as a value so the API and the UI can enumerate them from
|
||||
// the same place the column is defined — a second hand-written list of role names is how they drift.
|
||||
@@ -10,28 +16,49 @@ import { pgTable, serial, text, integer, timestamp, index } from 'drizzle-orm/pg
|
||||
// 'Super Admin' is the server owner, and this column is the ONLY thing that says so. isSuperAdmin()
|
||||
// and getOwnerUser() both read it; there is no SUPER_ADMIN_EMAIL and no "first account wins" fallback
|
||||
// any more. Two consequences worth holding on to: bootstrap must set it explicitly on the first
|
||||
// account, and demoting the last Super Admin leaves the platform with no owner — nothing in the schema
|
||||
// prevents that, so whatever eventually edits roles has to.
|
||||
// account, and demoting the last Super Admin would leave the platform with no owner. The CHECK on the
|
||||
// table below stops that for the bootstrap account specifically: user 1 cannot be anything else.
|
||||
export const USER_ROLES = ['Super Admin', 'Admin', 'Member', 'Developer'] as const;
|
||||
export type UserRole = (typeof USER_ROLES)[number];
|
||||
|
||||
export const users = pgTable('users', {
|
||||
id: serial('id').primaryKey(),
|
||||
email: text('email').notNull().unique(),
|
||||
password: text('password'),
|
||||
status: text('status', { enum: ['Unverified', 'Active', 'Prospect', 'Invited', 'Blocked', 'Banned', 'Deleted'] })
|
||||
.notNull()
|
||||
.default('Unverified'),
|
||||
// Defaults to the least privileged role so a row created by any path that does not think about
|
||||
// authorisation cannot accidentally mint an admin.
|
||||
role: text('role', { enum: USER_ROLES }).notNull().default('Member'),
|
||||
name: text('name'),
|
||||
username: text('username').unique(),
|
||||
avatar: text('avatar'),
|
||||
passwordChangedAt: timestamp('password_changed_at', { withTimezone: true }),
|
||||
createdAt: timestamp('created_at', { withTimezone: true }).notNull().defaultNow(),
|
||||
updatedAt: timestamp('updated_at', { withTimezone: true }).notNull().defaultNow(),
|
||||
});
|
||||
export const users = pgTable(
|
||||
'users',
|
||||
{
|
||||
id: serial('id').primaryKey(),
|
||||
email: text('email').notNull().unique(),
|
||||
password: text('password'),
|
||||
status: text('status', { enum: ['Unverified', 'Active', 'Prospect', 'Invited', 'Blocked', 'Banned', 'Deleted'] })
|
||||
.notNull()
|
||||
.default('Unverified'),
|
||||
// Defaults to the least privileged role so a row created by any path that does not think about
|
||||
// authorisation cannot accidentally mint an admin.
|
||||
role: text('role', { enum: USER_ROLES }).notNull().default('Member'),
|
||||
name: text('name'),
|
||||
username: text('username').unique(),
|
||||
avatar: text('avatar'),
|
||||
passwordChangedAt: timestamp('password_changed_at', { withTimezone: true }),
|
||||
createdAt: timestamp('created_at', { withTimezone: true }).notNull().defaultNow(),
|
||||
updatedAt: timestamp('updated_at', { withTimezone: true }).notNull().defaultNow(),
|
||||
},
|
||||
(table) => [
|
||||
// The owner cannot be demoted. Enforced here rather than in application code because the whole
|
||||
// point is that it holds "whatever happens" — a stray UPDATE, a migration script, someone at a psql
|
||||
// prompt. Postgres rejects the write; there is no path around it.
|
||||
//
|
||||
// A row-level CHECK can say "if this row is user 1 then its role is Super Admin". It cannot say
|
||||
// "some row must be Super Admin" — that would need to see other rows. So this pins the bootstrap
|
||||
// account and nothing more; promoting or demoting anyone else is still free.
|
||||
//
|
||||
// It cannot stop a DELETE either. deleteUser() in queries/auth.ts refuses id 1 for that half.
|
||||
//
|
||||
// sql.raw for the literal — an interpolated JS string binds as `$1`, and Postgres will not accept a
|
||||
// parameter inside a CHECK. Same trap as ck_server_integrations_provider.
|
||||
check(
|
||||
'ck_users_owner_is_super_admin',
|
||||
sql`${table.id} <> ${sql.raw(String(OWNER_USER_ID))} OR ${table.role} = ${sql.raw("'Super Admin'")}`,
|
||||
),
|
||||
],
|
||||
);
|
||||
|
||||
export const passkeys = pgTable('passkeys', {
|
||||
id: serial('id').primaryKey(),
|
||||
|
||||
Reference in New Issue
Block a user