From b848d6eab92fce11d5a50c93dfc4f63c3b77ae38 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Patrick=20M=C3=BCller?= Date: Sun, 6 Sep 2026 22:23:36 +0200 Subject: [PATCH] Move the calendar onto the shared session cookie Step 4 of docs/calendar-auth-migration.md, and the close of DEFERRED_SECURITY.md item 1: no calendar route reads sessionId/sessionKey from the query string any more, so a live credential no longer travels through access logs, browser history and Referer headers. The four write routes sit behind requireAppAccess('calendar'), which also narrows who may edit from "any activated @nachklang.art account" to an explicit per-user permission. They answer 401 signed out and 403 without the permission, where they previously answered 403 for both. The three read routes cannot use the middleware: one URL serves an anonymous visitor, an iCal subscription holding a shared password, and a signed-in editor who should see drafts. They resolve the session optionally instead, and a signed-in user without the calendar permission is treated as anonymous rather than refused - so they keep the public calendar access anyone has. That public calendar staying anonymous is load-bearing: nachklang.art reads it to show the next upcoming event. It is now pinned at both the password-table and the route level, and so is the rule that a shared password can never be used to write. credentials.service.ts loses its session half and becomes the password table it always wanted to be. The shared passwords survive only for iCal clients, which cannot send a cookie. Writes record the author as an admin user id and no longer have a legacy int to write, which is what migration 003 makes room for. /calendar/users/* is left in place: nothing calls it and a session it mints opens nothing, but they are still live password-accepting endpoints, so removing them belongs with the rest of the legacy path in step 5. Co-Authored-By: Claude Opus 5 --- DEFERRED_SECURITY.md | 35 +- docker/init/01-calendar-schema-dev.sql | 3 +- docs/calendar-auth-migration.md | 58 ++- .../003_allow_null_legacy_creator.sql | 32 ++ src/models/admin/admin.auth.ts | 6 +- src/models/admin/admin.config.ts | 6 +- .../calendar/events/credentials.service.ts | 102 +++-- src/models/calendar/events/events.router.ts | 349 +++++++++--------- src/models/calendar/events/events.service.ts | 20 +- test/admin/admin.config.test.ts | 5 +- test/calendar/credentials.service.test.ts | 46 ++- test/calendar/events.router.test.ts | 187 ++++++++++ 12 files changed, 540 insertions(+), 309 deletions(-) create mode 100644 sql/calendar/003_allow_null_legacy_creator.sql create mode 100644 test/calendar/events.router.test.ts diff --git a/DEFERRED_SECURITY.md b/DEFERRED_SECURITY.md index a933937..49383aa 100644 --- a/DEFERRED_SECURITY.md +++ b/DEFERRED_SECURITY.md @@ -5,27 +5,28 @@ These items were identified during a security review on 2026-05-02 and conscious --- -## 1. Session credentials in URL query parameters (logged-in users) +## 1. Session credentials in URL query parameters (logged-in users) — CLOSED 2026-09-06 **Files:** `src/models/calendar/events/events.router.ts` — all GET/PUT/DELETE handlers -`sessionId` and `sessionKey` are currently read from query parameters, which means they appear in server access logs, browser history, proxy logs, and `Referer` headers. +`sessionId` and `sessionKey` were read from query parameters, which meant they appeared in +server access logs, browser history, proxy logs, and `Referer` headers. -**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. +**Fixed** by step 4 of `docs/calendar-auth-migration.md`: the calendar's write routes now sit +behind `requireAppAccess('calendar')` against the better-auth session cookie, and the read +routes resolve the same cookie optionally. No route reads `sessionId`/`sessionKey` any more, +and the Angular frontend sends `withCredentials` instead of appending them to every URL. That +closed the item outright rather than moving the credential somewhere safer. -~~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. +Two things this did *not* change, both deliberate: -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. +- The shared calendar `password` parameter stays. An iCal client cannot send a cookie, so + this is the one caller that genuinely needs a credential in the URL. It grants read access + to one calendar and nothing else - `test/calendar/events.router.test.ts` pins that it can + never be used to write. +- The legacy `/calendar/users/*` routes still exist. Nothing calls them any more, and a + legacy session they mint no longer opens anything, but they are still live + password-accepting endpoints. Step 5 removes them. --- @@ -36,9 +37,9 @@ Either fix requires a corresponding frontend update. - `PUT /move/:eventId` (move) - `DELETE /:eventId` (delete) -Currently any active user can edit, move, or delete any event regardless of who created it. This is acceptable while all users are trusted admins. +Currently any account holding the `calendar` permission can edit, move, or delete any event regardless of who created it. This is acceptable while everyone holding it is a trusted admin. -**Fix:** When non-admin users are introduced, fetch the event first and verify `event.createdById === user.userId` before allowing the mutation. Add an `isAdmin` flag to the user model to let admins bypass the check. +**Fix (updated 2026-09-06):** fetch the event first and verify `event.createdByUserId === res.locals.admin.id` before allowing the mutation — `createdById`, the legacy INT, is no longer written and is gone at step 5. Rather than an `isAdmin` flag, the bypass belongs in the permission model that already exists: `requireAppAccess('calendar', 'manage')` alongside the current `access` role, which needs a row in `APP_ROLES` on both sides and nothing else. --- diff --git a/docker/init/01-calendar-schema-dev.sql b/docker/init/01-calendar-schema-dev.sql index da03336..06e8c6c 100644 --- a/docker/init/01-calendar-schema-dev.sql +++ b/docker/init/01-calendar-schema-dev.sql @@ -41,7 +41,8 @@ CREATE TABLE `events` ( `calendar_id` int(11) NOT NULL, `uuid` text NOT NULL, `created_date` datetime DEFAULT current_timestamp(), - `created_by_id` int(11) NOT NULL, + -- Nullable since the cutover; see sql/calendar/003_allow_null_legacy_creator.sql. + `created_by_id` int(11) DEFAULT NULL, -- Bridge to the admin module's user ids; see sql/calendar/001_add_admin_user_bridge.sql. `created_by_user_id` varchar(36) CHARACTER SET utf8mb4 COLLATE utf8mb4_unicode_ci DEFAULT NULL, -- Archived creator name; see sql/calendar/002_snapshot_legacy_creator_names.sql. diff --git a/docs/calendar-auth-migration.md b/docs/calendar-auth-migration.md index 63dc449..c9a229c 100644 --- a/docs/calendar-auth-migration.md +++ b/docs/calendar-auth-migration.md @@ -1,7 +1,7 @@ # Migrating the Calendar domain onto the admin identity module -Status: **steps 1 and 3 done** (2026-09-06), step 2 dropped by decision, part of step 5 -brought forward, step 4 next. +Status: **steps 1-4 done** (2026-09-06), step 2 dropped by decision, part of step 5 brought +forward. Only step 5, the removal of the legacy path, is left. Written 2026-09-05 alongside the admin module (step 2 of `docs/plan-admin-auth.md` in the nachklang-admin repo), which deliberately left the calendar alone. Steps 1-4 of that plan @@ -77,24 +77,50 @@ Each step is meant to leave production working on its own. **Writes are not dual-written**, contrary to the original plan: before the cutover the request only ever carries a legacy session, so there is no admin id available to write. Writes start filling `created_by_user_id` (and stop filling `created_by_id`) in step 4. -4. **Switch the routes.** Replace the query-parameter session checks in - `events.router.ts` and `users.router.ts` with `requireAppAccess('calendar')`, and change - the Angular frontend to `withCredentials: true` against the same origin list. Deploy the - API first; the calendar frontend is broken between the two deploys, so pick a quiet - time. This closes `DEFERRED_SECURITY.md` item 1. +4. **Switch the routes.** ~~Replace the query-parameter session checks in `events.router.ts` + and `users.router.ts` with `requireAppAccess('calendar')`, and change the Angular frontend + to `withCredentials: true`.~~ **Done 2026-09-06.** `DEFERRED_SECURITY.md` item 1 is closed: + no route reads `sessionId`/`sessionKey` any more. - Two things this step has to carry that the original sequence put in step 5: + How it came out, route by route: - - **`sql/calendar/003_*.sql` must make `events.created_by_id` nullable** (`MODIFY - created_by_id INT NULL`). It is `NOT NULL` today, so the first event created after the + - The four write routes sit behind `requireAppAccess('calendar')` as middleware. They + answer 401 when signed out and 403 without the permission, where they used to answer 403 + for both. + - The three read routes cannot use middleware - the same URL serves an anonymous visitor, + an iCal subscription holding a shared password, and a signed-in editor who should see + drafts. They call `resolveAccess` optionally instead (`signedInEditor` in the router), + and a signed-in user *without* the calendar permission is treated as anonymous rather + than refused, so they keep their access to the public calendar. + - `credentials.service.ts` lost its session half entirely and is now just the password + table. `hasAccess(calendar, password)`. + - `/calendar/users/*` was left alone. Nothing calls it and a session it mints opens + nothing, but they are live password-accepting endpoints - step 5 removes them. + + Also: `calendar.nachklang.art` joined `DEFAULT_APP_ORIGINS` (better-auth `trustedOrigins`, + without which sign-out from the calendar fails while everything else works), and + `localhost:4200` joined the dev origins for the same reason. + + Two things this step had to carry that the original sequence put in step 5: + + - **`sql/calendar/003_allow_null_legacy_creator.sql` makes `events.created_by_id` nullable** + (`MODIFY created_by_id INT NULL`). It is `NOT NULL` today, so the first event created after the cutover would otherwise fail to insert - there is no legacy int id to write any more. `event_versions.version_created_by_id` is already nullable. The foreign key can stay - until step 5; it permits NULL. The same migration should re-run 002's idempotent - name backfill, to catch anything created between the two deploys. - - **The public calendar must stay anonymous.** `hasAccess('public')` returns true before - any credential check, and nachklang.art reads `/calendar/events/public/json` and - `/public/json/next` with no session at all. Pinned by - `test/calendar/credentials.service.test.ts` so this cannot regress quietly. + until step 5; it permits NULL. It also re-runs 002's idempotent name backfill, to catch + anything created between the two migrations. Applying it early is safe - widening a + column to accept NULL cannot break the running pre-cutover build. + - **The public calendar stays anonymous.** `hasAccess('public')` returns true before any + credential check, and nachklang.art reads `/calendar/events/public/json` and + `/public/json/next` with no session at all. Pinned at both levels - the password table in + `test/calendar/credentials.service.test.ts`, the routes themselves in + `test/calendar/events.router.test.ts` - so this cannot regress quietly. + + **Deploy order:** migration 003, then the API, then the calendar frontend. The frontend is + broken between the last two (its old bundle sends query credentials the new API ignores), + so pick a quiet moment. Production also needs `calendar.nachklang.art` in the admin app's + `NEXT_PUBLIC_ALLOWED_REDIRECT_ORIGINS`, which is a **build-time** value: a rebuild, not a + restart. 5. **Drop the legacy path.** Remove `users.service.ts`'s session handling, the `sessions` table, `created_by_id`, and the legacy half of the step 3 read (the `users` join and its `legacy_*` aliases - the snapshot fallback stays, it is what makes dropping the table diff --git a/sql/calendar/003_allow_null_legacy_creator.sql b/sql/calendar/003_allow_null_legacy_creator.sql new file mode 100644 index 0000000..2c740bb --- /dev/null +++ b/sql/calendar/003_allow_null_legacy_creator.sql @@ -0,0 +1,32 @@ +-- Nachklang e.V. Calendar module — step 4 of docs/calendar-auth-migration.md, +-- the cutover. Apply manually against the CALENDAR_DB database, after 002, +-- and BEFORE deploying the API build that goes with it: +-- mysql -h -u -p < 003_allow_null_legacy_creator.sql +-- +-- From the cutover on, an event's creator is an admin-module user id. There is +-- no legacy calendar user id to write any more, and `events.created_by_id` is +-- NOT NULL - so without this the very first event created after the deploy +-- fails to insert. `event_versions.version_created_by_id` is already nullable. +-- +-- The foreign key to `users` is kept: it permits NULL, so it costs nothing +-- until step 5 drops the column and the table together. +-- +-- Applying this early is harmless. Widening a column to accept NULL cannot +-- break the running pre-cutover build, which always supplies a value, so this +-- can go out ahead of the deploy rather than during it. + +ALTER TABLE `events` + MODIFY COLUMN `created_by_id` INT(11) NULL DEFAULT NULL; + +-- Re-run of 002's backfill, to catch anything created between the two +-- migrations while the legacy path was still writing events. Idempotent by +-- construction: it only touches rows that have no snapshot yet. +UPDATE `events` e + JOIN `users` u ON u.user_id = e.created_by_id + SET e.created_by_name = u.full_name + WHERE e.created_by_name IS NULL; + +UPDATE `event_versions` v + JOIN `users` u ON u.user_id = v.version_created_by_id + SET v.version_created_by_name = u.full_name + WHERE v.version_created_by_name IS NULL; diff --git a/src/models/admin/admin.auth.ts b/src/models/admin/admin.auth.ts index 15fdc60..631ca41 100644 --- a/src/models/admin/admin.auth.ts +++ b/src/models/admin/admin.auth.ts @@ -33,7 +33,11 @@ const localhostOrigins = [ 'http://localhost:3000', 'http://localhost:3001', 'http://localhost:3002', - 'http://localhost:3003' + 'http://localhost:3003', + // The Angular calendar frontend; `ng serve` defaults to 4200. Missing from + // this list, sign-out from the calendar answers 403 in dev only, which is a + // confusing thing to debug against a production config that is fine. + 'http://localhost:4200' ]; const trustedOrigins = isProd diff --git a/src/models/admin/admin.config.ts b/src/models/admin/admin.config.ts index 75a1a2e..c4c8481 100644 --- a/src/models/admin/admin.config.ts +++ b/src/models/admin/admin.config.ts @@ -79,7 +79,8 @@ const parseList = (value: string | undefined, fallback: string[]): string[] => { * 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. + * origin that ever reached /admin/auth was the admin app itself. The calendar + * joined them with its own cutover (docs/calendar-auth-migration.md step 4). * * 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, @@ -89,7 +90,8 @@ const parseList = (value: string | undefined, fallback: string[]): string[] => { */ const DEFAULT_APP_ORIGINS = [ 'https://tickets.nachklang.art', - 'https://feedback.nachklang.art' + 'https://feedback.nachklang.art', + 'https://calendar.nachklang.art' ]; export const APP_ORIGINS = parseList(process.env.APP_ORIGINS, DEFAULT_APP_ORIGINS) diff --git a/src/models/calendar/events/credentials.service.ts b/src/models/calendar/events/credentials.service.ts index 589068b..1ac701b 100644 --- a/src/models/calendar/events/credentials.service.ts +++ b/src/models/calendar/events/credentials.service.ts @@ -1,73 +1,55 @@ import * as dotenv from 'dotenv'; -import * as UserService from '../users/users.service.js'; - dotenv.config(); /** - * Checks if the password gives admin privileges (view / create / edit / delete) - * @param password + * The shared calendar passwords, and nothing else. + * + * Before the step 4 cutover each function here also took a sessionId/sessionKey + * pair and checked it against the calendar's own sessions table, so "is this a + * signed-in user?" and "did they send the right shared password?" were tangled + * together in five places. Signed-in access is now decided by + * requireAppAccess('calendar') before the handler runs; what is left is the + * fallback for people who have no account at all. + * + * That fallback survives on purpose, for one reason: an iCal client subscribing + * to a calendar URL cannot send a cookie. Everything the Angular app does goes + * through the session cookie instead. See docs/calendar-auth-migration.md. + * + * `public` is deliberately open to everyone with no credential of any kind - + * nachklang.art reads it anonymously to show the next upcoming event. Pinned by + * test/calendar/credentials.service.test.ts. */ -export const checkAdminPrivileges = async (sessionId: string, sessionKey: string, ip: string) => { - if(sessionId) { - let user = await UserService.checkSession(sessionId, sessionKey, ip); - return user?.isActive ?? false; - } - return false; -} -/** - * Checks if the password gives member view privileges - * @param password - */ -export const checkMemberPrivileges = async (sessionId: string, sessionKey: string, password: string, ip: string) => { - if(sessionId) { - let user = await UserService.checkSession(sessionId, sessionKey, ip); - return user?.isActive ?? false; - } - - return password == process.env.MEMBER_CREDENTIAL; -} - -/** - * Checks if the password gives choir view privileges - * @param password - */ -export const checkChoirPrivileges = async (sessionId: string, sessionKey: string, password: string, ip: string) => { - if(sessionId) { - let user = await UserService.checkSession(sessionId, sessionKey, ip); - return user?.isActive ?? false; - } - - return password == process.env.CHOIR_CREDENTIAL; -} - -/** - * Checks if the password gives management view privileges - * @param password - */ -export const checkManagementPrivileges = async (sessionId: string, sessionKey: string, password: string, ip: string) => { - if(sessionId) { - let user = await UserService.checkSession(sessionId, sessionKey, ip); - return user?.isActive ?? false; - } - - return password == process.env.MANAGEMENT_CREDENTIAL; -} - -export const hasAccess = async (calendarName: string, sessionId: string, sessionKey: string, password: string, ip: string) => { +const credentialFor = (calendarName: string): string | undefined => { switch (calendarName) { - case 'public': - return true; case 'members': - return await checkMemberPrivileges(sessionId, sessionKey, password, ip); + return process.env.MEMBER_CREDENTIAL; case 'choir': - return await checkChoirPrivileges(sessionId, sessionKey, password, ip); + case 'birthdays': + return process.env.CHOIR_CREDENTIAL; case 'management': - return await checkManagementPrivileges(sessionId, sessionKey, password, ip); - case 'birthdays': - return await checkChoirPrivileges(sessionId, sessionKey, password, ip); + return process.env.MANAGEMENT_CREDENTIAL; default: - return false; + return undefined; } -} +}; + +/** + * Whether the given shared password opens the given calendar. Answers false + * for an unknown calendar, and - importantly - for a calendar whose credential + * is not configured at all: an unset MEMBER_CREDENTIAL must not turn into + * "everyone with an empty password gets in". + */ +export const hasAccess = async (calendarName: string, password: string): Promise => { + if (calendarName === 'public') { + return true; + } + + const expected = credentialFor(calendarName); + if (!expected) { + return false; + } + + return password === expected; +}; diff --git a/src/models/calendar/events/events.router.ts b/src/models/calendar/events/events.router.ts index 18d3fc5..af8f15f 100644 --- a/src/models/calendar/events/events.router.ts +++ b/src/models/calendar/events/events.router.ts @@ -7,7 +7,7 @@ import {Event} from './event.interface.js'; import * as EventService from './events.service.js'; import * as iCalService from './icalgenerator.service.js'; import * as CredentialService from './credentials.service.js'; -import * as UserService from '../users/users.service.js'; +import {requireAppAccess, resolveAccess, AdminAccess} from '../../admin/admin.middleware.js'; import {Guid} from 'guid-typescript'; import logger from '../../../middleware/logger.js'; @@ -29,6 +29,44 @@ export const calendarNames = new Map([ ['birthdays', {id: 5, name: 'Nachklang_birthday_calendar'}] ]); +/** + * The gate on everything that writes. Step 4 of + * docs/calendar-auth-migration.md replaced a sessionId/sessionKey pair in the + * query string (DEFERRED_SECURITY.md item 1) with the same session cookie the + * other three apps use, and "any activated @nachklang.art account" with an + * explicit per-user calendar permission. + */ +const requireCalendarAccess = requireAppAccess('calendar'); + +/** Set by requireCalendarAccess; the writer's admin identity. */ +const adminOf = (res: Response): AdminAccess => res.locals.admin as AdminAccess; + +/** + * Resolves a signed-in calendar user for the *read* routes, or null. + * + * Reads cannot use the middleware: the same URL serves an anonymous visitor + * (the public calendar the website polls), someone holding a shared password + * (an iCal subscription), and a signed-in editor who should see drafts. So it + * answers "who is this, if anyone?" instead of refusing the request, and each + * handler decides what that means. + * + * A failure to reach the admin database is swallowed for the same reason the + * name lookup in events.service.ts swallows one: it must not be able to take + * the anonymous public calendar down. + */ +const signedInEditor = async (req: Request): Promise => { + try { + const access = await resolveAccess(req); + if (!access || access.disabled || !access.apps.includes('calendar')) { + return null; + } + return access; + } catch (e: any) { + logger.warn('Calendar: could not resolve the session, continuing as anonymous: ' + e.message); + return null; + } +}; + /** * Controller Definitions @@ -39,7 +77,10 @@ export const calendarNames = new Map([ * /calendar/events/{calendar}/json: * get: * summary: Get all events from a specific calendar in JSON format - * description: Returns all events from the specified calendar in JSON format. Authentication required. + * description: > + * Returns the calendar's events. The public calendar is open to everyone; the + * others need either a signed-in account with the calendar permission - which + * also unlocks drafts - or the calendar's shared password. * tags: * - calendar * parameters: @@ -51,20 +92,10 @@ export const calendarNames = new Map([ * enum: [public, members, choir, management] * description: The name of the calendar to get events from * - in: query - * name: sessionId - * schema: - * type: string - * description: Session ID for authentication - * - in: query - * name: sessionKey - * schema: - * type: string - * description: Session key for authentication - * - in: query * name: password * schema: * type: string - * description: Password for calendar access (if not using session authentication) + * description: The calendar's shared password, for callers with no account * responses: * 200: * description: Success @@ -109,10 +140,7 @@ eventsRouter.get('/:calendar/json', async (req: Request, res: Response) => { try { // Get request params let calendarName: string = req.params.calendar as string ?? ''; - let sessionId: string = req.query.sessionId as string ?? ''; - let sessionKey: string = req.query.sessionKey as string ?? ''; let password: string = req.query.password as string ?? ''; - let ip: string = req.socket.remoteAddress ?? ''; if (calendarName.length < 1) { res.status(400).send({'message': 'Please state the name of the calendar you want events from.'}); @@ -126,23 +154,19 @@ eventsRouter.get('/:calendar/json', async (req: Request, res: Response) => { let calendarId: number = calendarNames.get(calendarName)!.id; - let user = await UserService.checkSession(sessionId, sessionKey, ip); + const editor = await signedInEditor(req); - // If no user was found, check if the password gives access to the calendar - if(user === null || !user.isActive) { - if (! await CredentialService.hasAccess(calendarName, sessionId, sessionKey, password, ip)) { - res.status(403).send({'message': 'You do not have access to the specified calendar.'}); - return; - } + // Not signed in: fall back to the shared password for this calendar. + if (!editor && ! await CredentialService.hasAccess(calendarName, password)) { + res.status(403).send({'message': 'You do not have access to the specified calendar.'}); + return; } - let events: Event[]; - - if(user?.isActive) { - events = await EventService.getAllEventsAdmin(calendarId); - } else { - events = await EventService.getAllEvents(calendarId); - } + // Editors get the admin view (drafts included, calendar includes ignored); + // everyone else gets published events only. + let events: Event[] = editor + ? await EventService.getAllEventsAdmin(calendarId) + : await EventService.getAllEvents(calendarId); // Send the events back res.status(200).send(events); @@ -170,20 +194,10 @@ eventsRouter.get('/:calendar/json', async (req: Request, res: Response) => { * enum: [public, members, choir, management] * description: The name of the calendar to get the next event from * - in: query - * name: sessionId - * schema: - * type: string - * description: Session ID for authentication - * - in: query - * name: sessionKey - * schema: - * type: string - * description: Session key for authentication - * - in: query * name: password * schema: * type: string - * description: Password for calendar access (if not using session authentication) + * description: The calendar's shared password, for callers with no account * responses: * 200: * description: Success @@ -242,10 +256,7 @@ eventsRouter.get('/:calendar/json/next', async (req: Request, res: Response) => try { // Get request params let calendarName: string = req.params.calendar as string ?? ''; - let sessionId: string = req.query.sessionId as string ?? ''; - let sessionKey: string = req.query.sessionKey as string ?? ''; let password: string = req.query.password as string ?? ''; - let ip: string = req.socket.remoteAddress ?? ''; if (calendarName.length < 1) { res.status(400).send({'message': 'Please state the name of the calendar you want events from.'}); @@ -259,7 +270,10 @@ eventsRouter.get('/:calendar/json/next', async (req: Request, res: Response) => let calendarId: number = calendarNames.get(calendarName)!.id; - if (! await CredentialService.hasAccess(calendarName, sessionId, sessionKey, password, ip)) { + // Signed in, or holding the calendar's shared password. The password path + // is what keeps iCal subscriptions working - a calendar client cannot + // send a cookie. + if (!await signedInEditor(req) && ! await CredentialService.hasAccess(calendarName, password)) { res.status(403).send({'message': 'You do not have access to the specified calendar.'}); return; } @@ -302,20 +316,10 @@ eventsRouter.get('/:calendar/json/next', async (req: Request, res: Response) => * enum: [public, members, choir, management] * description: The name of the calendar to get events from * - in: query - * name: sessionId - * schema: - * type: string - * description: Session ID for authentication - * - in: query - * name: sessionKey - * schema: - * type: string - * description: Session key for authentication - * - in: query * name: password * schema: * type: string - * description: Password for calendar access (if not using session authentication) + * description: The calendar's shared password, for callers with no account * responses: * 200: * description: Success - returns iCal file @@ -365,10 +369,7 @@ eventsRouter.get('/:calendar/ical', async (req: Request, res: Response) => { try { // Get request params let calendarName: string = req.params.calendar as string ?? ''; - let sessionId: string = req.query.sessionId as string ?? ''; - let sessionKey: string = req.query.sessionKey as string ?? ''; let password: string = req.query.password as string ?? ''; - let ip: string = req.socket.remoteAddress ?? ''; if (calendarName.length < 1) { res.status(400).send({'message': 'Please state the name of the calendar you want events from.'}); @@ -382,7 +383,10 @@ eventsRouter.get('/:calendar/ical', async (req: Request, res: Response) => { let calendarId: number = calendarNames.get(calendarName)!.id; - if (! await CredentialService.hasAccess(calendarName, sessionId, sessionKey, password, ip)) { + // Signed in, or holding the calendar's shared password. The password path + // is what keeps iCal subscriptions working - a calendar client cannot + // send a cookie. + if (!await signedInEditor(req) && ! await CredentialService.hasAccess(calendarName, password)) { res.status(403).send({'message': 'You do not have access to the specified calendar.'}); return; } @@ -413,22 +417,11 @@ eventsRouter.get('/:calendar/ical', async (req: Request, res: Response) => { * /calendar/events: * post: * summary: Create a new event - * description: Creates a new event in the specified calendar. Authentication required. + * description: Creates a new event. Requires a signed-in account with the calendar permission. * tags: * - calendar - * parameters: - * - in: query - * name: sessionId - * required: true - * schema: - * type: string - * description: Session ID for authentication - * - in: query - * name: sessionKey - * required: true - * schema: - * type: string - * description: Session key for authentication + * security: + * - AdminSessionCookie: [] * requestBody: * required: true * content: @@ -495,16 +488,32 @@ eventsRouter.get('/:calendar/ical', async (req: Request, res: Response) => { * message: * type: string * example: Required parameters missing - * 403: - * description: Forbidden - no access to create events + * 401: + * description: Unauthorized - not signed in * content: * application/json: * schema: * type: object * properties: + * status: + * type: string + * example: UNAUTHORIZED * message: * type: string - * example: You do not have access to the specified calendar. + * example: Anmeldung erforderlich. + * 403: + * description: Forbidden - the account lacks the calendar permission + * content: + * application/json: + * schema: + * type: object + * properties: + * status: + * type: string + * example: FORBIDDEN + * message: + * type: string + * example: "Für diesen Bereich fehlt dir die Berechtigung." * 500: * description: Server error * content: @@ -522,19 +531,9 @@ eventsRouter.get('/:calendar/ical', async (req: Request, res: Response) => { * type: string * example: 6ec1361c-4175-4e81-b2ef-a0792a9a1dc3 */ -eventsRouter.post('/', async (req: Request, res: Response) => { +eventsRouter.post('/', requireCalendarAccess, async (req: Request, res: Response) => { try { - // Get params - let sessionId: string = req.query.sessionId as string ?? ''; - let sessionKey: string = req.query.sessionKey as string ?? ''; - let ip: string = req.socket.remoteAddress ?? ''; - - let user = await UserService.checkSession(sessionId, sessionKey, ip); - - if (!user?.isActive) { - res.status(403).send({'message': 'You do not have access to the specified calendar.'}); - return; - } + const admin = adminOf(res); if ( req.body.calendarId === undefined || @@ -556,7 +555,9 @@ eventsRouter.post('/', async (req: Request, res: Response) => { endDateTime: new Date(req.body.endDateTime), createdDate: new Date(), location: req.body.location ?? '', - createdById: user.userId ?? -1, + // LEGACY createdById is deliberately not set: there is no calendar + // user id any more, and migration 003 made the column nullable. + createdByUserId: admin.id, url: req.body.url ?? '', wholeDay: req.body.wholeDay ?? false, repeatFrequency: req.body.repeatFrequency ?? '', @@ -585,9 +586,11 @@ eventsRouter.post('/', async (req: Request, res: Response) => { * /calendar/events/{eventId}: * put: * summary: Update an existing event - * description: Updates an existing event with the provided data. Authentication required. + * description: Updates an existing event. Requires a signed-in account with the calendar permission. * tags: * - calendar + * security: + * - AdminSessionCookie: [] * parameters: * - in: path * name: eventId @@ -595,18 +598,6 @@ eventsRouter.post('/', async (req: Request, res: Response) => { * schema: * type: integer * description: The ID of the event to update - * - in: query - * name: sessionId - * required: true - * schema: - * type: string - * description: Session ID for authentication - * - in: query - * name: sessionKey - * required: true - * schema: - * type: string - * description: Session key for authentication * requestBody: * required: true * content: @@ -673,16 +664,32 @@ eventsRouter.post('/', async (req: Request, res: Response) => { * message: * type: string * example: Required parameters missing - * 403: - * description: Forbidden - no access to update events + * 401: + * description: Unauthorized - not signed in * content: * application/json: * schema: * type: object * properties: + * status: + * type: string + * example: UNAUTHORIZED * message: * type: string - * example: You do not have access to the specified calendar. + * example: Anmeldung erforderlich. + * 403: + * description: Forbidden - the account lacks the calendar permission + * content: + * application/json: + * schema: + * type: object + * properties: + * status: + * type: string + * example: FORBIDDEN + * message: + * type: string + * example: "Für diesen Bereich fehlt dir die Berechtigung." * 500: * description: Server error * content: @@ -700,19 +707,9 @@ eventsRouter.post('/', async (req: Request, res: Response) => { * type: string * example: 6ec1361c-4175-4e81-b2ef-a0792a9a1dc3 */ -eventsRouter.put('/:eventId', async (req: Request, res: Response) => { +eventsRouter.put('/:eventId', requireCalendarAccess, async (req: Request, res: Response) => { try { - // Get params - let sessionId: string = req.query.sessionId as string ?? ''; - let sessionKey: string = req.query.sessionKey as string ?? ''; - let ip: string = req.socket.remoteAddress ?? ''; - - let user = await UserService.checkSession(sessionId, sessionKey, ip); - - if (!user?.isActive) { - res.status(403).send({'message': 'You do not have access to the specified calendar.'}); - return; - } + const admin = adminOf(res); if ( req.params.eventId === undefined || @@ -736,7 +733,9 @@ eventsRouter.put('/:eventId', async (req: Request, res: Response) => { createdDate: new Date(), location: req.body.location ?? '', createdBy: req.body.createdBy ?? '', - createdById: user.userId ?? -1, + // LEGACY createdById is deliberately not set: there is no calendar + // user id any more, and migration 003 made the column nullable. + createdByUserId: admin.id, url: req.body.url ?? '', wholeDay: req.body.wholeDay ?? false, repeatFrequency: req.body.repeatFrequency ?? '', @@ -768,9 +767,11 @@ eventsRouter.put('/:eventId', async (req: Request, res: Response) => { * /calendar/events/move/{eventId}: * put: * summary: Move an event to a different calendar - * description: Moves an existing event to a different calendar. Authentication required. + * description: Moves an event to a different calendar. Requires a signed-in account with the calendar permission. * tags: * - calendar + * security: + * - AdminSessionCookie: [] * parameters: * - in: path * name: eventId @@ -778,18 +779,6 @@ eventsRouter.put('/:eventId', async (req: Request, res: Response) => { * schema: * type: integer * description: The ID of the event to move - * - in: query - * name: sessionId - * required: true - * schema: - * type: string - * description: Session ID for authentication - * - in: query - * name: sessionKey - * required: true - * schema: - * type: string - * description: Session key for authentication * requestBody: * required: true * content: @@ -854,16 +843,32 @@ eventsRouter.put('/:eventId', async (req: Request, res: Response) => { * message: * type: string * example: Required parameters missing - * 403: - * description: Forbidden - no access to move events + * 401: + * description: Unauthorized - not signed in * content: * application/json: * schema: * type: object * properties: + * status: + * type: string + * example: UNAUTHORIZED * message: * type: string - * example: You do not have access to the specified calendar. + * example: Anmeldung erforderlich. + * 403: + * description: Forbidden - the account lacks the calendar permission + * content: + * application/json: + * schema: + * type: object + * properties: + * status: + * type: string + * example: FORBIDDEN + * message: + * type: string + * example: "Für diesen Bereich fehlt dir die Berechtigung." * 500: * description: Server error * content: @@ -881,19 +886,9 @@ eventsRouter.put('/:eventId', async (req: Request, res: Response) => { * type: string * example: 6ec1361c-4175-4e81-b2ef-a0792a9a1dc3 */ -eventsRouter.put('/move/:eventId', async (req: Request, res: Response) => { +eventsRouter.put('/move/:eventId', requireCalendarAccess, async (req: Request, res: Response) => { try { - // Get params - let sessionId: string = req.query.sessionId as string ?? ''; - let sessionKey: string = req.query.sessionKey as string ?? ''; - let ip: string = req.socket.remoteAddress ?? ''; - - let user = await UserService.checkSession(sessionId, sessionKey, ip); - - if (!user?.isActive) { - res.status(403).send({'message': 'You do not have access to the specified calendar.'}); - return; - } + const admin = adminOf(res); if ( req.params.eventId === undefined || @@ -914,7 +909,9 @@ eventsRouter.put('/move/:eventId', async (req: Request, res: Response) => { createdDate: new Date(), location: req.body.location ?? '', createdBy: req.body.createdBy ?? '', - createdById: user.userId ?? -1, + // LEGACY createdById is deliberately not set: there is no calendar + // user id any more, and migration 003 made the column nullable. + createdByUserId: admin.id, url: req.body.url ?? '', wholeDay: req.body.wholeDay ?? false, repeatFrequency: req.body.repeatFrequency ?? '', @@ -944,9 +941,11 @@ eventsRouter.put('/move/:eventId', async (req: Request, res: Response) => { * /calendar/events/{eventId}: * delete: * summary: Delete an event - * description: Deletes an existing event. Authentication required. + * description: Deletes an event. Requires a signed-in account with the calendar permission. * tags: * - calendar + * security: + * - AdminSessionCookie: [] * parameters: * - in: path * name: eventId @@ -954,18 +953,6 @@ eventsRouter.put('/move/:eventId', async (req: Request, res: Response) => { * schema: * type: integer * description: The ID of the event to delete - * - in: query - * name: sessionId - * required: true - * schema: - * type: string - * description: Session ID for authentication - * - in: query - * name: sessionKey - * required: true - * schema: - * type: string - * description: Session key for authentication * responses: * 200: * description: Event deleted successfully @@ -987,16 +974,32 @@ eventsRouter.put('/move/:eventId', async (req: Request, res: Response) => { * message: * type: string * example: Required parameters missing - * 403: - * description: Forbidden - no access to delete events + * 401: + * description: Unauthorized - not signed in * content: * application/json: * schema: * type: object * properties: + * status: + * type: string + * example: UNAUTHORIZED * message: * type: string - * example: You do not have access to the specified calendar. + * example: Anmeldung erforderlich. + * 403: + * description: Forbidden - the account lacks the calendar permission + * content: + * application/json: + * schema: + * type: object + * properties: + * status: + * type: string + * example: FORBIDDEN + * message: + * type: string + * example: "Für diesen Bereich fehlt dir die Berechtigung." * 500: * description: Server error * content: @@ -1014,19 +1017,9 @@ eventsRouter.put('/move/:eventId', async (req: Request, res: Response) => { * type: string * example: 6ec1361c-4175-4e81-b2ef-a0792a9a1dc3 */ -eventsRouter.delete('/:eventId', async (req: Request, res: Response) => { +eventsRouter.delete('/:eventId', requireCalendarAccess, async (req: Request, res: Response) => { try { - // Get params - let sessionId: string = req.query.sessionId as string ?? ''; - let sessionKey: string = req.query.sessionKey as string ?? ''; - let ip: string = req.socket.remoteAddress ?? ''; - - let user = await UserService.checkSession(sessionId, sessionKey, ip); - - if (!user?.isActive) { - res.status(403).send({'message': 'You do not have access to the specified calendar.'}); - return; - } + const admin = adminOf(res); if ( req.params.eventId === undefined @@ -1046,7 +1039,9 @@ eventsRouter.delete('/:eventId', async (req: Request, res: Response) => { createdDate: new Date(), location: '', createdBy: '', - createdById: user.userId ?? -1, + // LEGACY createdById is deliberately not set: there is no calendar + // user id any more, and migration 003 made the column nullable. + createdByUserId: admin.id, url: '', wholeDay: false, repeatFrequency: '', diff --git a/src/models/calendar/events/events.service.ts b/src/models/calendar/events/events.service.ts index 8cffb46..0f29f03 100644 --- a/src/models/calendar/events/events.service.ts +++ b/src/models/calendar/events/events.service.ts @@ -30,6 +30,10 @@ dotenv.config(); * 3. The admin module's `user.name`, looked up live for rows that carry an * admin id. It wins because it is the only one that follows a rename. * + * Writes only ever set the admin id: since the step 4 cutover there is no + * calendar user id to write, which is why migration 003 made `created_by_id` + * nullable. The reads below still handle rows that predate that. + * * Removal note: everything marked LEGACY below comes out in step 5, together * with the `users`/`sessions` tables and the `created_by_id` columns. The * snapshot stays - it is the reason step 5 can drop them. @@ -247,11 +251,11 @@ export const createEvent = async (event: Event): Promise => { try { await conn.beginTransaction(); let eventUUID = Guid.create().toString(); - const eventsQuery = 'INSERT INTO events (calendar_id, uuid, created_by_id) VALUES (?,?,?) RETURNING event_id'; - const eventsRes = await conn.execute(eventsQuery, [event.calendarId, eventUUID, event.createdById]); + const eventsQuery = 'INSERT INTO events (calendar_id, uuid, created_by_user_id) VALUES (?,?,?) RETURNING event_id'; + const eventsRes = await conn.execute(eventsQuery, [event.calendarId, eventUUID, event.createdByUserId ?? null]); - const versionQuery = 'INSERT INTO event_versions (event_id, name, description, start_datetime, end_datetime, whole_day, repeat_frequency, location, url, status, version_created_by_id) VALUES (?,?,?,?,?,?,?,?,?,?,?);' - await conn.execute(versionQuery, [eventsRes[0].event_id, event.name, event.description, event.startDateTime, event.endDateTime, event.wholeDay, event.repeatFrequency, event.location, event.url, event.status, event.createdById]); + const versionQuery = 'INSERT INTO event_versions (event_id, name, description, start_datetime, end_datetime, whole_day, repeat_frequency, location, url, status, version_created_by_user_id) VALUES (?,?,?,?,?,?,?,?,?,?,?);' + await conn.execute(versionQuery, [eventsRes[0].event_id, event.name, event.description, event.startDateTime, event.endDateTime, event.wholeDay, event.repeatFrequency, event.location, event.url, event.status, event.createdByUserId ?? null]); await conn.commit(); @@ -272,8 +276,8 @@ export const updateEvent = async (event: Event): Promise => { let conn = await NachklangCalendarDB.getConnection(); try { await conn.beginTransaction(); - const versionQuery = 'INSERT INTO event_versions (event_id, name, description, start_datetime, end_datetime, whole_day, repeat_frequency, location, url, status, version_created_by_id) VALUES (?,?,?,?,?,?,?,?,?,?,?);' - const versionRes = await conn.execute(versionQuery, [event.eventId, event.name, event.description, event.startDateTime, event.endDateTime, event.wholeDay, event.repeatFrequency, event.location, event.url, event.status, event.createdById]); + const versionQuery = 'INSERT INTO event_versions (event_id, name, description, start_datetime, end_datetime, whole_day, repeat_frequency, location, url, status, version_created_by_user_id) VALUES (?,?,?,?,?,?,?,?,?,?,?);' + const versionRes = await conn.execute(versionQuery, [event.eventId, event.name, event.description, event.startDateTime, event.endDateTime, event.wholeDay, event.repeatFrequency, event.location, event.url, event.status, event.createdByUserId ?? null]); await conn.commit(); @@ -294,8 +298,8 @@ export const deleteEvent = async (event: Event): Promise => { let conn = await NachklangCalendarDB.getConnection(); try { await conn.beginTransaction(); - const versionQuery = 'INSERT INTO event_versions (event_id, status, version_created_by_id) VALUES (?,?,?);' - const versionRes = await conn.execute(versionQuery, [event.eventId, 'DELETED', event.createdById]); + const versionQuery = 'INSERT INTO event_versions (event_id, status, version_created_by_user_id) VALUES (?,?,?);' + const versionRes = await conn.execute(versionQuery, [event.eventId, 'DELETED', event.createdByUserId ?? null]); await conn.commit(); diff --git a/test/admin/admin.config.test.ts b/test/admin/admin.config.test.ts index 37ce7fb..3b83b8f 100644 --- a/test/admin/admin.config.test.ts +++ b/test/admin/admin.config.test.ts @@ -86,11 +86,12 @@ describe('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 () => { + it('defaults to the three production frontends', async () => { const config = await loadConfig(); expect(config.APP_ORIGINS).toEqual([ 'https://tickets.nachklang.art', - 'https://feedback.nachklang.art' + 'https://feedback.nachklang.art', + 'https://calendar.nachklang.art' ]); }); diff --git a/test/calendar/credentials.service.test.ts b/test/calendar/credentials.service.test.ts index 09d94f9..8131846 100644 --- a/test/calendar/credentials.service.test.ts +++ b/test/calendar/credentials.service.test.ts @@ -1,37 +1,26 @@ -import {describe, expect, it, vi, beforeEach} from 'vitest'; +import {describe, expect, it, beforeEach} from 'vitest'; -vi.mock('../../src/models/calendar/users/users.service.js', () => ({ - checkSession: vi.fn() -})); - -import * as UserService from '../../src/models/calendar/users/users.service.js'; import * as CredentialService from '../../src/models/calendar/events/credentials.service.js'; /** * The public calendar is read anonymously by nachklang.art to show the next - * upcoming event. That is a load-bearing property, not an accident: the - * calendar auth migration (docs/calendar-auth-migration.md) keeps the shared - * credentials only for the iCal export and moves everything else onto session - * cookies, and the failure mode of getting that wrong is the public website - * silently losing its events feed. + * upcoming event. That is a load-bearing property, not an accident: the step 4 + * cutover moved every signed-in path onto session cookies and left these shared + * passwords behind only for iCal subscriptions, and the failure mode of getting + * it wrong is the public website silently losing its events feed. * * So this pins both halves: public needs nothing, and the restricted calendars * still need something. */ describe('hasAccess', () => { beforeEach(() => { - vi.resetAllMocks(); process.env.MEMBER_CREDENTIAL = 'member-secret'; process.env.CHOIR_CREDENTIAL = 'choir-secret'; process.env.MANAGEMENT_CREDENTIAL = 'management-secret'; }); - it('lets anyone read the public calendar with no session and no password', async () => { - await expect(CredentialService.hasAccess('public', '', '', '', '127.0.0.1')).resolves.toBe(true); - - // It must not even reach the session check - an anonymous read of the - // public calendar should not depend on the users table being available. - expect(UserService.checkSession).not.toHaveBeenCalled(); + it('lets anyone read the public calendar with no password at all', async () => { + await expect(CredentialService.hasAccess('public', '')).resolves.toBe(true); }); it.each([ @@ -39,15 +28,22 @@ describe('hasAccess', () => { ['choir', 'choir-secret'], ['management', 'management-secret'], ['birthdays', 'choir-secret'] - ])('refuses %s without a credential and allows it with one', async (calendar, secret) => { - (UserService.checkSession as any).mockResolvedValue(null); - - await expect(CredentialService.hasAccess(calendar, '', '', '', '127.0.0.1')).resolves.toBe(false); - await expect(CredentialService.hasAccess(calendar, '', '', 'wrong', '127.0.0.1')).resolves.toBe(false); - await expect(CredentialService.hasAccess(calendar, '', '', secret, '127.0.0.1')).resolves.toBe(true); + ])('refuses %s without the credential and allows it with one', async (calendar, secret) => { + await expect(CredentialService.hasAccess(calendar, '')).resolves.toBe(false); + await expect(CredentialService.hasAccess(calendar, 'wrong')).resolves.toBe(false); + await expect(CredentialService.hasAccess(calendar, secret)).resolves.toBe(true); }); it('refuses an unknown calendar outright', async () => { - await expect(CredentialService.hasAccess('nope', '', '', 'member-secret', '127.0.0.1')).resolves.toBe(false); + await expect(CredentialService.hasAccess('nope', 'member-secret')).resolves.toBe(false); + }); + + it('refuses a calendar whose credential is not configured', async () => { + // An unset MEMBER_CREDENTIAL must not become "any password works", and in + // particular must not become "an absent password works". + delete process.env.MEMBER_CREDENTIAL; + + await expect(CredentialService.hasAccess('members', '')).resolves.toBe(false); + await expect(CredentialService.hasAccess('members', undefined as any)).resolves.toBe(false); }); }); diff --git a/test/calendar/events.router.test.ts b/test/calendar/events.router.test.ts new file mode 100644 index 0000000..6b5ce8e --- /dev/null +++ b/test/calendar/events.router.test.ts @@ -0,0 +1,187 @@ +import {describe, expect, it, vi, beforeEach} from 'vitest'; +import express from 'express'; +import request from 'supertest'; + +vi.mock('../../src/models/calendar/events/events.service.js', () => ({ + getAllEvents: vi.fn(), + getAllEventsAdmin: vi.fn(), + getEventById: vi.fn(), + createEvent: vi.fn(), + updateEvent: vi.fn(), + deleteEvent: vi.fn(), + moveEvent: vi.fn(), + getNextUpcomingEvent: 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 EventService from '../../src/models/calendar/events/events.service.js'; +import {auth} from '../../src/models/admin/admin.auth.js'; +import * as UsersService from '../../src/models/admin/users/users.admin.service.js'; +import {eventsRouter} from '../../src/models/calendar/events/events.router.js'; + +/** + * Step 4 of docs/calendar-auth-migration.md at the route level. The unit test + * on credentials.service covers the password table; this covers the thing that + * table is wired into, which is where the interesting mistakes live: + * + * - the public calendar has to stay readable with no session and no password, + * - the shared password has to keep working for the restricted calendars, + * because iCal clients cannot send a cookie, + * - and every write has to be behind the session cookie *and* an explicit + * calendar permission, not merely behind "is signed in". + */ + +const app = express(); +app.use(express.json()); +app.use('/calendar/events', eventsRouter); + +const signedInAs = (apps: string[], disabled = false) => { + (auth.api.getSession as any).mockResolvedValue({user: {id: 'admin-1'}}); + (UsersService.loadAccess as any).mockResolvedValue({ + id: 'admin-1', + email: 'a@nachklang.art', + displayName: 'A', + disabled, + permissions: apps.map(app => ({app, role: 'access'})), + apps + }); +}; + +const signedOut = () => { + (auth.api.getSession as any).mockResolvedValue(null); +}; + +const validEvent = { + calendarId: 1, + name: 'Konzert', + startDateTime: '2026-04-18T19:00:00Z', + endDateTime: '2026-04-18T21:00:00Z' +}; + +beforeEach(() => { + vi.clearAllMocks(); + process.env.MEMBER_CREDENTIAL = 'member-secret'; + (EventService.getAllEvents as any).mockResolvedValue([]); + (EventService.getAllEventsAdmin as any).mockResolvedValue([]); + (EventService.createEvent as any).mockResolvedValue(1); + (EventService.updateEvent as any).mockResolvedValue(1); + (EventService.moveEvent as any).mockResolvedValue(true); + (EventService.deleteEvent as any).mockResolvedValue(true); + signedOut(); +}); + +describe('reading', () => { + it('serves the public calendar anonymously', async () => { + // The property nachklang.art depends on. No cookie, no password. + await request(app).get('/calendar/events/public/json').expect(200); + + // And as the non-admin view: an anonymous caller must not see drafts. + expect(EventService.getAllEvents).toHaveBeenCalled(); + expect(EventService.getAllEventsAdmin).not.toHaveBeenCalled(); + }); + + it('refuses a restricted calendar with neither session nor password', async () => { + await request(app).get('/calendar/events/members/json').expect(403); + }); + + it('serves a restricted calendar to a shared password, without drafts', async () => { + await request(app) + .get('/calendar/events/members/json') + .query({password: 'member-secret'}) + .expect(200); + + expect(EventService.getAllEvents).toHaveBeenCalled(); + expect(EventService.getAllEventsAdmin).not.toHaveBeenCalled(); + }); + + it('gives a signed-in editor the admin view instead', async () => { + signedInAs(['calendar']); + + await request(app).get('/calendar/events/members/json').expect(200); + + expect(EventService.getAllEventsAdmin).toHaveBeenCalled(); + expect(EventService.getAllEvents).not.toHaveBeenCalled(); + }); + + it('treats a signed-in user without the calendar permission as anonymous', async () => { + // Not a 403: they may still read the public calendar like anyone else. + signedInAs(['tickets']); + + await request(app).get('/calendar/events/public/json').expect(200); + expect(EventService.getAllEvents).toHaveBeenCalled(); + expect(EventService.getAllEventsAdmin).not.toHaveBeenCalled(); + + await request(app).get('/calendar/events/members/json').expect(403); + }); + + it('still serves the public calendar when the admin database is down', async () => { + (auth.api.getSession as any).mockRejectedValue(new Error('ECONNREFUSED')); + + await request(app).get('/calendar/events/public/json').expect(200); + }); + + it('keeps the shared password working on the iCal export', async () => { + (EventService.getAllEvents as any).mockResolvedValue([]); + + await request(app).get('/calendar/events/public/ical').expect(200); + await request(app).get('/calendar/events/members/ical').expect(403); + await request(app) + .get('/calendar/events/members/ical') + .query({password: 'member-secret'}) + .expect(200); + }); +}); + +describe('writing', () => { + it.each([ + ['post', '/calendar/events'], + ['put', '/calendar/events/1'], + ['put', '/calendar/events/move/1'], + ['delete', '/calendar/events/1'] + ])('%s %s answers 401 when signed out', async (method, path) => { + await (request(app) as any)[method](path).send(validEvent).expect(401); + }); + + it.each([ + ['post', '/calendar/events'], + ['put', '/calendar/events/1'], + ['put', '/calendar/events/move/1'], + ['delete', '/calendar/events/1'] + ])('%s %s answers 403 without the calendar permission', async (method, path) => { + signedInAs(['tickets', 'feedback', 'admin']); + await (request(app) as any)[method](path).send(validEvent).expect(403); + }); + + it('answers 403 for a disabled account that still holds the permission', async () => { + signedInAs(['calendar'], true); + await request(app).post('/calendar/events').send(validEvent).expect(403); + }); + + it('records the writer as an admin user id, never a legacy one', async () => { + signedInAs(['calendar']); + + await request(app).post('/calendar/events').send(validEvent).expect(201); + + const written = (EventService.createEvent as any).mock.calls[0][0]; + expect(written.createdByUserId).toBe('admin-1'); + // Migration 003 made the legacy column nullable precisely so this can be unset. + expect(written.createdById).toBeUndefined(); + }); + + it('refuses the shared password as a way to write', async () => { + // The passwords are a read fallback for clients that cannot hold a + // session. They must never become an editing credential. + await request(app) + .post('/calendar/events') + .query({password: 'member-secret'}) + .send(validEvent) + .expect(401); + }); +});