fix: leave out the id token when sign-in cookies would overflow nginx's header buffer #1096

app/api/auth/sso/complete/route.ts

@@ -95,7 +95,7 @@ export async function POST(request: NextRequest) {
       } else {
         cookieStore.delete(serverCookieName);
       }
-      storeIdToken(cookieStore, slot, tokens.id_token, request.nextUrl.basePath);
+      storeIdToken(cookieStore, slot, tokens.id_token, request.nextUrl.basePath, [tokens.refresh_token, pendingServerId]);
     }
 
     // Delete pending cookie

app/api/auth/token/route.ts

@@ -90,21 +90,23 @@ type CookieStore = Awaited<ReturnType<typeof cookies>>;
  *
  * Scoped to the token's own lifetime - once it expires the cookie is worthless
  * and should not linger. A token too large to store is simply not cached.
+ * Returns the cookie value written, or null.
  */
 function cacheAccessToken(
   cookieStore: CookieStore,
   slot: number,
   accessToken: string,
   expiresIn: number,
-): void {
+): string | null {
   const name = accessTokenCookieName(slot);
   const value = encodeCachedAccessToken(accessToken, expiresIn);
   if (!value) {
     // Oversized token: drop any stale entry rather than leaving a mismatch.
     cookieStore.delete(name);
-    return;
+    return null;
   }
   cookieStore.set(name, value, { ...getCookieOptions(), maxAge: expiresIn });
+  return value;
 }
 
 export async function POST(request: NextRequest) {
@@ -134,8 +136,8 @@ export async function POST(request: NextRequest) {
       const cookieName = refreshTokenCookieName(slot);
       cookieStore.set(cookieName, tokens.refresh_token, getCookieOptions());
     }
-    cacheAccessToken(cookieStore, slot, tokens.access_token, tokens.expires_in || 3600);
-    storeIdToken(cookieStore, slot, tokens.id_token, request.nextUrl.basePath);
+    const cachedAccessToken = cacheAccessToken(cookieStore, slot, tokens.access_token, tokens.expires_in || 3600);
+    storeIdToken(cookieStore, slot, tokens.id_token, request.nextUrl.basePath, [tokens.refresh_token, cachedAccessToken, serverId]);
     // Persist which server entry minted this refresh token so the PUT/DELETE
     // handlers can route the refresh/revocation calls to the right token
     // endpoint without the client having to track it across page loads.
@@ -234,11 +236,11 @@ export async function PUT(request: NextRequest) {
     }
 
     const expiresIn = tokens.expires_in || 3600;
-    cacheAccessToken(cookieStore, slot, tokens.access_token, expiresIn);
+    const cachedAccessToken = cacheAccessToken(cookieStore, slot, tokens.access_token, expiresIn);
     // Providers may reissue the id token on refresh. Only a slot signed in
     // through the provider keeps one; a password login's slot gains none here.
     if (typeof tokens.id_token === 'string' && cookieStore.get(idTokenCookieName(slot))) {
-      storeIdToken(cookieStore, slot, tokens.id_token, request.nextUrl.basePath);
+      storeIdToken(cookieStore, slot, tokens.id_token, request.nextUrl.basePath, [tokens.refresh_token, cachedAccessToken]);
     }
 
     return NextResponse.json({

lib/__tests__/oauth-token-route.test.ts

@@ -326,6 +326,34 @@ describe('oauth token route - sign-in keeps the id token (#905)', () => {
 
     expect(cookieStore.get('jmap_idt')).toBeUndefined();
   });
+
+  // Sizes of a stock Keycloak realm's tokens.
+  it('stores the id token beside tokens of a typical size', async () => {
+    exchangeCodeForTokens.mockResolvedValue({
+      access_token: 'a'.repeat(1130), expires_in: 300, refresh_token: 'r'.repeat(650), id_token: 'i'.repeat(1080),
+    });
+
+    await callRoute('POST', {}, { code: 'c', code_verifier: 'v', redirect_uri: 'https://mail.example.com/en/auth/callback' });
+
+    expect(cookieStore.get('jmap_idt')?.value).toBe('i'.repeat(1080));
+  });
+
+  it('leaves the id token out when the response would outgrow a proxy header buffer (#1096)', async () => {
+    // A Keycloak realm with roles and a groups claim: together the three
+    // cookies exceed nginx's default 4 KB proxy_buffer_size, and the
+    // sign-in failed with 502.
+    exchangeCodeForTokens.mockResolvedValue({
+      access_token: 'a'.repeat(1700), expires_in: 300, refresh_token: 'r'.repeat(650), id_token: 'i'.repeat(1300),
+    });
+    cookieStore.set('jmap_idt', 'stale');
+
+    const { status } = await callRoute('POST', {}, { code: 'c', code_verifier: 'v', redirect_uri: 'https://mail.example.com/en/auth/callback' });
+
+    expect(status).toBe(200);
+    expect(cookieStore.get('jmap_idt')).toBeUndefined();
+    expect(cookieStore.get('jmap_rt')?.value).toBe('r'.repeat(650));
+    expect(cookieStore.get('jmap_at')?.value).toMatch(/^\d+\.a+$/);
+  });
 });
 
 describe('oauth token route - refresh keeps the id token current (#905)', () => {
@@ -352,6 +380,23 @@ describe('oauth token route - refresh keeps the id token current (#905)', () =>
     expect(cookieStore.get('jmap_idt')?.value).toBe(ID_TOKEN);
   });
 
+  it('forgets the id token when a reissued one would outgrow a proxy header buffer (#1096)', async () => {
+    cookieStore.set('jmap_rt', 'rt');
+    cookieStore.set('jmap_idt', ID_TOKEN);
+    fetchMock.mockResolvedValue({
+      ok: true,
+      json: async () => ({
+        access_token: 'a'.repeat(1700), expires_in: 300, refresh_token: 'r'.repeat(650), id_token: 'i'.repeat(1300),
+      }),
+    });
+
+    const { status } = await callPut({ force: 'true' });
+
+    expect(status).toBe(200);
+    expect(cookieStore.get('jmap_idt')).toBeUndefined();
+    expect(cookieStore.get('jmap_rt')?.value).toBe('r'.repeat(650));
+  });
+
   it('does not start keeping an id token for a password sign-in', async () => {
     cookieStore.set('jmap_rt', 'rt');
     fetchMock.mockResolvedValue({ ok: true, json: async () => ({ access_token: 'at', expires_in: 300, id_token: ID_TOKEN }) });

lib/oauth/end-session.ts

@@ -16,11 +16,25 @@ import { getCookieOptions } from '@/lib/oauth/cookie-config';
 export const ID_TOKEN_COOKIE = 'jmap_idt';
 
 /**
- * Upper bound for a stored id token. Browsers cap a cookie at ~4096 bytes
- * including name and attributes; a larger token is not stored, and sign-out
- * falls back to identifying the client by `client_id` alone.
+ * Most bytes the Set-Cookie headers of a response may take for it to also
+ * carry the id token. nginx holds an upstream's response headers in
+ * `proxy_buffer_size`, one memory page (4 KB) unless raised, as does
+ * ingress-nginx, and answers 502 "upstream sent too big header" when they
+ * do not fit. The route's other headers take about 300 bytes; the refresh
+ * and access-token cookies beside the id token take 2-3 KB with a provider
+ * that issues large JWTs such as Keycloak. Adding the id token pushed such
+ * sign-ins over the limit (#1096), so it is only stored when it fits.
+ * Otherwise sign-out identifies the client by `client_id` alone. This also
+ * keeps the id-token cookie under the browsers' ~4096-byte cookie cap.
  */
-const MAX_ID_TOKEN_COOKIE_BYTES = 3500;
+const MAX_SET_COOKIE_BYTES = 3500;
+
+/**
+ * A Set-Cookie header's size beyond the cookie value: header and cookie
+ * name, attributes (path, expiry, flags) and line break. About 120-135 bytes
+ * for these cookies, rounded up.
+ */
+const SET_COOKIE_OVERHEAD_BYTES = 150;
 
 /** Get the id-token cookie name for a given account slot. Slot 0 uses the bare name. */
 export function idTokenCookieName(slot: number): string {
@@ -42,16 +56,26 @@ interface CookieWriter {
   delete(options: { name: string; path: string }): unknown;
 }
 
-/** Remember the slot's id token for sign-out, or forget a stale one. */
+function setCookieBytes(values: ReadonlyArray<string | null | undefined>): number {
+  return values.reduce((sum, value) => (value ? sum + value.length + SET_COOKIE_OVERHEAD_BYTES : sum), 0);
+}
+
+/**
+ * Remember the slot's id token for sign-out, or forget a stale one.
+ * `otherCookieValues` are the values of the other cookies the same response
+ * sets; the id token is only stored when all of them fit the response's
+ * header budget.
+ */
 export function storeIdToken(
   cookieStore: CookieWriter,
   slot: number,
   idToken: unknown,
   basePath: string | undefined,
+  otherCookieValues: ReadonlyArray<string | null | undefined> = [],
 ): void {
   const name = idTokenCookieName(slot);
   const path = idTokenCookiePath(basePath);
-  if (typeof idToken !== 'string' || !idToken || idToken.length > MAX_ID_TOKEN_COOKIE_BYTES) {
+  if (typeof idToken !== 'string' || !idToken || setCookieBytes([...otherCookieValues, idToken]) > MAX_SET_COOKIE_BYTES) {
     cookieStore.delete({ name, path });
     return;
   }