From b261af5d9ce08bbc243dc4b862daa939f2426fee Mon Sep 17 00:00:00 2001 From: Artem Niehrieiev Date: Thu, 5 Nov 2026 10:06:12 +0000 Subject: [PATCH 1/3] feat: Support legacy V1 ciphertext in encryption checks and upgrade to V2 format --- backend/src/helpers/encryption/encryptor.ts | 10 +++++--- backend/test/ava-tests/encryptor.test.ts | 4 +++- .../saas-tests/saas-user-login-e2e.test.ts | 24 +++++++++++++++++++ 3 files changed, 34 insertions(+), 4 deletions(-) diff --git a/backend/src/helpers/encryption/encryptor.ts b/backend/src/helpers/encryption/encryptor.ts index 83fdf4e20..0668466b8 100644 --- a/backend/src/helpers/encryption/encryptor.ts +++ b/backend/src/helpers/encryption/encryptor.ts @@ -9,6 +9,8 @@ import { appConfig } from '../../shared/config/app-config.js'; import { Constants } from '../constants/constants.js'; const ENCRYPTION_VERSION_PREFIX = '$v2:k1$'; +// Base64 of `Salted__`, the first bytes of every CryptoJS (V1) ciphertext. +const V1_LEGACY_PREFIX = 'U2FsdGVkX1'; const ENCRYPTION_ALGORITHM = 'aes-256-gcm'; const KDF_ITERATIONS = 100000; const SALT_LENGTH = 32; @@ -115,10 +117,12 @@ export class Encryptor { } } - // True when the value already carries the V2 envelope. The user entity hooks use it to - // encrypt a value once and to skip a value that is still in clear text on load. + // True when the value is ciphertext of this class: the V2 envelope, or the legacy V1 format. + // CryptoJS writes V1 as base64 of the OpenSSL `Salted__` block, so it always starts with + // `U2FsdGVkX1`. A clear-text value such as a base32 OTP seed never does. The user entity hooks + // use it to decrypt both formats on load and to encrypt only clear text on save. static isEncrypted(value: string): boolean { - return Encryptor.isV2Format(value); + return Encryptor.isV2Format(value) || value.startsWith(V1_LEGACY_PREFIX); } static decryptData(encryptedData: string): string; diff --git a/backend/test/ava-tests/encryptor.test.ts b/backend/test/ava-tests/encryptor.test.ts index 7b25dcc8d..1210c8bd8 100644 --- a/backend/test/ava-tests/encryptor.test.ts +++ b/backend/test/ava-tests/encryptor.test.ts @@ -213,7 +213,9 @@ test('encryptData: throws instead of returning the plaintext when the private ke } }); -test('isEncrypted: true only for the V2 envelope', (t) => { +test('isEncrypted: true for the V2 envelope and for legacy V1 ciphertext, false for clear text', (t) => { t.true(Encryptor.isEncrypted(Encryptor.encryptData(TEST_DATA))); + t.true(Encryptor.isEncrypted(CryptoJS.AES.encrypt(TEST_DATA, process.env.PRIVATE_KEY).toString())); t.false(Encryptor.isEncrypted(TEST_DATA)); + t.false(Encryptor.isEncrypted('JBSWY3DPEHPK3PXP'), 'a base32 OTP seed is clear text'); }); diff --git a/backend/test/ava-tests/saas-tests/saas-user-login-e2e.test.ts b/backend/test/ava-tests/saas-tests/saas-user-login-e2e.test.ts index 145b3770f..1be661dad 100644 --- a/backend/test/ava-tests/saas-tests/saas-user-login-e2e.test.ts +++ b/backend/test/ava-tests/saas-tests/saas-user-login-e2e.test.ts @@ -4,6 +4,7 @@ import { Test } from '@nestjs/testing'; import test from 'ava'; import { ValidationError } from 'class-validator'; import cookieParser from 'cookie-parser'; +import CryptoJS from 'crypto-js'; import jwt from 'jsonwebtoken'; import request from 'supertest'; import { DataSource } from 'typeorm'; @@ -216,3 +217,26 @@ test.serial(`${currentTest} stores the OTP seed encrypted before enrollment comp const reloaded = await userRepository.findOneByOrFail({ id: userId }); t.is(reloaded.otpSecretKey, seed, 'the entity hook decrypts the seed on load'); }); + +test.serial( + `${currentTest} decrypts a legacy V1 OTP seed on load and upgrades it to V2 on the next save`, + async (t) => { + const { userId } = await createCoreUser({ isOTPEnabled: true }); + const dataSource = app.get(BaseType.DATA_SOURCE); + const userRepository = dataSource.getRepository(UserEntity); + const seed = 'JBSWY3DPEHPK3PXP'; + // The row of an enrolled user from before the V2 scheme: CryptoJS ciphertext at rest. + const legacy = CryptoJS.AES.encrypt(seed, appConfig.auth.privateKey as string).toString(); + await dataSource.query('UPDATE "user" SET "otpSecretKey" = $1 WHERE id = $2', [legacy, userId]); + + const loaded = await userRepository.findOneByOrFail({ id: userId }); + t.is(loaded.otpSecretKey, seed, 'the load hook decrypts the V1 seed'); + + loaded.name = 'touched'; + await userRepository.save(loaded); + const [raw] = await dataSource.query('SELECT "otpSecretKey" FROM "user" WHERE id = $1', [userId]); + t.true(String(raw.otpSecretKey).startsWith('$v2:'), 'the save re-encrypts the seed as V2, not twice'); + const reloaded = await userRepository.findOneByOrFail({ id: userId }); + t.is(reloaded.otpSecretKey, seed); + }, +); From ad5573cc8ec73014ba4bfffcf1193a181b3905f6 Mon Sep 17 00:00:00 2001 From: Artem Niehrieiev Date: Thu, 5 Nov 2026 10:07:50 +0000 Subject: [PATCH 2/3] feat: Refactor encryption tests to simulate missing private key scenario --- backend/test/ava-tests/encryptor.test.ts | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/backend/test/ava-tests/encryptor.test.ts b/backend/test/ava-tests/encryptor.test.ts index 1210c8bd8..14dc0fed1 100644 --- a/backend/test/ava-tests/encryptor.test.ts +++ b/backend/test/ava-tests/encryptor.test.ts @@ -4,7 +4,6 @@ import CryptoJS from 'crypto-js'; process.env.PRIVATE_KEY = 'test-private-key-12345'; const { Encryptor } = await import('../../src/helpers/encryption/encryptor.js'); -const awaitedAppConfig = await import('../../src/shared/config/app-config.js'); const TEST_DATA = 'Hello, World! This is sensitive data.'; const TEST_MASTER_PWD = 'my-secure-master-password-123'; @@ -201,15 +200,17 @@ test('hashUserPassword: should return empty string for empty password', async (t }); test('encryptData: throws instead of returning the plaintext when the private key is missing', (t) => { - const { appConfig } = awaitedAppConfig; - const savedKey = appConfig.auth.privateKey; - appConfig.auth.privateKey = null; + // appConfig.auth is frozen, so the missing key is simulated on the accessor, not the config. + const originalGetPrivateKey = Encryptor.getPrivateKey; + Encryptor.getPrivateKey = () => { + throw new Error('Encryption private key is not configured.'); + }; try { const error = t.throws(() => Encryptor.encryptData(TEST_DATA)); t.true(error.message.includes('Data encryption failed')); t.false(error.message.includes(TEST_DATA)); } finally { - appConfig.auth.privateKey = savedKey; + Encryptor.getPrivateKey = originalGetPrivateKey; } }); From 5e40b9cb7f4c56b37fcb74de2fb0336ffaa41db4 Mon Sep 17 00:00:00 2001 From: Artem Niehrieiev Date: Thu, 5 Nov 2026 10:39:55 +0000 Subject: [PATCH 3/3] feat: Update OTP seed handling to preserve legacy V1 ciphertext on unrelated saves --- backend/src/entities/user/user.entity.ts | 12 ++++++++- .../saas-tests/saas-user-login-e2e.test.ts | 27 +++++++++++++++++-- 2 files changed, 36 insertions(+), 3 deletions(-) diff --git a/backend/src/entities/user/user.entity.ts b/backend/src/entities/user/user.entity.ts index ff0de8cc2..fab7b8475 100644 --- a/backend/src/entities/user/user.entity.ts +++ b/backend/src/entities/user/user.entity.ts @@ -55,13 +55,21 @@ export class UserEntity { this.emailToLowerCase(); } + // Not columns. The seed ciphertext as loaded and its decrypted form, so that a save with an + // unchanged seed writes the stored ciphertext back as it is. V1 decryption is not + // authenticated: with a wrong key it returns garbage, and a re-encryption of that garbage + // would destroy a recoverable seed. Only a changed seed is encrypted anew (as V2). + private otpSecretKeyAtRest: string | null = null; + private otpSecretKeyLoaded: string | null = null; + // The hook encrypts the OTP seed whenever it is set, not only after enrollment. Before this rule // a user who never completed verify-otp kept a clear-text seed at rest (ADR 01 ยง5.1). @BeforeInsert() @BeforeUpdate() encryptOtpSecretKey() { if (this.otpSecretKey && !Encryptor.isEncrypted(this.otpSecretKey)) { - this.otpSecretKey = Encryptor.encryptData(this.otpSecretKey); + const unchanged = this.otpSecretKeyAtRest !== null && this.otpSecretKey === this.otpSecretKeyLoaded; + this.otpSecretKey = unchanged ? this.otpSecretKeyAtRest : Encryptor.encryptData(this.otpSecretKey); } this.emailToLowerCase(); } @@ -69,7 +77,9 @@ export class UserEntity { @AfterLoad() decryptOtpSecretKey() { if (this.otpSecretKey && Encryptor.isEncrypted(this.otpSecretKey)) { + this.otpSecretKeyAtRest = this.otpSecretKey; this.otpSecretKey = Encryptor.decryptData(this.otpSecretKey); + this.otpSecretKeyLoaded = this.otpSecretKey; } this.emailToLowerCase(); } diff --git a/backend/test/ava-tests/saas-tests/saas-user-login-e2e.test.ts b/backend/test/ava-tests/saas-tests/saas-user-login-e2e.test.ts index 1be661dad..a5e178efb 100644 --- a/backend/test/ava-tests/saas-tests/saas-user-login-e2e.test.ts +++ b/backend/test/ava-tests/saas-tests/saas-user-login-e2e.test.ts @@ -219,7 +219,7 @@ test.serial(`${currentTest} stores the OTP seed encrypted before enrollment comp }); test.serial( - `${currentTest} decrypts a legacy V1 OTP seed on load and upgrades it to V2 on the next save`, + `${currentTest} decrypts a legacy V1 OTP seed on load and keeps the stored ciphertext on an unrelated save`, async (t) => { const { userId } = await createCoreUser({ isOTPEnabled: true }); const dataSource = app.get(BaseType.DATA_SOURCE); @@ -235,8 +235,31 @@ test.serial( loaded.name = 'touched'; await userRepository.save(loaded); const [raw] = await dataSource.query('SELECT "otpSecretKey" FROM "user" WHERE id = $1', [userId]); - t.true(String(raw.otpSecretKey).startsWith('$v2:'), 'the save re-encrypts the seed as V2, not twice'); + t.is(raw.otpSecretKey, legacy, 'an unchanged seed is written back as the stored V1 ciphertext'); const reloaded = await userRepository.findOneByOrFail({ id: userId }); t.is(reloaded.otpSecretKey, seed); + + // A changed seed is encrypted anew, as V2. + reloaded.otpSecretKey = 'KRSXG5CTMVRXEZLU'; + await userRepository.save(reloaded); + const [rawAfterChange] = await dataSource.query('SELECT "otpSecretKey" FROM "user" WHERE id = $1', [userId]); + t.true(String(rawAfterChange.otpSecretKey).startsWith('$v2:')); + t.is((await userRepository.findOneByOrFail({ id: userId })).otpSecretKey, 'KRSXG5CTMVRXEZLU'); }, ); + +test.serial(`${currentTest} does not re-encrypt a V2 seed with a new salt on an unrelated save`, async (t) => { + const { userId } = await createCoreUser({ isOTPEnabled: true }); + const dataSource = app.get(BaseType.DATA_SOURCE); + const userRepository = dataSource.getRepository(UserEntity); + const user = await userRepository.findOneByOrFail({ id: userId }); + user.otpSecretKey = 'JBSWY3DPEHPK3PXP'; + await userRepository.save(user); + const [before] = await dataSource.query('SELECT "otpSecretKey" FROM "user" WHERE id = $1', [userId]); + + const loaded = await userRepository.findOneByOrFail({ id: userId }); + loaded.name = 'touched again'; + await userRepository.save(loaded); + const [after] = await dataSource.query('SELECT "otpSecretKey" FROM "user" WHERE id = $1', [userId]); + t.is(after.otpSecretKey, before.otpSecretKey); +});