Be/feature/ri 5072 docker encryption #2811
Conversation
|
|
||
| const ALGORITHM = 'aes-256-cbc'; | ||
| const HASH_ALGORITHM = 'sha256'; | ||
| const SERVER_CONFIG = config.get('server'); |
There was a problem hiding this comment.
If we cast this as type Config['server'] we'll get a bit of type safety. Specifically, that the value we passed to get is valid for the function and also that any config values we reference are valid.
| this.key = SERVER_CONFIG.encryptionKey; | ||
| } | ||
|
|
||
| /** |
There was a problem hiding this comment.
Comments are copy/paste error from keytar strategy? They don't match the contents of the functions. Also line 48-49
| export class KeyEncryptionStrategy implements IEncryptionStrategy { | ||
| private logger = new Logger('KeyEncryptionStrategy'); | ||
|
|
||
| private cipherKey; |
There was a problem hiding this comment.
Maybe add a typing here since we know this is going to be a Buffer?
| * Get password from storage and create cipher key | ||
| * Note: Will generate new password if it doesn't exists yet | ||
| */ | ||
| private async getCipherKey(): Promise<Buffer> { |
There was a problem hiding this comment.
Since this function is private and the isAvailable function a bit further down is not part of the interface, maybe we can remove async and remove the promise return types from both of them? There's no benefit to having these functions return a promise since they're not doing any async work.
| expect(await service.encrypt(mockDataToEncrypt)).toEqual(mockKeyEncryptResult); | ||
| expect(service['cipherKey']).not.toEqual(undefined); | ||
| }); | ||
| it('Should throw KeytarEncryptionError when unable to encrypt', async () => { |
There was a problem hiding this comment.
I don't think you meant KeytarEncryptionError here but instead KeyEncryptionError
| @@ -55,6 +55,7 @@ export default { | |||
| pluginsAssetsUri: '/static/resources/plugins', | |||
| base: process.env.RI_BASE || '/', | |||
| secretStoragePassword: process.env.SECRET_STORAGE_PASSWORD, | |||
| encryptionKey: process.env.RI_ENCRYPTION_KEY, | |||
There was a problem hiding this comment.
Is this a configuration value that we're planning on exposing to the user in the first phase? If so, we should go ahead and add this to the DOCKER_README.md while we're in here.
ArtemHoruzhenko
merged commit ea548d6
into
feature/RI-5036/optimize-docker-image
No description provided.