diff --git a/.env.sample b/.env.sample index 00906bb..e69f31d 100644 --- a/.env.sample +++ b/.env.sample @@ -17,6 +17,11 @@ MOODLE_MASTER_KEY= JWT_SECRET= REFRESH_SECRET= +# Optional JWT tuning (defaults shown) +# JWT_ACCESS_TOKEN_EXPIRY=300s +# JWT_REFRESH_TOKEN_EXPIRY=30d +# JWT_BCRYPT_ROUNDS=10 + # Redis (caching and job queues) REDIS_URL=redis://localhost:6379 diff --git a/.github/workflows/pr-lint.yml b/.github/workflows/pr-lint.yml index 67a0ff2..2ccd345 100644 --- a/.github/workflows/pr-lint.yml +++ b/.github/workflows/pr-lint.yml @@ -53,6 +53,7 @@ jobs: - name: Discord notification if: ${{ github.event_name == 'pull_request' }} + continue-on-error: true env: DISCORD_WEBHOOK: ${{ secrets.DISCORD_PR_WEBHOOK }} DISCORD_USERNAME: "Leo Bermudez" @@ -77,6 +78,24 @@ jobs: "timestamp": "${{ github.event.pull_request.updated_at }}" } ] - uses: Ilshidur/action-discord@0.4.0 - with: - args: ${{ env.LINT_STATUS == 'βœ… Lint passed, clean code! ✨' && 'βœ… Lint passed! pwede na mag NU' || 'Naa kay sayup, wala kay respeto! 😑' }} + DISCORD_MESSAGE: ${{ env.LINT_STATUS == 'βœ… Lint passed, clean code! ✨' && 'βœ… Lint passed! pwede na mag NU' || 'Naa kay sayup, wala kay respeto! 😑' }} + run: | + payload=$(jq -nc \ + --arg username "$DISCORD_USERNAME" \ + --arg avatar_url "$DISCORD_AVATAR" \ + --arg content "$DISCORD_MESSAGE" \ + --argjson embeds "$DISCORD_EMBEDS" \ + '{username: $username, avatar_url: $avatar_url, content: $content, embeds: $embeds}') + + if [ -z "$DISCORD_WEBHOOK" ]; then + echo "DISCORD_PR_WEBHOOK is not configured; skipping Discord notification." + exit 0 + fi + + curl \ + --fail \ + --show-error \ + --silent \ + -H "Content-Type: application/json" \ + -d "$payload" \ + "$DISCORD_WEBHOOK" diff --git a/.github/workflows/pr-test.yml b/.github/workflows/pr-test.yml index eb8361f..58cdd99 100644 --- a/.github/workflows/pr-test.yml +++ b/.github/workflows/pr-test.yml @@ -140,6 +140,7 @@ jobs: - name: Discord notification if: ${{ github.event_name == 'pull_request' }} + continue-on-error: true env: DISCORD_WEBHOOK: ${{ secrets.DISCORD_PR_WEBHOOK }} DISCORD_USERNAME: "Jennifer Garrido-Amores" @@ -169,6 +170,24 @@ jobs: "timestamp": "${{ github.event.pull_request.updated_at }}" } ] - uses: Ilshidur/action-discord@0.4.0 - with: - args: ${{ env.TEST_STATUS == 'βœ… All tests passed, nice ka preπŸ‘Œ' && 'Ikaw na jud dong. πŸš€' || 'Please ko fix today. Salamat' }} + DISCORD_MESSAGE: ${{ env.TEST_STATUS == 'βœ… All tests passed, nice ka preπŸ‘Œ' && 'Ikaw na jud dong. πŸš€' || 'Please ko fix today. Salamat' }} + run: | + payload=$(jq -nc \ + --arg username "$DISCORD_USERNAME" \ + --arg avatar_url "$DISCORD_AVATAR" \ + --arg content "$DISCORD_MESSAGE" \ + --argjson embeds "$DISCORD_EMBEDS" \ + '{username: $username, avatar_url: $avatar_url, content: $content, embeds: $embeds}') + + if [ -z "$DISCORD_WEBHOOK" ]; then + echo "DISCORD_PR_WEBHOOK is not configured; skipping Discord notification." + exit 0 + fi + + curl \ + --fail \ + --show-error \ + --silent \ + -H "Content-Type: application/json" \ + -d "$payload" \ + "$DISCORD_WEBHOOK" diff --git a/AGENTS.md b/AGENTS.md index 31ad980..3c972e0 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -191,6 +191,9 @@ Required environment variables (see `.env.sample`): Optional: +- `JWT_ACCESS_TOKEN_EXPIRY`: Access token lifetime (default: `300s`) +- `JWT_REFRESH_TOKEN_EXPIRY`: Refresh token lifetime (default: `30d`) +- `JWT_BCRYPT_ROUNDS`: Bcrypt cost factor for refresh-token hashing (default: `10`; values below `10` log a warning outside production) - `OPENAPI_MODE`: Set to `"true"` to enable Swagger docs (default: disabled) - `SYNC_ON_STARTUP`: Set to `"true"` to run Course and Enrollment sync on startup (default: disabled) - `SUPER_ADMIN_USERNAME`: Default super admin username (default: `superadmin`) diff --git a/CLAUDE.md b/CLAUDE.md index 11cbc3f..d81c145 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -159,6 +159,9 @@ Required environment variables (see `.env.sample`): Optional: +- `JWT_ACCESS_TOKEN_EXPIRY`: Access token lifetime (default: `300s`) +- `JWT_REFRESH_TOKEN_EXPIRY`: Refresh token lifetime (default: `30d`) +- `JWT_BCRYPT_ROUNDS`: Bcrypt cost factor for refresh-token hashing (default: `10`; values below `10` warn outside production) - `OPENAPI_MODE`: Set to `"true"` to enable Swagger docs (default: disabled) - `SYNC_ON_STARTUP`: Set to `"true"` to run Course and Enrollment sync on startup (default: disabled) - `SUPER_ADMIN_USERNAME`: Default super admin username (default: `superadmin`) diff --git a/src/configurations/env/env.validation.ts b/src/configurations/env/env.validation.ts index 8f2ca02..1ad3cd4 100644 --- a/src/configurations/env/env.validation.ts +++ b/src/configurations/env/env.validation.ts @@ -1,5 +1,6 @@ import { z } from 'zod'; import { envSchema } from '.'; +import { warnOnWeakJwtConfig } from './jwt.env'; export const validateEnv = (config: Record) => { const result = envSchema.safeParse(config); @@ -10,5 +11,6 @@ export const validateEnv = (config: Record) => { process.exit(1); } + warnOnWeakJwtConfig(result.data); return result.data; // Return validated config for NestJS }; diff --git a/src/configurations/env/index.ts b/src/configurations/env/index.ts index d331ac0..b7b2366 100644 --- a/src/configurations/env/index.ts +++ b/src/configurations/env/index.ts @@ -5,7 +5,7 @@ import { serverEnvSchema } from './server.env'; import { corsEnvSchema } from './cors.env'; import { DEFAULT_PORT } from '../common/constants'; import { databaseEnvSchema } from './database.env'; -import { jwtEnvSchema } from './jwt.env'; +import { jwtEnvSchema, warnOnWeakJwtConfig } from './jwt.env'; import { openaiEnvSchema } from './openai.env'; import { adminEnvSchema } from './admin.env'; import { redisEnvSchema } from './redis.env'; @@ -27,4 +27,6 @@ export type Env = z.infer; export const env = envSchema.parse(process.env); +warnOnWeakJwtConfig(env); + export const envPortResolve = () => env.PORT ?? DEFAULT_PORT; diff --git a/src/configurations/env/jwt-duration.util.ts b/src/configurations/env/jwt-duration.util.ts new file mode 100644 index 0000000..1395aab --- /dev/null +++ b/src/configurations/env/jwt-duration.util.ts @@ -0,0 +1,70 @@ +const JWT_DURATION_PATTERN = + /^\s*(?-?(?:\d+\.?\d*|\.\d+))\s*(?milliseconds?|msecs?|ms|seconds?|secs?|s|minutes?|mins?|m|hours?|hrs?|h|days?|d|weeks?|w|years?|yrs?|y)?\s*$/i; + +const JWT_DURATION_MULTIPLIERS: Record = { + ms: 1, + msec: 1, + msecs: 1, + millisecond: 1, + milliseconds: 1, + s: 1000, + sec: 1000, + secs: 1000, + second: 1000, + seconds: 1000, + m: 60 * 1000, + min: 60 * 1000, + mins: 60 * 1000, + minute: 60 * 1000, + minutes: 60 * 1000, + h: 60 * 60 * 1000, + hr: 60 * 60 * 1000, + hrs: 60 * 60 * 1000, + hour: 60 * 60 * 1000, + hours: 60 * 60 * 1000, + d: 24 * 60 * 60 * 1000, + day: 24 * 60 * 60 * 1000, + days: 24 * 60 * 60 * 1000, + w: 7 * 24 * 60 * 60 * 1000, + week: 7 * 24 * 60 * 60 * 1000, + weeks: 7 * 24 * 60 * 60 * 1000, + y: 365.25 * 24 * 60 * 60 * 1000, + yr: 365.25 * 24 * 60 * 60 * 1000, + yrs: 365.25 * 24 * 60 * 60 * 1000, + year: 365.25 * 24 * 60 * 60 * 1000, + years: 365.25 * 24 * 60 * 60 * 1000, +}; + +export const parseJwtDurationToMilliseconds = ( + value: string, +): number | null => { + const match = JWT_DURATION_PATTERN.exec(value); + + if (!match?.groups) { + return null; + } + + const durationValue = Number.parseFloat(match.groups.value); + + if (!Number.isFinite(durationValue) || durationValue <= 0) { + return null; + } + + const unit = (match.groups.unit ?? 'ms').toLowerCase(); + const multiplier = JWT_DURATION_MULTIPLIERS[unit]; + + if (!multiplier) { + return null; + } + + const milliseconds = durationValue * multiplier; + + if (!Number.isFinite(milliseconds) || milliseconds <= 0) { + return null; + } + + return Math.floor(milliseconds); +}; + +export const isValidJwtDuration = (value: string): boolean => + parseJwtDurationToMilliseconds(value) !== null; diff --git a/src/configurations/env/jwt.env.spec.ts b/src/configurations/env/jwt.env.spec.ts new file mode 100644 index 0000000..ece5879 --- /dev/null +++ b/src/configurations/env/jwt.env.spec.ts @@ -0,0 +1,80 @@ +import { jwtEnvSchema, warnOnWeakJwtConfig } from './jwt.env'; + +describe('jwtEnvSchema', () => { + const originalWarn = console.warn; + + beforeEach(() => { + console.warn = jest.fn(); + }); + + afterEach(() => { + console.warn = originalWarn; + }); + + it('applies defaults for optional JWT settings', () => { + const result = jwtEnvSchema.parse({ + JWT_SECRET: 'secret', + REFRESH_SECRET: 'refresh-secret', + }); + + expect(result.JWT_ACCESS_TOKEN_EXPIRY).toBe('300s'); + expect(result.JWT_REFRESH_TOKEN_EXPIRY).toBe('30d'); + expect(result.JWT_BCRYPT_ROUNDS).toBe(10); + }); + + it('rejects invalid duration strings', () => { + expect(() => + jwtEnvSchema.parse({ + JWT_SECRET: 'secret', + REFRESH_SECRET: 'refresh-secret', + JWT_ACCESS_TOKEN_EXPIRY: 'later', + }), + ).toThrow(); + + expect(() => + jwtEnvSchema.parse({ + JWT_SECRET: 'secret', + REFRESH_SECRET: 'refresh-secret', + JWT_REFRESH_TOKEN_EXPIRY: '0d', + }), + ).toThrow(); + }); + + it('rejects non-positive bcrypt rounds', () => { + expect(() => + jwtEnvSchema.parse({ + JWT_SECRET: 'secret', + REFRESH_SECRET: 'refresh-secret', + JWT_BCRYPT_ROUNDS: '0', + }), + ).toThrow(); + + expect(() => + jwtEnvSchema.parse({ + JWT_SECRET: 'secret', + REFRESH_SECRET: 'refresh-secret', + JWT_BCRYPT_ROUNDS: '1.5', + }), + ).toThrow(); + }); + + it('warns for weak bcrypt rounds outside production', () => { + warnOnWeakJwtConfig({ + NODE_ENV: 'development', + JWT_BCRYPT_ROUNDS: 8, + }); + + expect(console.warn).toHaveBeenCalledWith( + expect.stringContaining('JWT_BCRYPT_ROUNDS'), + ); + }); + + it('does not warn for weak bcrypt rounds in production', () => { + warnOnWeakJwtConfig({ + NODE_ENV: 'production', + JWT_BCRYPT_ROUNDS: 8, + }); + + expect(console.warn).not.toHaveBeenCalled(); + }); +}); diff --git a/src/configurations/env/jwt.env.ts b/src/configurations/env/jwt.env.ts index cedd20f..4413fe8 100644 --- a/src/configurations/env/jwt.env.ts +++ b/src/configurations/env/jwt.env.ts @@ -1,8 +1,44 @@ import z from 'zod'; +import { isValidJwtDuration } from './jwt-duration.util'; + +let hasWarnedWeakJwtBcryptRounds = false; export const jwtEnvSchema = z.object({ JWT_SECRET: z.string(), REFRESH_SECRET: z.string(), + JWT_ACCESS_TOKEN_EXPIRY: z + .string() + .trim() + .refine(isValidJwtDuration, { + message: 'JWT_ACCESS_TOKEN_EXPIRY must be a valid positive duration', + }) + .default('300s'), + JWT_REFRESH_TOKEN_EXPIRY: z + .string() + .trim() + .refine(isValidJwtDuration, { + message: 'JWT_REFRESH_TOKEN_EXPIRY must be a valid positive duration', + }) + .default('30d'), + JWT_BCRYPT_ROUNDS: z.coerce.number().int().positive().default(10), }); -export type DatabaseEnv = z.infer; +export type JwtEnv = z.infer; + +export const warnOnWeakJwtConfig = (config: { + NODE_ENV: 'development' | 'production' | 'test'; + JWT_BCRYPT_ROUNDS: number; +}) => { + if ( + config.NODE_ENV === 'production' || + config.JWT_BCRYPT_ROUNDS >= 10 || + hasWarnedWeakJwtBcryptRounds + ) { + return; + } + + hasWarnedWeakJwtBcryptRounds = true; + console.warn( + `JWT_BCRYPT_ROUNDS is set to ${config.JWT_BCRYPT_ROUNDS}. Values below 10 reduce refresh-token hashing cost and should only be used for non-production convenience.`, + ); +}; diff --git a/src/entities/refresh-token.entity.ts b/src/entities/refresh-token.entity.ts index 4245264..619afde 100644 --- a/src/entities/refresh-token.entity.ts +++ b/src/entities/refresh-token.entity.ts @@ -37,22 +37,17 @@ export class RefreshToken extends CustomBaseEntity { userId: string, metaData: RequestMetadata, refreshId: string, + expiresAt: Date, ) { const newRefreshToken = new RefreshToken(); newRefreshToken.id = refreshId; newRefreshToken.tokenHash = hashedToken; newRefreshToken.userId = userId; - newRefreshToken.expiresAt = RefreshToken.addDays(new Date(), 30); + newRefreshToken.expiresAt = expiresAt; newRefreshToken.isActive = true; newRefreshToken.browserName = metaData.browserName; newRefreshToken.os = metaData.os; newRefreshToken.ipAddress = metaData.ipAddress; return newRefreshToken; } - - static addDays(date: Date, days: number): Date { - const result = new Date(date); - result.setDate(result.getDate() + days); - return result; - } } diff --git a/src/modules/common/custom-jwt-service/index.spec.ts b/src/modules/common/custom-jwt-service/index.spec.ts new file mode 100644 index 0000000..f86dc87 --- /dev/null +++ b/src/modules/common/custom-jwt-service/index.spec.ts @@ -0,0 +1,118 @@ +import { JwtService } from '@nestjs/jwt'; +import * as bcrypt from 'bcrypt'; +import { env } from 'src/configurations/env'; +import { RefreshToken } from 'src/entities/refresh-token.entity'; +import { RefreshTokenRepository } from 'src/repositories/refresh-token.repository'; +import { RequestMetadataService } from '../cls/request-metadata.service'; +import { CustomJwtService } from './index'; + +jest.mock('bcrypt', () => ({ + hash: jest.fn(), +})); + +describe('CustomJwtService', () => { + const mockMetadata = { + browserName: 'Chrome', + os: 'Linux', + ipAddress: '127.0.0.1', + }; + + const originalRefreshTokenExpiry = env.JWT_REFRESH_TOKEN_EXPIRY; + const originalBcryptRounds = env.JWT_BCRYPT_ROUNDS; + + let service: CustomJwtService; + let jwtService: jest.Mocked; + let refreshTokenRepository: jest.Mocked; + let requestMetadataService: jest.Mocked; + + beforeEach(() => { + env.JWT_REFRESH_TOKEN_EXPIRY = '2h'; + env.JWT_BCRYPT_ROUNDS = 12; + + jwtService = { + signAsync: jest + .fn() + .mockResolvedValueOnce('access-token') + .mockResolvedValueOnce('refresh-token'), + } as unknown as jest.Mocked; + + refreshTokenRepository = { + revokeActiveForDevice: jest.fn(), + create: jest.fn(), + } as unknown as jest.Mocked; + + requestMetadataService = { + getOrFail: jest.fn().mockReturnValue(mockMetadata), + } as unknown as jest.Mocked; + + service = new CustomJwtService( + jwtService, + refreshTokenRepository, + requestMetadataService, + ); + }); + + afterEach(() => { + env.JWT_REFRESH_TOKEN_EXPIRY = originalRefreshTokenExpiry; + env.JWT_BCRYPT_ROUNDS = originalBcryptRounds; + jest.clearAllMocks(); + }); + + it('uses the configured refresh-token expiry and bcrypt rounds', async () => { + const hashMock = bcrypt.hash as jest.MockedFunction; + hashMock.mockResolvedValue('hashed-token'); + + await service.CreateSignedTokens({ + jwt: { sub: 'user-id' }, + refreshJwt: { sub: 'user-id', jti: 'refresh-id' }, + userId: 'user-id', + }); + + expect(jwtService.signAsync.mock.calls[1]).toEqual([ + { + sub: 'user-id', + jti: 'refresh-id', + }, + { + secret: env.REFRESH_SECRET, + expiresIn: '2h', + }, + ]); + expect(hashMock).toHaveBeenCalledWith('refresh-token', 12); + }); + + it('persists a refresh token with an expiry derived from the same duration', async () => { + const hashMock = bcrypt.hash as jest.MockedFunction; + hashMock.mockResolvedValue('hashed-token'); + const createSpy = jest.spyOn(RefreshToken, 'Create'); + + const before = Date.now(); + await service.CreateSignedTokens({ + jwt: { sub: 'user-id' }, + refreshJwt: { sub: 'user-id', jti: 'refresh-id' }, + userId: 'user-id', + }); + const after = Date.now(); + + expect(refreshTokenRepository.revokeActiveForDevice.mock.calls[0]).toEqual([ + 'user-id', + mockMetadata.browserName, + mockMetadata.os, + mockMetadata.ipAddress, + ]); + + expect(createSpy).toHaveBeenCalledWith( + 'hashed-token', + 'user-id', + mockMetadata, + 'refresh-id', + expect.any(Date), + ); + + const expiresAt = createSpy.mock.calls[0][4]; + expect(expiresAt.getTime()).toBeGreaterThanOrEqual( + before + 2 * 60 * 60 * 1000, + ); + expect(expiresAt.getTime()).toBeLessThanOrEqual(after + 2 * 60 * 60 * 1000); + }); +}); diff --git a/src/modules/common/custom-jwt-service/index.ts b/src/modules/common/custom-jwt-service/index.ts index 66cee02..50240f6 100644 --- a/src/modules/common/custom-jwt-service/index.ts +++ b/src/modules/common/custom-jwt-service/index.ts @@ -1,8 +1,10 @@ import { Injectable } from '@nestjs/common'; import { JwtService } from '@nestjs/jwt'; +import type { StringValue } from 'ms'; import { JwtPayload } from './jwt-payload.dto'; import { RefreshJwtPayload } from './refresh-jwt-payload.dto'; import { env } from 'src/configurations/env'; +import { parseJwtDurationToMilliseconds } from 'src/configurations/env/jwt-duration.util'; import { RefreshTokenRepository } from 'src/repositories/refresh-token.repository'; import * as bcrypt from 'bcrypt'; import { RefreshToken } from 'src/entities/refresh-token.entity'; @@ -19,6 +21,8 @@ export type CreateTokensPayload = { userId: string; }; +const asJwtExpiresIn = (value: string): StringValue => value as StringValue; + @Injectable() export class CustomJwtService { constructor( @@ -30,16 +34,25 @@ export class CustomJwtService { async CreateSignedTokens( payload: CreateTokensPayload, ): Promise { + const refreshTokenExpiryMs = parseJwtDurationToMilliseconds( + env.JWT_REFRESH_TOKEN_EXPIRY, + ); + + if (refreshTokenExpiryMs === null) { + throw new Error('JWT_REFRESH_TOKEN_EXPIRY must be a valid duration'); + } + const token = await this.jwtService.signAsync(payload.jwt); const refreshToken = await this.jwtService.signAsync(payload.refreshJwt, { secret: env.REFRESH_SECRET, - expiresIn: '30d', + expiresIn: asJwtExpiresIn(env.JWT_REFRESH_TOKEN_EXPIRY), }); await this.PersistRefreshToken( refreshToken, payload.userId, payload.refreshJwt.jti, + new Date(Date.now() + refreshTokenExpiryMs), ); return { @@ -52,9 +65,10 @@ export class CustomJwtService { refreshToken: string, userId: string, refreshId: string, + expiresAt: Date, ) { const metaData = this.requestMetadataService.getOrFail(); - const hashedToken = await bcrypt.hash(refreshToken, 10); + const hashedToken = await bcrypt.hash(refreshToken, env.JWT_BCRYPT_ROUNDS); // revoke refresh refresh tokens await this.refreshTokenRepository.revokeActiveForDevice( @@ -70,6 +84,7 @@ export class CustomJwtService { userId, metaData, refreshId, + expiresAt, ); this.refreshTokenRepository.create(newRefreshToken); } diff --git a/src/modules/index.module.ts b/src/modules/index.module.ts index 604b943..e1a7e23 100644 --- a/src/modules/index.module.ts +++ b/src/modules/index.module.ts @@ -23,8 +23,11 @@ import { CurriculumModule } from './curriculum/curriculum.module'; import { AdminModule } from './admin/admin.module'; import { LoggerModule } from 'nestjs-pino'; import { ClsModule } from 'nestjs-cls'; +import type { StringValue } from 'ms'; import { v4 } from 'uuid'; +const asJwtExpiresIn = (value: string): StringValue => value as StringValue; + export const ApplicationModules = [ HealthModule, MoodleModule, @@ -51,7 +54,7 @@ export const InfrastructureModules = [ global: true, secret: env.JWT_SECRET, signOptions: { - expiresIn: '300s', + expiresIn: asJwtExpiresIn(env.JWT_ACCESS_TOKEN_EXPIRY), }, }), ClsModule.forRoot({