diff --git a/apps/backend/src/aws/s3/README.md b/apps/backend/src/aws/s3/README.md index ad89861ff..429459727 100644 --- a/apps/backend/src/aws/s3/README.md +++ b/apps/backend/src/aws/s3/README.md @@ -16,33 +16,37 @@ AWS_MY_BUCKET_NAME=my-bucket-name Import `AWSS3Module` once in your root `AppModule`. Because the module is `@Global()`, `AWSS3Service` is injectable in all feature modules without additional imports. -The service throws at startup if any bucket env var is unset, so misconfiguration is caught immediately rather than at runtime. +Every bucket env var **must** follow the `AWS__BUCKET_NAME` format, where `` is the `S3Buckets` enum member verbatim. `AWSS3Service` builds its bucket lookup from that convention, so a name that doesn't match will never be found. + +`AWSS3Module.onModuleInit` logs a warning listing any env var in its `REQUIRED_ENV_VARS` list that is unset or blank. ## Adding a New Bucket -**1. Add an env var** in `.env` and `example.env`: +**1. Add an env var** in `.env` and `example.env`, named `AWS__BUCKET_NAME`: ``` AWS_MY_BUCKET_NAME=my-bucket-name ``` -**2. Add an entry to the `s3Buckets` enum** (`types/s3Buckets.ts`): +**2. Add an entry to the `S3Buckets` enum** (`types/s3Buckets.ts`), matching the middle of that env var name: ```typescript -export enum s3Buckets { +export enum S3Buckets { MY_BUCKET = 'MY_BUCKET', } ``` -**3. Add a mapping to `mapBucket`** (`aws-s3.service.ts`): +**3. Add the env var name to `REQUIRED_ENV_VARS`** (`aws-s3.module.ts`), so a missing value is reported at startup: ```typescript -const bucketNames: Record = { - [s3Buckets.MY_BUCKET]: process.env.AWS_MY_BUCKET_NAME, -}; +const REQUIRED_ENV_VARS = [ + 'AWS_ACCESS_KEY', + 'AWS_SECRET_KEY', + 'AWS_MY_BUCKET_NAME', +] as const; ``` -Because `mapBucket` uses a `Record`, TypeScript will produce a compile error if you add an enum entry without adding the corresponding mapping — catching missed steps at build time. +No change to `aws-s3.service.ts` is needed: its constructor resolves `process.env['AWS_' + bucket + '_BUCKET_NAME']` for every member of `S3Buckets`. A bucket whose env var is missing resolves to `''`, and any `upload()` to it throws `Missing required environment variable for S3 bucket: MY_BUCKET`. ## Required IAM Permissions diff --git a/apps/backend/src/aws/s3/aws-s3.module.spec.ts b/apps/backend/src/aws/s3/aws-s3.module.spec.ts new file mode 100644 index 000000000..f97dfc92e --- /dev/null +++ b/apps/backend/src/aws/s3/aws-s3.module.spec.ts @@ -0,0 +1,70 @@ +import { AWSS3Module } from './aws-s3.module'; + +describe('AWSS3Module', () => { + let module: AWSS3Module; + let warnSpy: jest.SpyInstance; + let logSpy: jest.SpyInstance; + + beforeEach(() => { + process.env.AWS_ACCESS_KEY = 'test-access-key'; + process.env.AWS_SECRET_KEY = 'test-secret-key'; + module = new AWSS3Module(); + + warnSpy = jest + .spyOn(module['logger'], 'warn') + .mockImplementation(() => undefined); + logSpy = jest + .spyOn(module['logger'], 'log') + .mockImplementation(() => undefined); + }); + + afterEach(() => { + jest.restoreAllMocks(); + }); + + it('should log and not warn when required env vars are set', () => { + module.onModuleInit(); + + expect(warnSpy).not.toHaveBeenCalled(); + expect(logSpy).toHaveBeenCalledWith('S3 configured'); + }); + + it('should warn if AWS_ACCESS_KEY is missing', () => { + delete process.env.AWS_ACCESS_KEY; + + expect(() => module.onModuleInit()).not.toThrow(); + expect(warnSpy).toHaveBeenCalledWith( + expect.stringContaining('AWS_ACCESS_KEY'), + ); + }); + + it('should warn if AWS_SECRET_KEY is missing', () => { + delete process.env.AWS_SECRET_KEY; + + expect(() => module.onModuleInit()).not.toThrow(); + expect(warnSpy).toHaveBeenCalledWith( + expect.stringContaining('AWS_SECRET_KEY'), + ); + }); + + it('should warn if an env var is whitespace-only', () => { + process.env.AWS_ACCESS_KEY = ' '; + + expect(() => module.onModuleInit()).not.toThrow(); + expect(warnSpy).toHaveBeenCalledWith( + expect.stringContaining('AWS_ACCESS_KEY'), + ); + }); + + it('should list every missing env var in a single warning', () => { + delete process.env.AWS_ACCESS_KEY; + delete process.env.AWS_SECRET_KEY; + + module.onModuleInit(); + + expect(warnSpy).toHaveBeenCalledTimes(1); + expect(warnSpy).toHaveBeenCalledWith( + expect.stringContaining('AWS_ACCESS_KEY, AWS_SECRET_KEY'), + ); + }); +}); diff --git a/apps/backend/src/aws/s3/aws-s3.module.ts b/apps/backend/src/aws/s3/aws-s3.module.ts index a3a6a2482..9e932590c 100644 --- a/apps/backend/src/aws/s3/aws-s3.module.ts +++ b/apps/backend/src/aws/s3/aws-s3.module.ts @@ -1,9 +1,33 @@ -import { Global, Module } from '@nestjs/common'; +import { Global, Logger, Module, OnModuleInit } from '@nestjs/common'; import { AWSS3Service } from './aws-s3.service'; +// Required s3 env values. +// Add one entry per bucket here: +const REQUIRED_ENV_VARS = ['AWS_ACCESS_KEY', 'AWS_SECRET_KEY'] as const; + @Global() @Module({ providers: [AWSS3Service], exports: [AWSS3Service], }) -export class AWSS3Module {} +export class AWSS3Module implements OnModuleInit { + private readonly logger = new Logger(AWSS3Module.name); + + onModuleInit(): void { + // Treat unset and empty/whitespace-only values as missing. + const missing = REQUIRED_ENV_VARS.filter((name) => { + const value = process.env[name]; + return !value || value.trim().length === 0; + }); + + if (missing.length > 0) { + this.logger.warn( + `S3 not fully configured: missing env vars (${missing.join( + ', ', + )}). S3 uploads and downloads will fail.`, + ); + } else { + this.logger.log('S3 configured'); + } + } +} diff --git a/apps/backend/src/aws/s3/aws-s3.service.spec.ts b/apps/backend/src/aws/s3/aws-s3.service.spec.ts index 0e7659d08..2fb04a526 100644 --- a/apps/backend/src/aws/s3/aws-s3.service.spec.ts +++ b/apps/backend/src/aws/s3/aws-s3.service.spec.ts @@ -30,25 +30,11 @@ describe('AWSS3Service', () => { s3Mock.reset(); service = new AWSS3Service(); + // The constructor resolves AWS__BUCKET_NAME for every member of + // S3Buckets, but the scaffold enum is empty — inject the sentinel by hand. service['bucketNames'][testBucketEnum] = testBucket; }); - describe('constructor', () => { - it('should throw if AWS_ACCESS_KEY is missing', () => { - delete process.env.AWS_ACCESS_KEY; - expect(() => new AWSS3Service()).toThrow( - 'Missing required environment variable: AWS_ACCESS_KEY', - ); - }); - - it('should throw if AWS_SECRET_KEY is missing', () => { - delete process.env.AWS_SECRET_KEY; - expect(() => new AWSS3Service()).toThrow( - 'Missing required environment variable: AWS_SECRET_KEY', - ); - }); - }); - describe('upload', () => { const validInput: S3UploadInput = { fileBuffer: Buffer.from('test'), diff --git a/apps/backend/src/aws/s3/aws-s3.service.ts b/apps/backend/src/aws/s3/aws-s3.service.ts index f4da46362..8ef4a04f0 100644 --- a/apps/backend/src/aws/s3/aws-s3.service.ts +++ b/apps/backend/src/aws/s3/aws-s3.service.ts @@ -41,31 +41,23 @@ export class AWSS3Service { constructor() { this.region = process.env.AWS_REGION ?? 'us-east-2'; - // Add one entry per bucket in s3Buckets enum. - // Example: [s3Buckets.DOCUMENTS]: process.env.AWS_DOCUMENTS_BUCKET_NAME, + // Every bucket in the S3Buckets enum is read from an env var named AWS__BUCKET_NAME: + // - e.g. S3Buckets.DOCUMENTS reads AWS_DOCUMENTS_BUCKET_NAME. Add each of those names to REQUIRED_ENV_VARS this.bucketNames = {} as Record; for (const bucket of Object.values(S3Buckets) as unknown as S3Buckets[]) { - if (!this.bucketNames[bucket]) { - throw new Error( - `Missing required environment variable for S3 bucket: ${bucket}`, - ); - } - } - - const accessKeyId = process.env.AWS_ACCESS_KEY; - const secretAccessKey = process.env.AWS_SECRET_KEY; - - if (!accessKeyId) { - throw new Error('Missing required environment variable: AWS_ACCESS_KEY'); - } - if (!secretAccessKey) { - throw new Error('Missing required environment variable: AWS_SECRET_KEY'); + this.bucketNames[bucket] = process.env[`AWS_${bucket}_BUCKET_NAME`] ?? ''; } + // AWS credentials are checked at module initialization (see AWSS3Module), + // which warns rather than throws. The ?? '' keeps the client constructible + // when they are absent; requests against it then fail at the AWS call. this.client = new S3Client({ region: this.region, - credentials: { accessKeyId, secretAccessKey }, + credentials: { + accessKeyId: process.env.AWS_ACCESS_KEY ?? '', + secretAccessKey: process.env.AWS_SECRET_KEY ?? '', + }, }); } diff --git a/apps/backend/src/aws/ses/README.md b/apps/backend/src/aws/ses/README.md index 3841717d9..999362617 100644 --- a/apps/backend/src/aws/ses/README.md +++ b/apps/backend/src/aws/ses/README.md @@ -4,16 +4,16 @@ Thin wrapper around Amazon SES v2 for sending transactional emails (with optiona ## Injecting `EmailsService` -`EmailsModule` exports `EmailsService`, so any consuming module just needs to import `EmailsModule` and then inject `EmailsService` through the constructor. +`AWSSESModule` exports `EmailsService`, so any consuming module just needs to import `AWSSESModule` and then inject `EmailsService` through the constructor. -1. **Import `EmailsModule`** in the consuming module: +1. **Import `AWSSESModule`** in the consuming module: ```ts // users.module.ts - import { EmailsModule } from '../aws/ses/email.module'; + import { AWSSESModule } from '../aws/ses/email.module'; @Module({ - imports: [TypeOrmModule.forFeature([User]), EmailsModule], + imports: [TypeOrmModule.forFeature([User]), AWSSESModule], controllers: [UsersController], providers: [UsersService], }) @@ -62,5 +62,5 @@ If you swap `AWS_SES_SENDER_EMAIL` later, the new address must be verified separ A boolean env var (`'true'` to enable, anything else — including unset — to disable) that gates real SES dispatch. -- When `SEND_AUTOMATED_EMAILS === 'true'`: `sendEmail` runs DTO validation, then schedules the send through the rate limiter, then calls SES. Returns the `SendEmailCommandOutput` from SES (MessageId + metadata). `AWS_SES_SENDER_EMAIL` must be set at this point, or the send throws. +- When `SEND_AUTOMATED_EMAILS === 'true'`: `AWS_REGION`, `AWS_ACCESS_KEY_ID`, `AWS_SECRET_ACCESS_KEY`, and `AWS_SES_SENDER_EMAIL` are checked at module initialization (`AWSSESModule.onModuleInit`), which logs a warning listing any that are missing and boots anyway — sends then fail at the SES call. In production a missing var almost certainly means missing secrets: raise that `logger.warn` to `logger.error`, or throw, so the app fails at startup instead. `sendEmail` runs DTO validation, schedules the send through the rate limiter, then calls SES. Returns the `SendEmailCommandOutput` from SES (MessageId + metadata). - When `SEND_AUTOMATED_EMAILS` is unset or any other value: `sendEmail` still runs DTO validation (so a bad payload still throws), then logs a warning (`SEND_AUTOMATED_EMAILS is not "true". Email not sent.`) and returns `void` without contacting SES. Neither `AWS_SES_SENDER_EMAIL` nor the AWS credentials need to be defined — teams not using SES can omit them entirely and the app still boots. diff --git a/apps/backend/src/aws/ses/awsSes.wrapper.ts b/apps/backend/src/aws/ses/awsSes.wrapper.ts index c3893bbd3..d0caf79dc 100644 --- a/apps/backend/src/aws/ses/awsSes.wrapper.ts +++ b/apps/backend/src/aws/ses/awsSes.wrapper.ts @@ -30,8 +30,9 @@ export class AmazonSESWrapper { * or if SES rejects the send (bad recipient, throttling, unverified sender, quota exceeded). */ async sendEmail(dto: SendEmailDTO): Promise { - const senderEmail = process.env.AWS_SES_SENDER_EMAIL; - if (!senderEmail) throw new Error('AWS_SES_SENDER_EMAIL is not defined'); + // Checked at module initialization (see AWSSESModule) when SES is enabled; + // sendEmail is only ever reached when SEND_AUTOMATED_EMAILS is 'true', so senderEmail is expected to be present here. + const senderEmail = process.env.AWS_SES_SENDER_EMAIL ?? ''; const mailOptions: Mail.Options = { from: senderEmail, diff --git a/apps/backend/src/aws/ses/awsSesClient.factory.ts b/apps/backend/src/aws/ses/awsSesClient.factory.ts index ea288070f..1f4de8c3f 100644 --- a/apps/backend/src/aws/ses/awsSesClient.factory.ts +++ b/apps/backend/src/aws/ses/awsSesClient.factory.ts @@ -11,22 +11,19 @@ export const AMAZON_SES_CLIENT = 'AMAZON_SES_CLIENT'; export const AmazonSESClientFactory: Provider = { provide: AMAZON_SES_CLIENT, useFactory: () => { - // Create dummy client that is never used when email sending is set to false - if (process.env.SEND_AUTOMATED_EMAILS !== 'true') { + // Create dummy client that is NOT used when email sending is unset or set to false. + if (process.env.SEND_AUTOMATED_EMAILS?.toLowerCase() !== 'true') { return new SESv2Client({}); } - const region = process.env.AWS_REGION; - const accessKeyId = process.env.AWS_ACCESS_KEY_ID; - const secretAccessKey = process.env.AWS_SECRET_ACCESS_KEY; - - if (!region) throw new Error('AWS_REGION is not defined'); - if (!accessKeyId) throw new Error('AWS_ACCESS_KEY_ID is not defined'); - if (!secretAccessKey) - throw new Error('AWS_SECRET_ACCESS_KEY is not defined'); + // If email sending is enabled, AWSSESModule.onModuleInit() warns when these env vars are missing. + // The empty-string fallbacks keep the client constructible; sends against it fail at the SES call. return new SESv2Client({ - region, - credentials: { accessKeyId, secretAccessKey }, + region: process.env.AWS_REGION ?? '', + credentials: { + accessKeyId: process.env.AWS_ACCESS_KEY_ID ?? '', + secretAccessKey: process.env.AWS_SECRET_ACCESS_KEY ?? '', + }, }); }, }; diff --git a/apps/backend/src/aws/ses/email.module.spec.ts b/apps/backend/src/aws/ses/email.module.spec.ts new file mode 100644 index 000000000..b33f7c967 --- /dev/null +++ b/apps/backend/src/aws/ses/email.module.spec.ts @@ -0,0 +1,125 @@ +import { AWSSESModule } from './email.module'; + +describe('AWSSESModule', () => { + const ENV_VARS = [ + 'SEND_AUTOMATED_EMAILS', + 'AWS_REGION', + 'AWS_ACCESS_KEY_ID', + 'AWS_SECRET_ACCESS_KEY', + 'AWS_SES_SENDER_EMAIL', + ] as const; + + const REQUIRED_WHEN_ENABLED = [ + 'AWS_REGION', + 'AWS_ACCESS_KEY_ID', + 'AWS_SECRET_ACCESS_KEY', + 'AWS_SES_SENDER_EMAIL', + ] as const; + + const originalEnv: Record = {}; + let module: AWSSESModule; + let warnSpy: jest.SpyInstance; + let logSpy: jest.SpyInstance; + + beforeEach(() => { + for (const name of ENV_VARS) { + originalEnv[name] = process.env[name]; + } + + // Default to a fully-configured, enabled setup; individual tests override. + process.env.SEND_AUTOMATED_EMAILS = 'true'; + process.env.AWS_REGION = 'us-east-2'; + process.env.AWS_ACCESS_KEY_ID = 'test-access-key-id'; + process.env.AWS_SECRET_ACCESS_KEY = 'test-secret-access-key'; + process.env.AWS_SES_SENDER_EMAIL = 'sender@example.com'; + + module = new AWSSESModule(); + + warnSpy = jest + .spyOn(module['logger'], 'warn') + .mockImplementation(() => undefined); + logSpy = jest + .spyOn(module['logger'], 'log') + .mockImplementation(() => undefined); + }); + + afterEach(() => { + jest.restoreAllMocks(); + + for (const name of ENV_VARS) { + if (originalEnv[name] === undefined) { + delete process.env[name]; + } else { + process.env[name] = originalEnv[name]; + } + } + }); + + describe('onModuleInit', () => { + it('logs and does not warn when all required env vars are set and enabled', () => { + module.onModuleInit(); + + expect(warnSpy).not.toHaveBeenCalled(); + expect(logSpy).toHaveBeenCalledWith('SES enabled'); + }); + + it('does not warn when disabled, even if required vars are missing', () => { + process.env.SEND_AUTOMATED_EMAILS = 'false'; + for (const name of REQUIRED_WHEN_ENABLED) { + delete process.env[name]; + } + + module.onModuleInit(); + + expect(warnSpy).not.toHaveBeenCalled(); + expect(logSpy).toHaveBeenCalledWith( + expect.stringContaining('SES disabled'), + ); + }); + + it('does not warn when SEND_AUTOMATED_EMAILS is unset', () => { + delete process.env.SEND_AUTOMATED_EMAILS; + for (const name of REQUIRED_WHEN_ENABLED) { + delete process.env[name]; + } + + module.onModuleInit(); + + expect(warnSpy).not.toHaveBeenCalled(); + expect(logSpy).toHaveBeenCalledWith( + expect.stringContaining('SES disabled'), + ); + }); + + it.each(REQUIRED_WHEN_ENABLED)( + 'warns when enabled and %s is missing', + (name) => { + delete process.env[name]; + + expect(() => module.onModuleInit()).not.toThrow(); + expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining(name)); + }, + ); + + it('warns when enabled and a required var is empty/whitespace-only', () => { + process.env.AWS_SES_SENDER_EMAIL = ' '; + + expect(() => module.onModuleInit()).not.toThrow(); + expect(warnSpy).toHaveBeenCalledWith( + expect.stringContaining('AWS_SES_SENDER_EMAIL'), + ); + }); + + it('lists every missing env var in a single warning', () => { + delete process.env.AWS_REGION; + delete process.env.AWS_SES_SENDER_EMAIL; + + module.onModuleInit(); + + expect(warnSpy).toHaveBeenCalledTimes(1); + expect(warnSpy).toHaveBeenCalledWith( + expect.stringContaining('AWS_REGION, AWS_SES_SENDER_EMAIL'), + ); + }); + }); +}); diff --git a/apps/backend/src/aws/ses/email.module.ts b/apps/backend/src/aws/ses/email.module.ts index a6cd1bd12..316e85ba4 100644 --- a/apps/backend/src/aws/ses/email.module.ts +++ b/apps/backend/src/aws/ses/email.module.ts @@ -1,10 +1,47 @@ -import { Module } from '@nestjs/common'; +import { Logger, Module, OnModuleInit } from '@nestjs/common'; import { EmailsService } from './email.service'; import { AmazonSESWrapper } from './awsSes.wrapper'; import { AmazonSESClientFactory } from './awsSesClient.factory'; +// Env vars required only when SES dispatch is enabled (SEND_AUTOMATED_EMAILS === 'true') +const REQUIRED_ENV_VARS_WHEN_ENABLED = [ + 'AWS_REGION', + 'AWS_ACCESS_KEY_ID', + 'AWS_SECRET_ACCESS_KEY', + 'AWS_SES_SENDER_EMAIL', +] as const; + @Module({ providers: [AmazonSESWrapper, AmazonSESClientFactory, EmailsService], exports: [EmailsService], }) -export class EmailsModule {} +export class AWSSESModule implements OnModuleInit { + private readonly logger = new Logger(AWSSESModule.name); + + onModuleInit(): void { + // Email sending is disabled: skip validation so teams not using SES can + // boot without any AWS config. + if (process.env.SEND_AUTOMATED_EMAILS?.toLowerCase() !== 'true') { + this.logger.log( + 'SES disabled: SEND_AUTOMATED_EMAILS is not "true". No emails will be sent.', + ); + return; + } + + // Treat unset and empty/whitespace-only values as missing. + const missing = REQUIRED_ENV_VARS_WHEN_ENABLED.filter((name) => { + const value = process.env[name]; + return !value || value.trim().length === 0; + }); + + if (missing.length > 0) { + this.logger.warn( + `SES enabled but not fully configured: missing env vars (${missing.join( + ', ', + )}). Email sends will fail.`, + ); + } else { + this.logger.log('SES enabled'); + } + } +} diff --git a/apps/backend/src/users/users.module.ts b/apps/backend/src/users/users.module.ts index 2638f52fc..e59be58db 100644 --- a/apps/backend/src/users/users.module.ts +++ b/apps/backend/src/users/users.module.ts @@ -6,10 +6,10 @@ import { User } from './user.entity'; import { JwtStrategy } from '../auth/jwt.strategy'; import { CurrentUserInterceptor } from '../interceptors/current-user.interceptor'; import { AuthService } from '../auth/auth.service'; -import { EmailsModule } from '../aws/ses/email.module'; +import { AWSSESModule } from '../aws/ses/email.module'; @Module({ - imports: [TypeOrmModule.forFeature([User]), EmailsModule], + imports: [TypeOrmModule.forFeature([User]), AWSSESModule], controllers: [UsersController], providers: [UsersService, AuthService, JwtStrategy, CurrentUserInterceptor], })