From 616c33cd76b70d939d4ed2776c3e4330f2507745 Mon Sep 17 00:00:00 2001 From: Demian Date: Sat, 11 Apr 2026 18:24:29 -0400 Subject: [PATCH 1/8] feat: switch auth tokens to httpOnly cookies Moves access and refresh tokens out of the response body and into httpOnly cookies to prevent XSS-based token theft. Changes: - Register cookie-parser middleware in main.ts - CORS configured with credentials: true for cross-origin cookie support - JwtStrategy extracts access_token from req.cookies - login sets access_token (15m) and refresh_token (7d) as httpOnly cookies - refresh rotates both cookies; logout clears them - RefreshTokenAuthGuard rewritten as CanActivate reading refresh_token cookie - Remove passport-http-bearer dependency (no longer needed) Closes #93 --- backend/package.json | 4 +- backend/src/main.ts | 31 +++--- backend/src/modules/auth/auth.controller.ts | 73 ++++++++++--- backend/src/modules/auth/auth.module.ts | 3 +- backend/src/modules/auth/auth.service.ts | 15 ++- backend/src/modules/auth/jwt.strategy.ts | 7 +- .../modules/auth/refresh-token-auth.guard.ts | 21 +++- .../modules/auth/refresh-token.strategy.ts | 20 ---- pnpm-lock.yaml | 102 +++++------------- 9 files changed, 136 insertions(+), 140 deletions(-) delete mode 100644 backend/src/modules/auth/refresh-token.strategy.ts diff --git a/backend/package.json b/backend/package.json index 76649e9..2beee56 100644 --- a/backend/package.json +++ b/backend/package.json @@ -50,10 +50,10 @@ "cache-manager-redis-yet": "^5.1.5", "class-transformer": "^0.5.1", "class-validator": "^0.14.2", + "cookie-parser": "^1.4.7", "dotenv": "^16.4.5", "figlet": "^1.8.0", "passport": "^0.7.0", - "passport-http-bearer": "^1.0.1", "passport-jwt": "^4.0.1", "passport-local": "^1.0.0", "pg": "^8.11.5", @@ -67,11 +67,11 @@ "@nestjs/cli": "^10.0.0", "@nestjs/schematics": "^10.0.0", "@nestjs/testing": "^10.3.9", + "@types/cookie-parser": "^1.4.8", "@types/express": "^4.17.21", "@types/figlet": "^1.7.0", "@types/jest": "^29.5.12", "@types/node": "^20.12.12", - "@types/passport-http-bearer": "^1.0.42", "@types/passport-jwt": "^4.0.1", "@types/passport-local": "^1.0.38", "@types/supertest": "^6.0.0", diff --git a/backend/src/main.ts b/backend/src/main.ts index 04c2f9d..7951b2c 100644 --- a/backend/src/main.ts +++ b/backend/src/main.ts @@ -2,9 +2,11 @@ import 'reflect-metadata'; import { NestFactory } from '@nestjs/core'; import { AppModule } from './app.module'; import { Logger, ValidationPipe } from '@nestjs/common'; +import { ConfigService } from '@nestjs/config'; import { SwaggerModule, DocumentBuilder } from '@nestjs/swagger'; import * as figlet from 'figlet'; import * as dotenv from 'dotenv'; +import cookieParser from 'cookie-parser'; import { HttpExceptionFilter } from './common/filters/http-exception.filter'; dotenv.config(); @@ -13,14 +15,23 @@ async function bootstrap() { const app = await NestFactory.create(AppModule); // Application configuration - const port = process.env.PORT || 3001; - const appName = process.env.APP_NAME || 'STATION BACKEND'; + const configService = app.get(ConfigService); + const port = configService.get('PORT') || 3001; + const appName = configService.get('APP_NAME') || 'STATION BACKEND'; // ASCII Art for Application Name console.log(figlet.textSync(appName, { horizontalLayout: 'full' })); - // Enable CORS (if needed for APIs) - app.enableCors(); + // Cookie parser — must be registered before guards that read cookies + app.use(cookieParser()); + + // CORS — allow credentials so httpOnly cookies are sent cross-origin + app.enableCors({ + origin: + configService.get('ALLOWED_ORIGIN') || 'http://localhost:5173', + credentials: true, + methods: ['GET', 'POST', 'PUT', 'PATCH', 'DELETE', 'OPTIONS'], + }); // Global Validation Pipe app.useGlobalPipes( @@ -62,16 +73,6 @@ async function bootstrap() { }, 'access-token', ) - .addBearerAuth( - { - type: 'http', - scheme: 'bearer', - name: 'Refresh Token', - description: 'Enter refresh token', - in: 'header', - }, - 'refresh-token', - ) .build(); const document = SwaggerModule.createDocument(app, config); @@ -81,7 +82,7 @@ async function bootstrap() { // Log application startup information await app.listen(port); Logger.log( - `🚀 Application '${appName}' is running on: http://localhost:${port}`, + `Application '${appName}' is running on: http://localhost:${port}`, 'Bootstrap', ); if (process.env.NODE_ENV !== 'production') { diff --git a/backend/src/modules/auth/auth.controller.ts b/backend/src/modules/auth/auth.controller.ts index 080e06a..4e5db24 100644 --- a/backend/src/modules/auth/auth.controller.ts +++ b/backend/src/modules/auth/auth.controller.ts @@ -1,4 +1,11 @@ -import { Controller, Post, UseGuards, Request, Body } from '@nestjs/common'; +import { + Controller, + Post, + UseGuards, + Request, + Body, + Res, +} from '@nestjs/common'; import { ApiTags, ApiOperation, @@ -11,7 +18,7 @@ import { LocalAuthGuard } from './local-auth.guard'; import { JwtAuthGuard } from './jwt-auth.guard'; import { RefreshTokenAuthGuard } from './refresh-token-auth.guard'; import { UserDto } from '../users/dto/user.dto'; -import { Request as ExpressRequest } from 'express'; +import { Request as ExpressRequest, Response } from 'express'; import { ChangePasswordDto, ForgotPasswordDto, @@ -37,8 +44,29 @@ export class AuthController { @ApiResponse({ status: 401, description: 'Invalid credentials' }) @UseGuards(LocalAuthGuard) @Post('login') - async login(@Request() req: ExpressRequest) { - return this.authService.login(req.user); + async login( + @Request() req: ExpressRequest, + @Res({ passthrough: true }) res: Response, + ) { + const tokens = await this.authService.login( + req.user as Parameters[0], + ); + const isProduction = process.env.NODE_ENV === 'production'; + const cookieBase = { + httpOnly: true, + secure: isProduction, + sameSite: 'lax' as const, + path: '/', + }; + res.cookie('access_token', tokens.accessToken, { + ...cookieBase, + maxAge: 15 * 60 * 1000, + }); + res.cookie('refresh_token', tokens.refreshToken, { + ...cookieBase, + maxAge: 7 * 24 * 60 * 60 * 1000, + }); + return { message: 'Login successful' }; } @ApiOperation({ summary: 'Register new user' }) @@ -50,26 +78,45 @@ export class AuthController { return this.authService.register(userDto); } - @ApiOperation({ summary: 'Refresh access token using refresh token' }) - @ApiBearerAuth('refresh-token') + @ApiOperation({ summary: 'Refresh access token using refresh token cookie' }) @ApiResponse({ status: 200, description: 'Tokens refreshed successfully' }) @ApiResponse({ status: 401, description: 'Invalid or expired refresh token' }) @UseGuards(RefreshTokenAuthGuard) @Post('refresh') - async refresh(@Request() req: any) { - const refreshToken = req.user.refreshToken; - return this.authService.refreshAccessToken(refreshToken); + async refresh( + @Request() req: any, + @Res({ passthrough: true }) res: Response, + ) { + const tokens = await this.authService.refreshAccessToken( + req.user.refreshToken, + ); + const isProduction = process.env.NODE_ENV === 'production'; + const cookieBase = { + httpOnly: true, + secure: isProduction, + sameSite: 'lax' as const, + path: '/', + }; + res.cookie('access_token', tokens.accessToken, { + ...cookieBase, + maxAge: 15 * 60 * 1000, + }); + res.cookie('refresh_token', tokens.refreshToken, { + ...cookieBase, + maxAge: 7 * 24 * 60 * 60 * 1000, + }); + return { message: 'Tokens refreshed successfully' }; } @ApiOperation({ summary: 'Logout user and revoke refresh token' }) - @ApiBearerAuth('refresh-token') @ApiResponse({ status: 200, description: 'Successfully logged out' }) @ApiResponse({ status: 401, description: 'Invalid refresh token' }) @UseGuards(RefreshTokenAuthGuard) @Post('logout') - async logout(@Request() req: any) { - const refreshToken = req.user.refreshToken; - await this.authService.revokeRefreshToken(refreshToken); + async logout(@Request() req: any, @Res({ passthrough: true }) res: Response) { + await this.authService.revokeRefreshToken(req.user.refreshToken); + res.clearCookie('access_token', { path: '/' }); + res.clearCookie('refresh_token', { path: '/' }); return { message: 'Logged out successfully' }; } diff --git a/backend/src/modules/auth/auth.module.ts b/backend/src/modules/auth/auth.module.ts index fb2778c..d20eb70 100644 --- a/backend/src/modules/auth/auth.module.ts +++ b/backend/src/modules/auth/auth.module.ts @@ -6,7 +6,6 @@ import { AuthController } from './auth.controller'; import { AuthService } from './auth.service'; import { LocalStrategy } from './local.strategy'; import { JwtStrategy } from './jwt.strategy'; -import { RefreshTokenStrategy } from './refresh-token.strategy'; import { UsersModule } from '../users/users.module'; import { RefreshToken } from './refresh-token.entity'; import { PasswordReset } from './password-reset.entity'; @@ -27,7 +26,7 @@ import { ConfigModule, ConfigService } from '@nestjs/config'; }), ], controllers: [AuthController], - providers: [AuthService, LocalStrategy, JwtStrategy, RefreshTokenStrategy], + providers: [AuthService, LocalStrategy, JwtStrategy], exports: [AuthService], }) export class AuthModule {} diff --git a/backend/src/modules/auth/auth.service.ts b/backend/src/modules/auth/auth.service.ts index 2917db2..4efc02b 100644 --- a/backend/src/modules/auth/auth.service.ts +++ b/backend/src/modules/auth/auth.service.ts @@ -60,16 +60,13 @@ export class AuthService { } async login( - user: any, - ): Promise<{ access_token: string; refresh_token: string }> { + user: Omit, + ): Promise<{ accessToken: string; refreshToken: string }> { const payload = { username: user.username, sub: user.id }; const accessToken = this.jwtService.sign(payload); const refreshToken = await this.generateRefreshToken(user.id); - return { - access_token: accessToken, - refresh_token: refreshToken, - }; + return { accessToken, refreshToken }; } async register(userDto: UserDto): Promise> { @@ -100,7 +97,7 @@ export class AuthService { async refreshAccessToken( refreshToken: string, - ): Promise<{ access_token: string; refresh_token: string }> { + ): Promise<{ accessToken: string; refreshToken: string }> { // Find the refresh token const storedToken = await this.refreshTokenRepository.findOne({ where: { token: refreshToken }, @@ -132,8 +129,8 @@ export class AuthService { ); return { - access_token: newAccessToken, - refresh_token: newRefreshToken, + accessToken: newAccessToken, + refreshToken: newRefreshToken, }; } diff --git a/backend/src/modules/auth/jwt.strategy.ts b/backend/src/modules/auth/jwt.strategy.ts index 4e0e372..732be7e 100644 --- a/backend/src/modules/auth/jwt.strategy.ts +++ b/backend/src/modules/auth/jwt.strategy.ts @@ -2,18 +2,21 @@ import { Injectable } from '@nestjs/common'; import { PassportStrategy } from '@nestjs/passport'; import { ExtractJwt, Strategy } from 'passport-jwt'; import { ConfigService } from '@nestjs/config'; +import { Request } from 'express'; @Injectable() export class JwtStrategy extends PassportStrategy(Strategy) { constructor(private configService: ConfigService) { super({ - jwtFromRequest: ExtractJwt.fromAuthHeaderAsBearerToken(), + jwtFromRequest: ExtractJwt.fromExtractors([ + (req: Request) => req?.cookies?.access_token ?? null, + ]), ignoreExpiration: false, secretOrKey: configService.get('JWT_SECRET'), }); } - async validate(payload: any) { + async validate(payload: { sub: number; username: string }) { return { userId: payload.sub, username: payload.username }; } } diff --git a/backend/src/modules/auth/refresh-token-auth.guard.ts b/backend/src/modules/auth/refresh-token-auth.guard.ts index e2b8cf1..4e613f5 100644 --- a/backend/src/modules/auth/refresh-token-auth.guard.ts +++ b/backend/src/modules/auth/refresh-token-auth.guard.ts @@ -1,5 +1,20 @@ -import { Injectable } from '@nestjs/common'; -import { AuthGuard } from '@nestjs/passport'; +import { + Injectable, + CanActivate, + ExecutionContext, + UnauthorizedException, +} from '@nestjs/common'; +import { Request } from 'express'; @Injectable() -export class RefreshTokenAuthGuard extends AuthGuard('refresh-token') {} +export class RefreshTokenAuthGuard implements CanActivate { + canActivate(context: ExecutionContext): boolean { + const request = context.switchToHttp().getRequest(); + const refreshToken = request.cookies?.refresh_token; + if (!refreshToken) { + throw new UnauthorizedException('No refresh token provided'); + } + request.user = { refreshToken }; + return true; + } +} diff --git a/backend/src/modules/auth/refresh-token.strategy.ts b/backend/src/modules/auth/refresh-token.strategy.ts deleted file mode 100644 index 4644859..0000000 --- a/backend/src/modules/auth/refresh-token.strategy.ts +++ /dev/null @@ -1,20 +0,0 @@ -import { Injectable } from '@nestjs/common'; -import { PassportStrategy } from '@nestjs/passport'; -import { Strategy } from 'passport-http-bearer'; -import { AuthService } from './auth.service'; - -@Injectable() -export class RefreshTokenStrategy extends PassportStrategy( - Strategy, - 'refresh-token', -) { - constructor(private authService: AuthService) { - super(); - } - - async validate(token: string) { - // The token is already extracted by passport-http-bearer - // We just need to return it so it's available in the request - return { refreshToken: token }; - } -} diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index f6c590f..662e1f9 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -77,6 +77,9 @@ importers: class-validator: specifier: ^0.14.2 version: 0.14.2 + cookie-parser: + specifier: ^1.4.7 + version: 1.4.7 dotenv: specifier: ^16.4.5 version: 16.6.1 @@ -86,9 +89,6 @@ importers: passport: specifier: ^0.7.0 version: 0.7.0 - passport-http-bearer: - specifier: ^1.0.1 - version: 1.0.1 passport-jwt: specifier: ^4.0.1 version: 4.0.1 @@ -123,6 +123,9 @@ importers: '@nestjs/testing': specifier: ^10.3.9 version: 10.4.20(@nestjs/common@10.4.20(class-transformer@0.5.1)(class-validator@0.14.2)(reflect-metadata@0.2.2)(rxjs@7.8.2))(@nestjs/core@10.4.20)(@nestjs/platform-express@10.4.20) + '@types/cookie-parser': + specifier: ^1.4.8 + version: 1.4.10(@types/express@4.17.25) '@types/express': specifier: ^4.17.21 version: 4.17.25 @@ -135,9 +138,6 @@ importers: '@types/node': specifier: ^20.12.12 version: 20.19.25 - '@types/passport-http-bearer': - specifier: ^1.0.42 - version: 1.0.42 '@types/passport-jwt': specifier: ^4.0.1 version: 4.0.1 @@ -1337,9 +1337,6 @@ packages: '@tsconfig/node16@1.0.4': resolution: {integrity: sha512-vxhUy4J8lyeyinH7Azl1pdd43GJhZH/tP2weN8TntQblOY+A0XbT8DJk1/oCPuOOyg/Ja757rG0CgHcWC8OfMA==} - '@types/accepts@1.3.7': - resolution: {integrity: sha512-Pay9fq2lM2wXPWbteBsRAGiWH2hig4ZE2asK+mm7kUzlxRTfL961rj89I6zV/E3PcIkDqyuBEcMxFT7rccugeQ==} - '@types/aria-query@5.0.4': resolution: {integrity: sha512-rfT93uj5s0PRL7EzccGMs3brplhcrghnDoV26NqKhCAS1hVo+WdNsPvE/yb6ilfr5hi2MEk6d5EWJTKdxg8jVw==} @@ -1364,15 +1361,14 @@ packages: '@types/connect@3.4.38': resolution: {integrity: sha512-K6uROf1LD88uDQqJCktA4yzL1YYAK6NgfsI0v/mTgyPKWsX1CnJ0XPSDhViejru1GcRkLWb8RlzFYJRqGUbaug==} - '@types/content-disposition@0.5.9': - resolution: {integrity: sha512-8uYXI3Gw35MhiVYhG3s295oihrxRyytcRHjSjqnqZVDDy/xcGBRny7+Xj1Wgfhv5QzRtN2hB2dVRBUX9XW3UcQ==} + '@types/cookie-parser@1.4.10': + resolution: {integrity: sha512-B4xqkqfZ8Wek+rCOeRxsjMS9OgvzebEzzLYw7NHYuvzb7IdxOkI0ZHGgeEBX4PUM7QGVvNSK60T3OvWj3YfBRg==} + peerDependencies: + '@types/express': '*' '@types/cookiejar@2.1.5': resolution: {integrity: sha512-he+DHOWReW0nghN24E1WUqM0efK4kI9oTqDm6XmK8ZPe2djZ90BSNdGnIyCLzCPw7/pogPlGbzI2wHGGmi4O/Q==} - '@types/cookies@0.9.2': - resolution: {integrity: sha512-1AvkDdZM2dbyFybL4fxpuNCaWyv//0AwsuUk2DWeXyM1/5ZKm6W3z6mQi24RZ4l2ucY+bkSHzbDVpySqPGuV8A==} - '@types/eslint-scope@3.7.7': resolution: {integrity: sha512-MzMFlSLBqNF2gcHWO0G1vP/YQyfvrxZ0bF+u7mzUdZ1/xK4A4sru+nraZz5i3iEIk1l1uyicaDVTB4QbbEkAYg==} @@ -1397,9 +1393,6 @@ packages: '@types/history@4.7.11': resolution: {integrity: sha512-qjDJRrmvBMiTx+jyLxvLfJU7UznFuokDv4f3WRuriHKERccVpFU+8XMQUAbDzoiJCsmexxRExQeMwwCdamSKDA==} - '@types/http-assert@1.5.6': - resolution: {integrity: sha512-TTEwmtjgVbYAzZYWyeHPrrtWnfVkm8tQkP8P21uQifPgMRgjrow3XDEYqucuC8SKZJT7pUnhU/JymvjggxO9vw==} - '@types/http-errors@2.0.5': resolution: {integrity: sha512-r8Tayk8HJnX0FztbZN7oVqGccWgw98T/0neJphO91KkmOzug1KkofZURD4UaD5uH8AqcFLfdPErnBod0u71/qg==} @@ -1424,15 +1417,6 @@ packages: '@types/jsonwebtoken@9.0.5': resolution: {integrity: sha512-VRLSGzik+Unrup6BsouBeHsf4d1hOEgYWTm/7Nmw1sXoN1+tRly/Gy/po3yeahnP4jfnQWWAhQAqcNfH7ngOkA==} - '@types/keygrip@1.0.6': - resolution: {integrity: sha512-lZuNAY9xeJt7Bx4t4dx0rYCDqGPW8RXhQZK1td7d4H6E9zYbLoOtjBvfwdTKpsyxQI/2jv+armjX/RW+ZNpXOQ==} - - '@types/koa-compose@3.2.9': - resolution: {integrity: sha512-BroAZ9FTvPiCy0Pi8tjD1OfJ7bgU1gQf0eR6e1Vm+JJATy9eKOG3hQMFtMciMawiSOVnLMdmUOC46s7HBhSTsA==} - - '@types/koa@3.0.1': - resolution: {integrity: sha512-VkB6WJUQSe0zBpR+Q7/YIUESGp5wPHcaXr0xueU5W0EOUWtlSbblsl+Kl31lyRQ63nIILh0e/7gXjQ09JXJIHw==} - '@types/luxon@3.7.1': resolution: {integrity: sha512-H3iskjFIAn5SlJU7OuxUmTEpebK6TKB8rxZShDslBMZJ5u9S//KM1sbdAisiSrqwLQncVjnpi2OK2J51h+4lsg==} @@ -1451,9 +1435,6 @@ packages: '@types/parse-json@4.0.2': resolution: {integrity: sha512-dISoDXWWQwUquiKsyZ4Ng+HX2KsPL7LyHKHQwgGFEA3IaKac4Obd+h2a/a6waisAoepJlBcx9paWqjA8/HVjCw==} - '@types/passport-http-bearer@1.0.42': - resolution: {integrity: sha512-cGezyf9hy3Cth+zWS779FR9XYhIX/DExsVZURqcSeUU/nhj0Aw8PUhvCyfS35ScwOSd5AFiFhtfWmqHa/2aYZg==} - '@types/passport-jwt@4.0.1': resolution: {integrity: sha512-Y0Ykz6nWP4jpxgEUYq8NoVZeCQPo1ZndJLfapI249g1jHChvRfZRO/LS3tqu26YgAS/laI1qx98sYGz0IalRXQ==} @@ -2061,6 +2042,10 @@ packages: convert-source-map@2.0.0: resolution: {integrity: sha512-Kvp459HrV2FEJ1CAsi1Ku+MY3kasH19TFykTz2xWmMeq6bk2NU3XXvfJ+Q61m0xktWwt+1HSYf3JZsTms3aRJg==} + cookie-parser@1.4.7: + resolution: {integrity: sha512-nGUvgXnotP3BsjiLX2ypbQnWoGUPIIfHQNZkkC668ntrzGWEZVW70HDEB1qnNGMicPje6EttlIgzo51YSwNQGw==} + engines: {node: '>= 0.8.0'} + cookie-signature@1.0.6: resolution: {integrity: sha512-QADzlaHc8icV8I7vbaJXJwod9HWYp8uCqf1xa4OfNu1T7JVxQIrUgOWtHdNDtPiywmFbiS12VjotIXLrKM3orQ==} @@ -2068,6 +2053,10 @@ packages: resolution: {integrity: sha512-6DnInpx7SJ2AK3+CTUE/ZM0vWTUboZCegxhC2xiIydHR9jNuTAASBrfEpHhiGOZw/nX51bHt6YQl8jsGo4y/0w==} engines: {node: '>= 0.6'} + cookie@0.7.2: + resolution: {integrity: sha512-yki5XnKuf750l50uGTllt6kKILY4nQ1eNIQatoXEByZ5dWgnKqbnqmTrBE5B4N7lrMJKQ2ytWMiTO2o0v6Ew/w==} + engines: {node: '>= 0.6'} + cookiejar@2.1.4: resolution: {integrity: sha512-LDx6oHrK+PhzLKJU9j5S7/Y3jM/mUHvD/DeI1WQmJn652iPC5Y4TBzC9l+5OMOXlyTTA+SmVUPm0HQUwpD5Jqw==} @@ -3445,10 +3434,6 @@ packages: resolution: {integrity: sha512-CiyeOxFT/JZyN5m0z9PfXw4SCBJ6Sygz1Dpl0wqjlhDEGGBP1GnsUVEL0p63hoG1fcj3fHynXi9NYO4nWOL+qQ==} engines: {node: '>= 0.8'} - passport-http-bearer@1.0.1: - resolution: {integrity: sha512-SELQM+dOTuMigr9yu8Wo4Fm3ciFfkMq5h/ZQ8ffi4ELgZrX1xh9PlglqZdcUZ1upzJD/whVyt+YWF62s3U6Ipw==} - engines: {node: '>= 0.4.0'} - passport-jwt@4.0.1: resolution: {integrity: sha512-UCKMDYhNuGOBE9/9Ycuoyh7vP6jpeTp/+sfMJl7nLff/t6dps+iaeE0hhNkKN8/HZHcJ7lCdOyDxHdDoxoSvdQ==} @@ -5668,10 +5653,6 @@ snapshots: '@tsconfig/node16@1.0.4': {} - '@types/accepts@1.3.7': - dependencies: - '@types/node': 20.19.25 - '@types/aria-query@5.0.4': {} '@types/babel__core@7.20.5': @@ -5708,16 +5689,11 @@ snapshots: dependencies: '@types/node': 20.19.25 - '@types/content-disposition@0.5.9': {} - - '@types/cookiejar@2.1.5': {} - - '@types/cookies@0.9.2': + '@types/cookie-parser@1.4.10(@types/express@4.17.25)': dependencies: - '@types/connect': 3.4.38 '@types/express': 4.17.25 - '@types/keygrip': 1.0.6 - '@types/node': 20.19.25 + + '@types/cookiejar@2.1.5': {} '@types/eslint-scope@3.7.7': dependencies: @@ -5753,8 +5729,6 @@ snapshots: '@types/history@4.7.11': {} - '@types/http-assert@1.5.6': {} - '@types/http-errors@2.0.5': {} '@types/istanbul-lib-coverage@2.0.6': {} @@ -5783,23 +5757,6 @@ snapshots: dependencies: '@types/node': 20.19.25 - '@types/keygrip@1.0.6': {} - - '@types/koa-compose@3.2.9': - dependencies: - '@types/koa': 3.0.1 - - '@types/koa@3.0.1': - dependencies: - '@types/accepts': 1.3.7 - '@types/content-disposition': 0.5.9 - '@types/cookies': 0.9.2 - '@types/http-assert': 1.5.6 - '@types/http-errors': 2.0.5 - '@types/keygrip': 1.0.6 - '@types/koa-compose': 3.2.9 - '@types/node': 20.19.25 - '@types/luxon@3.7.1': {} '@types/methods@1.1.4': {} @@ -5814,12 +5771,6 @@ snapshots: '@types/parse-json@4.0.2': {} - '@types/passport-http-bearer@1.0.42': - dependencies: - '@types/express': 4.17.25 - '@types/koa': 3.0.1 - '@types/passport': 1.0.17 - '@types/passport-jwt@4.0.1': dependencies: '@types/jsonwebtoken': 9.0.10 @@ -6541,10 +6492,17 @@ snapshots: convert-source-map@2.0.0: {} + cookie-parser@1.4.7: + dependencies: + cookie: 0.7.2 + cookie-signature: 1.0.6 + cookie-signature@1.0.6: {} cookie@0.7.1: {} + cookie@0.7.2: {} + cookiejar@2.1.4: {} core-util-is@1.0.3: {} @@ -8183,10 +8141,6 @@ snapshots: parseurl@1.3.3: {} - passport-http-bearer@1.0.1: - dependencies: - passport-strategy: 1.0.0 - passport-jwt@4.0.1: dependencies: jsonwebtoken: 9.0.2 From 5bca8c16fa0fd89c40d7b5c03487bb74e71e4a57 Mon Sep 17 00:00:00 2001 From: Demian Date: Sun, 12 Apr 2026 22:43:50 -0400 Subject: [PATCH 2/8] fix: address PR review comments on httpOnly cookie implementation Controller: - Extract cookie options into private cookieOptions() to avoid duplication - Switch sameSite from 'lax' to 'strict' to minimize CSRF surface - clearCookie now passes full options (httpOnly, secure, sameSite) to ensure consistent removal across deployments - login returns { message, username } so clients know who is logged in JwtStrategy: - Add Authorization: Bearer as fallback extractor so Swagger and API clients continue to work alongside cookie-based browser clients E2E tests: - Register cookie-parser middleware in all test app instances - Extract access_token from Set-Cookie response header after login - Replace Authorization: Bearer with Cookie header across all specs --- backend/src/modules/auth/auth.controller.ts | 75 ++++++++++--------- backend/src/modules/auth/jwt.strategy.ts | 3 + backend/test/auth-password-reset.e2e-spec.ts | 21 ++++-- backend/test/organizations.e2e-spec.ts | 29 ++++--- backend/test/roles.e2e-spec.ts | 25 ++++--- .../test/user-organization-roles.e2e-spec.ts | 41 +++++----- 6 files changed, 111 insertions(+), 83 deletions(-) diff --git a/backend/src/modules/auth/auth.controller.ts b/backend/src/modules/auth/auth.controller.ts index 4e5db24..75c6a68 100644 --- a/backend/src/modules/auth/auth.controller.ts +++ b/backend/src/modules/auth/auth.controller.ts @@ -30,6 +30,16 @@ import { export class AuthController { constructor(private authService: AuthService) {} + private cookieOptions(maxAge: number) { + return { + httpOnly: true, + secure: process.env.NODE_ENV === 'production', + sameSite: 'strict' as const, + path: '/', + maxAge, + }; + } + @ApiOperation({ summary: 'Login user' }) @ApiBody({ schema: { @@ -48,25 +58,19 @@ export class AuthController { @Request() req: ExpressRequest, @Res({ passthrough: true }) res: Response, ) { - const tokens = await this.authService.login( - req.user as Parameters[0], + const user = req.user as Parameters[0]; + const tokens = await this.authService.login(user); + res.cookie( + 'access_token', + tokens.accessToken, + this.cookieOptions(15 * 60 * 1000), ); - const isProduction = process.env.NODE_ENV === 'production'; - const cookieBase = { - httpOnly: true, - secure: isProduction, - sameSite: 'lax' as const, - path: '/', - }; - res.cookie('access_token', tokens.accessToken, { - ...cookieBase, - maxAge: 15 * 60 * 1000, - }); - res.cookie('refresh_token', tokens.refreshToken, { - ...cookieBase, - maxAge: 7 * 24 * 60 * 60 * 1000, - }); - return { message: 'Login successful' }; + res.cookie( + 'refresh_token', + tokens.refreshToken, + this.cookieOptions(7 * 24 * 60 * 60 * 1000), + ); + return { message: 'Login successful', username: user.username }; } @ApiOperation({ summary: 'Register new user' }) @@ -90,21 +94,16 @@ export class AuthController { const tokens = await this.authService.refreshAccessToken( req.user.refreshToken, ); - const isProduction = process.env.NODE_ENV === 'production'; - const cookieBase = { - httpOnly: true, - secure: isProduction, - sameSite: 'lax' as const, - path: '/', - }; - res.cookie('access_token', tokens.accessToken, { - ...cookieBase, - maxAge: 15 * 60 * 1000, - }); - res.cookie('refresh_token', tokens.refreshToken, { - ...cookieBase, - maxAge: 7 * 24 * 60 * 60 * 1000, - }); + res.cookie( + 'access_token', + tokens.accessToken, + this.cookieOptions(15 * 60 * 1000), + ); + res.cookie( + 'refresh_token', + tokens.refreshToken, + this.cookieOptions(7 * 24 * 60 * 60 * 1000), + ); return { message: 'Tokens refreshed successfully' }; } @@ -115,8 +114,14 @@ export class AuthController { @Post('logout') async logout(@Request() req: any, @Res({ passthrough: true }) res: Response) { await this.authService.revokeRefreshToken(req.user.refreshToken); - res.clearCookie('access_token', { path: '/' }); - res.clearCookie('refresh_token', { path: '/' }); + const clearOpts = { + httpOnly: true, + secure: process.env.NODE_ENV === 'production', + sameSite: 'strict' as const, + path: '/', + }; + res.clearCookie('access_token', clearOpts); + res.clearCookie('refresh_token', clearOpts); return { message: 'Logged out successfully' }; } diff --git a/backend/src/modules/auth/jwt.strategy.ts b/backend/src/modules/auth/jwt.strategy.ts index 732be7e..97a4646 100644 --- a/backend/src/modules/auth/jwt.strategy.ts +++ b/backend/src/modules/auth/jwt.strategy.ts @@ -9,7 +9,10 @@ export class JwtStrategy extends PassportStrategy(Strategy) { constructor(private configService: ConfigService) { super({ jwtFromRequest: ExtractJwt.fromExtractors([ + // Prefer httpOnly cookie (browser clients) (req: Request) => req?.cookies?.access_token ?? null, + // Fallback to Authorization: Bearer header (Swagger / API clients) + ExtractJwt.fromAuthHeaderAsBearerToken(), ]), ignoreExpiration: false, secretOrKey: configService.get('JWT_SECRET'), diff --git a/backend/test/auth-password-reset.e2e-spec.ts b/backend/test/auth-password-reset.e2e-spec.ts index 77b5def..5041766 100644 --- a/backend/test/auth-password-reset.e2e-spec.ts +++ b/backend/test/auth-password-reset.e2e-spec.ts @@ -1,6 +1,7 @@ import { Test, TestingModule } from '@nestjs/testing'; import { INestApplication, ValidationPipe } from '@nestjs/common'; import request from 'supertest'; +import cookieParser from 'cookie-parser'; import { AppModule } from '../src/app.module'; import { DataSource } from 'typeorm'; import { User } from '../src/modules/users/user.entity'; @@ -12,7 +13,7 @@ describe('Auth - Password Reset (e2e)', () => { let app: INestApplication; let dataSource: DataSource; let testUser: User; - let accessToken: string; + let authCookie: string; beforeAll(async () => { const moduleFixture: TestingModule = await Test.createTestingModule({ @@ -20,6 +21,7 @@ describe('Auth - Password Reset (e2e)', () => { }).compile(); app = moduleFixture.createNestApplication(); + app.use(cookieParser()); app.useGlobalPipes( new ValidationPipe({ whitelist: true, @@ -53,7 +55,10 @@ describe('Auth - Password Reset (e2e)', () => { }) .expect(201); - accessToken = loginResponse.body.access_token; + const setCookies = loginResponse.headers['set-cookie'] as string[]; + authCookie = + setCookies.find((c) => c.startsWith('access_token='))?.split(';')[0] ?? + ''; }); afterAll(async () => { @@ -252,7 +257,7 @@ describe('Auth - Password Reset (e2e)', () => { const response = await request(app.getHttpServer()) .post('/auth/change-password') - .set('Authorization', `Bearer ${accessToken}`) + .set('Cookie', authCookie) .send({ currentPassword, newPassword, @@ -279,7 +284,7 @@ describe('Auth - Password Reset (e2e)', () => { it('should reject incorrect current password', async () => { await request(app.getHttpServer()) .post('/auth/change-password') - .set('Authorization', `Bearer ${accessToken}`) + .set('Cookie', authCookie) .send({ currentPassword: 'wrongPassword', newPassword: 'newPassword789', @@ -300,7 +305,7 @@ describe('Auth - Password Reset (e2e)', () => { it('should reject invalid access token', async () => { await request(app.getHttpServer()) .post('/auth/change-password') - .set('Authorization', 'Bearer invalid-token') + .set('Cookie', 'access_token=invalid-token') .send({ currentPassword: 'password123', newPassword: 'newPassword789', @@ -311,7 +316,7 @@ describe('Auth - Password Reset (e2e)', () => { it('should reject missing currentPassword', async () => { await request(app.getHttpServer()) .post('/auth/change-password') - .set('Authorization', `Bearer ${accessToken}`) + .set('Cookie', authCookie) .send({ newPassword: 'newPassword789', }) @@ -321,7 +326,7 @@ describe('Auth - Password Reset (e2e)', () => { it('should reject missing newPassword', async () => { await request(app.getHttpServer()) .post('/auth/change-password') - .set('Authorization', `Bearer ${accessToken}`) + .set('Cookie', authCookie) .send({ currentPassword: 'password123', }) @@ -331,7 +336,7 @@ describe('Auth - Password Reset (e2e)', () => { it('should reject newPassword shorter than 6 characters', async () => { await request(app.getHttpServer()) .post('/auth/change-password') - .set('Authorization', `Bearer ${accessToken}`) + .set('Cookie', authCookie) .send({ currentPassword: 'password123', newPassword: '12345', diff --git a/backend/test/organizations.e2e-spec.ts b/backend/test/organizations.e2e-spec.ts index 415d6d6..49fd789 100644 --- a/backend/test/organizations.e2e-spec.ts +++ b/backend/test/organizations.e2e-spec.ts @@ -1,6 +1,7 @@ import { Test, TestingModule } from '@nestjs/testing'; import { INestApplication, ValidationPipe } from '@nestjs/common'; import request from 'supertest'; +import cookieParser from 'cookie-parser'; import { AppModule } from '../src/app.module'; import { DatabaseSeederService } from '../src/database/seeds/database-seeder.service'; import { DataSource } from 'typeorm'; @@ -8,7 +9,7 @@ import { seedSystemUser } from './helpers/seed-system-user'; describe('Organizations (e2e)', () => { let app: INestApplication; - let authToken: string; + let authCookie: string; let createdOrgId: number; beforeAll(async () => { @@ -17,6 +18,7 @@ describe('Organizations (e2e)', () => { }).compile(); app = moduleFixture.createNestApplication(); + app.use(cookieParser()); app.useGlobalPipes(new ValidationPipe()); await app.init(); @@ -44,7 +46,10 @@ describe('Organizations (e2e)', () => { password: 'password123', }); - authToken = loginResponse.body.access_token; + const setCookies = loginResponse.headers['set-cookie'] as string[]; + authCookie = + setCookies.find((c) => c.startsWith('access_token='))?.split(';')[0] ?? + ''; }); afterAll(async () => { @@ -55,7 +60,7 @@ describe('Organizations (e2e)', () => { it('should create a new organization', () => { return request(app.getHttpServer()) .post('/organizations') - .set('Authorization', `Bearer ${authToken}`) + .set('Cookie', authCookie) .send({ name: 'Test Corp', description: 'A test organization', @@ -83,7 +88,7 @@ describe('Organizations (e2e)', () => { it('should create organization with minimal data', () => { return request(app.getHttpServer()) .post('/organizations') - .set('Authorization', `Bearer ${authToken}`) + .set('Cookie', authCookie) .send({ name: 'Minimal Org', }) @@ -99,7 +104,7 @@ describe('Organizations (e2e)', () => { it('should return all active organizations', () => { return request(app.getHttpServer()) .get('/organizations') - .set('Authorization', `Bearer ${authToken}`) + .set('Cookie', authCookie) .expect(200) .then((response) => { expect(Array.isArray(response.body)).toBe(true); @@ -112,7 +117,7 @@ describe('Organizations (e2e)', () => { it('should return a specific organization', () => { return request(app.getHttpServer()) .get(`/organizations/${createdOrgId}`) - .set('Authorization', `Bearer ${authToken}`) + .set('Cookie', authCookie) .expect(200) .then((response) => { expect(response.body.id).toBe(createdOrgId); @@ -123,7 +128,7 @@ describe('Organizations (e2e)', () => { it('should return 404 for non-existent organization', () => { return request(app.getHttpServer()) .get('/organizations/99999') - .set('Authorization', `Bearer ${authToken}`) + .set('Cookie', authCookie) .expect(404); }); }); @@ -132,7 +137,7 @@ describe('Organizations (e2e)', () => { it('should return organization with members', () => { return request(app.getHttpServer()) .get(`/organizations/${createdOrgId}/members`) - .set('Authorization', `Bearer ${authToken}`) + .set('Cookie', authCookie) .expect(200) .then((response) => { expect(response.body).toHaveProperty('id'); @@ -146,7 +151,7 @@ describe('Organizations (e2e)', () => { it('should update an organization', () => { return request(app.getHttpServer()) .put(`/organizations/${createdOrgId}`) - .set('Authorization', `Bearer ${authToken}`) + .set('Cookie', authCookie) .send({ description: 'Updated organization description', isActive: true, @@ -162,7 +167,7 @@ describe('Organizations (e2e)', () => { it('should deactivate an organization', () => { return request(app.getHttpServer()) .put(`/organizations/${createdOrgId}`) - .set('Authorization', `Bearer ${authToken}`) + .set('Cookie', authCookie) .send({ isActive: false, }) @@ -177,14 +182,14 @@ describe('Organizations (e2e)', () => { it('should delete an organization', () => { return request(app.getHttpServer()) .delete(`/organizations/${createdOrgId}`) - .set('Authorization', `Bearer ${authToken}`) + .set('Cookie', authCookie) .expect(204); }); it('should return 404 when deleting non-existent organization', () => { return request(app.getHttpServer()) .delete('/organizations/99999') - .set('Authorization', `Bearer ${authToken}`) + .set('Cookie', authCookie) .expect(404); }); }); diff --git a/backend/test/roles.e2e-spec.ts b/backend/test/roles.e2e-spec.ts index 6eecf38..79dd3db 100644 --- a/backend/test/roles.e2e-spec.ts +++ b/backend/test/roles.e2e-spec.ts @@ -1,13 +1,14 @@ import { Test, TestingModule } from '@nestjs/testing'; import { INestApplication, ValidationPipe } from '@nestjs/common'; import request from 'supertest'; +import cookieParser from 'cookie-parser'; import { AppModule } from '../src/app.module'; import { DataSource } from 'typeorm'; import { seedSystemUser } from './helpers/seed-system-user'; describe('Roles (e2e)', () => { let app: INestApplication; - let authToken: string; + let authCookie: string; let createdRoleId: number; beforeAll(async () => { @@ -16,6 +17,7 @@ describe('Roles (e2e)', () => { }).compile(); app = moduleFixture.createNestApplication(); + app.use(cookieParser()); app.useGlobalPipes(new ValidationPipe()); await app.init(); @@ -37,7 +39,10 @@ describe('Roles (e2e)', () => { password: 'password123', }); - authToken = loginResponse.body.access_token; + const setCookies = loginResponse.headers['set-cookie'] as string[]; + authCookie = + setCookies.find((c) => c.startsWith('access_token='))?.split(';')[0] ?? + ''; }); afterAll(async () => { @@ -48,7 +53,7 @@ describe('Roles (e2e)', () => { it('should create a new role', () => { return request(app.getHttpServer()) .post('/roles') - .set('Authorization', `Bearer ${authToken}`) + .set('Cookie', authCookie) .send({ name: 'Test Admin', description: 'Test administrator role', @@ -73,7 +78,7 @@ describe('Roles (e2e)', () => { it('should fail to create role with duplicate name', () => { return request(app.getHttpServer()) .post('/roles') - .set('Authorization', `Bearer ${authToken}`) + .set('Cookie', authCookie) .send({ name: 'Test Admin', description: 'Duplicate role', @@ -95,7 +100,7 @@ describe('Roles (e2e)', () => { it('should return all roles', () => { return request(app.getHttpServer()) .get('/roles') - .set('Authorization', `Bearer ${authToken}`) + .set('Cookie', authCookie) .expect(200) .then((response) => { expect(Array.isArray(response.body)).toBe(true); @@ -108,7 +113,7 @@ describe('Roles (e2e)', () => { it('should return a specific role', () => { return request(app.getHttpServer()) .get(`/roles/${createdRoleId}`) - .set('Authorization', `Bearer ${authToken}`) + .set('Cookie', authCookie) .expect(200) .then((response) => { expect(response.body.id).toBe(createdRoleId); @@ -119,7 +124,7 @@ describe('Roles (e2e)', () => { it('should return 404 for non-existent role', () => { return request(app.getHttpServer()) .get('/roles/99999') - .set('Authorization', `Bearer ${authToken}`) + .set('Cookie', authCookie) .expect(404); }); }); @@ -128,7 +133,7 @@ describe('Roles (e2e)', () => { it('should update a role', () => { return request(app.getHttpServer()) .put(`/roles/${createdRoleId}`) - .set('Authorization', `Bearer ${authToken}`) + .set('Cookie', authCookie) .send({ description: 'Updated description', permissions: { @@ -149,14 +154,14 @@ describe('Roles (e2e)', () => { it('should delete a role', () => { return request(app.getHttpServer()) .delete(`/roles/${createdRoleId}`) - .set('Authorization', `Bearer ${authToken}`) + .set('Cookie', authCookie) .expect(204); }); it('should return 404 when deleting non-existent role', () => { return request(app.getHttpServer()) .delete('/roles/99999') - .set('Authorization', `Bearer ${authToken}`) + .set('Cookie', authCookie) .expect(404); }); }); diff --git a/backend/test/user-organization-roles.e2e-spec.ts b/backend/test/user-organization-roles.e2e-spec.ts index 1fa76f1..4fe90d1 100644 --- a/backend/test/user-organization-roles.e2e-spec.ts +++ b/backend/test/user-organization-roles.e2e-spec.ts @@ -1,6 +1,7 @@ import { Test, TestingModule } from '@nestjs/testing'; import { INestApplication, ValidationPipe } from '@nestjs/common'; import request from 'supertest'; +import cookieParser from 'cookie-parser'; import { AppModule } from '../src/app.module'; import { DatabaseSeederService } from '../src/database/seeds/database-seeder.service'; import { DataSource } from 'typeorm'; @@ -8,7 +9,7 @@ import { seedSystemUser } from './helpers/seed-system-user'; describe('UserOrganizationRoles (e2e)', () => { let app: INestApplication; - let authToken: string; + let authCookie: string; let userId: number; let organizationId: number; let roleId: number; @@ -19,6 +20,7 @@ describe('UserOrganizationRoles (e2e)', () => { }).compile(); app = moduleFixture.createNestApplication(); + app.use(cookieParser()); app.useGlobalPipes(new ValidationPipe()); await app.init(); @@ -46,13 +48,16 @@ describe('UserOrganizationRoles (e2e)', () => { password: 'password123', }); - authToken = loginResponse.body.access_token; + const setCookies = loginResponse.headers['set-cookie'] as string[]; + authCookie = + setCookies.find((c) => c.startsWith('access_token='))?.split(';')[0] ?? + ''; userId = loginResponse.body.userId || 1; // Create a test organization const orgResponse = await request(app.getHttpServer()) .post('/organizations') - .set('Authorization', `Bearer ${authToken}`) + .set('Cookie', authCookie) .send({ name: 'Test Organization', description: 'For role testing', @@ -63,7 +68,7 @@ describe('UserOrganizationRoles (e2e)', () => { // Create a test role const roleResponse = await request(app.getHttpServer()) .post('/roles') - .set('Authorization', `Bearer ${authToken}`) + .set('Cookie', authCookie) .send({ name: 'Test Role', permissions: { @@ -83,7 +88,7 @@ describe('UserOrganizationRoles (e2e)', () => { it('should assign a role to a user in an organization', () => { return request(app.getHttpServer()) .post('/user-organization-roles/assign') - .set('Authorization', `Bearer ${authToken}`) + .set('Cookie', authCookie) .send({ userId, organizationId, @@ -101,7 +106,7 @@ describe('UserOrganizationRoles (e2e)', () => { it('should fail to assign duplicate role', () => { return request(app.getHttpServer()) .post('/user-organization-roles/assign') - .set('Authorization', `Bearer ${authToken}`) + .set('Cookie', authCookie) .send({ userId, organizationId, @@ -113,7 +118,7 @@ describe('UserOrganizationRoles (e2e)', () => { it('should fail with invalid user', () => { return request(app.getHttpServer()) .post('/user-organization-roles/assign') - .set('Authorization', `Bearer ${authToken}`) + .set('Cookie', authCookie) .send({ userId: 99999, organizationId, @@ -128,7 +133,7 @@ describe('UserOrganizationRoles (e2e)', () => { // Create additional roles const role2Response = await request(app.getHttpServer()) .post('/roles') - .set('Authorization', `Bearer ${authToken}`) + .set('Cookie', authCookie) .send({ name: 'Developer Role', permissions: { canDeploy: true }, @@ -136,7 +141,7 @@ describe('UserOrganizationRoles (e2e)', () => { const role3Response = await request(app.getHttpServer()) .post('/roles') - .set('Authorization', `Bearer ${authToken}`) + .set('Cookie', authCookie) .send({ name: 'Viewer Role', permissions: { canView: true }, @@ -144,7 +149,7 @@ describe('UserOrganizationRoles (e2e)', () => { return request(app.getHttpServer()) .post('/user-organization-roles/assign-multiple') - .set('Authorization', `Bearer ${authToken}`) + .set('Cookie', authCookie) .send({ userId, organizationId, @@ -164,7 +169,7 @@ describe('UserOrganizationRoles (e2e)', () => { .get( `/user-organization-roles/user/${userId}/organization/${organizationId}`, ) - .set('Authorization', `Bearer ${authToken}`) + .set('Cookie', authCookie) .expect(200) .then((response) => { expect(Array.isArray(response.body)).toBe(true); @@ -178,7 +183,7 @@ describe('UserOrganizationRoles (e2e)', () => { it('should get all organizations for a user', () => { return request(app.getHttpServer()) .get(`/user-organization-roles/user/${userId}/organizations`) - .set('Authorization', `Bearer ${authToken}`) + .set('Cookie', authCookie) .expect(200) .then((response) => { expect(Array.isArray(response.body)).toBe(true); @@ -193,7 +198,7 @@ describe('UserOrganizationRoles (e2e)', () => { it('should get all members of an organization', () => { return request(app.getHttpServer()) .get(`/user-organization-roles/organization/${organizationId}/members`) - .set('Authorization', `Bearer ${authToken}`) + .set('Cookie', authCookie) .expect(200) .then((response) => { expect(Array.isArray(response.body)).toBe(true); @@ -210,7 +215,7 @@ describe('UserOrganizationRoles (e2e)', () => { .get( `/user-organization-roles/organization/${organizationId}/role/${roleId}/users`, ) - .set('Authorization', `Bearer ${authToken}`) + .set('Cookie', authCookie) .expect(200) .then((response) => { expect(Array.isArray(response.body)).toBe(true); @@ -226,7 +231,7 @@ describe('UserOrganizationRoles (e2e)', () => { .delete( `/user-organization-roles/user/${userId}/organization/${organizationId}/role/${roleId}`, ) - .set('Authorization', `Bearer ${authToken}`) + .set('Cookie', authCookie) .expect(204); }); @@ -235,7 +240,7 @@ describe('UserOrganizationRoles (e2e)', () => { .delete( `/user-organization-roles/user/${userId}/organization/${organizationId}/role/99999`, ) - .set('Authorization', `Bearer ${authToken}`) + .set('Cookie', authCookie) .expect(404); }); }); @@ -245,7 +250,7 @@ describe('UserOrganizationRoles (e2e)', () => { // First assign a role await request(app.getHttpServer()) .post('/user-organization-roles/assign') - .set('Authorization', `Bearer ${authToken}`) + .set('Cookie', authCookie) .send({ userId, organizationId, @@ -254,7 +259,7 @@ describe('UserOrganizationRoles (e2e)', () => { return request(app.getHttpServer()) .get(`/permissions/user/${userId}/organization/${organizationId}`) - .set('Authorization', `Bearer ${authToken}`) + .set('Cookie', authCookie) .expect(200) .then((response) => { expect(response.body).toHaveProperty('permissions'); From 67076ef53b24e8a61b296448e1b378973e1f28a4 Mon Sep 17 00:00:00 2001 From: Demian Date: Sun, 12 Apr 2026 23:40:53 -0400 Subject: [PATCH 3/8] fix: cast set-cookie header through unknown to satisfy strict TS supertest types headers as { [key: string]: string } so a direct cast to string[] fails TS2352. Going through unknown is the correct escape hatch for this known library type gap. --- backend/test/auth-password-reset.e2e-spec.ts | 4 +++- backend/test/organizations.e2e-spec.ts | 4 +++- backend/test/roles.e2e-spec.ts | 4 +++- backend/test/user-organization-roles.e2e-spec.ts | 4 +++- 4 files changed, 12 insertions(+), 4 deletions(-) diff --git a/backend/test/auth-password-reset.e2e-spec.ts b/backend/test/auth-password-reset.e2e-spec.ts index 5041766..3c9f23e 100644 --- a/backend/test/auth-password-reset.e2e-spec.ts +++ b/backend/test/auth-password-reset.e2e-spec.ts @@ -55,7 +55,9 @@ describe('Auth - Password Reset (e2e)', () => { }) .expect(201); - const setCookies = loginResponse.headers['set-cookie'] as string[]; + const setCookies = loginResponse.headers[ + 'set-cookie' + ] as unknown as string[]; authCookie = setCookies.find((c) => c.startsWith('access_token='))?.split(';')[0] ?? ''; diff --git a/backend/test/organizations.e2e-spec.ts b/backend/test/organizations.e2e-spec.ts index 49fd789..f606b59 100644 --- a/backend/test/organizations.e2e-spec.ts +++ b/backend/test/organizations.e2e-spec.ts @@ -46,7 +46,9 @@ describe('Organizations (e2e)', () => { password: 'password123', }); - const setCookies = loginResponse.headers['set-cookie'] as string[]; + const setCookies = loginResponse.headers[ + 'set-cookie' + ] as unknown as string[]; authCookie = setCookies.find((c) => c.startsWith('access_token='))?.split(';')[0] ?? ''; diff --git a/backend/test/roles.e2e-spec.ts b/backend/test/roles.e2e-spec.ts index 79dd3db..5abd305 100644 --- a/backend/test/roles.e2e-spec.ts +++ b/backend/test/roles.e2e-spec.ts @@ -39,7 +39,9 @@ describe('Roles (e2e)', () => { password: 'password123', }); - const setCookies = loginResponse.headers['set-cookie'] as string[]; + const setCookies = loginResponse.headers[ + 'set-cookie' + ] as unknown as string[]; authCookie = setCookies.find((c) => c.startsWith('access_token='))?.split(';')[0] ?? ''; diff --git a/backend/test/user-organization-roles.e2e-spec.ts b/backend/test/user-organization-roles.e2e-spec.ts index 4fe90d1..6c528de 100644 --- a/backend/test/user-organization-roles.e2e-spec.ts +++ b/backend/test/user-organization-roles.e2e-spec.ts @@ -48,7 +48,9 @@ describe('UserOrganizationRoles (e2e)', () => { password: 'password123', }); - const setCookies = loginResponse.headers['set-cookie'] as string[]; + const setCookies = loginResponse.headers[ + 'set-cookie' + ] as unknown as string[]; authCookie = setCookies.find((c) => c.startsWith('access_token='))?.split(';')[0] ?? ''; From 87ae8cd71455ab79a420a4b1f60a8a48ced4846b Mon Sep 17 00:00:00 2001 From: Demian Date: Mon, 13 Apr 2026 00:25:50 -0400 Subject: [PATCH 4/8] fix: address review comments on cookie auth controller and e2e test - Derive clearCookie options from cookieOptions() helper by destructuring out maxAge, eliminating duplicated security-critical flags in logout - Align @ApiBearerAuth decorator with Swagger scheme name 'access-token' so Swagger UI sends the Authorization header on secured endpoints - Capture userId from /auth/register response instead of login body since login no longer returns userId (tokens moved to httpOnly cookies) --- backend/src/modules/auth/auth.controller.ts | 9 ++------- backend/test/user-organization-roles.e2e-spec.ts | 15 +++++++++------ 2 files changed, 11 insertions(+), 13 deletions(-) diff --git a/backend/src/modules/auth/auth.controller.ts b/backend/src/modules/auth/auth.controller.ts index 75c6a68..1fd220f 100644 --- a/backend/src/modules/auth/auth.controller.ts +++ b/backend/src/modules/auth/auth.controller.ts @@ -114,12 +114,7 @@ export class AuthController { @Post('logout') async logout(@Request() req: any, @Res({ passthrough: true }) res: Response) { await this.authService.revokeRefreshToken(req.user.refreshToken); - const clearOpts = { - httpOnly: true, - secure: process.env.NODE_ENV === 'production', - sameSite: 'strict' as const, - path: '/', - }; + const { maxAge: _maxAge, ...clearOpts } = this.cookieOptions(0); res.clearCookie('access_token', clearOpts); res.clearCookie('refresh_token', clearOpts); return { message: 'Logged out successfully' }; @@ -148,7 +143,7 @@ export class AuthController { } @ApiOperation({ summary: 'Change password (requires authentication)' }) - @ApiBearerAuth() + @ApiBearerAuth('access-token') @ApiBody({ type: ChangePasswordDto }) @ApiResponse({ status: 200, description: 'Password changed successfully' }) @ApiResponse({ status: 400, description: 'Current password is incorrect' }) diff --git a/backend/test/user-organization-roles.e2e-spec.ts b/backend/test/user-organization-roles.e2e-spec.ts index 6c528de..980cf92 100644 --- a/backend/test/user-organization-roles.e2e-spec.ts +++ b/backend/test/user-organization-roles.e2e-spec.ts @@ -35,11 +35,15 @@ describe('UserOrganizationRoles (e2e)', () => { await seeder.seedAll(); // Register and login a test user - await request(app.getHttpServer()).post('/auth/register').send({ - username: 'roleuser', - email: 'roleuser@example.com', - password: 'password123', - }); + const registerResponse = await request(app.getHttpServer()) + .post('/auth/register') + .send({ + username: 'roleuser', + email: 'roleuser@example.com', + password: 'password123', + }); + + userId = registerResponse.body.id; const loginResponse = await request(app.getHttpServer()) .post('/auth/login') @@ -54,7 +58,6 @@ describe('UserOrganizationRoles (e2e)', () => { authCookie = setCookies.find((c) => c.startsWith('access_token='))?.split(';')[0] ?? ''; - userId = loginResponse.body.userId || 1; // Create a test organization const orgResponse = await request(app.getHttpServer()) From cf830f73e268877d11aa1651a51219185eafb5ad Mon Sep 17 00:00:00 2001 From: Demian Date: Mon, 13 Apr 2026 00:50:33 -0400 Subject: [PATCH 5/8] fix: address review comments on port type and e2e test assertions - Read PORT as number via configService.get() so app.listen() receives a proper TCP port; Joi schema already coerces it to a number - Add .expect(201) to login calls in roles and organizations e2e tests so cookie-extraction failures are caught at the right step - Add .expect(201) + expect(id).toBeDefined() to register call in user-organization-roles e2e test to catch registration failures early - Add .expect(201) to org and role creation in beforeAll so auth regressions surface at the setup step rather than as undefined IDs --- backend/src/main.ts | 2 +- backend/test/organizations.e2e-spec.ts | 3 ++- backend/test/roles.e2e-spec.ts | 3 ++- backend/test/user-organization-roles.e2e-spec.ts | 10 +++++++--- 4 files changed, 12 insertions(+), 6 deletions(-) diff --git a/backend/src/main.ts b/backend/src/main.ts index 7951b2c..0fab9c4 100644 --- a/backend/src/main.ts +++ b/backend/src/main.ts @@ -16,7 +16,7 @@ async function bootstrap() { // Application configuration const configService = app.get(ConfigService); - const port = configService.get('PORT') || 3001; + const port = configService.get('PORT') ?? 3001; const appName = configService.get('APP_NAME') || 'STATION BACKEND'; // ASCII Art for Application Name diff --git a/backend/test/organizations.e2e-spec.ts b/backend/test/organizations.e2e-spec.ts index f606b59..7ec3675 100644 --- a/backend/test/organizations.e2e-spec.ts +++ b/backend/test/organizations.e2e-spec.ts @@ -44,7 +44,8 @@ describe('Organizations (e2e)', () => { .send({ username: 'orguser', password: 'password123', - }); + }) + .expect(201); const setCookies = loginResponse.headers[ 'set-cookie' diff --git a/backend/test/roles.e2e-spec.ts b/backend/test/roles.e2e-spec.ts index 5abd305..649079d 100644 --- a/backend/test/roles.e2e-spec.ts +++ b/backend/test/roles.e2e-spec.ts @@ -37,7 +37,8 @@ describe('Roles (e2e)', () => { .send({ username: 'testuser', password: 'password123', - }); + }) + .expect(201); const setCookies = loginResponse.headers[ 'set-cookie' diff --git a/backend/test/user-organization-roles.e2e-spec.ts b/backend/test/user-organization-roles.e2e-spec.ts index 980cf92..94bad08 100644 --- a/backend/test/user-organization-roles.e2e-spec.ts +++ b/backend/test/user-organization-roles.e2e-spec.ts @@ -41,8 +41,10 @@ describe('UserOrganizationRoles (e2e)', () => { username: 'roleuser', email: 'roleuser@example.com', password: 'password123', - }); + }) + .expect(201); + expect(registerResponse.body.id).toBeDefined(); userId = registerResponse.body.id; const loginResponse = await request(app.getHttpServer()) @@ -66,7 +68,8 @@ describe('UserOrganizationRoles (e2e)', () => { .send({ name: 'Test Organization', description: 'For role testing', - }); + }) + .expect(201); organizationId = orgResponse.body.id; @@ -80,7 +83,8 @@ describe('UserOrganizationRoles (e2e)', () => { canEdit: true, canDelete: false, }, - }); + }) + .expect(201); roleId = roleResponse.body.id; }); From 472247b1fe32a295cde3e9f1e1c699a22818bd7c Mon Sep 17 00:00:00 2001 From: Demian Date: Mon, 13 Apr 2026 01:27:14 -0400 Subject: [PATCH 6/8] fix: address PR117 review comments - Add @HttpCode(HttpStatus.OK) to all auth POST endpoints that document 200 (login, refresh, logout, forgot/reset/change-password) so Swagger docs match actual HTTP status codes; update e2e expectations - Add explicit set-cookie header and access_token cookie assertions in roles and organizations e2e beforeAll to surface cookie auth failures early with clear messages rather than silent empty authCookie --- backend/src/modules/auth/auth.controller.ts | 8 +++++++ backend/test/auth-password-reset.e2e-spec.ts | 23 +++++++++++--------- backend/test/organizations.e2e-spec.ts | 11 ++++++---- backend/test/roles.e2e-spec.ts | 11 ++++++---- 4 files changed, 35 insertions(+), 18 deletions(-) diff --git a/backend/src/modules/auth/auth.controller.ts b/backend/src/modules/auth/auth.controller.ts index 1fd220f..c07cb4e 100644 --- a/backend/src/modules/auth/auth.controller.ts +++ b/backend/src/modules/auth/auth.controller.ts @@ -5,6 +5,8 @@ import { Request, Body, Res, + HttpCode, + HttpStatus, } from '@nestjs/common'; import { ApiTags, @@ -53,6 +55,7 @@ export class AuthController { @ApiResponse({ status: 200, description: 'Successfully logged in' }) @ApiResponse({ status: 401, description: 'Invalid credentials' }) @UseGuards(LocalAuthGuard) + @HttpCode(HttpStatus.OK) @Post('login') async login( @Request() req: ExpressRequest, @@ -86,6 +89,7 @@ export class AuthController { @ApiResponse({ status: 200, description: 'Tokens refreshed successfully' }) @ApiResponse({ status: 401, description: 'Invalid or expired refresh token' }) @UseGuards(RefreshTokenAuthGuard) + @HttpCode(HttpStatus.OK) @Post('refresh') async refresh( @Request() req: any, @@ -111,6 +115,7 @@ export class AuthController { @ApiResponse({ status: 200, description: 'Successfully logged out' }) @ApiResponse({ status: 401, description: 'Invalid refresh token' }) @UseGuards(RefreshTokenAuthGuard) + @HttpCode(HttpStatus.OK) @Post('logout') async logout(@Request() req: any, @Res({ passthrough: true }) res: Response) { await this.authService.revokeRefreshToken(req.user.refreshToken); @@ -127,6 +132,7 @@ export class AuthController { description: 'If an account with that email exists, a password reset link has been sent', }) + @HttpCode(HttpStatus.OK) @Post('forgot-password') async forgotPassword(@Body() forgotPasswordDto: ForgotPasswordDto) { return this.authService.requestPasswordReset(forgotPasswordDto.email); @@ -136,6 +142,7 @@ export class AuthController { @ApiBody({ type: ResetPasswordDto }) @ApiResponse({ status: 200, description: 'Password reset successfully' }) @ApiResponse({ status: 400, description: 'Invalid or expired token' }) + @HttpCode(HttpStatus.OK) @Post('reset-password') async resetPassword(@Body() resetPasswordDto: ResetPasswordDto) { const { token, newPassword } = resetPasswordDto; @@ -149,6 +156,7 @@ export class AuthController { @ApiResponse({ status: 400, description: 'Current password is incorrect' }) @ApiResponse({ status: 401, description: 'Unauthorized' }) @UseGuards(JwtAuthGuard) + @HttpCode(HttpStatus.OK) @Post('change-password') async changePassword( @Request() req: any, diff --git a/backend/test/auth-password-reset.e2e-spec.ts b/backend/test/auth-password-reset.e2e-spec.ts index 3c9f23e..223fb75 100644 --- a/backend/test/auth-password-reset.e2e-spec.ts +++ b/backend/test/auth-password-reset.e2e-spec.ts @@ -53,14 +53,17 @@ describe('Auth - Password Reset (e2e)', () => { username: 'testuser', password: 'password123', }) - .expect(201); + .expect(200); const setCookies = loginResponse.headers[ 'set-cookie' ] as unknown as string[]; - authCookie = - setCookies.find((c) => c.startsWith('access_token='))?.split(';')[0] ?? - ''; + expect(Array.isArray(setCookies)).toBe(true); + const accessTokenCookie = setCookies.find((c) => + c.startsWith('access_token='), + ); + expect(accessTokenCookie).toBeDefined(); + authCookie = accessTokenCookie!.split(';')[0]; }); afterAll(async () => { @@ -79,7 +82,7 @@ describe('Auth - Password Reset (e2e)', () => { const response = await request(app.getHttpServer()) .post('/auth/forgot-password') .send({ email: 'test@example.com' }) - .expect(201); + .expect(200); expect(response.body).toHaveProperty('message'); expect(response.body.message).toContain( @@ -102,7 +105,7 @@ describe('Auth - Password Reset (e2e)', () => { const response = await request(app.getHttpServer()) .post('/auth/forgot-password') .send({ email: 'nonexistent@example.com' }) - .expect(201); + .expect(200); expect(response.body.message).toContain( 'If an account with that email exists', @@ -148,7 +151,7 @@ describe('Auth - Password Reset (e2e)', () => { token: validToken, newPassword, }) - .expect(201); + .expect(200); expect(response.body.message).toContain('reset successfully'); @@ -167,7 +170,7 @@ describe('Auth - Password Reset (e2e)', () => { username: 'testuser', password: newPassword, }) - .expect(201); + .expect(200); // Reset password back for other tests const hashedPassword = await bcrypt.hash('password123', 10); @@ -264,7 +267,7 @@ describe('Auth - Password Reset (e2e)', () => { currentPassword, newPassword, }) - .expect(201); + .expect(200); expect(response.body.message).toContain('changed successfully'); @@ -275,7 +278,7 @@ describe('Auth - Password Reset (e2e)', () => { username: 'testuser', password: newPassword, }) - .expect(201); + .expect(200); // Reset password back for other tests const hashedPassword = await bcrypt.hash('password123', 10); diff --git a/backend/test/organizations.e2e-spec.ts b/backend/test/organizations.e2e-spec.ts index 7ec3675..af56464 100644 --- a/backend/test/organizations.e2e-spec.ts +++ b/backend/test/organizations.e2e-spec.ts @@ -45,14 +45,17 @@ describe('Organizations (e2e)', () => { username: 'orguser', password: 'password123', }) - .expect(201); + .expect(200); const setCookies = loginResponse.headers[ 'set-cookie' ] as unknown as string[]; - authCookie = - setCookies.find((c) => c.startsWith('access_token='))?.split(';')[0] ?? - ''; + expect(Array.isArray(setCookies)).toBe(true); + const accessTokenCookie = setCookies.find((c) => + c.startsWith('access_token='), + ); + expect(accessTokenCookie).toBeDefined(); + authCookie = accessTokenCookie!.split(';')[0]; }); afterAll(async () => { diff --git a/backend/test/roles.e2e-spec.ts b/backend/test/roles.e2e-spec.ts index 649079d..3321916 100644 --- a/backend/test/roles.e2e-spec.ts +++ b/backend/test/roles.e2e-spec.ts @@ -38,14 +38,17 @@ describe('Roles (e2e)', () => { username: 'testuser', password: 'password123', }) - .expect(201); + .expect(200); const setCookies = loginResponse.headers[ 'set-cookie' ] as unknown as string[]; - authCookie = - setCookies.find((c) => c.startsWith('access_token='))?.split(';')[0] ?? - ''; + expect(Array.isArray(setCookies)).toBe(true); + const accessTokenCookie = setCookies.find((c) => + c.startsWith('access_token='), + ); + expect(accessTokenCookie).toBeDefined(); + authCookie = accessTokenCookie!.split(';')[0]; }); afterAll(async () => { From 6f8970d70d9cd0d0e751cea565923db76ba4927b Mon Sep 17 00:00:00 2001 From: Demian Date: Mon, 13 Apr 2026 01:38:51 -0400 Subject: [PATCH 7/8] fix: correct LocalStrategy return type and tighten e2e cookie assertions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Fix LocalStrategy.validate() return type from Omit to Omit — validateUser() strips password from a User entity, not a UserDto; the wrong annotation caused the unsafe cast in the controller - Replace the unsafe cast in AuthController.login() with Omit now that the type flows correctly from LocalStrategy - Add .expect(200) and explicit set-cookie/access_token assertions to user-organization-roles.e2e-spec.ts beforeAll (consistent with the pattern already applied to roles and organizations specs) - Add .expect(201) to role assignment setup in permissions test --- backend/src/modules/auth/auth.controller.ts | 3 ++- backend/src/modules/auth/local.strategy.ts | 4 ++-- backend/test/user-organization-roles.e2e-spec.ts | 15 ++++++++++----- 3 files changed, 14 insertions(+), 8 deletions(-) diff --git a/backend/src/modules/auth/auth.controller.ts b/backend/src/modules/auth/auth.controller.ts index c07cb4e..a8acf09 100644 --- a/backend/src/modules/auth/auth.controller.ts +++ b/backend/src/modules/auth/auth.controller.ts @@ -20,6 +20,7 @@ import { LocalAuthGuard } from './local-auth.guard'; import { JwtAuthGuard } from './jwt-auth.guard'; import { RefreshTokenAuthGuard } from './refresh-token-auth.guard'; import { UserDto } from '../users/dto/user.dto'; +import { User } from '../users/user.entity'; import { Request as ExpressRequest, Response } from 'express'; import { ChangePasswordDto, @@ -61,7 +62,7 @@ export class AuthController { @Request() req: ExpressRequest, @Res({ passthrough: true }) res: Response, ) { - const user = req.user as Parameters[0]; + const user = req.user as Omit; const tokens = await this.authService.login(user); res.cookie( 'access_token', diff --git a/backend/src/modules/auth/local.strategy.ts b/backend/src/modules/auth/local.strategy.ts index 27ddb0a..c4f431c 100644 --- a/backend/src/modules/auth/local.strategy.ts +++ b/backend/src/modules/auth/local.strategy.ts @@ -2,7 +2,7 @@ import { Strategy } from 'passport-local'; import { PassportStrategy } from '@nestjs/passport'; import { Injectable, UnauthorizedException, Logger } from '@nestjs/common'; import { AuthService } from './auth.service'; -import { UserDto } from '../users/dto/user.dto'; +import { User } from '../users/user.entity'; @Injectable() export class LocalStrategy extends PassportStrategy(Strategy) { @@ -18,7 +18,7 @@ export class LocalStrategy extends PassportStrategy(Strategy) { async validate( username: string, password: string, - ): Promise> { + ): Promise> { this.logger.debug(`Validating user: ${username}`); const user = await this.authService.validateUser(username, password); diff --git a/backend/test/user-organization-roles.e2e-spec.ts b/backend/test/user-organization-roles.e2e-spec.ts index 94bad08..d0e324a 100644 --- a/backend/test/user-organization-roles.e2e-spec.ts +++ b/backend/test/user-organization-roles.e2e-spec.ts @@ -52,14 +52,18 @@ describe('UserOrganizationRoles (e2e)', () => { .send({ username: 'roleuser', password: 'password123', - }); + }) + .expect(200); const setCookies = loginResponse.headers[ 'set-cookie' ] as unknown as string[]; - authCookie = - setCookies.find((c) => c.startsWith('access_token='))?.split(';')[0] ?? - ''; + expect(Array.isArray(setCookies)).toBe(true); + const accessTokenCookie = setCookies.find((c) => + c.startsWith('access_token='), + ); + expect(accessTokenCookie).toBeDefined(); + authCookie = accessTokenCookie!.split(';')[0]; // Create a test organization const orgResponse = await request(app.getHttpServer()) @@ -264,7 +268,8 @@ describe('UserOrganizationRoles (e2e)', () => { userId, organizationId, roleId, - }); + }) + .expect(201); return request(app.getHttpServer()) .get(`/permissions/user/${userId}/organization/${organizationId}`) From 1f63a40cb91608a9dee77546803f89b30907d9de Mon Sep 17 00:00:00 2001 From: Demian Date: Mon, 13 Apr 2026 09:34:40 -0400 Subject: [PATCH 8/8] feat: add GET /auth/me endpoint for session state population With httpOnly cookie auth the frontend cannot decode the JWT to read user info. GET /auth/me returns { id, username } from the validated access token so the frontend can populate session state on load or page refresh. --- backend/src/modules/auth/auth.controller.ts | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/backend/src/modules/auth/auth.controller.ts b/backend/src/modules/auth/auth.controller.ts index a8acf09..7f7e90e 100644 --- a/backend/src/modules/auth/auth.controller.ts +++ b/backend/src/modules/auth/auth.controller.ts @@ -1,5 +1,6 @@ import { Controller, + Get, Post, UseGuards, Request, @@ -86,6 +87,16 @@ export class AuthController { return this.authService.register(userDto); } + @ApiOperation({ summary: 'Get current authenticated user' }) + @ApiBearerAuth('access-token') + @ApiResponse({ status: 200, description: 'Current user info' }) + @ApiResponse({ status: 401, description: 'Unauthorized' }) + @UseGuards(JwtAuthGuard) + @Get('me') + me(@Request() req: any) { + return { id: req.user.userId, username: req.user.username }; + } + @ApiOperation({ summary: 'Refresh access token using refresh token cookie' }) @ApiResponse({ status: 200, description: 'Tokens refreshed successfully' }) @ApiResponse({ status: 401, description: 'Invalid or expired refresh token' })