diff --git a/src/node/hooks/express/openapi-admin.ts b/src/node/hooks/express/openapi-admin.ts index b1074e3f8..3d659aa32 100644 --- a/src/node/hooks/express/openapi-admin.ts +++ b/src/node/hooks/express/openapi-admin.ts @@ -20,7 +20,10 @@ export const generateAdminDefinition = (): any => ({ title: 'Etherpad Admin API', description: 'Authenticated administrative endpoints consumed by the Etherpad admin UI. ' + - 'Distinct from the public /api/{version}/* surface served by /api/openapi.json.', + 'Distinct from the public /api/{version}/* surface served by /api/openapi.json. ' + + 'For completeness this document also includes non-admin endpoints that are ' + + 'consumed by the pad UI itself (e.g. /api/version-status) since they share the ' + + 'same internal route registration.', version: getEpVersion(), }, paths: { diff --git a/src/node/hooks/express/updateStatus.ts b/src/node/hooks/express/updateStatus.ts index 0371ccc58..bdeb1179a 100644 --- a/src/node/hooks/express/updateStatus.ts +++ b/src/node/hooks/express/updateStatus.ts @@ -31,31 +31,32 @@ export const firstAuthorOf = (pad: {pool?: {numToAttrib?: Recordtoken`) with every + * same-origin request, and `authorManager.getAuthorId(token, user)` maps the + * token to a stable authorID via the `token2author` DB key. + * + * We do NOT use `req.session.user.author` because Etherpad does not populate + * an authorID into the express-session `user` object for pad visitors, so that + * field is always undefined in production. + * + * On any failure path we return null and the caller treats the request as + * anonymous, resulting in an EMPTY (no-badge) response. */ export const resolveRequestAuthor = async (req: any): Promise => { - const readAuthor = (): string | null => { - const a = req?.session?.user?.author; - return typeof a === 'string' && a !== '' ? a : null; - }; - const fromSession = readAuthor(); - if (fromSession !== null) return fromSession; try { - const expressModule = await import('../express'); - const mw = (expressModule as any).sessionMiddleware; - if (typeof mw !== 'function') return null; - await new Promise((resolve, reject) => { - mw(req, {} as any, (err?: unknown) => (err ? reject(err) : resolve())); - }); + const cookiePrefix = (settings as any).cookie?.prefix ?? ''; + const token = req?.cookies?.[`${cookiePrefix}token`]; + if (typeof token !== 'string' || token === '') return null; + const authorManagerMod: any = await import('../../db/AuthorManager'); + const authorManager = authorManagerMod.default ?? authorManagerMod; + if (typeof authorManager.getAuthorId !== 'function') return null; + const authorId = await authorManager.getAuthorId(token, req?.session?.user); + return typeof authorId === 'string' && authorId !== '' ? authorId : null; } catch { return null; } - return readAuthor(); }; interface OutdatedResponse { diff --git a/src/tests/backend-new/specs/hooks/express/updateStatus.test.ts b/src/tests/backend-new/specs/hooks/express/updateStatus.test.ts index 9e4299bda..cfe95099d 100644 --- a/src/tests/backend-new/specs/hooks/express/updateStatus.test.ts +++ b/src/tests/backend-new/specs/hooks/express/updateStatus.test.ts @@ -5,12 +5,12 @@ * (same as production), then exercised via supertest. `loadState` is mocked so * tests control the "latest" version without touching the filesystem. * `PadManager` is mocked so pad-creation doesn't require a running database. + * `AuthorManager` is mocked so token→authorID resolution doesn't require a DB. * - * Session injection: `resolveRequestAuthor` reads `req.session.user.author` - * directly. A simple before-route middleware sets that property on each - * request, keyed by the `X-Test-Author` request header, so individual tests - * can choose which author the request "belongs to" without needing real - * session middleware. + * Author injection: `resolveRequestAuthor` reads the `token` cookie and calls + * `authorManager.getAuthorId(token, user)`. Tests set `sessionAuthor` to + * control which authorID the mock returns for the test token `t.alice`. + * `null` means "anonymous" (getAuthorId returns null / empty for the token). */ import {describe, it, expect, vi, beforeAll, beforeEach, afterEach} from 'vitest'; @@ -38,6 +38,14 @@ vi.mock('../../../../../node/updater', () => ({ getDetectedInstallMethod: () => 'git', })); +// AuthorManager is dynamically imported inside resolveRequestAuthor(). Stubbing +// it here lets tests control token→authorID resolution without a DB. +vi.mock('../../../../../node/db/AuthorManager', () => ({ + default: { + getAuthorId: vi.fn(), + }, +})); + // PadManager is dynamically imported inside computeOutdated(). Stubbing it // here lets us control pad existence and author-pool contents without a DB. vi.mock('../../../../../node/db/PadManager', () => { @@ -58,6 +66,7 @@ vi.mock('../../../../../node/db/PadManager', () => { // --------------------------------------------------------------------------- import * as stateModule from '../../../../../node/updater/state'; +import * as authorManagerModule from '../../../../../node/db/AuthorManager'; import { expressCreateServer, _resetBadgeCacheForTests, @@ -112,21 +121,22 @@ let app: Express; let request: ReturnType; /** - * The author that the fake session middleware will inject into req.session. - * Tests that need a specific author set this before making a request. - * `null` means "anonymous" (no session author). + * The author that `authorManager.getAuthorId` will return for the test token + * `t.alice`. Tests set this before making a request. `null` means "anonymous" + * (getAuthorId returns null/empty, so resolveRequestAuthor returns null). */ let sessionAuthor: string | null = null; +/** Fixed test token used in every request. The cookie name has no prefix in tests. */ +const TEST_TOKEN = 't.alice'; + beforeAll(() => { app = express(); - // Fake session middleware: sets req.session.user.author from our test - // variable, so resolveRequestAuthor() in the route sees the right identity. + // Inject the test token cookie so resolveRequestAuthor() sees it. + // The real cookie-parser middleware is not needed: we set req.cookies directly. app.use((req: any, _res, next) => { - if (sessionAuthor !== null) { - req.session = {user: {author: sessionAuthor}}; - } + req.cookies = {token: TEST_TOKEN}; next(); }); @@ -143,6 +153,13 @@ beforeEach(() => { sessionAuthor = null; // Reset the loadState spy so each test controls its own return value. vi.mocked(stateModule.loadState).mockReset(); + // Wire up the AuthorManager mock: return sessionAuthor (or null) for our test token. + vi.mocked((authorManagerModule as any).default.getAuthorId).mockImplementation( + async (token: string) => { + if (token === TEST_TOKEN && sessionAuthor !== null) return sessionAuthor; + return null; + }, + ); }); afterEach(() => {