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 e70493b..06e8c6c 100644 --- a/docker/init/01-calendar-schema-dev.sql +++ b/docker/init/01-calendar-schema-dev.sql @@ -1,5 +1,8 @@ -- Local dev only. Real schema, provided directly by the repo owner -- (calendars, events, event_versions, sessions, users) - not a guess. +-- Columns added by this repo's own migrations under sql/calendar/ are folded +-- in here rather than appended, so a fresh dev container matches production +-- after every migration has been applied. Keep the two in step. USE nachklang_calendar; CREATE TABLE `calendars` ( @@ -38,10 +41,16 @@ 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. + `created_by_name` varchar(255) DEFAULT NULL, PRIMARY KEY (`event_id`), KEY `events_calendars_calendar_id_fk` (`calendar_id`), KEY `events_users_user_id_fk` (`created_by_id`), + KEY `events_created_by_user_idx` (`created_by_user_id`), CONSTRAINT `events_calendars_calendar_id_fk` FOREIGN KEY (`calendar_id`) REFERENCES `calendars` (`calendar_id`), CONSTRAINT `events_users_user_id_fk` FOREIGN KEY (`created_by_id`) REFERENCES `users` (`user_id`) ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_general_ci; @@ -58,11 +67,16 @@ CREATE TABLE `event_versions` ( `location` text DEFAULT NULL, `url` text DEFAULT NULL, `version_created_by_id` int(11) DEFAULT NULL, + -- Bridge to the admin module's user ids; see sql/calendar/001_add_admin_user_bridge.sql. + `version_created_by_user_id` varchar(36) CHARACTER SET utf8mb4 COLLATE utf8mb4_unicode_ci DEFAULT NULL, + -- Archived editor name; see sql/calendar/002_snapshot_legacy_creator_names.sql. + `version_created_by_name` varchar(255) DEFAULT NULL, `status` text DEFAULT NULL, `version_created_at` datetime DEFAULT current_timestamp(), PRIMARY KEY (`event_version_id`), KEY `event_versions_events_event_id_fk` (`event_id`), KEY `event_versions_users_user_id_fk` (`version_created_by_id`), + KEY `event_versions_created_by_user_idx` (`version_created_by_user_id`), CONSTRAINT `event_versions_events_event_id_fk` FOREIGN KEY (`event_id`) REFERENCES `events` (`event_id`) ON DELETE CASCADE ON UPDATE CASCADE, CONSTRAINT `event_versions_users_user_id_fk` FOREIGN KEY (`version_created_by_id`) REFERENCES `users` (`user_id`) ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_general_ci; @@ -78,12 +92,16 @@ INSERT INTO calendars (calendar_id, name, includes_calendars) VALUES INSERT INTO users (email, password_hash, full_name, is_active) VALUES ('dev@nachklang.art', '$2b$10$vmj7POS/68SGE.eI7pGjMegrw0vNNZ2HVSUTra5NRsl8iOLwiMgZK', 'Dev Admin', 1); -INSERT INTO events (calendar_id, uuid, created_by_id) VALUES - (1, UUID(), 1), - (1, UUID(), 1), - (1, UUID(), 1); +-- Two rows are left on the legacy path and one carries an admin user id, so +-- dev exercises both branches of the step 3 dual-read rather than only the +-- happy one. It is deliberately a PUBLIC event, so the anonymous listing the +-- website uses covers both. The id is the dev admin from 04-admin-schema.sql. +INSERT INTO events (calendar_id, uuid, created_by_id, created_by_user_id, created_by_name) VALUES + (1, UUID(), 1, NULL, 'Dev Admin'), + (1, UUID(), 1, 'dev-user-0000-0000-0000-000000000001', NULL), + (1, UUID(), 1, NULL, 'Dev Admin'); -INSERT INTO event_versions (event_id, name, description, start_datetime, end_datetime, whole_day, location, url, status, version_created_by_id) VALUES - (1, 'Frühlingskonzert 2026', 'Erstes Konzert der Reihe', '2026-04-18 19:00:00', '2026-04-18 21:00:00', 0, 'Musikhochschule, Karlsruhe', 'https://www.nachklang.art/events/fruehlingskonzert-2026', 'PUBLIC', 1), - (2, 'Sommerkonzert 2026', 'Zweites Konzert der Reihe', '2026-07-11 19:00:00', '2026-07-11 21:00:00', 0, 'Christuskirche, Karlsruhe', 'https://www.nachklang.art/events/sommerkonzert-2026', 'PUBLIC', 1), - (3, 'Adventskonzert 2026', 'Drittes Konzert der Reihe', '2026-12-05 19:00:00', '2026-12-05 21:00:00', 0, 'Stadtkirche, Karlsruhe', 'https://www.nachklang.art/events/adventskonzert-2026', 'DRAFT', 1); +INSERT INTO event_versions (event_id, name, description, start_datetime, end_datetime, whole_day, location, url, status, version_created_by_id, version_created_by_user_id, version_created_by_name) VALUES + (1, 'Frühlingskonzert 2026', 'Erstes Konzert der Reihe', '2026-04-18 19:00:00', '2026-04-18 21:00:00', 0, 'Musikhochschule, Karlsruhe', 'https://www.nachklang.art/events/fruehlingskonzert-2026', 'PUBLIC', 1, NULL, 'Dev Admin'), + (2, 'Sommerkonzert 2026', 'Zweites Konzert der Reihe', '2026-07-11 19:00:00', '2026-07-11 21:00:00', 0, 'Christuskirche, Karlsruhe', 'https://www.nachklang.art/events/sommerkonzert-2026', 'PUBLIC', 1, 'dev-user-0000-0000-0000-000000000001', NULL), + (3, 'Adventskonzert 2026', 'Drittes Konzert der Reihe', '2026-12-05 19:00:00', '2026-12-05 21:00:00', 0, 'Stadtkirche, Karlsruhe', 'https://www.nachklang.art/events/adventskonzert-2026', 'DRAFT', 1, NULL, 'Dev Admin'); diff --git a/docs/calendar-auth-migration.md b/docs/calendar-auth-migration.md index ac3792f..080e2ce 100644 --- a/docs/calendar-auth-migration.md +++ b/docs/calendar-auth-migration.md @@ -1,8 +1,18 @@ # Migrating the Calendar domain onto the admin identity module -Status: **not started.** 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. +Status: **steps 1-4 implemented 2026-09-06, not yet merged or deployed.** Step 2 dropped by +decision, part of step 5 brought forward. Only step 5, the removal of the legacy path, is +left to write. + +> Read the deploy checklist under step 4 before applying anything. "Done" below means the +> code exists on a branch, **not** that production has it - and in particular production has +> none of the three migrations. Step 5 is scoped but deliberately unstarted: it must not be +> built on top of a step 4 that has not been deployed and watched. + +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 +are now live, so the calendar is the last module still on the legacy query-parameter +sessions. ## Why the calendar was left out @@ -38,37 +48,218 @@ permissions can be granted before anything else moves. Each step is meant to leave production working on its own. -1. **Add a bridging column.** `ALTER TABLE events ADD COLUMN created_by_user_id - VARCHAR(36) NULL`, indexed. Nothing reads it yet. -2. **Map the accounts.** For every legacy `users` row that should survive, invite the - person through the admin UI. On acceptance, backfill `events.created_by_user_id` from - `events.created_by_id` via an email-to-new-id mapping. Everyone not re-invited keeps - working on the legacy path until step 4. -3. **Dual-read.** Change `events.service.ts` to prefer `created_by_user_id` and fall back - to `created_by_id`. Writes fill both. This is the only step that is temporary code, and - it should carry a removal note pointing at step 5. -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. -5. **Drop the legacy path.** Remove `users.service.ts`'s session handling, the `sessions` - table, `created_by_id`, and the dual-read from step 3. Legacy `/calendar/users/*` stays - only if something still calls it - otherwise delete it too. `X-Session-Id` / - `X-Session-Key` can then come out of the CORS `allowedHeaders` list in - `src/app.factory.ts`. +1. **Add a bridging column.** ~~`ALTER TABLE events ADD COLUMN created_by_user_id + VARCHAR(36) NULL`, indexed. Nothing reads it yet.~~ **Done 2026-09-06**, as + `sql/calendar/001_add_admin_user_bridge.sql` - the first migration this repo owns for the + calendar schema, mirrored into `docker/init/01-calendar-schema-dev.sql`. It covers both + `events.created_by_user_id` and `event_versions.version_created_by_user_id`, and carries + no foreign key (see "What the code actually looks like" below). The dev seed leaves two + events on the legacy path and gives one an admin id, so step 3's dual-read has both cases + to exercise. Verified by applying the pre-migration schema and then the migration to a + throwaway MariaDB 11 container, and diffing `SHOW CREATE TABLE` against a fresh dev + schema: identical. Applied to the running dev database on the same day; a dev container + created before then needs it applied, or recreating. +2. ~~**Map the accounts.**~~ **Dropped 2026-09-06.** There is no backfill: since the + creator is only ever a display name (see below), old events keep resolving through the + legacy join until step 5 and then simply lose the name. Re-inviting the people who + actually still need calendar access remains an operational task, but it is no longer a + migration step and nothing is blocked on it. +3. **Dual-read.** ~~Change `events.service.ts` to prefer `created_by_user_id` and fall back + to `created_by_id`. Writes fill both.~~ **Done 2026-09-06.** `events.service.ts` now reads + both columns and prefers the admin one, resolving the name through a single + `findDisplayNames` lookup against the admin database per result set (added to + `users.admin.service.ts` for this). Four copies of the same SELECT and four copies of the + row mapper were collapsed into one of each first - the dual read would otherwise have had + to be written four times. + + A name now has three possible sources, tried weakest first: the legacy join, then the + `created_by_name` snapshot from migration 002, then the live admin lookup - which wins + because it is the only one that follows an account being renamed. An admin id that no + longer resolves falls back rather than blanking, and a failure to reach the admin database + is caught and logged rather than propagated, so an anonymous read of the public calendar + never depends on the admin database being up. Covered by + `test/calendar/events.service.test.ts`. + + **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`.~~ **Done 2026-09-06.** `DEFERRED_SECURITY.md` item 1 is closed: + no route reads `sessionId`/`sessionKey` any more. + + How it came out, route by route: + + - 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. 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 checklist + + Production has **none** of the three migrations: 001 and 002 were only ever applied to the + dev database. The API build below selects `created_by_user_id` and `created_by_name` on + every read, so deploying it against a database missing them fails every calendar request + including the anonymous public feed the website uses. In order: + + 1. **Apply `sql/calendar/001`, `002`, `003`, in that order**, against `CALENDAR_DB`. All + three are re-runnable, so applying one that is already applied is a no-op. Verify + before continuing: + `SHOW COLUMNS FROM events LIKE '%by_user%'; SHOW COLUMNS FROM events LIKE '%by_name%';` + - four rows across the two tables, and `created_by_id` nullable. + 2. **Check `APP_ORIGINS` on the API vhost.** `calendar.nachklang.art` is in the code's + default list, but the environment variable *replaces* that list rather than adding to + it - so if it is set at all (the tickets/feedback cutover may have set it), append + `https://calendar.nachklang.art` or the calendar's sign-out will 403 while everything + else works. That is the failure mode the comment in `admin.config.ts` warns about. + 3. **Deploy the API.** + 4. **Deploy the calendar frontend immediately after.** Do not leave a gap - see below. + 5. **Re-run 002's two `UPDATE` statements.** Between step 1 and step 3 the old API was + still writing `created_by_id` with no snapshot; those few rows would otherwise lose + their author at step 5. + 6. **Rebuild the admin app** if `NEXT_PUBLIC_ALLOWED_REDIRECT_ORIGINS` does not already + contain `https://calendar.nachklang.art`. It is a **build-time** value, so a restart + does nothing. + + **The window between steps 3 and 4 does not look broken, which is the danger.** The old + Angular bundle starts by calling `POST /calendar/users/checkSessionValid`, and those + legacy routes are untouched - so it still succeeds and the page renders as signed in. What + the user then sees is an empty event table and saves that silently do nothing. It looks + like the calendar lost its data, not like a deploy in progress. Keep the gap to minutes, + or take the frontend offline for it. + + **One-way door:** any iCal subscription whose URL carries `?sessionId=&sessionKey=` rather + than `?password=` stops working permanently. The shared-password URLs are unaffected. +5. **Drop the legacy path.** Not started - and deliberately not started until step 4 has been + deployed and watched, because it removes the fallback step 4 still leans on. Scoped and + decided 2026-09-06; what follows is the agreed shape, not a suggestion. + + **Prerequisite: step 4 live in production and behaving.** Until then the legacy join is + what renders the author of every pre-cutover event, and the legacy routes are what an old + cached bundle talks to. Doing this first turns a recoverable deploy into an unrecoverable + one. + + Code, in one branch: + + - **Delete `src/models/calendar/users/` entirely** - `users.router.ts`, `users.service.ts`, + `session.interface.ts`, `user.interface.ts` - and the `calendarRouter.use('/users', ...)` + line in `Calendar.router.ts`. *(Decided: delete outright rather than unmount.)* This + removes the last unauthenticated account-creation and mail-sending endpoint in the API. + A survey on 2026-09-06 confirmed nothing outside that directory imports it, and nothing + outside it touches the `users`/`sessions` tables except the two joins below. + - **Drop the legacy half of the read** in `events.service.ts`: the two + `LEFT OUTER JOIN users` clauses, the `legacy_*` aliases, and `created_by_id` / + `version_created_by_id` from the SELECT and the row mapper. The snapshot fallback stays - + it is what makes this safe. Remove `createdById` / `lastModifiedById` from + `event.interface.ts` and their (already deprecated) swagger properties. + - **Remove `X-Session-Id` / `X-Session-Key`** from the CORS `allowedHeaders` in + `src/app.factory.ts`. Nothing has sent them since the tickets and feedback frontends were + redeployed. + - **Drop the obsolete test mocks**: `test/feedback/feedback.auth.test.ts`, + `test/tickets/tickets.auth.test.ts` and `test/admin/auth-binding.ts` each mock + `calendar/users/users.service.js` and assert `checkSession` is never called. That + tripwire is meaningless once the module does not exist; remove the mock and the + assertion, keep the rest. + + Database, as `sql/calendar/004_*.sql`: + + - Drop the foreign keys `events_users_user_id_fk` and `event_versions_users_user_id_fk`, + then the `created_by_id` and `version_created_by_id` columns. + - **`RENAME TABLE users TO users_legacy_archive`**, same for `sessions`. *(Decided: rename + rather than drop.)* The reasoning: the display names are already snapshotted so nothing + visible depends on these rows, but they still hold the old e-mail addresses and password + hashes, and a rename makes the tables unreachable without destroying anything. Dropping + them later is one statement, at a moment when nobody is mid-deploy. + - Mirror all of it in `docker/init/01-calendar-schema-dev.sql` (the archive tables need no + mirror - a fresh dev database has nothing to archive). + + Documentation: `DEFERRED_SECURITY.md` items **3** (activation token has no expiry) and + **4** (password reset token has no expiry) close outright - both describe code that ceases + to exist. Item 2 (no event ownership check) stays open. + + Two consequences to accept explicitly rather than discover: + + - Any activation or password-reset e-mail already sent points at + `api.nachklang.art/calendar/users/activate` and becomes a 404. Those links were only ever + valid for legacy accounts, which no longer open anything. + - `Event.createdById` disappears from the API response. The Angular frontend never read it + (its `Event` model has only `createdBy`, the name), so this is not a breaking change for + the only known consumer - but it is a wire-format removal, so check anything else that + reads `/calendar/events/*/json` first. + +## What the code actually looks like (surveyed 2026-09-06) + +Four things found while doing step 1 that change how the later steps should be built: + +- **`created_by_id` is display-only.** Nothing authorises on it. `events.router.ts` gates + PUT, POST, DELETE and `/move` on `user?.isActive` alone - there is no "only the creator may + edit" rule anywhere - and the column is read back solely to render `created_by_name` and + `last_modified_by_name`. That de-risks steps 2, 3 and 5 considerably: an event whose + creator never gets re-invited loses a name in the UI, it does not become uneditable or + invisible. It also means the step 2 backfill is best-effort, not a precondition. +- **The two schemas are separate databases.** `nachklang_calendar` and `nachklang_admin` + have their own connection pools (`Calendar.db.ts` vs the admin module's Kysely instance). + So the bridging columns get no foreign key, and - the part the original sequence missed - + **the `LEFT OUTER JOIN users` that produces the creator's name cannot simply be repointed**. + It would have to become a cross-schema join, which hardcodes the admin database name into + calendar SQL and ties the two schemas together exactly as an FK would. Recommendation for + step 3: drop the join for the new path and resolve names in the service layer instead - + collect the distinct ids from the result set and do one lookup against the admin users + service. One extra query per listing, no coupling, and it keeps working if the admin + database ever moves. +- **`events.created_by_id` is `NOT NULL`.** Step 5 cannot simply stop writing it; that step + has to drop the column (and its FK to `users`) in the same migration that stops the writes, + or make it nullable first. +- **Every calendar read already hits the session table.** `/:calendar/json` calls + `UserService.checkSession` before falling back to `credentials.service.ts`, so the shared + credentials are the *fallback*, not the primary path. Step 4 replaces the first half of + that with `requireAppAccess('calendar')` and has to decide what happens to the second half + - which is the first open question below. ## Open questions to settle before starting -- **The shared calendar credentials.** Do `MEMBER_CREDENTIAL` and friends stay as a - separate mechanism (they serve people with no account at all, and iCal clients that - cannot send headers), or do read-only accounts replace them? This is a product decision, - not a technical one, and it decides how much of `credentials.service.ts` survives. -- **The iCal export.** `GET /calendar/events/{calendar}/ical` takes a password in the query - string on purpose, because iCal clients cannot send headers. Cookie sessions do not help - here; this endpoint likely keeps its own scheme. -- **Which legacy accounts to keep.** Step 2 is the moment to not re-invite people who no - longer need access. -- **`event_versions.version_created_by_id`.** The same INT reference again, joined in - `events.service.ts` for the "last modified by" name. It has to move with `events`, and it - is the reason step 1's bridging column needs a sibling on `event_versions`. +**Settled 2026-09-06:** + +- **The shared calendar credentials keep working, but only for iCal.** The web app goes + cookie-only at step 4; `MEMBER_CREDENTIAL` and friends survive on + `GET /calendar/events/{calendar}/ical`, which is the one case where the client genuinely + cannot send a cookie. Everything else in `credentials.service.ts` goes with step 5. + `public` stays anonymous everywhere - see the note under step 4. +- **The iCal export keeps its own scheme.** Same reasoning; it is the reason the shared + credentials survive at all rather than an exception to their removal. +- **No account backfill.** See step 2 above. +- **Pre-cutover authorship is archived, not discarded.** `events.created_by_name` and + `event_versions.version_created_by_name`, backfilled once by migration 002 and never + written again. This was originally listed as a step 5 question; it was brought forward so + the data is safe well before the table that holds it is dropped. + +**Nothing is open.** The last one - ~~`event_versions.version_created_by_id`~~, the same INT +reference on the version rows - was handled in passing: step 1 gave it a sibling bridging +column, step 2 a sibling snapshot, and step 3 reads it exactly like `events`. diff --git a/sql/calendar/001_add_admin_user_bridge.sql b/sql/calendar/001_add_admin_user_bridge.sql new file mode 100644 index 0000000..35e0bad --- /dev/null +++ b/sql/calendar/001_add_admin_user_bridge.sql @@ -0,0 +1,38 @@ +-- Nachklang e.V. Calendar module — step 1 of docs/calendar-auth-migration.md. +-- Adds the bridging columns that let an event record who created it as an +-- *admin* user id (VARCHAR(36)) alongside the legacy calendar users.user_id +-- (INT). Apply manually against the CALENDAR_DB database: +-- mysql -h -u -p < 001_add_admin_user_bridge.sql +-- +-- Numbered 001 because this is the first migration this repo owns for the +-- calendar schema: the tables themselves predate it and were provided by the +-- repo owner (mirrored for dev in docker/init/01-calendar-schema-dev.sql). +-- +-- Nothing reads these columns yet — step 3 introduces the dual-read. Adding +-- them first means the backfill in step 2 has somewhere to write, and this +-- migration can be applied to production on its own without any code change. +-- +-- No foreign key, on purpose. The admin `user` table lives in a *different* +-- database (nachklang_admin) behind a different connection pool, and a +-- cross-schema FK would tie the two schemas' lifecycles together: you could no +-- longer dump, restore or move one without the other. The reference is +-- enforced in application code, which is also where the legacy/new fallback +-- lives. +-- +-- The collation is pinned to the admin database's (utf8mb4_unicode_ci) rather +-- than inherited from the calendar tables' utf8mb4_general_ci. These columns +-- hold ids that only ever compare against nachklang_admin.user.id, and a +-- mismatched collation makes any such comparison fail at runtime with +-- "Illegal mix of collations" instead of at review time. + +ALTER TABLE `events` + ADD COLUMN IF NOT EXISTS `created_by_user_id` VARCHAR(36) + CHARACTER SET utf8mb4 COLLATE utf8mb4_unicode_ci + NULL DEFAULT NULL AFTER `created_by_id`, + ADD KEY IF NOT EXISTS `events_created_by_user_idx` (`created_by_user_id`); + +ALTER TABLE `event_versions` + ADD COLUMN IF NOT EXISTS `version_created_by_user_id` VARCHAR(36) + CHARACTER SET utf8mb4 COLLATE utf8mb4_unicode_ci + NULL DEFAULT NULL AFTER `version_created_by_id`, + ADD KEY IF NOT EXISTS `event_versions_created_by_user_idx` (`version_created_by_user_id`); diff --git a/sql/calendar/002_snapshot_legacy_creator_names.sql b/sql/calendar/002_snapshot_legacy_creator_names.sql new file mode 100644 index 0000000..b11ae6a --- /dev/null +++ b/sql/calendar/002_snapshot_legacy_creator_names.sql @@ -0,0 +1,42 @@ +-- Nachklang e.V. Calendar module — step 5 preparation, brought forward. +-- Apply manually against the CALENDAR_DB database, after 001: +-- mysql -h -u -p < 002_snapshot_legacy_creator_names.sql +-- +-- Snapshots the creator's and last editor's *name* onto the event itself. +-- +-- Why: the creator is only ever rendered as a name (nothing authorises on it), +-- and today that name comes from joining the calendar's own `users` table. +-- Step 5 drops that table, which would silently erase the authorship of every +-- event created before the cutover. There is no account backfill to save them +-- either - that was dropped deliberately, see docs/calendar-auth-migration.md. +-- One text column per reference keeps the history at no ongoing cost. +-- +-- These columns are an archive, not a source of truth. Nothing writes them +-- after this backfill: events created from the cutover onwards carry an admin +-- user id, whose name is resolved live so that renaming an account updates +-- everywhere. The read path prefers the live admin name, falls back to this +-- snapshot, and falls back again to the join until step 5 removes it. +-- +-- The whole file is re-runnable: IF NOT EXISTS on the columns, and the backfill +-- only touches rows with no snapshot yet. Step 4's migration re-runs the +-- backfill, to catch anything created between this migration and the cutover. +-- +-- No charset clause: unlike 001's id columns these hold display text that is +-- only ever compared against other calendar data, so they inherit the tables' +-- utf8mb4_general_ci like the columns they are copied from. + +ALTER TABLE `events` + ADD COLUMN IF NOT EXISTS `created_by_name` VARCHAR(255) NULL DEFAULT NULL AFTER `created_by_user_id`; + +ALTER TABLE `event_versions` + ADD COLUMN IF NOT EXISTS `version_created_by_name` VARCHAR(255) NULL DEFAULT NULL AFTER `version_created_by_user_id`; + +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/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/app.factory.ts b/src/app.factory.ts index 9261a10..3328f90 100644 --- a/src/app.factory.ts +++ b/src/app.factory.ts @@ -63,13 +63,14 @@ 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-* 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. + // X-Session-* are no longer read by anything on this side, and no longer + // sent by anything either: the tickets and feedback cutover took the last + // two readers off them, and the calendar cutover removed the last legacy + // credential path in the API (its session used to travel in query + // parameters - DEFERRED_SECURITY.md item 1, now closed). They stay allowed + // only so a browser still running a pre-cutover tickets or feedback bundle + // gets a clean 401 rather than a CORS preflight failure. Drop them once + // those have aged out - 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.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/admin/users/users.admin.service.ts b/src/models/admin/users/users.admin.service.ts index f2af3ee..a3ff2d3 100644 --- a/src/models/admin/users/users.admin.service.ts +++ b/src/models/admin/users/users.admin.service.ts @@ -437,3 +437,30 @@ export const findUserByEmail = async (email: string): Promise<{id: string; email return row ?? null; }; + +/** + * Display names for a set of user ids, as an id -> name map. Ids that no + * longer exist are simply absent from the map rather than mapping to a + * placeholder, so callers can distinguish "deleted account" from "never had + * one" and choose their own fallback. + * + * This exists for the calendar migration (docs/calendar-auth-migration.md + * step 3): the calendar lives in a different database, so it cannot join + * against `user` to render "created by". One lookup per result set keeps that + * cheap without coupling the two schemas. + */ +export const findDisplayNames = async (ids: readonly string[]): Promise> => { + const distinct = Array.from(new Set(ids.filter(id => id))); + if (distinct.length === 0) { + // Kysely renders `in ()` for an empty list, which MariaDB rejects. + return new Map(); + } + + const rows = await db + .selectFrom('user') + .select(['id', 'name']) + .where('id', 'in', distinct) + .execute(); + + return new Map(rows.map(row => [row.id, row.name])); +}; 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/event.interface.ts b/src/models/calendar/events/event.interface.ts index cafb87f..0caf877 100644 --- a/src/models/calendar/events/event.interface.ts +++ b/src/models/calendar/events/event.interface.ts @@ -67,16 +67,35 @@ * example: "John Doe" * createdById: * type: integer - * description: The ID of the user who created the event + * deprecated: true + * description: > + * The legacy calendar user id of the creator. Being replaced by + * createdByUserId; see docs/calendar-auth-migration.md. Null on + * events created after the cutover. + * nullable: true * example: 456 + * createdByUserId: + * type: string + * nullable: true + * description: The admin-module user id of the creator, once it has one + * example: "8f1c0f2e-0f1a-4b9e-9a7c-2d5f1b3c4d5e" * lastModifiedBy: * type: string * description: The name of the user who last modified the event * example: "John Doe" * lastModifiedById: * type: integer - * description: The ID of the user who last modified the event + * deprecated: true + * nullable: true + * description: > + * The legacy calendar user id of the last editor. Being replaced + * by lastModifiedByUserId. * example: 456 + * lastModifiedByUserId: + * type: string + * nullable: true + * description: The admin-module user id of the last editor, once it has one + * example: "8f1c0f2e-0f1a-4b9e-9a7c-2d5f1b3c4d5e" * url: * type: string * description: A URL with more information about the event @@ -102,10 +121,15 @@ export interface Event { createdDate: Date; lastModifiedDate?: Date; location: string; + /** Display name of the creator, from whichever id below resolved. */ createdBy?: string; - createdById: number; + createdById?: number | null; + /** Set once the event's creator exists in the admin module. Preferred over + * createdById when both are present; see docs/calendar-auth-migration.md. */ + createdByUserId?: string | null; lastModifiedBy?: string; - lastModifiedById?: number; + lastModifiedById?: number | null; + lastModifiedByUserId?: string | null; url: string; wholeDay: boolean; repeatFrequency: string; diff --git a/src/models/calendar/events/events.router.ts b/src/models/calendar/events/events.router.ts index 18d3fc5..3d6c25c 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: @@ -48,23 +89,13 @@ export const calendarNames = new Map([ * required: true * schema: * type: string - * enum: [public, members, choir, management] + * enum: [public, members, choir, management, birthdays] * 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); @@ -158,7 +182,10 @@ eventsRouter.get('/:calendar/json', async (req: Request, res: Response) => { * /calendar/events/{calendar}/json/next: * get: * summary: Get the next upcoming event from a calendar - * description: Returns the next upcoming event from the specified calendar. Authentication required. + * description: > + * The next upcoming event. The public calendar is open to everyone; the + * others need either a signed-in account with the calendar permission or the + * calendar's shared password. * tags: * - calendar * parameters: @@ -167,23 +194,13 @@ eventsRouter.get('/:calendar/json', async (req: Request, res: Response) => { * required: true * schema: * type: string - * enum: [public, members, choir, management] + * enum: [public, members, choir, management, birthdays] * 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 +259,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 +273,19 @@ 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)) { + // Holding the calendar's shared password, or signed in. The password path + // is what keeps iCal subscriptions working - a calendar client cannot + // send a cookie. + // + // The password is checked FIRST so that `public`, which needs no + // credential at all, short-circuits before signedInEditor runs. Otherwise + // every request from a browser that happens to hold a .nachklang.art + // cookie - which is any signed-in user on any of the four apps - would put + // an admin-database query in front of the anonymous public feed, with no + // timeout. Both operands are side-effect free, so the order is free to + // choose; this order is the one that keeps the public calendar + // independent of the admin database. + if (! await CredentialService.hasAccess(calendarName, password) && !await signedInEditor(req)) { res.status(403).send({'message': 'You do not have access to the specified calendar.'}); return; } @@ -290,7 +316,10 @@ eventsRouter.get('/:calendar/json/next', async (req: Request, res: Response) => * /calendar/events/{calendar}/ical: * get: * summary: Get all events from a specific calendar in iCal format - * description: Returns all events from the specified calendar in iCal format for calendar applications. Authentication required. + * description: > + * The calendar in iCal format. The public calendar is open to everyone; the + * others take the calendar's shared password in the query string, which is + * why that mechanism survives - an iCal client cannot send a cookie. * tags: * - calendar * parameters: @@ -299,23 +328,13 @@ eventsRouter.get('/:calendar/json/next', async (req: Request, res: Response) => * required: true * schema: * type: string - * enum: [public, members, choir, management] + * enum: [public, members, choir, management, birthdays] * 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 +384,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 +398,19 @@ 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)) { + // Holding the calendar's shared password, or signed in. The password path + // is what keeps iCal subscriptions working - a calendar client cannot + // send a cookie. + // + // The password is checked FIRST so that `public`, which needs no + // credential at all, short-circuits before signedInEditor runs. Otherwise + // every request from a browser that happens to hold a .nachklang.art + // cookie - which is any signed-in user on any of the four apps - would put + // an admin-database query in front of the anonymous public feed, with no + // timeout. Both operands are side-effect free, so the order is free to + // choose; this order is the one that keeps the public calendar + // independent of the admin database. + if (! await CredentialService.hasAccess(calendarName, password) && !await signedInEditor(req)) { res.status(403).send({'message': 'You do not have access to the specified calendar.'}); return; } @@ -413,22 +441,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 +512,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 +555,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 +579,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 +610,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 +622,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: @@ -639,9 +654,6 @@ eventsRouter.post('/', async (req: Request, res: Response) => { * location: * type: string * example: "Musikhochschule, Karlsruhe" - * createdBy: - * type: string - * example: "John Doe" * url: * type: string * example: "https://www.nachklang.art/events/concert" @@ -673,16 +685,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 +728,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 || @@ -735,8 +753,9 @@ eventsRouter.put('/:eventId', async (req: Request, res: Response) => { endDateTime: new Date(req.body.endDateTime), 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 +787,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 +799,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: @@ -820,9 +829,6 @@ eventsRouter.put('/:eventId', async (req: Request, res: Response) => { * location: * type: string * example: "Musikhochschule, Karlsruhe" - * createdBy: - * type: string - * example: "John Doe" * url: * type: string * example: "https://www.nachklang.art/events/concert" @@ -854,16 +860,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 +903,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 || @@ -913,8 +925,9 @@ eventsRouter.put('/move/:eventId', async (req: Request, res: Response) => { endDateTime: new Date(req.body.endDateTime), 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 +957,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 +969,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 +990,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 +1033,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 +1055,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 0d261ab..0f29f03 100644 --- a/src/models/calendar/events/events.service.ts +++ b/src/models/calendar/events/events.service.ts @@ -2,28 +2,56 @@ import * as dotenv from 'dotenv'; import {Guid} from 'guid-typescript'; import {Event} from './event.interface.js'; import {NachklangCalendarDB} from '../Calendar.db.js'; +import * as AdminUsersService from '../../admin/users/users.admin.service.js'; +import logger from '../../../middleware/logger.js'; dotenv.config(); /** - * Returns all events for the given calendar - * @param calendarId The calendar Id + * Step 3 of docs/calendar-auth-migration.md: the dual read. + * + * An event records its creator twice - `created_by_id`, the legacy INT into + * the calendar database's own `users` table, and `created_by_user_id`, the + * admin module's VARCHAR(36) id. Old rows have only the first, rows written + * after the step 4 cutover will have only the second, and the two live in + * different databases, so this file has to read both and prefer the new one. + * + * The one thing the creator is used for is a display name. Nothing authorises + * on it - there is no "only the creator may edit" rule anywhere - which is why + * a name that cannot be resolved degrades to blank instead of to an error. + * + * That name has three possible sources, and they are tried weakest first: + * + * 1. LEGACY - joining the calendar's own `users` table on `created_by_id`. + * 2. `created_by_name`, the snapshot migration 002 took of exactly that join, + * so the authorship of pre-cutover events survives step 5 dropping the + * table. An archive: nothing writes it after the backfill. + * 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. */ -export const getAllEvents = async (calendarId: number): Promise => { - let conn = await NachklangCalendarDB.getConnection(); - let eventRows: Event[] = []; - try { - const calendarQuery = 'SELECT calendar_id, includes_calendars FROM calendars WHERE calendar_id = ?'; - const calendarRes = await conn.query(calendarQuery, calendarId); - let calendarsToFetch: number[] = [calendarId]; - for(let row of calendarRes) { - let includes: number[] = JSON.parse(row.includes_calendars); - calendarsToFetch = [...calendarsToFetch, ...includes]; - } - const eventsQuery = ` - SELECT e.calendar_id, e.uuid, e.created_date, e.created_by_id, u.full_name as created_by_name, u2.full_name as last_modified_by_name, v.* FROM events e +/** + * The one SELECT the four read paths share. It was copied out four times + * before, which is precisely why the dual read had to be added in four + * places; callers append their own WHERE and ORDER BY. + * + * `v.*` carries `version_created_by_user_id` and `version_created_by_name` + * along with the rest of the version row, so only the `events` columns need + * naming. The two joined names are aliased `legacy_*` because the unprefixed + * names are now real columns. + */ +const EVENT_SELECT = ` + SELECT e.calendar_id, e.uuid, e.created_date, e.created_by_id, e.created_by_user_id, e.created_by_name, + u.full_name as legacy_created_by_name, u2.full_name as legacy_last_modified_by_name, v.* FROM events e INNER JOIN ( SELECT event_id, MAX(event_version_id) AS latest_version FROM event_versions @@ -33,34 +61,124 @@ export const getAllEvents = async (calendarId: number): Promise => { INNER JOIN event_versions v ON v.event_id = latest_versions.event_id AND v.event_version_id = latest_versions.latest_version LEFT OUTER JOIN users u ON u.user_id = e.created_by_id - LEFT OUTER JOIN users u2 ON u2.user_id = v.version_created_by_id - WHERE e.calendar_id IN (?) AND v.status = 'PUBLIC' - ORDER BY e.event_id`; - const eventsRes = await conn.query(eventsQuery, [calendarsToFetch]); + LEFT OUTER JOIN users u2 ON u2.user_id = v.version_created_by_id`; - for (let row of eventsRes) { - eventRows.push({ - eventId: row.event_id, - calendarId: row.calendar_id, - uuid: row.uuid, - name: row.name, - description: row.description, - startDateTime: row.start_datetime, - endDateTime: row.end_datetime, - createdDate: row.created_date, - lastModifiedDate: row.version_created_at, - location: row.location, - createdBy: row.created_by_name, - createdById: row.created_by_id, - lastModifiedBy: row.last_modified_by_name, - lastModifiedById: row.version_created_by_id, - url: row.url, - wholeDay: row.whole_day, - repeatFrequency: row.repeat_frequency - }); +/** + * Maps a result row to an Event. `status` is included only where it always + * was: the admin views and the by-id lookup return it, the two public listings + * do not. + */ +const toEvent = (row: any, includeStatus: boolean): Event => { + const event: Event = { + eventId: row.event_id, + calendarId: row.calendar_id, + uuid: row.uuid, + name: row.name, + description: row.description, + startDateTime: row.start_datetime, + endDateTime: row.end_datetime, + createdDate: row.created_date, + lastModifiedDate: row.version_created_at, + location: row.location, + // Name resolution, weakest first: the LEGACY join against the calendar + // users table, then the snapshot taken in migration 002, then - in + // resolveAdminNames below - the live admin name, which wins because it + // is the only one that follows an account being renamed. + createdBy: row.created_by_name ?? row.legacy_created_by_name, + createdById: row.created_by_id, + createdByUserId: row.created_by_user_id ?? null, + lastModifiedBy: row.version_created_by_name ?? row.legacy_last_modified_by_name, + lastModifiedById: row.version_created_by_id, + lastModifiedByUserId: row.version_created_by_user_id ?? null, + url: row.url, + wholeDay: row.whole_day, + repeatFrequency: row.repeat_frequency + }; + + if (includeStatus) { + event.status = row.status; + } + + return event; +}; + +/** + * Fills in creator/editor names for rows that carry an admin user id, by way + * of a single lookup against the admin database. The calendar cannot join + * against `user` - it is a different schema behind a different pool - and + * making it one would tie the two schemas together as tightly as a foreign key + * would. + * + * A failure here is swallowed on purpose. These endpoints include the public + * calendar the website reads anonymously, and a name is decoration: if the + * admin database is unreachable, an event should still render with whatever + * the legacy join produced rather than 500 the whole listing. The alternative + * would widen the public calendar's blast radius to include the admin + * database, which it has never depended on before. + */ +const resolveAdminNames = async (events: Event[]): Promise => { + const ids = events + .flatMap(event => [event.createdByUserId, event.lastModifiedByUserId]) + .filter((id): id is string => Boolean(id)); + + if (ids.length === 0) { + return; + } + + let names: Map; + try { + names = await AdminUsersService.findDisplayNames(ids); + } catch (e: any) { + logger.warn('Calendar: could not resolve creator names from the admin database: ' + e.message); + return; + } + + for (const event of events) { + const createdBy = event.createdByUserId ? names.get(event.createdByUserId) : undefined; + if (createdBy) { + event.createdBy = createdBy; } - return eventRows; + const lastModifiedBy = event.lastModifiedByUserId ? names.get(event.lastModifiedByUserId) : undefined; + if (lastModifiedBy) { + event.lastModifiedBy = lastModifiedBy; + } + } +}; + +/** + * The calendars a listing has to cover: the requested one plus whatever it + * declares in `includes_calendars`. + */ +const calendarsToFetch = async (conn: any, calendarId: number): Promise => { + const calendarQuery = 'SELECT calendar_id, includes_calendars FROM calendars WHERE calendar_id = ?'; + const calendarRes = await conn.query(calendarQuery, calendarId); + let calendars: number[] = [calendarId]; + for (let row of calendarRes) { + let includes: number[] = JSON.parse(row.includes_calendars); + calendars = [...calendars, ...includes]; + } + return calendars; +}; + +/** + * Returns all events for the given calendar + * @param calendarId The calendar Id + */ +export const getAllEvents = async (calendarId: number): Promise => { + let conn = await NachklangCalendarDB.getConnection(); + try { + const calendars = await calendarsToFetch(conn, calendarId); + + const eventsQuery = `${EVENT_SELECT} + WHERE e.calendar_id IN (?) AND v.status = 'PUBLIC' + ORDER BY e.event_id`; + const eventsRes = await conn.query(eventsQuery, [calendars]); + + const events = eventsRes.map((row: any) => toEvent(row, false)); + await resolveAdminNames(events); + + return events; } catch (err) { throw err; } finally { @@ -76,48 +194,16 @@ export const getAllEvents = async (calendarId: number): Promise => { */ export const getAllEventsAdmin = async (calendarId: number): Promise => { let conn = await NachklangCalendarDB.getConnection(); - let eventRows: Event[] = []; try { - const eventsQuery = ` - SELECT e.calendar_id, e.uuid, e.created_date, e.created_by_id, u.full_name as created_by_name, u2.full_name as last_modified_by_name, v.* FROM events e - INNER JOIN ( - SELECT event_id, MAX(event_version_id) AS latest_version - FROM event_versions - GROUP BY event_id - ) latest_versions - ON e.event_id = latest_versions.event_id - INNER JOIN event_versions v - ON v.event_id = latest_versions.event_id AND v.event_version_id = latest_versions.latest_version - LEFT OUTER JOIN users u ON u.user_id = e.created_by_id - LEFT OUTER JOIN users u2 ON u2.user_id = v.version_created_by_id + const eventsQuery = `${EVENT_SELECT} WHERE e.calendar_id = ? ORDER BY e.event_id`; const eventsRes = await conn.query(eventsQuery, calendarId); - for (let row of eventsRes) { - eventRows.push({ - eventId: row.event_id, - calendarId: row.calendar_id, - uuid: row.uuid, - name: row.name, - description: row.description, - startDateTime: row.start_datetime, - endDateTime: row.end_datetime, - createdDate: row.created_date, - lastModifiedDate: row.version_created_at, - location: row.location, - createdBy: row.created_by_name, - createdById: row.created_by_id, - lastModifiedBy: row.last_modified_by_name, - lastModifiedById: row.version_created_by_id, - url: row.url, - wholeDay: row.whole_day, - repeatFrequency: row.repeat_frequency, - status: row.status - }); - } + const events = eventsRes.map((row: any) => toEvent(row, true)); + await resolveAdminNames(events); - return eventRows; + return events; } catch (err) { throw err; } finally { @@ -136,18 +222,7 @@ export const getAllEventsAdmin = async (calendarId: number): Promise => export const getEventById = async (eventId: number): Promise => { let conn = await NachklangCalendarDB.getConnection(); try { - const eventsQuery = ` - SELECT e.calendar_id, e.uuid, e.created_date, e.created_by_id, u.full_name as created_by_name, u2.full_name as last_modified_by_name, v.* FROM events e - INNER JOIN ( - SELECT event_id, MAX(event_version_id) AS latest_version - FROM event_versions - GROUP BY event_id - ) latest_versions - ON e.event_id = latest_versions.event_id - INNER JOIN event_versions v - ON v.event_id = latest_versions.event_id AND v.event_version_id = latest_versions.latest_version - LEFT OUTER JOIN users u ON u.user_id = e.created_by_id - LEFT OUTER JOIN users u2 ON u2.user_id = v.version_created_by_id + const eventsQuery = `${EVENT_SELECT} WHERE e.event_id = ?`; const eventsRes = await conn.query(eventsQuery, eventId); @@ -155,27 +230,10 @@ export const getEventById = async (eventId: number): Promise => { return null; } - const row = eventsRes[0]; - return { - eventId: row.event_id, - calendarId: row.calendar_id, - uuid: row.uuid, - name: row.name, - description: row.description, - startDateTime: row.start_datetime, - endDateTime: row.end_datetime, - createdDate: row.created_date, - lastModifiedDate: row.version_created_at, - location: row.location, - createdBy: row.created_by_name, - createdById: row.created_by_id, - lastModifiedBy: row.last_modified_by_name, - lastModifiedById: row.version_created_by_id, - url: row.url, - wholeDay: row.whole_day, - repeatFrequency: row.repeat_frequency, - status: row.status - } as Event; + const event = toEvent(eventsRes[0], true); + await resolveAdminNames([event]); + + return event; } catch (err) { throw err; } finally { @@ -193,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(); @@ -218,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(); @@ -240,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(); @@ -283,56 +341,23 @@ export const moveEvent = async (event: Event): Promise => { export const getNextUpcomingEvent = async (calendarId: number): Promise => { let conn = await NachklangCalendarDB.getConnection(); try { - const calendarQuery = 'SELECT calendar_id, includes_calendars FROM calendars WHERE calendar_id = ?'; - const calendarRes = await conn.query(calendarQuery, calendarId); - let calendarsToFetch: number[] = [calendarId]; - for(let row of calendarRes) { - let includes: number[] = JSON.parse(row.includes_calendars); - calendarsToFetch = [...calendarsToFetch, ...includes]; - } + const calendars = await calendarsToFetch(conn, calendarId); const now = new Date(); - const eventsQuery = ` - SELECT e.calendar_id, e.uuid, e.created_date, e.created_by_id, u.full_name as created_by_name, u2.full_name as last_modified_by_name, v.* FROM events e - INNER JOIN ( - SELECT event_id, MAX(event_version_id) AS latest_version - FROM event_versions - GROUP BY event_id - ) latest_versions - ON e.event_id = latest_versions.event_id - INNER JOIN event_versions v - ON v.event_id = latest_versions.event_id AND v.event_version_id = latest_versions.latest_version - LEFT OUTER JOIN users u ON u.user_id = e.created_by_id - LEFT OUTER JOIN users u2 ON u2.user_id = v.version_created_by_id + const eventsQuery = `${EVENT_SELECT} WHERE e.calendar_id IN (?) AND v.status = 'PUBLIC' AND v.start_datetime > ? ORDER BY v.start_datetime ASC LIMIT 1`; - const eventsRes = await conn.query(eventsQuery, [calendarsToFetch, now]); + const eventsRes = await conn.query(eventsQuery, [calendars, now]); if (eventsRes.length === 0) { return null; } - const row = eventsRes[0]; - return { - eventId: row.event_id, - calendarId: row.calendar_id, - uuid: row.uuid, - name: row.name, - description: row.description, - startDateTime: row.start_datetime, - endDateTime: row.end_datetime, - createdDate: row.created_date, - lastModifiedDate: row.version_created_at, - location: row.location, - createdBy: row.created_by_name, - createdById: row.created_by_id, - lastModifiedBy: row.last_modified_by_name, - lastModifiedById: row.version_created_by_id, - url: row.url, - wholeDay: row.whole_day, - repeatFrequency: row.repeat_frequency - } as Event; + const event = toEvent(eventsRes[0], false); + await resolveAdminNames([event]); + + return event; } catch (err) { throw err; } finally { 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 new file mode 100644 index 0000000..8131846 --- /dev/null +++ b/test/calendar/credentials.service.test.ts @@ -0,0 +1,49 @@ +import {describe, expect, it, beforeEach} from 'vitest'; + +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 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(() => { + 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 password at all', async () => { + await expect(CredentialService.hasAccess('public', '')).resolves.toBe(true); + }); + + it.each([ + ['members', 'member-secret'], + ['choir', 'choir-secret'], + ['management', 'management-secret'], + ['birthdays', 'choir-secret'] + ])('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')).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..79c3ef5 --- /dev/null +++ b/test/calendar/events.router.test.ts @@ -0,0 +1,224 @@ +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.getNextUpcomingEvent as any).mockResolvedValue({eventId: 1, name: 'Konzert'}); + (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); + }); + + // The endpoint www.nachklang.art actually calls for its next-event teaser. + // Tested separately from /json because it takes a different code path - it + // has no admin view and no editor branch - so covering /json proves nothing + // about it, and its failure is invisible until someone notices the website + // has gone quiet. + it('serves the next upcoming event anonymously on the public calendar', async () => { + await request(app).get('/calendar/events/public/json/next').expect(200); + + // And without asking the admin database who the caller is: the public + // feed must not acquire a dependency it has never had. + expect(auth.api.getSession).not.toHaveBeenCalled(); + }); + + it('refuses the next upcoming event on a restricted calendar without a credential', async () => { + await request(app).get('/calendar/events/members/json/next').expect(403); + }); + + it('serves the next upcoming event to a shared password', async () => { + await request(app) + .get('/calendar/events/members/json/next') + .query({password: 'member-secret'}) + .expect(200); + }); + + it('serves the next upcoming event to a signed-in editor', async () => { + signedInAs(['calendar']); + + await request(app).get('/calendar/events/members/json/next').expect(200); + }); + + it('does not consult the admin database for the anonymous public iCal export', async () => { + await request(app).get('/calendar/events/public/ical').expect(200); + + expect(auth.api.getSession).not.toHaveBeenCalled(); + }); + + 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); + }); +}); diff --git a/test/calendar/events.service.test.ts b/test/calendar/events.service.test.ts new file mode 100644 index 0000000..19489e1 --- /dev/null +++ b/test/calendar/events.service.test.ts @@ -0,0 +1,191 @@ +import {describe, expect, it, vi, beforeEach} from 'vitest'; + +const connection = { + query: vi.fn(), + execute: vi.fn(), + beginTransaction: vi.fn(), + commit: vi.fn(), + rollback: vi.fn(), + end: vi.fn() +}; + +vi.mock('../../src/models/calendar/Calendar.db.js', () => ({ + NachklangCalendarDB: {getConnection: vi.fn(async () => connection)} +})); + +vi.mock('../../src/models/admin/users/users.admin.service.js', () => ({ + findDisplayNames: vi.fn() +})); + +import {NachklangCalendarDB} from '../../src/models/calendar/Calendar.db.js'; +import * as AdminUsersService from '../../src/models/admin/users/users.admin.service.js'; +import * as EventService from '../../src/models/calendar/events/events.service.js'; + +/** + * Step 3 of docs/calendar-auth-migration.md. The property under test is that + * an event's creator resolves from whichever of its three possible sources is + * strongest - the live admin name, then the snapshot from migration 002, then + * the legacy join - and that a failure to reach the admin database costs a name + * rather than the whole response: the public calendar is read anonymously by + * the website and has never depended on the admin database being up. + */ + +// One row of the shape the shared SELECT produces. +const row = (over: Record = {}) => ({ + event_id: 1, + calendar_id: 1, + uuid: 'uuid-1', + name: 'Konzert', + description: '', + start_datetime: new Date('2026-04-18T19:00:00Z'), + end_datetime: new Date('2026-04-18T21:00:00Z'), + created_date: new Date('2026-01-01T00:00:00Z'), + version_created_at: new Date('2026-01-02T00:00:00Z'), + location: '', + created_by_id: 7, + created_by_user_id: null, + created_by_name: null, + legacy_created_by_name: 'Legacy Person', + version_created_by_id: 7, + version_created_by_user_id: null, + version_created_by_name: null, + legacy_last_modified_by_name: 'Legacy Person', + url: '', + whole_day: 0, + repeat_frequency: '', + status: 'PUBLIC', + ...over +}); + +/** getAllEvents runs the calendars lookup first, then the events query. */ +const givenEvents = (...rows: unknown[]) => { + connection.query.mockReset(); + connection.query + .mockResolvedValueOnce([{calendar_id: 1, includes_calendars: '[]'}]) + .mockResolvedValueOnce(rows); +}; + +beforeEach(() => { + vi.clearAllMocks(); + connection.end.mockResolvedValue(undefined); + (NachklangCalendarDB.getConnection as any).mockResolvedValue(connection); +}); + +describe('creator names', () => { + it('uses the legacy join when the row has no admin id', async () => { + givenEvents(row()); + + const events = await EventService.getAllEvents(1); + + expect(events[0].createdBy).toBe('Legacy Person'); + expect(events[0].createdById).toBe(7); + expect(events[0].createdByUserId).toBeNull(); + // Nothing to resolve, so the admin database is not touched at all. + expect(AdminUsersService.findDisplayNames).not.toHaveBeenCalled(); + }); + + it('prefers the admin name when the row carries an admin id', async () => { + givenEvents(row({ + created_by_user_id: 'admin-1', + version_created_by_user_id: 'admin-2' + })); + (AdminUsersService.findDisplayNames as any).mockResolvedValue( + new Map([['admin-1', 'Neue Person'], ['admin-2', 'Andere Person']]) + ); + + const events = await EventService.getAllEvents(1); + + expect(events[0].createdBy).toBe('Neue Person'); + expect(events[0].lastModifiedBy).toBe('Andere Person'); + // The legacy id is still reported during the transition. + expect(events[0].createdById).toBe(7); + expect(events[0].createdByUserId).toBe('admin-1'); + }); + + it('prefers the snapshot over the legacy join', async () => { + givenEvents(row({ + created_by_name: 'Archived Person', + version_created_by_name: 'Archived Person' + })); + + const events = await EventService.getAllEvents(1); + + expect(events[0].createdBy).toBe('Archived Person'); + expect(events[0].lastModifiedBy).toBe('Archived Person'); + }); + + it('prefers the live admin name over the snapshot', async () => { + // A renamed account has to win over an archive that was correct when it + // was taken - otherwise renaming someone would leave stale names behind. + givenEvents(row({created_by_user_id: 'admin-1', created_by_name: 'Archived Person'})); + (AdminUsersService.findDisplayNames as any).mockResolvedValue(new Map([['admin-1', 'Neue Person']])); + + const events = await EventService.getAllEvents(1); + + expect(events[0].createdBy).toBe('Neue Person'); + }); + + it('keeps the snapshot when step 5 has removed the legacy join', async () => { + // What a post-step-5 row looks like: no legacy id, no join, snapshot only. + givenEvents(row({ + created_by_id: null, + legacy_created_by_name: undefined, + legacy_last_modified_by_name: undefined, + created_by_name: 'Archived Person', + version_created_by_name: 'Archived Person' + })); + + const events = await EventService.getAllEvents(1); + + expect(events[0].createdBy).toBe('Archived Person'); + expect(events[0].lastModifiedBy).toBe('Archived Person'); + }); + + it('falls back to the legacy name when the admin account is gone', async () => { + givenEvents(row({created_by_user_id: 'deleted'})); + (AdminUsersService.findDisplayNames as any).mockResolvedValue(new Map()); + + const events = await EventService.getAllEvents(1); + + expect(events[0].createdBy).toBe('Legacy Person'); + }); + + it('resolves a mixed result set in a single lookup', async () => { + givenEvents( + row({event_id: 1}), + row({event_id: 2, created_by_user_id: 'admin-1', version_created_by_user_id: 'admin-1'}), + row({event_id: 3, created_by_user_id: 'admin-1', version_created_by_user_id: 'admin-1'}) + ); + (AdminUsersService.findDisplayNames as any).mockResolvedValue(new Map([['admin-1', 'Neue Person']])); + + const events = await EventService.getAllEvents(1); + + expect(events.map(e => e.createdBy)).toEqual(['Legacy Person', 'Neue Person', 'Neue Person']); + expect(AdminUsersService.findDisplayNames).toHaveBeenCalledTimes(1); + }); + + it('still returns the events when the admin database is unreachable', async () => { + givenEvents(row({created_by_user_id: 'admin-1'})); + (AdminUsersService.findDisplayNames as any).mockRejectedValue(new Error('ECONNREFUSED')); + + const events = await EventService.getAllEvents(1); + + expect(events).toHaveLength(1); + expect(events[0].name).toBe('Konzert'); + // Degrades to the legacy name rather than failing the request. + expect(events[0].createdBy).toBe('Legacy Person'); + }); +}); + +describe('status', () => { + it('is omitted from the public listing and present in the admin one', async () => { + givenEvents(row()); + const publicEvents = await EventService.getAllEvents(1); + expect(publicEvents[0].status).toBeUndefined(); + + connection.query.mockReset(); + connection.query.mockResolvedValueOnce([row()]); + const adminEvents = await EventService.getAllEventsAdmin(1); + expect(adminEvents[0].status).toBe('PUBLIC'); + }); +});