#22: fix the admin lockout, and the regressions the UI rewrite brought back
The console demanded a second factor and refused to open without one, but
enrolment lived on the other surface - so an operator whose only account
was the administrator had no way in. The password was already verified at
that point, so the refusal now carries a short-lived grant that permits
enrolling a factor and nothing else, and the admin login screen walks
through it: secret, confirmation, recovery codes, sign in. Two tests cover
it, including that the grant opens no other endpoint.
The rewrite also undid work that was already in place:
- The postcss override was dropped and the advisory came back, along
with a critical one in vitest. postcss is a direct dependency now, so
the fix is the version plus an override scoped to Next's own copy;
vitest moves to 3.2.7. The lockfile needed regenerating for either to
take effect. Zero advisories now.
- 100 language-lint violations: the admin console had been rewritten
with its strings in Portuguese, hardcoded. That fails CI, and it also
puts those strings beyond the reach of the translation bundles - an
English or Spanish user would have read Portuguese. They are English
in the source again.
- The mail screen put a fixed 320px list beside the reader with no
breakpoint, so on a phone the viewport overflowed and the message was
a few pixels wide. One pane at a time below md, with a way back.
- The icon-only navigation toggles had nothing for a screen reader to
announce.
The layouts' own mobile navigation and every previously fixed behaviour
survived the rewrite - checked rather than assumed.
Also fixes a marker collision I introduced: the E2E helper locates the
repository by a migrations directory, and services/migrations satisfied
that one directory too early. It now requires the SQL to actually be
there.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
1 parent
2282dc96e9
commit
3a82d82428
26 files changed
+3554
-1402
No files matched your search
@@ -211,11 +211,64 @@ describeDb("auth API against a real database", () => {
|
||||
}
|
||||
});
|
||||
|
||||
it("refuses the admin console until a second factor is enrolled", async () => {
|
||||
const res = await post("/api/v1/auth/admin/login", { email: adminEmail(), password: PASSWORD });
|
||||
// Refusing is right, but refusing without a way forward was a dead end: the
|
||||
// console needs a second factor, and the operator had to already know that
|
||||
// enrolment lives on the other surface.
|
||||
it("hands back an enrolment grant instead of a dead end", async () => {
|
||||
const email = `deadend@authtest-${DOMAIN_ID}.invalid`;
|
||||
await query(
|
||||
`INSERT INTO mailboxes (domain_id, local_part, email_address, password_hash, role)
|
||||
VALUES ($1, 'deadend', $2, $3, 'SUPER_ADMIN') ON CONFLICT (domain_id, local_part) DO NOTHING`,
|
||||
[DOMAIN_ID, email, await hashPassword(PASSWORD)],
|
||||
);
|
||||
|
||||
const res = await post("/api/v1/auth/admin/login", { email, password: PASSWORD });
|
||||
expect(res.status).toBe(403);
|
||||
expect(res.body.error).toBe("MFA_ENROLLMENT_REQUIRED");
|
||||
});
|
||||
|
||||
const grant = res.body.enrollment_token as unknown as string;
|
||||
expect(grant).toBeDefined();
|
||||
|
||||
// The grant enrols, and the account can then reach the console.
|
||||
const enroll = await post("/api/v1/user/mfa/totp/enroll", {}, grant);
|
||||
expect(enroll.status).toBe(200);
|
||||
|
||||
const secret = enroll.body.secret as unknown as string;
|
||||
const confirm = await post("/api/v1/user/mfa/totp/confirm",
|
||||
{ code: generateHotp(base32Decode(secret), getCurrentStep()) }, grant);
|
||||
expect(confirm.status).toBe(200);
|
||||
|
||||
const mailboxId = (await query<{ id: string }>(
|
||||
`SELECT id FROM mailboxes WHERE email_address = $1`, [email]))[0].id;
|
||||
await query(`UPDATE mfa_totp SET last_used_step = NULL WHERE mailbox_id = $1`, [mailboxId]);
|
||||
|
||||
const challenge = await post("/api/v1/auth/admin/login", { email, password: PASSWORD });
|
||||
expect(challenge.body.mfa_required).toBe(true);
|
||||
const verified = await post("/api/v1/auth/mfa/verify", {
|
||||
challenge_token: challenge.body.challenge_token,
|
||||
code: generateHotp(base32Decode(secret), getCurrentStep()),
|
||||
});
|
||||
expect(verified.status).toBe(200);
|
||||
expect((await get("/api/v1/admin/dashboard", verified.body.token)).status).toBe(200);
|
||||
}, 120000);
|
||||
|
||||
// The grant is for enrolling and nothing else.
|
||||
it("refuses to use an enrolment grant as a session", async () => {
|
||||
const email = `grantscope@authtest-${DOMAIN_ID}.invalid`;
|
||||
await query(
|
||||
`INSERT INTO mailboxes (domain_id, local_part, email_address, password_hash, role)
|
||||
VALUES ($1, 'grantscope', $2, $3, 'SUPER_ADMIN') ON CONFLICT (domain_id, local_part) DO NOTHING`,
|
||||
[DOMAIN_ID, email, await hashPassword(PASSWORD)],
|
||||
);
|
||||
|
||||
const res = await post("/api/v1/auth/admin/login", { email, password: PASSWORD });
|
||||
const grant = res.body.enrollment_token as unknown as string;
|
||||
|
||||
expect((await get("/api/v1/admin/dashboard", grant)).status).toBe(401);
|
||||
expect((await get("/api/v1/user/me", grant)).status).toBe(401);
|
||||
expect((await get("/api/v1/user/sessions", grant)).status).toBe(401);
|
||||
expect((await get("/api/v1/user/app-passwords", grant)).status).toBe(401);
|
||||
}, 90000);
|
||||
|
||||
it("grants the admin console after a real second factor", { timeout: 60000 }, async () => {
|
||||
// Enrollment happens on the user surface, which the admin also owns.
|
||||
|
||||
@@ -6,6 +6,9 @@ import { createJwt, verifyJwt, isSurfaceAuthorized, type SessionTokenPayload } f
|
||||
import * as repo from "./repository.js";
|
||||
|
||||
const CHALLENGE_TTL_SECONDS = 300;
|
||||
// Long enough to scan a code and type one back, short enough that a token
|
||||
// granting enrolment is not left lying around.
|
||||
const ENROLMENT_TTL_SECONDS = 900;
|
||||
const SESSION_TTL_SECONDS = 28800;
|
||||
|
||||
/** Cookie scoped to the whole origin: the API lives under /api, so a cookie
|
||||
@@ -94,7 +97,19 @@ async function route(req: IncomingMessage, res: ServerResponse, url: string): Pr
|
||||
if (url.startsWith("/api/v1/admin/") && !isSurfaceAuthorized(session, "admin")) {
|
||||
return sendJson(res, 401, { error: "UNAUTHORIZED", message: "Admin session required" });
|
||||
}
|
||||
if (url.startsWith("/api/v1/user/") && !isSurfaceAuthorized(session, "user")) {
|
||||
// The two enrolment endpoints also accept the short-lived token handed out
|
||||
// when a password was correct but no second factor exists. It grants
|
||||
// nothing else, so an operator can enrol from the admin login screen
|
||||
// instead of having to discover the webmail settings.
|
||||
const isEnrolmentRoute =
|
||||
url === "/api/v1/user/mfa/totp/enroll" || url === "/api/v1/user/mfa/totp/confirm";
|
||||
const hasEnrolmentGrant = session?.purpose === "mfa_enrollment";
|
||||
|
||||
if (
|
||||
url.startsWith("/api/v1/user/") &&
|
||||
!isSurfaceAuthorized(session, "user") &&
|
||||
!(isEnrolmentRoute && hasEnrolmentGrant)
|
||||
) {
|
||||
return sendJson(res, 401, { error: "UNAUTHORIZED", message: "User session required" });
|
||||
}
|
||||
|
||||
@@ -217,9 +232,23 @@ async function login(req: IncomingMessage, res: ServerResponse, surface: "user"
|
||||
// An admin whose second factor is not enrolled must not fall through to a
|
||||
// full session: the admin surface requires MFA unconditionally.
|
||||
if (surface === "admin" && !(await repo.hasConfirmedTotp(mailbox.id))) {
|
||||
const enrolmentToken = createJwt(
|
||||
{
|
||||
sub: mailbox.id,
|
||||
email: mailbox.email_address,
|
||||
role: mailbox.role,
|
||||
domainId: mailbox.domain_id,
|
||||
surface: "user",
|
||||
aud: "lambdamail:user",
|
||||
mfaSatisfied: false,
|
||||
purpose: "mfa_enrollment",
|
||||
},
|
||||
ENROLMENT_TTL_SECONDS,
|
||||
);
|
||||
return sendJson(res, 403, {
|
||||
error: "MFA_ENROLLMENT_REQUIRED",
|
||||
message: "Enroll a second factor from the webmail settings before using the admin console",
|
||||
message: "The admin console requires a second factor. Enrol one to continue.",
|
||||
enrollment_token: enrolmentToken,
|
||||
});
|
||||
}
|
||||
|
||||
|
||||
@@ -12,7 +12,10 @@ export interface SessionTokenPayload {
|
||||
// Separates the short-lived token handed out between password and second
|
||||
// factor from a real session. Without it, a challenge token - issued before
|
||||
// any second factor was proven - would be accepted anywhere a session is.
|
||||
purpose: "mfa_challenge" | "session";
|
||||
// mfa_enrollment is issued when the password was right but no second factor
|
||||
// exists yet. It permits enrolling one and nothing else, which is what turns
|
||||
// the admin console's "enroll first" from a dead end into a step.
|
||||
purpose: "mfa_challenge" | "mfa_enrollment" | "session";
|
||||
iat: number;
|
||||
exp: number;
|
||||
}
|
||||
|
||||
Reference in new issue
Block a user