Re-validate JWT against the DB in isLoggedIn (fixes #12) #16

wtclaude · 2026-07-03T02:23:02Z

wtclaude commented

2026-07-03 02:23:02 +00:00

Summary

Fixes #12. isLoggedIn trusted id and role straight from the JWT and never re-checked the database. Because the token embeds role and lasts JWT_EXPIRES_IN (default 1d):

a user demoted from admin keeps their admin token until it expires;
a deleted user keeps a working session until the token expires.

This also undercut the "cannot demote/delete the last admin" guards in admin.controller.js, since the demoted/deleted admin still holds a valid token.

What changed

server/src/utils/auth.js
- isLoggedIn is now async and re-loads the user from the DB on every request:
  1. verify the token (unchanged) to obtain the id;
  2. users.getById(decoded.id);
  3. if the user no longer exists -> 401 (deleted since the token was issued);
  4. set req.user to the fresh DB row so the current role is always used downstream — the token payload's role is no longer trusted.
- DB errors are caught and return 500 rather than falling through.
- Added const users = require('../model/users/users.model').

Why this approach

DB re-fetch over a "session/token version" claim — the version-claim approach also works but needs a new column, a bump on every role-change/delete/password-reset, and a comparison here. The DB re-fetch is simpler, has no schema change, and is the same pattern the /auth/me handler already uses — so the authorization path now behaves consistently with the identity path.
In isLoggedIn rather than per-route — every admin route already funnels through isLoggedIn, so one change covers them all. This is an admin panel (low request volume), so one extra indexed lookup per request is a negligible cost for immediate revocation.
req.user becomes the sanitized DB row — getById already strips the password hash and returns { id, username, role, ... }. Every controller uses req.user.id / .username / .role, all still present, so nothing downstream breaks.

Scope / what was intentionally left alone

getUserFromRequest (token-only) is still used by the maintenance-mode bypass in middleware/siteMode.js. That path only decides whether a logged-in user can view the site during maintenance — not an access-control boundary — so it was left as-is to keep this change focused on the privilege path.

Testing

node -c syntax check.
Runtime require('./src/utils/auth') smoke test to confirm the new auth.js -> users.model -> users.db require chain has no circular dependency and loads cleanly.

Notes

Backend only, no schema/client changes. Independent of the companion PR for #10.

## Summary Fixes #12. `isLoggedIn` trusted `id` and `role` straight from the JWT and never re-checked the database. Because the token embeds `role` and lasts `JWT_EXPIRES_IN` (default `1d`): - a user **demoted** from admin keeps their admin token until it expires; - a **deleted** user keeps a working session until the token expires. This also undercut the "cannot demote/delete the last admin" guards in `admin.controller.js`, since the demoted/deleted admin still holds a valid token. ## What changed - **`server/src/utils/auth.js`** - `isLoggedIn` is now `async` and re-loads the user from the DB on every request: 1. verify the token (unchanged) to obtain the `id`; 2. `users.getById(decoded.id)`; 3. if the user no longer exists -> `401` (deleted since the token was issued); 4. set `req.user` to the **fresh DB row** so the current `role` is always used downstream — the token payload's `role` is no longer trusted. - DB errors are caught and return `500` rather than falling through. - Added `const users = require('../model/users/users.model')`. ## Why this approach - **DB re-fetch over a "session/token version" claim** — the version-claim approach also works but needs a new column, a bump on every role-change/delete/password-reset, and a comparison here. The DB re-fetch is simpler, has no schema change, and is the same pattern the `/auth/me` handler already uses — so the authorization path now behaves consistently with the identity path. - **In `isLoggedIn` rather than per-route** — every admin route already funnels through `isLoggedIn`, so one change covers them all. This is an admin panel (low request volume), so one extra indexed lookup per request is a negligible cost for immediate revocation. - **`req.user` becomes the sanitized DB row** — `getById` already strips the password hash and returns `{ id, username, role, ... }`. Every controller uses `req.user.id` / `.username` / `.role`, all still present, so nothing downstream breaks. ## Scope / what was intentionally left alone `getUserFromRequest` (token-only) is still used by the maintenance-mode bypass in `middleware/siteMode.js`. That path only decides whether a logged-in user can *view* the site during maintenance — not an access-control boundary — so it was left as-is to keep this change focused on the privilege path. ## Testing - `node -c` syntax check. - Runtime `require('./src/utils/auth')` smoke test to confirm the new `auth.js -> users.model -> users.db` require chain has **no circular dependency** and loads cleanly. ## Notes Backend only, no schema/client changes. Independent of the companion PR for #10.

wtclaude added 1 commit 2026-07-03 02:23:05 +00:00

Re-validate JWT against the DB in isLoggedIn (#12 ) 8ad18140d0

isLoggedIn trusted id and role straight from the JWT and never
re-checked the database, so a demoted admin kept their old role and a
deleted user kept a working session until the token expired (up to
JWT_EXPIRES_IN). This also undercut the "last admin" guards.

isLoggedIn now loads the user from the DB by the token's id on every
request: a missing user returns 401 (deleted), and req.user carries the
fresh DB row so the current role is always used downstream.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

whitlocktech approved these changes 2026-07-03 02:31:40 +00:00

whitlocktech scheduled this pull request to auto merge when all checks succeed 2026-07-03 02:31:47 +00:00

whitlocktech added 1 commit 2026-07-03 02:32:04 +00:00

Merge branch 'main' into fix/stale-jwt-revalidation 81318ae264

whitlocktech merged commit 6b04aa72c1 into main

2026-07-03 02:32:08 +00:00

whitlocktech referenced this issue from a commit

2026-07-03 02:32:09 +00:00

Merge pull request 'Re-validate JWT against the DB in isLoggedIn (fixes #12)' (#16) from fix/stale-jwt-revalidation into main

wtclaude referenced this pull request

2026-07-22 21:21:38 +00:00

ci(docs): auto-sync PROJECT_TREE.md to the docs repo on push to main #98