#34: fix the mail counters, the reader pane, and the state between surfaces
The counts were computed inline in JSX where nothing could test them. The
sidebar matched a folder by a special-use role folders do not always carry, its
badges were fetched once and then froze - new mail did not raise them and
reading a message did not lower it - and the list header printed the size of
the loaded page, capped at 50, as the size of the folder. The rules now live in
mailCounts.ts behind tests, useFolders keeps them current from the server's
push and from this tab's own changes, and each filter chip carries its number.
Inline images never left the server: RenderMessage dropped every part a body
references as cid:, so mail written by any normal client rendered with broken
icons. They now come back as bounded data URIs, refusing image/svg+xml, and the
reader resolves them, sizes its frame from the message instead of a fixed
500px, and renders on the light surface mail assumes.
Reading mail no longer asks for a second factor - the mailbox is behind the
account's own password - while the console still requires one unconditionally,
now collected by a step-up that does not ask again for the password the session
in hand just proved. An admin sign-in issues the webmail session too, so
leaving the console is a link rather than another login.
Two defects found while testing the above:
- The admin sign-in checked the password and the second factor and never the
role, so any account that had enrolled a factor from its own settings could
obtain an admin-audience session.
- Session tokens carried nothing that varied below one second, so signing in
twice within the same second minted the identical token and collided with
the unique index on web_sessions.refresh_token_hash. A correct password
answered 500, and a double-clicked sign-in was enough to do it. Tokens now
carry a jti.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
1 parent
e2ea58a4c9
commit
8b69521c76
25 files changed
+2276
-380
No files matched your search
@@ -168,12 +168,16 @@ describeDb("auth API against a real database", () => {
|
||||
});
|
||||
|
||||
// The second headline regression: any non-empty MFA code used to work.
|
||||
// The challenge comes from the admin sign-in, which is the surface that
|
||||
// collects a second factor. The webmail no longer does - reading your own
|
||||
// mail is behind your own password - so this used to obtain its challenge
|
||||
// from a path that has none.
|
||||
it("rejects a wrong TOTP code and accepts only the real one", { timeout: 60000 }, async () => {
|
||||
const login = await post("/api/v1/auth/user/login", { email: userEmail(), password: PASSWORD });
|
||||
const secret = await enrollTotp(USER_ID, login.body.token as unknown as string);
|
||||
const login = await post("/api/v1/auth/user/login", { email: adminEmail(), password: PASSWORD });
|
||||
await query(`DELETE FROM mfa_totp WHERE mailbox_id = $1`, [ADMIN_ID]);
|
||||
const secret = await enrollTotp(ADMIN_ID, login.body.token as unknown as string);
|
||||
|
||||
// With a factor enrolled, login now demands it.
|
||||
const challenge = await post("/api/v1/auth/user/login", { email: userEmail(), password: PASSWORD });
|
||||
const challenge = await post("/api/v1/auth/admin/login", { email: adminEmail(), password: PASSWORD });
|
||||
expect(challenge.body.mfa_required).toBe(true);
|
||||
|
||||
const wrong = await post("/api/v1/auth/mfa/verify", {
|
||||
@@ -306,7 +310,11 @@ describeDb("auth API against a real database", () => {
|
||||
}, 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.
|
||||
// Enrollment happens on the user surface, which the admin also owns. The
|
||||
// existing factor is cleared first, as adminSession does: enrolling over a
|
||||
// confirmed one leaves two secrets on the account and the code generated
|
||||
// here is then checked against the wrong one.
|
||||
await query(`DELETE FROM mfa_totp WHERE mailbox_id = $1`, [ADMIN_ID]);
|
||||
const userLogin = await post("/api/v1/auth/user/login", { email: adminEmail(), password: PASSWORD });
|
||||
const secret = await enrollTotp(ADMIN_ID, userLogin.body.token as unknown as string);
|
||||
|
||||
@@ -360,12 +368,14 @@ describeDb("auth API against a real database", () => {
|
||||
expect(res.body.error).toBe("ACCOUNT_LOCKED");
|
||||
});
|
||||
|
||||
// Redeemed at the admin sign-in, which is where a second factor is collected
|
||||
// now. The account is an admin for the same reason.
|
||||
it("consumes a recovery code exactly once", { timeout: 60000 }, async () => {
|
||||
const email = `rec@authtest-${DOMAIN_ID}.invalid`;
|
||||
const id = crypto.randomUUID();
|
||||
await query(
|
||||
`INSERT INTO mailboxes (id, domain_id, local_part, email_address, password_hash)
|
||||
VALUES ($1, $2, 'rec', $3, $4)`,
|
||||
`INSERT INTO mailboxes (id, domain_id, local_part, email_address, password_hash, role)
|
||||
VALUES ($1, $2, 'rec', $3, $4, 'SUPER_ADMIN')`,
|
||||
[id, DOMAIN_ID, email, await hashPassword(PASSWORD)],
|
||||
);
|
||||
|
||||
@@ -378,14 +388,14 @@ describeDb("auth API against a real database", () => {
|
||||
const confirmed = await post("/api/v1/user/mfa/totp/confirm", { code }, login.body.token);
|
||||
const recoveryCode = (confirmed.body.recovery_codes as unknown as string[])[0];
|
||||
|
||||
const challenge = await post("/api/v1/auth/user/login", { email, password: PASSWORD });
|
||||
const challenge = await post("/api/v1/auth/admin/login", { email, password: PASSWORD });
|
||||
const first = await post("/api/v1/auth/mfa/verify", {
|
||||
challenge_token: challenge.body.challenge_token,
|
||||
code: recoveryCode,
|
||||
});
|
||||
expect(first.status).toBe(200);
|
||||
|
||||
const challenge2 = await post("/api/v1/auth/user/login", { email, password: PASSWORD });
|
||||
const challenge2 = await post("/api/v1/auth/admin/login", { email, password: PASSWORD });
|
||||
const second = await post("/api/v1/auth/mfa/verify", {
|
||||
challenge_token: challenge2.body.challenge_token,
|
||||
code: recoveryCode,
|
||||
@@ -663,6 +673,191 @@ describe("surface routing", () => {
|
||||
});
|
||||
});
|
||||
|
||||
/**
|
||||
* What each surface costs to open, and what a session grants once opened.
|
||||
*
|
||||
* Two rules made the app tiresome in a way no unit test could see. Reading your
|
||||
* own mail demanded a second factor, which the mailbox does not require - only
|
||||
* the console does. And a session issued at the admin sign-in carried the admin
|
||||
* audience alone, so leaving the console landed on the webmail's login screen,
|
||||
* asking for a password from somebody who had just proven a password and a
|
||||
* second factor a moment earlier.
|
||||
*/
|
||||
describeDb("moving between the surfaces", () => {
|
||||
const stepUpEmail = () => `stepup@authtest-${DOMAIN_ID}.invalid`;
|
||||
const plainEmail = () => `plainuser@authtest-${DOMAIN_ID}.invalid`;
|
||||
let stepUpId = "";
|
||||
|
||||
/** Reads the Set-Cookie headers, which is where the two sessions land. */
|
||||
async function postForCookies(path: string, body: unknown, token?: string) {
|
||||
const res = await fetch(`${base}${path}`, {
|
||||
method: "POST",
|
||||
headers: {
|
||||
"Content-Type": "application/json",
|
||||
...(token ? { Authorization: `Bearer ${token}` } : {}),
|
||||
},
|
||||
body: JSON.stringify(body),
|
||||
});
|
||||
return {
|
||||
status: res.status,
|
||||
cookies: res.headers.getSetCookie(),
|
||||
body: (await res.json().catch(() => ({}))) as Record<string, never>,
|
||||
};
|
||||
}
|
||||
|
||||
const cookieNames = (cookies: string[]) => cookies.map((c) => c.split("=")[0]).sort();
|
||||
|
||||
/**
|
||||
* One session per account, taken once.
|
||||
*
|
||||
* Every login costs an Argon2 verify, and this service deliberately bounds
|
||||
* how many of those run at a time, so a test file that signs in per assertion
|
||||
* spends its time queueing behind itself. A session does not expire during
|
||||
* the run, so reusing it tests the same thing and leaves the budget for the
|
||||
* assertions that need a fresh sign-in.
|
||||
*/
|
||||
let stepUpToken = "";
|
||||
let plainToken = "";
|
||||
let plainId = "";
|
||||
|
||||
beforeAll(async () => {
|
||||
await startServer();
|
||||
await seed();
|
||||
const hash = await hashPassword(PASSWORD);
|
||||
for (const [local, role] of [["stepup", "SUPER_ADMIN"], ["plainuser", "USER"]] as const) {
|
||||
await query(
|
||||
`INSERT INTO mailboxes (domain_id, local_part, email_address, password_hash, role)
|
||||
VALUES ($1, $2, $3, $4, $5) ON CONFLICT (domain_id, local_part) DO NOTHING`,
|
||||
[DOMAIN_ID, local, `${local}@authtest-${DOMAIN_ID}.invalid`, hash, role],
|
||||
);
|
||||
}
|
||||
stepUpId = (await query<{ id: string }>(
|
||||
`SELECT id FROM mailboxes WHERE email_address = $1`, [stepUpEmail()]))[0].id;
|
||||
plainId = (await query<{ id: string }>(
|
||||
`SELECT id FROM mailboxes WHERE email_address = $1`, [plainEmail()]))[0].id;
|
||||
|
||||
stepUpToken = (await post("/api/v1/auth/user/login",
|
||||
{ email: stepUpEmail(), password: PASSWORD })).body.token as unknown as string;
|
||||
plainToken = (await post("/api/v1/auth/user/login",
|
||||
{ email: plainEmail(), password: PASSWORD })).body.token as unknown as string;
|
||||
}, 120000);
|
||||
|
||||
afterAll(async () => {
|
||||
server?.close();
|
||||
await query(`DELETE FROM domains WHERE id = $1`, [DOMAIN_ID]).catch(() => undefined);
|
||||
await closePool();
|
||||
});
|
||||
|
||||
// The mailbox is one account's own mail behind that account's own password.
|
||||
// Demanding a code as well meant an enrolled account could not read its mail
|
||||
// without its phone.
|
||||
it("opens the webmail on the password alone, even with a factor enrolled", async () => {
|
||||
await enrollTotp(stepUpId, stepUpToken);
|
||||
|
||||
const login = await post("/api/v1/auth/user/login", { email: stepUpEmail(), password: PASSWORD });
|
||||
expect(login.status).toBe(200);
|
||||
expect(login.body.mfa_required).toBeUndefined();
|
||||
expect(login.body.token).toBeDefined();
|
||||
|
||||
// And the session it hands out is one the API actually accepts.
|
||||
expect((await get("/api/v1/user/me", login.body.token)).status).toBe(200);
|
||||
}, 120000);
|
||||
|
||||
// A webmail session is still not an admin session; that is the isolation the
|
||||
// whole audience split exists for.
|
||||
it("does not let the webmail session open the console by itself", async () => {
|
||||
expect((await get("/api/v1/admin/dashboard", stepUpToken)).status).toBe(401);
|
||||
});
|
||||
|
||||
// The sign-in checked the password and the second factor and never the role,
|
||||
// so an ordinary account that had enrolled a factor - which any account may
|
||||
// do from its own settings - could sign in at /admin/login and be handed an
|
||||
// admin-audience session. The step-up checks the role; this path did not.
|
||||
it("refuses an ordinary account at the admin sign-in, factor or no factor", async () => {
|
||||
await query(`DELETE FROM mfa_totp WHERE mailbox_id = $1`, [plainId]);
|
||||
const secret = await enrollTotp(plainId, plainToken);
|
||||
|
||||
const attempt = await post("/api/v1/auth/admin/login", { email: plainEmail(), password: PASSWORD });
|
||||
expect(attempt.status).toBe(403);
|
||||
expect(attempt.body.error).toBe("FORBIDDEN");
|
||||
|
||||
// And no challenge came back that could be verified into one anyway.
|
||||
expect(attempt.body.challenge_token).toBeUndefined();
|
||||
void secret;
|
||||
}, 120000);
|
||||
|
||||
it("refuses a step-up without a webmail session behind it", async () => {
|
||||
expect((await post("/api/v1/auth/admin/step-up", { code: "123456" })).status).toBe(401);
|
||||
});
|
||||
|
||||
it("refuses a step-up with the wrong code, and never on the password alone", async () => {
|
||||
const res = await post("/api/v1/auth/admin/step-up", { code: "000000" }, stepUpToken);
|
||||
expect(res.status).toBe(401);
|
||||
expect(res.body.error).toBe("INVALID_MFA_CODE");
|
||||
});
|
||||
|
||||
// The role is read from the database rather than from the token, so an
|
||||
// ordinary account cannot cross over however valid its session is.
|
||||
it("refuses a step-up from an account that may not open the console", async () => {
|
||||
const res = await post("/api/v1/auth/admin/step-up", { code: "123456" }, plainToken);
|
||||
expect(res.status).toBe(403);
|
||||
});
|
||||
|
||||
// The point of the step-up: the second factor, and not the password, which
|
||||
// the session in hand proved already.
|
||||
it("crosses into the console on the second factor alone", async () => {
|
||||
await query(`DELETE FROM mfa_totp WHERE mailbox_id = $1`, [stepUpId]);
|
||||
const secret = await enrollTotp(stepUpId, stepUpToken);
|
||||
|
||||
const stepUp = await postForCookies(
|
||||
"/api/v1/auth/admin/step-up",
|
||||
{ code: generateHotp(base32Decode(secret), getCurrentStep()) },
|
||||
stepUpToken,
|
||||
);
|
||||
expect(stepUp.status).toBe(200);
|
||||
expect((await get("/api/v1/admin/dashboard", stepUp.body.token)).status).toBe(200);
|
||||
|
||||
// Both sessions come back, which is what makes leaving the console free.
|
||||
expect(cookieNames(stepUp.cookies)).toEqual(["lm_admin_session", "lm_user_session"]);
|
||||
}, 120000);
|
||||
|
||||
// Proving a password and a second factor is strictly more than the webmail
|
||||
// asks for, so it must not leave the operator without a webmail session.
|
||||
it("issues a webmail session at the admin sign-in too", async () => {
|
||||
await query(`DELETE FROM mfa_totp WHERE mailbox_id = $1`, [stepUpId]);
|
||||
const secret = await enrollTotp(stepUpId, stepUpToken);
|
||||
|
||||
const challenge = await post("/api/v1/auth/admin/login", { email: stepUpEmail(), password: PASSWORD });
|
||||
expect(challenge.body.mfa_required).toBe(true);
|
||||
|
||||
const verified = await postForCookies("/api/v1/auth/mfa/verify", {
|
||||
challenge_token: challenge.body.challenge_token,
|
||||
code: generateHotp(base32Decode(secret), getCurrentStep()),
|
||||
});
|
||||
expect(verified.status).toBe(200);
|
||||
expect(cookieNames(verified.cookies)).toEqual(["lm_admin_session", "lm_user_session"]);
|
||||
|
||||
// The webmail cookie has to be a webmail token, not the admin one copied
|
||||
// across: an admin-audience token is refused by every /user route.
|
||||
const userCookie = verified.cookies.find((c) => c.startsWith("lm_user_session="))!;
|
||||
const userToken = userCookie.slice("lm_user_session=".length).split(";")[0];
|
||||
expect((await get("/api/v1/user/me", userToken)).status).toBe(200);
|
||||
}, 120000);
|
||||
|
||||
// Signing in twice in the same second produced a byte-identical token, which
|
||||
// collided with the unique index on web_sessions.refresh_token_hash and
|
||||
// answered 500 to a correct password. A double-clicked sign-in was enough.
|
||||
it("survives two sign-ins in the same second", async () => {
|
||||
const [first, second] = await Promise.all([
|
||||
post("/api/v1/auth/user/login", { email: plainEmail(), password: PASSWORD }),
|
||||
post("/api/v1/auth/user/login", { email: plainEmail(), password: PASSWORD }),
|
||||
]);
|
||||
expect(first.status).toBe(200);
|
||||
expect(second.status).toBe(200);
|
||||
expect(first.body.token).not.toBe(second.body.token);
|
||||
}, 60000);
|
||||
});
|
||||
|
||||
if (!hasDatabase) {
|
||||
// eslint-disable-next-line no-console
|
||||
console.warn("TEST_DATABASE_URL not set - auth API integration tests skipped");
|
||||
|
||||
+160
-48
@@ -3,6 +3,13 @@ import crypto from "node:crypto";
|
||||
import { generateTotpSecret, generateTotpUri } from "./totp.js";
|
||||
import { generateRecoveryCodes, generateAppPassword } from "./mfaHelpers.js";
|
||||
import { createJwt, verifyJwt, isSurfaceAuthorized, type SessionTokenPayload } from "./session.js";
|
||||
import {
|
||||
secondFactorRequired,
|
||||
surfacesGrantedBy,
|
||||
cookieNameFor,
|
||||
stepUpNeedsSecondFactor,
|
||||
type Surface,
|
||||
} from "./surfaceAccess.js";
|
||||
import * as repo from "./repository.js";
|
||||
|
||||
const CHALLENGE_TTL_SECONDS = 300;
|
||||
@@ -125,6 +132,10 @@ async function route(req: IncomingMessage, res: ServerResponse, url: string): Pr
|
||||
if (url === "/api/v1/auth/user/login" && method === "POST") return login(req, res, "user");
|
||||
if (url === "/api/v1/auth/admin/login" && method === "POST") return login(req, res, "admin");
|
||||
if (url === "/api/v1/auth/mfa/verify" && method === "POST") return mfaVerify(req, res);
|
||||
// Under /auth/ rather than /admin/: the caller does not hold an admin
|
||||
// session yet - obtaining one is the whole point - so the admin guard above
|
||||
// must not stand in front of it.
|
||||
if (url === "/api/v1/auth/admin/step-up" && method === "POST") return adminStepUp(req, res);
|
||||
if (url === "/api/v1/auth/logout" && method === "POST") return logout(res);
|
||||
|
||||
if (url === "/api/v1/user/me" && method === "GET") return me(res, session!);
|
||||
@@ -230,30 +241,35 @@ async function login(req: IncomingMessage, res: ServerResponse, surface: "user"
|
||||
|
||||
const { mailbox, mfaRequired } = outcome;
|
||||
|
||||
// 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,
|
||||
);
|
||||
// The console is for the roles that may operate it. This checked the password
|
||||
// and the second factor and never the role, so an ordinary account that had
|
||||
// enrolled a factor - which any account may do from its own settings - was
|
||||
// handed an admin-audience session, and the routes that do not re-check the
|
||||
// role opened for it. Refused before the challenge, so no token exists that
|
||||
// could be verified into one.
|
||||
if (surface === "admin" && mailbox.role !== "SUPER_ADMIN" && mailbox.role !== "DOMAIN_ADMIN") {
|
||||
return sendJson(res, 403, {
|
||||
error: "MFA_ENROLLMENT_REQUIRED",
|
||||
message: "The admin console requires a second factor. Enrol one to continue.",
|
||||
enrollment_token: enrolmentToken,
|
||||
error: "FORBIDDEN",
|
||||
message: "This account may not open the admin console",
|
||||
});
|
||||
}
|
||||
|
||||
if (mfaRequired) {
|
||||
// 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))) {
|
||||
return sendJson(res, 403, {
|
||||
error: "MFA_ENROLLMENT_REQUIRED",
|
||||
message: "The admin console requires a second factor. Enrol one to continue.",
|
||||
enrollment_token: enrolmentGrant(mailbox),
|
||||
});
|
||||
}
|
||||
|
||||
// Only the console collects a second factor at sign-in. The webmail is one
|
||||
// account's own mail behind that account's own password, and demanding a code
|
||||
// to read it made every message cost a phone - while the surface that can
|
||||
// rewrite other people's mailboxes is the one that needs it. An enrolled
|
||||
// factor is still asked for on the way into the console, by step-up.
|
||||
if (mfaRequired && secondFactorRequired(surface)) {
|
||||
const challengeToken = createJwt(
|
||||
{
|
||||
sub: mailbox.id,
|
||||
@@ -270,48 +286,144 @@ async function login(req: IncomingMessage, res: ServerResponse, surface: "user"
|
||||
return sendJson(res, 200, { mfa_required: true, challenge_token: challengeToken });
|
||||
}
|
||||
|
||||
// No factor enrolled yet. The session is issued so the user can reach the
|
||||
// settings screen and enroll; when policy obliges them to, the response says
|
||||
// so and the UI keeps them on that screen.
|
||||
// The session is issued so the user can reach the settings screen and enroll;
|
||||
// when policy obliges them to, the response says so and the UI keeps them on
|
||||
// that screen.
|
||||
await issueSession(req, res, mailbox, surface, repo.isMfaEnrollmentRequired(mailbox));
|
||||
}
|
||||
|
||||
/**
|
||||
* Crosses a signed-in webmail session over into the console.
|
||||
*
|
||||
* The password is not asked for again: the session presented here already
|
||||
* proved it, minutes ago. What the console adds over the webmail is the second
|
||||
* factor, so that is all this collects. Without this endpoint the only way over
|
||||
* was the admin sign-in form, which is why crossing surfaces used to mean
|
||||
* typing the password a second time.
|
||||
*/
|
||||
async function adminStepUp(req: IncomingMessage, res: ServerResponse): Promise<void> {
|
||||
const token = extractToken(req, "/api/v1/user/");
|
||||
if (!stepUpNeedsSecondFactor(token)) {
|
||||
return sendJson(res, 401, { error: "UNAUTHORIZED", message: "A signed-in webmail session is required" });
|
||||
}
|
||||
const payload = verifyJwt(token)!;
|
||||
|
||||
const mailbox = await repo.findMailboxByEmail(payload.email);
|
||||
if (!mailbox || !mailbox.is_active) {
|
||||
return sendJson(res, 401, { error: "INVALID_CREDENTIALS", message: "Account unavailable" });
|
||||
}
|
||||
// Role is read from the database rather than from the token: a role changed
|
||||
// since sign-in must take effect on the surface that acts on it.
|
||||
if (mailbox.role !== "SUPER_ADMIN" && mailbox.role !== "DOMAIN_ADMIN") {
|
||||
return sendJson(res, 403, { error: "FORBIDDEN", message: "This account may not open the admin console" });
|
||||
}
|
||||
|
||||
// No factor to ask for yet. The same grant the admin sign-in hands out is
|
||||
// returned, so the caller can enrol one and come straight back.
|
||||
if (!(await repo.hasConfirmedTotp(mailbox.id))) {
|
||||
return sendJson(res, 403, {
|
||||
error: "MFA_ENROLLMENT_REQUIRED",
|
||||
message: "The admin console requires a second factor. Enrol one to continue.",
|
||||
enrollment_token: enrolmentGrant(mailbox),
|
||||
});
|
||||
}
|
||||
|
||||
const body = await parseJsonBody(req);
|
||||
const code = typeof body?.code === "string" ? body.code.trim() : "";
|
||||
if (!code) {
|
||||
return sendJson(res, 400, { error: "CODE_REQUIRED", message: "Verification code required" });
|
||||
}
|
||||
|
||||
const accepted = /^\d{6}$/.test(code)
|
||||
? await repo.verifyTotpForLogin(mailbox.id, code)
|
||||
: await repo.consumeRecoveryCode(mailbox.id, code, clientIp(req));
|
||||
if (!accepted) {
|
||||
return sendJson(res, 401, { error: "INVALID_MFA_CODE", message: "Invalid verification code" });
|
||||
}
|
||||
|
||||
await issueSession(req, res, mailbox, "admin");
|
||||
}
|
||||
|
||||
/** The short-lived grant that permits enrolling a factor and nothing else. */
|
||||
function enrolmentGrant(mailbox: repo.MailboxRecord): string {
|
||||
return 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,
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Issues the sessions a sign-in on `surface` earns, and sets their cookies.
|
||||
*
|
||||
* An admin sign-in yields a webmail session as well. The two audiences are
|
||||
* separate tokens - a token minted for the console is refused by every /user
|
||||
* route, by design - so granting both means minting both. Doing so is what
|
||||
* turns leaving the console into a plain link: whoever proved a password and a
|
||||
* second factor has more than the webmail asks for, and being sent back to a
|
||||
* password prompt on the way out was the app forgetting that.
|
||||
*
|
||||
* mfaSatisfied records what was actually proven, so it is true only where a
|
||||
* factor was collected. isSurfaceAuthorized requires it for the console alone.
|
||||
*/
|
||||
async function issueSession(
|
||||
req: IncomingMessage,
|
||||
res: ServerResponse,
|
||||
mailbox: repo.MailboxRecord,
|
||||
surface: "user" | "admin",
|
||||
surface: Surface,
|
||||
enrollmentRequired = false,
|
||||
// Whether a second factor was actually collected on the way here. It defaults
|
||||
// to what the surface demands; mfaVerify passes it explicitly, because
|
||||
// recording a proven factor as unproven would make isStepUpMfaRequired fire
|
||||
// straight away.
|
||||
factorProven = secondFactorRequired(surface),
|
||||
): Promise<void> {
|
||||
const sessionToken = createJwt({
|
||||
sub: mailbox.id,
|
||||
email: mailbox.email_address,
|
||||
role: mailbox.role,
|
||||
domainId: mailbox.domain_id,
|
||||
surface,
|
||||
aud: surface === "admin" ? "lambdamail:admin" : "lambdamail:user",
|
||||
mfaSatisfied: true,
|
||||
mfaSatisfiedAt: Date.now(),
|
||||
purpose: "session",
|
||||
});
|
||||
const cookies: string[] = [];
|
||||
let primaryToken = "";
|
||||
|
||||
await repo.recordSession(
|
||||
mailbox.id,
|
||||
surface,
|
||||
crypto.createHash("sha256").update(sessionToken).digest("hex"),
|
||||
new Date(Date.now() + SESSION_TTL_SECONDS * 1000),
|
||||
clientIp(req),
|
||||
req.headers["user-agent"] ?? null,
|
||||
);
|
||||
for (const granted of surfacesGrantedBy(surface)) {
|
||||
const token = createJwt({
|
||||
sub: mailbox.id,
|
||||
email: mailbox.email_address,
|
||||
role: mailbox.role,
|
||||
domainId: mailbox.domain_id,
|
||||
surface: granted,
|
||||
aud: granted === "admin" ? "lambdamail:admin" : "lambdamail:user",
|
||||
mfaSatisfied: factorProven,
|
||||
...(factorProven ? { mfaSatisfiedAt: Date.now() } : {}),
|
||||
purpose: "session",
|
||||
});
|
||||
if (!primaryToken) primaryToken = token;
|
||||
|
||||
const cookieName = surface === "admin" ? "lm_admin_session" : "lm_user_session";
|
||||
res.setHeader("Set-Cookie", sessionCookie(cookieName, sessionToken, SESSION_TTL_SECONDS, isSecureRequest()));
|
||||
// Each token is recorded separately, or "sign out this device" would revoke
|
||||
// one audience and leave the other working.
|
||||
await repo.recordSession(
|
||||
mailbox.id,
|
||||
granted,
|
||||
crypto.createHash("sha256").update(token).digest("hex"),
|
||||
new Date(Date.now() + SESSION_TTL_SECONDS * 1000),
|
||||
clientIp(req),
|
||||
req.headers["user-agent"] ?? null,
|
||||
);
|
||||
|
||||
cookies.push(sessionCookie(cookieNameFor(granted), token, SESSION_TTL_SECONDS, isSecureRequest()));
|
||||
}
|
||||
|
||||
res.setHeader("Set-Cookie", cookies);
|
||||
sendJson(res, 200, {
|
||||
token: sessionToken,
|
||||
token: primaryToken,
|
||||
surface,
|
||||
role: mailbox.role,
|
||||
locale: mailbox.locale,
|
||||
mfa_satisfied: true,
|
||||
mfa_satisfied: factorProven,
|
||||
mfa_enrollment_required: enrollmentRequired,
|
||||
});
|
||||
}
|
||||
@@ -349,7 +461,7 @@ async function mfaVerify(req: IncomingMessage, res: ServerResponse): Promise<voi
|
||||
return sendJson(res, 401, { error: "INVALID_CREDENTIALS", message: "Account unavailable" });
|
||||
}
|
||||
|
||||
await issueSession(req, res, mailbox, payload.surface);
|
||||
await issueSession(req, res, mailbox, payload.surface, false, true);
|
||||
}
|
||||
|
||||
function logout(res: ServerResponse): void {
|
||||
|
||||
@@ -16,6 +16,14 @@ export interface SessionTokenPayload {
|
||||
// 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";
|
||||
// A unique identity per token (RFC 7519 "jti").
|
||||
//
|
||||
// Without it nothing in the payload varied below one second - iat and exp
|
||||
// are in seconds - so signing in twice in the same second produced the
|
||||
// identical token, which hashes to the identical value and collides with the
|
||||
// unique index on web_sessions.refresh_token_hash. A correct password
|
||||
// answered 500, and a double-clicked sign-in was enough to do it.
|
||||
jti: string;
|
||||
iat: number;
|
||||
exp: number;
|
||||
}
|
||||
@@ -51,11 +59,17 @@ function base64UrlDecode(str: string): string {
|
||||
return Buffer.from(base64, "base64").toString("utf8");
|
||||
}
|
||||
|
||||
export function createJwt(payload: Omit<SessionTokenPayload, "iat" | "exp">, expiresInSeconds = 28800): string {
|
||||
export function createJwt(
|
||||
payload: Omit<SessionTokenPayload, "jti" | "iat" | "exp">,
|
||||
expiresInSeconds = 28800,
|
||||
): string {
|
||||
const header = { alg: "HS256", typ: "JWT" };
|
||||
const now = Math.floor(Date.now() / 1000);
|
||||
const fullPayload: SessionTokenPayload = {
|
||||
...payload,
|
||||
// Minted here rather than by each caller, so no path can forget it and
|
||||
// reintroduce the collision described on the field.
|
||||
jti: crypto.randomUUID(),
|
||||
iat: now,
|
||||
exp: now + expiresInSeconds,
|
||||
};
|
||||
|
||||
@@ -0,0 +1,139 @@
|
||||
import { describe, it, expect } from "vitest";
|
||||
|
||||
process.env.JWT_SECRET ||= "a-test-signing-secret-long-enough-for-the-guard";
|
||||
|
||||
const { createJwt, verifyJwt, isSurfaceAuthorized } = await import("./session.js");
|
||||
const { secondFactorRequired, surfacesGrantedBy, cookieNameFor, stepUpNeedsSecondFactor } =
|
||||
await import("./surfaceAccess.js");
|
||||
|
||||
/**
|
||||
* Which surface a login opens, and what it costs to cross between them.
|
||||
*
|
||||
* The rules used to be spread through the login handler and got two things
|
||||
* wrong that made the app tiresome to use. Reading mail demanded a second
|
||||
* factor, which the mailbox does not require - only the console does. And a
|
||||
* session issued at the admin sign-in granted the admin audience alone, so
|
||||
* leaving the console landed on the webmail's login screen even though the
|
||||
* operator had authenticated a moment earlier, with a stronger factor.
|
||||
*/
|
||||
|
||||
describe("what each surface costs to open", () => {
|
||||
// The webmail is the account's own mail behind the account's own password.
|
||||
it("does not demand a second factor to read mail", () => {
|
||||
expect(secondFactorRequired("user")).toBe(false);
|
||||
});
|
||||
|
||||
// The console changes other people's mailboxes, domains and DNS.
|
||||
it("always demands one for the admin console", () => {
|
||||
expect(secondFactorRequired("admin")).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe("what a session grants", () => {
|
||||
it("gives a webmail sign-in the webmail and nothing more", () => {
|
||||
expect(surfacesGrantedBy("user")).toEqual(["user"]);
|
||||
});
|
||||
|
||||
// Proving a second factor is strictly more than the webmail asks for, so
|
||||
// being sent back to a password prompt on the way out of the console was
|
||||
// asking for something already provided.
|
||||
it("gives an admin sign-in the webmail as well, since it costs strictly more", () => {
|
||||
expect(surfacesGrantedBy("admin")).toEqual(["admin", "user"]);
|
||||
});
|
||||
|
||||
it("names a separate cookie per surface, so neither overwrites the other", () => {
|
||||
expect(cookieNameFor("user")).toBe("lm_user_session");
|
||||
expect(cookieNameFor("admin")).toBe("lm_admin_session");
|
||||
});
|
||||
});
|
||||
|
||||
describe("crossing from the webmail into the console", () => {
|
||||
const userSession = () =>
|
||||
createJwt({
|
||||
sub: "1", email: "a@b.test", role: "SUPER_ADMIN", domainId: "d",
|
||||
surface: "user", aud: "lambdamail:user", mfaSatisfied: false, purpose: "session",
|
||||
});
|
||||
|
||||
// The point of the step-up: the password was already proven by the session in
|
||||
// hand, so only the factor the console adds is asked for.
|
||||
it("asks for the second factor and not for the password again", () => {
|
||||
const decision = stepUpNeedsSecondFactor(userSession());
|
||||
expect(decision).toBe(true);
|
||||
});
|
||||
|
||||
it("refuses to step up from a challenge token, which proves only a password", () => {
|
||||
const challenge = createJwt({
|
||||
sub: "1", email: "a@b.test", role: "SUPER_ADMIN", domainId: "d",
|
||||
surface: "user", aud: "lambdamail:user", mfaSatisfied: false, purpose: "mfa_challenge",
|
||||
});
|
||||
expect(stepUpNeedsSecondFactor(challenge)).toBe(false);
|
||||
});
|
||||
|
||||
it("refuses to step up from nothing at all", () => {
|
||||
expect(stepUpNeedsSecondFactor(null)).toBe(false);
|
||||
expect(stepUpNeedsSecondFactor("not-a-token")).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe("every issued session is its own session", () => {
|
||||
const claims = {
|
||||
sub: "1", email: "a@b.test", role: "USER" as const, domainId: "d",
|
||||
surface: "user" as const, aud: "lambdamail:user" as const,
|
||||
mfaSatisfied: false, purpose: "session" as const,
|
||||
};
|
||||
|
||||
/**
|
||||
* Signing in twice in the same second used to mint the identical token.
|
||||
*
|
||||
* Nothing in the payload varied below one second - iat and exp are in
|
||||
* seconds - so two sessions hashed to the same value and the second insert
|
||||
* hit the unique index on web_sessions.refresh_token_hash. The user saw a
|
||||
* 500 from a correct password, which is what a double-clicked sign-in does.
|
||||
*/
|
||||
it("mints a different token for two sign-ins in the same second", () => {
|
||||
expect(createJwt(claims)).not.toBe(createJwt(claims));
|
||||
});
|
||||
|
||||
// The two audiences an admin sign-in issues are recorded as separate rows,
|
||||
// so they must not collide with each other either.
|
||||
it("mints a different token per surface within one sign-in", () => {
|
||||
const admin = createJwt({ ...claims, surface: "admin", aud: "lambdamail:admin" });
|
||||
expect(admin).not.toBe(createJwt(claims));
|
||||
});
|
||||
|
||||
// The identity has to survive verification, or a revoked session could not
|
||||
// be told apart from a reissued one.
|
||||
it("carries a distinct identifier through verification", () => {
|
||||
const first = verifyJwt(createJwt(claims));
|
||||
const second = verifyJwt(createJwt(claims));
|
||||
expect(first?.jti).toBeTruthy();
|
||||
expect(first?.jti).not.toBe(second?.jti);
|
||||
});
|
||||
});
|
||||
|
||||
describe("the guard on each surface", () => {
|
||||
const session = (surface: "user" | "admin", mfaSatisfied: boolean) =>
|
||||
createJwt({
|
||||
sub: "1", email: "a@b.test", role: "SUPER_ADMIN", domainId: "d",
|
||||
surface,
|
||||
aud: surface === "admin" ? "lambdamail:admin" : "lambdamail:user",
|
||||
mfaSatisfied,
|
||||
purpose: "session",
|
||||
});
|
||||
|
||||
// The webmail guard must agree with the login it issues, or a password-only
|
||||
// sign-in would produce a session its own API refuses.
|
||||
it("admits a webmail session that never proved a second factor", () => {
|
||||
const payload = JSON.parse(
|
||||
Buffer.from(session("user", false).split(".")[1], "base64url").toString(),
|
||||
);
|
||||
expect(isSurfaceAuthorized(payload, "user")).toBe(true);
|
||||
});
|
||||
|
||||
it("still refuses the console without one", () => {
|
||||
const payload = JSON.parse(
|
||||
Buffer.from(session("admin", false).split(".")[1], "base64url").toString(),
|
||||
);
|
||||
expect(isSurfaceAuthorized(payload, "admin")).toBe(false);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,61 @@
|
||||
import { verifyJwt } from "./session.js";
|
||||
|
||||
/**
|
||||
* Which surface a login opens, and what crossing between them costs.
|
||||
*
|
||||
* These rules were spread through the login handler, where two of them were
|
||||
* wrong in a way no test could reach:
|
||||
*
|
||||
* - Reading your own mail demanded a second factor. The mailbox is already
|
||||
* behind the account's password; it is the console that needs more.
|
||||
* - A session issued at the admin sign-in carried the admin audience alone, so
|
||||
* leaving the console landed on the webmail's login screen - asking for a
|
||||
* password from somebody who had just proven a password *and* a second
|
||||
* factor. Proving more must never grant less.
|
||||
*/
|
||||
|
||||
export type Surface = "user" | "admin";
|
||||
|
||||
/**
|
||||
* Whether the surface may only open once a second factor has been proven.
|
||||
*
|
||||
* The console can change any mailbox, any domain and the DNS records the mail
|
||||
* flow depends on, which is why PLAN.md section 14.5 makes it unconditional
|
||||
* there. The webmail is one account's own mail, so the password that owns it is
|
||||
* the bar. An account may still enrol a factor and be asked for it on the way
|
||||
* into the console.
|
||||
*/
|
||||
export function secondFactorRequired(surface: Surface): boolean {
|
||||
return surface === "admin";
|
||||
}
|
||||
|
||||
/**
|
||||
* The surfaces a sign-in on `surface` should hand out sessions for.
|
||||
*
|
||||
* An admin sign-in also yields a webmail session because it satisfied strictly
|
||||
* more than the webmail asks for. Issuing both at once is what makes leaving
|
||||
* the console a plain link instead of another sign-in.
|
||||
*/
|
||||
export function surfacesGrantedBy(surface: Surface): Surface[] {
|
||||
return surface === "admin" ? ["admin", "user"] : ["user"];
|
||||
}
|
||||
|
||||
/** Cookies are per surface, so an admin session cannot displace a webmail one. */
|
||||
export function cookieNameFor(surface: Surface): string {
|
||||
return surface === "admin" ? "lm_admin_session" : "lm_user_session";
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether this token is a webmail session that may be stepped up to the
|
||||
* console by presenting a second factor and nothing else.
|
||||
*
|
||||
* A challenge or enrolment token is refused: neither proves more than that a
|
||||
* password was typed, and accepting one here would turn the step-up into a
|
||||
* second way of skipping the factor it exists to collect.
|
||||
*/
|
||||
export function stepUpNeedsSecondFactor(token: string | null | undefined): boolean {
|
||||
if (!token) return false;
|
||||
const payload = verifyJwt(token);
|
||||
if (!payload) return false;
|
||||
return payload.purpose === "session" && payload.aud === "lambdamail:user";
|
||||
}
|
||||
Reference in new issue
Block a user