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 5f96544a..d119a4f9 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,22 @@ 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(); + + // 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; userRoles = user.roles; permissions = user.permissions; @@ -268,6 +285,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,