Files
API/DEFERRED_SECURITY.md
T
Paddy 3c892d02ed
Jenkins Production Deployment
Put the feedback and tickets admin areas behind the shared identity (#13)
Reviewed-on: #13
Co-authored-by: Patrick Müller <mail@pmueller.me>
Co-committed-by: Patrick Müller <mail@pmueller.me>
2026-09-06 19:06:30 +00:00

3.7 KiB

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)

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.

Fix (updated 2026-09-06): Move the calendar onto the shared admin identity - requireAppAccess('calendar') against the better-auth session cookie, per docs/calendar-auth-migration.md. That closes this item outright rather than moving the credential to a safer place, and it is now the cheaper of the two: the feedback and tickets modules made the same move on 2026-09-06 for one line each.

Move to request headers (X-Session-Id / X-Session-Key) or the request body. No longer the recommendation. Nothing on the server reads those two headers any more - the calendar's query parameters are the last legacy credential path in the API - so this would build a second mechanism just as the first is being retired. They survive only in the CORS allowedHeaders list, and only until both frontends are redeployed.

Either fix requires a corresponding frontend update.

Note: the shared calendar password parameter in query params is intentional (iCal clients don't support headers) and is acceptable for the current setup.


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 active user can edit, move, or delete any event regardless of who created it. This is acceptable while all users are trusted admins.

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.


3. Activation token has no expiry

Superseded for new accounts (2026-09-05). The admin module (src/models/admin/) replaced account creation for the feedback, tickets and admin apps: accounts now come from invitations, whose tokens expire after 7 days and are stored only as a SHA-256 hash. The item below still stands for the legacy calendar users table, which the admin module deliberately left alone - see docs/calendar-auth-migration.md.

File: src/models/calendar/users/users.service.tscreateUser / activateUser

The email activation link is valid indefinitely. Acceptable for a small, trusted userbase.

Fix:

  1. Add an activation_expires column to the users table (e.g. DATETIME).
  2. Set it to NOW() + INTERVAL 24 HOUR in createUser.
  3. Check activation_expires > NOW() in activateUser before accepting the token.

4. Password reset token has no expiry

Superseded for new accounts (2026-09-05). Password resets for admin-module accounts go through better-auth, whose reset tokens expire after one hour. As with item 3, the text below still applies to the legacy calendar users table.

File: src/models/calendar/users/users.service.tsinitiatePasswordReset / finalizePasswordReset

The reset token stored in pw_reset_token_hash never expires. Acceptable for a small, trusted userbase.

Fix:

  1. Add a pw_reset_expires column to the users table (e.g. DATETIME).
  2. Set it to NOW() + INTERVAL 15 MINUTE in initiatePasswordReset.
  3. Check pw_reset_expires > NOW() in finalizePasswordReset before accepting the token.