Backend improvement proposals 2026 11

backend/src/entities/user/user.entity.ts

@@ -55,21 +55,31 @@ 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();
 	}
 
 	@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();
 	}

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;

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,19 +200,23 @@ 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;
 	}
 });
 
-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');
 });

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,49 @@ 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 keeps the stored ciphertext on an unrelated save`,
+	async (t) => {
+		const { userId } = await createCoreUser({ isOTPEnabled: true });
+		const dataSource = app.get<DataSource>(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.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<DataSource>(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);
+});