mirror of
https://github.com/outline/outline.git
synced 2026-08-03 13:27:25 +03:00
* fix: OIDC 502 on sign-in from oversized id_token cookie (#12986) Storing the full OIDC id_token in the `oidcIdToken` cookie inflated the sign-in response headers enough to exceed reverse proxy buffers, causing a 502 Bad Gateway. Store the id_token server-side in Redis keyed by a short session identifier and keep only that identifier in the cookie, preserving spec-compliant RP-initiated logout via `id_token_hint`. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix: Make OIDC logout token persistence best-effort A Redis failure when storing or reading the logout id_token_hint should not block sign-in or logout; fall back to omitting the hint instead. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs * refactor: Extract portable LogoutTokenStore for provider logout hints Move the cookie-referenced, Redis-backed logout token persistence out of the OIDC router into a provider-agnostic `LogoutTokenStore`. Any auth provider that supports provider-initiated logout can now persist and consume a logout hint via `new LogoutTokenStore(providerId)` without duplicating the cookie/Redis/error handling. The OIDC-specific end-session URL construction stays in the router. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
416268b683
commit
533efdfcfe
@@ -185,7 +185,8 @@ if (env.DISCORD_CLIENT_ID && env.DISCORD_CLIENT_SECRET) {
|
||||
}
|
||||
}
|
||||
const user =
|
||||
context.state?.auth?.user ?? (await getUserFromOAuthState(context));
|
||||
context.state?.auth?.user ??
|
||||
(await getUserFromOAuthState(context));
|
||||
|
||||
// if a team can be inferred, we assume the user is only interested in signing into
|
||||
// that team in particular; otherwise, we will do a best effort at finding their account
|
||||
|
||||
@@ -1,3 +1,5 @@
|
||||
import Redis from "@server/storage/redis";
|
||||
import { RedisPrefixHelper } from "@server/utils/RedisPrefixHelper";
|
||||
import { getTestServer } from "@server/test/support";
|
||||
|
||||
const server = getTestServer();
|
||||
@@ -32,10 +34,16 @@ describe("oidc", () => {
|
||||
});
|
||||
|
||||
it("should include the id_token_hint when present", async () => {
|
||||
const sessionId = "test-session-id";
|
||||
await Redis.defaultClient.set(
|
||||
RedisPrefixHelper.getLogoutTokenKey("oidc", sessionId),
|
||||
"fake-id-token"
|
||||
);
|
||||
|
||||
const res = await server.get("/auth/oidc.logout", {
|
||||
redirect: "manual",
|
||||
headers: {
|
||||
Cookie: "oidcIdToken=fake-id-token",
|
||||
Cookie: `oidcSession=${sessionId}`,
|
||||
},
|
||||
});
|
||||
expect(res.status).toEqual(302);
|
||||
@@ -44,8 +52,14 @@ describe("oidc", () => {
|
||||
"fake-id-token"
|
||||
);
|
||||
expect(res.headers.get("set-cookie")).toContain(
|
||||
"oidcIdToken=; path=/auth/oidc.logout;"
|
||||
"oidcSession=; path=/auth/oidc.logout;"
|
||||
);
|
||||
// The token is consumed from the server-side store on logout.
|
||||
expect(
|
||||
await Redis.defaultClient.get(
|
||||
RedisPrefixHelper.getLogoutTokenKey("oidc", sessionId)
|
||||
)
|
||||
).toBeNull();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1,11 +1,10 @@
|
||||
import passport from "@outlinewiki/koa-passport";
|
||||
import { addMonths, subMinutes } from "date-fns";
|
||||
import JWT from "jsonwebtoken";
|
||||
import type { Context, Request } from "koa";
|
||||
import type Router from "koa-router";
|
||||
import { get } from "es-toolkit/compat";
|
||||
import { toError } from "@shared/utils/error";
|
||||
import { getCookieDomain, slugifyDomain } from "@shared/utils/domains";
|
||||
import { slugifyDomain } from "@shared/utils/domains";
|
||||
import { parseEmail } from "@shared/utils/email";
|
||||
import { isBase64Url } from "@shared/utils/urls";
|
||||
import accountProvisioner from "@server/commands/accountProvisioner";
|
||||
@@ -18,6 +17,7 @@ import passportMiddleware from "@server/middlewares/passport";
|
||||
import type { User } from "@server/models";
|
||||
import { AuthenticationProvider } from "@server/models";
|
||||
import type { AuthenticationResult } from "@server/types";
|
||||
import { LogoutTokenStore } from "@server/utils/LogoutTokenStore";
|
||||
import {
|
||||
StateStore,
|
||||
getTeamFromContext,
|
||||
@@ -32,8 +32,6 @@ import env from "../env";
|
||||
import { OIDCStrategy } from "./OIDCStrategy";
|
||||
import { createContext } from "@server/context";
|
||||
|
||||
const OIDC_LOGOUT_PATH = "/auth/oidc.logout";
|
||||
|
||||
export interface OIDCEndpoints {
|
||||
authorizationURL: string;
|
||||
tokenURL: string;
|
||||
@@ -50,6 +48,7 @@ export function createOIDCRouter(
|
||||
endpoints: OIDCEndpoints
|
||||
): void {
|
||||
const scopes = env.OIDC_SCOPES.split(" ");
|
||||
const logoutTokens = new LogoutTokenStore(config.id);
|
||||
|
||||
passport.use(
|
||||
config.id,
|
||||
@@ -142,7 +141,8 @@ export function createOIDCRouter(
|
||||
const team = await getTeamFromContext(context);
|
||||
const client = getClientFromOAuthState(context);
|
||||
const user =
|
||||
context.state?.auth?.user ?? (await getUserFromOAuthState(context));
|
||||
context.state?.auth?.user ??
|
||||
(await getUserFromOAuthState(context));
|
||||
const { domain } = parseEmail(email);
|
||||
|
||||
// Only a single OIDC provider is supported – find the existing, if any.
|
||||
@@ -242,17 +242,7 @@ export function createOIDCRouter(
|
||||
// the `id_token_hint`, allowing the provider to scope the logout to
|
||||
// this session rather than terminating its global SSO session.
|
||||
if (endpoints.logoutURL && params.id_token) {
|
||||
context.cookies.set("oidcIdToken", params.id_token, {
|
||||
httpOnly: true,
|
||||
sameSite: "lax",
|
||||
secure: env.isProduction,
|
||||
path: OIDC_LOGOUT_PATH,
|
||||
domain: getCookieDomain(
|
||||
context.request.hostname,
|
||||
env.isCloudHosted
|
||||
),
|
||||
expires: addMonths(new Date(), 3),
|
||||
});
|
||||
await logoutTokens.persist(context, params.id_token);
|
||||
}
|
||||
|
||||
return done(null, result.user, { ...result, client });
|
||||
@@ -273,18 +263,8 @@ export function createOIDCRouter(
|
||||
// ended so the provider can scope the logout and skip a confirmation prompt,
|
||||
// while `post_logout_redirect_uri` returns the user to Outline afterwards.
|
||||
// https://openid.net/specs/openid-connect-rpinitiated-1_0.html
|
||||
router.get(`${config.id}.logout`, (ctx: Context) => {
|
||||
const idToken = ctx.cookies.get("oidcIdToken");
|
||||
|
||||
// Always discard our copy of the id_token, regardless of where we redirect.
|
||||
ctx.cookies.set("oidcIdToken", "", {
|
||||
httpOnly: true,
|
||||
sameSite: "lax",
|
||||
secure: env.isProduction,
|
||||
path: OIDC_LOGOUT_PATH,
|
||||
domain: getCookieDomain(ctx.request.hostname, env.isCloudHosted),
|
||||
expires: subMinutes(new Date(), 1),
|
||||
});
|
||||
router.get(`${config.id}.logout`, async (ctx: Context) => {
|
||||
const idToken = await logoutTokens.consume(ctx);
|
||||
|
||||
if (!endpoints.logoutURL) {
|
||||
return ctx.redirect("/");
|
||||
|
||||
@@ -0,0 +1,72 @@
|
||||
import type { Context } from "koa";
|
||||
import Redis from "@server/storage/redis";
|
||||
import { LogoutTokenStore } from "./LogoutTokenStore";
|
||||
import { RedisPrefixHelper } from "./RedisPrefixHelper";
|
||||
|
||||
/**
|
||||
* Minimal Koa context stub exposing only the cookie jar and hostname the store
|
||||
* relies on.
|
||||
*/
|
||||
function buildContext(cookies: Record<string, string> = {}) {
|
||||
const jar = new Map(Object.entries(cookies));
|
||||
return {
|
||||
request: { hostname: "localhost" },
|
||||
cookies: {
|
||||
get: (name: string) => jar.get(name),
|
||||
set: (name: string, value: string) => {
|
||||
if (value) {
|
||||
jar.set(name, value);
|
||||
} else {
|
||||
jar.delete(name);
|
||||
}
|
||||
},
|
||||
},
|
||||
} as unknown as Context;
|
||||
}
|
||||
|
||||
describe("LogoutTokenStore", () => {
|
||||
const store = new LogoutTokenStore("oidc");
|
||||
|
||||
it("persists a token behind a session cookie and consumes it once", async () => {
|
||||
const signInCtx = buildContext();
|
||||
await store.persist(signInCtx, "the-token");
|
||||
|
||||
const sessionId = signInCtx.cookies.get("oidcSession");
|
||||
expect(sessionId).toBeTruthy();
|
||||
expect(
|
||||
await Redis.defaultClient.get(
|
||||
RedisPrefixHelper.getLogoutTokenKey("oidc", sessionId!)
|
||||
)
|
||||
).toEqual("the-token");
|
||||
|
||||
const logoutCtx = buildContext({ oidcSession: sessionId! });
|
||||
expect(await store.consume(logoutCtx)).toEqual("the-token");
|
||||
|
||||
// Consuming clears both the cookie and the server-side token.
|
||||
expect(logoutCtx.cookies.get("oidcSession")).toBeUndefined();
|
||||
expect(
|
||||
await Redis.defaultClient.get(
|
||||
RedisPrefixHelper.getLogoutTokenKey("oidc", sessionId!)
|
||||
)
|
||||
).toBeNull();
|
||||
});
|
||||
|
||||
it("returns null when there is no session cookie", async () => {
|
||||
expect(await store.consume(buildContext())).toBeNull();
|
||||
});
|
||||
|
||||
it("namespaces the cookie and key by provider", async () => {
|
||||
const samlStore = new LogoutTokenStore("saml");
|
||||
const ctx = buildContext();
|
||||
await samlStore.persist(ctx, "saml-token");
|
||||
|
||||
const sessionId = ctx.cookies.get("samlSession");
|
||||
expect(sessionId).toBeTruthy();
|
||||
expect(ctx.cookies.get("oidcSession")).toBeUndefined();
|
||||
expect(
|
||||
await Redis.defaultClient.get(
|
||||
RedisPrefixHelper.getLogoutTokenKey("saml", sessionId!)
|
||||
)
|
||||
).toEqual("saml-token");
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,118 @@
|
||||
import crypto from "node:crypto";
|
||||
import { addSeconds, subMinutes } from "date-fns";
|
||||
import type { Context } from "koa";
|
||||
import { toError } from "@shared/utils/error";
|
||||
import { getCookieDomain } from "@shared/utils/domains";
|
||||
import env from "@server/env";
|
||||
import Logger from "@server/logging/Logger";
|
||||
import Redis from "@server/storage/redis";
|
||||
import { RedisPrefixHelper } from "./RedisPrefixHelper";
|
||||
|
||||
// ~3 months. Matches the default cookie lifetime so the hint is available for
|
||||
// as long as the cookie referencing it can be sent back.
|
||||
const DEFAULT_TTL_SECONDS = 90 * 24 * 60 * 60;
|
||||
|
||||
/**
|
||||
* Persists a per-session logout hint (e.g. an OIDC `id_token`) for an auth
|
||||
* provider that supports provider-initiated logout.
|
||||
*
|
||||
* The token is stored server-side keyed by a short random session identifier;
|
||||
* only that identifier is placed in a cookie. Tokens can be large and storing
|
||||
* one directly in the cookie inflates response headers enough to exceed reverse
|
||||
* proxy buffers, resulting in a 502 on sign-in. Both operations are best-effort:
|
||||
* a Redis failure never blocks sign-in or logout, it only causes the hint to be
|
||||
* omitted.
|
||||
*/
|
||||
export class LogoutTokenStore {
|
||||
/**
|
||||
* @param provider The auth provider id, used to namespace the cookie, Redis
|
||||
* key, and logout path (e.g. "oidc").
|
||||
* @param ttlSeconds How long the token is retained, also the cookie lifetime.
|
||||
*/
|
||||
constructor(
|
||||
private provider: string,
|
||||
private ttlSeconds: number = DEFAULT_TTL_SECONDS
|
||||
) {}
|
||||
|
||||
/**
|
||||
* Persists a logout token for the current session and references it from a
|
||||
* scoped cookie. Best-effort — never throws.
|
||||
*
|
||||
* @param ctx The Koa context of the sign-in request.
|
||||
* @param token The provider token to persist as a later logout hint.
|
||||
*/
|
||||
public async persist(ctx: Context, token: string): Promise<void> {
|
||||
try {
|
||||
const sessionId = crypto.randomBytes(16).toString("hex");
|
||||
await Redis.defaultClient.set(
|
||||
RedisPrefixHelper.getLogoutTokenKey(this.provider, sessionId),
|
||||
token,
|
||||
"EX",
|
||||
this.ttlSeconds
|
||||
);
|
||||
ctx.cookies.set(
|
||||
this.cookieName,
|
||||
sessionId,
|
||||
this.cookieOptions(ctx, addSeconds(new Date(), this.ttlSeconds))
|
||||
);
|
||||
} catch (err) {
|
||||
Logger.warn("Failed to persist logout token", {
|
||||
provider: this.provider,
|
||||
error: toError(err).message,
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Consumes the logout token for the current session and clears its cookie.
|
||||
* Best-effort — never throws.
|
||||
*
|
||||
* @param ctx The Koa context of the logout request.
|
||||
* @returns The persisted token, or null if absent or unavailable.
|
||||
*/
|
||||
public async consume(ctx: Context): Promise<string | null> {
|
||||
const sessionId = ctx.cookies.get(this.cookieName);
|
||||
let token: string | null = null;
|
||||
|
||||
if (sessionId) {
|
||||
try {
|
||||
token = await Redis.defaultClient.getdel(
|
||||
RedisPrefixHelper.getLogoutTokenKey(this.provider, sessionId)
|
||||
);
|
||||
} catch (err) {
|
||||
Logger.warn("Failed to read logout token", {
|
||||
provider: this.provider,
|
||||
error: toError(err).message,
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
// Always discard the session identifier, regardless of the result.
|
||||
ctx.cookies.set(
|
||||
this.cookieName,
|
||||
"",
|
||||
this.cookieOptions(ctx, subMinutes(new Date(), 1))
|
||||
);
|
||||
|
||||
return token;
|
||||
}
|
||||
|
||||
private get cookieName(): string {
|
||||
return `${this.provider}Session`;
|
||||
}
|
||||
|
||||
private get cookiePath(): string {
|
||||
return `/auth/${this.provider}.logout`;
|
||||
}
|
||||
|
||||
private cookieOptions(ctx: Context, expires: Date) {
|
||||
return {
|
||||
httpOnly: true,
|
||||
sameSite: "lax" as const,
|
||||
secure: env.isProduction,
|
||||
path: this.cookiePath,
|
||||
domain: getCookieDomain(ctx.request.hostname, env.isCloudHosted),
|
||||
expires,
|
||||
};
|
||||
}
|
||||
}
|
||||
@@ -69,4 +69,16 @@ export class RedisPrefixHelper {
|
||||
) {
|
||||
return `count:${modelName}:${relationName}:${id}`;
|
||||
}
|
||||
|
||||
/**
|
||||
* Gets key for storing an auth provider's token used as a logout hint during
|
||||
* provider-initiated logout, referenced by a short session identifier.
|
||||
*
|
||||
* @param provider The auth provider id (e.g. "oidc").
|
||||
* @param sessionId The logout session identifier to generate a key for.
|
||||
* @returns the cache key string.
|
||||
*/
|
||||
public static getLogoutTokenKey(provider: string, sessionId: string) {
|
||||
return `auth:logout:${provider}:${sessionId}`;
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user