the owner can create accounts
POST /api/users plus an Add-account form in Settings > User management. Until now createUser had one call site — bootstrap, gated on an empty user table — so every non-owner account anywhere had been inserted into Postgres by hand. Created accounts are Active. The column defaults to Unverified and signin refuses anything else with a bare UNAUTHORIZED, which is exactly what made the hand-INSERT route look like a wrong password. Also closes a hole found while reading the write path: a second Super Admin was storable. The CHECK constraint pins user 1's role but cannot see other rows, and getOwnerUser() was LIMIT 1 with no ORDER BY, so two holders would have made "who owns this server" a question the query plan answered — and that answer feeds the agent sidecar's identity, vault access and origin scoping. Both write paths now refuse the role and getOwnerUser() orders by id. USER_DIRS and provisionUserDirs move into data-path.ts so the create handler and scripts/provision-user-dirs.ts cannot disagree about what an account's skeleton is. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,89 @@
|
||||
import type { Handler } from 'hono';
|
||||
import { createUser, getUserByEmail, getUserByUsername, USER_ROLES } from 'officerdb';
|
||||
import type { UserRole } from 'officerdb';
|
||||
import argon2 from 'argon2';
|
||||
import * as errors from '@@/custom-errors';
|
||||
import { provisionUserDirs } from '@@/data-path';
|
||||
import { validatePassword } from '../auth/validate-password';
|
||||
import { validateUsername } from '../auth/validate-username';
|
||||
import { toPublicUser } from './manage-users';
|
||||
|
||||
// The owner creating a second account. Until this existed, `createUser` had exactly one call site —
|
||||
// `auth/bootstrap.ts`, gated on an empty user table — so every non-owner account on any instance had
|
||||
// been inserted into Postgres by hand.
|
||||
//
|
||||
// ── Why the owner sets the password ──
|
||||
//
|
||||
// The alternative is an invite: a token emailed to the person, who then sets their own. That is the
|
||||
// better shape and it needs a mail path, a token table and an expiry policy. This is the honest
|
||||
// intermediate: the owner types a password and tells the person, the same way they would hand over a
|
||||
// wifi key. `passwordChangedAt` stays null, so nothing pretends the person chose it.
|
||||
//
|
||||
// ── Status is 'Active', deliberately ──
|
||||
//
|
||||
// The column defaults to 'Unverified' and `signin.ts` refuses anything that is not 'Active' with a bare
|
||||
// UNAUTHORIZED. So an account created at the default would be indistinguishable from a wrong password,
|
||||
// which is precisely the trap the hand-INSERT route fell into. An account the owner created in the admin
|
||||
// UI is verified by definition — the owner is the verification.
|
||||
|
||||
/** Roles this route may assign. Never 'Super Admin' — see below. */
|
||||
const ASSIGNABLE_ROLES = USER_ROLES.filter((r) => r !== 'Super Admin');
|
||||
|
||||
export const createUserHandler: Handler = async function (ctx) {
|
||||
const body = (ctx.get('body') ?? {}) as Record<string, unknown>;
|
||||
|
||||
const email = typeof body.email === 'string' ? body.email.trim().toLowerCase() : '';
|
||||
const name = typeof body.name === 'string' ? body.name.trim() : '';
|
||||
const password = typeof body.password === 'string' ? body.password : '';
|
||||
const role = typeof body.role === 'string' ? body.role : 'Member';
|
||||
|
||||
if (!email || !/^[^\s@]+@[^\s@]+\.[^\s@]+$/.test(email)) throw errors.BAD_REQUEST('Invalid email address');
|
||||
if (!name) throw errors.BAD_REQUEST('Name is required');
|
||||
|
||||
// The same validators bootstrap uses. A member's password protects the same surface a member can
|
||||
// reach, so there is no argument for a weaker rule here — and two different rules would mean the
|
||||
// owner could create an account that could not then change its own password to something similar.
|
||||
const username = validateUsername(typeof body.username === 'string' ? body.username : undefined);
|
||||
validatePassword(password);
|
||||
|
||||
// Refused rather than filtered, so the owner is told instead of quietly getting a Member.
|
||||
//
|
||||
// There is exactly one owner. The database only pins user 1's role — a row-level CHECK cannot say
|
||||
// "no OTHER row may hold this" — so a second Super Admin is storable, and `getOwnerUser()` would then
|
||||
// return whichever the query reached first. That answer decides the identity the agent sidecar runs
|
||||
// as, who reaches the vault and which origin is privileged, so it is not a thing to leave to a query
|
||||
// plan. If the owner ever needs to hand the server over, that is a deliberate transfer, not a dropdown.
|
||||
if (!(ASSIGNABLE_ROLES as readonly string[]).includes(role)) {
|
||||
throw errors.BAD_REQUEST(
|
||||
role === 'Super Admin'
|
||||
? 'There is one server owner and it cannot be created here.'
|
||||
: `Role must be one of: ${ASSIGNABLE_ROLES.join(', ')}`,
|
||||
);
|
||||
}
|
||||
|
||||
// Checked before the insert purely for the message — both columns are unique, so the database is the
|
||||
// real guard and this is a race it can lose harmlessly (the insert then throws).
|
||||
if (await getUserByEmail(email)) throw errors.CONFLICT('An account with that email already exists');
|
||||
if (await getUserByUsername(username)) throw errors.CONFLICT('That username is taken');
|
||||
|
||||
const user = await createUser({
|
||||
email,
|
||||
password: await argon2.hash(password),
|
||||
name,
|
||||
username,
|
||||
status: 'Active',
|
||||
role: role as UserRole,
|
||||
});
|
||||
|
||||
// After the row, and not fatal if it fails. A missing directory is repairable from a shell
|
||||
// (`bun scripts/provision-user-dirs.ts <email>`); an account that half-exists because mkdir failed
|
||||
// is not, and the owner would have to go into Postgres to clean it up — the exact thing this route
|
||||
// is here to stop being necessary.
|
||||
try {
|
||||
provisionUserDirs(email);
|
||||
} catch (ex) {
|
||||
console.warn(`[users] created ${email} but could not provision its data directories`, ex);
|
||||
}
|
||||
|
||||
return ctx.json({ user: toPublicUser(user) }, 201);
|
||||
};
|
||||
@@ -21,7 +21,8 @@ type PublicUser = {
|
||||
isOwner: boolean;
|
||||
};
|
||||
|
||||
const toPublicUser = (u: Awaited<ReturnType<typeof getUsers>>[number]): PublicUser => ({
|
||||
/** Shared with create-user.ts, so a created account and a listed one are described the same way. */
|
||||
export const toPublicUser = (u: Awaited<ReturnType<typeof getUsers>>[number]): PublicUser => ({
|
||||
id: u.id,
|
||||
email: u.email,
|
||||
name: u.name,
|
||||
@@ -37,7 +38,10 @@ export const listUsersHandler: Handler = async function (ctx) {
|
||||
const users = await getUsers();
|
||||
return ctx.json({
|
||||
users: users.sort((a, b) => a.id - b.id).map(toPublicUser),
|
||||
// Every role, so the owner's own row can display its value. The UI must not offer 'Super Admin' in a
|
||||
// picker — both write paths refuse it — which is what `assignableRoles` is for.
|
||||
roles: USER_ROLES,
|
||||
assignableRoles: USER_ROLES.filter((r) => r !== 'Super Admin'),
|
||||
ownerId: OWNER_USER_ID,
|
||||
});
|
||||
};
|
||||
@@ -58,6 +62,15 @@ export const updateUserRoleHandler: Handler = async function (ctx) {
|
||||
throw errors.FORBIDDEN('The server owner cannot be demoted.');
|
||||
}
|
||||
|
||||
// And nobody else can be promoted INTO it. The CHECK constraint pins user 1's role but cannot stop a
|
||||
// second row holding it — a row-level check cannot see other rows — and `getOwnerUser()` resolves the
|
||||
// owner by that role, so two holders make "who owns this server" a question the query plan answers.
|
||||
// It decides the agent sidecar's identity, vault access and which origin is privileged. Handing the
|
||||
// server over is a deliberate act, not a dropdown.
|
||||
if (id !== OWNER_USER_ID && role === 'Super Admin') {
|
||||
throw errors.FORBIDDEN('There is one server owner, and this is not how it changes.');
|
||||
}
|
||||
|
||||
const existing = await getUserById(id);
|
||||
if (!existing) throw errors.NOT_FOUND('User not found');
|
||||
|
||||
|
||||
@@ -5,6 +5,7 @@ import { isSuperAdmin } from '@@/super-admin';
|
||||
import * as errors from '@@/custom-errors';
|
||||
import { updateUserHandler } from './update-user';
|
||||
import { listUsersHandler, updateUserRoleHandler, deleteUserHandler } from './manage-users';
|
||||
import { createUserHandler } from './create-user';
|
||||
import { capabilityAdminRouter } from './capabilities-routes';
|
||||
|
||||
export const usersRouter = createRouter();
|
||||
@@ -24,6 +25,9 @@ const ownerGate: MiddlewareHandler = async (ctx, next) => {
|
||||
};
|
||||
|
||||
usersRouter.get('/', ownerGate, listUsersHandler);
|
||||
// POST, not PUT — and worth noting they sit one line apart. `PUT /` is the selfService exception every
|
||||
// account may call on itself; `POST /` creates somebody else and is the owner's alone.
|
||||
usersRouter.post('/', ownerGate, createUserHandler);
|
||||
usersRouter.patch('/:id/role', ownerGate, updateUserRoleHandler);
|
||||
usersRouter.delete('/:id', ownerGate, deleteUserHandler);
|
||||
|
||||
|
||||
Reference in New Issue
Block a user