drop the postgres role when a member is decommissioned
It was written and never called — dropPostgresRole had zero call sites, so deleting a
member left their role and databases on the cluster.
That is the uid trap in a different id space, and worse. provisionPostgresRole ADOPTS
an existing role, so a role left behind is inherited whole, with its databases, by the
next member who gets the same username. useradd hands out the lowest free uid by
accident; the owner hands out usernames on purpose, so reuse is likelier here, not
less.
Rewritten to PRESERVE rather than destroy. The first version dropped the databases,
which is inconsistent with severMemberTree three files away — that chowns a member's
files to the service user rather than deleting them, and a database is the same kind
of thing. The owner removing an account has not necessarily asked to destroy the work
in it, and dropping is the one choice that cannot be walked back.
Needs the full idiom, per database, and both halves matter:
ALTER DATABASE .. OWNER TO REASSIGN OWNED does not move database ownership
REASSIGN OWNED BY .. TO .. moves tables, schemas, functions
DROP OWNED BY .. removes what is left, which after a reassign is
only the GRANTS — without it DROP ROLE still
refuses, an ACL entry is a dependency too
Both statements act only on the database they are connected to, so it is a connection
per database rather than a loop over `db`.
Ordered after the Linux teardown (which can fail and abort, and must not do so after
something irreversible) and before deleteUser (the row is what remembers there is
anything to clean up).
Verified live: a member with a database, a table, a row and a schema. Naive DROP ROLE
refused with "2 objects in database carol_app". After the sequence: role gone, row
intact, table owned by postgres. Then recreated the same username and confirmed she is
REFUSED from the old database — the login trigger holds because ownership moved.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -3,6 +3,7 @@ import { getUsers, getUserById, updateUser, deleteUser, USER_ROLES, OWNER_USER_I
|
|||||||
import type { UserRole } from 'officerdb';
|
import type { UserRole } from 'officerdb';
|
||||||
import * as errors from '@@/custom-errors';
|
import * as errors from '@@/custom-errors';
|
||||||
import { deprovisionOsAccount } from '@@/os-user-deprovision';
|
import { deprovisionOsAccount } from '@@/os-user-deprovision';
|
||||||
|
import { dropPostgresRole } from '@@/os-user-postgres';
|
||||||
import { DATA_PATH } from '@@/data-path';
|
import { DATA_PATH } from '@@/data-path';
|
||||||
|
|
||||||
// Owner-only management of the other accounts. Everything here is gated by ownerGate in
|
// Owner-only management of the other accounts. Everything here is gated by ownerGate in
|
||||||
@@ -153,6 +154,38 @@ export const deleteUserHandler: Handler = async function (ctx) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// ── The Postgres side, after the Linux side and before the row ──
|
||||||
|
//
|
||||||
|
// The same trap as the uid, in a different id space: `provisionPostgresRole` ADOPTS a role that already
|
||||||
|
// exists, so a role left behind here is inherited whole — with its databases — by the next member who
|
||||||
|
// gets the same username. The owner hands out usernames, so reuse is likelier than a reissued uid, not
|
||||||
|
// less.
|
||||||
|
//
|
||||||
|
// Ordered after the Linux teardown deliberately. That step can fail and abort, and it must not abort
|
||||||
|
// AFTER something irreversible has happened to their databases. Ordered before `deleteUser` for the
|
||||||
|
// reason stated above: the row is what remembers there is anything left to clean up.
|
||||||
|
//
|
||||||
|
// Their data is kept — the databases are reassigned to the platform role, not dropped, the same way
|
||||||
|
// `severMemberTree` chowns their files rather than deleting them.
|
||||||
|
if (existing.osUser) {
|
||||||
|
const pg = await dropPostgresRole(existing.osUser);
|
||||||
|
if (!pg.ok) {
|
||||||
|
console.error(`[users] POSTGRES DEPROVISION FAILED for ${existing.email}: ${pg.error}`);
|
||||||
|
throw errors.INTERNAL_SERVER_ERROR(
|
||||||
|
`Could not remove ${existing.email}'s Postgres role: ${pg.error} ` +
|
||||||
|
`The platform account was NOT deleted, so this can be retried.`,
|
||||||
|
);
|
||||||
|
}
|
||||||
|
if (pg.removed) {
|
||||||
|
console.info(
|
||||||
|
`[users] dropped Postgres role ${existing.osUser}` +
|
||||||
|
(pg.reassigned.length
|
||||||
|
? ` — ${pg.reassigned.length} database(s) kept and reassigned: ${pg.reassigned.join(', ')}`
|
||||||
|
: ' — it owned no databases'),
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// Deleting a user cascades: passkeys, dashboards, screens, email accounts, playlists, everything keyed
|
// Deleting a user cascades: passkeys, dashboards, screens, email accounts, playlists, everything keyed
|
||||||
// to them. There is no undo, which is why the UI asks first.
|
// to them. There is no undo, which is why the UI asks first.
|
||||||
await deleteUser(id);
|
await deleteUser(id);
|
||||||
|
|||||||
@@ -377,22 +377,105 @@ export async function provisionPostgresRole(params: {
|
|||||||
* Deliberately NOT wired into `deprovisionOsAccount` yet: that function's contract is that a failure
|
* Deliberately NOT wired into `deprovisionOsAccount` yet: that function's contract is that a failure
|
||||||
* leaves the account intact and retryable, and dropping databases is not reversible. See the call site.
|
* leaves the account intact and retryable, and dropping databases is not reversible. See the call site.
|
||||||
*/
|
*/
|
||||||
export type DropRoleResult = { ok: true; dropped: string[] } | { ok: false; error: string };
|
export type DropRoleResult = { ok: true; removed: boolean; reassigned: string[] } | { ok: false; error: string };
|
||||||
|
|
||||||
|
/** Legal database name. Separate from ROLE_NAME_RE because a member names these, not us. */
|
||||||
|
const DB_NAME_RE = /^[a-zA-Z0-9._-]{1,63}$/;
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Take a member's Postgres role away, keeping their data.
|
||||||
|
*
|
||||||
|
* ── Why this exists, and why it is not optional ──
|
||||||
|
*
|
||||||
|
* Exactly the trap `deprovisionOsAccount` was written for, in a different id space. `provisionPostgresRole`
|
||||||
|
* ADOPTS a role that already exists — so a role left behind by a deleted member is inherited whole, with its
|
||||||
|
* databases, by the next member who happens to get the same username. `useradd` hands out the lowest free
|
||||||
|
* uid; the owner hands out usernames, and reusing one is likelier, not less.
|
||||||
|
*
|
||||||
|
* ── Preserve, not destroy ──
|
||||||
|
*
|
||||||
|
* Their databases are REASSIGNED to the platform role, not dropped. `severMemberTree` chowns a member's
|
||||||
|
* files to the service user rather than deleting them, and a database is the same kind of thing: the owner
|
||||||
|
* removing an account has not necessarily asked to destroy the work in it. Dropping is also the one choice
|
||||||
|
* that cannot be walked back.
|
||||||
|
*
|
||||||
|
* The sequence per database is the documented idiom and both halves are needed:
|
||||||
|
*
|
||||||
|
* REASSIGN OWNED BY <member> TO <platform> moves the objects — tables, schemas, functions
|
||||||
|
* DROP OWNED BY <member> removes what is left, which after a reassign is only the
|
||||||
|
* privilege GRANTS. Without it `DROP ROLE` still refuses,
|
||||||
|
* because an ACL entry naming the role is a dependency too.
|
||||||
|
*
|
||||||
|
* It has to run INSIDE each database, because both statements act only on the one they are connected to —
|
||||||
|
* hence a connection per database rather than a loop over `db`.
|
||||||
|
*/
|
||||||
export async function dropPostgresRole(osUser: string): Promise<DropRoleResult> {
|
export async function dropPostgresRole(osUser: string): Promise<DropRoleResult> {
|
||||||
if (!ROLE_NAME_RE.test(osUser)) return { ok: false, error: `'${osUser}' is not a usable Postgres role name` };
|
if (!ROLE_NAME_RE.test(osUser)) return { ok: false, error: `'${osUser}' is not a usable Postgres role name` };
|
||||||
|
|
||||||
|
const url = process.env.POSTGRES_URL;
|
||||||
|
if (!url) return { ok: false, error: 'POSTGRES_URL is not set' };
|
||||||
|
|
||||||
try {
|
try {
|
||||||
|
const present = await db.execute<{ rolname: string }>(
|
||||||
|
sql`select rolname from pg_roles where rolname = ${osUser}`,
|
||||||
|
);
|
||||||
|
// Already gone is success, so a retry after a partial teardown finishes rather than refuses.
|
||||||
|
if (present.length === 0) return { ok: true, removed: false, reassigned: [] };
|
||||||
|
|
||||||
|
const [platform] = await db.execute<{ current_user: string }>(sql`select current_user`);
|
||||||
|
const owner = platform?.current_user;
|
||||||
|
if (!owner || !ROLE_NAME_RE.test(owner)) return { ok: false, error: 'could not determine the platform role' };
|
||||||
|
|
||||||
const owned = await db.execute<{ datname: string }>(
|
const owned = await db.execute<{ datname: string }>(
|
||||||
sql`select datname from pg_database where pg_get_userbyid(datdba) = ${osUser}`,
|
sql`select datname from pg_database where pg_get_userbyid(datdba) = ${osUser}`,
|
||||||
);
|
);
|
||||||
|
|
||||||
|
const reassigned: string[] = [];
|
||||||
for (const row of owned) {
|
for (const row of owned) {
|
||||||
if (!ROLE_NAME_RE.test(row.datname) && !/^[a-zA-Z0-9._-]+$/.test(row.datname)) continue;
|
if (!DB_NAME_RE.test(row.datname)) {
|
||||||
await db.execute(sql.raw(`DROP DATABASE IF EXISTS "${row.datname}" WITH (FORCE)`));
|
return { ok: false, error: `refusing to touch database '${row.datname}': unexpected name` };
|
||||||
|
}
|
||||||
|
// The database itself first: REASSIGN OWNED does not move database ownership, only objects inside one.
|
||||||
|
await db.execute(sql.raw(`ALTER DATABASE "${row.datname}" OWNER TO "${owner}"`));
|
||||||
|
const moved = await reassignInsideDatabase({ url, database: row.datname, from: osUser, to: owner });
|
||||||
|
if (!moved.ok) return moved;
|
||||||
|
reassigned.push(row.datname);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// And in the platform's own database, where they own nothing but may still hold a grant.
|
||||||
|
await db.execute(sql.raw(`REASSIGN OWNED BY "${osUser}" TO "${owner}"`));
|
||||||
|
await db.execute(sql.raw(`DROP OWNED BY "${osUser}"`));
|
||||||
|
|
||||||
await db.execute(sql.raw(`DROP ROLE IF EXISTS "${osUser}"`));
|
await db.execute(sql.raw(`DROP ROLE IF EXISTS "${osUser}"`));
|
||||||
return { ok: true, dropped: owned.map((row) => row.datname) };
|
return { ok: true, removed: true, reassigned };
|
||||||
} catch (ex) {
|
} catch (ex) {
|
||||||
return { ok: false, error: `could not drop the Postgres role ${osUser}: ${asMessage(ex)}` };
|
return { ok: false, error: `could not remove the Postgres role ${osUser}: ${asMessage(ex)}` };
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
async function reassignInsideDatabase(params: {
|
||||||
|
url: string;
|
||||||
|
database: string;
|
||||||
|
from: string;
|
||||||
|
to: string;
|
||||||
|
}): Promise<{ ok: true } | { ok: false; error: string }> {
|
||||||
|
let target: string;
|
||||||
|
try {
|
||||||
|
const parsed = new URL(params.url);
|
||||||
|
parsed.pathname = `/${params.database}`;
|
||||||
|
target = parsed.toString();
|
||||||
|
} catch {
|
||||||
|
return { ok: false, error: 'POSTGRES_URL could not be parsed' };
|
||||||
|
}
|
||||||
|
|
||||||
|
const client = postgres(target, { max: 1, idle_timeout: 5, connection: { application_name: 'officer-deprovision' } });
|
||||||
|
try {
|
||||||
|
await client.unsafe(`REASSIGN OWNED BY "${params.from}" TO "${params.to}"`);
|
||||||
|
await client.unsafe(`DROP OWNED BY "${params.from}"`);
|
||||||
|
return { ok: true };
|
||||||
|
} catch (ex) {
|
||||||
|
return { ok: false, error: `could not reassign objects in '${params.database}': ${asMessage(ex)}` };
|
||||||
|
} finally {
|
||||||
|
await client.end({ timeout: 5 });
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user