# Deferred Security Issues

These items were identified during a security review on 2026-05-02 and consciously deferred.
**Must be addressed before opening the application to a larger or public userbase.**

---

## 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` were read from query parameters, which meant they appeared in
server access logs, browser history, proxy logs, and `Referer` headers.

**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.

Two things this did *not* change, both deliberate:

- 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.~~ Removed by step 5 on 2026-09-06,
  along with the `users` and `sessions` tables they used - renamed aside rather than dropped,
  so nothing was destroyed.

---

## 2. No event ownership check

**Files:** `src/models/calendar/events/events.router.ts`
- `PUT /:eventId` (update)
- `PUT /move/:eventId` (move)
- `DELETE /:eventId` (delete)

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 (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.

---

## 3. Activation token has no expiry — CLOSED 2026-09-06

**File:** ~~`src/models/calendar/users/users.service.ts`~~ — deleted.

The e-mail activation link was valid indefinitely. Closed not by adding an expiry but by
removing the thing that issued it: step 5 of `docs/calendar-auth-migration.md` deleted the
calendar's own account system. Accounts now come only from the admin module's `invitations`,
whose tokens expire after 7 days and are stored as a SHA-256 hash.

Any activation link still sitting in an inbox now 404s. It only ever activated a legacy
account, which no longer opens anything.

---

## 4. Password reset token has no expiry — CLOSED 2026-09-06

**File:** ~~`src/models/calendar/users/users.service.ts`~~ — deleted.

Same as item 3: `pw_reset_token_hash` never expired, and the code that set it no longer
exists. Password resets go through better-auth, whose reset tokens expire after one hour.