Merge pull request #5836 from jmrplens/fix/invalidate-tokens-on-password-change

Invalidate tokens issued before a password change

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(() => {

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);
 			}
 		},

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;

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,