From 22661674672be20cbb99d4584c138c716f3919d3 Mon Sep 17 00:00:00 2001 From: barsa Date: Fri, 12 Dec 2025 15:29:58 +0900 Subject: [PATCH] Enhance JWT handling and authentication flow - Introduced support for previous JWT secrets in the environment configuration to facilitate key rotation. - Refactored the JoseJwtService to manage multiple signing and verification keys, improving security during token validation. - Updated the AuthTokenService to include family identifiers for refresh tokens, enhancing session management and security. - Modified the PasswordWorkflowService and SignupWorkflowService to return session metadata instead of token strings, aligning with security best practices. - Improved error handling and token revocation logic in the TokenBlacklistService and AuthTokenService to prevent replay attacks. - Updated documentation to reflect changes in the authentication architecture and security model. --- apps/bff/src/core/config/env.validation.ts | 3 + apps/bff/src/modules/auth/auth.types.ts | 11 + .../auth/infra/token/jose-jwt.service.ts | 60 +++++- .../infra/token/token-blacklist.service.ts | 20 +- .../modules/auth/infra/token/token.service.ts | 203 ++++++++++++++++-- .../workflows/password-workflow.service.ts | 23 +- .../workflows/signup-workflow.service.ts | 4 +- .../auth/presentation/http/auth.controller.ts | 30 ++- .../src/features/auth/services/auth.store.ts | 8 +- docs/auth/AUTH-MODULE-ARCHITECTURE.md | 19 ++ docs/auth/AUTH-SCHEMA-IMPROVEMENTS.md | 67 ++++-- packages/domain/auth/contract.ts | 5 +- packages/domain/auth/index.ts | 10 +- packages/domain/auth/schema.ts | 48 +++-- 14 files changed, 411 insertions(+), 100 deletions(-) diff --git a/apps/bff/src/core/config/env.validation.ts b/apps/bff/src/core/config/env.validation.ts index ffb13e62..300576bc 100644 --- a/apps/bff/src/core/config/env.validation.ts +++ b/apps/bff/src/core/config/env.validation.ts @@ -7,6 +7,9 @@ export const envSchema = z.object({ APP_NAME: z.string().default("customer-portal-bff"), JWT_SECRET: z.string().min(32, "JWT secret must be at least 32 characters"), + // Optional comma-separated list of previous secrets for key rotation (HS256). + // Example: JWT_SECRET_PREVIOUS="oldsecret1,oldsecret2" + JWT_SECRET_PREVIOUS: z.string().optional(), JWT_EXPIRES_IN: z.string().default("7d"), JWT_ISSUER: z.string().min(1).optional(), JWT_AUDIENCE: z.string().min(1).optional(), // supports CSV: "portal,admin" diff --git a/apps/bff/src/modules/auth/auth.types.ts b/apps/bff/src/modules/auth/auth.types.ts index acc54405..24375bfb 100644 --- a/apps/bff/src/modules/auth/auth.types.ts +++ b/apps/bff/src/modules/auth/auth.types.ts @@ -1,4 +1,15 @@ import type { User } from "@customer-portal/domain/customer"; import type { Request } from "express"; +import type { AuthTokens } from "@customer-portal/domain/auth"; +import type { UserAuth } from "@customer-portal/domain/customer"; export type RequestWithUser = Request & { user: User }; + +/** + * Internal auth result used inside the BFF to set cookies. + * Token strings must not be returned to the browser. + */ +export type AuthResultInternal = { + user: UserAuth; + tokens: AuthTokens; +}; diff --git a/apps/bff/src/modules/auth/infra/token/jose-jwt.service.ts b/apps/bff/src/modules/auth/infra/token/jose-jwt.service.ts index 8f24092c..9019d07d 100644 --- a/apps/bff/src/modules/auth/infra/token/jose-jwt.service.ts +++ b/apps/bff/src/modules/auth/infra/token/jose-jwt.service.ts @@ -5,7 +5,8 @@ import { parseJwtExpiry } from "../../utils/jwt-expiry.util.js"; @Injectable() export class JoseJwtService { - private readonly secretKey: Uint8Array; + private readonly signingKey: Uint8Array; + private readonly verificationKeys: Uint8Array[]; private readonly issuer?: string; private readonly audience?: string | string[]; @@ -14,7 +15,14 @@ export class JoseJwtService { if (!secret) { throw new Error("JWT_SECRET is required in environment variables"); } - this.secretKey = new TextEncoder().encode(secret); + this.signingKey = new TextEncoder().encode(secret); + + const previousRaw = configService.get("JWT_SECRET_PREVIOUS"); + const previousSecrets = this.parsePreviousSecrets(previousRaw).filter(s => s !== secret); + this.verificationKeys = [ + this.signingKey, + ...previousSecrets.map(s => new TextEncoder().encode(s)), + ]; const issuer = configService.get("JWT_ISSUER"); this.issuer = issuer && issuer.trim().length > 0 ? issuer.trim() : undefined; @@ -24,6 +32,16 @@ export class JoseJwtService { this.audience = parsedAudience; } + private parsePreviousSecrets(raw: string | undefined): string[] { + if (!raw) return []; + const trimmed = raw.trim(); + if (!trimmed) return []; + return trimmed + .split(",") + .map(s => s.trim()) + .filter(Boolean); + } + private parseAudience(raw: string | undefined): string | string[] | undefined { if (!raw) return undefined; const trimmed = raw.trim(); @@ -36,8 +54,8 @@ export class JoseJwtService { return parts.length === 1 ? parts[0] : parts; } - async sign(payload: JWTPayload, expiresIn: string): Promise { - const expiresInSeconds = parseJwtExpiry(expiresIn); + async sign(payload: JWTPayload, expiresIn: string | number): Promise { + const expiresInSeconds = typeof expiresIn === "number" ? expiresIn : parseJwtExpiry(expiresIn); const nowSeconds = Math.floor(Date.now() / 1000); const tokenId = (payload as { tokenId?: unknown }).tokenId; @@ -60,16 +78,42 @@ export class JoseJwtService { builder = builder.setJti(tokenId); } - return builder.sign(this.secretKey); + return builder.sign(this.signingKey); } async verify(token: string): Promise { - const { payload } = await jwtVerify(token, this.secretKey, { + const options = { algorithms: ["HS256"], issuer: this.issuer, audience: this.audience, - }); - return payload as T; + }; + + let lastError: unknown; + + for (let i = 0; i < this.verificationKeys.length; i++) { + const key = this.verificationKeys[i]; + try { + const { payload } = await jwtVerify(token, key, options); + return payload as T; + } catch (err) { + lastError = err; + const isLast = i === this.verificationKeys.length - 1; + if (isLast) { + break; + } + + // Only try the next key on signature-related failures. + if (err instanceof errors.JWSSignatureVerificationFailed) { + continue; + } + throw err; + } + } + + if (lastError instanceof Error) { + throw lastError; + } + throw new Error("JWT verification failed"); } async verifyAllowExpired(token: string): Promise { diff --git a/apps/bff/src/modules/auth/infra/token/token-blacklist.service.ts b/apps/bff/src/modules/auth/infra/token/token-blacklist.service.ts index 87f14a59..b739996c 100644 --- a/apps/bff/src/modules/auth/infra/token/token-blacklist.service.ts +++ b/apps/bff/src/modules/auth/infra/token/token-blacklist.service.ts @@ -26,9 +26,14 @@ export class TokenBlacklistService { return; } - // Use JwtService to safely decode and validate token + // Safety notes: + // - Logout can be called on an expired session and is public. + // - We MUST avoid writing arbitrary attacker-controlled tokens to Redis. + // - Therefore, only blacklist tokens that have a valid signature (allowing expiry). try { - const decoded = this.jwtService.decode<{ sub?: string; exp?: number }>(token); + const decoded = await this.jwtService.verifyAllowExpired<{ sub?: string; exp?: number }>( + token + ); if (!decoded || typeof decoded !== "object" || Array.isArray(decoded)) { this.logger.warn("Invalid JWT payload structure for blacklisting"); @@ -43,7 +48,12 @@ export class TokenBlacklistService { const expiryTime = exp * 1000; // Convert to milliseconds const currentTime = Date.now(); - const ttl = Math.max(0, Math.floor((expiryTime - currentTime) / 1000)); // Convert to seconds + const rawTtl = Math.max(0, Math.floor((expiryTime - currentTime) / 1000)); // seconds + + // Cap TTL to prevent extremely long-lived keys in case of mis-issued tokens. + // Access tokens should be short-lived anyway. + const maxTtlSeconds = 60 * 60 * 24 * 30; // 30 days + const ttl = Math.min(rawTtl, maxTtlSeconds); if (ttl > 0) { await this.redis.setex(this.buildBlacklistKey(token), ttl, "1"); @@ -55,7 +65,9 @@ export class TokenBlacklistService { // If we can't parse the token, blacklist it for the default JWT expiry time try { const defaultTtl = parseJwtExpiry(this.configService.get("JWT_EXPIRES_IN", "7d")); - await this.redis.setex(this.buildBlacklistKey(token), defaultTtl, "1"); + const maxTtlSeconds = 60 * 60 * 24 * 30; // 30 days + const ttl = Math.min(defaultTtl, maxTtlSeconds); + await this.redis.setex(this.buildBlacklistKey(token), ttl, "1"); this.logger.debug(`Token blacklisted with default TTL: ${defaultTtl} seconds`); } catch (err) { this.logger.warn( diff --git a/apps/bff/src/modules/auth/infra/token/token.service.ts b/apps/bff/src/modules/auth/infra/token/token.service.ts index d8245308..362dcf68 100644 --- a/apps/bff/src/modules/auth/infra/token/token.service.ts +++ b/apps/bff/src/modules/auth/infra/token/token.service.ts @@ -17,6 +17,15 @@ import { JoseJwtService } from "./jose-jwt.service.js"; export interface RefreshTokenPayload extends JWTPayload { userId: string; + /** + * Refresh token family identifier (stable across rotations). + * Present on newly issued tokens; legacy tokens used `tokenId` for this value. + */ + familyId?: string; + /** + * Refresh token identifier (unique per token). Used for replay/reuse detection. + * For legacy tokens, this was equal to the family id. + */ tokenId: string; deviceId?: string; userAgent?: string; @@ -35,6 +44,11 @@ interface StoredRefreshTokenFamily { deviceId?: string; userAgent?: string; createdAt?: string; + /** + * Absolute refresh-session expiration timestamp (ISO). + * Used to avoid indefinitely extending sessions on refresh (RFC 9700 guidance). + */ + absoluteExpiresAt?: string; } @Injectable() @@ -103,22 +117,24 @@ export class AuthTokenService { ): Promise { this.checkServiceAvailability(); - const tokenId = this.generateTokenId(); - const familyId = this.generateTokenId(); + const accessTokenId = this.generateTokenId(); + const refreshFamilyId = this.generateTokenId(); + const refreshTokenId = this.generateTokenId(); // Create access token payload const accessPayload = { sub: user.id, email: user.email, role: user.role || "USER", - tokenId, + tokenId: accessTokenId, type: "access", }; // Create refresh token payload const refreshPayload: RefreshTokenPayload = { userId: user.id, - tokenId: familyId, + familyId: refreshFamilyId, + tokenId: refreshTokenId, deviceId: deviceInfo?.deviceId, userAgent: deviceInfo?.userAgent, type: "refresh", @@ -127,20 +143,24 @@ export class AuthTokenService { // Generate tokens const accessToken = await this.jwtService.sign(accessPayload, this.ACCESS_TOKEN_EXPIRY); - const refreshToken = await this.jwtService.sign(refreshPayload, this.REFRESH_TOKEN_EXPIRY); + const refreshExpirySeconds = this.parseExpiryToSeconds(this.REFRESH_TOKEN_EXPIRY); + const refreshToken = await this.jwtService.sign(refreshPayload, refreshExpirySeconds); // Store refresh token family in Redis const refreshTokenHash = this.hashToken(refreshToken); - const refreshExpirySeconds = this.parseExpiryToSeconds(this.REFRESH_TOKEN_EXPIRY); + const refreshAbsoluteExpiresAt = new Date( + Date.now() + refreshExpirySeconds * 1000 + ).toISOString(); if (this.redis.status === "ready") { try { await this.storeRefreshTokenInRedis( user.id, - familyId, + refreshFamilyId, refreshTokenHash, deviceInfo, - refreshExpirySeconds + refreshExpirySeconds, + refreshAbsoluteExpiresAt ); } catch (error) { this.logger.error("Failed to store refresh token in Redis", { @@ -169,11 +189,14 @@ export class AuthTokenService { const accessExpiresAt = new Date( Date.now() + this.parseExpiryToMs(this.ACCESS_TOKEN_EXPIRY) ).toISOString(); - const refreshExpiresAt = new Date( - Date.now() + this.parseExpiryToMs(this.REFRESH_TOKEN_EXPIRY) - ).toISOString(); + const refreshExpiresAt = refreshAbsoluteExpiresAt; - this.logger.debug("Generated new token pair", { userId: user.id, tokenId, familyId }); + this.logger.debug("Generated new token pair", { + userId: user.id, + accessTokenId, + refreshFamilyId, + refreshTokenId, + }); return { accessToken, @@ -223,12 +246,22 @@ export class AuthTokenService { throw new UnauthorizedException("Invalid refresh token"); } + const familyId = + typeof payload.familyId === "string" && payload.familyId.length > 0 + ? payload.familyId + : payload.tokenId; // legacy tokens used tokenId as family id + const refreshTokenHash = this.hashToken(refreshToken); + const familyKey = `${this.REFRESH_TOKEN_FAMILY_PREFIX}${familyId}`; + const tokenKey = `${this.REFRESH_TOKEN_PREFIX}${refreshTokenHash}`; // Check if refresh token exists and is valid let storedToken: string | null; + let familyData: string | null; try { - storedToken = await this.redis.get(`${this.REFRESH_TOKEN_PREFIX}${refreshTokenHash}`); + const results = await Promise.all([this.redis.get(tokenKey), this.redis.get(familyKey)]); + storedToken = results[0]; + familyData = results[1]; } catch (error) { this.logger.error("Redis error during token refresh", { error: error instanceof Error ? error.message : String(error), @@ -240,6 +273,8 @@ export class AuthTokenService { this.logger.warn("Refresh token not found or expired", { tokenHash: refreshTokenHash.slice(0, 8), }); + // Best-effort: treat this as a replay/reuse signal and revoke the family. + await this.invalidateTokenFamily(familyId); throw new UnauthorizedException("Invalid refresh token"); } @@ -248,7 +283,15 @@ export class AuthTokenService { this.logger.warn("Stored refresh token payload was invalid JSON", { tokenHash: refreshTokenHash.slice(0, 8), }); - await this.redis.del(`${this.REFRESH_TOKEN_PREFIX}${refreshTokenHash}`); + await this.redis.del(tokenKey); + throw new UnauthorizedException("Invalid refresh token"); + } + + if (tokenRecord.familyId !== familyId || tokenRecord.userId !== payload.userId) { + this.logger.warn("Refresh token record mismatch", { + tokenHash: refreshTokenHash.slice(0, 8), + }); + await this.invalidateTokenFamily(tokenRecord.familyId); throw new UnauthorizedException("Invalid refresh token"); } @@ -261,6 +304,50 @@ export class AuthTokenService { throw new UnauthorizedException("Invalid refresh token"); } + const family = familyData ? this.parseRefreshTokenFamilyRecord(familyData) : null; + if (family && family.tokenHash !== refreshTokenHash) { + // Token record says it's valid, but it's not the family's current token hash: + // treat this as reuse or out-of-order rotation and revoke family. + this.logger.warn("Refresh token does not match current family token", { + familyId: familyId.slice(0, 8), + tokenHash: refreshTokenHash.slice(0, 8), + }); + await this.invalidateTokenFamily(familyId); + throw new UnauthorizedException("Invalid refresh token"); + } + + // Determine remaining lifetime for this refresh session (absolute, not sliding). + let remainingSeconds: number | null = null; + let absoluteExpiresAt: string | undefined = family?.absoluteExpiresAt; + + if (absoluteExpiresAt) { + const absMs = Date.parse(absoluteExpiresAt); + if (!Number.isNaN(absMs)) { + remainingSeconds = Math.max(0, Math.floor((absMs - Date.now()) / 1000)); + } else { + absoluteExpiresAt = undefined; + } + } + + if (remainingSeconds === null) { + const ttl = await this.redis.ttl(familyKey); + if (typeof ttl === "number" && ttl > 0) { + remainingSeconds = ttl; + } else { + const tokenTtl = await this.redis.ttl(tokenKey); + remainingSeconds = + typeof tokenTtl === "number" && tokenTtl > 0 + ? tokenTtl + : this.parseExpiryToSeconds(this.REFRESH_TOKEN_EXPIRY); + } + absoluteExpiresAt = new Date(Date.now() + remainingSeconds * 1000).toISOString(); + } + + if (!remainingSeconds || remainingSeconds <= 0) { + await this.invalidateTokenFamily(familyId); + throw new UnauthorizedException("Invalid refresh token"); + } + // Get user info from database (using internal method to get role) const user = await this.usersFacade.findByIdInternal(payload.userId); if (!user) { @@ -271,16 +358,87 @@ export class AuthTokenService { // Convert to the format expected by generateTokenPair const userProfile = mapPrismaUserToDomain(user); - // Invalidate current refresh token - await this.redis.del(`${this.REFRESH_TOKEN_PREFIX}${refreshTokenHash}`); + // Mark current refresh token as invalid (keep it for the remaining TTL so reuse is detectable). + await this.redis.setex( + tokenKey, + remainingSeconds, + JSON.stringify({ + familyId: tokenRecord.familyId, + userId: tokenRecord.userId, + valid: false, + }) + ); - // Generate new token pair - const newTokenPair = await this.generateTokenPair(user, deviceInfo); + // Generate new token pair (keep family id, do not extend absolute lifetime). + const accessTokenId = this.generateTokenId(); + const refreshTokenId = this.generateTokenId(); + + const accessPayload = { + sub: user.id, + email: user.email, + role: user.role || "USER", + tokenId: accessTokenId, + type: "access", + }; + + const newRefreshPayload: RefreshTokenPayload = { + userId: user.id, + familyId, + tokenId: refreshTokenId, + deviceId: deviceInfo?.deviceId, + userAgent: deviceInfo?.userAgent, + type: "refresh", + }; + + const newAccessToken = await this.jwtService.sign(accessPayload, this.ACCESS_TOKEN_EXPIRY); + const newRefreshToken = await this.jwtService.sign(newRefreshPayload, remainingSeconds); + const newRefreshTokenHash = this.hashToken(newRefreshToken); + + const createdAt = family?.createdAt ?? new Date().toISOString(); + const refreshExpiresAt = + absoluteExpiresAt ?? new Date(Date.now() + remainingSeconds * 1000).toISOString(); + const userFamilySetKey = `${this.REFRESH_USER_SET_PREFIX}${user.id}`; + + const pipeline = this.redis.pipeline(); + pipeline.setex( + familyKey, + remainingSeconds, + JSON.stringify({ + userId: user.id, + tokenHash: newRefreshTokenHash, + deviceId: deviceInfo?.deviceId, + userAgent: deviceInfo?.userAgent, + createdAt, + absoluteExpiresAt: refreshExpiresAt, + }) + ); + pipeline.setex( + `${this.REFRESH_TOKEN_PREFIX}${newRefreshTokenHash}`, + remainingSeconds, + JSON.stringify({ + familyId, + userId: user.id, + valid: true, + }) + ); + pipeline.sadd(userFamilySetKey, familyId); + pipeline.expire(userFamilySetKey, remainingSeconds); + await pipeline.exec(); + + const accessExpiresAt = new Date( + Date.now() + this.parseExpiryToMs(this.ACCESS_TOKEN_EXPIRY) + ).toISOString(); this.logger.debug("Refreshed token pair", { userId: payload.userId }); return { - tokens: newTokenPair, + tokens: { + accessToken: newAccessToken, + refreshToken: newRefreshToken, + expiresAt: accessExpiresAt, + refreshExpiresAt, + tokenType: "Bearer", + }, user: userProfile, }; } catch (error) { @@ -336,10 +494,12 @@ export class AuthTokenService { familyId: string, refreshTokenHash: string, deviceInfo?: { deviceId?: string; userAgent?: string }, - refreshExpirySeconds?: number + refreshExpirySeconds?: number, + absoluteExpiresAt?: string ): Promise { const expiry = refreshExpirySeconds || this.parseExpiryToSeconds(this.REFRESH_TOKEN_EXPIRY); const userFamilySetKey = `${this.REFRESH_USER_SET_PREFIX}${userId}`; + const absolute = absoluteExpiresAt ?? new Date(Date.now() + expiry * 1000).toISOString(); const pipeline = this.redis.pipeline(); @@ -353,6 +513,7 @@ export class AuthTokenService { deviceId: deviceInfo?.deviceId, userAgent: deviceInfo?.userAgent, createdAt: new Date().toISOString(), + absoluteExpiresAt: absolute, }) ); @@ -664,6 +825,8 @@ export class AuthTokenService { deviceId: typeof parsed.deviceId === "string" ? parsed.deviceId : undefined, userAgent: typeof parsed.userAgent === "string" ? parsed.userAgent : undefined, createdAt: typeof parsed.createdAt === "string" ? parsed.createdAt : undefined, + absoluteExpiresAt: + typeof parsed.absoluteExpiresAt === "string" ? parsed.absoluteExpiresAt : undefined, }; } } catch (error) { diff --git a/apps/bff/src/modules/auth/infra/workflows/workflows/password-workflow.service.ts b/apps/bff/src/modules/auth/infra/workflows/workflows/password-workflow.service.ts index 1a08d369..97698039 100644 --- a/apps/bff/src/modules/auth/infra/workflows/workflows/password-workflow.service.ts +++ b/apps/bff/src/modules/auth/infra/workflows/workflows/password-workflow.service.ts @@ -17,11 +17,11 @@ import { AuthTokenService } from "../../token/token.service.js"; import { AuthRateLimitService } from "../../rate-limiting/auth-rate-limit.service.js"; import { JoseJwtService } from "../../token/jose-jwt.service.js"; import { - type PasswordChangeResult, type ChangePasswordRequest, changePasswordRequestSchema, } from "@customer-portal/domain/auth"; import { mapPrismaUserToDomain } from "@bff/infra/mappers/index.js"; +import type { AuthResultInternal } from "@bff/modules/auth/auth.types.js"; @Injectable() export class PasswordWorkflowService { @@ -125,7 +125,7 @@ export class PasswordWorkflowService { } } - async resetPassword(token: string, newPassword: string): Promise { + async resetPassword(token: string, newPassword: string): Promise { try { const payload = await this.jwtService.verify<{ sub: string; purpose: string }>(token); if (payload.purpose !== "password_reset") { @@ -142,17 +142,9 @@ export class PasswordWorkflowService { if (!freshUser) { throw new Error("Failed to load user after password reset"); } - const userProfile = mapPrismaUserToDomain(freshUser); - - const tokens = await this.tokenService.generateTokenPair({ - id: userProfile.id, - email: userProfile.email, - }); - - return { - user: userProfile, - tokens, - }; + // Force re-login everywhere after password reset + await this.tokenService.revokeAllUserTokens(freshUser.id); + return; } catch (error: unknown) { this.logger.error("Reset password failed", { error: getErrorMessage(error) }); throw new BadRequestException("Invalid or expired token"); @@ -163,7 +155,7 @@ export class PasswordWorkflowService { userId: string, data: ChangePasswordRequest, request?: Request - ): Promise { + ): Promise { const user = await this.usersFacade.findByIdInternal(userId); if (!user) { @@ -210,6 +202,9 @@ export class PasswordWorkflowService { true ); + // Revoke existing refresh tokens before issuing new pair (logout other sessions) + await this.tokenService.revokeAllUserTokens(userProfile.id); + const tokens = await this.tokenService.generateTokenPair({ id: userProfile.id, email: userProfile.email, diff --git a/apps/bff/src/modules/auth/infra/workflows/workflows/signup-workflow.service.ts b/apps/bff/src/modules/auth/infra/workflows/workflows/signup-workflow.service.ts index 9b097d0a..074743b1 100644 --- a/apps/bff/src/modules/auth/infra/workflows/workflows/signup-workflow.service.ts +++ b/apps/bff/src/modules/auth/infra/workflows/workflows/signup-workflow.service.ts @@ -21,7 +21,6 @@ import { getErrorMessage } from "@bff/core/utils/error.util.js"; import { signupRequestSchema, type SignupRequest, - type SignupResult, type ValidateSignupRequest, } from "@customer-portal/domain/auth"; import { mapPrismaUserToDomain } from "@bff/infra/mappers/index.js"; @@ -32,6 +31,7 @@ import { PORTAL_STATUS_ACTIVE, type PortalRegistrationSource, } from "@bff/modules/auth/constants/portal.constants.js"; +import type { AuthResultInternal } from "@bff/modules/auth/auth.types.js"; type _SanitizedPrismaUser = Omit< PrismaUser, @@ -146,7 +146,7 @@ export class SignupWorkflowService { } } - async signup(signupData: SignupRequest, request?: Request): Promise { + async signup(signupData: SignupRequest, request?: Request): Promise { if (request) { await this.authRateLimitService.consumeSignupAttempt(request); } diff --git a/apps/bff/src/modules/auth/presentation/http/auth.controller.ts b/apps/bff/src/modules/auth/presentation/http/auth.controller.ts index 9beadeab..6e3805e8 100644 --- a/apps/bff/src/modules/auth/presentation/http/auth.controller.ts +++ b/apps/bff/src/modules/auth/presentation/http/auth.controller.ts @@ -54,7 +54,6 @@ import { type SsoLinkRequest, type CheckPasswordNeededRequest, type RefreshTokenRequest, - type AuthTokens, } from "@customer-portal/domain/auth"; type CookieValue = string | undefined; @@ -72,6 +71,7 @@ const calculateCookieMaxAge = (isoTimestamp: string): number => { const ACCESS_COOKIE_PATH = "/api"; const REFRESH_COOKIE_PATH = "/api/auth/refresh"; +const TOKEN_TYPE = "Bearer" as const; @Controller("auth") export class AuthController { @@ -80,7 +80,15 @@ export class AuthController { private readonly jwtService: JoseJwtService ) {} - private setAuthCookies(res: Response, tokens: AuthTokens): void { + private setAuthCookies( + res: Response, + tokens: { + accessToken: string; + refreshToken: string; + expiresAt: string; + refreshExpiresAt: string; + } + ): void { const accessMaxAge = calculateCookieMaxAge(tokens.expiresAt); const refreshMaxAge = calculateCookieMaxAge(tokens.refreshExpiresAt); @@ -94,6 +102,14 @@ export class AuthController { }); } + private toSession(tokens: { expiresAt: string; refreshExpiresAt: string }) { + return { + expiresAt: tokens.expiresAt, + refreshExpiresAt: tokens.refreshExpiresAt, + tokenType: TOKEN_TYPE, + }; + } + private clearAuthCookies(res: Response): void { // Clear current cookie paths res.setSecureCookie("access_token", "", { maxAge: 0, path: ACCESS_COOKIE_PATH }); @@ -152,7 +168,7 @@ export class AuthController { ) { const result = await this.authFacade.signup(signupData, req); this.setAuthCookies(res, result.tokens); - return result; + return { user: result.user, session: this.toSession(result.tokens) }; } @Public() @@ -166,7 +182,7 @@ export class AuthController { const result = await this.authFacade.login(req.user, req); this.setAuthCookies(res, result.tokens); this.applyAuthRateLimitHeaders(req, res); - return result; + return { user: result.user, session: this.toSession(result.tokens) }; } @Public() @@ -210,7 +226,7 @@ export class AuthController { userAgent, }); this.setAuthCookies(res, result.tokens); - return result; + return { user: result.user, session: this.toSession(result.tokens) }; } @Public() @@ -235,7 +251,7 @@ export class AuthController { ) { const result = await this.authFacade.setPassword(setPasswordData); this.setAuthCookies(res, result.tokens); - return result; + return { user: result.user, session: this.toSession(result.tokens) }; } @Public() @@ -285,7 +301,7 @@ export class AuthController { ) { const result = await this.authFacade.changePassword(req.user.id, body, req); this.setAuthCookies(res, result.tokens); - return result; + return { user: result.user, session: this.toSession(result.tokens) }; } @Get("me") diff --git a/apps/portal/src/features/auth/services/auth.store.ts b/apps/portal/src/features/auth/services/auth.store.ts index b65c061b..5e701a51 100644 --- a/apps/portal/src/features/auth/services/auth.store.ts +++ b/apps/portal/src/features/auth/services/auth.store.ts @@ -12,7 +12,7 @@ import { authResponseSchema, checkPasswordNeededResponseSchema, linkWhmcsResponseSchema, - type AuthTokens, + type AuthSession, type CheckPasswordNeededResponse, type LinkWhmcsRequest, type LinkWhmcsResponse, @@ -59,7 +59,7 @@ export interface AuthState { type AuthResponseData = { user: AuthenticatedUser; - tokens: AuthTokens; + session: AuthSession; }; export const useAuthStore = create()((set, get) => { @@ -67,8 +67,8 @@ export const useAuthStore = create()((set, get) => { set({ user: data.user, session: { - accessExpiresAt: data.tokens.expiresAt, - refreshExpiresAt: data.tokens.refreshExpiresAt, + accessExpiresAt: data.session.expiresAt, + refreshExpiresAt: data.session.refreshExpiresAt, }, isAuthenticated: true, loading: keepLoading, diff --git a/docs/auth/AUTH-MODULE-ARCHITECTURE.md b/docs/auth/AUTH-MODULE-ARCHITECTURE.md index 7fe5988a..96397c6e 100644 --- a/docs/auth/AUTH-MODULE-ARCHITECTURE.md +++ b/docs/auth/AUTH-MODULE-ARCHITECTURE.md @@ -4,6 +4,17 @@ The authentication feature in `apps/bff` now follows a layered structure that separates HTTP concerns, orchestration logic, and infrastructure details. This document explains the layout, responsibilities, and integration points so new contributors can quickly locate code and understand the data flow. +## Security Model (TL;DR) + +- **Token storage**: Access/refresh token strings are stored in **httpOnly cookies** set by the BFF. +- **API responses**: Auth endpoints return **`user` + `session` (expiry metadata)** — **token strings are never returned to the browser**. +- **Access token validation**: Per-request validation is handled by `GlobalAuthGuard`: + - verifies JWT signature/claims + - rejects wrong token type (expects `type: "access"` when present) + - checks Redis blacklist (revoked access tokens) +- **Refresh token rotation**: `/api/auth/refresh` uses Redis-backed rotation with reuse detection and an **absolute refresh lifetime** (no indefinite extension). +- **CSRF**: Portal fetches token via `GET /api/security/csrf/token` and sends it as `X-CSRF-Token` on unsafe requests. + ``` modules/auth/ ├── application/ @@ -94,6 +105,11 @@ LOGIN_RATE_LIMIT_TTL=900000 AUTH_REFRESH_RATE_LIMIT_LIMIT=10 AUTH_REFRESH_RATE_LIMIT_TTL=300000 +# JWT hardening (optional but recommended in prod) +JWT_ISSUER=customer-portal +JWT_AUDIENCE=portal +JWT_SECRET_PREVIOUS=oldsecret1,oldsecret2 + # CAPTCHA options (future integration) AUTH_CAPTCHA_PROVIDER=none # none|turnstile|hcaptcha AUTH_CAPTCHA_SECRET= @@ -105,6 +121,9 @@ CSRF_TOKEN_EXPIRY=3600000 CSRF_SECRET_KEY=... # recommended in prod CSRF_COOKIE_NAME=csrf-secret CSRF_HEADER_NAME=X-CSRF-Token + +# Redis failure-mode (security vs availability tradeoff) +AUTH_BLACKLIST_FAIL_CLOSED=false ``` Refer to `DEVELOPMENT-AUTH-SETUP.md` for dev-only overrides (`DISABLE_CSRF`, etc.). diff --git a/docs/auth/AUTH-SCHEMA-IMPROVEMENTS.md b/docs/auth/AUTH-SCHEMA-IMPROVEMENTS.md index 89e82136..d99bd874 100644 --- a/docs/auth/AUTH-SCHEMA-IMPROVEMENTS.md +++ b/docs/auth/AUTH-SCHEMA-IMPROVEMENTS.md @@ -5,21 +5,24 @@ ### 1. **`z.unknown()` Type Safety Issue** ❌ → ✅ **Problem:** + ```typescript // BEFORE - NO TYPE SAFETY export const signupResultSchema = z.object({ - user: z.unknown(), // ❌ Loses all type information and validation + user: z.unknown(), // ❌ Loses all type information and validation tokens: authTokensSchema, }); ``` **Why it was wrong:** + - `z.unknown()` provides **zero validation** at runtime - TypeScript can't infer proper types from it - Defeats the purpose of schema-first architecture - Any object could pass validation, even malformed data **Solution:** + ```typescript // AFTER - FULL TYPE SAFETY export const userProfileSchema = z.object({ @@ -30,12 +33,13 @@ export const userProfileSchema = z.object({ }); export const signupResultSchema = z.object({ - user: userProfileSchema, // ✅ Full validation and type inference + user: userProfileSchema, // ✅ Full validation and type inference tokens: authTokensSchema, }); ``` **Benefits:** + - Runtime validation of user data structure - Proper TypeScript type inference - Catches invalid data early @@ -49,9 +53,10 @@ export const signupResultSchema = z.object({ The `z.string().email()` in `checkPasswordNeededResponseSchema` was correct, but using `z.unknown()` elsewhere meant emails weren't being validated in user objects. **Solution:** + ```typescript export const userProfileSchema = z.object({ - email: z.string().email(), // ✅ Validates email format + email: z.string().email(), // ✅ Validates email format // ... }); ``` @@ -63,6 +68,7 @@ Now all user objects have their emails properly validated. ### 3. **UserProfile Alias Confusion** 🤔 → ✅ **Problem:** + ```typescript export type UserProfile = AuthenticatedUser; ``` @@ -70,13 +76,14 @@ export type UserProfile = AuthenticatedUser; This created confusion about why we have two names for the same thing. **Solution - Added Clear Documentation:** + ```typescript /** * UserProfile type alias - * - * Note: This is an alias for backward compatibility. + * + * Note: This is an alias for backward compatibility. * Both types represent the same thing: a complete user profile with auth state. - * + * * Architecture: * - PortalUser: Only auth state from portal DB (id, email, role, emailVerified, etc.) * - CustomerProfile: Profile data from WHMCS (firstname, lastname, address, etc.) @@ -86,6 +93,7 @@ export type UserProfile = AuthenticatedUser; ``` **Why the alias exists:** + - **Backward compatibility**: Code may use either name - **Domain language**: "UserProfile" is more business-friendly than "AuthenticatedUser" - **Convention**: Many authentication libraries use "UserProfile" @@ -116,6 +124,7 @@ export type UserProfile = AuthenticatedUser; ``` ### 1. **PortalUser** (Portal Database) + - **Purpose**: Authentication state only - **Source**: Portal's Prisma database - **Fields**: id, email, role, emailVerified, mfaEnabled, lastLoginAt @@ -123,6 +132,7 @@ export type UserProfile = AuthenticatedUser; - **Use Cases**: JWT validation, token refresh, auth checks ### 2. **CustomerProfile** (WHMCS) + - **Purpose**: Business profile data - **Source**: WHMCS API (single source of truth for profile data) - **Fields**: firstname, lastname, address, phone, language, currency @@ -130,6 +140,7 @@ export type UserProfile = AuthenticatedUser; - **Use Cases**: Displaying user info, profile updates ### 3. **AuthenticatedUser / UserProfile** (Combined) + - **Purpose**: Complete user representation - **Source**: Portal DB + WHMCS (merged) - **Fields**: All fields from PortalUser + CustomerProfile @@ -141,16 +152,20 @@ export type UserProfile = AuthenticatedUser; ## Why This Matters ### Before (with `z.unknown()`): + ```typescript // Could pass completely invalid data ❌ const badData = { user: { totally: "wrong", structure: true }, - tokens: { /* ... */ } + tokens: { + /* ... */ + }, }; // Would validate successfully! ❌ ``` ### After (with proper schema): + ```typescript // Validates structure correctly ✅ const goodData = { @@ -160,13 +175,17 @@ const goodData = { role: "USER", // ... all required fields }, - tokens: { /* ... */ } + tokens: { + /* ... */ + }, }; // Validates ✅ const badData = { user: { wrong: "structure" }, - tokens: { /* ... */ } + tokens: { + /* ... */ + }, }; // Throws validation error ✅ ``` @@ -187,7 +206,7 @@ export const userProfileSchema = z.object({ // 2. Then use it in response schemas export const authResponseSchema = z.object({ - user: userProfileSchema, // ✅ Now defined + user: userProfileSchema, // ✅ Now defined tokens: authTokensSchema, }); ``` @@ -214,11 +233,14 @@ type InferredUser = z.infer; ## Migration Impact -### ✅ No Breaking Changes +### ⚠️ Potential Breaking Change (Security Improvement) -- Existing code continues to work -- `UserProfile` and `AuthenticatedUser` still exist -- Only **adds** validation, doesn't remove functionality +Auth endpoints now return **session metadata** instead of **token strings**: + +- **Before**: `{ user, tokens: { accessToken, refreshToken, expiresAt, refreshExpiresAt } }` +- **After**: `{ user, session: { expiresAt, refreshExpiresAt } }` + +Token strings are delivered via **httpOnly cookies** only. ### ✅ Improved Type Safety @@ -236,14 +258,13 @@ type InferredUser = z.infer; ## Summary -| Issue | Before | After | -|-------|--------|-------| -| User validation | `z.unknown()` ❌ | `userProfileSchema` ✅ | -| Type safety | None | Full | -| Runtime validation | None | Complete | -| Documentation | Unclear | Self-documenting | -| Email validation | Partial | Complete | -| UserProfile clarity | Confusing | Documented | +| Issue | Before | After | +| ------------------- | ---------------- | ---------------------- | +| User validation | `z.unknown()` ❌ | `userProfileSchema` ✅ | +| Type safety | None | Full | +| Runtime validation | None | Complete | +| Documentation | Unclear | Self-documenting | +| Email validation | Partial | Complete | +| UserProfile clarity | Confusing | Documented | **Result**: Proper schema-first architecture with full type safety and validation! 🎉 - diff --git a/packages/domain/auth/contract.ts b/packages/domain/auth/contract.ts index 72a37ef6..cd168dda 100644 --- a/packages/domain/auth/contract.ts +++ b/packages/domain/auth/contract.ts @@ -1,6 +1,6 @@ /** * Auth Domain - Contract - * + * * Constants and types for the authentication domain. * All validated types are derived from schemas (see schema.ts). */ @@ -68,6 +68,7 @@ export type { RefreshTokenRequest, // Token types AuthTokens, + AuthSession, // Response types AuthResponse, SignupResult, @@ -77,4 +78,4 @@ export type { LinkWhmcsResponse, // Error types AuthError, -} from './schema.js'; +} from "./schema.js"; diff --git a/packages/domain/auth/index.ts b/packages/domain/auth/index.ts index e8d19e4e..8c95ac07 100644 --- a/packages/domain/auth/index.ts +++ b/packages/domain/auth/index.ts @@ -1,11 +1,11 @@ /** * Auth Domain - * + * * Contains ONLY authentication mechanisms: * - Login, Signup, Password Management * - Token Management (JWT) * - MFA, SSO - * + * * User entity types are in customer domain (@customer-portal/domain/customer) */ @@ -39,6 +39,7 @@ export type { RefreshTokenRequest, // Token types AuthTokens, + AuthSession, // Response types AuthResponse, SignupResult, @@ -72,10 +73,11 @@ export { ssoLinkRequestSchema, checkPasswordNeededRequestSchema, refreshTokenRequestSchema, - + // Token schemas authTokensSchema, - + authSessionSchema, + // Response schemas authResponseSchema, signupResultSchema, diff --git a/packages/domain/auth/schema.ts b/packages/domain/auth/schema.ts index 41c8b0ce..963fe2ab 100644 --- a/packages/domain/auth/schema.ts +++ b/packages/domain/auth/schema.ts @@ -1,11 +1,11 @@ /** * Auth Domain - Types - * + * * Contains ONLY authentication mechanism types: * - Login, Signup, Password Management * - Token Management * - MFA, SSO - * + * * User entity types are in customer domain. * Auth responses reference User from customer domain. */ @@ -95,7 +95,7 @@ export const updateCustomerProfileRequestSchema = z.object({ lastname: nameSchema.optional(), companyname: z.string().max(100).optional(), phonenumber: phoneSchema.optional(), - + // Address (optional fields for partial updates) address1: z.string().max(200).optional(), address2: z.string().max(200).optional(), @@ -103,7 +103,7 @@ export const updateCustomerProfileRequestSchema = z.object({ state: z.string().max(100).optional(), postcode: z.string().max(20).optional(), country: z.string().length(2).optional(), // ISO country code - + // Additional language: z.string().max(10).optional(), }); @@ -140,6 +140,19 @@ export const authTokensSchema = z.object({ tokenType: z.literal("Bearer"), }); +/** + * Auth session metadata returned to clients. + * + * Security note: + * - Token strings are stored in httpOnly cookies and MUST NOT be returned to the browser. + * - The client only needs expiry metadata for UX (timeout warnings, refresh scheduling). + */ +export const authSessionSchema = z.object({ + expiresAt: z.string().min(1, "Access token expiry required"), + refreshExpiresAt: z.string().min(1, "Refresh token expiry required"), + tokenType: z.literal("Bearer"), +}); + // ============================================================================ // Authentication Response Schemas (Reference User from Customer Domain) // ============================================================================ @@ -148,24 +161,24 @@ export const authTokensSchema = z.object({ * Auth response - returns User from customer domain */ export const authResponseSchema = z.object({ - user: userSchema, // User from customer domain - tokens: authTokensSchema, + user: userSchema, // User from customer domain + session: authSessionSchema, }); /** * Signup result - returns User from customer domain */ export const signupResultSchema = z.object({ - user: userSchema, // User from customer domain - tokens: authTokensSchema, + user: userSchema, // User from customer domain + session: authSessionSchema, }); /** * Password change result - returns User from customer domain */ export const passwordChangeResultSchema = z.object({ - user: userSchema, // User from customer domain - tokens: authTokensSchema, + user: userSchema, // User from customer domain + session: authSessionSchema, }); /** @@ -214,6 +227,7 @@ export type RefreshTokenRequest = z.infer; // Token types export type AuthTokens = z.infer; +export type AuthSession = z.infer; // Response types export type AuthResponse = z.infer; @@ -228,8 +242,18 @@ export type LinkWhmcsResponse = z.infer; // ============================================================================ export interface AuthError { - code: "INVALID_CREDENTIALS" | "EMAIL_NOT_VERIFIED" | "ACCOUNT_LOCKED" | "MFA_REQUIRED" | "INVALID_TOKEN" | "TOKEN_EXPIRED" | "PASSWORD_TOO_WEAK" | "EMAIL_ALREADY_EXISTS" | "WHMCS_ACCOUNT_NOT_FOUND" | "SALESFORCE_ACCOUNT_NOT_FOUND" | "LINKING_FAILED"; + code: + | "INVALID_CREDENTIALS" + | "EMAIL_NOT_VERIFIED" + | "ACCOUNT_LOCKED" + | "MFA_REQUIRED" + | "INVALID_TOKEN" + | "TOKEN_EXPIRED" + | "PASSWORD_TOO_WEAK" + | "EMAIL_ALREADY_EXISTS" + | "WHMCS_ACCOUNT_NOT_FOUND" + | "SALESFORCE_ACCOUNT_NOT_FOUND" + | "LINKING_FAILED"; message: string; details?: unknown; } -