From 1ffe3609f41fd3ff9bd3462e480f105dc917d676 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20M=2E=20Requena=20Plens?= Date: Sun, 6 Sep 2026 19:48:02 +0200 Subject: [PATCH 1/2] Invalidate tokens issued before a password change Tokens are stateless JWTs, so changing a password left every session that the old one had opened working until its own expiry, up to a day later. That is the case the password change is meant to close: an administrator resetting a compromised account did not evict whoever was already in it. The auth row already records when the password last changed, so no migration is needed: `Access.init()` reads it alongside the user it already loads and refuses a token whose `iat` is older. Both sides are compared as whole seconds, which is all `iat` carries, so a token minted in the same second as the change is kept. Postgres stores that column to the microsecond, which is why the comparison is not done in milliseconds. It is reported as 401 rather than the usual 403 because that is what the frontend clears the session on, so the browser holding the dead token lands on the login page instead of a page full of errors, and `can()` lets that one error through unwrapped for the same reason. Only the password does this. A user row changing (a rename, an avatar, permissions) does not, and a user with no password auth row, which is what a login through an external provider looks like, is not affected. --- backend/lib/access.js | 29 +++++++++++++++++++++ backend/lib/error.js | 9 +++++++ test/cypress/e2e/api/Users.cy.js | 43 ++++++++++++++++++++++++++++++++ 3 files changed, 81 insertions(+) diff --git a/backend/lib/access.js b/backend/lib/access.js index 5f96544a..9de1461b 100644 --- a/backend/lib/access.js +++ b/backend/lib/access.js @@ -12,6 +12,7 @@ import { fileURLToPath } from "node:url"; import Ajv from "ajv/dist/2020.js"; import _ from "lodash"; import { access as logger } from "../logger.js"; +import authModel from "../models/auth.js"; import proxyHostModel from "../models/proxy_host.js"; import TokenModel from "../models/token.js"; import userModel from "../models/user.js"; @@ -80,6 +81,29 @@ export default function (tokenString) { if (!ok) { throw new errs.AuthError("Invalid token scope for User"); } + + // A token issued before the password was last changed is no longer valid: taking an account + // back from whoever has the old password has to end the sessions that password opened. + const auth = await authModel + .query() + .where("user_id", "=", user.id) + .where("type", "=", "password") + .first(); + + if (auth && typeof tokenData.iat === "number") { + // SQLite gives this back as a local time string, the other drivers as a Date. + const changedAt = + auth.modified_on instanceof Date + ? auth.modified_on.getTime() + : Date.parse(String(auth.modified_on).replace(" ", "T")); + + // Whole seconds on both sides, which is all `iat` carries, so a token issued in the + // same second as the change is kept. Postgres stores this column to the microsecond. + if (!Number.isNaN(changedAt) && tokenData.iat < Math.floor(changedAt / 1000)) { + throw new errs.TokenRevokedError("Token was issued before the password was changed"); + } + } + initialised = true; userRoles = user.roles; permissions = user.permissions; @@ -268,6 +292,11 @@ export default function (tokenString) { err.permission = permission; err.permission_data = data; logger.error(permission, data, err.message); + // A revoked token is not a permission problem, and the client can tell: the frontend + // clears the session on a 401 and on nothing else. + if (err instanceof errs.TokenRevokedError) { + throw err; + } throw new errs.PermissionError("Permission Denied", err); } }, diff --git a/backend/lib/error.js b/backend/lib/error.js index d7dbf0c9..06098890 100644 --- a/backend/lib/error.js +++ b/backend/lib/error.js @@ -22,6 +22,15 @@ const errs = { this.status = 404; }, + TokenRevokedError: function (message, previous) { + Error.captureStackTrace(this, this.constructor); + this.name = this.constructor.name; + this.previous = previous; + this.message = message; + this.public = true; + this.status = 401; + }, + AuthError: function (message, messageI18n, previous) { Error.captureStackTrace(this, this.constructor); this.name = this.constructor.name; diff --git a/test/cypress/e2e/api/Users.cy.js b/test/cypress/e2e/api/Users.cy.js index 044dcdee..41808d0a 100644 --- a/test/cypress/e2e/api/Users.cy.js +++ b/test/cypress/e2e/api/Users.cy.js @@ -31,6 +31,49 @@ describe('Users endpoints', () => { }); }); + it('Should reject a token that was issued before the password changed', () => { + // The token carries whole seconds, so it has to predate the change by one. + cy.wait(1100); + + cy.task('backendApiPut', { + token: token, + path: '/api/users/me/auth', + data: { + type: 'password', + current: 'changeme', + secret: 'changeme2' + } + }).then(() => { + cy.task('backendApiGet', { + token: token, + path: '/api/users/me', + returnOnError: true + }).then((data) => { + expect(data).to.have.property('error'); + expect(data.error).to.have.property('code'); + expect(data.error.code).to.equal(401); + }); + + // Put the password back, the rest of the suite shares this user, and take a + // token minted after the change: restoring it invalidates the one that made it. + cy.getToken(null, {secret: 'changeme2'}).then((tempToken) => { + cy.task('backendApiPut', { + token: tempToken, + path: '/api/users/me/auth', + data: { + type: 'password', + current: 'changeme2', + secret: 'changeme' + } + }).then(() => { + cy.getToken().then((freshToken) => { + token = freshToken; + }); + }); + }); + }); + }); + it('Should be able to update yourself', () => { cy.task('backendApiPut', { token: token, From e4585ac688891a2c20931367d46415efb0378c7b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20M=2E=20Requena=20Plens?= Date: Sun, 6 Sep 2026 20:15:12 +0200 Subject: [PATCH 2/2] Stamp the password change from the app clock, not the database one Your CI caught this: the check passed on SQLite and never fired on the stack where the database container runs on a different timezone from the app, so a stale token stayed valid. The comparison was between a token's `iat`, which is UTC seconds from Node, and `auth.modified_on`, which the driver hands back interpreted in the app's timezone. With the app on Australia/Brisbane and the database on UTC, that column comes back ten hours in the past and the token always looks newer than the change. Record the moment in `auth.meta.password_changed_at` instead, written by `setPassword` with the same `Date.now()` clock that mints `iat`. Same unit on both sides, one clock, and no timestamp parsing: the Date and local-string branch is gone, and so is the whole-second flooring that Postgres microseconds made necessary. Rows written before this have no marker and revoke nothing until their next password change, which is the safe direction to be wrong in. --- backend/internal/user.js | 12 +++++++++++- backend/lib/access.js | 17 +++++------------ 2 files changed, 16 insertions(+), 13 deletions(-) diff --git a/backend/internal/user.js b/backend/internal/user.js index d4080dd7..8dd86f46 100644 --- a/backend/internal/user.js +++ b/backend/internal/user.js @@ -388,11 +388,21 @@ const internalUser = { .andWhere("type", data.type) .first() .then((existing_auth) => { + // Stamped here rather than read off modified_on, because it is compared against a + // token's `iat` and the two only line up when the same clock writes both. The + // database clock is a different one: with the app on one timezone and the database + // on another, its timestamps come back hours away from where Node thinks it is. + const password_changed_at = Math.floor(Date.now() / 1000); + if (existing_auth) { // patch + const meta = existing_auth.meta || {}; + meta.password_changed_at = password_changed_at; + return authModel.query().where("user_id", user.id).andWhere("type", data.type).patch({ type: data.type, // This is required for the model to encrypt on save secret: data.secret, + meta, }); } // insert @@ -400,7 +410,7 @@ const internalUser = { user_id: user.id, type: data.type, secret: data.secret, - meta: {}, + meta: { password_changed_at }, }); }) .then(() => { diff --git a/backend/lib/access.js b/backend/lib/access.js index 9de1461b..d119a4f9 100644 --- a/backend/lib/access.js +++ b/backend/lib/access.js @@ -90,18 +90,11 @@ export default function (tokenString) { .where("type", "=", "password") .first(); - if (auth && typeof tokenData.iat === "number") { - // SQLite gives this back as a local time string, the other drivers as a Date. - const changedAt = - auth.modified_on instanceof Date - ? auth.modified_on.getTime() - : Date.parse(String(auth.modified_on).replace(" ", "T")); - - // Whole seconds on both sides, which is all `iat` carries, so a token issued in the - // same second as the change is kept. Postgres stores this column to the microsecond. - if (!Number.isNaN(changedAt) && tokenData.iat < Math.floor(changedAt / 1000)) { - throw new errs.TokenRevokedError("Token was issued before the password was changed"); - } + // Both sides come from the same clock and in the same unit, whole seconds since + // the epoch: `setPassword` stamps the marker and `jsonwebtoken` stamps `iat`. + const changedAt = auth?.meta?.password_changed_at; + if (changedAt && typeof tokenData.iat === "number" && tokenData.iat < changedAt) { + throw new errs.TokenRevokedError("Token was issued before the password was changed"); } initialised = true;