From 92d03f1ff6d950981e194728af2f25d6fb4a5808 Mon Sep 17 00:00:00 2001 From: claude_code Date: Sun, 21 Jun 2026 17:05:35 +0300 Subject: [PATCH] test(server): cover returnToken body opt-in and CORS/Swagger env parsers Close the two "[test coverage]" review gaps on PR #116 (mobile bootstrap): - auth.controller.spec.ts: unit-test AuthController.login() returnToken branches via direct instantiation. returnToken:true returns exactly { authToken } alongside the httpOnly cookie; omitted/explicit-false return strictly undefined (the token must never leak into the response body for web clients) while the cookie is still set. - environment.service.spec.ts: table-driven tests for getCorsAllowedOrigins() (split/trim/filter of CORS_ALLOWED_ORIGINS) and isSwaggerEnabled() (case-insensitive SWAGGER_ENABLED === 'true'), the two parsers feeding the CORS allowlist and Swagger exposure trust boundaries. Tests only; no production code changed. Co-Authored-By: Claude Opus 4.8 --- .../src/core/auth/auth.controller.spec.ts | 89 +++++++++++++++++++ .../environment/environment.service.spec.ts | 65 ++++++++++++++ 2 files changed, 154 insertions(+) diff --git a/apps/server/src/core/auth/auth.controller.spec.ts b/apps/server/src/core/auth/auth.controller.spec.ts index 4746f249..dfd4cd5b 100644 --- a/apps/server/src/core/auth/auth.controller.spec.ts +++ b/apps/server/src/core/auth/auth.controller.spec.ts @@ -20,3 +20,92 @@ describe('AuthController', () => { expect(controller).toBeDefined(); }); }); + +// The EE MFA module is absent in this build, so login() always falls through to +// the bottom path: it mints the token, sets the httpOnly cookie, and ONLY echoes +// the token in the response body when loginInput.returnToken is truthy. Web +// clients omit returnToken, so the token must never leak into the body for them. +describe('login (returnToken body opt-in)', () => { + const TOKEN = 'access-token-123'; + + let controller: AuthController; + let authService: { login: jest.Mock }; + let environmentService: { getCookieExpiresIn: jest.Mock; isHttps: jest.Mock }; + let res: { setCookie: jest.Mock }; + let workspace: any; + + beforeEach(() => { + // Fresh stubs per test so setCookie/login call counts are isolated. + authService = { login: jest.fn().mockResolvedValue(TOKEN) }; + environmentService = { + getCookieExpiresIn: jest.fn().mockReturnValue(new Date(0)), + isHttps: jest.fn().mockReturnValue(false), + }; + res = { setCookie: jest.fn() }; + workspace = { id: 'workspace-1' }; // no enforceSso -> SSO check passes + + controller = new AuthController( + authService as any, // authService + {} as any, // sessionService + environmentService as any, // environmentService + {} as any, // moduleRef (MFA module absent -> never used) + {} as any, // auditService + ); + }); + + it('returns { authToken } in the body and sets the cookie when returnToken is true', async () => { + const loginInput = { + email: 'a@b.com', + password: 'pw', + returnToken: true, + } as any; + + const result = await controller.login(workspace, res as any, loginInput); + + // Body token is emitted alongside the cookie for native (Bearer) clients. + expect(result).toEqual({ authToken: TOKEN }); + expect(authService.login).toHaveBeenCalledWith(loginInput, workspace.id); + expect(res.setCookie).toHaveBeenCalledTimes(1); + expect(res.setCookie).toHaveBeenCalledWith( + 'authToken', + TOKEN, + expect.objectContaining({ httpOnly: true, sameSite: 'lax', path: '/' }), + ); + }); + + it('returns undefined (token only in cookie) when returnToken is omitted', async () => { + // Web client: returnToken is not present on the body at all. + const loginInput = { email: 'a@b.com', password: 'pw' } as any; + + const result = await controller.login(workspace, res as any, loginInput); + + // Security-critical: the token must NOT leak into the response body. + expect(result).toBeUndefined(); + expect(res.setCookie).toHaveBeenCalledTimes(1); + expect(res.setCookie).toHaveBeenCalledWith( + 'authToken', + TOKEN, + expect.objectContaining({ httpOnly: true, sameSite: 'lax', path: '/' }), + ); + }); + + it('returns undefined when returnToken is explicitly false', async () => { + // Guards against an `!== undefined` style bug: explicit false must behave + // exactly like the omitted case. + const loginInput = { + email: 'a@b.com', + password: 'pw', + returnToken: false, + } as any; + + const result = await controller.login(workspace, res as any, loginInput); + + expect(result).toBeUndefined(); + expect(res.setCookie).toHaveBeenCalledTimes(1); + expect(res.setCookie).toHaveBeenCalledWith( + 'authToken', + TOKEN, + expect.objectContaining({ httpOnly: true, sameSite: 'lax', path: '/' }), + ); + }); +}); diff --git a/apps/server/src/integrations/environment/environment.service.spec.ts b/apps/server/src/integrations/environment/environment.service.spec.ts index efef25b0..a36b2bde 100644 --- a/apps/server/src/integrations/environment/environment.service.spec.ts +++ b/apps/server/src/integrations/environment/environment.service.spec.ts @@ -15,3 +15,68 @@ describe('EnvironmentService', () => { expect(service).toBeDefined(); }); }); + +// Fake ConfigService that honors the (key, default) signature: returns the +// mapped value when the key is present, otherwise the provided default. This +// matters because getCorsAllowedOrigins/isSwaggerEnabled call .split/.toLowerCase +// on the result, so an unset key MUST fall back to the default string. +const makeConfig = (map: Record) => + new EnvironmentService({ + get: (key: string, def?: string) => (key in map ? map[key] : def), + } as any); + +// These two parsers feed trust boundaries (CORS allowlist + Swagger exposure), +// so a parsing bug has a direct security effect. Style mirrors +// trust-proxy.util.spec.ts. +describe('EnvironmentService.getCorsAllowedOrigins', () => { + it.each([ + ['key unset -> empty list', {}, []], + ['empty string -> empty list', { CORS_ALLOWED_ORIGINS: '' }, []], + [ + 'single origin', + { CORS_ALLOWED_ORIGINS: 'https://app.example' }, + ['https://app.example'], + ], + [ + 'multiple origins', + { CORS_ALLOWED_ORIGINS: 'https://a.example,https://b.example' }, + ['https://a.example', 'https://b.example'], + ], + [ + 'trims surrounding and inner spaces', + { CORS_ALLOWED_ORIGINS: ' https://a.example , https://b.example ' }, + ['https://a.example', 'https://b.example'], + ], + [ + 'double/trailing commas produce no empty strings', + { CORS_ALLOWED_ORIGINS: 'a,, ,b' }, + ['a', 'b'], + ], + [ + 'leading/trailing commas with spaces', + { CORS_ALLOWED_ORIGINS: ' , a , , b ' }, + ['a', 'b'], + ], + ])('%s', (_label, map, expected) => { + expect(makeConfig(map as Record).getCorsAllowedOrigins()).toEqual( + expected, + ); + }); +}); + +describe('EnvironmentService.isSwaggerEnabled', () => { + it.each([ + ["'true' -> true", { SWAGGER_ENABLED: 'true' }, true], + ["'TRUE' -> true", { SWAGGER_ENABLED: 'TRUE' }, true], + ["'True' -> true", { SWAGGER_ENABLED: 'True' }, true], + ['key unset -> false', {}, false], + ["'false' -> false", { SWAGGER_ENABLED: 'false' }, false], + ["empty string -> false", { SWAGGER_ENABLED: '' }, false], + ["'yes' -> false", { SWAGGER_ENABLED: 'yes' }, false], + ["'1' -> false", { SWAGGER_ENABLED: '1' }, false], + ])('%s', (_label, map, expected) => { + expect(makeConfig(map as Record).isSwaggerEnabled()).toBe( + expected, + ); + }); +});