From 2c9d4e55aa96dca48f0dc41ee2c605b91961d6d1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Padez?= Date: Tue, 11 Aug 2026 17:58:46 +0000 Subject: [PATCH] retry a linux account in place instead of deleting the person MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit POST /users/:id/provision-linux, and a terminal button on each user row. One operation covering three needs that were all previously answered by "delete the account and make it again": backfill an account created before the feature existed, or while the host was not set up for it retry the first attempt failed for something since fixed — the traversable ancestor chmod being the one everybody hits once re-key replace authorized_keys with a new public key Deleting to redo a retryable side effect throws away the password, the dashboards and everything else keyed to the row. The provisioning block moves out of create-user into provisionOsAccount, shared by both entry points for the same reason app-store/members.ts is shaped that way: two moments, one piece of work. Found by testing the retry rather than the create: provisionUserDirs re-chmods every directory including home, and home belongs to the MEMBER after the first successful run — chmod requires ownership, so it threw EPERM and took every retry down before it started. Those chmods are now a default for directories being created, not an assertion about ones that already exist; os-user.ts sets the home's mode through sudo and is the authority for it. The route answers 200 with the error in the body, because the interesting cases are partial: "the account exists and is confined but the keys failed" is not nothing having happened, and the row shows both halves. Verified end to end: blocked ancestor reports the chmod and leaves osUser null, the retry after that chmod succeeds and records the row, and a re-key replaces authorized_keys without rotating the outbound key. Co-Authored-By: Claude Opus 5 --- .../Settings/UserManagement/UsersSection.tsx | 64 +++++++++++++++- src/servers/api/users/create-user.ts | 73 +++++------------- src/servers/api/users/manage-users.ts | 4 + .../api/users/provision-linux-route.ts | 54 +++++++++++++ src/servers/api/users/provision-os.ts | 76 +++++++++++++++++++ src/servers/api/users/users-router.ts | 2 + src/servers/data-path.ts | 13 +++- 7 files changed, 232 insertions(+), 54 deletions(-) create mode 100644 src/servers/api/users/provision-linux-route.ts create mode 100644 src/servers/api/users/provision-os.ts diff --git a/src/apps/officer-web/Screens/Dashboard/Settings/UserManagement/UsersSection.tsx b/src/apps/officer-web/Screens/Dashboard/Settings/UserManagement/UsersSection.tsx index 66d952c7..9bc9bcd9 100644 --- a/src/apps/officer-web/Screens/Dashboard/Settings/UserManagement/UsersSection.tsx +++ b/src/apps/officer-web/Screens/Dashboard/Settings/UserManagement/UsersSection.tsx @@ -1,7 +1,7 @@ import { useState } from 'react'; import { useQuery, useQueryClient } from '@tanstack/react-query'; import { toast } from 'sonner'; -import { Crown, Trash2, Loader2, KeyRound } from 'lucide-react'; +import { Crown, Trash2, Loader2, KeyRound, SquareTerminal as TerminalIcon } from 'lucide-react'; import { useClient } from 'hooks/useClient'; import { Button } from '@/components/ui/button'; import { Select, SelectContent, SelectItem, SelectTrigger, SelectValue } from '@/components/ui/select'; @@ -32,6 +32,8 @@ type ManagedUser = { type UsersResponse = { users: ManagedUser[]; + /** False on a host without per-user Linux accounts, where those controls would only ever refuse. */ + osUsersEnabled: boolean; /** Every role, for displaying the owner's own value. */ roles: string[]; /** Roles the server will accept in a write. Excludes the owner role — both write paths refuse it. */ @@ -66,6 +68,45 @@ export const UsersSection = () => { } }; + /** + * Create or repair the account's Linux side in place. + * + * Prompts for a key rather than putting a whole form on the row: replacing it is the rarer of the two + * reasons to press this, and an empty answer means "leave authorized_keys alone" rather than "remove it". + */ + const provisionLinux = async (user: ManagedUser) => { + const key = window.prompt( + `Linux account for ${user.email}.\n\n` + + `Paste an SSH public key to allow them to SSH in, or leave empty to keep the current one.`, + '', + ); + // Cancel is null; empty string is a deliberate "no change". + if (key === null) return; + + setPendingId(user.id); + try { + const result = await client.post<{ osUser: string | null; sshPublicKey: string | null; error: string | null }>( + `/users/${user.id}/provision-linux`, + { sshPublicKey: key.trim() }, + ); + await queryClient.invalidateQueries({ queryKey: USERS_KEY }); + // Partial success is the interesting case and must not read as a clean win: the account can exist and + // be confined while the keys failed. + if (result.error) { + toast.warning(result.osUser ? `${result.osUser} created, but not finished` : 'Could not finish', { + description: result.error, + duration: 30_000, + }); + } else { + toast.success(`${result.osUser} is ready`); + } + } catch (ex) { + toast.error(ex instanceof Error ? ex.message : 'Could not provision the Linux account'); + } finally { + setPendingId(null); + } + }; + const remove = async (user: ManagedUser) => { setPendingId(user.id); setConfirmDelete(null); @@ -139,6 +180,27 @@ export const UsersSection = () => { + {/* Create or repair the Linux side. Offered for anyone lacking an account (backfill) and for + anyone who has one (retry after fixing a host problem, or replace their key) — the + underlying operation is idempotent, so there is no state where pressing it is wrong. */} + {data.osUsersEnabled && !user.isOwner && ( + + )} + {/* The errand the create screen promised would still be here: this key has to end up on their Gitea account, and nothing else will remind anyone. */} {user.osSshPublicKey && ( diff --git a/src/servers/api/users/create-user.ts b/src/servers/api/users/create-user.ts index 27770bc0..d873b9fd 100644 --- a/src/servers/api/users/create-user.ts +++ b/src/servers/api/users/create-user.ts @@ -1,11 +1,11 @@ import type { Handler } from 'hono'; -import { createUser, updateUser, getUserByEmail, getUserByUsername, USER_ROLES } from 'officerdb'; +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 { OS_USERS_ENABLED, ensureOsUser } from '@@/os-user'; -import { provisionSshAccess, validatePublicKey } from '@@/os-user-ssh'; +import { OS_USERS_ENABLED } from '@@/os-user'; +import { validatePublicKey } from '@@/os-user-ssh'; +import { provisionOsAccount } from './provision-os'; import { validatePassword } from '../auth/validate-password'; import { validateUsername } from '../auth/validate-username'; import { toPublicUser } from './manage-users'; @@ -90,55 +90,24 @@ export const createUserHandler: Handler = async function (ctx) { 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 `); 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); - } - - // The Linux account, when the host is set up for it. Same posture as the directories and for the same - // reason: this is a side effect of creating a platform account, and a failed `useradd` must not undo an - // account that otherwise exists and can sign in. The row simply keeps `osUser: null`, which every - // consumer already has to handle — that is what an account made before this feature looks like. + // The Linux side: directories, the Linux user, the confinement, the keys. Non-fatal and reported rather + // than thrown — this is a side effect of creating a platform account, and a failed `useradd` must not undo + // an account that otherwise exists and can sign in. The row simply keeps `osUser: null`, which is what an + // account created before this feature looks like, and which every consumer already handles. // - // Reported back in the response rather than only logged, so the owner sees "created, but no OS account" - // at the moment they click rather than discovering it when a terminal opens in the wrong home. - let osUser: string | null = null; - let osUserError: string | null = null; - let sshPublicKey: string | null = null; - if (OS_USERS_ENABLED) { - const result = await ensureOsUser({ email, username }); - if (result.ok) { - osUser = result.osUser; + // Retryable in place afterwards via POST /users/:id/provision-linux, so a host that was not ready when the + // account was made does not cost anybody their password and dashboards. + const os = OS_USERS_ENABLED + ? await provisionOsAccount({ userId: user.id, email, username, inboundKey }) + : { osUser: null, sshPublicKey: null, error: null }; - // SSH after the account, because everything it writes lives inside a home that does not belong to - // us until `ensureOsUser` has chowned it. - const ssh = await provisionSshAccess({ - email, - osUser: result.osUser, - uid: result.uid, - gid: result.gid, - authorizedKey: inboundKey, - }); - if (ssh.ok) { - sshPublicKey = ssh.publicKey; - } else { - // The Linux account is real and usable either way — it just has no keys yet. Reported rather than - // thrown for the same reason as the rest of this block. - osUserError = ssh.error; - console.warn(`[users] created the Linux account for ${email} but SSH setup failed: ${ssh.error}`); - } + if (os.error) console.warn(`[users] created ${email} but its Linux side did not finish: ${os.error}`); - await updateUser(user.id, { osUser: result.osUser, osSshPublicKey: sshPublicKey }); - } else { - osUserError = result.error; - console.warn(`[users] created ${email} but could not create its Linux account: ${result.error}`); - } - } - - return ctx.json({ user: { ...toPublicUser(user), osUser, osSshPublicKey: sshPublicKey }, osUserError }, 201); + return ctx.json( + { + user: { ...toPublicUser(user), osUser: os.osUser, osSshPublicKey: os.sshPublicKey }, + osUserError: os.error, + }, + 201, + ); }; diff --git a/src/servers/api/users/manage-users.ts b/src/servers/api/users/manage-users.ts index b040231e..7d65b998 100644 --- a/src/servers/api/users/manage-users.ts +++ b/src/servers/api/users/manage-users.ts @@ -2,6 +2,7 @@ import type { Handler } from 'hono'; import { getUsers, getUserById, updateUser, deleteUser, USER_ROLES, OWNER_USER_ID } from 'officerdb'; import type { UserRole } from 'officerdb'; import * as errors from '@@/custom-errors'; +import { OS_USERS_ENABLED } from '@@/os-user'; // Owner-only management of the other accounts. Everything here is gated by ownerGate in // users-router.ts; these handlers assume the caller is the Super Admin. @@ -53,6 +54,9 @@ export const listUsersHandler: Handler = async function (ctx) { roles: USER_ROLES, assignableRoles: USER_ROLES.filter((r) => r !== 'Super Admin'), ownerId: OWNER_USER_ID, + // So the UI offers the Linux-account controls only where they can work. On a host without the feature + // they would be a button that always reports the same refusal. + osUsersEnabled: OS_USERS_ENABLED, }); }; diff --git a/src/servers/api/users/provision-linux-route.ts b/src/servers/api/users/provision-linux-route.ts new file mode 100644 index 00000000..b4b8f1d2 --- /dev/null +++ b/src/servers/api/users/provision-linux-route.ts @@ -0,0 +1,54 @@ +import type { Handler } from 'hono'; +import { getUserById, OWNER_USER_ID } from 'officerdb'; +import * as errors from '@@/custom-errors'; +import { validatePublicKey } from '@@/os-user-ssh'; +import { provisionOsAccount } from './provision-os'; + +// POST /api/users/:id/provision-linux — give an existing account its Linux side, or repair it. +// +// One route for what are the same operation from the owner's point of view: +// +// backfill an account created before per-user Linux accounts existed, or while the host was not set up +// for them, gets one now. +// retry the first attempt failed for a reason the owner has since fixed — the traversable-ancestor +// chmod being the one everybody hits once. +// re-key a new inbound public key replaces the old `authorized_keys`. +// +// Before this, the answer to all three was "delete the account and create it again", which throws away the +// password, the dashboards and everything else keyed to the row to redo a retryable side effect. + +export const provisionLinuxHandler: Handler = async function (ctx) { + const id = Number(ctx.req.param('id')); + if (!Number.isInteger(id) || id < 1) throw errors.BAD_REQUEST('Invalid user id'); + + // The owner runs as the service user itself and its home is HOME_DIR — there is nothing to provision, and + // creating a second Linux account for it would be actively confusing. + if (id === OWNER_USER_ID) throw errors.BAD_REQUEST('The server owner already runs as the service account.'); + + const user = await getUserById(id); + if (!user) throw errors.NOT_FOUND('User not found'); + if (!user.username) throw errors.BAD_REQUEST('This account has no username to name a Linux user after.'); + + // Optional. Absent means "leave authorized_keys as it is" rather than "remove inbound access": clearing a + // key should be a deliberate act, not the consequence of submitting a form with an empty field. + const body = (ctx.get('body') ?? {}) as Record; + const raw = typeof body.sshPublicKey === 'string' ? body.sshPublicKey.trim() : ''; + let inboundKey: string | null = null; + if (raw) { + const checked = validatePublicKey(raw); + if (!checked.ok) throw errors.BAD_REQUEST(`SSH public key: ${checked.error}`); + inboundKey = checked.key; + } + + const result = await provisionOsAccount({ + userId: user.id, + email: user.email, + username: user.username, + inboundKey, + }); + + // 200 with the error in the body rather than a 4xx: the interesting cases are partial. "The Linux account + // exists and is confined but the keys failed" is not an error the caller should treat as nothing having + // happened, and the UI has to be able to show both halves. + return ctx.json(result); +}; diff --git a/src/servers/api/users/provision-os.ts b/src/servers/api/users/provision-os.ts new file mode 100644 index 00000000..b1126ccc --- /dev/null +++ b/src/servers/api/users/provision-os.ts @@ -0,0 +1,76 @@ +import { updateUser } from 'officerdb'; +import { OS_USERS_ENABLED, ensureOsUser } from '@@/os-user'; +import { provisionSshAccess } from '@@/os-user-ssh'; +import { provisionUserDirs } from '@@/data-path'; + +// Giving an account its Linux side: the directory skeleton, the Linux user, the confinement, the keys. +// +// Shared by account creation and the retry route, because they are the same work and there are two moments +// it has to happen at — the same reasoning as app-store/members.ts. The list of reasons a retry is needed is +// not exotic: +// +// - the host was not set up for it when the account was made (`OFFICER_OS_USERS` off, no sudoers entry) +// - an ancestor directory was not traversable, which is the one everybody hits once +// - the owner wants to replace the inbound SSH key +// +// Before this existed the answer to all three was "delete the account and make it again", which loses the +// password, the dashboards and anything else keyed to the row — for what is really a retryable side effect. + +export type OsProvisionOutcome = { + osUser: string | null; + sshPublicKey: string | null; + /** Null on success. Everything here is non-fatal — the platform account works regardless. */ + error: string | null; +}; + +/** + * Idempotent. Every step underneath adopts what already exists: `useradd` is skipped for an account whose + * home already matches, the confinement re-applies mode bits, and an existing `id_ed25519` is kept rather + * than rotated (it has been added to Gitea by then). + * + * Never throws. A failure here must not undo or block a platform account that otherwise works, so the error + * is returned and the row keeps `osUser: null` — which is exactly what an account created before this + * feature looks like, and which every consumer already handles. + */ +export async function provisionOsAccount(params: { + userId: number; + email: string; + username: string; + /** Inbound SSH key for `authorized_keys`. Already validated by the caller. */ + inboundKey?: string | null; +}): Promise { + if (!OS_USERS_ENABLED) { + return { osUser: null, sshPublicKey: null, error: 'per-user Linux accounts are not enabled on this server' }; + } + + // First, because a missing skeleton is the reason `useradd --home-dir … -M` would have nothing to point at. + try { + provisionUserDirs(params.email); + } catch (ex) { + return { + osUser: null, + sshPublicKey: null, + error: `could not provision data directories: ${ex instanceof Error ? ex.message : String(ex)}`, + }; + } + + const account = await ensureOsUser({ email: params.email, username: params.username }); + if (!account.ok) return { osUser: null, sshPublicKey: null, error: account.error }; + + // SSH after the account, because everything it writes lives inside a home that is not ours until + // `ensureOsUser` has chowned it away. + const ssh = await provisionSshAccess({ + email: params.email, + osUser: account.osUser, + uid: account.uid, + gid: account.gid, + authorizedKey: params.inboundKey, + }); + + // The Linux account is recorded either way: it exists, it is confined, and a member's terminal can run as + // it. Only the keys are missing, and that is what the error says. + const sshPublicKey = ssh.ok ? ssh.publicKey : null; + await updateUser(params.userId, { osUser: account.osUser, osSshPublicKey: sshPublicKey }); + + return { osUser: account.osUser, sshPublicKey, error: ssh.ok ? null : ssh.error }; +} diff --git a/src/servers/api/users/users-router.ts b/src/servers/api/users/users-router.ts index 326ef152..e52e1684 100644 --- a/src/servers/api/users/users-router.ts +++ b/src/servers/api/users/users-router.ts @@ -6,6 +6,7 @@ import * as errors from '@@/custom-errors'; import { updateUserHandler } from './update-user'; import { listUsersHandler, updateUserRoleHandler, deleteUserHandler } from './manage-users'; import { createUserHandler } from './create-user'; +import { provisionLinuxHandler } from './provision-linux-route'; import { capabilityAdminRouter } from './capabilities-routes'; export const usersRouter = createRouter(); @@ -29,6 +30,7 @@ usersRouter.get('/', ownerGate, listUsersHandler); // 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.post('/:id/provision-linux', ownerGate, provisionLinuxHandler); usersRouter.delete('/:id', ownerGate, deleteUserHandler); // Which capabilities each role holds. Owner-gated inside its own router. diff --git a/src/servers/data-path.ts b/src/servers/data-path.ts index c364a3b6..27b7461d 100644 --- a/src/servers/data-path.ts +++ b/src/servers/data-path.ts @@ -96,7 +96,18 @@ export const provisionUserDirs = (email: string): void => { for (const dir of USER_DIRS) mkdirSync(join(accountDir, dir), { recursive: true }); // Traversable, not listable: reaching `home` must not mean enumerating the platform's tree beside it. chmodSync(accountDir, 0o711); - for (const dir of USER_DIRS) chmodSync(join(accountDir, dir), 0o700); + + for (const dir of USER_DIRS) { + try { + chmodSync(join(accountDir, dir), 0o700); + } catch { + // A directory that is no longer OURS to chmod. `home` becomes the member's on the first successful + // provision, and `chmod` requires ownership — so re-running this threw EPERM and took every RETRY down + // before it began, which is how this was found. os-user.ts sets the home's mode through sudo and is the + // authority for it; here the mode is a default for directories we are creating, not an assertion about + // ones that already exist. + } + } }; export const getTmpAttachmentsDir = (email: string) => join(DATA_PATH, email, 'attachments', 'tmp');