From 3c33834e324c3caab43806bd9f46bcf9bd92b40b Mon Sep 17 00:00:00 2001 From: Gosha Date: Wed, 3 Jan 2024 16:52:49 +0200 Subject: [PATCH 1/7] feat: add field-level encryption to api keys --- .../encrypt-api-keys-migration.spec.ts | 110 ++++++++++++++++++ .../encrypt-api-keys-migration.ts | 65 +++++++++++ .../encrypt-credentials-migration.spec.ts | 4 +- .../encrypt-credentials-migration.ts | 8 +- .../src/app/auth/e2e/user.auth.guard.e2e.ts | 2 +- .../dtos/environment-response.dto.ts | 13 ++- .../create-environment.usecase.ts | 9 +- .../generate-unique-api-key.usecase.ts | 16 ++- .../get-api-keys/get-api-keys.usecase.ts | 17 ++- .../get-environment.usecase.ts | 9 +- .../get-my-environments.usecase.ts | 26 ++++- .../regenerate-api-keys.usecase.ts | 6 +- .../update-profile-email.usecase.ts | 14 ++- .../environment/environment.entity.ts | 6 +- .../environment/environment.repository.ts | 21 +++- libs/shared/src/types/shared/index.ts | 4 + .../src/encryption/encrypt-provider.spec.ts | 10 +- .../src/encryption/encrypt-provider.ts | 46 +++++--- .../src/services/auth/auth.service.ts | 61 +++++++--- .../services/cache/key-builders/entities.ts | 3 +- 20 files changed, 379 insertions(+), 71 deletions(-) create mode 100644 apps/api/migrations/encrypt-api-keys/encrypt-api-keys-migration.spec.ts create mode 100644 apps/api/migrations/encrypt-api-keys/encrypt-api-keys-migration.ts diff --git a/apps/api/migrations/encrypt-api-keys/encrypt-api-keys-migration.spec.ts b/apps/api/migrations/encrypt-api-keys/encrypt-api-keys-migration.spec.ts new file mode 100644 index 00000000000..12acca05470 --- /dev/null +++ b/apps/api/migrations/encrypt-api-keys/encrypt-api-keys-migration.spec.ts @@ -0,0 +1,110 @@ +import { expect } from 'chai'; +import { faker } from '@faker-js/faker'; + +import { UserSession } from '@novu/testing'; +import { ChannelTypeEnum } from '@novu/stateless'; +import { EnvironmentRepository } from '@novu/dal'; + +import { encryptApiKeysMigration } from './encrypt-api-keys-migration'; + +describe('Encrypt Old api keys', function () { + let session: UserSession; + const environmentRepository = new EnvironmentRepository(); + + beforeEach(async () => { + session = new UserSession(); + await session.initialize(); + }); + + it('should decrypt all old api keys', async function () { + await pruneIntegration({ environmentRepository }); + + for (let i = 0; i < 2; i++) { + await environmentRepository.create({ + identifier: 'identifier' + i, + name: faker.name.jobTitle(), + _organizationId: session.organization._id, + apiKeys: [ + { + key: 'not-encrypted-secret-key', + _userId: session.user._id, + }, + ], + }); + } + + const newEnvironments = await environmentRepository.find({}); + + expect(newEnvironments.length).to.equal(2); + + for (const environment of newEnvironments) { + expect(environment.identifier).to.contains('identifier'); + expect(environment.name).to.exist; + expect(environment._organizationId).to.equal(session.organization._id); + expect(environment.apiKeys[0].key).to.equal('not-encrypted-secret-key'); + expect(environment.apiKeys[0]._userId).to.equal(session.user._id); + } + + await encryptApiKeysMigration(); + + const encryptEnvironments = await environmentRepository.find({}); + + for (const environment of encryptEnvironments) { + expect(environment.identifier).to.contains('identifier'); + expect(environment.name).to.exist; + expect(environment._organizationId).to.equal(session.organization._id); + expect(environment.apiKeys[0].key).to.contains('nvsk.'); + expect(environment.apiKeys[0]._userId).to.equal(session.user._id); + } + }); + + it('should validate migration idempotence', async function () { + await pruneIntegration({ environmentRepository }); + + const data = { + providerId: 'sendgrid', + channel: ChannelTypeEnum.EMAIL, + active: false, + }; + + for (let i = 0; i < 2; i++) { + await environmentRepository.create({ + identifier: 'identifier' + i, + name: faker.name.jobTitle(), + _organizationId: session.organization._id, + apiKeys: [ + { + key: 'not-encrypted-secret-key', + _userId: session.user._id, + }, + ], + }); + } + + await encryptApiKeysMigration(); + const firstMigrationExecution = await environmentRepository.find({}); + + await encryptApiKeysMigration(); + const secondMigrationExecution = await environmentRepository.find({}); + + expect(firstMigrationExecution[0].identifier).to.contains(secondMigrationExecution[0].identifier); + expect(firstMigrationExecution[0].name).to.exist; + expect(firstMigrationExecution[0]._organizationId).to.equal(secondMigrationExecution[0]._organizationId); + expect(firstMigrationExecution[0].apiKeys[0].key).to.contains(secondMigrationExecution[0].apiKeys[0].key); + expect(firstMigrationExecution[0].apiKeys[0]._userId).to.equal(secondMigrationExecution[0].apiKeys[0]._userId); + + expect(firstMigrationExecution[1].identifier).to.contains(secondMigrationExecution[1].identifier); + expect(firstMigrationExecution[1].name).to.exist; + expect(firstMigrationExecution[1]._organizationId).to.equal(secondMigrationExecution[1]._organizationId); + expect(firstMigrationExecution[1].apiKeys[0].key).to.contains(secondMigrationExecution[1].apiKeys[0].key); + expect(firstMigrationExecution[1].apiKeys[0]._userId).to.equal(secondMigrationExecution[1].apiKeys[0]._userId); + }); +}); + +async function pruneIntegration({ environmentRepository }: { environmentRepository: EnvironmentRepository }) { + const old = await environmentRepository.find({}); + + for (const oldKey of old) { + await environmentRepository.delete({ _id: oldKey._id }); + } +} diff --git a/apps/api/migrations/encrypt-api-keys/encrypt-api-keys-migration.ts b/apps/api/migrations/encrypt-api-keys/encrypt-api-keys-migration.ts new file mode 100644 index 00000000000..34cb764972c --- /dev/null +++ b/apps/api/migrations/encrypt-api-keys/encrypt-api-keys-migration.ts @@ -0,0 +1,65 @@ +import { EnvironmentRepository, IApiKey } from '@novu/dal'; +import { encryptSecret } from '@novu/application-generic'; +import { EncryptedSecret } from '@novu/shared'; + +export async function encryptApiKeysMigration() { + // eslint-disable-next-line no-console + console.log('start migration - encrypt api keys'); + + const environmentRepository = new EnvironmentRepository(); + const environments = await environmentRepository.find({}); + + for (const environment of environments) { + // eslint-disable-next-line no-console + console.log(`environment ${environment._id}`); + + if (!environment.apiKeys) { + // eslint-disable-next-line no-console + console.log(`environment ${environment._id} - is not contains api keys, skipping..`); + continue; + } + + if ( + environment.apiKeys.every((key) => { + isEncrypted(key.key); + }) + ) { + // eslint-disable-next-line no-console + console.log(`environment ${environment._id} - api keys are already encrypted, skipping..`); + continue; + } + + const updatePayload: IEncryptedApiKey[] = encryptApiKeysWithGuard(environment.apiKeys); + + await environmentRepository.update( + { _id: environment._id }, + { + $set: { apiKeys: updatePayload }, + } + ); + // eslint-disable-next-line no-console + console.log(`environment ${environment._id} - api keys updated`); + } + // eslint-disable-next-line no-console + console.log('end migration'); +} + +export function encryptApiKeysWithGuard(apiKeys: IApiKey[]): IEncryptedApiKey[] { + return apiKeys.map((apiKey: IApiKey) => { + const encryptedApiKey: IEncryptedApiKey = { + key: isEncrypted(apiKey.key) ? apiKey.key : encryptSecret(apiKey.key), + _userId: apiKey._userId, + }; + + return encryptedApiKey; + }); +} + +function isEncrypted(apiKey: string): apiKey is EncryptedSecret { + return apiKey.startsWith('nvsk.'); +} + +export interface IEncryptedApiKey { + key: EncryptedSecret; + _userId: string; +} diff --git a/apps/api/migrations/encrypt-credentials/encrypt-credentials-migration.spec.ts b/apps/api/migrations/encrypt-credentials/encrypt-credentials-migration.spec.ts index a38e6fe9c49..1db746189cb 100644 --- a/apps/api/migrations/encrypt-credentials/encrypt-credentials-migration.spec.ts +++ b/apps/api/migrations/encrypt-credentials/encrypt-credentials-migration.spec.ts @@ -51,13 +51,13 @@ describe('Encrypt Old Credentials', function () { }); } - const newIntegration = await integrationRepository.find({}); + const newIntegration = await integrationRepository.find({} as any); expect(newIntegration.length).to.equal(2); await encryptOldCredentialsMigration(); - const encryptIntegration = await integrationRepository.find({}); + const encryptIntegration = await integrationRepository.find({} as any); for (const integrationKey in encryptIntegration) { const integration = encryptIntegration[integrationKey]; diff --git a/apps/api/migrations/encrypt-credentials/encrypt-credentials-migration.ts b/apps/api/migrations/encrypt-credentials/encrypt-credentials-migration.ts index e85b4106124..eb2e18abd90 100644 --- a/apps/api/migrations/encrypt-credentials/encrypt-credentials-migration.ts +++ b/apps/api/migrations/encrypt-credentials/encrypt-credentials-migration.ts @@ -1,14 +1,14 @@ import { IntegrationEntity } from '@novu/dal'; -import { encryptProviderSecret } from '../../src/app/shared/services/encryption'; import { IntegrationRepository } from '@novu/dal'; import { ICredentialsDto, secureCredentials } from '@novu/shared'; +import { encryptSecret } from '@novu/application-generic'; export async function encryptOldCredentialsMigration() { // eslint-disable-next-line no-console console.log('start migration - encrypt credentials'); const integrationRepository = new IntegrationRepository(); - const integrations = await integrationRepository.find({}); + const integrations = await integrationRepository.find({} as any); for (const integration of integrations) { // eslint-disable-next-line no-console @@ -25,7 +25,7 @@ export async function encryptOldCredentialsMigration() { updatePayload.credentials = encryptCredentialsWithGuard(integration); await integrationRepository.update( - { _id: integration._id }, + { _id: integration._id, _environmentId: integration._environmentId }, { $set: updatePayload, } @@ -45,7 +45,7 @@ export function encryptCredentialsWithGuard(integration: IntegrationEntity): ICr const credential = credentials[key]; if (needEncryption(key, credential, integration)) { - encryptedCredentials[key] = encryptProviderSecret(credential); + encryptedCredentials[key] = encryptSecret(credential); } else { encryptedCredentials[key] = credential; } diff --git a/apps/api/src/app/auth/e2e/user.auth.guard.e2e.ts b/apps/api/src/app/auth/e2e/user.auth.guard.e2e.ts index 9d0cc3a1494..2c2a3e3413f 100644 --- a/apps/api/src/app/auth/e2e/user.auth.guard.e2e.ts +++ b/apps/api/src/app/auth/e2e/user.auth.guard.e2e.ts @@ -46,7 +46,7 @@ describe('UserAuthGuard', () => { it('should return 401 when ApiKey auth scheme is provided with an invalid value', async () => { const response = await request(`${ApiAuthSchemeEnum.API_KEY} invalid_key`); expect(response.statusCode).to.equal(401); - expect(response.body.message).to.equal('API Key not found'); + expect(response.body.message).to.equal('Environment not found'); }); it('should return 401 when ApiKey auth scheme is used for an externally inaccessible API route', async () => { diff --git a/apps/api/src/app/environments/dtos/environment-response.dto.ts b/apps/api/src/app/environments/dtos/environment-response.dto.ts index d59c97454f7..0435b8a0c35 100644 --- a/apps/api/src/app/environments/dtos/environment-response.dto.ts +++ b/apps/api/src/app/environments/dtos/environment-response.dto.ts @@ -1,6 +1,4 @@ import { ApiProperty, ApiPropertyOptional } from '@nestjs/swagger'; -import { IApiKey } from '@novu/dal'; -import { ApiKey } from '../../shared/dtos/api-key'; export class EnvironmentResponseDto { @ApiPropertyOptional() @@ -15,11 +13,14 @@ export class EnvironmentResponseDto { @ApiProperty() identifier: string; - @ApiProperty({ - type: [ApiKey], - }) - apiKeys: IApiKey[]; + @ApiPropertyOptional() + apiKeys?: IApiKeyDto[]; @ApiProperty() _parentId: string; } + +export interface IApiKeyDto { + key: string; + _userId: string; +} diff --git a/apps/api/src/app/environments/usecases/create-environment/create-environment.usecase.ts b/apps/api/src/app/environments/usecases/create-environment/create-environment.usecase.ts index 16ee9f735f7..444c5ec10c7 100644 --- a/apps/api/src/app/environments/usecases/create-environment/create-environment.usecase.ts +++ b/apps/api/src/app/environments/usecases/create-environment/create-environment.usecase.ts @@ -1,9 +1,10 @@ +import { nanoid } from 'nanoid'; import { Injectable } from '@nestjs/common'; + import { EnvironmentRepository } from '@novu/dal'; -import { nanoid } from 'nanoid'; +import { encryptApiKey } from '@novu/application-generic'; import { CreateEnvironmentCommand } from './create-environment.command'; - import { GenerateUniqueApiKey } from '../generate-unique-api-key/generate-unique-api-key.usecase'; // eslint-disable-next-line max-len import { CreateNotificationGroupCommand } from '../../../notification-groups/usecases/create-notification-group/create-notification-group.command'; @@ -20,7 +21,7 @@ export class CreateEnvironment { ) {} async execute(command: CreateEnvironmentCommand) { - const key = await this.generateUniqueApiKey.execute(); + const encryptedKey = encryptApiKey(await this.generateUniqueApiKey.execute()); const environment = await this.environmentRepository.create({ _organizationId: command.organizationId, @@ -29,7 +30,7 @@ export class CreateEnvironment { _parentId: command.parentEnvironmentId, apiKeys: [ { - key, + key: encryptedKey, _userId: command.userId, }, ], diff --git a/apps/api/src/app/environments/usecases/generate-unique-api-key/generate-unique-api-key.usecase.ts b/apps/api/src/app/environments/usecases/generate-unique-api-key/generate-unique-api-key.usecase.ts index 7c4886dafba..f8e9ebf35f3 100644 --- a/apps/api/src/app/environments/usecases/generate-unique-api-key/generate-unique-api-key.usecase.ts +++ b/apps/api/src/app/environments/usecases/generate-unique-api-key/generate-unique-api-key.usecase.ts @@ -1,6 +1,7 @@ import { Injectable, InternalServerErrorException } from '@nestjs/common'; import { EnvironmentRepository } from '@novu/dal'; import * as hat from 'hat'; +import { encryptApiKey } from '@novu/application-generic'; const API_KEY_GENERATION_MAX_RETRIES = 3; @@ -14,8 +15,7 @@ export class GenerateUniqueApiKey { let isApiKeyUsed = true; while (isApiKeyUsed) { apiKey = this.generateApiKey(); - const environment = await this.environmentRepository.findByApiKey(apiKey); - isApiKeyUsed = environment ? true : false; + isApiKeyUsed = await this.validateIsApiKeyUsed(apiKey); count += 1; if (count === API_KEY_GENERATION_MAX_RETRIES) { @@ -27,6 +27,18 @@ export class GenerateUniqueApiKey { return apiKey as string; } + private async validateIsApiKeyUsed(apiKey: string) { + const encryptedApiKey = encryptApiKey(apiKey); + + // backward compatibility - update to `findByApiKey` after encrypt-api-keys-migration run + const environment = await this.environmentRepository.findByApiKeyBackwardCompatibility({ + decryptedKey: apiKey, + encryptedKey: encryptedApiKey, + }); + + return !!environment; + } + /** * Extracting the generation functionality so it can be stubbed for functional testing * diff --git a/apps/api/src/app/environments/usecases/get-api-keys/get-api-keys.usecase.ts b/apps/api/src/app/environments/usecases/get-api-keys/get-api-keys.usecase.ts index 708e022745e..bf212dba5c2 100644 --- a/apps/api/src/app/environments/usecases/get-api-keys/get-api-keys.usecase.ts +++ b/apps/api/src/app/environments/usecases/get-api-keys/get-api-keys.usecase.ts @@ -1,12 +1,23 @@ import { Injectable } from '@nestjs/common'; -import { IApiKey, EnvironmentRepository } from '@novu/dal'; + +import { EnvironmentRepository, IApiKey } from '@novu/dal'; + import { GetApiKeysCommand } from './get-api-keys.command'; +import { ApiKey } from '../../../shared/dtos/api-key'; +import { decryptApiKey } from '@novu/application-generic'; @Injectable() export class GetApiKeys { constructor(private environmentRepository: EnvironmentRepository) {} - async execute(command: GetApiKeysCommand): Promise { - return await this.environmentRepository.getApiKeys(command.environmentId); + async execute(command: GetApiKeysCommand): Promise { + const keys = await this.environmentRepository.getApiKeys(command.environmentId); + + return keys.map((apiKey: IApiKey) => { + return { + key: decryptApiKey(apiKey.key), + _userId: apiKey._userId, + }; + }); } } diff --git a/apps/api/src/app/environments/usecases/get-environment/get-environment.usecase.ts b/apps/api/src/app/environments/usecases/get-environment/get-environment.usecase.ts index dc7e1244dd0..86383d86f82 100644 --- a/apps/api/src/app/environments/usecases/get-environment/get-environment.usecase.ts +++ b/apps/api/src/app/environments/usecases/get-environment/get-environment.usecase.ts @@ -1,13 +1,16 @@ import { Injectable, NotFoundException } from '@nestjs/common'; -import { EnvironmentRepository } from '@novu/dal'; + +import { EnvironmentEntity, EnvironmentRepository } from '@novu/dal'; + import { GetEnvironmentCommand } from './get-environment.command'; +import { EnvironmentResponseDto } from '../../dtos/environment-response.dto'; @Injectable() export class GetEnvironment { constructor(private environmentRepository: EnvironmentRepository) {} - async execute(command: GetEnvironmentCommand) { - const environment = await this.environmentRepository.findOne( + async execute(command: GetEnvironmentCommand): Promise { + const environment: Omit | null = await this.environmentRepository.findOne( { _id: command.environmentId, _organizationId: command.organizationId, diff --git a/apps/api/src/app/environments/usecases/get-my-environments/get-my-environments.usecase.ts b/apps/api/src/app/environments/usecases/get-my-environments/get-my-environments.usecase.ts index cffe3da5e2c..878c2772ef0 100644 --- a/apps/api/src/app/environments/usecases/get-my-environments/get-my-environments.usecase.ts +++ b/apps/api/src/app/environments/usecases/get-my-environments/get-my-environments.usecase.ts @@ -1,6 +1,10 @@ -import { Injectable, Logger, Scope } from '@nestjs/common'; +import { Injectable, Logger, NotFoundException, Scope } from '@nestjs/common'; + import { EnvironmentEntity, EnvironmentRepository } from '@novu/dal'; +import { decryptApiKey } from '@novu/application-generic'; + import { GetMyEnvironmentsCommand } from './get-my-environments.command'; +import { EnvironmentResponseDto } from '../../dtos/environment-response.dto'; @Injectable({ scope: Scope.REQUEST, @@ -8,14 +12,17 @@ import { GetMyEnvironmentsCommand } from './get-my-environments.command'; export class GetMyEnvironments { constructor(private environmentRepository: EnvironmentRepository) {} - async execute(command: GetMyEnvironmentsCommand): Promise { + async execute(command: GetMyEnvironmentsCommand): Promise { Logger.verbose('Getting Environments'); const environments = await this.environmentRepository.findOrganizationEnvironments(command.organizationId); + if (!environments?.length) + throw new NotFoundException(`Environments for organization ${command.organizationId} not found`); + return environments.map((environment) => { if (environment._id === command.environmentId) { - return environment; + return this.decryptApiKeys(environment); } environment.apiKeys = []; @@ -23,4 +30,17 @@ export class GetMyEnvironments { return environment; }); } + + private decryptApiKeys(environment: EnvironmentEntity): EnvironmentResponseDto { + const res = { ...environment }; + + res.apiKeys = environment.apiKeys.map((apiKey) => { + return { + _userId: apiKey._userId, + key: decryptApiKey(apiKey.key), + }; + }); + + return res; + } } diff --git a/apps/api/src/app/environments/usecases/regenerate-api-keys/regenerate-api-keys.usecase.ts b/apps/api/src/app/environments/usecases/regenerate-api-keys/regenerate-api-keys.usecase.ts index d7a59bb1ed8..71a9df1290d 100644 --- a/apps/api/src/app/environments/usecases/regenerate-api-keys/regenerate-api-keys.usecase.ts +++ b/apps/api/src/app/environments/usecases/regenerate-api-keys/regenerate-api-keys.usecase.ts @@ -1,5 +1,8 @@ import { Injectable } from '@nestjs/common'; + import { IApiKey, EnvironmentRepository } from '@novu/dal'; +import { encryptApiKey } from '@novu/application-generic'; + import { ApiException } from '../../../shared/exceptions/api.exception'; import { GenerateUniqueApiKey } from '../generate-unique-api-key/generate-unique-api-key.usecase'; import { GetApiKeysCommand } from '../get-api-keys/get-api-keys.command'; @@ -19,7 +22,8 @@ export class RegenerateApiKeys { } const key = await this.generateUniqueApiKey.execute(); + const encryptedApiKey = encryptApiKey(key); - return await this.environmentRepository.updateApiKey(command.environmentId, key, command.userId); + return await this.environmentRepository.updateApiKey(command.environmentId, encryptedApiKey, command.userId); } } diff --git a/apps/api/src/app/user/usecases/update-profile-email/update-profile-email.usecase.ts b/apps/api/src/app/user/usecases/update-profile-email/update-profile-email.usecase.ts index 53c516ae6ba..d354a70f224 100644 --- a/apps/api/src/app/user/usecases/update-profile-email/update-profile-email.usecase.ts +++ b/apps/api/src/app/user/usecases/update-profile-email/update-profile-email.usecase.ts @@ -1,7 +1,13 @@ import { BadRequestException, forwardRef, Inject, Injectable, NotFoundException } from '@nestjs/common'; import { UserRepository } from '@novu/dal'; -import { AnalyticsService, buildAuthServiceKey, buildUserKey, InvalidateCacheService } from '@novu/application-generic'; +import { + AnalyticsService, + buildAuthServiceKey, + buildUserKey, + encryptApiKey, + InvalidateCacheService, +} from '@novu/application-generic'; import { EnvironmentRepository } from '@novu/dal'; import { UpdateProfileEmailCommand } from './update-profile-email.command'; @@ -40,9 +46,13 @@ export class UpdateProfileEmail { }); const apiKeys = await this.environmentRepository.getApiKeys(command.environmentId); + + // backward compatibility - remove encryption once encrypt-api-keys-migration executed + const encryptedApiKey = encryptApiKey(apiKeys[0].key); + await this.invalidateCache.invalidateByKey({ key: buildAuthServiceKey({ - apiKey: apiKeys[0].key, + apiKey: encryptedApiKey, }), }); diff --git a/libs/dal/src/repositories/environment/environment.entity.ts b/libs/dal/src/repositories/environment/environment.entity.ts index 76e35281381..53d004ae6e9 100644 --- a/libs/dal/src/repositories/environment/environment.entity.ts +++ b/libs/dal/src/repositories/environment/environment.entity.ts @@ -1,11 +1,13 @@ import { Types } from 'mongoose'; +import { EncryptedSecret, IApiRateLimitMaximum } from '@novu/shared'; + import type { OrganizationId } from '../organization'; import type { ChangePropsValueType } from '../../types/helpers'; -import { IApiRateLimitMaximum } from '@novu/shared'; export interface IApiKey { - key: string; + // backward compatibility - remove `string` type after encrypt-api-keys-migration run + key: EncryptedSecret | string; _userId: string; } diff --git a/libs/dal/src/repositories/environment/environment.repository.ts b/libs/dal/src/repositories/environment/environment.repository.ts index c8c94f9e696..576e96742b3 100644 --- a/libs/dal/src/repositories/environment/environment.repository.ts +++ b/libs/dal/src/repositories/environment/environment.repository.ts @@ -1,4 +1,4 @@ -import { IApiRateLimitMaximum } from '@novu/shared'; +import { EncryptedSecret, IApiRateLimitMaximum } from '@novu/shared'; import { BaseRepository } from '../base-repository'; import { IApiKey, EnvironmentEntity, EnvironmentDBModel } from './environment.entity'; import { Environment } from './environment.schema'; @@ -35,7 +35,7 @@ export class EnvironmentRepository extends BaseRepository { const environment = await this.findOne( { @@ -69,7 +82,7 @@ export class EnvironmentRepository extends BaseRepository secureCred === key); } diff --git a/packages/application-generic/src/services/auth/auth.service.ts b/packages/application-generic/src/services/auth/auth.service.ts index 06c6a5b4145..2188a7a66e7 100644 --- a/packages/application-generic/src/services/auth/auth.service.ts +++ b/packages/application-generic/src/services/auth/auth.service.ts @@ -17,10 +17,10 @@ import { UserEntity, UserRepository, EnvironmentEntity, - IApiKey, } from '@novu/dal'; import { AuthProviderEnum, + EncryptedSecret, IJwtPayload, ISubscriberJwt, MemberRoleEnum, @@ -46,6 +46,7 @@ import { CachedEntity, } from '../cache'; import { normalizeEmail } from '../../utils/email-normalization'; +import { encryptApiKey } from '../../encryption'; @Injectable() export class AuthService { @@ -194,12 +195,12 @@ export class AuthService { @Instrument() public async validateApiKey(apiKey: string): Promise { - const { - environment, - user, - error, - key, // In the future, roles/scopes will be assigned to the API Key. - } = await this.getApiKeyUser({ apiKey }); + const encryptedApiKey = encryptApiKey(apiKey); + + const { environment, user, error } = await this.getApiKeyUser({ + decryptedKey: apiKey, + apiKey: encryptedApiKey, + }); if (error) throw new UnauthorizedException(error); @@ -379,23 +380,55 @@ export class AuthService { } @CachedEntity({ - builder: ({ apiKey }: { apiKey: string }) => + builder: ({ + decryptedKey, + apiKey, + }: { + decryptedKey: string; + apiKey: EncryptedSecret; + }) => buildAuthServiceKey({ apiKey: apiKey, }), }) - private async getApiKeyUser({ apiKey }: { apiKey: string }): Promise<{ + private async getApiKeyUser({ + decryptedKey, + apiKey, + }: { + decryptedKey: string; + apiKey: EncryptedSecret; + }): Promise<{ environment?: EnvironmentEntity; user?: UserEntity; - key?: IApiKey; error?: string; }> { - const environment = await this.environmentRepository.findByApiKey(apiKey); + /* + * backward compatibility - after encrypt-api-keys-migration execution: + * * update `findByApiKeyBackwardCompatibility` to `findByApiKey` + * * remove decryptedKey param from getApiKeyUser function input + * * remove decryptedKey param from CachedEntity decorator input + */ + const environment = + await this.environmentRepository.findByApiKeyBackwardCompatibility({ + decryptedKey, + encryptedKey: apiKey, + }); + if (!environment) { - return { error: 'API Key not found' }; + return { error: 'Environment not found' }; + } + + let key = environment.apiKeys.find((i) => i.key === apiKey); + + if (!key) { + /* + * backward compatibility - delete after encrypt-api-keys-migration execution + * find by decrypted key if key not found, because of backward compatibility + * use-case: findByApiKeyBackwardCompatibility found by decrypted key, so we need to validate by decrypted key + */ + key = environment.apiKeys.find((i) => i.key === decryptedKey); } - const key = environment.apiKeys.find((i) => i.key === apiKey); if (!key) { return { error: 'API Key not found' }; } @@ -405,6 +438,6 @@ export class AuthService { return { error: 'User not found' }; } - return { environment, user, key }; + return { environment, user }; } } diff --git a/packages/application-generic/src/services/cache/key-builders/entities.ts b/packages/application-generic/src/services/cache/key-builders/entities.ts index 0521d2cd2b3..87ed488e792 100644 --- a/packages/application-generic/src/services/cache/key-builders/entities.ts +++ b/packages/application-generic/src/services/cache/key-builders/entities.ts @@ -9,6 +9,7 @@ import { prefixWrapper, ServiceConfigIdentifierEnum, } from './shared'; +import { EncryptedSecret } from '@novu/shared'; const buildSubscriberKey = ({ subscriberId, @@ -80,7 +81,7 @@ const buildGroupedBlueprintsKey = (): string => identifier: BLUEPRINT_IDENTIFIER, }); -const buildAuthServiceKey = ({ apiKey }: { apiKey: string }): string => +const buildAuthServiceKey = ({ apiKey }: { apiKey: EncryptedSecret }): string => buildKeyById({ type: CacheKeyTypeEnum.ENTITY, keyEntity: CacheKeyPrefixEnum.AUTH_SERVICE, From 7d36a57aeeb2276bd83d3c7c12fd3b3bb601ae8f Mon Sep 17 00:00:00 2001 From: Gosha Date: Wed, 3 Jan 2024 17:38:16 +0200 Subject: [PATCH 2/7] fix: tests stub --- .../generate-unique-api-key/generate-unique-api-key.spec.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/apps/api/src/app/environments/usecases/generate-unique-api-key/generate-unique-api-key.spec.ts b/apps/api/src/app/environments/usecases/generate-unique-api-key/generate-unique-api-key.spec.ts index 71c2ac772e2..8e9bbee8db2 100644 --- a/apps/api/src/app/environments/usecases/generate-unique-api-key/generate-unique-api-key.spec.ts +++ b/apps/api/src/app/environments/usecases/generate-unique-api-key/generate-unique-api-key.spec.ts @@ -12,7 +12,8 @@ let generateApiKeyStub; let findByApiKeyStub; describe('Generate Unique Api Key', () => { beforeEach(() => { - findByApiKeyStub = sinon.stub(environmentRepository, 'findByApiKey'); + // backward compatibility - update `findByApiKeyBackwardCompatibility` to `findByApiKey` after encrypt-api-keys-migration execution + findByApiKeyStub = sinon.stub(environmentRepository, 'findByApiKeyBackwardCompatibility'); generateApiKeyStub = sinon.stub(generateUniqueApiKey, 'generateApiKey' as any); }); From e4ec7a29116bb112ec1f0c08b29c2bd7103bd90a Mon Sep 17 00:00:00 2001 From: Gosha Date: Wed, 3 Jan 2024 18:51:21 +0200 Subject: [PATCH 3/7] fix: tests mock --- packages/node/src/lib/changes/changes.spec.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/packages/node/src/lib/changes/changes.spec.ts b/packages/node/src/lib/changes/changes.spec.ts index b55975d317a..632d3bff81e 100644 --- a/packages/node/src/lib/changes/changes.spec.ts +++ b/packages/node/src/lib/changes/changes.spec.ts @@ -26,7 +26,13 @@ describe('test use of novus node package - Changes class', () => { await novu.changes.get({ page, limit, promoted }); expect(mockedAxios.get).toHaveBeenCalled(); - expect(mockedAxios.get).toHaveBeenCalledWith('/changes'); + expect(mockedAxios.get).toHaveBeenCalledWith('/changes', { + params: { + limit: 20, + page: 1, + promoted: false, + }, + }); }); test('should get count of changes correctly', async () => { From b229fb8d13cd93fc4a767945d3daa80f932fd6d7 Mon Sep 17 00:00:00 2001 From: Gosha Date: Sat, 6 Jan 2024 11:39:43 +0200 Subject: [PATCH 4/7] feat: update after pr comments --- .../app/environments/e2e/get-api-keys.e2e.ts | 23 +++++++++++++ .../e2e/regenerate-api-keys.e2e.ts | 3 ++ .../create-environment.e2e.ts | 12 +++++-- .../get-my-environments.e2e.ts | 3 ++ .../regenerate-api-keys.usecase.ts | 20 ++++++++--- .../update-profile-email.usecase.ts | 7 ++-- .../src/services/auth/auth.service.ts | 34 +++++-------------- .../services/cache/key-builders/entities.ts | 18 +++++++--- 8 files changed, 80 insertions(+), 40 deletions(-) create mode 100644 apps/api/src/app/environments/e2e/get-api-keys.e2e.ts diff --git a/apps/api/src/app/environments/e2e/get-api-keys.e2e.ts b/apps/api/src/app/environments/e2e/get-api-keys.e2e.ts new file mode 100644 index 00000000000..a7d40303866 --- /dev/null +++ b/apps/api/src/app/environments/e2e/get-api-keys.e2e.ts @@ -0,0 +1,23 @@ +import { UserSession } from '@novu/testing'; +import { expect } from 'chai'; +import { NOVU_SUB_MASK } from '@novu/shared'; + +describe('Get Environment API Keys - /environments/api-keys (GET)', async () => { + let session: UserSession; + before(async () => { + session = new UserSession(); + await session.initialize({}); + }); + + it('should get environment api keys correctly', async () => { + const demoEnvironment = { + name: 'Hello App', + }; + await session.testAgent.post('/v1/environments').send(demoEnvironment).expect(201); + + const { body } = await session.testAgent.get('/v1/environments/api-keys').send(); + + expect(body.data[0].key).to.not.contains(NOVU_SUB_MASK); + expect(body.data[0]._userId).to.equal(session.user._id); + }); +}); diff --git a/apps/api/src/app/environments/e2e/regenerate-api-keys.e2e.ts b/apps/api/src/app/environments/e2e/regenerate-api-keys.e2e.ts index 718d3d63bc9..10e59efef85 100644 --- a/apps/api/src/app/environments/e2e/regenerate-api-keys.e2e.ts +++ b/apps/api/src/app/environments/e2e/regenerate-api-keys.e2e.ts @@ -1,5 +1,6 @@ import { UserSession } from '@novu/testing'; import { expect } from 'chai'; +import { NOVU_SUB_MASK } from '@novu/shared'; describe('Environment - Regenerate Api Key', async () => { let session: UserSession; @@ -14,11 +15,13 @@ describe('Environment - Regenerate Api Key', async () => { body: { data: oldApiKeys }, } = await session.testAgent.get('/v1/environments/api-keys').send({}); const oldApiKey = oldApiKeys[0].key; + expect(oldApiKey).to.not.contains(NOVU_SUB_MASK); const { body: { data: newApiKeys }, } = await session.testAgent.post('/v1/environments/api-keys/regenerate').send({}); const newApiKey = newApiKeys[0].key; + expect(newApiKey).to.not.contains(NOVU_SUB_MASK); expect(oldApiKey).to.not.equal(newApiKey); diff --git a/apps/api/src/app/environments/usecases/create-environment/create-environment.e2e.ts b/apps/api/src/app/environments/usecases/create-environment/create-environment.e2e.ts index 59b349ae046..ac7a515a0f0 100644 --- a/apps/api/src/app/environments/usecases/create-environment/create-environment.e2e.ts +++ b/apps/api/src/app/environments/usecases/create-environment/create-environment.e2e.ts @@ -1,11 +1,12 @@ -import { EnvironmentRepository, LayoutRepository } from '@novu/dal'; -import { UserSession } from '@novu/testing'; import { expect } from 'chai'; +import { EnvironmentRepository } from '@novu/dal'; +import { UserSession } from '@novu/testing'; +import { NOVU_SUB_MASK } from '@novu/shared'; + describe('Create Environment - /environments (POST)', async () => { let session: UserSession; const environmentRepository = new EnvironmentRepository(); - const layoutRepository = new LayoutRepository(); before(async () => { session = new UserSession(); await session.initialize({ @@ -24,8 +25,13 @@ describe('Create Environment - /environments (POST)', async () => { expect(body.data.identifier).to.be.ok; const dbApp = await environmentRepository.findOne({ _id: body.data._id }); + if (!dbApp) { + throw new Error('App not found'); + } + expect(dbApp.apiKeys.length).to.equal(1); expect(dbApp.apiKeys[0].key).to.be.ok; + expect(dbApp.apiKeys[0].key).to.contains(NOVU_SUB_MASK); expect(dbApp.apiKeys[0]._userId).to.equal(session.user._id); }); diff --git a/apps/api/src/app/environments/usecases/get-my-environments/get-my-environments.e2e.ts b/apps/api/src/app/environments/usecases/get-my-environments/get-my-environments.e2e.ts index 644ec7a5397..f577b1cd556 100644 --- a/apps/api/src/app/environments/usecases/get-my-environments/get-my-environments.e2e.ts +++ b/apps/api/src/app/environments/usecases/get-my-environments/get-my-environments.e2e.ts @@ -1,5 +1,6 @@ import { UserSession } from '@novu/testing'; import { expect } from 'chai'; +import { NOVU_SUB_MASK } from '@novu/shared'; describe('Get My Environments - /environments (GET)', async () => { let session: UserSession; @@ -19,6 +20,8 @@ describe('Get My Environments - /environments (GET)', async () => { if (elem._id !== session.environment._id) { expect(elem.apiKeys.length).to.eq(0); } else { + expect(elem.apiKeys[0].key).to.not.contains(NOVU_SUB_MASK); + expect(elem.apiKeys[0]._userId).to.equal(session.user._id); expect(elem.apiKeys.length).to.be.greaterThanOrEqual(1); } } diff --git a/apps/api/src/app/environments/usecases/regenerate-api-keys/regenerate-api-keys.usecase.ts b/apps/api/src/app/environments/usecases/regenerate-api-keys/regenerate-api-keys.usecase.ts index 71a9df1290d..68f24160bd2 100644 --- a/apps/api/src/app/environments/usecases/regenerate-api-keys/regenerate-api-keys.usecase.ts +++ b/apps/api/src/app/environments/usecases/regenerate-api-keys/regenerate-api-keys.usecase.ts @@ -1,11 +1,12 @@ import { Injectable } from '@nestjs/common'; -import { IApiKey, EnvironmentRepository } from '@novu/dal'; -import { encryptApiKey } from '@novu/application-generic'; +import { EnvironmentRepository } from '@novu/dal'; +import { decryptApiKey, encryptApiKey } from '@novu/application-generic'; import { ApiException } from '../../../shared/exceptions/api.exception'; import { GenerateUniqueApiKey } from '../generate-unique-api-key/generate-unique-api-key.usecase'; import { GetApiKeysCommand } from '../get-api-keys/get-api-keys.command'; +import { IApiKeyDto } from '../../dtos/environment-response.dto'; @Injectable() export class RegenerateApiKeys { @@ -14,7 +15,7 @@ export class RegenerateApiKeys { private generateUniqueApiKey: GenerateUniqueApiKey ) {} - async execute(command: GetApiKeysCommand): Promise { + async execute(command: GetApiKeysCommand): Promise { const environment = await this.environmentRepository.findOne({ _id: command.environmentId }); if (!environment) { @@ -24,6 +25,17 @@ export class RegenerateApiKeys { const key = await this.generateUniqueApiKey.execute(); const encryptedApiKey = encryptApiKey(key); - return await this.environmentRepository.updateApiKey(command.environmentId, encryptedApiKey, command.userId); + const environments = await this.environmentRepository.updateApiKey( + command.environmentId, + encryptedApiKey, + command.userId + ); + + return environments.map((item) => { + return { + _userId: item._userId, + key: decryptApiKey(item.key), + }; + }); } } diff --git a/apps/api/src/app/user/usecases/update-profile-email/update-profile-email.usecase.ts b/apps/api/src/app/user/usecases/update-profile-email/update-profile-email.usecase.ts index d354a70f224..2219ad0ef9f 100644 --- a/apps/api/src/app/user/usecases/update-profile-email/update-profile-email.usecase.ts +++ b/apps/api/src/app/user/usecases/update-profile-email/update-profile-email.usecase.ts @@ -5,7 +5,7 @@ import { AnalyticsService, buildAuthServiceKey, buildUserKey, - encryptApiKey, + decryptApiKey, InvalidateCacheService, } from '@novu/application-generic'; import { EnvironmentRepository } from '@novu/dal'; @@ -47,12 +47,11 @@ export class UpdateProfileEmail { const apiKeys = await this.environmentRepository.getApiKeys(command.environmentId); - // backward compatibility - remove encryption once encrypt-api-keys-migration executed - const encryptedApiKey = encryptApiKey(apiKeys[0].key); + const decryptedApiKey = decryptApiKey(apiKeys[0].key); await this.invalidateCache.invalidateByKey({ key: buildAuthServiceKey({ - apiKey: encryptedApiKey, + apiKey: decryptedApiKey, }), }); diff --git a/packages/application-generic/src/services/auth/auth.service.ts b/packages/application-generic/src/services/auth/auth.service.ts index 2188a7a66e7..fcc4cfaa602 100644 --- a/packages/application-generic/src/services/auth/auth.service.ts +++ b/packages/application-generic/src/services/auth/auth.service.ts @@ -20,7 +20,6 @@ import { } from '@novu/dal'; import { AuthProviderEnum, - EncryptedSecret, IJwtPayload, ISubscriberJwt, MemberRoleEnum, @@ -195,11 +194,8 @@ export class AuthService { @Instrument() public async validateApiKey(apiKey: string): Promise { - const encryptedApiKey = encryptApiKey(apiKey); - const { environment, user, error } = await this.getApiKeyUser({ - decryptedKey: apiKey, - apiKey: encryptedApiKey, + apiKey, }); if (error) throw new UnauthorizedException(error); @@ -380,45 +376,33 @@ export class AuthService { } @CachedEntity({ - builder: ({ - decryptedKey, - apiKey, - }: { - decryptedKey: string; - apiKey: EncryptedSecret; - }) => + builder: ({ apiKey }: { apiKey: string }) => buildAuthServiceKey({ apiKey: apiKey, }), }) - private async getApiKeyUser({ - decryptedKey, - apiKey, - }: { - decryptedKey: string; - apiKey: EncryptedSecret; - }): Promise<{ + private async getApiKeyUser({ apiKey }: { apiKey: string }): Promise<{ environment?: EnvironmentEntity; user?: UserEntity; error?: string; }> { + const encryptedApiKey = encryptApiKey(apiKey); + /* * backward compatibility - after encrypt-api-keys-migration execution: * * update `findByApiKeyBackwardCompatibility` to `findByApiKey` - * * remove decryptedKey param from getApiKeyUser function input - * * remove decryptedKey param from CachedEntity decorator input */ const environment = await this.environmentRepository.findByApiKeyBackwardCompatibility({ - decryptedKey, - encryptedKey: apiKey, + decryptedKey: apiKey, + encryptedKey: encryptedApiKey, }); if (!environment) { return { error: 'Environment not found' }; } - let key = environment.apiKeys.find((i) => i.key === apiKey); + let key = environment.apiKeys.find((i) => i.key === encryptedApiKey); if (!key) { /* @@ -426,7 +410,7 @@ export class AuthService { * find by decrypted key if key not found, because of backward compatibility * use-case: findByApiKeyBackwardCompatibility found by decrypted key, so we need to validate by decrypted key */ - key = environment.apiKeys.find((i) => i.key === decryptedKey); + key = environment.apiKeys.find((i) => i.key === apiKey); } if (!key) { diff --git a/packages/application-generic/src/services/cache/key-builders/entities.ts b/packages/application-generic/src/services/cache/key-builders/entities.ts index 87ed488e792..af4c2a86007 100644 --- a/packages/application-generic/src/services/cache/key-builders/entities.ts +++ b/packages/application-generic/src/services/cache/key-builders/entities.ts @@ -9,7 +9,7 @@ import { prefixWrapper, ServiceConfigIdentifierEnum, } from './shared'; -import { EncryptedSecret } from '@novu/shared'; +import { createHash as createHashCrypto } from 'crypto'; const buildSubscriberKey = ({ subscriberId, @@ -81,13 +81,23 @@ const buildGroupedBlueprintsKey = (): string => identifier: BLUEPRINT_IDENTIFIER, }); -const buildAuthServiceKey = ({ apiKey }: { apiKey: EncryptedSecret }): string => - buildKeyById({ +const createHash = (apiKey: string): string => { + const hash = createHashCrypto('sha256'); + hash.update(apiKey); + + return hash.digest('hex'); +}; + +const buildAuthServiceKey = ({ apiKey }: { apiKey: string }): string => { + const apiKeyHash = createHash(apiKey); + + return buildKeyById({ type: CacheKeyTypeEnum.ENTITY, keyEntity: CacheKeyPrefixEnum.AUTH_SERVICE, - identifier: apiKey, + identifier: apiKeyHash, identifierPrefix: IdentifierPrefixEnum.API_KEY, }); +}; const buildMaximumApiRateLimitKey = ({ apiRateLimitCategory, From b481c366ce4d8ce64336e29fd6f9d5f055bddaa7 Mon Sep 17 00:00:00 2001 From: Gosha Date: Sat, 6 Jan 2024 13:36:55 +0200 Subject: [PATCH 5/7] fix: store and search by hash --- .../encrypt-api-keys-migration.spec.ts | 9 +++++++ .../encrypt-api-keys-migration.ts | 5 ++++ .../dtos/environment-response.dto.ts | 1 + .../create-environment.usecase.ts | 6 ++++- .../generate-unique-api-key.usecase.ts | 11 ++++---- .../regenerate-api-keys.usecase.ts | 5 +++- .../environment/environment.entity.ts | 7 +++++- .../environment/environment.repository.ts | 25 ++++++++----------- .../environment/environment.schema.ts | 1 + .../src/services/auth/auth.service.ts | 10 ++++---- 10 files changed, 53 insertions(+), 27 deletions(-) diff --git a/apps/api/migrations/encrypt-api-keys/encrypt-api-keys-migration.spec.ts b/apps/api/migrations/encrypt-api-keys/encrypt-api-keys-migration.spec.ts index 12acca05470..333a4e0de6c 100644 --- a/apps/api/migrations/encrypt-api-keys/encrypt-api-keys-migration.spec.ts +++ b/apps/api/migrations/encrypt-api-keys/encrypt-api-keys-migration.spec.ts @@ -1,9 +1,11 @@ import { expect } from 'chai'; import { faker } from '@faker-js/faker'; +import { createHash } from 'crypto'; import { UserSession } from '@novu/testing'; import { ChannelTypeEnum } from '@novu/stateless'; import { EnvironmentRepository } from '@novu/dal'; +import { decryptApiKey } from '@novu/application-generic'; import { encryptApiKeysMigration } from './encrypt-api-keys-migration'; @@ -42,6 +44,7 @@ describe('Encrypt Old api keys', function () { expect(environment.name).to.exist; expect(environment._organizationId).to.equal(session.organization._id); expect(environment.apiKeys[0].key).to.equal('not-encrypted-secret-key'); + expect(environment.apiKeys[0].hash).to.not.exist; expect(environment.apiKeys[0]._userId).to.equal(session.user._id); } @@ -50,10 +53,14 @@ describe('Encrypt Old api keys', function () { const encryptEnvironments = await environmentRepository.find({}); for (const environment of encryptEnvironments) { + const decryptedApiKey = decryptApiKey(environment.apiKeys[0].key); + const hashedApiKey = createHash('sha256').update(decryptedApiKey).digest('hex'); + expect(environment.identifier).to.contains('identifier'); expect(environment.name).to.exist; expect(environment._organizationId).to.equal(session.organization._id); expect(environment.apiKeys[0].key).to.contains('nvsk.'); + expect(environment.apiKeys[0].hash).to.equal(hashedApiKey); expect(environment.apiKeys[0]._userId).to.equal(session.user._id); } }); @@ -91,12 +98,14 @@ describe('Encrypt Old api keys', function () { expect(firstMigrationExecution[0].name).to.exist; expect(firstMigrationExecution[0]._organizationId).to.equal(secondMigrationExecution[0]._organizationId); expect(firstMigrationExecution[0].apiKeys[0].key).to.contains(secondMigrationExecution[0].apiKeys[0].key); + expect(firstMigrationExecution[0].apiKeys[0].hash).to.contains(secondMigrationExecution[0].apiKeys[0].hash); expect(firstMigrationExecution[0].apiKeys[0]._userId).to.equal(secondMigrationExecution[0].apiKeys[0]._userId); expect(firstMigrationExecution[1].identifier).to.contains(secondMigrationExecution[1].identifier); expect(firstMigrationExecution[1].name).to.exist; expect(firstMigrationExecution[1]._organizationId).to.equal(secondMigrationExecution[1]._organizationId); expect(firstMigrationExecution[1].apiKeys[0].key).to.contains(secondMigrationExecution[1].apiKeys[0].key); + expect(firstMigrationExecution[1].apiKeys[0].hash).to.contains(secondMigrationExecution[1].apiKeys[0].hash); expect(firstMigrationExecution[1].apiKeys[0]._userId).to.equal(secondMigrationExecution[1].apiKeys[0]._userId); }); }); diff --git a/apps/api/migrations/encrypt-api-keys/encrypt-api-keys-migration.ts b/apps/api/migrations/encrypt-api-keys/encrypt-api-keys-migration.ts index 34cb764972c..0efcf935df3 100644 --- a/apps/api/migrations/encrypt-api-keys/encrypt-api-keys-migration.ts +++ b/apps/api/migrations/encrypt-api-keys/encrypt-api-keys-migration.ts @@ -1,6 +1,7 @@ import { EnvironmentRepository, IApiKey } from '@novu/dal'; import { encryptSecret } from '@novu/application-generic'; import { EncryptedSecret } from '@novu/shared'; +import { createHash } from 'crypto'; export async function encryptApiKeysMigration() { // eslint-disable-next-line no-console @@ -46,7 +47,10 @@ export async function encryptApiKeysMigration() { export function encryptApiKeysWithGuard(apiKeys: IApiKey[]): IEncryptedApiKey[] { return apiKeys.map((apiKey: IApiKey) => { + const hashedApiKey = createHash('sha256').update(apiKey.key).digest('hex'); + const encryptedApiKey: IEncryptedApiKey = { + hash: apiKey?.hash ? apiKey?.hash : hashedApiKey, key: isEncrypted(apiKey.key) ? apiKey.key : encryptSecret(apiKey.key), _userId: apiKey._userId, }; @@ -62,4 +66,5 @@ function isEncrypted(apiKey: string): apiKey is EncryptedSecret { export interface IEncryptedApiKey { key: EncryptedSecret; _userId: string; + hash: string; } diff --git a/apps/api/src/app/environments/dtos/environment-response.dto.ts b/apps/api/src/app/environments/dtos/environment-response.dto.ts index 0435b8a0c35..4b25f5a3834 100644 --- a/apps/api/src/app/environments/dtos/environment-response.dto.ts +++ b/apps/api/src/app/environments/dtos/environment-response.dto.ts @@ -23,4 +23,5 @@ export class EnvironmentResponseDto { export interface IApiKeyDto { key: string; _userId: string; + hash?: string; } diff --git a/apps/api/src/app/environments/usecases/create-environment/create-environment.usecase.ts b/apps/api/src/app/environments/usecases/create-environment/create-environment.usecase.ts index 444c5ec10c7..7c873f88fa1 100644 --- a/apps/api/src/app/environments/usecases/create-environment/create-environment.usecase.ts +++ b/apps/api/src/app/environments/usecases/create-environment/create-environment.usecase.ts @@ -1,5 +1,6 @@ import { nanoid } from 'nanoid'; import { Injectable } from '@nestjs/common'; +import { createHash } from 'crypto'; import { EnvironmentRepository } from '@novu/dal'; import { encryptApiKey } from '@novu/application-generic'; @@ -21,7 +22,9 @@ export class CreateEnvironment { ) {} async execute(command: CreateEnvironmentCommand) { - const encryptedKey = encryptApiKey(await this.generateUniqueApiKey.execute()); + const key = await this.generateUniqueApiKey.execute(); + const encryptedKey = encryptApiKey(key); + const hashedApiKey = createHash('sha256').update(key).digest('hex'); const environment = await this.environmentRepository.create({ _organizationId: command.organizationId, @@ -32,6 +35,7 @@ export class CreateEnvironment { { key: encryptedKey, _userId: command.userId, + hash: hashedApiKey, }, ], }); diff --git a/apps/api/src/app/environments/usecases/generate-unique-api-key/generate-unique-api-key.usecase.ts b/apps/api/src/app/environments/usecases/generate-unique-api-key/generate-unique-api-key.usecase.ts index f8e9ebf35f3..aa31f3ca6b8 100644 --- a/apps/api/src/app/environments/usecases/generate-unique-api-key/generate-unique-api-key.usecase.ts +++ b/apps/api/src/app/environments/usecases/generate-unique-api-key/generate-unique-api-key.usecase.ts @@ -1,7 +1,8 @@ +import * as hat from 'hat'; +import { createHash } from 'crypto'; import { Injectable, InternalServerErrorException } from '@nestjs/common'; + import { EnvironmentRepository } from '@novu/dal'; -import * as hat from 'hat'; -import { encryptApiKey } from '@novu/application-generic'; const API_KEY_GENERATION_MAX_RETRIES = 3; @@ -28,12 +29,12 @@ export class GenerateUniqueApiKey { } private async validateIsApiKeyUsed(apiKey: string) { - const encryptedApiKey = encryptApiKey(apiKey); + const hashedApiKey = createHash('sha256').update(apiKey).digest('hex'); // backward compatibility - update to `findByApiKey` after encrypt-api-keys-migration run const environment = await this.environmentRepository.findByApiKeyBackwardCompatibility({ - decryptedKey: apiKey, - encryptedKey: encryptedApiKey, + key: apiKey, + hash: hashedApiKey, }); return !!environment; diff --git a/apps/api/src/app/environments/usecases/regenerate-api-keys/regenerate-api-keys.usecase.ts b/apps/api/src/app/environments/usecases/regenerate-api-keys/regenerate-api-keys.usecase.ts index 68f24160bd2..91013dd775f 100644 --- a/apps/api/src/app/environments/usecases/regenerate-api-keys/regenerate-api-keys.usecase.ts +++ b/apps/api/src/app/environments/usecases/regenerate-api-keys/regenerate-api-keys.usecase.ts @@ -1,3 +1,4 @@ +import { createHash } from 'crypto'; import { Injectable } from '@nestjs/common'; import { EnvironmentRepository } from '@novu/dal'; @@ -24,11 +25,13 @@ export class RegenerateApiKeys { const key = await this.generateUniqueApiKey.execute(); const encryptedApiKey = encryptApiKey(key); + const hashedApiKey = createHash('sha256').update(key).digest('hex'); const environments = await this.environmentRepository.updateApiKey( command.environmentId, encryptedApiKey, - command.userId + command.userId, + hashedApiKey ); return environments.map((item) => { diff --git a/libs/dal/src/repositories/environment/environment.entity.ts b/libs/dal/src/repositories/environment/environment.entity.ts index 53d004ae6e9..e575d49c90b 100644 --- a/libs/dal/src/repositories/environment/environment.entity.ts +++ b/libs/dal/src/repositories/environment/environment.entity.ts @@ -6,8 +6,13 @@ import type { OrganizationId } from '../organization'; import type { ChangePropsValueType } from '../../types/helpers'; export interface IApiKey { - // backward compatibility - remove `string` type after encrypt-api-keys-migration run + /* + * backward compatibility - + * remove `string` type after encrypt-api-keys-migration run + * remove the optional from hash + */ key: EncryptedSecret | string; + hash?: string; _userId: string; } diff --git a/libs/dal/src/repositories/environment/environment.repository.ts b/libs/dal/src/repositories/environment/environment.repository.ts index 576e96742b3..9837de75e0d 100644 --- a/libs/dal/src/repositories/environment/environment.repository.ts +++ b/libs/dal/src/repositories/environment/environment.repository.ts @@ -51,23 +51,19 @@ export class EnvironmentRepository extends BaseRepository { @@ -82,7 +78,7 @@ export class EnvironmentRepository extends BaseRepository( type: Schema.Types.String, unique: true, }, + hash: Schema.Types.String, _userId: { type: Schema.Types.ObjectId, ref: 'User', diff --git a/packages/application-generic/src/services/auth/auth.service.ts b/packages/application-generic/src/services/auth/auth.service.ts index fcc4cfaa602..65c23bd72ac 100644 --- a/packages/application-generic/src/services/auth/auth.service.ts +++ b/packages/application-generic/src/services/auth/auth.service.ts @@ -1,3 +1,4 @@ +import { createHash } from 'crypto'; import { forwardRef, Inject, @@ -45,7 +46,6 @@ import { CachedEntity, } from '../cache'; import { normalizeEmail } from '../../utils/email-normalization'; -import { encryptApiKey } from '../../encryption'; @Injectable() export class AuthService { @@ -386,7 +386,7 @@ export class AuthService { user?: UserEntity; error?: string; }> { - const encryptedApiKey = encryptApiKey(apiKey); + const hashedApiKey = createHash('sha256').update(apiKey).digest('hex'); /* * backward compatibility - after encrypt-api-keys-migration execution: @@ -394,15 +394,15 @@ export class AuthService { */ const environment = await this.environmentRepository.findByApiKeyBackwardCompatibility({ - decryptedKey: apiKey, - encryptedKey: encryptedApiKey, + key: apiKey, + hash: hashedApiKey, }); if (!environment) { return { error: 'Environment not found' }; } - let key = environment.apiKeys.find((i) => i.key === encryptedApiKey); + let key = environment.apiKeys.find((i) => i.hash === hashedApiKey); if (!key) { /* From 78b472cd1e85593e4f51a06a59151416eba57f3c Mon Sep 17 00:00:00 2001 From: Gosha Date: Sun, 21 Jan 2024 15:00:56 +0200 Subject: [PATCH 6/7] refactor: update after pr comments --- .../encrypt-api-keys-migration.spec.ts | 16 +++++++------- .../app/environments/e2e/get-api-keys.e2e.ts | 4 ++-- .../e2e/regenerate-api-keys.e2e.ts | 6 ++--- .../create-environment.e2e.ts | 5 +++-- .../create-environment.usecase.ts | 4 ++-- .../generate-unique-api-key.spec.ts | 3 +-- .../generate-unique-api-key.usecase.ts | 3 +-- .../get-my-environments.e2e.ts | 4 ++-- .../get-my-environments.usecase.ts | 8 +++---- .../environment/environment.repository.ts | 14 ++---------- libs/shared/src/types/shared/index.ts | 4 ++-- .../src/encryption/encrypt-provider.ts | 22 +++++++++---------- .../src/services/auth/auth.service.ts | 18 ++++++--------- 13 files changed, 48 insertions(+), 63 deletions(-) diff --git a/apps/api/migrations/encrypt-api-keys/encrypt-api-keys-migration.spec.ts b/apps/api/migrations/encrypt-api-keys/encrypt-api-keys-migration.spec.ts index 333a4e0de6c..9671f3fa5c5 100644 --- a/apps/api/migrations/encrypt-api-keys/encrypt-api-keys-migration.spec.ts +++ b/apps/api/migrations/encrypt-api-keys/encrypt-api-keys-migration.spec.ts @@ -9,6 +9,14 @@ import { decryptApiKey } from '@novu/application-generic'; import { encryptApiKeysMigration } from './encrypt-api-keys-migration'; +async function pruneIntegration({ environmentRepository }: { environmentRepository: EnvironmentRepository }) { + const old = await environmentRepository.find({}); + + for (const oldKey of old) { + await environmentRepository.delete({ _id: oldKey._id }); + } +} + describe('Encrypt Old api keys', function () { let session: UserSession; const environmentRepository = new EnvironmentRepository(); @@ -109,11 +117,3 @@ describe('Encrypt Old api keys', function () { expect(firstMigrationExecution[1].apiKeys[0]._userId).to.equal(secondMigrationExecution[1].apiKeys[0]._userId); }); }); - -async function pruneIntegration({ environmentRepository }: { environmentRepository: EnvironmentRepository }) { - const old = await environmentRepository.find({}); - - for (const oldKey of old) { - await environmentRepository.delete({ _id: oldKey._id }); - } -} diff --git a/apps/api/src/app/environments/e2e/get-api-keys.e2e.ts b/apps/api/src/app/environments/e2e/get-api-keys.e2e.ts index a7d40303866..e6f68713983 100644 --- a/apps/api/src/app/environments/e2e/get-api-keys.e2e.ts +++ b/apps/api/src/app/environments/e2e/get-api-keys.e2e.ts @@ -1,6 +1,6 @@ import { UserSession } from '@novu/testing'; import { expect } from 'chai'; -import { NOVU_SUB_MASK } from '@novu/shared'; +import { NOVU_ENCRYPTION_SUB_MASK } from '@novu/shared'; describe('Get Environment API Keys - /environments/api-keys (GET)', async () => { let session: UserSession; @@ -17,7 +17,7 @@ describe('Get Environment API Keys - /environments/api-keys (GET)', async () => const { body } = await session.testAgent.get('/v1/environments/api-keys').send(); - expect(body.data[0].key).to.not.contains(NOVU_SUB_MASK); + expect(body.data[0].key).to.not.contains(NOVU_ENCRYPTION_SUB_MASK); expect(body.data[0]._userId).to.equal(session.user._id); }); }); diff --git a/apps/api/src/app/environments/e2e/regenerate-api-keys.e2e.ts b/apps/api/src/app/environments/e2e/regenerate-api-keys.e2e.ts index 10e59efef85..135b372a644 100644 --- a/apps/api/src/app/environments/e2e/regenerate-api-keys.e2e.ts +++ b/apps/api/src/app/environments/e2e/regenerate-api-keys.e2e.ts @@ -1,6 +1,6 @@ import { UserSession } from '@novu/testing'; import { expect } from 'chai'; -import { NOVU_SUB_MASK } from '@novu/shared'; +import { NOVU_ENCRYPTION_SUB_MASK } from '@novu/shared'; describe('Environment - Regenerate Api Key', async () => { let session: UserSession; @@ -15,13 +15,13 @@ describe('Environment - Regenerate Api Key', async () => { body: { data: oldApiKeys }, } = await session.testAgent.get('/v1/environments/api-keys').send({}); const oldApiKey = oldApiKeys[0].key; - expect(oldApiKey).to.not.contains(NOVU_SUB_MASK); + expect(oldApiKey).to.not.contains(NOVU_ENCRYPTION_SUB_MASK); const { body: { data: newApiKeys }, } = await session.testAgent.post('/v1/environments/api-keys/regenerate').send({}); const newApiKey = newApiKeys[0].key; - expect(newApiKey).to.not.contains(NOVU_SUB_MASK); + expect(newApiKey).to.not.contains(NOVU_ENCRYPTION_SUB_MASK); expect(oldApiKey).to.not.equal(newApiKey); diff --git a/apps/api/src/app/environments/usecases/create-environment/create-environment.e2e.ts b/apps/api/src/app/environments/usecases/create-environment/create-environment.e2e.ts index ac7a515a0f0..9ec017db7a3 100644 --- a/apps/api/src/app/environments/usecases/create-environment/create-environment.e2e.ts +++ b/apps/api/src/app/environments/usecases/create-environment/create-environment.e2e.ts @@ -2,7 +2,7 @@ import { expect } from 'chai'; import { EnvironmentRepository } from '@novu/dal'; import { UserSession } from '@novu/testing'; -import { NOVU_SUB_MASK } from '@novu/shared'; +import { NOVU_ENCRYPTION_SUB_MASK } from '@novu/shared'; describe('Create Environment - /environments (POST)', async () => { let session: UserSession; @@ -26,12 +26,13 @@ describe('Create Environment - /environments (POST)', async () => { const dbApp = await environmentRepository.findOne({ _id: body.data._id }); if (!dbApp) { + expect(dbApp).to.be.ok; throw new Error('App not found'); } expect(dbApp.apiKeys.length).to.equal(1); expect(dbApp.apiKeys[0].key).to.be.ok; - expect(dbApp.apiKeys[0].key).to.contains(NOVU_SUB_MASK); + expect(dbApp.apiKeys[0].key).to.contains(NOVU_ENCRYPTION_SUB_MASK); expect(dbApp.apiKeys[0]._userId).to.equal(session.user._id); }); diff --git a/apps/api/src/app/environments/usecases/create-environment/create-environment.usecase.ts b/apps/api/src/app/environments/usecases/create-environment/create-environment.usecase.ts index 7c873f88fa1..882ac67f2ac 100644 --- a/apps/api/src/app/environments/usecases/create-environment/create-environment.usecase.ts +++ b/apps/api/src/app/environments/usecases/create-environment/create-environment.usecase.ts @@ -23,7 +23,7 @@ export class CreateEnvironment { async execute(command: CreateEnvironmentCommand) { const key = await this.generateUniqueApiKey.execute(); - const encryptedKey = encryptApiKey(key); + const encryptedApiKey = encryptApiKey(key); const hashedApiKey = createHash('sha256').update(key).digest('hex'); const environment = await this.environmentRepository.create({ @@ -33,7 +33,7 @@ export class CreateEnvironment { _parentId: command.parentEnvironmentId, apiKeys: [ { - key: encryptedKey, + key: encryptedApiKey, _userId: command.userId, hash: hashedApiKey, }, diff --git a/apps/api/src/app/environments/usecases/generate-unique-api-key/generate-unique-api-key.spec.ts b/apps/api/src/app/environments/usecases/generate-unique-api-key/generate-unique-api-key.spec.ts index 8e9bbee8db2..71c2ac772e2 100644 --- a/apps/api/src/app/environments/usecases/generate-unique-api-key/generate-unique-api-key.spec.ts +++ b/apps/api/src/app/environments/usecases/generate-unique-api-key/generate-unique-api-key.spec.ts @@ -12,8 +12,7 @@ let generateApiKeyStub; let findByApiKeyStub; describe('Generate Unique Api Key', () => { beforeEach(() => { - // backward compatibility - update `findByApiKeyBackwardCompatibility` to `findByApiKey` after encrypt-api-keys-migration execution - findByApiKeyStub = sinon.stub(environmentRepository, 'findByApiKeyBackwardCompatibility'); + findByApiKeyStub = sinon.stub(environmentRepository, 'findByApiKey'); generateApiKeyStub = sinon.stub(generateUniqueApiKey, 'generateApiKey' as any); }); diff --git a/apps/api/src/app/environments/usecases/generate-unique-api-key/generate-unique-api-key.usecase.ts b/apps/api/src/app/environments/usecases/generate-unique-api-key/generate-unique-api-key.usecase.ts index aa31f3ca6b8..f5b2d81b5e2 100644 --- a/apps/api/src/app/environments/usecases/generate-unique-api-key/generate-unique-api-key.usecase.ts +++ b/apps/api/src/app/environments/usecases/generate-unique-api-key/generate-unique-api-key.usecase.ts @@ -31,8 +31,7 @@ export class GenerateUniqueApiKey { private async validateIsApiKeyUsed(apiKey: string) { const hashedApiKey = createHash('sha256').update(apiKey).digest('hex'); - // backward compatibility - update to `findByApiKey` after encrypt-api-keys-migration run - const environment = await this.environmentRepository.findByApiKeyBackwardCompatibility({ + const environment = await this.environmentRepository.findByApiKey({ key: apiKey, hash: hashedApiKey, }); diff --git a/apps/api/src/app/environments/usecases/get-my-environments/get-my-environments.e2e.ts b/apps/api/src/app/environments/usecases/get-my-environments/get-my-environments.e2e.ts index f577b1cd556..d32f240e0ac 100644 --- a/apps/api/src/app/environments/usecases/get-my-environments/get-my-environments.e2e.ts +++ b/apps/api/src/app/environments/usecases/get-my-environments/get-my-environments.e2e.ts @@ -1,6 +1,6 @@ import { UserSession } from '@novu/testing'; import { expect } from 'chai'; -import { NOVU_SUB_MASK } from '@novu/shared'; +import { NOVU_ENCRYPTION_SUB_MASK } from '@novu/shared'; describe('Get My Environments - /environments (GET)', async () => { let session: UserSession; @@ -20,7 +20,7 @@ describe('Get My Environments - /environments (GET)', async () => { if (elem._id !== session.environment._id) { expect(elem.apiKeys.length).to.eq(0); } else { - expect(elem.apiKeys[0].key).to.not.contains(NOVU_SUB_MASK); + expect(elem.apiKeys[0].key).to.not.contains(NOVU_ENCRYPTION_SUB_MASK); expect(elem.apiKeys[0]._userId).to.equal(session.user._id); expect(elem.apiKeys.length).to.be.greaterThanOrEqual(1); } diff --git a/apps/api/src/app/environments/usecases/get-my-environments/get-my-environments.usecase.ts b/apps/api/src/app/environments/usecases/get-my-environments/get-my-environments.usecase.ts index 878c2772ef0..0acaeb1d012 100644 --- a/apps/api/src/app/environments/usecases/get-my-environments/get-my-environments.usecase.ts +++ b/apps/api/src/app/environments/usecases/get-my-environments/get-my-environments.usecase.ts @@ -32,15 +32,15 @@ export class GetMyEnvironments { } private decryptApiKeys(environment: EnvironmentEntity): EnvironmentResponseDto { - const res = { ...environment }; + const decryptedApiKeysEnvironment = { ...environment }; - res.apiKeys = environment.apiKeys.map((apiKey) => { + decryptedApiKeysEnvironment.apiKeys = environment.apiKeys.map((apiKey) => { return { - _userId: apiKey._userId, + ...apiKey, key: decryptApiKey(apiKey.key), }; }); - return res; + return decryptedApiKeysEnvironment; } } diff --git a/libs/dal/src/repositories/environment/environment.repository.ts b/libs/dal/src/repositories/environment/environment.repository.ts index 9837de75e0d..2ac423c7ea7 100644 --- a/libs/dal/src/repositories/environment/environment.repository.ts +++ b/libs/dal/src/repositories/environment/environment.repository.ts @@ -51,18 +51,8 @@ export class EnvironmentRepository extends BaseRepository secureCred === key); } diff --git a/packages/application-generic/src/services/auth/auth.service.ts b/packages/application-generic/src/services/auth/auth.service.ts index 65c23bd72ac..7c00311deea 100644 --- a/packages/application-generic/src/services/auth/auth.service.ts +++ b/packages/application-generic/src/services/auth/auth.service.ts @@ -388,18 +388,14 @@ export class AuthService { }> { const hashedApiKey = createHash('sha256').update(apiKey).digest('hex'); - /* - * backward compatibility - after encrypt-api-keys-migration execution: - * * update `findByApiKeyBackwardCompatibility` to `findByApiKey` - */ - const environment = - await this.environmentRepository.findByApiKeyBackwardCompatibility({ - key: apiKey, - hash: hashedApiKey, - }); + const environment = await this.environmentRepository.findByApiKey({ + key: apiKey, + hash: hashedApiKey, + }); if (!environment) { - return { error: 'Environment not found' }; + // Failed to find the environment for the provided API key. + return { error: 'API Key not found' }; } let key = environment.apiKeys.find((i) => i.hash === hashedApiKey); @@ -408,7 +404,7 @@ export class AuthService { /* * backward compatibility - delete after encrypt-api-keys-migration execution * find by decrypted key if key not found, because of backward compatibility - * use-case: findByApiKeyBackwardCompatibility found by decrypted key, so we need to validate by decrypted key + * use-case: findByApiKey found by decrypted key, so we need to validate by decrypted key */ key = environment.apiKeys.find((i) => i.key === apiKey); } From 3422b7e3081b43ab9330d93a5abcb8bde4a4b204 Mon Sep 17 00:00:00 2001 From: Gosha Date: Sun, 21 Jan 2024 16:28:56 +0200 Subject: [PATCH 7/7] test(api): fix revert error message --- apps/api/src/app/auth/e2e/user.auth.guard.e2e.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/api/src/app/auth/e2e/user.auth.guard.e2e.ts b/apps/api/src/app/auth/e2e/user.auth.guard.e2e.ts index 2c2a3e3413f..9d0cc3a1494 100644 --- a/apps/api/src/app/auth/e2e/user.auth.guard.e2e.ts +++ b/apps/api/src/app/auth/e2e/user.auth.guard.e2e.ts @@ -46,7 +46,7 @@ describe('UserAuthGuard', () => { it('should return 401 when ApiKey auth scheme is provided with an invalid value', async () => { const response = await request(`${ApiAuthSchemeEnum.API_KEY} invalid_key`); expect(response.statusCode).to.equal(401); - expect(response.body.message).to.equal('Environment not found'); + expect(response.body.message).to.equal('API Key not found'); }); it('should return 401 when ApiKey auth scheme is used for an externally inaccessible API route', async () => {