From 3c892d02ed8496e81afe2b64a59ec2e6a34ae7a8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Patrick=20M=C3=BCller?= Date: Sun, 6 Sep 2026 19:06:30 +0000 Subject: [PATCH] Put the feedback and tickets admin areas behind the shared identity (#13) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reviewed-on: https://git.plutodev.de/Nachklang/API/pulls/13 Co-authored-by: Patrick Müller Co-committed-by: Patrick Müller --- DEFERRED_SECURITY.md | 14 ++- src/app.factory.ts | 9 +- src/models/admin/admin.config.ts | 23 +++- src/models/feedback/admin/admin.interface.ts | 13 -- src/models/feedback/admin/admin.router.ts | 13 +- .../feedback/admin/events.admin.router.ts | 62 +++++---- .../feedback/admin/questions.admin.router.ts | 25 ++-- .../feedback/admin/reports.admin.router.ts | 30 +++-- .../feedback/admin/songs.admin.router.ts | 12 +- src/models/feedback/feedback.auth.ts | 88 ++++--------- src/models/tickets/admin/admin.router.ts | 7 +- .../tickets/admin/events.admin.router.ts | 32 +++-- .../tickets/admin/redemptions.admin.router.ts | 36 ++++-- .../tickets/admin/vouchers.admin.router.ts | 32 +++-- src/models/tickets/tickets.auth.ts | 66 +++------- test/admin/admin.config.test.ts | 34 +++++ test/admin/auth-binding.ts | 118 ++++++++++++++++++ test/feedback/feedback.auth.test.ts | 97 +++----------- test/integration/admin.auth.test.ts | 32 +++-- test/tickets/tickets.auth.test.ts | 23 ++++ 20 files changed, 457 insertions(+), 309 deletions(-) create mode 100644 test/admin/auth-binding.ts create mode 100644 test/tickets/tickets.auth.test.ts diff --git a/DEFERRED_SECURITY.md b/DEFERRED_SECURITY.md index 0321d1a..a933937 100644 --- a/DEFERRED_SECURITY.md +++ b/DEFERRED_SECURITY.md @@ -11,7 +11,19 @@ These items were identified during a security review on 2026-05-02 and conscious `sessionId` and `sessionKey` are currently read from query parameters, which means they appear in server access logs, browser history, proxy logs, and `Referer` headers. -**Fix:** Move to request headers (`X-Session-Id` / `X-Session-Key`) or the request body. Requires a corresponding frontend update. +**Fix (updated 2026-09-06):** Move the calendar onto the shared admin identity - +`requireAppAccess('calendar')` against the better-auth session cookie, per +`docs/calendar-auth-migration.md`. That closes this item outright rather than moving the +credential to a safer place, and it is now the cheaper of the two: the feedback and tickets +modules made the same move on 2026-09-06 for one line each. + +~~Move to request headers (`X-Session-Id` / `X-Session-Key`) or the request body.~~ No longer +the recommendation. Nothing on the server reads those two headers any more - the calendar's +query parameters are the last legacy credential path in the API - so this would build a second +mechanism just as the first is being retired. They survive only in the CORS `allowedHeaders` +list, and only until both frontends are redeployed. + +Either fix requires a corresponding frontend update. > Note: the shared calendar `password` parameter in query params is intentional (iCal clients don't support headers) and is acceptable for the current setup. diff --git a/src/app.factory.ts b/src/app.factory.ts index 6b7154d..9261a10 100644 --- a/src/app.factory.ts +++ b/src/app.factory.ts @@ -63,8 +63,13 @@ export const createApp = (): express.Application => { // the dev machine's LAN IP, never "localhost"). Dev-only, same as above. const lanIpRegex = /^http:\/\/(192\.168\.\d{1,3}\.\d{1,3}|10\.\d{1,3}\.\d{1,3}\.\d{1,3}|172\.(1[6-9]|2\d|3[01])\.\d{1,3}\.\d{1,3}):\d+$/; app.use(cors({ - // X-Session-* stay allowed until the calendar module is migrated off the - // legacy header sessions (see docs/calendar-auth-migration.md). + // X-Session-* are no longer read by anything on this side: the step 4 + // cutover took the last two readers (feedback.auth.ts, tickets.auth.ts) + // off them, and the calendar module passes its session in query + // parameters (DEFERRED_SECURITY.md item 1). They stay allowed only so a + // browser still running the pre-cutover tickets or feedback bundle gets + // a clean 401 rather than a CORS preflight failure. Drop them once both + // frontends are deployed - see docs/calendar-auth-migration.md step 5. allowedHeaders: ['Content-Type', 'X-Session-Id', 'X-Session-Key'], // The admin session lives in a cookie, so browsers must be allowed to send // it cross-origin - this is what makes credentials: 'include' work. diff --git a/src/models/admin/admin.config.ts b/src/models/admin/admin.config.ts index 5317ace..75a1a2e 100644 --- a/src/models/admin/admin.config.ts +++ b/src/models/admin/admin.config.ts @@ -73,8 +73,27 @@ const parseList = (value: string | undefined, fallback: string[]): string[] => { return parsed.length > 0 ? parsed : fallback; }; -// The apps whose frontends may talk to /admin/* with credentials. -export const APP_ORIGINS = parseList(process.env.APP_ORIGINS, []).map(origin => origin.replace(/\/$/, '')); +/** + * The apps whose frontends may talk to /admin/* with credentials. + * + * They feed better-auth's `trustedOrigins`, which is what lets the tickets and + * feedback admin areas call /admin/auth/sign-out from their own origin. That + * became load-bearing with the step 4 cutover: before it, the only browser + * origin that ever reached /admin/auth was the admin app itself. + * + * Hence the production default rather than an empty list. An origin missing + * here fails in a way that is easy to misread - sign-in works, the app works, + * and only sign-out returns an origin error - so the two frontends we know + * about are named here and APP_ORIGINS overrides them for a staging host. + * Dev adds the localhost ports separately (see admin.auth.ts). + */ +const DEFAULT_APP_ORIGINS = [ + 'https://tickets.nachklang.art', + 'https://feedback.nachklang.art' +]; + +export const APP_ORIGINS = parseList(process.env.APP_ORIGINS, DEFAULT_APP_ORIGINS) + .map(origin => origin.replace(/\/$/, '')); // Kept in sync by construction rather than by three separate lists: the admin // app itself always counts, and dev adds the local ports. diff --git a/src/models/feedback/admin/admin.interface.ts b/src/models/feedback/admin/admin.interface.ts index 0e3d2c8..3688d10 100644 --- a/src/models/feedback/admin/admin.interface.ts +++ b/src/models/feedback/admin/admin.interface.ts @@ -1,19 +1,6 @@ /** * @swagger * components: - * parameters: - * SessionIdHeader: - * in: header - * name: X-Session-Id - * required: true - * schema: - * type: string - * SessionKeyHeader: - * in: header - * name: X-Session-Key - * required: true - * schema: - * type: string * schemas: * EventAdminSummary: * type: object diff --git a/src/models/feedback/admin/admin.router.ts b/src/models/feedback/admin/admin.router.ts index 5cdf728..1640c66 100644 --- a/src/models/feedback/admin/admin.router.ts +++ b/src/models/feedback/admin/admin.router.ts @@ -26,9 +26,8 @@ adminRouter.use(requireAdminAuth); * summary: Validate the current admin session * description: Used by the Next.js middleware/proxy to gate /admin. Returns the authenticated admin's identity. * tags: [feedback-admin] - * parameters: - * - $ref: '#/components/parameters/SessionIdHeader' - * - $ref: '#/components/parameters/SessionKeyHeader' + * security: + * - AdminSessionCookie: [] * responses: * 200: * description: Success @@ -43,6 +42,8 @@ adminRouter.use(requireAdminAuth); * type: string * 401: * description: Unauthorized + * 403: + * description: Signed in without the permission for this app, or account disabled */ adminRouter.get('/me', (req: Request, res: Response) => { res.status(200).send({email: res.locals.admin.email, fullName: res.locals.admin.displayName}); @@ -55,9 +56,9 @@ adminRouter.get('/me', (req: Request, res: Response) => { * summary: Delete a single submission * description: Removes the submission and everything under it (its answers, guest book entry, newsletter signup) - for removing an individual abusive or inappropriate entry. Not a bulk moderation tool. * tags: [feedback-admin] + * security: + * - AdminSessionCookie: [] * parameters: - * - $ref: '#/components/parameters/SessionIdHeader' - * - $ref: '#/components/parameters/SessionKeyHeader' * - in: path * name: submissionId * required: true @@ -70,6 +71,8 @@ adminRouter.get('/me', (req: Request, res: Response) => { * description: Unknown submission * 401: * description: Unauthorized + * 403: + * description: Signed in without the permission for this app, or account disabled */ adminRouter.delete('/submissions/:submissionId', async (req: Request, res: Response) => { try { diff --git a/src/models/feedback/admin/events.admin.router.ts b/src/models/feedback/admin/events.admin.router.ts index 68430a4..517549f 100644 --- a/src/models/feedback/admin/events.admin.router.ts +++ b/src/models/feedback/admin/events.admin.router.ts @@ -18,9 +18,8 @@ export const eventsAdminRouter = express.Router(); * summary: List all events (admin) * description: All events, published or not, past or future, with submission counts. * tags: [feedback-admin] - * parameters: - * - $ref: '#/components/parameters/SessionIdHeader' - * - $ref: '#/components/parameters/SessionKeyHeader' + * security: + * - AdminSessionCookie: [] * responses: * 200: * description: Success @@ -32,13 +31,14 @@ export const eventsAdminRouter = express.Router(); * $ref: '#/components/schemas/EventAdminSummary' * 401: * description: Unauthorized + * 403: + * description: Signed in without the permission for this app, or account disabled * post: * summary: Create an event * description: Auto-generates the slug from the name and event year; defaults feedback_deadline to event_date + 14 days 23:59:59 unless supplied. * tags: [feedback-admin] - * parameters: - * - $ref: '#/components/parameters/SessionIdHeader' - * - $ref: '#/components/parameters/SessionKeyHeader' + * security: + * - AdminSessionCookie: [] * requestBody: * required: true * content: @@ -68,6 +68,8 @@ export const eventsAdminRouter = express.Router(); * description: Missing required fields * 401: * description: Unauthorized + * 403: + * description: Signed in without the permission for this app, or account disabled */ eventsAdminRouter.get('/', async (req: Request, res: Response) => { try { @@ -101,9 +103,9 @@ eventsAdminRouter.post('/', async (req: Request, res: Response) => { * summary: Get one event (admin) * description: Full event detail including setlist and assigned questions. * tags: [feedback-admin] + * security: + * - AdminSessionCookie: [] * parameters: - * - $ref: '#/components/parameters/SessionIdHeader' - * - $ref: '#/components/parameters/SessionKeyHeader' * - in: path * name: eventId * required: true @@ -120,12 +122,14 @@ eventsAdminRouter.post('/', async (req: Request, res: Response) => { * description: Unknown event * 401: * description: Unauthorized + * 403: + * description: Signed in without the permission for this app, or account disabled * put: * summary: Update an event * tags: [feedback-admin] + * security: + * - AdminSessionCookie: [] * parameters: - * - $ref: '#/components/parameters/SessionIdHeader' - * - $ref: '#/components/parameters/SessionKeyHeader' * - in: path * name: eventId * required: true @@ -138,13 +142,15 @@ eventsAdminRouter.post('/', async (req: Request, res: Response) => { * description: Unknown event * 401: * description: Unauthorized + * 403: + * description: Signed in without the permission for this app, or account disabled * delete: * summary: Delete an event * description: Refuses with 409 if submissions exist unless ?force=true is passed. * tags: [feedback-admin] + * security: + * - AdminSessionCookie: [] * parameters: - * - $ref: '#/components/parameters/SessionIdHeader' - * - $ref: '#/components/parameters/SessionKeyHeader' * - in: path * name: eventId * required: true @@ -163,6 +169,8 @@ eventsAdminRouter.post('/', async (req: Request, res: Response) => { * description: Submissions exist and force was not set * 401: * description: Unauthorized + * 403: + * description: Signed in without the permission for this app, or account disabled */ eventsAdminRouter.get('/:eventId', async (req: Request, res: Response) => { try { @@ -214,9 +222,9 @@ eventsAdminRouter.delete('/:eventId', async (req: Request, res: Response) => { * get: * summary: Get an event's setlist * tags: [feedback-admin] + * security: + * - AdminSessionCookie: [] * parameters: - * - $ref: '#/components/parameters/SessionIdHeader' - * - $ref: '#/components/parameters/SessionKeyHeader' * - in: path * name: eventId * required: true @@ -227,12 +235,14 @@ eventsAdminRouter.delete('/:eventId', async (req: Request, res: Response) => { * description: Success * 401: * description: Unauthorized + * 403: + * description: Signed in without the permission for this app, or account disabled * post: * summary: Add a song to an event's setlist * tags: [feedback-admin] + * security: + * - AdminSessionCookie: [] * parameters: - * - $ref: '#/components/parameters/SessionIdHeader' - * - $ref: '#/components/parameters/SessionKeyHeader' * - in: path * name: eventId * required: true @@ -257,6 +267,8 @@ eventsAdminRouter.delete('/:eventId', async (req: Request, res: Response) => { * description: Missing title * 401: * description: Unauthorized + * 403: + * description: Signed in without the permission for this app, or account disabled */ eventsAdminRouter.get('/:eventId/songs', async (req: Request, res: Response) => { try { @@ -292,9 +304,9 @@ eventsAdminRouter.post('/:eventId/songs', async (req: Request, res: Response) => * summary: Bulk reorder an event's setlist * description: Rewrites song positions as a dense 0..n-1 sequence in one transaction. * tags: [feedback-admin] + * security: + * - AdminSessionCookie: [] * parameters: - * - $ref: '#/components/parameters/SessionIdHeader' - * - $ref: '#/components/parameters/SessionKeyHeader' * - in: path * name: eventId * required: true @@ -317,6 +329,8 @@ eventsAdminRouter.post('/:eventId/songs', async (req: Request, res: Response) => * description: Reordered * 401: * description: Unauthorized + * 403: + * description: Signed in without the permission for this app, or account disabled */ eventsAdminRouter.put('/:eventId/songs/order', async (req: Request, res: Response) => { try { @@ -334,9 +348,9 @@ eventsAdminRouter.put('/:eventId/songs/order', async (req: Request, res: Respons * get: * summary: Get an event's assigned questions * tags: [feedback-admin] + * security: + * - AdminSessionCookie: [] * parameters: - * - $ref: '#/components/parameters/SessionIdHeader' - * - $ref: '#/components/parameters/SessionKeyHeader' * - in: path * name: eventId * required: true @@ -347,13 +361,15 @@ eventsAdminRouter.put('/:eventId/songs/order', async (req: Request, res: Respons * description: Success * 401: * description: Unauthorized + * 403: + * description: Signed in without the permission for this app, or account disabled * put: * summary: Bulk-set an event's assigned questions * description: One transaction - inserts new, updates existing, deletes removed. Keeps the admin UI a simple save-the-whole-list form. * tags: [feedback-admin] + * security: + * - AdminSessionCookie: [] * parameters: - * - $ref: '#/components/parameters/SessionIdHeader' - * - $ref: '#/components/parameters/SessionKeyHeader' * - in: path * name: eventId * required: true @@ -383,6 +399,8 @@ eventsAdminRouter.put('/:eventId/songs/order', async (req: Request, res: Respons * description: Saved * 401: * description: Unauthorized + * 403: + * description: Signed in without the permission for this app, or account disabled */ eventsAdminRouter.get('/:eventId/questions', async (req: Request, res: Response) => { try { diff --git a/src/models/feedback/admin/questions.admin.router.ts b/src/models/feedback/admin/questions.admin.router.ts index 57c2ca5..ec8c894 100644 --- a/src/models/feedback/admin/questions.admin.router.ts +++ b/src/models/feedback/admin/questions.admin.router.ts @@ -16,9 +16,9 @@ export const questionsAdminRouter = express.Router(); * get: * summary: List the question library * tags: [feedback-admin] + * security: + * - AdminSessionCookie: [] * parameters: - * - $ref: '#/components/parameters/SessionIdHeader' - * - $ref: '#/components/parameters/SessionKeyHeader' * - in: query * name: includeArchived * schema: @@ -34,12 +34,13 @@ export const questionsAdminRouter = express.Router(); * $ref: '#/components/schemas/AdminQuestion' * 401: * description: Unauthorized + * 403: + * description: Signed in without the permission for this app, or account disabled * post: * summary: Create a question * tags: [feedback-admin] - * parameters: - * - $ref: '#/components/parameters/SessionIdHeader' - * - $ref: '#/components/parameters/SessionKeyHeader' + * security: + * - AdminSessionCookie: [] * requestBody: * required: true * content: @@ -61,6 +62,8 @@ export const questionsAdminRouter = express.Router(); * description: Missing or invalid fields * 401: * description: Unauthorized + * 403: + * description: Signed in without the permission for this app, or account disabled */ questionsAdminRouter.get('/', async (req: Request, res: Response) => { try { @@ -94,9 +97,9 @@ questionsAdminRouter.post('/', async (req: Request, res: Response) => { * summary: Edit a question's label/help text * description: question_type is immutable after creation - the admin UI offers "archive and create new" instead. * tags: [feedback-admin] + * security: + * - AdminSessionCookie: [] * parameters: - * - $ref: '#/components/parameters/SessionIdHeader' - * - $ref: '#/components/parameters/SessionKeyHeader' * - in: path * name: questionId * required: true @@ -123,13 +126,15 @@ questionsAdminRouter.post('/', async (req: Request, res: Response) => { * description: Unknown question * 401: * description: Unauthorized + * 403: + * description: Signed in without the permission for this app, or account disabled * delete: * summary: Archive (or hard-delete) a question * description: Archives the question if it has ever been used; hard-deletes it if it has never been assigned to any event. * tags: [feedback-admin] + * security: + * - AdminSessionCookie: [] * parameters: - * - $ref: '#/components/parameters/SessionIdHeader' - * - $ref: '#/components/parameters/SessionKeyHeader' * - in: path * name: questionId * required: true @@ -142,6 +147,8 @@ questionsAdminRouter.post('/', async (req: Request, res: Response) => { * description: Unknown question * 401: * description: Unauthorized + * 403: + * description: Signed in without the permission for this app, or account disabled */ questionsAdminRouter.put('/:questionId', async (req: Request, res: Response) => { try { diff --git a/src/models/feedback/admin/reports.admin.router.ts b/src/models/feedback/admin/reports.admin.router.ts index 04ece63..2cb88ee 100644 --- a/src/models/feedback/admin/reports.admin.router.ts +++ b/src/models/feedback/admin/reports.admin.router.ts @@ -19,9 +19,9 @@ export const reportsAdminRouter = express.Router(); * summary: Aggregated feedback report for one event * description: Song-pick vote counts, song-rating averages, capped free-text list, guest book count, and newsletter sync counts. * tags: [feedback-admin] + * security: + * - AdminSessionCookie: [] * parameters: - * - $ref: '#/components/parameters/SessionIdHeader' - * - $ref: '#/components/parameters/SessionKeyHeader' * - in: path * name: eventId * required: true @@ -34,6 +34,8 @@ export const reportsAdminRouter = express.Router(); * description: Unknown event * 401: * description: Unauthorized + * 403: + * description: Signed in without the permission for this app, or account disabled */ reportsAdminRouter.get('/:eventId/report', async (req: Request, res: Response) => { try { @@ -55,9 +57,9 @@ reportsAdminRouter.get('/:eventId/report', async (req: Request, res: Response) = * summary: Guest Book entries for one event * description: Newest first, paginated. * tags: [feedback-admin] + * security: + * - AdminSessionCookie: [] * parameters: - * - $ref: '#/components/parameters/SessionIdHeader' - * - $ref: '#/components/parameters/SessionKeyHeader' * - in: path * name: eventId * required: true @@ -81,6 +83,8 @@ reportsAdminRouter.get('/:eventId/report', async (req: Request, res: Response) = * description: Success * 401: * description: Unauthorized + * 403: + * description: Signed in without the permission for this app, or account disabled */ reportsAdminRouter.get('/:eventId/guestbook', async (req: Request, res: Response) => { try { @@ -101,9 +105,9 @@ reportsAdminRouter.get('/:eventId/guestbook', async (req: Request, res: Response * summary: Newsletter signups for one event * description: Includes sync_status, so failures can be handled manually. Newest first, paginated. * tags: [feedback-admin] + * security: + * - AdminSessionCookie: [] * parameters: - * - $ref: '#/components/parameters/SessionIdHeader' - * - $ref: '#/components/parameters/SessionKeyHeader' * - in: path * name: eventId * required: true @@ -127,6 +131,8 @@ reportsAdminRouter.get('/:eventId/guestbook', async (req: Request, res: Response * description: Success * 401: * description: Unauthorized + * 403: + * description: Signed in without the permission for this app, or account disabled */ reportsAdminRouter.get('/:eventId/newsletter', async (req: Request, res: Response) => { try { @@ -147,9 +153,9 @@ reportsAdminRouter.get('/:eventId/newsletter', async (req: Request, res: Respons * summary: CSV export of all answers for one event * description: Long format, one row per answer. UTF-8 BOM, `;` separator, RFC 4180 escaping, formula-injection guard. * tags: [feedback-admin] + * security: + * - AdminSessionCookie: [] * parameters: - * - $ref: '#/components/parameters/SessionIdHeader' - * - $ref: '#/components/parameters/SessionKeyHeader' * - in: path * name: eventId * required: true @@ -162,6 +168,8 @@ reportsAdminRouter.get('/:eventId/newsletter', async (req: Request, res: Respons * text/csv: {} * 401: * description: Unauthorized + * 403: + * description: Signed in without the permission for this app, or account disabled */ reportsAdminRouter.get('/:eventId/export/responses.csv', async (req: Request, res: Response) => { try { @@ -187,9 +195,9 @@ reportsAdminRouter.get('/:eventId/export/responses.csv', async (req: Request, re * get: * summary: CSV export of Guest Book entries for one event * tags: [feedback-admin] + * security: + * - AdminSessionCookie: [] * parameters: - * - $ref: '#/components/parameters/SessionIdHeader' - * - $ref: '#/components/parameters/SessionKeyHeader' * - in: path * name: eventId * required: true @@ -202,6 +210,8 @@ reportsAdminRouter.get('/:eventId/export/responses.csv', async (req: Request, re * text/csv: {} * 401: * description: Unauthorized + * 403: + * description: Signed in without the permission for this app, or account disabled */ reportsAdminRouter.get('/:eventId/export/guestbook.csv', async (req: Request, res: Response) => { try { diff --git a/src/models/feedback/admin/songs.admin.router.ts b/src/models/feedback/admin/songs.admin.router.ts index d9002fe..c35bb5d 100644 --- a/src/models/feedback/admin/songs.admin.router.ts +++ b/src/models/feedback/admin/songs.admin.router.ts @@ -16,9 +16,9 @@ export const songsAdminRouter = express.Router(); * put: * summary: Edit a song's title/composer * tags: [feedback-admin] + * security: + * - AdminSessionCookie: [] * parameters: - * - $ref: '#/components/parameters/SessionIdHeader' - * - $ref: '#/components/parameters/SessionKeyHeader' * - in: path * name: songId * required: true @@ -45,13 +45,15 @@ export const songsAdminRouter = express.Router(); * description: Unknown song * 401: * description: Unauthorized + * 403: + * description: Signed in without the permission for this app, or account disabled * delete: * summary: Remove a song * description: Past answers keep their song_title_snapshot even after the song is removed. * tags: [feedback-admin] + * security: + * - AdminSessionCookie: [] * parameters: - * - $ref: '#/components/parameters/SessionIdHeader' - * - $ref: '#/components/parameters/SessionKeyHeader' * - in: path * name: songId * required: true @@ -64,6 +66,8 @@ export const songsAdminRouter = express.Router(); * description: Unknown song * 401: * description: Unauthorized + * 403: + * description: Signed in without the permission for this app, or account disabled */ songsAdminRouter.put('/:songId', async (req: Request, res: Response) => { try { diff --git a/src/models/feedback/feedback.auth.ts b/src/models/feedback/feedback.auth.ts index 9ee26d8..35e34d8 100644 --- a/src/models/feedback/feedback.auth.ts +++ b/src/models/feedback/feedback.auth.ts @@ -1,78 +1,38 @@ -import express from 'express'; -import * as UserService from '../calendar/users/users.service.js'; -import {sendServerError} from './feedback.errors.js'; +import {requireAppAccess} from '../admin/admin.middleware.js'; /** * This file is the ONLY place in the feedback module that knows how admin - * authentication works today. No route handler and no service outside this - * file may import users.service, read session headers, or touch bcrypt. + * authentication works. No route handler and no service outside this file may + * read session headers or resolve a user itself. * - * Today: reuses the existing Calendar users/sessions mechanism. Any - * activated @nachklang.art account may administer feedback — no roles. - * Migrating to Keycloak later means writing a keycloakJwtAuthenticator - * below and changing the one `activeAuthenticator` binding (plus the - * frontend's login route handler) — nothing else in the feedback module - * needs to change. + * Today: the shared admin identity in `src/models/admin/`. A session cookie + * set by /admin/auth on admin.nachklang.art, plus a `feedback` permission on + * the account. Both are re-checked on every request, so disabling a user or + * taking their feedback permission away takes effect immediately. * - * Explicitly forbidden: accepting sessionId/sessionKey from query - * parameters, even "temporarily". That is the exact mistake documented in - * DEFERRED_SECURITY.md item 1 for the Calendar domain, where credentials - * end up in access logs, browser history, proxy logs, and Referer headers. - * Headers only. + * Before 2026-09-06 this was a header session against the calendar users + * table, and any activated @nachklang.art account could administer feedback. + * That is why the swap is a one-line binding: everything downstream only ever + * saw `requireAdminAuth` and `res.locals.admin`, and both still mean what + * they meant. What changed is that access is now granted per user rather than + * implied by having an account. + * + * Explicitly forbidden: accepting session credentials from query parameters, + * even "temporarily". That is the exact mistake documented in + * DEFERRED_SECURITY.md item 1 for the Calendar domain, where credentials end + * up in access logs, browser history, proxy logs, and Referer headers. */ -// The only thing the rest of the feedback module knows about an admin. +// The only thing the rest of the feedback module knows about an admin. The +// shared middleware puts a superset of this on res.locals.admin. export interface AdminIdentity { id: string; email: string; displayName: string; } -// Pluggable strategy: extract + verify credentials from a request. -// Returns the identity, or null if unauthenticated. Throws only on -// infrastructure errors (e.g. the DB being unreachable). -export type AdminAuthenticator = (req: express.Request) => Promise; - -// Current implementation: reads X-Session-Id / X-Session-Key headers, -// delegates to the existing calendar UserService.checkSession(...). -export const sessionHeaderAuthenticator: AdminAuthenticator = async (req) => { - const sessionId = req.header('X-Session-Id'); - const sessionKey = req.header('X-Session-Key'); - if (!sessionId || !sessionKey) { - return null; - } - - const ip = req.ip || ''; - const user = await UserService.checkSession(sessionId, sessionKey, ip); - - // Mirrors the Calendar domain's own convention: a valid session on an - // inactive (not yet activated) account is not sufficient. - if (!user || !user.isActive) { - return null; - } - - return { - id: String(user.userId), - email: user.email, - displayName: user.fullName - }; -}; - -// Swap point: change this one binding to migrate to Keycloak. -export const activeAuthenticator: AdminAuthenticator = sessionHeaderAuthenticator; - // Express middleware used by every admin route. On success: -// res.locals.admin = AdminIdentity, calls next(). On failure: 401. -export const requireAdminAuth: express.RequestHandler = async (req, res, next) => { - try { - const identity = await activeAuthenticator(req); - if (!identity) { - res.status(401).send({status: 'UNAUTHORIZED', message: 'Anmeldung erforderlich.'}); - return; - } - res.locals.admin = identity; - next(); - } catch (e: any) { - sendServerError(res, e); - } -}; +// res.locals.admin = AdminAccess (an AdminIdentity plus permissions), calls +// next(). On failure: 401 when not signed in, 403 when signed in without the +// feedback permission. +export const requireAdminAuth = requireAppAccess('feedback'); diff --git a/src/models/tickets/admin/admin.router.ts b/src/models/tickets/admin/admin.router.ts index dfaf0dc..b7f471c 100644 --- a/src/models/tickets/admin/admin.router.ts +++ b/src/models/tickets/admin/admin.router.ts @@ -16,14 +16,15 @@ adminRouter.use(requireAdminAuth); * get: * summary: Validate the current admin session * tags: [tickets-admin] - * parameters: - * - $ref: '#/components/parameters/SessionIdHeader' - * - $ref: '#/components/parameters/SessionKeyHeader' + * security: + * - AdminSessionCookie: [] * responses: * 200: * description: Success * 401: * description: Unauthorized + * 403: + * description: Signed in without the permission for this app, or account disabled */ adminRouter.get('/me', (req: Request, res: Response) => { res.status(200).send({email: res.locals.admin.email, fullName: res.locals.admin.displayName}); diff --git a/src/models/tickets/admin/events.admin.router.ts b/src/models/tickets/admin/events.admin.router.ts index 94642e5..d483a3a 100644 --- a/src/models/tickets/admin/events.admin.router.ts +++ b/src/models/tickets/admin/events.admin.router.ts @@ -11,14 +11,15 @@ export const eventsAdminRouter = express.Router(); * summary: List concerts for the admin event picker * description: Wraps the Calendar module's public-calendar admin listing (includes DRAFT events). * tags: [tickets-admin] - * parameters: - * - $ref: '#/components/parameters/SessionIdHeader' - * - $ref: '#/components/parameters/SessionKeyHeader' + * security: + * - AdminSessionCookie: [] * responses: * 200: * description: Success * 401: * description: Unauthorized + * 403: + * description: Signed in without the permission for this app, or account disabled */ eventsAdminRouter.get('/', async (req: Request, res: Response) => { try { @@ -35,14 +36,15 @@ eventsAdminRouter.get('/', async (req: Request, res: Response) => { * summary: List public-calendar events not yet added to the ticket shop * description: Source list for the "add a concert" picker - the public calendar holds more than concerts, so events only appear in the ticket shop once explicitly added. * tags: [tickets-admin] - * parameters: - * - $ref: '#/components/parameters/SessionIdHeader' - * - $ref: '#/components/parameters/SessionKeyHeader' + * security: + * - AdminSessionCookie: [] * responses: * 200: * description: Success * 401: * description: Unauthorized + * 403: + * description: Signed in without the permission for this app, or account disabled */ eventsAdminRouter.get('/available', async (req: Request, res: Response) => { try { @@ -58,9 +60,9 @@ eventsAdminRouter.get('/available', async (req: Request, res: Response) => { * get: * summary: Get a concert's voucher/capacity stats * tags: [tickets-admin] + * security: + * - AdminSessionCookie: [] * parameters: - * - $ref: '#/components/parameters/SessionIdHeader' - * - $ref: '#/components/parameters/SessionKeyHeader' * - in: path * name: eventId * required: true @@ -75,6 +77,8 @@ eventsAdminRouter.get('/available', async (req: Request, res: Response) => { * $ref: '#/components/schemas/EventStats' * 401: * description: Unauthorized + * 403: + * description: Signed in without the permission for this app, or account disabled */ eventsAdminRouter.get('/:eventId/stats', async (req: Request, res: Response) => { try { @@ -91,9 +95,9 @@ eventsAdminRouter.get('/:eventId/stats', async (req: Request, res: Response) => * summary: Set a concert's voucher settings * description: Upserts capacity (null = uncapped), redemption deadline (null = none), and whether to collect a mailing address. * tags: [tickets-admin] + * security: + * - AdminSessionCookie: [] * parameters: - * - $ref: '#/components/parameters/SessionIdHeader' - * - $ref: '#/components/parameters/SessionKeyHeader' * - in: path * name: eventId * required: true @@ -123,6 +127,8 @@ eventsAdminRouter.get('/:eventId/stats', async (req: Request, res: Response) => * description: Saved * 401: * description: Unauthorized + * 403: + * description: Signed in without the permission for this app, or account disabled */ eventsAdminRouter.put('/:eventId/settings', async (req: Request, res: Response) => { try { @@ -149,9 +155,9 @@ eventsAdminRouter.put('/:eventId/settings', async (req: Request, res: Response) * summary: Remove an event from the ticket shop * description: Deletes its settings row, so it drops out of the picker and reappears in the "add" list. Refused with 409 if vouchers already reference the event - existing vouchers/redemptions stay valid either way. * tags: [tickets-admin] + * security: + * - AdminSessionCookie: [] * parameters: - * - $ref: '#/components/parameters/SessionIdHeader' - * - $ref: '#/components/parameters/SessionKeyHeader' * - in: path * name: eventId * required: true @@ -164,6 +170,8 @@ eventsAdminRouter.put('/:eventId/settings', async (req: Request, res: Response) * description: Vouchers already reference this event * 401: * description: Unauthorized + * 403: + * description: Signed in without the permission for this app, or account disabled */ eventsAdminRouter.delete('/:eventId/settings', async (req: Request, res: Response) => { try { diff --git a/src/models/tickets/admin/redemptions.admin.router.ts b/src/models/tickets/admin/redemptions.admin.router.ts index 4938536..dd07e29 100644 --- a/src/models/tickets/admin/redemptions.admin.router.ts +++ b/src/models/tickets/admin/redemptions.admin.router.ts @@ -11,9 +11,9 @@ export const redemptionsAdminRouter = express.Router(); * summary: List redemptions (admin) * description: Filterable by event and status (ACTIVE/UNDONE). * tags: [tickets-admin] + * security: + * - AdminSessionCookie: [] * parameters: - * - $ref: '#/components/parameters/SessionIdHeader' - * - $ref: '#/components/parameters/SessionKeyHeader' * - in: query * name: eventId * schema: @@ -33,6 +33,8 @@ export const redemptionsAdminRouter = express.Router(); * $ref: '#/components/schemas/RedemptionSummary' * 401: * description: Unauthorized + * 403: + * description: Signed in without the permission for this app, or account disabled */ redemptionsAdminRouter.get('/', async (req: Request, res: Response) => { try { @@ -50,9 +52,9 @@ redemptionsAdminRouter.get('/', async (req: Request, res: Response) => { * get: * summary: Get a single redemption (admin) * tags: [tickets-admin] + * security: + * - AdminSessionCookie: [] * parameters: - * - $ref: '#/components/parameters/SessionIdHeader' - * - $ref: '#/components/parameters/SessionKeyHeader' * - in: path * name: redemptionId * required: true @@ -65,13 +67,15 @@ redemptionsAdminRouter.get('/', async (req: Request, res: Response) => { * description: Unknown redemption * 401: * description: Unauthorized + * 403: + * description: Signed in without the permission for this app, or account disabled * patch: * summary: Edit a redemption's contact info and/or guest list * description: Only fields present in the body are changed. Growing the guest count is re-checked against the voucher's max guests and the event's remaining capacity. Logs to the audit trail with an optional admin-supplied reason. * tags: [tickets-admin] + * security: + * - AdminSessionCookie: [] * parameters: - * - $ref: '#/components/parameters/SessionIdHeader' - * - $ref: '#/components/parameters/SessionKeyHeader' * - in: path * name: redemptionId * required: true @@ -105,6 +109,8 @@ redemptionsAdminRouter.get('/', async (req: Request, res: Response) => { * description: Not active, exceeds max guests, or exceeds remaining capacity * 401: * description: Unauthorized + * 403: + * description: Signed in without the permission for this app, or account disabled */ redemptionsAdminRouter.get('/:redemptionId', async (req: Request, res: Response) => { try { @@ -161,9 +167,9 @@ redemptionsAdminRouter.patch('/:redemptionId', async (req: Request, res: Respons * summary: Undo a redemption * description: Reopens the code (back to UNUSED) and marks the redemption UNDONE. Guest data is kept for the audit trail; a later re-redemption creates a new redemption record. * tags: [tickets-admin] + * security: + * - AdminSessionCookie: [] * parameters: - * - $ref: '#/components/parameters/SessionIdHeader' - * - $ref: '#/components/parameters/SessionKeyHeader' * - in: path * name: redemptionId * required: true @@ -186,6 +192,8 @@ redemptionsAdminRouter.patch('/:redemptionId', async (req: Request, res: Respons * description: Redemption is not active * 401: * description: Unauthorized + * 403: + * description: Signed in without the permission for this app, or account disabled */ redemptionsAdminRouter.post('/:redemptionId/undo', async (req: Request, res: Response) => { try { @@ -211,9 +219,9 @@ redemptionsAdminRouter.post('/:redemptionId/undo', async (req: Request, res: Res * summary: Resend the redemption confirmation email * description: Rebuilds the confirmation email from the stored redemption data and sends it again, then records the outcome on the redemption. Intended for redemptions whose original confirmation email failed. * tags: [tickets-admin] + * security: + * - AdminSessionCookie: [] * parameters: - * - $ref: '#/components/parameters/SessionIdHeader' - * - $ref: '#/components/parameters/SessionKeyHeader' * - in: path * name: redemptionId * required: true @@ -230,6 +238,8 @@ redemptionsAdminRouter.post('/:redemptionId/undo', async (req: Request, res: Res * description: The email relay rejected the send * 401: * description: Unauthorized + * 403: + * description: Signed in without the permission for this app, or account disabled */ redemptionsAdminRouter.post('/:redemptionId/resend-confirmation', async (req: Request, res: Response) => { try { @@ -259,9 +269,9 @@ redemptionsAdminRouter.post('/:redemptionId/resend-confirmation', async (req: Re * get: * summary: Get a voucher's admin-action audit trail * tags: [tickets-admin] + * security: + * - AdminSessionCookie: [] * parameters: - * - $ref: '#/components/parameters/SessionIdHeader' - * - $ref: '#/components/parameters/SessionKeyHeader' * - in: path * name: code * required: true @@ -278,6 +288,8 @@ redemptionsAdminRouter.post('/:redemptionId/resend-confirmation', async (req: Re * $ref: '#/components/schemas/AuditLogEntry' * 401: * description: Unauthorized + * 403: + * description: Signed in without the permission for this app, or account disabled */ export const voucherHistoryRouter = express.Router(); voucherHistoryRouter.get('/:code/history', async (req: Request, res: Response) => { diff --git a/src/models/tickets/admin/vouchers.admin.router.ts b/src/models/tickets/admin/vouchers.admin.router.ts index c1f7892..427c305 100644 --- a/src/models/tickets/admin/vouchers.admin.router.ts +++ b/src/models/tickets/admin/vouchers.admin.router.ts @@ -11,9 +11,9 @@ export const vouchersAdminRouter = express.Router(); * summary: List vouchers (admin) * description: Filterable by event and status. * tags: [tickets-admin] + * security: + * - AdminSessionCookie: [] * parameters: - * - $ref: '#/components/parameters/SessionIdHeader' - * - $ref: '#/components/parameters/SessionKeyHeader' * - in: query * name: eventId * schema: @@ -33,6 +33,8 @@ export const vouchersAdminRouter = express.Router(); * $ref: '#/components/schemas/VoucherCode' * 401: * description: Unauthorized + * 403: + * description: Signed in without the permission for this app, or account disabled */ vouchersAdminRouter.get('/', async (req: Request, res: Response) => { try { @@ -51,9 +53,8 @@ vouchersAdminRouter.get('/', async (req: Request, res: Response) => { * summary: Batch-generate wildcard codes * description: Generates `quantity` codes sharing the same eligible events and max-guest count, grouped under one batchId. * tags: [tickets-admin] - * parameters: - * - $ref: '#/components/parameters/SessionIdHeader' - * - $ref: '#/components/parameters/SessionKeyHeader' + * security: + * - AdminSessionCookie: [] * requestBody: * required: true * content: @@ -87,6 +88,8 @@ vouchersAdminRouter.get('/', async (req: Request, res: Response) => { * description: Invalid input * 401: * description: Unauthorized + * 403: + * description: Signed in without the permission for this app, or account disabled */ vouchersAdminRouter.post('/wildcard', async (req: Request, res: Response) => { try { @@ -112,9 +115,8 @@ vouchersAdminRouter.post('/wildcard', async (req: Request, res: Response) => { * summary: Bulk-create personalized codes * description: One code per row (name, email, eligible events, max guests), grouped under one batchId. * tags: [tickets-admin] - * parameters: - * - $ref: '#/components/parameters/SessionIdHeader' - * - $ref: '#/components/parameters/SessionKeyHeader' + * security: + * - AdminSessionCookie: [] * requestBody: * required: true * content: @@ -147,6 +149,8 @@ vouchersAdminRouter.post('/wildcard', async (req: Request, res: Response) => { * description: Invalid input * 401: * description: Unauthorized + * 403: + * description: Signed in without the permission for this app, or account disabled */ vouchersAdminRouter.post('/personalized', async (req: Request, res: Response) => { try { @@ -169,9 +173,9 @@ vouchersAdminRouter.post('/personalized', async (req: Request, res: Response) => * get: * summary: Get a single voucher (admin) * tags: [tickets-admin] + * security: + * - AdminSessionCookie: [] * parameters: - * - $ref: '#/components/parameters/SessionIdHeader' - * - $ref: '#/components/parameters/SessionKeyHeader' * - in: path * name: code * required: true @@ -184,6 +188,8 @@ vouchersAdminRouter.post('/personalized', async (req: Request, res: Response) => * description: Unknown code * 401: * description: Unauthorized + * 403: + * description: Signed in without the permission for this app, or account disabled */ vouchersAdminRouter.get('/:code', async (req: Request, res: Response) => { try { @@ -205,9 +211,9 @@ vouchersAdminRouter.get('/:code', async (req: Request, res: Response) => { * summary: Void an unredeemed code * description: Only allowed while the code is UNUSED. Logs to the voucher's audit trail. * tags: [tickets-admin] + * security: + * - AdminSessionCookie: [] * parameters: - * - $ref: '#/components/parameters/SessionIdHeader' - * - $ref: '#/components/parameters/SessionKeyHeader' * - in: path * name: code * required: true @@ -230,6 +236,8 @@ vouchersAdminRouter.get('/:code', async (req: Request, res: Response) => { * description: Code is not in UNUSED status * 401: * description: Unauthorized + * 403: + * description: Signed in without the permission for this app, or account disabled */ vouchersAdminRouter.post('/:code/void', async (req: Request, res: Response) => { try { diff --git a/src/models/tickets/tickets.auth.ts b/src/models/tickets/tickets.auth.ts index 4975685..e1ec1a9 100644 --- a/src/models/tickets/tickets.auth.ts +++ b/src/models/tickets/tickets.auth.ts @@ -1,20 +1,23 @@ -import express from 'express'; -import * as UserService from '../calendar/users/users.service.js'; -import {sendServerError} from './tickets.errors.js'; +import {requireAppAccess} from '../admin/admin.middleware.js'; /** * Mirrors the Feedback module's feedback.auth.ts: this is the ONLY place in * the tickets module that knows how admin authentication works. No route - * handler and no service outside this file may import users.service, read - * session headers, or touch bcrypt. + * handler and no service outside this file may read session headers or + * resolve a user itself. * - * Today: reuses the existing Calendar users/sessions mechanism. Any - * activated @nachklang.art account may administer vouchers - no roles, same - * policy as Feedback (see docs/plan-ticket-shop.md). A dedicated - * roles/permissions model is explicitly out of scope for v1. + * Today: the shared admin identity in `src/models/admin/`. A session cookie + * set by /admin/auth on admin.nachklang.art, plus a `tickets` permission on + * the account. Both are re-checked on every request, so disabling a user or + * taking their tickets permission away takes effect immediately. * - * Explicitly forbidden: accepting sessionId/sessionKey from query - * parameters - headers only (see DEFERRED_SECURITY.md item 1). + * Before 2026-09-06 this was a header session against the calendar users + * table, and any activated @nachklang.art account could administer vouchers + * (see docs/plan-ticket-shop.md, which called a roles model out of scope for + * v1). It is in scope now, and lives in the admin module rather than here. + * + * Explicitly forbidden: accepting session credentials from query parameters - + * see DEFERRED_SECURITY.md item 1. */ export interface AdminIdentity { @@ -23,41 +26,6 @@ export interface AdminIdentity { displayName: string; } -export type AdminAuthenticator = (req: express.Request) => Promise; - -export const sessionHeaderAuthenticator: AdminAuthenticator = async (req) => { - const sessionId = req.header('X-Session-Id'); - const sessionKey = req.header('X-Session-Key'); - if (!sessionId || !sessionKey) { - return null; - } - - const ip = req.ip || ''; - const user = await UserService.checkSession(sessionId, sessionKey, ip); - - if (!user || !user.isActive) { - return null; - } - - return { - id: String(user.userId), - email: user.email, - displayName: user.fullName - }; -}; - -export const activeAuthenticator: AdminAuthenticator = sessionHeaderAuthenticator; - -export const requireAdminAuth: express.RequestHandler = async (req, res, next) => { - try { - const identity = await activeAuthenticator(req); - if (!identity) { - res.status(401).send({status: 'UNAUTHORIZED', message: 'Anmeldung erforderlich.'}); - return; - } - res.locals.admin = identity; - next(); - } catch (e: any) { - sendServerError(res, e); - } -}; +// On failure: 401 when not signed in, 403 when signed in without the tickets +// permission. +export const requireAdminAuth = requireAppAccess('tickets'); diff --git a/test/admin/admin.config.test.ts b/test/admin/admin.config.test.ts index c30073f..37ce7fb 100644 --- a/test/admin/admin.config.test.ts +++ b/test/admin/admin.config.test.ts @@ -78,6 +78,40 @@ describe('CLIENT_IP_HEADERS', () => { }); }); +describe('APP_ORIGINS', () => { + beforeEach(() => { + delete process.env.APP_ORIGINS; + }); + + // These reach better-auth's trustedOrigins, and the step 4 cutover made the + // tickets and feedback origins load-bearing: without them their sign-out + // call is rejected while everything else still works. + it('defaults to the two production frontends', async () => { + const config = await loadConfig(); + expect(config.APP_ORIGINS).toEqual([ + 'https://tickets.nachklang.art', + 'https://feedback.nachklang.art' + ]); + }); + + it('is overridden wholesale by the environment, for a staging host', async () => { + process.env.APP_ORIGINS = 'https://tickets.staging.example, https://feedback.staging.example/'; + const config = await loadConfig(); + expect(config.APP_ORIGINS).toEqual([ + 'https://tickets.staging.example', + // Trailing slash stripped: an origin with one never matches. + 'https://feedback.staging.example' + ]); + }); + + it('always includes the admin app itself in ADMIN_ALLOWED_ORIGINS', async () => { + process.env.ADMIN_APP_URL = 'https://admin.nachklang.art'; + const config = await loadConfig(); + expect(config.ADMIN_ALLOWED_ORIGINS).toContain('https://admin.nachklang.art'); + expect(config.ADMIN_ALLOWED_ORIGINS).toContain('https://tickets.nachklang.art'); + }); +}); + describe('isProd', () => { it('is false only for the explicit relaxed environments', async () => { process.env.NODE_ENV = 'development'; diff --git a/test/admin/auth-binding.ts b/test/admin/auth-binding.ts new file mode 100644 index 0000000..d50a0c9 --- /dev/null +++ b/test/admin/auth-binding.ts @@ -0,0 +1,118 @@ +import {vi, describe, it, expect, beforeEach, type Mock} from 'vitest'; +import express, {Request, Response} from 'express'; + +/** + * Shared body for the two cutover tests (2026-09-06). feedback.auth.ts and + * tickets.auth.ts used to carry their own header-session authenticator against + * the calendar users table; both are now one binding to the shared admin gate. + * + * What is worth asserting is not how that gate works - admin.middleware.test.ts + * owns that - but that each module is bound to *its own* app, and that neither + * consults the calendar users service any more. The mocks live in the calling + * file because vi.mock is per-module-graph; only the assertions are shared. + */ + +export interface BindingMocks { + /** auth.api.getSession from the mocked admin.auth.js */ + getSession: Mock; + /** loadAccess from the mocked users.admin.service.js */ + loadAccess: Mock; + /** checkSession from the mocked calendar users.service.js */ + checkSession: Mock; +} + +const makeReq = (): Request => ({headers: {cookie: 'nachklang.session_token=abc'}} as unknown as Request); + +const makeRes = (): Response => { + const res: any = {}; + res.status = vi.fn().mockReturnValue(res); + res.send = vi.fn().mockReturnValue(res); + res.locals = {}; + return res as Response; +}; + +const userWith = (...apps: string[]) => ({ + id: 'u1', + email: 'a@nachklang.art', + displayName: 'Anna Admin', + disabled: false, + permissions: apps.map(app => ({app, role: 'access'})), + apps +}); + +export const describeAdminBinding = ( + app: string, + otherApp: string, + middleware: express.RequestHandler, + mocks: () => BindingMocks +): void => { + describe(`${app} requireAdminAuth`, () => { + let m: BindingMocks; + + const run = async () => { + const res = makeRes(); + const next = vi.fn(); + await middleware(makeReq(), res, next); + return {res, next}; + }; + + beforeEach(() => { + m = mocks(); + m.getSession.mockReset(); + m.loadAccess.mockReset(); + m.checkSession.mockReset(); + }); + + it('responds 401 and does not call next() without a session', async () => { + m.getSession.mockResolvedValue(null); + + const {res, next} = await run(); + + expect(res.status).toHaveBeenCalledWith(401); + expect(next).not.toHaveBeenCalled(); + }); + + it(`responds 403 for a signed-in user who only has ${otherApp}`, async () => { + m.getSession.mockResolvedValue({user: {id: 'u1'}}); + m.loadAccess.mockResolvedValue(userWith(otherApp)); + + const {res, next} = await run(); + + expect(res.status).toHaveBeenCalledWith(403); + expect(next).not.toHaveBeenCalled(); + }); + + it('responds 403 for a disabled user who still holds the permission', async () => { + m.getSession.mockResolvedValue({user: {id: 'u1'}}); + m.loadAccess.mockResolvedValue({...userWith(app), disabled: true}); + + const {res, next} = await run(); + + expect(res.status).toHaveBeenCalledWith(403); + expect(next).not.toHaveBeenCalled(); + }); + + it('sets res.locals.admin and calls next() with the permission', async () => { + m.getSession.mockResolvedValue({user: {id: 'u1'}}); + m.loadAccess.mockResolvedValue(userWith(app)); + + const {res, next} = await run(); + + expect(next).toHaveBeenCalled(); + expect(res.locals.admin).toMatchObject({id: 'u1', email: 'a@nachklang.art', displayName: 'Anna Admin'}); + }); + + // Weaker than it looks and kept deliberately: neither module imports + // checkSession any more, so this cannot fail today. It is a tripwire for + // the change that would matter - someone reintroducing a header-session + // fallback "just for the calendar users who have not been invited yet", + // which is exactly the shortcut the cutover exists to close. + it('never falls back to a calendar header session', async () => { + m.getSession.mockResolvedValue(null); + + await run(); + + expect(m.checkSession).not.toHaveBeenCalled(); + }); + }); +}; diff --git a/test/feedback/feedback.auth.test.ts b/test/feedback/feedback.auth.test.ts index a2717c2..9b66930 100644 --- a/test/feedback/feedback.auth.test.ts +++ b/test/feedback/feedback.auth.test.ts @@ -1,88 +1,23 @@ -import {vi, describe, it, expect, beforeEach, type Mock} from 'vitest'; -import {Request, Response} from 'express'; +import {vi, type Mock} from 'vitest'; vi.mock('../../src/models/calendar/users/users.service.js', () => ({ checkSession: vi.fn() })); +vi.mock('../../src/models/admin/admin.auth.js', () => ({ + auth: {api: {getSession: vi.fn()}} +})); +vi.mock('../../src/models/admin/users/users.admin.service.js', () => ({ + loadAccess: vi.fn() +})); import * as UserService from '../../src/models/calendar/users/users.service.js'; -import {requireAdminAuth, sessionHeaderAuthenticator} from '../../src/models/feedback/feedback.auth.js'; +import {auth} from '../../src/models/admin/admin.auth.js'; +import * as UsersService from '../../src/models/admin/users/users.admin.service.js'; +import {requireAdminAuth} from '../../src/models/feedback/feedback.auth.js'; +import {describeAdminBinding} from '../admin/auth-binding.js'; -const mockCheckSession = UserService.checkSession as Mock; - -const makeReq = (headers: Record): Request => { - return { - header: (name: string) => headers[name], - ip: '203.0.113.42' - } as unknown as Request; -}; - -const makeRes = (): Response => { - const res: any = {}; - res.status = vi.fn().mockReturnValue(res); - res.send = vi.fn().mockReturnValue(res); - res.locals = {}; - return res as Response; -}; - -describe('sessionHeaderAuthenticator', () => { - beforeEach(() => mockCheckSession.mockReset()); - - it('returns null when headers are missing', async () => { - const identity = await sessionHeaderAuthenticator(makeReq({})); - expect(identity).toBeNull(); - expect(mockCheckSession).not.toHaveBeenCalled(); - }); - - it('returns null when checkSession finds no user', async () => { - mockCheckSession.mockResolvedValue(null); - const identity = await sessionHeaderAuthenticator(makeReq({'X-Session-Id': '1', 'X-Session-Key': 'k'})); - expect(identity).toBeNull(); - }); - - it('returns null for a valid session on an inactive account', async () => { - mockCheckSession.mockResolvedValue({userId: 1, email: 'a@nachklang.art', fullName: 'A', isActive: false}); - const identity = await sessionHeaderAuthenticator(makeReq({'X-Session-Id': '1', 'X-Session-Key': 'k'})); - expect(identity).toBeNull(); - }); - - it('returns the identity for a valid session on an active account', async () => { - mockCheckSession.mockResolvedValue({userId: 1, email: 'a@nachklang.art', fullName: 'Anna Admin', isActive: true}); - const identity = await sessionHeaderAuthenticator(makeReq({'X-Session-Id': '1', 'X-Session-Key': 'k'})); - expect(identity).toEqual({id: '1', email: 'a@nachklang.art', displayName: 'Anna Admin'}); - }); - - it('passes the session id and key from headers through to checkSession, never from query params', async () => { - mockCheckSession.mockResolvedValue({userId: 1, email: 'a@nachklang.art', fullName: 'A', isActive: true}); - await sessionHeaderAuthenticator(makeReq({'X-Session-Id': '42', 'X-Session-Key': 'sekret'})); - expect(mockCheckSession).toHaveBeenCalledWith('42', 'sekret', '203.0.113.42'); - }); -}); - -describe('requireAdminAuth', () => { - beforeEach(() => mockCheckSession.mockReset()); - - it('responds 401 and does not call next() when unauthenticated', async () => { - mockCheckSession.mockResolvedValue(null); - const req = makeReq({}); - const res = makeRes(); - const next = vi.fn(); - - await requireAdminAuth(req, res, next); - - expect(res.status).toHaveBeenCalledWith(401); - expect(next).not.toHaveBeenCalled(); - }); - - it('sets res.locals.admin and calls next() when authenticated', async () => { - mockCheckSession.mockResolvedValue({userId: 1, email: 'a@nachklang.art', fullName: 'Anna Admin', isActive: true}); - const req = makeReq({'X-Session-Id': '1', 'X-Session-Key': 'k'}); - const res = makeRes(); - const next = vi.fn(); - - await requireAdminAuth(req, res, next); - - expect(next).toHaveBeenCalled(); - expect(res.locals.admin).toEqual({id: '1', email: 'a@nachklang.art', displayName: 'Anna Admin'}); - }); -}); +describeAdminBinding('feedback', 'tickets', requireAdminAuth, () => ({ + getSession: auth.api.getSession as unknown as Mock, + loadAccess: UsersService.loadAccess as Mock, + checkSession: UserService.checkSession as Mock +})); diff --git a/test/integration/admin.auth.test.ts b/test/integration/admin.auth.test.ts index 9a14bc6..468ab23 100644 --- a/test/integration/admin.auth.test.ts +++ b/test/integration/admin.auth.test.ts @@ -221,16 +221,32 @@ describe('requireAppAccess', () => { expect(Array.isArray(res.body)).toBe(true); }); - // Step 2 deliberately does NOT swap the feedback and tickets authenticators: - // they still authenticate against the legacy calendar sessions, so an admin - // cookie means nothing to them yet. This asserts that boundary rather than - // the end state - when step 4 lands, these two expectations become 200/403 - // and this comment goes away. - it('leaves the feedback and tickets admin areas on their legacy authenticator', async () => { + // The step 4 cutover (2026-09-06): the feedback and tickets admin areas now + // sit behind this same gate, so one sign-in reaches every app the user has a + // permission for - and reaches no further. Until step 4 these two returned + // 401 for an admin cookie, because each module still ran its own header + // session against the calendar users table. + it('lets an admin cookie into the feedback and tickets admin areas', async () => { const user = await createAndAcceptInvitation(app, 'o@nachklang.art', 'O', ['feedback', 'tickets']); - expect((await user.agent.get('/feedback/admin/me')).status).toBe(401); - expect((await user.agent.get('/tickets/admin/me')).status).toBe(401); + expect((await user.agent.get('/feedback/admin/me')).status).toBe(200); + expect((await user.agent.get('/tickets/admin/me')).status).toBe(200); + }); + + it('403s each app separately for a user who only holds the other one', async () => { + const user = await createAndAcceptInvitation(app, 'q@nachklang.art', 'Q', ['feedback']); + + expect((await user.agent.get('/feedback/admin/me')).status).toBe(200); + expect((await user.agent.get('/tickets/admin/me')).status).toBe(403); + }); + + it('401s the feedback and tickets admin areas for a legacy header session', async () => { + const res = await request(app) + .get('/feedback/admin/me') + .set('X-Session-Id', '1') + .set('X-Session-Key', 'whatever'); + + expect(res.status).toBe(401); }); }); diff --git a/test/tickets/tickets.auth.test.ts b/test/tickets/tickets.auth.test.ts new file mode 100644 index 0000000..e1f7f0f --- /dev/null +++ b/test/tickets/tickets.auth.test.ts @@ -0,0 +1,23 @@ +import {vi, type Mock} from 'vitest'; + +vi.mock('../../src/models/calendar/users/users.service.js', () => ({ + checkSession: vi.fn() +})); +vi.mock('../../src/models/admin/admin.auth.js', () => ({ + auth: {api: {getSession: vi.fn()}} +})); +vi.mock('../../src/models/admin/users/users.admin.service.js', () => ({ + loadAccess: vi.fn() +})); + +import * as UserService from '../../src/models/calendar/users/users.service.js'; +import {auth} from '../../src/models/admin/admin.auth.js'; +import * as UsersService from '../../src/models/admin/users/users.admin.service.js'; +import {requireAdminAuth} from '../../src/models/tickets/tickets.auth.js'; +import {describeAdminBinding} from '../admin/auth-binding.js'; + +describeAdminBinding('tickets', 'feedback', requireAdminAuth, () => ({ + getSession: auth.api.getSession as unknown as Mock, + loadAccess: UsersService.loadAccess as Mock, + checkSession: UserService.checkSession as Mock +}));