mptyl/ThothII
1
0
Fork 0
Code Issues 25 Pull Requests Actions Packages Projects Releases 1 Wiki Activity

fix(auth): harden OIDC browser transactions

Browse Source
This commit is contained in:
marcopan
2026-08-17 06:18:49 +02:00
parent 4fe51cbeb1
commit 8573500121
9 changed files with 384 additions and 59 deletions
Download Patch File Download Diff File Expand all files Collapse all files
backend/test/auth-routes-oidc.test.ts
+130 -21
View File
@@ -11,8 +11,25 @@ const issuer = "https://issuer.example.test";
const state = "s".repeat(43);
const nonce = "n".repeat(43);
const verifier = "v".repeat(43);
const transactionCookieName = "__Host-thothii_oidc_tx";
const createdApps: Array<ReturnType<typeof Fastify>> = [];
function setCookieHeaders(response: { headers: Record<string, string | string[] | undefined> }): string[] {
const header = response.headers["set-cookie"];
return header === undefined ? [] : Array.isArray(header) ? header : [header];
}
function transactionCookie(response: { headers: Record<string, string | string[] | undefined> }): string | undefined {
return setCookieHeaders(response)
.find((header) => header.startsWith(`${transactionCookieName}=`))
?.split(";", 1)[0];
}
function expectTransactionCleared(response: { headers: Record<string, string | string[] | undefined> }): void {
expect(setCookieHeaders(response).some((header) =>
header.startsWith(`${transactionCookieName}=`) && header.includes("Max-Age=0"))).toBe(true);
}
function config(overrides: Partial<LoadedAuthConfig["value"]> = {}): LoadedAuthConfig {
return {
revision,
@@ -40,6 +57,7 @@ function stateRecord(extra: Partial<OidcStateRecord> = {}): OidcStateRecord {
return {
version: 1, nonce, codeVerifier: verifier, returnTo: "/",
authConfigRevision: revision, issuer,
browserTransactionDigest: "b".repeat(64),
createdAt: "2030-01-01T00:00:00.000Z", expiresAt: "2030-01-01T00:10:00.000Z",
...extra,
} as OidcStateRecord;
@@ -56,6 +74,7 @@ function fixture(options: {
let storedState: OidcStateRecord | undefined;
const stateInputs: Array<Record<string, unknown>> = [];
const creates: Array<Record<string, unknown>> = [];
const createTimes: Array<Date | undefined> = [];
const callbacks: URL[] = [];
const protocol: OidcProtocol = {
authorizationUrl: async ({ state: received, nonce: receivedNonce, codeVerifier }) => {
@@ -83,6 +102,8 @@ function fixture(options: {
authConfigRevision: input.authConfigRevision as string,
issuer: input.issuer as string,
});
(record as OidcStateRecord & { browserTransactionDigest: string }).browserTransactionDigest =
input.browserTransactionDigest as string;
if (options.stateReturnTo) (record as { returnTo: string }).returnTo = options.stateReturnTo;
storedState = record;
return { state, record: storedState };
@@ -93,8 +114,9 @@ function fixture(options: {
storedState = undefined;
return consumed;
},
create: async (input: Record<string, unknown>) => {
create: async (input: Record<string, unknown>, now?: Date) => {
creates.push(input);
createTimes.push(now);
return { token: "opaque-session-token", csrfToken: "c".repeat(43), record: {} };
},
} as unknown as AuthSessionStore;
@@ -111,7 +133,7 @@ function fixture(options: {
});
createdApps.push(app);
return {
app, creates, callbacks, stateInputs,
app, creates, createTimes, callbacks, stateInputs,
setConfig(next: LoadedAuthConfig) { loaded = next; },
setProtocolAvailable(available: boolean) { protocolAvailable = available; },
stateWasConsumed: () => storedState === undefined,
@@ -122,23 +144,50 @@ afterEach(async () => {
await Promise.all(createdApps.splice(0).map((app) => app.close()));
});
async function beginOidcLogin(subject: ReturnType<typeof fixture>) {
const response = await subject.app.inject({ method: "GET", url: "/auth/oidc/login" });
return { response, cookie: transactionCookie(response) };
}
async function finishOidcLogin(
subject: ReturnType<typeof fixture>,
cookie: string | undefined,
query = `code=good&state=${state}`,
) {
return await subject.app.inject({
method: "GET",
url: `/auth/oidc/callback?${query}`,
...(cookie ? { headers: { cookie } } : {}),
});
}
test("creates digest-only bound state, maps exact groups, creates a cookie session, and redirects safely", async () => {
const subject = fixture();
const start = await subject.app.inject({ method: "GET", url: "/auth/oidc/login" });
const { response: start, cookie } = await beginOidcLogin(subject);
expect(start.statusCode).toBe(302);
expect(new URL(start.headers.location ?? "").searchParams.get("state")).toBe(state);
expect(subject.stateInputs[0]).toMatchObject({ returnTo: "/", authConfigRevision: revision, issuer });
expect(subject.stateInputs[0]).toMatchObject({
returnTo: "/", authConfigRevision: revision, issuer,
browserTransactionDigest: expect.stringMatching(/^[a-f0-9]{64}$/),
});
expect(cookie).toMatch(new RegExp(`^${transactionCookieName}=[A-Za-z0-9_-]{43}$`));
expect(setCookieHeaders(start).join("\n")).toContain("HttpOnly");
expect(setCookieHeaders(start).join("\n")).toContain("SameSite=Lax");
expect(setCookieHeaders(start).join("\n")).toContain("Secure");
expect(setCookieHeaders(start).join("\n")).toContain("Path=/");
expect(JSON.stringify(subject.stateInputs)).not.toContain(cookie?.split("=", 2)[1] ?? "missing-cookie");
const callback = await subject.app.inject({
method: "GET",
url: `/auth/oidc/callback?code=good&state=${state}`,
headers: { host: "attacker.example.test" },
headers: { host: "attacker.example.test", cookie: cookie ?? "" },
});
expect(callback.statusCode).toBe(302);
expect(callback.headers.location).toBe("/");
expect(callback.headers["set-cookie"]).toContain("HttpOnly");
expect(callback.headers["set-cookie"]).toContain("SameSite=Lax");
expect(callback.headers["set-cookie"]).toContain("Secure");
expect(setCookieHeaders(callback).join("\n")).toContain("HttpOnly");
expect(setCookieHeaders(callback).join("\n")).toContain("SameSite=Lax");
expect(setCookieHeaders(callback).join("\n")).toContain("Secure");
expectTransactionCleared(callback);
expect(subject.callbacks[0]?.href).toBe(`https://thothii.example.test/api/auth/oidc/callback?code=good&state=${state}`);
expect(subject.creates).toHaveLength(1);
expect(subject.creates[0]).toMatchObject({
@@ -156,27 +205,28 @@ test("creates digest-only bound state, maps exact groups, creates a cookie sessi
test("consumes state on callback failure and refuses replay", async () => {
const subject = fixture({ callbackFailure: true });
await subject.app.inject({ method: "GET", url: "/auth/oidc/login" });
const failed = await subject.app.inject({ method: "GET", url: `/auth/oidc/callback?code=good&state=${state}` });
const { cookie } = await beginOidcLogin(subject);
const failed = await finishOidcLogin(subject, cookie);
expect(failed.statusCode).toBe(401);
const replay = await subject.app.inject({ method: "GET", url: `/auth/oidc/callback?code=good&state=${state}` });
expectTransactionCleared(failed);
const replay = await finishOidcLogin(subject, cookie);
expect(replay.statusCode).toBe(401);
expect(subject.callbacks).toHaveLength(1);
});
test("consumes state when the protocol becomes unavailable before callback", async () => {
const subject = fixture();
await subject.app.inject({ method: "GET", url: "/auth/oidc/login" });
const { cookie } = await beginOidcLogin(subject);
subject.setProtocolAvailable(false);
const failed = await subject.app.inject({ method: "GET", url: `/auth/oidc/callback?code=good&state=${state}` });
const failed = await finishOidcLogin(subject, cookie);
expect(failed.statusCode).toBe(401);
expect(subject.stateWasConsumed()).toBe(true);
});
test("rejects a consumed state with a non-root return target", async () => {
const subject = fixture({ stateReturnTo: "https://attacker.example.test" });
await subject.app.inject({ method: "GET", url: "/auth/oidc/login" });
const callback = await subject.app.inject({ method: "GET", url: `/auth/oidc/callback?code=good&state=${state}` });
const { cookie } = await beginOidcLogin(subject);
const callback = await finishOidcLogin(subject, cookie);
expect(callback.statusCode).toBe(401);
expect(subject.callbacks).toEqual([]);
expect(subject.creates).toEqual([]);
@@ -184,22 +234,22 @@ test("rejects a consumed state with a non-root return target", async () => {
test("rejects an OIDC state when its configuration revision changes before callback", async () => {
const subject = fixture();
await subject.app.inject({ method: "GET", url: "/auth/oidc/login" });
const { cookie } = await beginOidcLogin(subject);
subject.setConfig({ ...config(), revision: "b".repeat(64) });
const callback = await subject.app.inject({ method: "GET", url: `/auth/oidc/callback?code=good&state=${state}` });
const callback = await finishOidcLogin(subject, cookie);
expect(callback.statusCode).toBe(401);
expect(subject.callbacks).toEqual([]);
});
test("rejects an OIDC state when its issuer changes before callback", async () => {
const subject = fixture();
await subject.app.inject({ method: "GET", url: "/auth/oidc/login" });
const { cookie } = await beginOidcLogin(subject);
const previous = config();
subject.setConfig({
...previous,
value: { ...previous.value, oidc: { ...previous.value.oidc, issuer: "https://other.example.test" } },
} as LoadedAuthConfig);
const callback = await subject.app.inject({ method: "GET", url: `/auth/oidc/callback?code=good&state=${state}` });
const callback = await finishOidcLogin(subject, cookie);
expect(callback.statusCode).toBe(401);
expect(subject.callbacks).toEqual([]);
});
@@ -208,8 +258,67 @@ test("creates an authenticated but forbidden principal for extra unmapped groups
const subject = fixture({ identity: {
issuer, subject: "user-123", groups: ["Unmapped"], tokenExpiresAt: new Date(Date.now() + 60_000),
} });
await subject.app.inject({ method: "GET", url: "/auth/oidc/login" });
const callback = await subject.app.inject({ method: "GET", url: `/auth/oidc/callback?code=good&state=${state}` });
const { cookie } = await beginOidcLogin(subject);
const callback = await finishOidcLogin(subject, cookie);
expect(callback.statusCode).toBe(302);
expect(subject.creates[0]).toMatchObject({ principal: { roles: [], permissions: [], isAdmin: false } });
});
test("rejects a callback from a different browser and clears the transaction cookie", async () => {
const subject = fixture();
const { cookie } = await beginOidcLogin(subject);
expect(cookie).toBeDefined();
const failed = await finishOidcLogin(subject, `${transactionCookieName}=${"x".repeat(43)}`);
expect(failed.statusCode).toBe(401);
expectTransactionCleared(failed);
expect(subject.stateWasConsumed()).toBe(true);
expect(subject.creates).toEqual([]);
const replay = await finishOidcLogin(subject, cookie);
expect(replay.statusCode).toBe(401);
expect(subject.creates).toEqual([]);
});
test.each([
["unknown", `code=good&state=${state}&unexpected=value`],
["duplicate", `code=good&code=other&state=${state}`],
["malformed", `code=good&state=${state}&error_description=%00bad`],
])("burns canonical state before rejecting %s callback parameters", async (_label, query) => {
const subject = fixture();
const { cookie } = await beginOidcLogin(subject);
const failed = await finishOidcLogin(subject, cookie, query);
expect(failed.statusCode).toBe(401);
expectTransactionCleared(failed);
expect(subject.stateWasConsumed()).toBe(true);
expect(subject.callbacks).toEqual([]);
const replay = await finishOidcLogin(subject, cookie);
expect(replay.statusCode).toBe(401);
expect(subject.callbacks).toEqual([]);
});
test("rejects an empty direct groups claim without creating a session cookie", async () => {
const subject = fixture({ identity: {
issuer, subject: "user-123", groups: [], tokenExpiresAt: new Date(Date.now() + 60_000),
} });
const { cookie } = await beginOidcLogin(subject);
const callback = await finishOidcLogin(subject, cookie);
expect(callback.statusCode).toBe(401);
expectTransactionCleared(callback);
expect(subject.creates).toEqual([]);
expect(setCookieHeaders(callback).join("\n")).not.toContain("opaque-session-token");
});
test("uses one captured instant for token TTL derivation and session creation", async () => {
const tokenExpiresAt = new Date(Date.now() + 120_000);
const subject = fixture({ identity: {
issuer, subject: "user-123", groups: ["Users"], tokenExpiresAt,
} });
const { cookie } = await beginOidcLogin(subject);
const callback = await finishOidcLogin(subject, cookie);
expect(callback.statusCode).toBe(302);
expect(subject.createTimes[0]).toBeInstanceOf(Date);
expect((subject.createTimes[0] as Date).getTime() + (subject.creates[0]?.absoluteTtlMs as number))
.toBe(tokenExpiresAt.getTime());
});