diff --git a/apps/backend/src/aws/cognito/README.md b/apps/backend/src/aws/cognito/README.md index 70e709ed5..a516995fe 100644 --- a/apps/backend/src/aws/cognito/README.md +++ b/apps/backend/src/aws/cognito/README.md @@ -52,13 +52,21 @@ Copy placeholders from the repo root `example.env` into `.env` (or your deployme > If `COGNITO_USER_POOL_ID` or `COGNITO_CLIENT_ID` variables are unset, authentication via JWT enforcement is **disabled entirely** and every route is left open. `getCognitoConfig()` returns `null` when either of these two is missing/empty, and `isAuthEnabled()` is derived from it. `COGNITO_REGION` is **not** part of this check — when it is missing the region is derived from the user pool ID, so auth stays enabled. > At startup `CognitoModule` logs the auth state exactly once (`Cognito auth enabled`, or `Cognito auth disabled: env vars missing. All routes open.`). > -> The disabled message should be logged at **error** level when `NODE_ENV === 'production'`: +> The disabled message is logged at **error** level when `NODE_ENV === 'production'` and at **warn** level otherwise, because running without Cognito is a normal local workflow but almost always a missing-secrets bug in production: +```typescript +if (process.env.NODE_ENV === 'production') { + this.logger.error(message); +} else { + this.logger.warn(message); +} ``` -// if (process.env.NODE_ENV === 'production') { -// this.logger.error(message); -// } -``` + +### You have to set `NODE_ENV` yourself + +Nothing in this repo sets it. `@nx/webpack` builds node targets with `mode: 'none'` specifically so `process.env.NODE_ENV` is **not** substituted at build time, which means the compiled backend reads it from the runtime environment. If your deployment never exports it, the check above is inert and a production app with auth off will only warn. + +Set it where the process actually starts — the ECS task definition, the Elastic Beanstalk environment config, the systemd unit — rather than in a `.env` baked into the image, so the value tracks the environment rather than the build. `example.env` carries `NODE_ENV=development` as the local default. On the frontend, `apps/frontend/src/main.tsx` logs a `console.error` when auth is disabled in a production build (`import.meta.env.PROD`), since a production bundle with no login gate is almost always a build-time misconfiguration. Vite sets `PROD` from the build mode, so this needs no environment variable of its own. diff --git a/apps/backend/src/aws/cognito/cognito.module.spec.ts b/apps/backend/src/aws/cognito/cognito.module.spec.ts new file mode 100644 index 000000000..af0f60d81 --- /dev/null +++ b/apps/backend/src/aws/cognito/cognito.module.spec.ts @@ -0,0 +1,142 @@ +import { Logger } from '@nestjs/common'; + +// Importing the module pulls in CognitoJWTGuard, and through it jwks-rsa, whose +// `jose` dependency ships ESM that Jest cannot parse. Nothing here exercises the +// guard, so stub the module out -- cognito.guard.spec.ts does the same. +jest.mock('jwks-rsa', () => ({ + __esModule: true, + default: jest.fn(() => ({ getSigningKey: jest.fn() })), +})); + +import { CognitoModule } from './cognito.module'; + +// Environment variables that decide whether auth is enabled, plus NODE_ENV, +// which decides how loudly a disabled state is reported. +const ENV_KEYS = [ + 'COGNITO_USER_POOL_ID', + 'COGNITO_CLIENT_ID', + 'COGNITO_REGION', + 'NODE_ENV', +] as const; + +const ACTIVE_ENV = { + COGNITO_USER_POOL_ID: 'us-east-2_TestPool', + COGNITO_CLIENT_ID: 'test-client-id', + COGNITO_REGION: 'us-east-2', +}; + +const DISABLED_MESSAGE = + 'Cognito auth disabled: env vars missing. All routes open.'; + +function setActiveEnv(): void { + Object.assign(process.env, ACTIVE_ENV); +} + +describe('CognitoModule', () => { + let module: CognitoModule; + let error: jest.SpyInstance; + let warn: jest.SpyInstance; + let log: jest.SpyInstance; + + // Snapshot of the keys this suite mutates, so tests cannot leak into each + // other or into the rest of the run. Jest sets NODE_ENV='test' by default, + // and that value has to be restored like any other. + const originalEnv: Partial> = {}; + + beforeEach(() => { + ENV_KEYS.forEach((key) => { + originalEnv[key] = process.env[key]; + delete process.env[key]; + }); + + module = new CognitoModule(); + error = jest.spyOn(Logger.prototype, 'error').mockImplementation(); + warn = jest.spyOn(Logger.prototype, 'warn').mockImplementation(); + log = jest.spyOn(Logger.prototype, 'log').mockImplementation(); + }); + + afterEach(() => { + ENV_KEYS.forEach((key) => { + const value = originalEnv[key]; + if (value === undefined) { + delete process.env[key]; + } else { + process.env[key] = value; + } + }); + jest.restoreAllMocks(); + }); + + describe('When auth is enabled', () => { + beforeEach(() => { + setActiveEnv(); + }); + + it('logs that auth is enabled and raises nothing', () => { + module.onModuleInit(); + + expect(log).toHaveBeenCalledWith('Cognito auth enabled'); + expect(warn).not.toHaveBeenCalled(); + expect(error).not.toHaveBeenCalled(); + }); + + // The severity branch keys off NODE_ENV, but it is only reached when auth + // is disabled -- a correctly configured production app stays quiet. + it('stays quiet in production', () => { + process.env.NODE_ENV = 'production'; + + module.onModuleInit(); + + expect(log).toHaveBeenCalledWith('Cognito auth enabled'); + expect(error).not.toHaveBeenCalled(); + }); + }); + + describe('When auth is disabled', () => { + // Auth is off for every test here: no COGNITO_* vars are set. + + it('logs at error level in production', () => { + process.env.NODE_ENV = 'production'; + + module.onModuleInit(); + + expect(error).toHaveBeenCalledWith(DISABLED_MESSAGE); + expect(warn).not.toHaveBeenCalled(); + expect(log).not.toHaveBeenCalled(); + }); + + it('logs at warn level in development', () => { + process.env.NODE_ENV = 'development'; + + module.onModuleInit(); + + expect(warn).toHaveBeenCalledWith(DISABLED_MESSAGE); + expect(error).not.toHaveBeenCalled(); + expect(log).not.toHaveBeenCalled(); + }); + + // Nothing in this repo sets NODE_ENV, so an unset value is the common case. + // It must degrade to the dev-safe warning rather than the error, otherwise + // every local run would report a problem that isn't one. + it('logs at warn level when NODE_ENV is unset', () => { + expect(process.env.NODE_ENV).toBeUndefined(); + + module.onModuleInit(); + + expect(warn).toHaveBeenCalledWith(DISABLED_MESSAGE); + expect(error).not.toHaveBeenCalled(); + }); + + // Partial configuration is still disabled configuration: getCognitoConfig() + // returns null unless BOTH the pool ID and the client ID are present. + it('escalates in production when only the user pool ID is set', () => { + process.env.NODE_ENV = 'production'; + process.env.COGNITO_USER_POOL_ID = ACTIVE_ENV.COGNITO_USER_POOL_ID; + + module.onModuleInit(); + + expect(error).toHaveBeenCalledWith(DISABLED_MESSAGE); + expect(log).not.toHaveBeenCalled(); + }); + }); +}); diff --git a/apps/backend/src/aws/cognito/cognito.module.ts b/apps/backend/src/aws/cognito/cognito.module.ts index 8e6e40d65..6a9d03632 100644 --- a/apps/backend/src/aws/cognito/cognito.module.ts +++ b/apps/backend/src/aws/cognito/cognito.module.ts @@ -22,12 +22,19 @@ export class CognitoModule implements OnModuleInit { /** * IMPORTANT: * Running without Cognito is a normal development workflow, so a warning - * is enough here. In production it almost certainly signals a - * misconfiguration (missing secrets) and should be surfaced at error - * level instead. See this module's README for that change, and for how to - * make production fail hard (throw) rather than merely logging. + * is enough there. In production it almost certainly signals a + * misconfiguration (missing secrets), so surface it at error level. + * + * NODE_ENV is read at runtime, not inlined at build time, so this only + * escalates if the deploy environment actually exports + * NODE_ENV=production. See this module's README for the deployment step + * and for how to make production fail hard (throw) rather than log. */ - this.logger.warn(message); + if (process.env.NODE_ENV === 'production') { + this.logger.error(message); + } else { + this.logger.warn(message); + } } else { this.logger.log(`Cognito auth enabled`); } diff --git a/example.env b/example.env index 3156bfb9a..6c7e7edbd 100644 --- a/example.env +++ b/example.env @@ -8,10 +8,19 @@ NX_DB_PASSWORD= NX_DB_DATABASE=jumpstart NX_DB_PORT=5432 +# Environment name. Nothing in this repo sets it for you: the backend reads +# process.env.NODE_ENV at runtime (nx builds node targets with webpack +# mode 'none' precisely so it is NOT baked into the bundle). +# Set NODE_ENV=production in your DEPLOY environment -- the ECS task definition, +# Elastic Beanstalk config, or systemd unit -- not in a .env shipped in the image. +# Today this only raises the severity of the "Cognito auth disabled" startup log +# from warn to error, but treat it as the switch that marks a real deployment. +NODE_ENV=development + # AWS Cognito # Note: Leaving COGNITO_USER_POOL_ID or COGNITO_CLIENT_ID empty/missing disables auth ENTIRELY. # COGNITO_REGION is optional: when unset it is derived from the user pool ID (_). -# In production a missing config should be logged at error level (dev: warning). +# A missing config is logged at warn level, or at ERROR level when NODE_ENV=production. # These vars drive BOTH the backend (JWT verification) and the frontend: # vite.config.ts re-exports them to the client as VITE_COGNITO_* at build time, # so you never set the VITE_ variables by hand.