diff --git a/packages/twenty-server/src/database/commands/upgrade-version-command/2-5/2-5-instance-command-slow-1798000009000-encrypt-totp-secrets.ts b/packages/twenty-server/src/database/commands/upgrade-version-command/2-5/2-5-instance-command-slow-1798000009000-encrypt-totp-secrets.ts new file mode 100644 index 0000000000..53a0b0c9e0 --- /dev/null +++ b/packages/twenty-server/src/database/commands/upgrade-version-command/2-5/2-5-instance-command-slow-1798000009000-encrypt-totp-secrets.ts @@ -0,0 +1,94 @@ +import { isDefined } from 'twenty-shared/utils'; +import { DataSource, QueryRunner } from 'typeorm'; + +import { SECRET_ENCRYPTION_ENVELOPE_V2_PREFIX } from 'src/engine/core-modules/secret-encryption/constants/secret-encryption.constant'; +import { SecretEncryptionService } from 'src/engine/core-modules/secret-encryption/secret-encryption.service'; +import { SimpleSecretEncryptionUtil } from 'src/engine/core-modules/two-factor-authentication/utils/simple-secret-encryption.util'; +import { RegisteredInstanceCommand } from 'src/engine/core-modules/upgrade/decorators/registered-instance-command.decorator'; +import { SlowInstanceCommand } from 'src/engine/core-modules/upgrade/interfaces/slow-instance-command.interface'; + +const BACKFILL_BATCH_SIZE = 500; + +const SECRET_CHECK_CONSTRAINT_NAME = + 'CHK_twoFactorAuthenticationMethod_secret_encrypted'; + +const V2_ENCRYPTED_LIKE_PATTERN = `${SECRET_ENCRYPTION_ENVELOPE_V2_PREFIX}%`; + +type TwoFactorMethodRow = { + id: string; + workspaceId: string; + userId: string; + secret: string; +}; + +@RegisteredInstanceCommand('2.5.0', 1798000009000, { type: 'slow' }) +export class EncryptTotpSecretsSlowInstanceCommand + implements SlowInstanceCommand +{ + constructor( + private readonly secretEncryptionService: SecretEncryptionService, + private readonly simpleSecretEncryptionUtil: SimpleSecretEncryptionUtil, + ) {} + + async runDataMigration(dataSource: DataSource): Promise { + let cursor = '00000000-0000-0000-0000-000000000000'; + + while (true) { + const rows: TwoFactorMethodRow[] = await dataSource.query( + `SELECT m.id, m."workspaceId", uw."userId", m."secret" + FROM "core"."twoFactorAuthenticationMethod" m + JOIN "core"."userWorkspace" uw + ON uw.id = m."userWorkspaceId" + WHERE m.id > $1 + AND m."secret" NOT LIKE $2 + ORDER BY m.id + LIMIT $3`, + [cursor, V2_ENCRYPTED_LIKE_PATTERN, BACKFILL_BATCH_SIZE], + ); + + if (rows.length === 0) { + break; + } + + for (const row of rows) { + const plaintext = await this.simpleSecretEncryptionUtil.decryptSecret( + row.secret, + `${row.userId}${row.workspaceId}otp-secret`, + ); + + if (!isDefined(plaintext)) { + continue; + } + + const encryptedValue = this.secretEncryptionService.encryptVersioned( + plaintext, + { workspaceId: row.workspaceId }, + ); + + await dataSource.query( + `UPDATE "core"."twoFactorAuthenticationMethod" + SET "secret" = $2 + WHERE id = $1`, + [row.id, encryptedValue], + ); + } + + cursor = rows[rows.length - 1].id; + } + } + + public async up(queryRunner: QueryRunner): Promise { + await queryRunner.query( + `ALTER TABLE "core"."twoFactorAuthenticationMethod" + ADD CONSTRAINT "${SECRET_CHECK_CONSTRAINT_NAME}" + CHECK ("secret" LIKE '${V2_ENCRYPTED_LIKE_PATTERN}')`, + ); + } + + public async down(queryRunner: QueryRunner): Promise { + await queryRunner.query( + `ALTER TABLE "core"."twoFactorAuthenticationMethod" + DROP CONSTRAINT IF EXISTS "${SECRET_CHECK_CONSTRAINT_NAME}"`, + ); + } +} diff --git a/packages/twenty-server/src/database/commands/upgrade-version-command/instance-command-provider.module.ts b/packages/twenty-server/src/database/commands/upgrade-version-command/instance-command-provider.module.ts index c3d4fd861b..a526a91130 100644 --- a/packages/twenty-server/src/database/commands/upgrade-version-command/instance-command-provider.module.ts +++ b/packages/twenty-server/src/database/commands/upgrade-version-command/instance-command-provider.module.ts @@ -1,11 +1,20 @@ import { Module } from '@nestjs/common'; import { INSTANCE_COMMANDS } from 'src/database/commands/upgrade-version-command/instance-commands.constant'; +import { JwtModule } from 'src/engine/core-modules/jwt/jwt.module'; import { SecretEncryptionModule } from 'src/engine/core-modules/secret-encryption/secret-encryption.module'; +import { SimpleSecretEncryptionUtil } from 'src/engine/core-modules/two-factor-authentication/utils/simple-secret-encryption.util'; import { ConnectedAccountTokenEncryptionModule } from 'src/engine/metadata-modules/connected-account/services/connected-account-token-encryption.module'; @Module({ - imports: [ConnectedAccountTokenEncryptionModule, SecretEncryptionModule], - providers: [...INSTANCE_COMMANDS], + imports: [ + ConnectedAccountTokenEncryptionModule, + SecretEncryptionModule, + // JwtModule is required by SimpleSecretEncryptionUtil. Drop both once the + // 2.5 cross-upgrade window closes and the encrypt-totp-secrets slow command + // is retired. + JwtModule, + ], + providers: [...INSTANCE_COMMANDS, SimpleSecretEncryptionUtil], }) export class InstanceCommandProviderModule {} diff --git a/packages/twenty-server/src/database/commands/upgrade-version-command/instance-commands.constant.ts b/packages/twenty-server/src/database/commands/upgrade-version-command/instance-commands.constant.ts index c762fd4d40..e52e98e7eb 100644 --- a/packages/twenty-server/src/database/commands/upgrade-version-command/instance-commands.constant.ts +++ b/packages/twenty-server/src/database/commands/upgrade-version-command/instance-commands.constant.ts @@ -38,6 +38,7 @@ import { EncryptApplicationVariableSlowInstanceCommand } from 'src/database/comm import { EncryptApplicationRegistrationVariableSlowInstanceCommand } from 'src/database/commands/upgrade-version-command/2-5/2-5-instance-command-slow-1798000006000-encrypt-application-registration-variable'; import { EncryptSigningKeyPrivateKeysSlowInstanceCommand } from 'src/database/commands/upgrade-version-command/2-5/2-5-instance-command-slow-1798000007000-encrypt-signing-key-private-keys'; import { EncryptSensitiveConfigStorageSlowInstanceCommand } from 'src/database/commands/upgrade-version-command/2-5/2-5-instance-command-slow-1798000008000-encrypt-sensitive-config-storage'; +import { EncryptTotpSecretsSlowInstanceCommand } from 'src/database/commands/upgrade-version-command/2-5/2-5-instance-command-slow-1798000009000-encrypt-totp-secrets'; import { AddSubFieldNameToViewSortFastInstanceCommand } from 'src/database/commands/upgrade-version-command/2-5/2-5-instance-command-fast-1778502963794-add-sub-field-name-to-view-sort'; import { DropPostgresCredentialsTableFastInstanceCommand } from 'src/database/commands/upgrade-version-command/2-5/2-5-instance-command-fast-1798500000000-drop-postgres-credentials-table'; @@ -80,6 +81,7 @@ export const INSTANCE_COMMANDS = [ EncryptApplicationRegistrationVariableSlowInstanceCommand, EncryptSigningKeyPrivateKeysSlowInstanceCommand, EncryptSensitiveConfigStorageSlowInstanceCommand, + EncryptTotpSecretsSlowInstanceCommand, AddSubFieldNameToViewSortFastInstanceCommand, DropPostgresCredentialsTableFastInstanceCommand, ]; diff --git a/packages/twenty-server/src/engine/core-modules/secret-encryption/secret-encryption.service.ts b/packages/twenty-server/src/engine/core-modules/secret-encryption/secret-encryption.service.ts index ff22a424a6..ca5555e995 100644 --- a/packages/twenty-server/src/engine/core-modules/secret-encryption/secret-encryption.service.ts +++ b/packages/twenty-server/src/engine/core-modules/secret-encryption/secret-encryption.service.ts @@ -21,7 +21,7 @@ type VersionedOptions = { @Injectable() export class SecretEncryptionService { private readonly logger = new Logger(SecretEncryptionService.name); - private hasLoggedLegacyDecryption = false; + private hasLoggedLegacyCtrDecryption = false; constructor( private readonly environmentConfigDriver: EnvironmentConfigDriver, @@ -139,19 +139,19 @@ export class SecretEncryptionService { }); } - this.warnLegacyDecryptionOnce(); + this.warnLegacyCtrDecryptionOnce(); return this.decrypt(value); } - private warnLegacyDecryptionOnce(): void { - if (this.hasLoggedLegacyDecryption) { + private warnLegacyCtrDecryptionOnce(): void { + if (this.hasLoggedLegacyCtrDecryption) { return; } - this.hasLoggedLegacyDecryption = true; + this.hasLoggedLegacyCtrDecryption = true; this.logger.warn( - 'Decrypted a legacy unprefixed ciphertext. These rows should be re-encrypted into the enc:v2 envelope in a follow-up migration.', + 'Decrypted a legacy unprefixed AES-CTR ciphertext. These rows should be re-encrypted into the enc:v2 envelope in a follow-up migration.', ); } } diff --git a/packages/twenty-server/src/engine/core-modules/two-factor-authentication/entities/two-factor-authentication-method.entity.ts b/packages/twenty-server/src/engine/core-modules/two-factor-authentication/entities/two-factor-authentication-method.entity.ts index f82c086690..cb5b35febe 100644 --- a/packages/twenty-server/src/engine/core-modules/two-factor-authentication/entities/two-factor-authentication-method.entity.ts +++ b/packages/twenty-server/src/engine/core-modules/two-factor-authentication/entities/two-factor-authentication-method.entity.ts @@ -1,5 +1,6 @@ import { TwoFactorAuthenticationStrategy } from 'twenty-shared/types'; import { + Check, Column, CreateDateColumn, Entity, @@ -17,6 +18,10 @@ import type { WorkspaceEntity } from 'src/engine/core-modules/workspace/workspac @Index(['userWorkspaceId', 'strategy'], { unique: true }) @Entity({ name: 'twoFactorAuthenticationMethod', schema: 'core' }) +@Check( + 'CHK_twoFactorAuthenticationMethod_secret_encrypted', + `"secret" LIKE 'enc:v2:%'`, +) export class TwoFactorAuthenticationMethodEntity { @PrimaryGeneratedColumn('uuid') id: string; diff --git a/packages/twenty-server/src/engine/core-modules/two-factor-authentication/two-factor-authentication.module.ts b/packages/twenty-server/src/engine/core-modules/two-factor-authentication/two-factor-authentication.module.ts index 020781ffb2..c66f5da7e5 100644 --- a/packages/twenty-server/src/engine/core-modules/two-factor-authentication/two-factor-authentication.module.ts +++ b/packages/twenty-server/src/engine/core-modules/two-factor-authentication/two-factor-authentication.module.ts @@ -5,6 +5,7 @@ import { TokenModule } from 'src/engine/core-modules/auth/token/token.module'; import { WorkspaceDomainsModule } from 'src/engine/core-modules/domain/workspace-domains/workspace-domains.module'; import { JwtModule } from 'src/engine/core-modules/jwt/jwt.module'; import { MetricsModule } from 'src/engine/core-modules/metrics/metrics.module'; +import { SecretEncryptionModule } from 'src/engine/core-modules/secret-encryption/secret-encryption.module'; import { UserWorkspaceEntity } from 'src/engine/core-modules/user-workspace/user-workspace.entity'; import { UserWorkspaceModule } from 'src/engine/core-modules/user-workspace/user-workspace.module'; import { UserEntity } from 'src/engine/core-modules/user/user.entity'; @@ -22,7 +23,10 @@ import { SimpleSecretEncryptionUtil } from './utils/simple-secret-encryption.uti WorkspaceDomainsModule, MetricsModule, TokenModule, + // JwtModule is required by the deprecated SimpleSecretEncryptionUtil; drop + // it together with the util once the 2.5 cross-upgrade window closes. JwtModule, + SecretEncryptionModule, TypeOrmModule.forFeature([ UserEntity, TwoFactorAuthenticationMethodEntity, diff --git a/packages/twenty-server/src/engine/core-modules/two-factor-authentication/two-factor-authentication.service.spec.ts b/packages/twenty-server/src/engine/core-modules/two-factor-authentication/two-factor-authentication.service.spec.ts index 5214ce42f0..b3deaac65c 100644 --- a/packages/twenty-server/src/engine/core-modules/two-factor-authentication/two-factor-authentication.service.spec.ts +++ b/packages/twenty-server/src/engine/core-modules/two-factor-authentication/two-factor-authentication.service.spec.ts @@ -7,7 +7,7 @@ import { AuthException, AuthExceptionCode, } from 'src/engine/core-modules/auth/auth.exception'; -import { SimpleSecretEncryptionUtil } from 'src/engine/core-modules/two-factor-authentication/utils/simple-secret-encryption.util'; +import { SecretEncryptionService } from 'src/engine/core-modules/secret-encryption/secret-encryption.service'; import { UserWorkspaceService } from 'src/engine/core-modules/user-workspace/user-workspace.service'; import { type WorkspaceEntity } from 'src/engine/core-modules/workspace/workspace.entity'; @@ -19,6 +19,9 @@ import { TwoFactorAuthenticationService } from './two-factor-authentication.serv import { TwoFactorAuthenticationMethodEntity } from './entities/two-factor-authentication-method.entity'; import { OTPStatus } from './strategies/otp/otp.constants'; +import { SimpleSecretEncryptionUtil } from './utils/simple-secret-encryption.util'; + +const V2_ENVELOPE_PREFIX = 'enc:v2:'; const totpStrategyMocks = { validate: jest.fn(), @@ -57,6 +60,7 @@ describe('TwoFactorAuthenticationService', () => { let service: TwoFactorAuthenticationService; let repository: any; let userWorkspaceService: any; + let secretEncryptionService: any; let simpleSecretEncryptionUtil: any; const mockUser = { id: 'user_123', email: 'test@example.com' }; @@ -67,7 +71,8 @@ describe('TwoFactorAuthenticationService', () => { }; const rawSecret = 'RAW_OTP_SECRET'; - const encryptedSecret = 'ENCRYPTED_SECRET_STRING'; + const encryptedSecret = `${V2_ENVELOPE_PREFIX}abcdef12:payload`; + const legacyCbcSecret = '0123456789abcdef0123456789abcdef:cafebabe'; beforeEach(async () => { const module: TestingModule = await Test.createTestingModule({ @@ -86,10 +91,16 @@ describe('TwoFactorAuthenticationService', () => { getUserWorkspaceForUserOrThrow: jest.fn(), }, }, + { + provide: SecretEncryptionService, + useValue: { + encryptVersioned: jest.fn(), + decryptVersioned: jest.fn(), + }, + }, { provide: SimpleSecretEncryptionUtil, useValue: { - encryptSecret: jest.fn(), decryptSecret: jest.fn(), }, }, @@ -104,6 +115,9 @@ describe('TwoFactorAuthenticationService', () => { ); userWorkspaceService = module.get(UserWorkspaceService); + secretEncryptionService = module.get( + SecretEncryptionService, + ); simpleSecretEncryptionUtil = module.get( SimpleSecretEncryptionUtil, ); @@ -171,9 +185,7 @@ describe('TwoFactorAuthenticationService', () => { it('should initiate configuration for a new user', async () => { repository.findOne.mockResolvedValue(null); - simpleSecretEncryptionUtil.encryptSecret.mockResolvedValue( - encryptedSecret, - ); + secretEncryptionService.encryptVersioned.mockReturnValue(encryptedSecret); const uri = await service.initiateStrategyConfiguration( mockUser.id, @@ -185,9 +197,9 @@ describe('TwoFactorAuthenticationService', () => { expect(uri).toBe( 'otpauth://totp/test@example.com?secret=RAW_OTP_SECRET&issuer=Twenty%20-%20Test%20Workspace', ); - expect(simpleSecretEncryptionUtil.encryptSecret).toHaveBeenCalledWith( + expect(secretEncryptionService.encryptVersioned).toHaveBeenCalledWith( rawSecret, - mockUser.id + workspace.id + 'otp-secret', + { workspaceId: workspace.id }, ); expect(repository.save).toHaveBeenCalledWith({ id: undefined, @@ -226,9 +238,7 @@ describe('TwoFactorAuthenticationService', () => { }; repository.findOne.mockResolvedValue(existingMethod); - simpleSecretEncryptionUtil.encryptSecret.mockResolvedValue( - encryptedSecret, - ); + secretEncryptionService.encryptVersioned.mockReturnValue(encryptedSecret); const uri = await service.initiateStrategyConfiguration( mockUser.id, @@ -283,9 +293,8 @@ describe('TwoFactorAuthenticationService', () => { }; repository.findOne.mockResolvedValue(existingMethod); - simpleSecretEncryptionUtil.decryptSecret.mockResolvedValue(rawSecret); + secretEncryptionService.decryptVersioned.mockReturnValue(rawSecret); - // Mock authenticator.keyuri to return a URI const expectedUri = 'otpauth://totp/test@example.com?secret=RAW_OTP_SECRET&issuer=Twenty%20-%20Test%20Workspace'; @@ -297,17 +306,48 @@ describe('TwoFactorAuthenticationService', () => { ); expect(uri).toBe(expectedUri); - expect(simpleSecretEncryptionUtil.decryptSecret).toHaveBeenCalledWith( + expect(secretEncryptionService.decryptVersioned).toHaveBeenCalledWith( encryptedSecret, - mockUser.id + workspace.id + 'otp-secret', + { workspaceId: workspace.id }, ); + expect(simpleSecretEncryptionUtil.decryptSecret).not.toHaveBeenCalled(); // Should not create new method or call initiate expect(totpStrategyMocks.initiate).not.toHaveBeenCalled(); expect(repository.save).not.toHaveBeenCalled(); }); + it('falls back to SimpleSecretEncryptionUtil when the stored secret is in the legacy AES-CBC format', async () => { + const recentTime = new Date(Date.now() - 5 * 60 * 1000); + const existingMethod = { + id: 'existing_method_id', + status: 'PENDING', + secret: legacyCbcSecret, + createdAt: recentTime, + }; + + repository.findOne.mockResolvedValue(existingMethod); + simpleSecretEncryptionUtil.decryptSecret.mockResolvedValue(rawSecret); + + const uri = await service.initiateStrategyConfiguration( + mockUser.id, + mockUser.email, + workspace.id, + workspace.displayName, + ); + + expect(uri).toBe( + 'otpauth://totp/test@example.com?secret=RAW_OTP_SECRET&issuer=Twenty%20-%20Test%20Workspace', + ); + expect(simpleSecretEncryptionUtil.decryptSecret).toHaveBeenCalledWith( + legacyCbcSecret, + `${mockUser.id}${workspace.id}otp-secret`, + ); + expect(secretEncryptionService.decryptVersioned).not.toHaveBeenCalled(); + expect(totpStrategyMocks.initiate).not.toHaveBeenCalled(); + expect(repository.save).not.toHaveBeenCalled(); + }); + it('should create new method when existing pending method is too old', async () => { - // Create a method that was created 2 hours ago (outside 1 hour window) const oldTime = new Date(Date.now() - 2 * 60 * 60 * 1000); const existingMethod = { id: 'existing_method_id', @@ -317,9 +357,7 @@ describe('TwoFactorAuthenticationService', () => { }; repository.findOne.mockResolvedValue(existingMethod); - simpleSecretEncryptionUtil.encryptSecret.mockResolvedValue( - encryptedSecret, - ); + secretEncryptionService.encryptVersioned.mockReturnValue(encryptedSecret); const uri = await service.initiateStrategyConfiguration( mockUser.id, @@ -346,23 +384,21 @@ describe('TwoFactorAuthenticationService', () => { }); it('should throw error when decryption of existing method fails', async () => { - // Create a recent method but decryption will fail const recentTime = new Date(Date.now() - 5 * 60 * 1000); const existingMethod = { id: 'existing_method_id', status: 'PENDING', - secret: 'corrupted_secret', + secret: `${V2_ENVELOPE_PREFIX}corrupted:payload`, createdAt: recentTime, }; repository.findOne.mockResolvedValue(existingMethod); const decryptionError = new Error('Decryption failed'); - simpleSecretEncryptionUtil.decryptSecret.mockRejectedValue( - decryptionError, - ); + secretEncryptionService.decryptVersioned.mockImplementation(() => { + throw decryptionError; + }); - // Should throw the decryption error instead of silently handling it await expect( service.initiateStrategyConfiguration( mockUser.id, @@ -372,7 +408,6 @@ describe('TwoFactorAuthenticationService', () => { ), ).rejects.toThrow(decryptionError); - // Should not save anything since we errored out expect(repository.save).not.toHaveBeenCalled(); }); @@ -381,13 +416,11 @@ describe('TwoFactorAuthenticationService', () => { id: 'existing_method_id', status: 'PENDING', secret: encryptedSecret, - createdAt: null, // No timestamp + createdAt: null, }; repository.findOne.mockResolvedValue(existingMethod); - simpleSecretEncryptionUtil.encryptSecret.mockResolvedValue( - encryptedSecret, - ); + secretEncryptionService.encryptVersioned.mockReturnValue(encryptedSecret); const uri = await service.initiateStrategyConfiguration( mockUser.id, @@ -426,7 +459,7 @@ describe('TwoFactorAuthenticationService', () => { it('should successfully validate a valid token', async () => { repository.findOne.mockResolvedValue(mock2FAMethod); - simpleSecretEncryptionUtil.decryptSecret.mockResolvedValue(rawSecret); + secretEncryptionService.decryptVersioned.mockReturnValue(rawSecret); totpStrategyMocks.validate.mockReturnValue({ isValid: true, @@ -440,6 +473,11 @@ describe('TwoFactorAuthenticationService', () => { TwoFactorAuthenticationStrategy.TOTP, ); + expect(secretEncryptionService.decryptVersioned).toHaveBeenCalledWith( + encryptedSecret, + { workspaceId: workspace.id }, + ); + expect(simpleSecretEncryptionUtil.decryptSecret).not.toHaveBeenCalled(); expect(totpStrategyMocks.validate).toHaveBeenCalledWith(otpToken, { status: mock2FAMethod.status, secret: rawSecret, @@ -452,9 +490,36 @@ describe('TwoFactorAuthenticationService', () => { ); }); + it('dispatches to SimpleSecretEncryptionUtil for legacy AES-CBC secrets', async () => { + const legacyMethod = { + ...mock2FAMethod, + secret: legacyCbcSecret, + }; + + repository.findOne.mockResolvedValue(legacyMethod); + simpleSecretEncryptionUtil.decryptSecret.mockResolvedValue(rawSecret); + totpStrategyMocks.validate.mockReturnValue({ + isValid: true, + context: { status: legacyMethod.status, secret: rawSecret }, + }); + + await service.validateStrategy( + mockUser.id, + otpToken, + workspace.id, + TwoFactorAuthenticationStrategy.TOTP, + ); + + expect(simpleSecretEncryptionUtil.decryptSecret).toHaveBeenCalledWith( + legacyCbcSecret, + `${mockUser.id}${workspace.id}otp-secret`, + ); + expect(secretEncryptionService.decryptVersioned).not.toHaveBeenCalled(); + }); + it('should throw if the token is invalid', async () => { repository.findOne.mockResolvedValue(mock2FAMethod); - simpleSecretEncryptionUtil.decryptSecret.mockResolvedValue(rawSecret); + secretEncryptionService.decryptVersioned.mockReturnValue(rawSecret); totpStrategyMocks.validate.mockReturnValue({ isValid: false, context: mock2FAMethod, @@ -517,9 +582,9 @@ describe('TwoFactorAuthenticationService', () => { it('should handle secret decryption errors', async () => { repository.findOne.mockResolvedValue(mock2FAMethod); - simpleSecretEncryptionUtil.decryptSecret.mockRejectedValue( - new Error('Secret decryption failed'), - ); + secretEncryptionService.decryptVersioned.mockImplementation(() => { + throw new Error('Secret decryption failed'); + }); await expect( service.validateStrategy( @@ -544,7 +609,7 @@ describe('TwoFactorAuthenticationService', () => { it('should successfully verify and return success', async () => { repository.findOne.mockResolvedValue(mock2FAMethod); - simpleSecretEncryptionUtil.decryptSecret.mockResolvedValue(rawSecret); + secretEncryptionService.decryptVersioned.mockReturnValue(rawSecret); totpStrategyMocks.validate.mockReturnValue({ isValid: true, @@ -573,7 +638,7 @@ describe('TwoFactorAuthenticationService', () => { it('should throw if the token is invalid', async () => { repository.findOne.mockResolvedValue(mock2FAMethod); - simpleSecretEncryptionUtil.decryptSecret.mockResolvedValue(rawSecret); + secretEncryptionService.decryptVersioned.mockReturnValue(rawSecret); totpStrategyMocks.validate.mockReturnValue({ isValid: false, context: mock2FAMethod, diff --git a/packages/twenty-server/src/engine/core-modules/two-factor-authentication/two-factor-authentication.service.ts b/packages/twenty-server/src/engine/core-modules/two-factor-authentication/two-factor-authentication.service.ts index 82c809da2c..c853fc0582 100644 --- a/packages/twenty-server/src/engine/core-modules/two-factor-authentication/two-factor-authentication.service.ts +++ b/packages/twenty-server/src/engine/core-modules/two-factor-authentication/two-factor-authentication.service.ts @@ -10,11 +10,12 @@ import { AuthException, AuthExceptionCode, } from 'src/engine/core-modules/auth/auth.exception'; +import { SECRET_ENCRYPTION_ENVELOPE_V2_PREFIX } from 'src/engine/core-modules/secret-encryption/constants/secret-encryption.constant'; +import { SecretEncryptionService } from 'src/engine/core-modules/secret-encryption/secret-encryption.service'; import { UserEntity } from 'src/engine/core-modules/user/user.entity'; import { TwoFactorAuthenticationMethodEntity } from 'src/engine/core-modules/two-factor-authentication/entities/two-factor-authentication-method.entity'; import { TOTP_DEFAULT_CONFIGURATION } from 'src/engine/core-modules/two-factor-authentication/strategies/otp/totp/constants/totp.strategy.constants'; import { TotpStrategy } from 'src/engine/core-modules/two-factor-authentication/strategies/otp/totp/totp.strategy'; -import { SimpleSecretEncryptionUtil } from 'src/engine/core-modules/two-factor-authentication/utils/simple-secret-encryption.util'; import { UserWorkspaceService } from 'src/engine/core-modules/user-workspace/user-workspace.service'; import { WorkspaceEntity } from 'src/engine/core-modules/workspace/workspace.entity'; @@ -25,9 +26,19 @@ import { import { twoFactorAuthenticationMethodsValidator } from './two-factor-authentication.validation'; import { OTPStatus } from './strategies/otp/otp.constants'; +import { SimpleSecretEncryptionUtil } from './utils/simple-secret-encryption.util'; const PENDING_METHOD_REUSE_WINDOW_MS = 60 * 60 * 1000; +// TODO: drop this helper, the `simpleSecretEncryptionUtil` dep, and the legacy +// branch in `decryptStoredSecret` below once the 2.5 cross-upgrade window +// closes and every TOTP secret row has been backfilled to enc:v2 by the +// matching slow instance command. +const buildLegacyTotpCbcPurpose = ( + userId: string, + workspaceId: string, +): string => `${userId}${workspaceId}otp-secret`; + @Injectable() // oxlint-disable-next-line twenty/inject-workspace-repository export class TwoFactorAuthenticationService { @@ -35,17 +46,29 @@ export class TwoFactorAuthenticationService { @InjectRepository(TwoFactorAuthenticationMethodEntity) private readonly twoFactorAuthenticationMethodRepository: Repository, private readonly userWorkspaceService: UserWorkspaceService, + private readonly secretEncryptionService: SecretEncryptionService, private readonly simpleSecretEncryptionUtil: SimpleSecretEncryptionUtil, ) {} - /** - * Generates encryption key for OTP secret based on user and workspace identifiers. - */ - private generateOtpSecretEncryptionKey( - userId: string, - workspaceId: string, - ): string { - return userId + workspaceId + 'otp-secret'; + private async decryptStoredSecret({ + storedSecret, + userId, + workspaceId, + }: { + storedSecret: string; + userId: string; + workspaceId: string; + }): Promise { + if (storedSecret.startsWith(SECRET_ENCRYPTION_ENVELOPE_V2_PREFIX)) { + return this.secretEncryptionService.decryptVersioned(storedSecret, { + workspaceId, + }); + } + + return this.simpleSecretEncryptionUtil.decryptSecret( + storedSecret, + buildLegacyTotpCbcPurpose(userId, workspaceId), + ); } /** @@ -114,11 +137,11 @@ export class TwoFactorAuthenticationService { Date.now() - existing2FAMethod.createdAt.getTime() < PENDING_METHOD_REUSE_WINDOW_MS ) { - const existingSecret = - await this.simpleSecretEncryptionUtil.decryptSecret( - existing2FAMethod.secret, - this.generateOtpSecretEncryptionKey(userId, workspaceId), - ); + const existingSecret = await this.decryptStoredSecret({ + storedSecret: existing2FAMethod.secret, + userId, + workspaceId, + }); const issuer = `Twenty${workspaceDisplayName ? ` - ${workspaceDisplayName}` : ''}`; const reuseUri = authenticator.keyuri(userEmail, issuer, existingSecret); @@ -133,9 +156,9 @@ export class TwoFactorAuthenticationService { `Twenty${workspaceDisplayName ? ` - ${workspaceDisplayName}` : ''}`, ); - const encryptedSecret = await this.simpleSecretEncryptionUtil.encryptSecret( + const encryptedSecret = this.secretEncryptionService.encryptVersioned( context.secret, - this.generateOtpSecretEncryptionKey(userId, workspaceId), + { workspaceId }, ); await this.twoFactorAuthenticationMethodRepository.save({ @@ -181,10 +204,11 @@ export class TwoFactorAuthenticationService { ); } - const originalSecret = await this.simpleSecretEncryptionUtil.decryptSecret( - userTwoFactorAuthenticationMethod.secret, - this.generateOtpSecretEncryptionKey(userId, workspaceId), - ); + const originalSecret = await this.decryptStoredSecret({ + storedSecret: userTwoFactorAuthenticationMethod.secret, + userId, + workspaceId, + }); const otpContext = { status: userTwoFactorAuthenticationMethod.status, diff --git a/packages/twenty-server/src/engine/core-modules/two-factor-authentication/utils/simple-secret-encryption.util.spec.ts b/packages/twenty-server/src/engine/core-modules/two-factor-authentication/utils/simple-secret-encryption.util.spec.ts index 9a025e3c68..383084fd13 100644 --- a/packages/twenty-server/src/engine/core-modules/two-factor-authentication/utils/simple-secret-encryption.util.spec.ts +++ b/packages/twenty-server/src/engine/core-modules/two-factor-authentication/utils/simple-secret-encryption.util.spec.ts @@ -1,10 +1,36 @@ import { Test, type TestingModule } from '@nestjs/testing'; +import { createCipheriv, createHash, randomBytes } from 'crypto'; + import { JwtTokenTypeEnum } from 'src/engine/core-modules/auth/types/auth-context.type'; import { JwtWrapperService } from 'src/engine/core-modules/jwt/services/jwt-wrapper.service'; import { SimpleSecretEncryptionUtil } from './simple-secret-encryption.util'; +// Mirrors the production write path the util used to perform; kept inside +// the spec so the deprecated decryptSecret can still be exercised end-to-end +// without shipping an encrypt method. +const encryptLegacySecret = ({ + plaintext, + appSecret, +}: { + plaintext: string; + appSecret: string; +}): string => { + const encryptionKey = createHash('sha256') + .update(appSecret) + .digest() + .slice(0, 32); + const iv = randomBytes(16); + const cipher = createCipheriv('aes-256-cbc', encryptionKey, iv); + const encrypted = Buffer.concat([ + cipher.update(plaintext, 'utf8'), + cipher.final(), + ]); + + return `${iv.toString('hex')}:${encrypted.toString('hex')}`; +}; + describe('SimpleSecretEncryptionUtil', () => { let util: SimpleSecretEncryptionUtil; let jwtWrapperService: any; @@ -22,10 +48,9 @@ describe('SimpleSecretEncryptionUtil', () => { useValue: { generateAppSecret: jest .fn() - .mockImplementation((_type, purpose) => { - // Return different secrets for different purposes to simulate real behavior - return `${mockAppSecret}-${purpose}`; - }), + .mockImplementation( + (_type, purpose) => `${mockAppSecret}-${purpose}`, + ), }, }, ], @@ -41,31 +66,27 @@ describe('SimpleSecretEncryptionUtil', () => { expect(util).toBeDefined(); }); - describe('encryptSecret and decryptSecret', () => { - it('should encrypt and decrypt a secret correctly', async () => { - const encrypted = await util.encryptSecret(testSecret, testPurpose); + describe('decryptSecret', () => { + it('decrypts a legacy ciphertext produced with the matching purpose', async () => { + const appSecret = `${mockAppSecret}-${testPurpose}`; + const encrypted = encryptLegacySecret({ + plaintext: testSecret, + appSecret, + }); + const decrypted = await util.decryptSecret(encrypted, testPurpose); expect(decrypted).toBe(testSecret); - expect(encrypted).not.toBe(testSecret); - expect(encrypted).toContain(':'); // Should contain IV separator }); - it('should generate different encrypted values for the same secret', async () => { - const encrypted1 = await util.encryptSecret(testSecret, testPurpose); - const encrypted2 = await util.encryptSecret(testSecret, testPurpose); + it('uses the KEY_ENCRYPTION_KEY JWT token type and the provided purpose', async () => { + const appSecret = `${mockAppSecret}-${testPurpose}`; + const encrypted = encryptLegacySecret({ + plaintext: testSecret, + appSecret, + }); - expect(encrypted1).not.toBe(encrypted2); // Different IVs should produce different results - - const decrypted1 = await util.decryptSecret(encrypted1, testPurpose); - const decrypted2 = await util.decryptSecret(encrypted2, testPurpose); - - expect(decrypted1).toBe(testSecret); - expect(decrypted2).toBe(testSecret); - }); - - it('should use the correct JWT token type and purpose', async () => { - await util.encryptSecret(testSecret, testPurpose); + await util.decryptSecret(encrypted, testPurpose); expect(jwtWrapperService.generateAppSecret).toHaveBeenCalledWith( JwtTokenTypeEnum.KEY_ENCRYPTION_KEY, @@ -73,43 +94,42 @@ describe('SimpleSecretEncryptionUtil', () => { ); }); - it('should handle special characters in secrets', async () => { + it('handles special characters in plaintext', async () => { const specialSecret = 'SECRET-WITH_SPECIAL@CHARS#123!'; + const appSecret = `${mockAppSecret}-${testPurpose}`; + const encrypted = encryptLegacySecret({ + plaintext: specialSecret, + appSecret, + }); - const encrypted = await util.encryptSecret(specialSecret, testPurpose); const decrypted = await util.decryptSecret(encrypted, testPurpose); expect(decrypted).toBe(specialSecret); }); - it('should not recover original secret with wrong purpose', async () => { - const encrypted = await util.encryptSecret(testSecret, testPurpose); + it('does not recover the plaintext when the purpose is wrong', async () => { + const appSecret = `${mockAppSecret}-${testPurpose}`; + const encrypted = encryptLegacySecret({ + plaintext: testSecret, + appSecret, + }); - // AES-256-CBC may either throw (invalid padding) or produce garbage. - // Both outcomes are acceptable — the key property is that the original - // secret is never returned. + // AES-256-CBC with a different key may either throw (invalid padding) + // or produce garbage. Both outcomes are acceptable - the key property is + // that the original secret is never returned. try { const decrypted = await util.decryptSecret(encrypted, 'wrong-purpose'); expect(decrypted).not.toBe(testSecret); } catch { - // Expected: wrong key produced invalid padding + // Expected: wrong key produced invalid padding. } }); - it('should fail to decrypt malformed encrypted data', async () => { + it('throws on malformed ciphertext', async () => { await expect( util.decryptSecret('invalid-encrypted-data', testPurpose), ).rejects.toThrow(); }); - - it('should handle empty secrets', async () => { - const emptySecret = ''; - - const encrypted = await util.encryptSecret(emptySecret, testPurpose); - const decrypted = await util.decryptSecret(encrypted, testPurpose); - - expect(decrypted).toBe(emptySecret); - }); }); }); diff --git a/packages/twenty-server/src/engine/core-modules/two-factor-authentication/utils/simple-secret-encryption.util.ts b/packages/twenty-server/src/engine/core-modules/two-factor-authentication/utils/simple-secret-encryption.util.ts index 2c8d0f4f9c..1d892681e3 100644 --- a/packages/twenty-server/src/engine/core-modules/two-factor-authentication/utils/simple-secret-encryption.util.ts +++ b/packages/twenty-server/src/engine/core-modules/two-factor-authentication/utils/simple-secret-encryption.util.ts @@ -1,53 +1,28 @@ import { Injectable } from '@nestjs/common'; -import { - createCipheriv, - createDecipheriv, - createHash, - randomBytes, -} from 'crypto'; +import { createDecipheriv, createHash } from 'crypto'; import { JwtTokenTypeEnum } from 'src/engine/core-modules/auth/types/auth-context.type'; import { JwtWrapperService } from 'src/engine/core-modules/jwt/services/jwt-wrapper.service'; +// TODO: delete this util once the 2.5 cross-upgrade window closes and every +// `core.twoFactorAuthenticationMethod.secret` row is known to be in the +// `enc:v2:` envelope. Also drop the call sites in TwoFactorAuthenticationService +// and the matching slow instance command, and stop providing this util in +// TwoFactorAuthenticationModule and InstanceCommandProviderModule. /** - * Simplified encryption utility for TOTP secrets. + * @deprecated Legacy TOTP secret decryption (AES-256-CBC keyed off + * `APP_SECRET + userId + workspaceId + 'otp-secret' + 'KEY_ENCRYPTION_KEY'`). + * Kept only to read pre-2.5 rows during the cross-upgrade window. New rows are + * written by `SecretEncryptionService.encryptVersioned` (enc:v2 envelope). */ @Injectable() export class SimpleSecretEncryptionUtil { private readonly algorithm = 'aes-256-cbc'; private readonly keyLength = 32; - private readonly ivLength = 16; constructor(private readonly jwtWrapperService: JwtWrapperService) {} - /** - * Encrypts a TOTP secret string - */ - async encryptSecret(secret: string, purpose: string): Promise { - const appSecret = this.jwtWrapperService.generateAppSecret( - JwtTokenTypeEnum.KEY_ENCRYPTION_KEY, - purpose, - ); - - const encryptionKey = createHash('sha256') - .update(appSecret) - .digest() - .slice(0, this.keyLength); - - const iv = randomBytes(this.ivLength); - - const cipher = createCipheriv(this.algorithm, encryptionKey, iv); - let encrypted = cipher.update(secret, 'utf8', 'hex'); - - encrypted += cipher.final('hex'); - - return iv.toString('hex') + ':' + encrypted; - } - - /** - * Decrypts a TOTP secret string - */ async decryptSecret( encryptedSecret: string, purpose: string, diff --git a/packages/twenty-server/test/integration/upgrade/suites/2-5-instance-command-slow-1798000009000-encrypt-totp-secrets.integration-spec.ts b/packages/twenty-server/test/integration/upgrade/suites/2-5-instance-command-slow-1798000009000-encrypt-totp-secrets.integration-spec.ts new file mode 100644 index 0000000000..00f3680457 --- /dev/null +++ b/packages/twenty-server/test/integration/upgrade/suites/2-5-instance-command-slow-1798000009000-encrypt-totp-secrets.integration-spec.ts @@ -0,0 +1,260 @@ +import { createCipheriv, createHash, randomBytes, randomUUID } from 'crypto'; + +import { config } from 'dotenv'; +import { isDefined } from 'twenty-shared/utils'; +import { DataSource } from 'typeorm'; + +import { buildSecretEncryptionServiceFromEnv } from 'test/integration/upgrade/utils/build-secret-encryption-service.util'; + +import { JwtTokenTypeEnum } from 'src/engine/core-modules/auth/types/auth-context.type'; +import { type JwtWrapperService } from 'src/engine/core-modules/jwt/services/jwt-wrapper.service'; +import { SECRET_ENCRYPTION_ENVELOPE_V2_PREFIX } from 'src/engine/core-modules/secret-encryption/constants/secret-encryption.constant'; +import { SecretEncryptionService } from 'src/engine/core-modules/secret-encryption/secret-encryption.service'; +import { SimpleSecretEncryptionUtil } from 'src/engine/core-modules/two-factor-authentication/utils/simple-secret-encryption.util'; + +import { EncryptTotpSecretsSlowInstanceCommand } from 'src/database/commands/upgrade-version-command/2-5/2-5-instance-command-slow-1798000009000-encrypt-totp-secrets'; + +jest.useRealTimers(); + +config({ + path: process.env.NODE_ENV === 'test' ? '.env.test' : '.env', + override: true, +}); + +const CHECK_CONSTRAINT_NAME = + 'CHK_twoFactorAuthenticationMethod_secret_encrypted'; +const CHECK_CONSTRAINT_EXPR = `"secret" LIKE '${SECRET_ENCRYPTION_ENVELOPE_V2_PREFIX}%'`; + +const buildLegacyAesCbcCiphertext = ({ + plaintext, + appSecret, + purpose, +}: { + plaintext: string; + appSecret: string; + purpose: string; +}): string => { + const appSecretHex = createHash('sha256') + .update(`${appSecret}${purpose}KEY_ENCRYPTION_KEY`) + .digest('hex'); + const key = createHash('sha256') + .update(appSecretHex) + .digest() + .subarray(0, 32); + const iv = randomBytes(16); + const cipher = createCipheriv('aes-256-cbc', key, iv); + const encrypted = Buffer.concat([ + cipher.update(plaintext, 'utf8'), + cipher.final(), + ]); + + return `${iv.toString('hex')}:${encrypted.toString('hex')}`; +}; + +const dropCheckConstraint = (dataSource: DataSource): Promise => + dataSource.query( + `ALTER TABLE "core"."twoFactorAuthenticationMethod" + DROP CONSTRAINT IF EXISTS "${CHECK_CONSTRAINT_NAME}"`, + ); + +const restoreCheckConstraint = async ( + dataSource: DataSource, +): Promise => { + await dropCheckConstraint(dataSource); + await dataSource.query( + `ALTER TABLE "core"."twoFactorAuthenticationMethod" + ADD CONSTRAINT "${CHECK_CONSTRAINT_NAME}" + CHECK (${CHECK_CONSTRAINT_EXPR})`, + ); +}; + +// Stand-in for the real JwtWrapperService used by SimpleSecretEncryptionUtil. +// Reproduces JwtWrapperService.generateAppSecret byte-for-byte so the legacy +// CBC key derivation matches what production rows were sealed with. +const buildJwtWrapperServiceStub = (appSecret: string): JwtWrapperService => { + return { + generateAppSecret: (type: JwtTokenTypeEnum, appSecretBody: string): string => + createHash('sha256') + .update(`${appSecret}${appSecretBody}${type}`) + .digest('hex'), + } as unknown as JwtWrapperService; +}; + +describe('2-5 slow instance command 1798000009000 - EncryptTotpSecretsSlowInstanceCommand (integration)', () => { + let dataSource: DataSource; + let secretEncryptionService: SecretEncryptionService; + let simpleSecretEncryptionUtil: SimpleSecretEncryptionUtil; + let command: EncryptTotpSecretsSlowInstanceCommand; + let appSecret: string; + let userId: string; + let workspaceId: string; + let userWorkspaceId: string; + const seededRowIds: string[] = []; + + const seedRow = async ({ secret }: { secret: string }): Promise => { + await dropCheckConstraint(dataSource); + + const id = randomUUID(); + + await dataSource.query( + `INSERT INTO "core"."twoFactorAuthenticationMethod" + (id, "workspaceId", "userWorkspaceId", "secret", "status", "strategy") + VALUES ($1, $2, $3, $4, 'VERIFIED', 'TOTP')`, + [id, workspaceId, userWorkspaceId, secret], + ); + + seededRowIds.push(id); + + return id; + }; + + beforeAll(async () => { + dataSource = new DataSource({ + type: 'postgres', + url: process.env.PG_DATABASE_URL, + schema: 'core', + entities: [], + synchronize: false, + }); + await dataSource.initialize(); + + if (!isDefined(process.env.APP_SECRET) || process.env.APP_SECRET === '') { + throw new Error( + 'APP_SECRET must be set in the integration test environment to build legacy CBC fixtures.', + ); + } + + appSecret = process.env.APP_SECRET; + secretEncryptionService = buildSecretEncryptionServiceFromEnv(); + simpleSecretEncryptionUtil = new SimpleSecretEncryptionUtil( + buildJwtWrapperServiceStub(appSecret), + ); + command = new EncryptTotpSecretsSlowInstanceCommand( + secretEncryptionService, + simpleSecretEncryptionUtil, + ); + + const [seedUserWorkspace] = await dataSource.query( + `SELECT id, "userId", "workspaceId" + FROM "core"."userWorkspace" + LIMIT 1`, + ); + + if (!isDefined(seedUserWorkspace)) { + throw new Error( + 'No seeded userWorkspace row found; run database:reset before the integration suite.', + ); + } + + userWorkspaceId = seedUserWorkspace.id as string; + userId = seedUserWorkspace.userId as string; + workspaceId = seedUserWorkspace.workspaceId as string; + }, 30000); + + afterEach(async () => { + if (seededRowIds.length > 0) { + await dataSource.query( + `DELETE FROM "core"."twoFactorAuthenticationMethod" WHERE id = ANY($1::uuid[])`, + [seededRowIds], + ); + seededRowIds.length = 0; + } + await restoreCheckConstraint(dataSource); + }); + + afterAll(async () => { + await dataSource?.destroy(); + }); + + it('upgrades legacy AES-CBC TOTP secrets to enc:v2 with workspaceId-bound HKDF', async () => { + const plaintext = 'KVKFKRCPNZQUYMLXOVYDSKLMNBVCXZ'; + const legacyCiphertext = buildLegacyAesCbcCiphertext({ + plaintext, + appSecret, + purpose: `${userId}${workspaceId}otp-secret`, + }); + const id = await seedRow({ secret: legacyCiphertext }); + + await command.runDataMigration(dataSource); + + const [row] = await dataSource.query( + `SELECT "secret" FROM "core"."twoFactorAuthenticationMethod" WHERE id = $1`, + [id], + ); + + expect(row.secret.startsWith(SECRET_ENCRYPTION_ENVELOPE_V2_PREFIX)).toBe( + true, + ); + expect( + secretEncryptionService.decryptVersioned(row.secret, { workspaceId }), + ).toBe(plaintext); + }); + + it('leaves enc:v2 rows untouched and is idempotent across re-runs', async () => { + const plaintext = 'already-v2-totp-secret'; + const preexistingV2 = secretEncryptionService.encryptVersioned(plaintext, { + workspaceId, + }); + const id = await seedRow({ secret: preexistingV2 }); + + await command.runDataMigration(dataSource); + const [afterFirstRun] = await dataSource.query( + `SELECT "secret" FROM "core"."twoFactorAuthenticationMethod" WHERE id = $1`, + [id], + ); + + expect(afterFirstRun.secret).toBe(preexistingV2); + + await command.runDataMigration(dataSource); + const [afterSecondRun] = await dataSource.query( + `SELECT "secret" FROM "core"."twoFactorAuthenticationMethod" WHERE id = $1`, + [id], + ); + + expect(afterSecondRun.secret).toBe(preexistingV2); + }); + + it('up() applies the CHECK constraint that rejects plaintext secret inserts', async () => { + await dropCheckConstraint(dataSource); + + const queryRunner = dataSource.createQueryRunner(); + + try { + await command.up(queryRunner); + + const id = randomUUID(); + + seededRowIds.push(id); + + await expect( + dataSource.query( + `INSERT INTO "core"."twoFactorAuthenticationMethod" + (id, "workspaceId", "userWorkspaceId", "secret", "status", "strategy") + VALUES ($1, $2, $3, 'plaintext-should-be-rejected', 'VERIFIED', 'TOTP')`, + [id, workspaceId, userWorkspaceId], + ), + ).rejects.toThrow(/check constraint/i); + } finally { + await queryRunner.release(); + } + }); + + it('down() removes the CHECK constraint and lets plaintext through (for rollback safety only)', async () => { + const queryRunner = dataSource.createQueryRunner(); + + try { + await command.down(queryRunner); + + const id = await seedRow({ secret: 'plaintext-allowed-after-down' }); + + const [row] = await dataSource.query( + `SELECT "secret" FROM "core"."twoFactorAuthenticationMethod" WHERE id = $1`, + [id], + ); + + expect(row.secret).toBe('plaintext-allowed-after-down'); + } finally { + await queryRunner.release(); + } + }); +});