feat: Disallow repeating last 5 passwords.

We'll store hashes for the last 5 passwords, fetch them all for the user
wanting to change their password, and make sure the password does not
verify against any of the 5 stored hashes.

src/lib/db/user-store.ts

@@ -11,8 +11,10 @@ import type {
     IUserUpdateFields,
 } from '../types/stores/user-store';
 import type { Db } from './db';
+import bcrypt from 'bcryptjs';
 
 const TABLE = 'users';
+const PASSWORD_HASH_TABLE = 'used_passwords';
 
 const USER_COLUMNS_PUBLIC = [
     'id',
@@ -25,6 +27,8 @@ const USER_COLUMNS_PUBLIC = [
     'scim_id',
 ];
 
+const USED_PASSWORDS = ['user_id', 'password_hash', 'used_at'];
+
 const USER_COLUMNS = [...USER_COLUMNS_PUBLIC, 'login_attempts', 'created_at'];
 
 const emptify = (value) => {
@@ -71,6 +75,35 @@ class UserStore implements IUserStore {
         this.logger = getLogger('user-store.ts');
     }
 
+    async passwordPreviouslyUsed(
+        userId: number,
+        password: string,
+    ): Promise<boolean> {
+        const previouslyUsedPasswords = await this.db(
+            PASSWORD_HASH_TABLE,
+        ).where({ user_id: userId });
+        return previouslyUsedPasswords.some((row) =>
+            bcrypt.compareSync(password, row.password_hash),
+        );
+    }
+
+    async deletePasswordsUsedMoreThanNTimesAgo(
+        userId: number,
+        keepLastN: number,
+    ): Promise<void> {
+        await this.db.raw(
+            `
+            WITH UserPasswords AS (
+                SELECT user_id, password_hash, used_at, ROW_NUMBER() OVER (PARTITION BY user_id ORDER BY used_at DESC) AS rn
+            FROM ${PASSWORD_HASH_TABLE}
+            WHERE user_id = ?)
+            DELETE FROM ${PASSWORD_HASH_TABLE} WHERE user_id = ? AND (user_id, password_hash, used_at) NOT IN (SELECT user_id, password_hash, used_at FROM UserPasswords WHERE rn <= ?
+            );
+        `,
+            [userId, userId, keepLastN],
+        );
+    }
+
     async update(id: number, fields: IUserUpdateFields): Promise<User> {
         await this.activeUsers()
             .where('id', id)
@@ -177,10 +210,24 @@ class UserStore implements IUserStore {
         return item.password_hash;
     }
 
-    async setPasswordHash(userId: number, passwordHash: string): Promise<void> {
-        return this.activeUsers().where('id', userId).update({
+    async setPasswordHash(
+        userId: number,
+        passwordHash: string,
+        disallowNPreviousPasswords: number,
+    ): Promise<void> {
+        await this.activeUsers().where('id', userId).update({
             password_hash: passwordHash,
         });
+        if (passwordHash) {
+            await this.db(PASSWORD_HASH_TABLE).insert({
+                user_id: userId,
+                password_hash: passwordHash,
+            });
+            await this.deletePasswordsUsedMoreThanNTimesAgo(
+                userId,
+                disallowNPreviousPasswords,
+            );
+        }
     }
 
     async incLoginAttempts(user: User): Promise<void> {

src/lib/error/password-previously-used.ts

@@ -0,0 +1,11 @@
+import { UnleashError } from './unleash-error';
+
+export class PasswordPreviouslyUsed extends UnleashError {
+    statusCode = 400;
+
+    constructor(
+        message: string = `You've previously used this password, please use a new password`,
+    ) {
+        super(message);
+    }
+}

src/lib/routes/admin-api/user/user.test.ts

@@ -17,6 +17,7 @@ async function getSetup() {
     await stores.userStore.setPasswordHash(
         currentUser.id,
         await bcrypt.hash(oldPassword, 10),
+        5,
     );
 
     const config = createTestConfig({

src/lib/services/user-service.ts

@@ -38,6 +38,7 @@ import PasswordMismatch from '../error/password-mismatch';
 import type EventService from '../features/events/event-service';
 
 import { SYSTEM_USER, SYSTEM_USER_AUDIT } from '../types';
+import { PasswordPreviouslyUsed } from '../error/password-previously-used';
 
 export interface ICreateUser {
     name?: string;
@@ -62,6 +63,7 @@ export interface ILoginUserRequest {
 }
 
 const saltRounds = 10;
+const disallowNPreviousPasswords = 5;
 
 class UserService {
     private logger: Logger;
@@ -164,7 +166,11 @@ class UserService {
                     username,
                 });
                 const passwordHash = await bcrypt.hash(password, saltRounds);
-                await this.store.setPasswordHash(user.id, passwordHash);
+                await this.store.setPasswordHash(
+                    user.id,
+                    passwordHash,
+                    disallowNPreviousPasswords,
+                );
                 await this.accessService.setUserRootRole(
                     user.id,
                     RoleName.ADMIN,
@@ -232,7 +238,11 @@ class UserService {
 
         if (password) {
             const passwordHash = await bcrypt.hash(password, saltRounds);
-            await this.store.setPasswordHash(user.id, passwordHash);
+            await this.store.setPasswordHash(
+                user.id,
+                passwordHash,
+                disallowNPreviousPasswords,
+            );
         }
 
         const userCreated = await this.getUser(user.id);
@@ -387,8 +397,20 @@ class UserService {
 
     async changePassword(userId: number, password: string): Promise<void> {
         this.validatePassword(password);
+        const previouslyUsed = await this.store.passwordPreviouslyUsed(
+            userId,
+            password,
+        );
+        if (previouslyUsed) {
+            throw new PasswordPreviouslyUsed();
+        }
         const passwordHash = await bcrypt.hash(password, saltRounds);
-        await this.store.setPasswordHash(userId, passwordHash);
+
+        await this.store.setPasswordHash(
+            userId,
+            passwordHash,
+            disallowNPreviousPasswords,
+        );
         await this.sessionService.deleteSessionsForUser(userId);
         await this.resetTokenService.expireExistingTokensForUser(userId);
     }

src/lib/types/stores/user-store.ts

@@ -28,7 +28,12 @@ export interface IUserStore extends Store<IUser, number> {
     getAllWithId(userIdList: number[]): Promise<IUser[]>;
     getByQuery(idQuery: IUserLookup): Promise<IUser>;
     getPasswordHash(userId: number): Promise<string>;
-    setPasswordHash(userId: number, passwordHash: string): Promise<void>;
+    setPasswordHash(
+        userId: number,
+        passwordHash: string,
+        disallowNPreviousPasswords: number,
+    ): Promise<void>;
+    passwordPreviouslyUsed(userId: number, password: string): Promise<boolean>;
     incLoginAttempts(user: IUser): Promise<void>;
     successfullyLogin(user: IUser): Promise<void>;
     count(): Promise<number>;

src/migrations/20240705111827-used-passwords-table.js

@@ -0,0 +1,15 @@
+exports.up = function(db, cb) {
+  db.runSql(`
+    CREATE TABLE used_passwords(user_id INTEGER REFERENCES users(id) ON DELETE CASCADE,
+                                password_hash TEXT NOT NULL,
+                                used_at TIMESTAMP WITH TIME ZONE DEFAULT (now() AT time zone 'utc'),
+                                PRIMARY KEY (user_id, password_hash)
+    );
+    INSERT INTO used_passwords(user_id, password_hash) SELECT id, password_hash FROM users WHERE password_hash IS NOT NULL;
+    CREATE INDEX used_passwords_pw_hash_idx ON used_passwords(password_hash);
+`, cb)
+};
+
+exports.down = function(db, cb) {
+  db.runSql(`DROP TABLE used_passwords;`, cb);
+};

src/test/config/test-config.ts

@@ -13,6 +13,7 @@ function mergeAll<T>(objects: Partial<T>[]): T {
 }
 
 export function createTestConfig(config?: IUnleashOptions): IUnleashConfig {
+    getLogger.setMuteError(true);
     const testConfig: IUnleashOptions = {
         getLogger,
         authentication: { type: IAuthType.NONE, createAdminUser: false },

src/test/e2e/api/auth/reset-password-controller.e2e.test.ts

@@ -177,14 +177,14 @@ test('Trying to reset password with same token twice does not work', async () =>
         .post('/auth/reset/password')
         .send({
             token,
-            password,
+            password: `${password}test`,
         })
         .expect(200);
     await app.request
         .post('/auth/reset/password')
         .send({
             token,
-            password,
+            password: `${password}othertest`,
         })
         .expect(401)
         .expect((res) => {
@@ -245,7 +245,7 @@ test('Calling reset endpoint with already existing session should logout/destroy
         .post('/auth/reset/password')
         .send({
             token,
-            password,
+            password: `${password}newpassword`,
         })
         .expect(200);
     await request.get('/api/admin/projects').expect(401); // we no longer have a valid session after using the reset password endpoint

src/test/e2e/services/user-service.e2e.test.ts

@@ -27,6 +27,7 @@ import {
     USER_UPDATED,
 } from '../../../lib/types';
 import { CUSTOM_ROOT_ROLE_TYPE } from '../../../lib/util';
+import { PasswordPreviouslyUsed } from '../../../lib/error/password-previously-used';
 
 let db: ITestDb;
 let stores: IUnleashStores;
@@ -511,3 +512,51 @@ test('should support a custom root role id when logging in and creating user via
     expect(permissions).toHaveLength(1);
     expect(permissions[0].permission).toBe(CREATE_ADDON);
 });
+
+describe('Should not be able to use any of previous 5 passwords', () => {
+    test('throws exception when trying to use a previously used password', async () => {
+        const name = 'same-password-is-not-allowed';
+        const email = `${name}@test.com`;
+        const password = 'externalScreaming$123';
+        const user = await userService.createUser({
+            email,
+            rootRole: customRole.id,
+            name,
+            password,
+        });
+        await expect(
+            userService.changePassword(user.id, password),
+        ).rejects.toThrow(new PasswordPreviouslyUsed());
+    });
+    test('Is still able to change password to one not used', async () => {
+        const name = 'new-password-is-allowed';
+        const email = `${name}@test.com`;
+        const password = 'externalScreaming$123';
+        const user = await userService.createUser({
+            email,
+            rootRole: customRole.id,
+            name,
+            password,
+        });
+        await expect(
+            userService.changePassword(user.id, 'internalScreaming$123'),
+        ).resolves.not.toThrow();
+    });
+    test('Remembers 5 passwords', async () => {
+        const name = 'remembers-5-passwords-like-a-boss';
+        const email = `${name}@test.com`;
+        const password = 'externalScreaming$123';
+        const user = await userService.createUser({
+            email,
+            rootRole: customRole.id,
+            name,
+            password,
+        });
+        for (let i = 0; i < 5; i++) {
+            await userService.changePassword(user.id, `${password}${i}`);
+        }
+        await expect(
+            userService.changePassword(user.id, `${password}`),
+        ).resolves.not.toThrow(); // We've added 5 new passwords, so the original should work again
+    });
+});

src/test/e2e/stores/user-store.e2e.test.ts

@@ -60,7 +60,7 @@ test('Should require email or username', async () => {
 test('should set password_hash for user', async () => {
     const store = stores.userStore;
     const user = await store.insert({ email: '[email protected]' });
-    await store.setPasswordHash(user.id, 'rubbish');
+    await store.setPasswordHash(user.id, 'rubbish', 5);
     const hash = await store.getPasswordHash(user.id);
 
     expect(hash).toBe('rubbish');

src/test/fixtures/fake-user-store.ts

@@ -15,6 +15,13 @@ class UserStoreMock implements IUserStore {
         this.idSeq = 1;
         this.data = [];
     }
+
+    passwordPreviouslyUsed(
+        _userId: number,
+        _passwordHash: string,
+    ): Promise<boolean> {
+        return Promise.resolve(false);
+    }
     countServiceAccounts(): Promise<number> {
         return Promise.resolve(0);
     }