feat(admin): repor palavra-passe e remover segundo factor no modal
Duas operacoes que so existiam pela linha de comandos, e que sao precisamente as que alguem pede ao telefone: esqueci-me da palavra-passe, perdi o autenticador. A reposicao termina todas as sessoes daquele utilizador, incluindo a que esta aberta nesse momento. Um administrador a repor uma palavra-passe esta a fazer uma passagem de conta ou a responder a um comprometimento, e nos dois casos as sessoes abertas com a palavra-passe anterior nao podem sobreviver. Exige os mesmos 12 caracteres que os outros caminhos, para a consola nao ser a maneira de contornar a regra. Remover o segundo factor apaga tambem os codigos de recuperacao: foram gerados contra o segredo antigo, nao servem contra um novo, e deixa-los para tras e um segundo conjunto de credenciais que ninguem esta a seguir. A rota do /totp teve de ficar acima do DELETE generico de /mailboxes/. O generico apanha qualquer caminho por baixo desse prefixo, por isso carregar em "remover segundo factor" era encaminhado para apagar a caixa do utilizador -- so nao a apagou porque o id saia malformado e o Postgres recusou o uuid. Ha um teste so para essa ordem. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
1 parent
bb12e603a3
commit
1e7c6a118c
7 files changed
+309
-3
No files matched your search
@@ -657,6 +657,65 @@ export async function changePassword(
|
||||
return "OK";
|
||||
}
|
||||
|
||||
/**
|
||||
* Sets a user's password from the console, without knowing the old one.
|
||||
*
|
||||
* Every other session that user holds is signed out. A password reset an
|
||||
* administrator performs is either a handover or a response to a compromise,
|
||||
* and in both cases the sessions opened with the previous password must not
|
||||
* survive it.
|
||||
*/
|
||||
export async function adminSetPassword(
|
||||
scope: AdminScope,
|
||||
mailboxId: string,
|
||||
newPassword: string,
|
||||
): Promise<boolean> {
|
||||
const args: unknown[] = [mailboxId, await hashPassword(newPassword)];
|
||||
let where = "id = $1";
|
||||
if (scope.role !== "SUPER_ADMIN") {
|
||||
args.push(scope.domainId);
|
||||
where += ` AND domain_id = $${args.length}`;
|
||||
}
|
||||
|
||||
const rows = await query<{ id: string }>(
|
||||
`UPDATE mailboxes
|
||||
SET password_hash = $2, password_updated_at = NOW(), updated_at = NOW()
|
||||
WHERE ${where} RETURNING id`,
|
||||
args,
|
||||
);
|
||||
if (rows.length !== 1) return false;
|
||||
|
||||
// Every session, including the one the user is holding right now: sessions
|
||||
// are revoked rather than deleted, which is how the rest of this service
|
||||
// records them.
|
||||
await query(
|
||||
`UPDATE web_sessions SET revoked_at = NOW() WHERE mailbox_id = $1 AND revoked_at IS NULL`,
|
||||
[mailboxId],
|
||||
);
|
||||
return true;
|
||||
}
|
||||
|
||||
/**
|
||||
* Removes a user's second factor.
|
||||
*
|
||||
* This is the way back in for somebody who lost their authenticator, so it
|
||||
* clears the enrolment and the recovery codes together: codes minted for the
|
||||
* old secret are no use against a new one, and leaving them would be a second
|
||||
* set of credentials nobody is tracking.
|
||||
*/
|
||||
export async function adminRemoveTotp(scope: AdminScope, mailboxId: string): Promise<boolean> {
|
||||
const { sql, params } = scopeClause(scope, "domain_id");
|
||||
const owner = await queryOne<{ id: string }>(
|
||||
`SELECT id FROM mailboxes WHERE id = $${params.length + 1} AND ${sql}`,
|
||||
[...params, mailboxId],
|
||||
);
|
||||
if (!owner) return false;
|
||||
|
||||
await query(`DELETE FROM mfa_totp WHERE mailbox_id = $1`, [mailboxId]);
|
||||
await query(`DELETE FROM mfa_recovery_codes WHERE mailbox_id = $1`, [mailboxId]).catch(() => undefined);
|
||||
return true;
|
||||
}
|
||||
|
||||
// ------------------------------------------------------------- audit log
|
||||
|
||||
/** Records an administrative action. PLAN.md section 14.3 requires every
|
||||
|
||||
@@ -846,6 +846,87 @@ describeDb("auth API against a real database", () => {
|
||||
expect(sources).toContain(mine);
|
||||
});
|
||||
|
||||
it("resets a password from the console and signs the sessions out", async () => {
|
||||
// A session opened with the old password, to prove it does not survive.
|
||||
const before = await post("/api/v1/auth/user/login", { email: userEmail(), password: PASSWORD });
|
||||
expect(before.status).toBe(200);
|
||||
const oldToken = before.body.token as unknown as string;
|
||||
|
||||
const NEXT = "AnotherCorrectHorse9!";
|
||||
const reset = await post(`/api/v1/admin/mailboxes/${USER_ID}/password`, { password: NEXT }, admin);
|
||||
expect(reset.status).toBe(200);
|
||||
|
||||
// The new password works.
|
||||
const after = await post("/api/v1/auth/user/login", { email: userEmail(), password: NEXT });
|
||||
expect(after.status).toBe(200);
|
||||
|
||||
// The old one does not.
|
||||
const stale = await post("/api/v1/auth/user/login", { email: userEmail(), password: PASSWORD });
|
||||
expect(stale.status).toBe(401);
|
||||
|
||||
// And the session minted before the reset is no longer accepted: an
|
||||
// administrator resetting a password is either a handover or a response
|
||||
// to a compromise, and both mean the old sessions must stop working.
|
||||
const reuse = await get("/api/v1/user/preferences", oldToken);
|
||||
expect(reuse.status).toBe(401);
|
||||
|
||||
// Put it back for whatever runs next.
|
||||
await post(`/api/v1/admin/mailboxes/${USER_ID}/password`, { password: PASSWORD }, admin);
|
||||
}, 60000);
|
||||
|
||||
it("refuses a short password on reset, like every other path does", async () => {
|
||||
const res = await post(`/api/v1/admin/mailboxes/${USER_ID}/password`, { password: "short" }, admin);
|
||||
expect(res.status).toBe(400);
|
||||
});
|
||||
|
||||
it("removes a second factor, with its recovery codes", async () => {
|
||||
// The enrolment state is built directly: this test is about the removal,
|
||||
// and USER_ID already carries a factor from the tests above, so going
|
||||
// through the enrolment flow again would only be testing that flow.
|
||||
await query(`DELETE FROM mfa_totp WHERE mailbox_id = $1`, [USER_ID]);
|
||||
await query(
|
||||
`INSERT INTO mfa_totp (mailbox_id, label, secret_enc, secret_nonce, key_version, status, confirmed_at)
|
||||
VALUES ($1, 'test', '\\x00'::bytea, '\\x00'::bytea, 1, 'CONFIRMED', NOW())`,
|
||||
[USER_ID],
|
||||
);
|
||||
await query(
|
||||
`INSERT INTO mfa_recovery_codes (mailbox_id, code_hash) VALUES ($1, 'not-a-real-hash')`,
|
||||
[USER_ID],
|
||||
);
|
||||
|
||||
const res = await fetch(`${base}/api/v1/admin/mailboxes/${USER_ID}/totp`, {
|
||||
method: "DELETE",
|
||||
headers: { Authorization: `Bearer ${admin}` },
|
||||
});
|
||||
expect(res.status).toBe(200);
|
||||
|
||||
expect(await query(`SELECT 1 FROM mfa_totp WHERE mailbox_id = $1`, [USER_ID])).toHaveLength(0);
|
||||
// Codes minted against the old secret are no use against a new one, and
|
||||
// leaving them behind is a second set of credentials nobody tracks.
|
||||
expect(await query(`SELECT 1 FROM mfa_recovery_codes WHERE mailbox_id = $1`, [USER_ID])).toHaveLength(0);
|
||||
});
|
||||
|
||||
// The catch-all DELETE under /mailboxes/ matches this path too. With the
|
||||
// routes in the wrong order, pressing "remove second factor" was routed to
|
||||
// deleting the mailbox - it only failed because the id came out malformed.
|
||||
it("removing a second factor does not delete the user", async () => {
|
||||
await query(`DELETE FROM mfa_totp WHERE mailbox_id = $1`, [USER_ID]);
|
||||
await query(
|
||||
`INSERT INTO mfa_totp (mailbox_id, label, secret_enc, secret_nonce, key_version, status, confirmed_at)
|
||||
VALUES ($1, 'test', '\\x00'::bytea, '\\x00'::bytea, 1, 'CONFIRMED', NOW())`,
|
||||
[USER_ID],
|
||||
);
|
||||
|
||||
const res = await fetch(`${base}/api/v1/admin/mailboxes/${USER_ID}/totp`, {
|
||||
method: "DELETE",
|
||||
headers: { Authorization: `Bearer ${admin}` },
|
||||
});
|
||||
expect(res.status).toBe(200);
|
||||
|
||||
const still = await query(`SELECT 1 FROM mailboxes WHERE id = $1`, [USER_ID]);
|
||||
expect(still).toHaveLength(1);
|
||||
});
|
||||
|
||||
it("answers 404 for a user that does not exist rather than leaking the difference", async () => {
|
||||
const res = await get(`/api/v1/admin/mailboxes/44444444-4444-4444-4444-444444444444/aliases`, admin);
|
||||
expect(res.status).toBe(404);
|
||||
|
||||
@@ -176,9 +176,19 @@ async function route(req: IncomingMessage, res: ServerResponse, url: string): Pr
|
||||
if (url.startsWith("/api/v1/admin/aliases/") && method === "DELETE") {
|
||||
return adminDeleteAlias(req, res, session!, url.substring("/api/v1/admin/aliases/".length));
|
||||
}
|
||||
if (url.startsWith("/api/v1/admin/mailboxes/") && url.endsWith("/totp") && method === "DELETE") {
|
||||
return adminRemoveTotp(req, res, session!, url.slice("/api/v1/admin/mailboxes/".length, -"/totp".length));
|
||||
}
|
||||
// Must stay below the sub-resource DELETEs above: this one matches any
|
||||
// path under /mailboxes/, so a more specific route placed after it would
|
||||
// be routed here instead - removing a second factor would delete the
|
||||
// mailbox.
|
||||
if (url.startsWith("/api/v1/admin/mailboxes/") && method === "DELETE") {
|
||||
return adminDeleteMailbox(req, res, session!, url.substring("/api/v1/admin/mailboxes/".length));
|
||||
}
|
||||
if (url.startsWith("/api/v1/admin/mailboxes/") && url.endsWith("/password") && method === "POST") {
|
||||
return adminResetPassword(req, res, session!, url.slice("/api/v1/admin/mailboxes/".length, -"/password".length));
|
||||
}
|
||||
if (url.startsWith("/api/v1/admin/mailboxes/") && url.endsWith("/aliases") && method === "GET") {
|
||||
return adminMailboxAliases(res, session!, url.slice("/api/v1/admin/mailboxes/".length, -"/aliases".length));
|
||||
}
|
||||
@@ -809,6 +819,41 @@ async function adminUpdateMailbox(
|
||||
sendJson(res, 200, { ok: true });
|
||||
}
|
||||
|
||||
/** Sets a user's password from the console and signs their sessions out. */
|
||||
async function adminResetPassword(
|
||||
req: IncomingMessage, res: ServerResponse, session: SessionTokenPayload, id: string,
|
||||
): Promise<void> {
|
||||
const body = await parseJsonBody(req);
|
||||
const password = typeof body?.password === "string" ? body.password : "";
|
||||
|
||||
// The same floor the account creation and the CLI enforce; a console reset
|
||||
// must not be the one way to get a weak password onto an account.
|
||||
if (password.length < 12) {
|
||||
return sendJson(res, 400, {
|
||||
error: "WEAK_PASSWORD",
|
||||
message: "The password must be at least 12 characters",
|
||||
});
|
||||
}
|
||||
|
||||
if (!(await repo.adminSetPassword(scopeOf(session), id, password))) {
|
||||
return sendJson(res, 404, { error: "NOT_FOUND", message: "No such user" });
|
||||
}
|
||||
// The password itself never reaches the audit log.
|
||||
await repo.recordAudit(session.sub, clientIp(req), "user.password_reset", "mailbox", id, {});
|
||||
sendJson(res, 200, { ok: true });
|
||||
}
|
||||
|
||||
/** Removes a user's second factor, for somebody who lost their authenticator. */
|
||||
async function adminRemoveTotp(
|
||||
req: IncomingMessage, res: ServerResponse, session: SessionTokenPayload, id: string,
|
||||
): Promise<void> {
|
||||
if (!(await repo.adminRemoveTotp(scopeOf(session), id))) {
|
||||
return sendJson(res, 404, { error: "NOT_FOUND", message: "No such user" });
|
||||
}
|
||||
await repo.recordAudit(session.sub, clientIp(req), "user.totp_removed", "mailbox", id, {});
|
||||
sendJson(res, 200, { ok: true });
|
||||
}
|
||||
|
||||
/** The aliases that deliver to one user. */
|
||||
async function adminMailboxAliases(
|
||||
res: ServerResponse, session: SessionTokenPayload, id: string,
|
||||
|
||||
@@ -106,7 +106,13 @@
|
||||
"runChecks": "Run",
|
||||
"checksRun": "{count} checks",
|
||||
"pickADomain": "Pick a domain.",
|
||||
"aliasSource": "Alias address"
|
||||
"aliasSource": "Alias address",
|
||||
"setPassword": "Set password",
|
||||
"atLeast12": "At least 12 characters",
|
||||
"passwordReset": "Password changed. This user's sessions were signed out.",
|
||||
"passwordResetWarning": "Signs out every session this user holds, including one open right now.",
|
||||
"removeTotp": "Remove second factor",
|
||||
"totpRemoved": "Second factor removed, with its recovery codes."
|
||||
},
|
||||
"auth": {
|
||||
"adminLoginTitle": "Admin console",
|
||||
|
||||
@@ -106,7 +106,13 @@
|
||||
"runChecks": "Ejecutar",
|
||||
"checksRun": "{count} comprobaciones",
|
||||
"pickADomain": "Elija un dominio.",
|
||||
"aliasSource": "Direccion del alias"
|
||||
"aliasSource": "Direccion del alias",
|
||||
"setPassword": "Definir contrasena",
|
||||
"atLeast12": "Al menos 12 caracteres",
|
||||
"passwordReset": "Contrasena cambiada. Las sesiones de este usuario se cerraron.",
|
||||
"passwordResetWarning": "Cierra todas las sesiones de este usuario, incluida una abierta ahora.",
|
||||
"removeTotp": "Quitar segundo factor",
|
||||
"totpRemoved": "Segundo factor eliminado, con sus codigos de recuperacion."
|
||||
},
|
||||
"auth": {
|
||||
"adminLoginTitle": "Consola de administración",
|
||||
|
||||
@@ -106,7 +106,13 @@
|
||||
"runChecks": "Correr",
|
||||
"checksRun": "{count} verificacoes",
|
||||
"pickADomain": "Escolha um dominio.",
|
||||
"aliasSource": "Endereco do apelido"
|
||||
"aliasSource": "Endereco do apelido",
|
||||
"setPassword": "Definir palavra-passe",
|
||||
"atLeast12": "Pelo menos 12 caracteres",
|
||||
"passwordReset": "Palavra-passe alterada. As sessoes deste utilizador foram terminadas.",
|
||||
"passwordResetWarning": "Termina todas as sessoes deste utilizador, incluindo a que esteja aberta agora.",
|
||||
"removeTotp": "Remover segundo factor",
|
||||
"totpRemoved": "Segundo factor removido, com os codigos de recuperacao."
|
||||
},
|
||||
"auth": {
|
||||
"adminLoginTitle": "Console de administração",
|
||||
|
||||
@@ -117,6 +117,9 @@ export default function AdminMailboxesPage() {
|
||||
const [saving, setSaving] = useState(false);
|
||||
const [newAliasForUser, setNewAliasForUser] = useState("");
|
||||
const [aliasBusy, setAliasBusy] = useState(false);
|
||||
const [newUserPassword, setNewUserPassword] = useState("");
|
||||
const [pwBusy, setPwBusy] = useState(false);
|
||||
const [notice, setNotice] = useState<string | null>(null);
|
||||
|
||||
const load = useCallback(async () => {
|
||||
try {
|
||||
@@ -247,6 +250,8 @@ export default function AdminMailboxesPage() {
|
||||
setEditQuotaMb(Math.round(mb.quotaBytes / 1048576));
|
||||
setEditLocale(mb.locale);
|
||||
setEditAliases(null);
|
||||
setNewUserPassword("");
|
||||
setNotice(null);
|
||||
try {
|
||||
const res = await fetch(`/api/v1/admin/mailboxes/${encodeURIComponent(mb.id)}/aliases`);
|
||||
const data = res.ok ? await res.json() : [];
|
||||
@@ -311,6 +316,54 @@ export default function AdminMailboxesPage() {
|
||||
}
|
||||
};
|
||||
|
||||
/**
|
||||
* Sets the user's password. Their sessions are signed out by the server,
|
||||
* which is the point: this is either a handover or a response to a
|
||||
* compromise.
|
||||
*/
|
||||
const resetUserPassword = async () => {
|
||||
if (!editing || newUserPassword.length < 12) return;
|
||||
setPwBusy(true);
|
||||
setError(null);
|
||||
setNotice(null);
|
||||
try {
|
||||
const res = await fetch(`/api/v1/admin/mailboxes/${encodeURIComponent(editing.id)}/password`, {
|
||||
method: "POST",
|
||||
headers: { "Content-Type": "application/json" },
|
||||
body: JSON.stringify({ password: newUserPassword }),
|
||||
});
|
||||
const data = await res.json().catch(() => ({}));
|
||||
if (!res.ok) throw new Error(data.message);
|
||||
setNewUserPassword("");
|
||||
setNotice(t("admin.passwordReset"));
|
||||
} catch (err) {
|
||||
setError(err instanceof Error && err.message ? err.message : t("errors.serverError"));
|
||||
} finally {
|
||||
setPwBusy(false);
|
||||
}
|
||||
};
|
||||
|
||||
/** Removes the second factor, for somebody who lost their authenticator. */
|
||||
const removeUserTotp = async () => {
|
||||
if (!editing) return;
|
||||
setPwBusy(true);
|
||||
setError(null);
|
||||
setNotice(null);
|
||||
try {
|
||||
const res = await fetch(`/api/v1/admin/mailboxes/${encodeURIComponent(editing.id)}/totp`, {
|
||||
method: "DELETE",
|
||||
});
|
||||
if (!res.ok) throw new Error();
|
||||
setNotice(t("admin.totpRemoved"));
|
||||
setEditing({ ...editing, mfaEnabled: false });
|
||||
await load();
|
||||
} catch {
|
||||
setError(t("errors.serverError"));
|
||||
} finally {
|
||||
setPwBusy(false);
|
||||
}
|
||||
};
|
||||
|
||||
const saveUser = async () => {
|
||||
if (!editing) return;
|
||||
setSaving(true);
|
||||
@@ -782,6 +835,56 @@ export default function AdminMailboxesPage() {
|
||||
</select>
|
||||
</div>
|
||||
|
||||
<div className="rounded-xl bg-dark-card p-3.5 shadow-edge">
|
||||
<div className="text-xs text-slate-400">{t("admin.setPassword")}</div>
|
||||
<div className="mt-2 flex flex-wrap items-center gap-2">
|
||||
<input
|
||||
type="password"
|
||||
value={newUserPassword}
|
||||
onChange={(e) => setNewUserPassword(e.target.value)}
|
||||
placeholder={t("admin.atLeast12")}
|
||||
aria-label={t("admin.setPassword")}
|
||||
autoComplete="new-password"
|
||||
className={`${input} min-w-[200px] flex-1`}
|
||||
/>
|
||||
<Button
|
||||
variant="secondary"
|
||||
size="sm"
|
||||
onClick={() => void resetUserPassword()}
|
||||
disabled={pwBusy || newUserPassword.length < 12}
|
||||
>
|
||||
{t("common.save")}
|
||||
</Button>
|
||||
</div>
|
||||
<p className="mt-1.5 text-[11.5px] leading-relaxed text-slate-500">
|
||||
{t("admin.passwordResetWarning")}
|
||||
</p>
|
||||
</div>
|
||||
|
||||
<div className="flex flex-wrap items-center justify-between gap-2.5 rounded-xl bg-dark-card p-3.5 shadow-edge">
|
||||
<div className="min-w-0">
|
||||
<div className="text-xs text-slate-400">{t("ui.twoFactorSection")}</div>
|
||||
<div className="mt-1 text-[13px] text-slate-200">
|
||||
{editing.mfaEnabled ? t("ui.mfaActive") : t("ui.mfaNone")}
|
||||
</div>
|
||||
</div>
|
||||
<Button
|
||||
variant="danger"
|
||||
size="sm"
|
||||
onClick={() => void removeUserTotp()}
|
||||
disabled={pwBusy || !editing.mfaEnabled}
|
||||
>
|
||||
<ShieldCheck className="h-3.5 w-3.5" />
|
||||
<span>{t("admin.removeTotp")}</span>
|
||||
</Button>
|
||||
</div>
|
||||
|
||||
{notice && (
|
||||
<div className="rounded-xl bg-dark-card px-3.5 py-2.5 text-[12.5px] text-slate-200 shadow-edge">
|
||||
{notice}
|
||||
</div>
|
||||
)}
|
||||
|
||||
<div className="rounded-xl bg-dark-card p-3.5 shadow-edge">
|
||||
<div className="text-xs text-slate-400">{t("admin.aliasesForUser")}</div>
|
||||
{editAliases === null ? (
|
||||
|
||||
Reference in new issue
Block a user