69c1e4926c
Every step is live as of 2026-09-06. The status header said "awaiting deploy", which is exactly the kind of staleness that nearly caused a bad deploy earlier in this project - the step 4 checklist was written because "done" had been read as "in production". Also records the one surprise from the deploy: a cached pre-cutover bundle looked signed in and showed every event's status as "Error", because the legacy checkSessionValid route still answered between steps 4 and 5 while the events call went out without a cookie and came back without a status field. That route is gone now, so a stale bundle fails honestly instead. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
279 lines
18 KiB
Markdown
279 lines
18 KiB
Markdown
# Migrating the Calendar domain onto the admin identity module
|
|
|
|
Status: **complete and deployed, 2026-09-06.** Every step is live. Step 2 was dropped by
|
|
decision and part of step 5 brought forward; the rest went out as written.
|
|
|
|
The calendar now shares one identity with the tickets, feedback and admin apps: writes sit
|
|
behind `requireAppAccess('calendar')` against the shared session cookie, reads resolve that
|
|
cookie optionally, and the calendar's own `users`/`sessions` tables are renamed aside and
|
|
referenced by nothing. `DEFERRED_SECURITY.md` items 1, 3 and 4 closed with it.
|
|
|
|
Verified in production after the final deploy: the public calendar answers anonymously on all
|
|
three endpoints (23 events, 23 VEVENTs in the iCal feed, the next-event teaser intact),
|
|
restricted calendars still refuse without a credential, unauthenticated writes answer 401,
|
|
all six `/calendar/users/*` routes answer 404, CORS grants only `Content-Type`, sign-out from
|
|
`calendar.nachklang.art` succeeds, and **every event kept its author** - rendered from the
|
|
`created_by_name` snapshot, since the table it was copied from no longer exists under that
|
|
name. That last one is the whole reason part of step 5 was brought forward.
|
|
|
|
Remaining, at your leisure: `DROP TABLE sessions_legacy_archive, users_legacy_archive;`
|
|
|
|
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
|
|
|
|
The admin module replaced authentication for feedback and tickets by swapping one
|
|
middleware. The calendar cannot be done that way, because its user identity is woven into
|
|
its data:
|
|
|
|
- `users`/`sessions` live in the **calendar** database and are the same tables the
|
|
feedback and tickets admin areas used to authenticate against.
|
|
- `events.created_by_id` is an **INT** foreign key into `users.user_id`. The admin module's
|
|
user ids are **VARCHAR(36)** strings. Migrating identity means migrating that column and
|
|
every query that joins it.
|
|
- The Angular frontend passes `sessionId`/`sessionKey` as **query parameters**
|
|
(`DEFERRED_SECURITY.md` item 1). Cookie sessions remove the parameters entirely, so
|
|
every calendar route signature and the frontend's HTTP layer change together.
|
|
- `credentials.service.ts` implements a second, parallel authorisation model: the
|
|
`MEMBER_CREDENTIAL` / `CHOIR_CREDENTIAL` / `MANAGEMENT_CREDENTIAL` shared secrets that
|
|
let non-users read specific calendars. That has no equivalent in the admin module and is
|
|
not a per-user permission at all.
|
|
|
|
What already exists today: `calendar` is a value in the `user_app_permissions.app` enum, so
|
|
permissions can be granted before anything else moves.
|
|
|
|
## What is in place to build on
|
|
|
|
- Cookie sessions across `*.nachklang.art`, and `requireAppAccess('calendar')` in
|
|
`src/models/admin/admin.middleware.ts` - usable the moment a calendar route wants it.
|
|
- `res.locals.admin` is `{id, email, displayName, apps}`; `id` is the string user id.
|
|
- Invitations, disable/enable and session revocation already cover calendar users, because
|
|
they are properties of the account rather than of an app.
|
|
|
|
## Suggested sequence
|
|
|
|
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.~~ **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.** **Deployed 2026-09-06.** Gated on step 4 being live, which it
|
|
was.
|
|
|
|
What went:
|
|
|
|
- `src/models/calendar/users/` in its entirety - registration, login, activation, both
|
|
password-reset routes, and the session checking the feedback and tickets admin areas
|
|
used to authenticate against - plus its mount in `Calendar.router.ts`. That was the
|
|
API's last unauthenticated account-creation and mail-sending endpoint.
|
|
- The two `LEFT OUTER JOIN users` clauses in `events.service.ts` and the `legacy_*`
|
|
aliases they fed, along with `created_by_id` / `version_created_by_id` in the SELECT, the
|
|
row mapper and the `Event` interface. One name source remains besides the live admin
|
|
lookup: the snapshot, which is what made this safe.
|
|
- `X-Session-Id` / `X-Session-Key` from the CORS `allowedHeaders`. Nothing had read them
|
|
since the tickets and feedback cutover, or sent them since this one.
|
|
- The tripwire in `test/admin/auth-binding.ts` asserting neither module fell back to a
|
|
calendar header session. There is nothing left to fall back to.
|
|
|
|
`DEFERRED_SECURITY.md` items **3** and **4** (activation and reset tokens never expiring)
|
|
close with it - not by adding expiries but by deleting the code that issued them.
|
|
|
|
### Deploy checklist — kept for the record; the order was REVERSED from step 4
|
|
|
|
Step 4's migration only added columns, so it went first. `004` *removes* columns and a
|
|
table that the currently running build still selects and joins, so running it first fails
|
|
every calendar read including the public feed. The step 5 build references none of them and
|
|
runs happily against the old schema. Therefore:
|
|
|
|
1. **Confirm the snapshot is complete.** Both must return 0:
|
|
```sql
|
|
SELECT SUM(created_by_id IS NOT NULL AND created_by_name IS NULL) FROM events;
|
|
SELECT SUM(version_created_by_id IS NOT NULL AND version_created_by_name IS NULL) FROM event_versions;
|
|
```
|
|
A non-zero count is an event whose author `004` would erase. Re-run 002's backfill first.
|
|
2. **Deploy the API.** No frontend deploy is needed: the calendar frontend never read
|
|
`createdById` (its `Event` model has only the name), and nothing else is known to.
|
|
3. **Confirm the calendar still works** - the public feed, a signed-in read, and one save.
|
|
At this point the old columns and tables still exist, unused, so this step is fully
|
|
reversible by redeploying the previous build.
|
|
|
|
Note that `tsc` does not remove output for deleted sources, so a build over an existing
|
|
`dist/` leaves `dist/src/models/calendar/users/*.js` behind. Nothing imports it and the
|
|
routes 404, but the deployed artifact still contains the code - clear `dist/` in the
|
|
pipeline if you want the artifact to match the source.
|
|
4. **Apply `sql/calendar/004_drop_legacy_auth.sql`.** This is the point of no return for
|
|
the columns; the accounts themselves are only renamed aside.
|
|
5. Optionally, later and at a quiet moment:
|
|
`DROP TABLE sessions_legacy_archive, users_legacy_archive;`
|
|
|
|
**One-way door:** `Event.createdById` and `lastModifiedById` leave the API response. Check
|
|
anything reading `/calendar/events/*/json` that is not the calendar frontend.
|
|
|
|
**Observed during the deploy, worth keeping.** A browser holding a *cached pre-cutover*
|
|
Angular bundle looked signed in and showed every event's status as "Error". The old bundle
|
|
called `/calendar/users/checkSessionValid`, which still existed between step 4 and step 5,
|
|
so it rendered as authenticated - then fetched events with no cookie, got the anonymous
|
|
listing, which omits `status`, and the UI's status switch fell through to its error label.
|
|
Signing out and back in fixed it. After this step that route 404s, so a stale bundle now
|
|
fails honestly instead of faking a session. This is the same "does not look broken" window
|
|
the step 4 checklist warns about, seen from the other side.
|
|
|
|
## 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
|
|
|
|
**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`.
|