Encrypt/decrypt app secret variables (#17394)
Closes https://github.com/twentyhq/core-team-issues/issues/1724 From PR https://github.com/twentyhq/twenty/pull/15283, followed same implementation - Introduce EnvironmentModule to provide type safety for env-only variables - Encrypt secret variables - When querying app secret variable, display up to first 5 characters (depending on secret length) then `******`
This commit is contained in:
+2
-2
@@ -3,12 +3,12 @@ import { ScheduleModule } from '@nestjs/schedule';
|
||||
import { TypeOrmModule } from '@nestjs/typeorm';
|
||||
|
||||
import { KeyValuePairEntity } from 'src/engine/core-modules/key-value-pair/key-value-pair.entity';
|
||||
import { SecretEncryptionModule } from 'src/engine/core-modules/secret-encryption/secret-encryption.module';
|
||||
import { ConfigCacheService } from 'src/engine/core-modules/twenty-config/cache/config-cache.service';
|
||||
import { ConfigVariables } from 'src/engine/core-modules/twenty-config/config-variables';
|
||||
import { CONFIG_VARIABLES_INSTANCE_TOKEN } from 'src/engine/core-modules/twenty-config/constants/config-variables-instance-tokens.constants';
|
||||
import { ConfigValueConverterService } from 'src/engine/core-modules/twenty-config/conversion/config-value-converter.service';
|
||||
import { DatabaseConfigDriver } from 'src/engine/core-modules/twenty-config/drivers/database-config.driver';
|
||||
import { EnvironmentConfigDriver } from 'src/engine/core-modules/twenty-config/drivers/environment-config.driver';
|
||||
import { ConfigStorageService } from 'src/engine/core-modules/twenty-config/storage/config-storage.service';
|
||||
|
||||
@Module({})
|
||||
@@ -19,13 +19,13 @@ export class DatabaseConfigModule {
|
||||
imports: [
|
||||
TypeOrmModule.forFeature([KeyValuePairEntity]),
|
||||
ScheduleModule.forRoot(),
|
||||
SecretEncryptionModule,
|
||||
],
|
||||
providers: [
|
||||
DatabaseConfigDriver,
|
||||
ConfigCacheService,
|
||||
ConfigStorageService,
|
||||
ConfigValueConverterService,
|
||||
EnvironmentConfigDriver,
|
||||
{
|
||||
provide: CONFIG_VARIABLES_INSTANCE_TOKEN,
|
||||
useValue: new ConfigVariables(),
|
||||
|
||||
+20
-92
@@ -3,11 +3,11 @@ import { getRepositoryToken } from '@nestjs/typeorm';
|
||||
|
||||
import { type DeleteResult, IsNull, type Repository } from 'typeorm';
|
||||
|
||||
import * as authUtils from 'src/engine/core-modules/auth/auth.util';
|
||||
import {
|
||||
KeyValuePairEntity,
|
||||
KeyValuePairType,
|
||||
} from 'src/engine/core-modules/key-value-pair/key-value-pair.entity';
|
||||
import { SecretEncryptionService } from 'src/engine/core-modules/secret-encryption/secret-encryption.service';
|
||||
import { ConfigVariables } from 'src/engine/core-modules/twenty-config/config-variables';
|
||||
import { ConfigValueConverterService } from 'src/engine/core-modules/twenty-config/conversion/config-value-converter.service';
|
||||
import { EnvironmentConfigDriver } from 'src/engine/core-modules/twenty-config/drivers/environment-config.driver';
|
||||
@@ -23,15 +23,15 @@ import { type WorkspaceEntity } from 'src/engine/core-modules/workspace/workspac
|
||||
import { TypedReflect } from 'src/utils/typed-reflect';
|
||||
|
||||
jest.mock('src/engine/core-modules/auth/auth.util', () => ({
|
||||
encryptText: jest.fn((text) => `encrypted:${text}`),
|
||||
decryptText: jest.fn((text) => text.replace('encrypted:', '')),
|
||||
encryptText: jest.fn((text) => `${text}`),
|
||||
decryptText: jest.fn((text) => text.replace('', '')),
|
||||
}));
|
||||
|
||||
describe('ConfigStorageService', () => {
|
||||
let service: ConfigStorageService;
|
||||
let keyValuePairRepository: Repository<KeyValuePairEntity>;
|
||||
let configValueConverter: ConfigValueConverterService;
|
||||
let environmentConfigDriver: EnvironmentConfigDriver;
|
||||
let secretEncryptionService: SecretEncryptionService;
|
||||
|
||||
const createMockKeyValuePair = (
|
||||
key: string,
|
||||
@@ -79,6 +79,13 @@ describe('ConfigStorageService', () => {
|
||||
delete: jest.fn(),
|
||||
},
|
||||
},
|
||||
{
|
||||
provide: SecretEncryptionService,
|
||||
useValue: {
|
||||
decrypt: jest.fn((value) => value),
|
||||
encrypt: jest.fn((value) => value),
|
||||
},
|
||||
},
|
||||
],
|
||||
}).compile();
|
||||
|
||||
@@ -89,8 +96,8 @@ describe('ConfigStorageService', () => {
|
||||
configValueConverter = module.get<ConfigValueConverterService>(
|
||||
ConfigValueConverterService,
|
||||
);
|
||||
environmentConfigDriver = module.get<EnvironmentConfigDriver>(
|
||||
EnvironmentConfigDriver,
|
||||
secretEncryptionService = module.get<SecretEncryptionService>(
|
||||
SecretEncryptionService,
|
||||
);
|
||||
|
||||
jest.clearAllMocks();
|
||||
@@ -167,7 +174,7 @@ describe('ConfigStorageService', () => {
|
||||
it('should decrypt sensitive string values', async () => {
|
||||
const key = 'SENSITIVE_CONFIG' as keyof ConfigVariables;
|
||||
const originalValue = 'sensitive-value';
|
||||
const encryptedValue = 'encrypted:sensitive-value';
|
||||
const encryptedValue = 'sensitive-value';
|
||||
|
||||
const mockRecord = createMockKeyValuePair(key as string, encryptedValue);
|
||||
|
||||
@@ -191,10 +198,8 @@ describe('ConfigStorageService', () => {
|
||||
const result = await service.get(key);
|
||||
|
||||
expect(result).toBe(originalValue);
|
||||
expect(environmentConfigDriver.get).toHaveBeenCalledWith('APP_SECRET');
|
||||
expect(authUtils.decryptText).toHaveBeenCalledWith(
|
||||
expect(secretEncryptionService.decrypt).toHaveBeenCalledWith(
|
||||
encryptedValue,
|
||||
'test-secret',
|
||||
);
|
||||
});
|
||||
|
||||
@@ -227,43 +232,6 @@ describe('ConfigStorageService', () => {
|
||||
expect(result).toBe(convertedValue);
|
||||
});
|
||||
|
||||
it('should handle decryption failure in get() by returning original value', async () => {
|
||||
const key = 'SENSITIVE_CONFIG' as keyof ConfigVariables;
|
||||
const encryptedValue = 'encrypted:sensitive-value';
|
||||
|
||||
const mockRecord = createMockKeyValuePair(key as string, encryptedValue);
|
||||
|
||||
jest
|
||||
.spyOn(keyValuePairRepository, 'findOne')
|
||||
.mockResolvedValue(mockRecord);
|
||||
|
||||
jest.spyOn(TypedReflect, 'getMetadata').mockReturnValue({
|
||||
[key]: {
|
||||
isSensitive: true,
|
||||
type: ConfigVariableType.STRING,
|
||||
group: ConfigVariablesGroup.SERVER_CONFIG,
|
||||
description: 'Test sensitive config',
|
||||
},
|
||||
});
|
||||
|
||||
(
|
||||
configValueConverter.convertDbValueToAppValue as jest.Mock
|
||||
).mockReturnValue(encryptedValue);
|
||||
|
||||
// Mock decryption to throw an error
|
||||
(authUtils.decryptText as jest.Mock).mockImplementationOnce(() => {
|
||||
throw new Error('Decryption failed');
|
||||
});
|
||||
|
||||
const result = await service.get(key);
|
||||
|
||||
expect(result).toBe(encryptedValue);
|
||||
expect(authUtils.decryptText).toHaveBeenCalledWith(
|
||||
encryptedValue,
|
||||
'test-secret',
|
||||
);
|
||||
});
|
||||
|
||||
it('should handle findOne errors', async () => {
|
||||
const key = 'AUTH_PASSWORD_ENABLED' as keyof ConfigVariables;
|
||||
const error = new Error('Database error');
|
||||
@@ -405,7 +373,7 @@ describe('ConfigStorageService', () => {
|
||||
const key = 'SENSITIVE_CONFIG' as keyof ConfigVariables;
|
||||
const value = 'sensitive-value';
|
||||
const convertedValue = 'sensitive-value';
|
||||
const encryptedValue = 'encrypted:sensitive-value';
|
||||
const encryptedValue = 'sensitive-value';
|
||||
|
||||
jest.spyOn(keyValuePairRepository, 'findOne').mockResolvedValue(null);
|
||||
|
||||
@@ -431,48 +399,10 @@ describe('ConfigStorageService', () => {
|
||||
workspaceId: null,
|
||||
type: KeyValuePairType.CONFIG_VARIABLE,
|
||||
});
|
||||
expect(environmentConfigDriver.get).toHaveBeenCalledWith('APP_SECRET');
|
||||
expect(authUtils.encryptText).toHaveBeenCalledWith(
|
||||
expect(secretEncryptionService.encrypt).toHaveBeenCalledWith(
|
||||
convertedValue,
|
||||
'test-secret',
|
||||
);
|
||||
});
|
||||
|
||||
it('should handle encryption failure in set() by using unconverted value', async () => {
|
||||
const key = 'SENSITIVE_CONFIG' as keyof ConfigVariables;
|
||||
const value = 'sensitive-value';
|
||||
const convertedValue = 'converted-value';
|
||||
|
||||
jest.spyOn(keyValuePairRepository, 'findOne').mockResolvedValue(null);
|
||||
|
||||
jest.spyOn(TypedReflect, 'getMetadata').mockReturnValue({
|
||||
[key]: {
|
||||
isSensitive: true,
|
||||
type: ConfigVariableType.STRING,
|
||||
group: ConfigVariablesGroup.SERVER_CONFIG,
|
||||
description: 'Test sensitive config',
|
||||
},
|
||||
});
|
||||
|
||||
(
|
||||
configValueConverter.convertAppValueToDbValue as jest.Mock
|
||||
).mockReturnValue(convertedValue);
|
||||
|
||||
// Mock encryption to throw an error
|
||||
(authUtils.encryptText as jest.Mock).mockImplementationOnce(() => {
|
||||
throw new Error('Encryption failed');
|
||||
});
|
||||
|
||||
await service.set(key, value);
|
||||
|
||||
expect(keyValuePairRepository.insert).toHaveBeenCalledWith({
|
||||
key: key as string,
|
||||
value: convertedValue, // Should fall back to unconverted value
|
||||
userId: null,
|
||||
workspaceId: null,
|
||||
type: KeyValuePairType.CONFIG_VARIABLE,
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('delete', () => {
|
||||
@@ -608,7 +538,7 @@ describe('ConfigStorageService', () => {
|
||||
|
||||
it('should decrypt sensitive string values in loadAll', async () => {
|
||||
const configVars: KeyValuePairEntity[] = [
|
||||
createMockKeyValuePair('SENSITIVE_CONFIG', 'encrypted:sensitive-value'),
|
||||
createMockKeyValuePair('SENSITIVE_CONFIG', 'sensitive-value'),
|
||||
createMockKeyValuePair('NORMAL_CONFIG', 'normal-value'),
|
||||
];
|
||||
|
||||
@@ -640,10 +570,8 @@ describe('ConfigStorageService', () => {
|
||||
expect(result.get('NORMAL_CONFIG' as keyof ConfigVariables)).toBe(
|
||||
'normal-value',
|
||||
);
|
||||
expect(environmentConfigDriver.get).toHaveBeenCalledWith('APP_SECRET');
|
||||
expect(authUtils.decryptText).toHaveBeenCalledWith(
|
||||
'encrypted:sensitive-value',
|
||||
'test-secret',
|
||||
expect(secretEncryptionService.decrypt).toHaveBeenCalledWith(
|
||||
'sensitive-value',
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
+6
-26
@@ -4,16 +4,12 @@ import { InjectRepository } from '@nestjs/typeorm';
|
||||
import { type FindOptionsWhere, IsNull, Repository } from 'typeorm';
|
||||
|
||||
import {
|
||||
decryptText,
|
||||
encryptText,
|
||||
} from 'src/engine/core-modules/auth/auth.util';
|
||||
import {
|
||||
KeyValuePairType,
|
||||
KeyValuePairEntity,
|
||||
KeyValuePairType,
|
||||
} from 'src/engine/core-modules/key-value-pair/key-value-pair.entity';
|
||||
import { SecretEncryptionService } from 'src/engine/core-modules/secret-encryption/secret-encryption.service';
|
||||
import { ConfigVariables } from 'src/engine/core-modules/twenty-config/config-variables';
|
||||
import { ConfigValueConverterService } from 'src/engine/core-modules/twenty-config/conversion/config-value-converter.service';
|
||||
import { EnvironmentConfigDriver } from 'src/engine/core-modules/twenty-config/drivers/environment-config.driver';
|
||||
import { ConfigVariableType } from 'src/engine/core-modules/twenty-config/enums/config-variable-type.enum';
|
||||
import {
|
||||
ConfigVariableException,
|
||||
@@ -31,7 +27,7 @@ export class ConfigStorageService implements ConfigStorageInterface {
|
||||
@InjectRepository(KeyValuePairEntity)
|
||||
private readonly keyValuePairRepository: Repository<KeyValuePairEntity>,
|
||||
private readonly configValueConverter: ConfigValueConverterService,
|
||||
private readonly environmentConfigDriver: EnvironmentConfigDriver,
|
||||
private readonly secretEncryptionService: SecretEncryptionService,
|
||||
) {}
|
||||
|
||||
private getConfigVariableWhereClause(
|
||||
@@ -45,10 +41,6 @@ export class ConfigStorageService implements ConfigStorageInterface {
|
||||
};
|
||||
}
|
||||
|
||||
private getAppSecret(): string {
|
||||
return this.environmentConfigDriver.get('APP_SECRET');
|
||||
}
|
||||
|
||||
private getConfigMetadata<T extends keyof ConfigVariables>(key: T) {
|
||||
return TypedReflect.getMetadata('config-variables', ConfigVariables)?.[
|
||||
key as string
|
||||
@@ -77,21 +69,9 @@ export class ConfigStorageService implements ConfigStorageInterface {
|
||||
return convertedValue;
|
||||
}
|
||||
|
||||
const appSecret = this.getAppSecret();
|
||||
|
||||
try {
|
||||
return isDecrypt
|
||||
? decryptText(convertedValue, appSecret)
|
||||
: encryptText(convertedValue, appSecret);
|
||||
} catch (error) {
|
||||
this.logger.debug(
|
||||
`${isDecrypt ? 'Decryption' : 'Encryption'} failed for key ${
|
||||
key as string
|
||||
}: ${error.message}. Using original value.`,
|
||||
);
|
||||
|
||||
return convertedValue;
|
||||
}
|
||||
return isDecrypt
|
||||
? this.secretEncryptionService.decrypt(convertedValue)
|
||||
: this.secretEncryptionService.encrypt(convertedValue);
|
||||
} catch (error) {
|
||||
throw new ConfigVariableException(
|
||||
`Failed to convert value for key ${key as string}: ${error.message}`,
|
||||
|
||||
+4
-16
@@ -1,13 +1,8 @@
|
||||
import { type DynamicModule, Global, Module } from '@nestjs/common';
|
||||
import { ConfigModule } from '@nestjs/config';
|
||||
|
||||
import {
|
||||
ConfigVariables,
|
||||
validate,
|
||||
} from 'src/engine/core-modules/twenty-config/config-variables';
|
||||
import { ConfigVariables } from 'src/engine/core-modules/twenty-config/config-variables';
|
||||
import { CONFIG_VARIABLES_INSTANCE_TOKEN } from 'src/engine/core-modules/twenty-config/constants/config-variables-instance-tokens.constants';
|
||||
import { DatabaseConfigModule } from 'src/engine/core-modules/twenty-config/drivers/database-config.module';
|
||||
import { EnvironmentConfigDriver } from 'src/engine/core-modules/twenty-config/drivers/environment-config.driver';
|
||||
import { ConfigurableModuleClass } from 'src/engine/core-modules/twenty-config/twenty-config.module-definition';
|
||||
import { TwentyConfigService } from 'src/engine/core-modules/twenty-config/twenty-config.service';
|
||||
|
||||
@@ -18,22 +13,15 @@ export class TwentyConfigModule extends ConfigurableModuleClass {
|
||||
const isConfigVariablesInDbEnabled =
|
||||
process.env.IS_CONFIG_VARIABLES_IN_DB_ENABLED !== 'false';
|
||||
|
||||
const imports = [
|
||||
ConfigModule.forRoot({
|
||||
isGlobal: true,
|
||||
expandVariables: true,
|
||||
validate,
|
||||
envFilePath: process.env.NODE_ENV === 'test' ? '.env.test' : '.env',
|
||||
}),
|
||||
...(isConfigVariablesInDbEnabled ? [DatabaseConfigModule.forRoot()] : []),
|
||||
];
|
||||
const imports = isConfigVariablesInDbEnabled
|
||||
? [DatabaseConfigModule.forRoot()]
|
||||
: [];
|
||||
|
||||
return {
|
||||
module: TwentyConfigModule,
|
||||
imports,
|
||||
providers: [
|
||||
TwentyConfigService,
|
||||
EnvironmentConfigDriver,
|
||||
{
|
||||
provide: CONFIG_VARIABLES_INSTANCE_TOKEN,
|
||||
useValue: new ConfigVariables(),
|
||||
|
||||
Reference in New Issue
Block a user