⚡️(yhub) skip the users/me call for callers without a session cookie

After removing the ForceSessionMiddleware we can detect without calling
the /users/me endpoint if a user is anonymous or not. We now check the
presence of the cookie session to avoid the http call and directly
return an anonymous user.

Author: lunika

CHANGELOG.md

@@ -8,6 +8,7 @@ and this project adheres to
 
 ### Changed
 
+- ⚡️(yhub) skip the `users/me` call for callers without a session cookie
 - 🔧(helm) allow to set the period and failure threshold of probes
 
 ### Fixed

documentation/load-testing.md

@@ -118,7 +118,12 @@ open one socket with a cookie of the manifest and read its logs for the
 store makes the clients anonymous, which looks like success on public
 documents. (In this repository's compose stack, the `Development`
 configuration keeps its sessions in another Redis database than `LoadTest`:
-mint with `REDIS_URL=redis://redis:6379/2` there.)
+mint with `REDIS_URL=redis://redis:6379/2` there.) One symptom has changed
+with the session-cookie fast path: an anonymous client is resolved locally
+by the collaboration server and no longer produces the `users_me:401` calls
+that used to betray a bad manifest in `yhub_backend_request_duration_seconds`
+— check `yhub_auth_duration_seconds{phase="authenticate"}` and the `userid`
+the logs report instead.
 
 ## Where the load comes from
 

src/backend/core/tests/test_settings.py

@@ -5,11 +5,26 @@
 import os
 import tempfile
 
+from django.conf import settings
+
 import pytest
 
 from impress.settings import Base, Production
 
 
+def test_session_cookie_name_matches_yhub_contract():
+    """
+    The yhub collaboration server (`src/yhub-server/src/config.ts`) hardcodes
+    the same session cookie name to decide whether a caller may hold a session
+    at all: a caller whose cookies carry no `docs_sessionid` is resolved to
+    anonymous there without calling `users/me`. Renaming the cookie on the
+    backend side alone would silently degrade every signed-in user to
+    anonymous, so the contract is pinned on both ends.
+    """
+
+    assert settings.SESSION_COOKIE_NAME == "docs_sessionid"
+
+
 def test_invalid_settings_oidc_email_configuration():
     """
     The OIDC_FALLBACK_TO_EMAIL_FOR_IDENTIFICATION and OIDC_ALLOW_DUPLICATE_EMAILS settings

src/yhub-server/README.md

@@ -30,7 +30,9 @@ It is not a fork of yhub — it is a thin wrapper, written in TypeScript under
   below),
 - plugs in an auth plugin that resolves users and per-document access rights
   by calling the Docs Django backend (`/api/v1.0/users/me/` and
-  `/api/v1.0/documents/{id}/`),
+  `/api/v1.0/documents/{id}/`; the user call is skipped for any caller whose
+  `Cookie` header does not carry the backend's `docs_sessionid` cookie —
+  they can only be anonymous),
 - serves every route under the `/collaboration/` prefix
   (`server.apiPrefix`), including the websocket sync route
   `/collaboration/ws/v1/{org}/{docid}`,
@@ -201,6 +203,13 @@ yhub refuses the upgrade of a caller that holds the write but has no identity,
 because attributions carry the userid. Every anonymous edit is therefore
 attributed to that one shared author.
 
+A caller whose `Cookie` header does not carry the backend's session cookie
+(`docs_sessionid`) is anonymous without a roundtrip: the backend only ever
+speaks sessions through that one cookie, so the `users/me` call could only
+answer 401 for them, and the identity is resolved locally instead. Browsers
+still send their other cookies (csrf, consent, …) on that header, which is why
+the check is on the session cookie rather than on the header being present.
+
 ### Origins and cors
 
 `COLLABORATION_SERVER_ORIGIN` is the list of origins a browser may reach this

src/yhub-server/__tests__/backend.spec.ts

@@ -172,6 +172,55 @@ describe('backendFetch', () => {
   });
 });
 
+describe('carriesSessionCookie', () => {
+  // imported lazily per test through `load()` like the other describes, so it
+  // runs after the file-level beforeEach has stubbed the env: a beforeAll on
+  // the describe would import config.ts before the first beforeEach runs, and
+  // only pass while every config read keeps a default
+
+  it('accepts a header carrying only the session cookie', async () => {
+    const { carriesSessionCookie } = await load();
+    expect(carriesSessionCookie('docs_sessionid=abc')).toBe(true);
+  });
+
+  it('accepts the session cookie among others', async () => {
+    const { carriesSessionCookie } = await load();
+    expect(
+      carriesSessionCookie('csrftoken=x; docs_sessionid=abc; consent=y'),
+    ).toBe(true);
+  });
+
+  it('accepts it despite surrounding whitespace around the pair', async () => {
+    // browsers separate cookies with "; " and may pad: the name is trimmed,
+    // though it stays case-sensitive — RFC 6265 cookie names are
+    const { carriesSessionCookie } = await load();
+    expect(carriesSessionCookie('csrftoken=x; docs_sessionid = abc')).toBe(
+      true,
+    );
+  });
+
+  it('is case-sensitive on the cookie name, as RFC 6265 makes them', async () => {
+    const { carriesSessionCookie } = await load();
+    expect(carriesSessionCookie('DOCS_SESSIONID=abc')).toBe(false);
+  });
+
+  it('refuses a header with other cookies but no session one', async () => {
+    const { carriesSessionCookie } = await load();
+    expect(carriesSessionCookie('csrftoken=x; consent=y')).toBe(false);
+  });
+
+  it('refuses lookalike names that are not the session cookie', async () => {
+    const { carriesSessionCookie } = await load();
+    expect(carriesSessionCookie('xdocs_sessionid=abc')).toBe(false);
+    expect(carriesSessionCookie('docs_sessionid_extra=abc')).toBe(false);
+  });
+
+  it('refuses an empty header', async () => {
+    const { carriesSessionCookie } = await load();
+    expect(carriesSessionCookie('')).toBe(false);
+  });
+});
+
 describe('the backend signing key', () => {
   it('publishes no JWK when YHUB_JWT_PRIVATE_KEY is unset', async () => {
     const { backendPublicJwk } = await load();

src/yhub-server/__tests__/server.spec.ts

@@ -204,3 +204,63 @@ describe('the S3 persistence plugin', () => {
     await expect(boot()).rejects.toThrow(/must be http:\/\/ or https:\/\//);
   });
 });
+
+describe('the authenticate cookie gate', () => {
+  // backend.ts is NOT mocked here: the spy on `fetch` observes the real
+  // `backendFetch`, so these pin what the auth plugin wires together — who
+  // is resolved locally, and who costs a backend call. Each test re-boots
+  // through boot(), after the file-level beforeEach has stubbed BASE_ENV.
+  const request = (
+    headers: Record<string, string>,
+    url = '/collaboration/ydoc/v1/docs/00000000-0000-4000-8000-000000000000/',
+  ) => ({
+    getHeader: (name: string) => headers[name] ?? '',
+    getUrl: () => url,
+  });
+
+  const bootGate = async (): Promise<{
+    authenticate: (req: ReturnType<typeof request>) => Promise<unknown>;
+    fetchMock: ReturnType<typeof vi.fn>;
+  }> => {
+    const fetchMock = vi.fn();
+    vi.stubGlobal('fetch', fetchMock);
+    await boot();
+    expect(createYHub.lastConfig.server).not.toBeNull();
+    return {
+      authenticate: createYHub.lastConfig.server.auth.authenticate,
+      fetchMock,
+    };
+  };
+
+  it('resolves a caller holding other cookies but no session one to anonymous, without calling the backend', async () => {
+    const { authenticate, fetchMock } = await bootGate();
+
+    const user = await authenticate(
+      request({ cookie: 'csrftoken=x; consent=y' }),
+    );
+
+    expect(user).toMatchObject({ userid: 'anonymous' });
+    expect(fetchMock).not.toHaveBeenCalled();
+  });
+
+  it('still asks the backend who a caller carrying the session cookie is', async () => {
+    const { authenticate, fetchMock } = await bootGate();
+    fetchMock.mockResolvedValue({
+      ok: true,
+      status: 200,
+      json: async () => ({ id: 'user-1' }),
+    });
+
+    const user = await authenticate(
+      request({ cookie: 'csrftoken=x; docs_sessionid=abc' }),
+    );
+
+    expect(user).toMatchObject({ userid: 'user-1' });
+    expect(fetchMock.mock.calls[0][0]).toBe(
+      'http://app-dev:8000/api/v1.0/users/me/',
+    );
+    expect(fetchMock.mock.calls[0][1].headers).toMatchObject({
+      cookie: 'csrftoken=x; docs_sessionid=abc',
+    });
+  });
+});

src/yhub-server/src/backend.ts

@@ -33,6 +33,7 @@ import {
   BACKEND_TOKEN_LIFETIME_S,
   BACKEND_TOKEN_MARGIN_MS,
   COLLABORATION_BACKEND_BASE_URL,
+  SESSION_COOKIE_NAME,
   Y_PROVIDER_API_KEY,
   YHUB_JWT_PRIVATE_KEY,
 } from './config.js';
@@ -47,6 +48,16 @@ export interface BackendDocument {
   abilities?: DocumentAbilities;
 }
 
+// Whether a caller's `Cookie` header carries the backend's session cookie.
+// The backend only ever speaks sessions through that one cookie, so a header
+// carrying others (csrf, consent, …) but not this one can only resolve to the
+// anonymous identity — the `users/me` call would answer 401 — and
+// `authenticate` answers it as such without the roundtrip.
+export const carriesSessionCookie = (cookieHeader: string): boolean =>
+  cookieHeader
+    .split(';')
+    .some((pair) => pair.split('=')[0].trim() === SESSION_COOKIE_NAME);
+
 const touchLog = logger.child({ module: 'updated-at-notifier' });
 
 const backendSigningKey = YHUB_JWT_PRIVATE_KEY

src/yhub-server/src/config.ts

@@ -58,6 +58,15 @@ export const MIN_MESSAGE_LIFETIME_MS = intEnv(
 );
 export const COLLABORATION_BACKEND_BASE_URL =
   process.env.COLLABORATION_BACKEND_BASE_URL || 'http://app-dev:8000';
+// The name of the backend's session cookie (`SESSION_COOKIE_NAME` on the
+// Django side, `docs_sessionid` there too). It decides whether
+// `authenticate` has to ask the backend who a caller is at all: the backend
+// only ever speaks sessions through that one cookie, so a `Cookie` header
+// carrying others (csrf, consent, …) but not this one can only resolve to the
+// anonymous identity — the `users/me` call would answer 401 — and is answered
+// anonymously without the roundtrip. Not read from the environment: a
+// two-party contract, hardcoded like the audiences.
+export const SESSION_COOKIE_NAME = 'docs_sessionid';
 export const allowedOrigins = (
   process.env.COLLABORATION_SERVER_ORIGIN || 'http://localhost:3000'
 ).split(',');

src/yhub-server/src/server.ts

@@ -12,7 +12,12 @@ import { jwtVerify } from 'jose';
 import { api } from './api.js';
 // outbound calls to the Docs Django backend, and the keys both ends verify with
 import type { BackendDocument, BackendUser } from './backend.js';
-import { JWKS, backendFetch, touchDocument } from './backend.js';
+import {
+  JWKS,
+  backendFetch,
+  carriesSessionCookie,
+  touchDocument,
+} from './backend.js';
 import {
   ANONYMOUS_USERID,
   API_PREFIX,
@@ -192,9 +197,16 @@ const auth = createAuthPlugin<AppAuthInfo>({
     // `Origin` header at all, so on a deployment where the page and /collaboration/ share a
     // host every round would 401 while the PATCH beside it succeeded.
     //
-    // A caller with no cookie at all is anonymous too: the probes arrive that way, and so does
-    // a public document opened in a fresh browser. What they may do is `authorize`'s answer.
-    if (!cookie) {
+    // The backend only ever speaks sessions through its one session cookie
+    // (`docs_sessionid`): a caller whose `Cookie` header does not carry it can
+    // only resolve to the anonymous identity, and is answered as such without
+    // the `users/me` roundtrip. This is not just the cookie-less case — the
+    // probes and a public document in a fresh browser — but every signed-out
+    // tab: since the backend stopped forcing a session onto anonymous users,
+    // they hold no `docs_sessionid` at all, and browsers still send their other
+    // cookies (csrf, consent, …) on this header. What an anonymous caller may
+    // do is `authorize`'s answer, as before.
+    if (!carriesSessionCookie(cookie)) {
       return { userid: ANONYMOUS_USERID, origin, endpoint };
     }
     try {