feat: add authentication provider abstraction#4
Conversation
- Create IdentityProvider interface for OAuth provider abstraction - Add GoogleProvider using google-auth-library - Add JwtTokenService for self-managed RS256 JWT generation - Add SelfIssuedJwtStrategy for Passport integration - Add /users/oauth/google endpoint for Google sign-in - Add key generation script (npm run generate:keys) - Update auth module with optional Google provider - Update UsersService with loginWithGoogle method 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
- Add GoogleProvider tests (12 tests): - Token verification - Error handling (missing email, null payload) - Provider ID formatting - Missing GOOGLE_CLIENT_ID validation - Add JwtTokenService tests (24 tests): - Token generation (access and refresh) - Token verification and expiration - RS256 signing algorithm - Key loading and availability - ExpiresIn parsing (seconds, minutes, hours, days) - Add SelfIssuedJwtStrategy tests (7 tests): - Payload validation - Role preservation - Email handling Total: 43 tests for auth abstraction layer 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
WalkthroughThis PR introduces JWT token service with RSA-based signing, Google OAuth provider integration, and supporting infrastructure. Changes include environment variable configuration, key generation script, JWT strategies, Google provider implementation, and updates to the users module to support Google OAuth login. Changes
Sequence Diagram(s)sequenceDiagram
actor Client
participant Controller as UsersController
participant Service as UsersService
participant GoogleProvider
participant Database as DB
participant JwtService as JwtTokenService
Client->>Controller: POST /oauth/google<br/>(idToken)
Controller->>Service: loginWithGoogle(idToken)
Service->>GoogleProvider: verifyToken(idToken)
GoogleProvider-->>Service: IdentityProviderUser<br/>(providerId, email, etc.)
alt User Exists
Service->>Database: findByGoogleProviderId
Database-->>Service: User
Service->>Database: Update user (status, provider IDs, image)
else User Not Found
Service->>Database: Create new user
Database-->>Service: User
end
Service->>Database: findUserAccount(userId)
Database-->>Service: UserAccount
Service->>JwtService: generateTokenPair(sub, email,<br/>accountId, role)
JwtService-->>Service: TokenPair<br/>(accessToken, refreshToken)
Service-->>Controller: OAuthTokenResponse<br/>(tokens + user info)
Controller-->>Client: 201 Created<br/>(tokens + user info)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45–60 minutes Key areas requiring focused attention:
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (9)
apps/backend/scripts/generate-keys.sh (1)
24-24: Consider 4096-bit keys for enhanced future-proofing.While 2048-bit RSA keys are currently secure and widely accepted, 4096-bit keys offer better long-term security and are recommended for new implementations.
Apply this diff to use 4096-bit keys:
-openssl genrsa -out "$KEYS_DIR/private.pem" 2048 +openssl genrsa -out "$KEYS_DIR/private.pem" 4096apps/backend/src/auth/jwt/self-issued-jwt.strategy.ts (2)
31-42: Silent failure hides key loading issues.The empty catch block silently swallows file read errors. Consider adding a warning log to help debugging when keys fail to load, similar to
JwtTokenService.loadKeys()which logs a warning.if (publicKeyPath) { try { const basePath = process.cwd(); publicKey = fs.readFileSync( path.resolve(basePath, publicKeyPath), 'utf8', ); - } catch { + } catch (error) { // Keys not available - strategy will fail validation + console.warn( + 'RSA public key not found for self-jwt strategy.', + 'Authentication with self-issued tokens will fail.', + ); } }
25-50: Key loading logic is duplicated with JwtTokenService.Both
SelfIssuedJwtStrategyandJwtTokenServiceindependently load the public key from the same path. Consider injectingJwtTokenServiceand usinggetPublicKey()to avoid duplication and ensure consistency.This would centralize key management:
constructor( private readonly configService: ConfigService, private readonly jwtTokenService: JwtTokenService, ) { const issuer = configService.get<string>('JWT_ISSUER') || 'https://api.boilerplate.local'; let publicKey: string | undefined; if (jwtTokenService.isAvailable()) { publicKey = jwtTokenService.getPublicKey(); } super({ jwtFromRequest: ExtractJwt.fromAuthHeaderAsBearerToken(), secretOrKey: publicKey || 'placeholder-key-will-fail', algorithms: ['RS256'], issuer, }); }apps/backend/src/auth/providers/google.provider.ts (1)
14-22: Avoid re-reading clientId from config on every token verification.The
GOOGLE_CLIENT_IDis already validated in the constructor but re-fetched inverifyToken. Store it as a class member for consistency and minor performance improvement.@Injectable() export class GoogleProvider implements IdentityProvider { readonly providerName = 'google'; private client: OAuth2Client; + private readonly clientId: string; constructor(private config: ConfigService) { const clientId = config.get<string>('GOOGLE_CLIENT_ID'); if (!clientId) { throw new Error( 'GOOGLE_CLIENT_ID is required when using GoogleProvider', ); } + this.clientId = clientId; this.client = new OAuth2Client(clientId); } async verifyToken(idToken: string): Promise<IdentityProviderUser> { const ticket = await this.client.verifyIdToken({ idToken, - audience: this.config.get<string>('GOOGLE_CLIENT_ID'), + audience: this.clientId, });Also applies to: 30-34
apps/backend/src/auth/jwt/jwt.service.spec.ts (1)
24-32: Consider typing mockUser more explicitly.Using
as anybypasses type checking. Consider usingPartial<User>or a proper mock factory to maintain type safety in tests.- const mockUser = { + const mockUser: Partial<User> & { id: string; email: string } = { id: 'user-123', email: 'test@example.com', name: 'Test User', status: 'accepted', providerIds: [], isSuperAdmin: false, profileImage: null, - } as any; + };Then cast only where needed in method calls.
apps/backend/src/auth/jwt/jwt.service.ts (3)
56-60: Use NestJS Logger instead of console.warn.Using
console.warnis inconsistent with NestJS patterns. The framework'sLoggerprovides structured logging with context.+import { Injectable, OnModuleInit, Logger } from '@nestjs/common'; -import { Injectable, OnModuleInit } from '@nestjs/common'; @Injectable() export class JwtTokenService implements OnModuleInit { + private readonly logger = new Logger(JwtTokenService.name); private privateKey: string | null = null; // ... private loadKeys(): void { // ... } catch (error) { - console.warn( + this.logger.warn( 'RSA keys not found. Self-issued JWT will not be available.', - 'Run `npm run generate:keys` to create RSA key pair.', ); + this.logger.warn('Run `npm run generate:keys` to create RSA key pair.'); } }
102-106: Redundant non-null assertions after null checks.Lines 102 and 125 use
this.privateKey!but the null check on lines 91-93 and 116-118 already guarantees non-null. TypeScript should infer this, but if not, consider extracting to a local variable.generateAccessToken(user: User, accountId: string, role: Role): string { if (!this.privateKey) { throw new Error('Private key not loaded. Run `npm run generate:keys`.'); } + const privateKey = this.privateKey; const payload = { /* ... */ }; - return jwt.sign(payload, this.privateKey!, { + return jwt.sign(payload, privateKey, { algorithm: 'RS256', // ... }); }Also applies to: 125-129
125-129: Refresh token expiry is hardcoded.The access token expiry is configurable via
JWT_EXPIRES_IN, but refresh token expiry is hardcoded to'7d'. Consider making this configurable for flexibility.constructor(private config: ConfigService) { this.issuer = config.get<string>('JWT_ISSUER') || 'https://api.boilerplate.local'; this.expiresIn = config.get<string>('JWT_EXPIRES_IN') || '1h'; + this.refreshExpiresIn = config.get<string>('JWT_REFRESH_EXPIRES_IN') || '7d'; }apps/backend/src/auth/providers/identity-provider.interface.ts (1)
1-59: Identity provider abstractions look good; consider naming the invitation result typeThe interface shapes and comments are clear and should work well across providers. One small refinement: the inline object return type of
sendInvitationcould be extracted to a named interface (e.g.,IdentityProviderInvitationResult) to better follow the “prefer interfaces for object definitions” guideline and keep this contract reusable and self-documenting.export interface IdentityProviderInvitationResult { userId: string; ticket?: string; } export interface IdentityProvider { // ... sendInvitation?( email: string, name: string, ): Promise<IdentityProviderInvitationResult>; // ... }This is optional and can be deferred if you prefer to keep the surface minimal for now.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (16)
apps/backend/.env.example(1 hunks)apps/backend/package.json(3 hunks)apps/backend/scripts/generate-keys.sh(1 hunks)apps/backend/src/auth/auth.module.ts(1 hunks)apps/backend/src/auth/jwt/index.ts(1 hunks)apps/backend/src/auth/jwt/jwt.service.spec.ts(1 hunks)apps/backend/src/auth/jwt/jwt.service.ts(1 hunks)apps/backend/src/auth/jwt/self-issued-jwt.strategy.spec.ts(1 hunks)apps/backend/src/auth/jwt/self-issued-jwt.strategy.ts(1 hunks)apps/backend/src/auth/providers/google.provider.spec.ts(1 hunks)apps/backend/src/auth/providers/google.provider.ts(1 hunks)apps/backend/src/auth/providers/identity-provider.interface.ts(1 hunks)apps/backend/src/auth/providers/index.ts(1 hunks)apps/backend/src/modules/users/dto/google-oauth.dto.ts(1 hunks)apps/backend/src/modules/users/users.controller.ts(2 hunks)apps/backend/src/modules/users/users.service.ts(4 hunks)
🧰 Additional context used
📓 Path-based instructions (13)
**/*.{js,ts,tsx,jsx,css,scss,json,yaml,yml,md}
📄 CodeRabbit inference engine (.cursor/rules/cursor.json)
**/*.{js,ts,tsx,jsx,css,scss,json,yaml,yml,md}: Maximum line length should not exceed 100 characters
Trim trailing whitespace from all lines
Insert a final newline at the end of every file
Files:
apps/backend/src/auth/providers/index.tsapps/backend/src/modules/users/dto/google-oauth.dto.tsapps/backend/src/auth/jwt/index.tsapps/backend/src/auth/providers/identity-provider.interface.tsapps/backend/src/auth/providers/google.provider.tsapps/backend/src/modules/users/users.controller.tsapps/backend/src/auth/providers/google.provider.spec.tsapps/backend/src/auth/jwt/jwt.service.spec.tsapps/backend/src/auth/jwt/self-issued-jwt.strategy.tsapps/backend/src/auth/jwt/jwt.service.tsapps/backend/package.jsonapps/backend/src/auth/auth.module.tsapps/backend/src/auth/jwt/self-issued-jwt.strategy.spec.tsapps/backend/src/modules/users/users.service.ts
**/*.{js,ts,tsx,jsx,css,scss,json,yaml,yml}
📄 CodeRabbit inference engine (.cursor/rules/cursor.json)
Use 2-space indentation for all files
Files:
apps/backend/src/auth/providers/index.tsapps/backend/src/modules/users/dto/google-oauth.dto.tsapps/backend/src/auth/jwt/index.tsapps/backend/src/auth/providers/identity-provider.interface.tsapps/backend/src/auth/providers/google.provider.tsapps/backend/src/modules/users/users.controller.tsapps/backend/src/auth/providers/google.provider.spec.tsapps/backend/src/auth/jwt/jwt.service.spec.tsapps/backend/src/auth/jwt/self-issued-jwt.strategy.tsapps/backend/src/auth/jwt/jwt.service.tsapps/backend/package.jsonapps/backend/src/auth/auth.module.tsapps/backend/src/auth/jwt/self-issued-jwt.strategy.spec.tsapps/backend/src/modules/users/users.service.ts
**/*
📄 CodeRabbit inference engine (.cursor/rules/cursor.json)
**/*: Use LF (line feed) as end-of-line character
Use UTF-8 character encoding for all files
Files:
apps/backend/src/auth/providers/index.tsapps/backend/src/modules/users/dto/google-oauth.dto.tsapps/backend/src/auth/jwt/index.tsapps/backend/src/auth/providers/identity-provider.interface.tsapps/backend/src/auth/providers/google.provider.tsapps/backend/src/modules/users/users.controller.tsapps/backend/src/auth/providers/google.provider.spec.tsapps/backend/src/auth/jwt/jwt.service.spec.tsapps/backend/src/auth/jwt/self-issued-jwt.strategy.tsapps/backend/src/auth/jwt/jwt.service.tsapps/backend/scripts/generate-keys.shapps/backend/package.jsonapps/backend/src/auth/auth.module.tsapps/backend/src/auth/jwt/self-issued-jwt.strategy.spec.tsapps/backend/src/modules/users/users.service.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/cursor.json)
**/*.{ts,tsx,js,jsx}: Use single quotes for string literals in TypeScript/JavaScript
Semicolons must be present at the end of statements in TypeScript/JavaScript
Use trailing commas in multiline constructs following ES5 convention
Enable bracket spacing in object literals (e.g., { foo: bar } instead of {foo: bar})
Omit parentheses around single arrow function parameters when possible
Files:
apps/backend/src/auth/providers/index.tsapps/backend/src/modules/users/dto/google-oauth.dto.tsapps/backend/src/auth/jwt/index.tsapps/backend/src/auth/providers/identity-provider.interface.tsapps/backend/src/auth/providers/google.provider.tsapps/backend/src/modules/users/users.controller.tsapps/backend/src/auth/providers/google.provider.spec.tsapps/backend/src/auth/jwt/jwt.service.spec.tsapps/backend/src/auth/jwt/self-issued-jwt.strategy.tsapps/backend/src/auth/jwt/jwt.service.tsapps/backend/src/auth/auth.module.tsapps/backend/src/auth/jwt/self-issued-jwt.strategy.spec.tsapps/backend/src/modules/users/users.service.ts
**/*.{ts,tsx,js,jsx,vue}
📄 CodeRabbit inference engine (.cursor/rules/etus-methodology.mdc)
**/*.{ts,tsx,js,jsx,vue}: All UI component imports must use@/components/ui/*from @ETUS Design System, fail with error if components are imported from outside this path
Do not create components outside @ETUS Design System
Files:
apps/backend/src/auth/providers/index.tsapps/backend/src/modules/users/dto/google-oauth.dto.tsapps/backend/src/auth/jwt/index.tsapps/backend/src/auth/providers/identity-provider.interface.tsapps/backend/src/auth/providers/google.provider.tsapps/backend/src/modules/users/users.controller.tsapps/backend/src/auth/providers/google.provider.spec.tsapps/backend/src/auth/jwt/jwt.service.spec.tsapps/backend/src/auth/jwt/self-issued-jwt.strategy.tsapps/backend/src/auth/jwt/jwt.service.tsapps/backend/src/auth/auth.module.tsapps/backend/src/auth/jwt/self-issued-jwt.strategy.spec.tsapps/backend/src/modules/users/users.service.ts
apps/backend/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/project-context.mdc)
All operations must include account context (accountId) for account isolation
Files:
apps/backend/src/auth/providers/index.tsapps/backend/src/modules/users/dto/google-oauth.dto.tsapps/backend/src/auth/jwt/index.tsapps/backend/src/auth/providers/identity-provider.interface.tsapps/backend/src/auth/providers/google.provider.tsapps/backend/src/modules/users/users.controller.tsapps/backend/src/auth/providers/google.provider.spec.tsapps/backend/src/auth/jwt/jwt.service.spec.tsapps/backend/src/auth/jwt/self-issued-jwt.strategy.tsapps/backend/src/auth/jwt/jwt.service.tsapps/backend/src/auth/auth.module.tsapps/backend/src/auth/jwt/self-issued-jwt.strategy.spec.tsapps/backend/src/modules/users/users.service.ts
apps/**/*.{vue,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/project-context.mdc)
Implement comprehensive error handling scenarios in Vue components and backend services
Files:
apps/backend/src/auth/providers/index.tsapps/backend/src/modules/users/dto/google-oauth.dto.tsapps/backend/src/auth/jwt/index.tsapps/backend/src/auth/providers/identity-provider.interface.tsapps/backend/src/auth/providers/google.provider.tsapps/backend/src/modules/users/users.controller.tsapps/backend/src/auth/providers/google.provider.spec.tsapps/backend/src/auth/jwt/jwt.service.spec.tsapps/backend/src/auth/jwt/self-issued-jwt.strategy.tsapps/backend/src/auth/jwt/jwt.service.tsapps/backend/src/auth/auth.module.tsapps/backend/src/auth/jwt/self-issued-jwt.strategy.spec.tsapps/backend/src/modules/users/users.service.ts
**/*.{ts,tsx,d.ts}
📄 CodeRabbit inference engine (.cursor/rules/typescript.mdc)
**/*.{ts,tsx,d.ts}: Prefer interfaces over types for object definitions
Use type for unions, intersections, and mapped types
Avoid usingany, preferunknownfor unknown types
Leverage TypeScript's built-in utility types
Use generics for reusable type patterns
Use PascalCase for type names and interfaces
Use camelCase for variables and functions
Use UPPER_CASE for constants
Use descriptive names with auxiliary verbs for boolean variables and functions (e.g., isLoading, hasError)
Prefix React component prop interfaces with 'Props' (e.g., ButtonProps)
Keep type definitions close to where they're used
Export types and interfaces from dedicated type files when shared
Use explicit return types for public functions
Use arrow functions for callbacks and methods
Implement proper error handling with custom error types
Use function overloads for complex type scenarios
Prefer async/await over Promises
Use readonly for immutable properties
Leverage discriminated unions for type safety
Use type guards for runtime type checking
Implement proper null checking
Avoid type assertions unless necessary
Create custom error types for domain-specific errors
Use Result types for operations that can fail
Use try-catch blocks with typed catch clauses
Handle Promise rejections properly
Files:
apps/backend/src/auth/providers/index.tsapps/backend/src/modules/users/dto/google-oauth.dto.tsapps/backend/src/auth/jwt/index.tsapps/backend/src/auth/providers/identity-provider.interface.tsapps/backend/src/auth/providers/google.provider.tsapps/backend/src/modules/users/users.controller.tsapps/backend/src/auth/providers/google.provider.spec.tsapps/backend/src/auth/jwt/jwt.service.spec.tsapps/backend/src/auth/jwt/self-issued-jwt.strategy.tsapps/backend/src/auth/jwt/jwt.service.tsapps/backend/src/auth/auth.module.tsapps/backend/src/auth/jwt/self-issued-jwt.strategy.spec.tsapps/backend/src/modules/users/users.service.ts
**/index.ts
📄 CodeRabbit inference engine (.cursor/rules/typescript.mdc)
Use barrel exports (index.ts) for organizing exports
Files:
apps/backend/src/auth/providers/index.tsapps/backend/src/auth/jwt/index.ts
apps/backend/src/modules/**/dto/*.dto.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Define DTOs with validation decorators in dto/ subdirectory
Files:
apps/backend/src/modules/users/dto/google-oauth.dto.ts
apps/backend/src/modules/**/*.controller.ts
📄 CodeRabbit inference engine (CLAUDE.md)
apps/backend/src/modules/**/*.controller.ts: Include API documentation with Swagger decorators on all controllers
Use JwtAuthGuard on all protected controller endpoints
Implement input validation with DTOs and class-validator in controllers
Files:
apps/backend/src/modules/users/users.controller.ts
apps/backend/src/**/*.spec.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Include unit tests for all backend services using Jest
Files:
apps/backend/src/auth/providers/google.provider.spec.tsapps/backend/src/auth/jwt/jwt.service.spec.tsapps/backend/src/auth/jwt/self-issued-jwt.strategy.spec.ts
apps/backend/src/modules/**/*.service.ts
📄 CodeRabbit inference engine (CLAUDE.md)
apps/backend/src/modules/**/*.service.ts: All database operations must include account context filtering using accountId from CLS (Continuation Local Storage)
Always filter queries by accountId and deletedAt: IsNull() in where clauses for soft delete compatibility
Use save() instead of update() for audit compatibility in TypeORM operations
Implement proper error handling with custom exceptions in backend services
Files:
apps/backend/src/modules/users/users.service.ts
🧠 Learnings (10)
📚 Learning: 2025-11-28T18:14:14.256Z
Learnt from: CR
Repo: etusdigital/boilerplate PR: 0
File: .cursor/rules/typescript.mdc:0-0
Timestamp: 2025-11-28T18:14:14.256Z
Learning: Applies to **/index.ts : Use barrel exports (index.ts) for organizing exports
Applied to files:
apps/backend/src/auth/providers/index.tsapps/backend/src/auth/jwt/index.ts
📚 Learning: 2025-11-28T18:14:14.256Z
Learnt from: CR
Repo: etusdigital/boilerplate PR: 0
File: .cursor/rules/typescript.mdc:0-0
Timestamp: 2025-11-28T18:14:14.256Z
Learning: Applies to **/*.{ts,tsx,d.ts} : Export types and interfaces from dedicated type files when shared
Applied to files:
apps/backend/src/auth/providers/index.tsapps/backend/src/auth/jwt/index.ts
📚 Learning: 2025-11-28T18:13:02.946Z
Learnt from: CR
Repo: etusdigital/boilerplate PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-28T18:13:02.946Z
Learning: Applies to apps/backend/src/modules/**/*.controller.ts : Include API documentation with Swagger decorators on all controllers
Applied to files:
apps/backend/src/auth/providers/index.tsapps/backend/src/modules/users/dto/google-oauth.dto.tsapps/backend/src/auth/jwt/index.tsapps/backend/src/modules/users/users.controller.ts
📚 Learning: 2025-11-28T18:13:02.946Z
Learnt from: CR
Repo: etusdigital/boilerplate PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-28T18:13:02.946Z
Learning: Applies to apps/frontend-{vue,react}/src/**/api/**/*.ts : Maintain account isolation with all API calls including account-id header via interceptors
Applied to files:
apps/backend/src/auth/providers/index.tsapps/backend/src/auth/jwt/index.ts
📚 Learning: 2025-11-28T18:13:02.946Z
Learnt from: CR
Repo: etusdigital/boilerplate PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-28T18:13:02.946Z
Learning: Applies to apps/backend/src/modules/**/*.controller.ts : Use JwtAuthGuard on all protected controller endpoints
Applied to files:
apps/backend/src/auth/providers/index.tsapps/backend/src/auth/jwt/index.tsapps/backend/src/modules/users/users.controller.tsapps/backend/src/auth/jwt/jwt.service.spec.tsapps/backend/src/auth/jwt/self-issued-jwt.strategy.tsapps/backend/src/auth/jwt/jwt.service.tsapps/backend/src/auth/auth.module.ts
📚 Learning: 2025-11-28T18:13:02.946Z
Learnt from: CR
Repo: etusdigital/boilerplate PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-28T18:13:02.946Z
Learning: Applies to apps/backend/src/modules/**/dto/*.dto.ts : Define DTOs with validation decorators in dto/ subdirectory
Applied to files:
apps/backend/src/modules/users/dto/google-oauth.dto.tsapps/backend/src/modules/users/users.controller.tsapps/backend/src/modules/users/users.service.ts
📚 Learning: 2025-11-28T18:13:02.946Z
Learnt from: CR
Repo: etusdigital/boilerplate PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-28T18:13:02.946Z
Learning: Applies to apps/backend/src/modules/**/*.controller.ts : Implement input validation with DTOs and class-validator in controllers
Applied to files:
apps/backend/src/modules/users/dto/google-oauth.dto.ts
📚 Learning: 2025-11-28T18:13:02.946Z
Learnt from: CR
Repo: etusdigital/boilerplate PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-28T18:13:02.946Z
Learning: Applies to apps/backend/src/modules/**/ : Organize backend modules in feature structure: [entity].module.ts, [entity].controller.ts, [entity].service.ts, [entity].spec.ts, dto/
Applied to files:
apps/backend/src/modules/users/dto/google-oauth.dto.tsapps/backend/src/auth/jwt/index.ts
📚 Learning: 2025-11-28T18:13:02.946Z
Learnt from: CR
Repo: etusdigital/boilerplate PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-28T18:13:02.946Z
Learning: Applies to apps/backend/src/**/*.spec.ts : Include unit tests for all backend services using Jest
Applied to files:
apps/backend/src/auth/jwt/index.tsapps/backend/src/auth/providers/google.provider.spec.tsapps/backend/src/auth/jwt/jwt.service.spec.tsapps/backend/package.jsonapps/backend/src/auth/jwt/self-issued-jwt.strategy.spec.ts
📚 Learning: 2025-11-28T18:13:58.953Z
Learnt from: CR
Repo: etusdigital/boilerplate PR: 0
File: .cursor/rules/project-context.mdc:0-0
Timestamp: 2025-11-28T18:13:58.953Z
Learning: Applies to apps/backend/src/**/{services,controllers}/**/*.ts : Validate account ownership before any CRUD operation
Applied to files:
apps/backend/src/modules/users/users.controller.ts
🧬 Code graph analysis (7)
apps/backend/src/auth/providers/google.provider.ts (1)
apps/backend/src/auth/providers/identity-provider.interface.ts (2)
IdentityProvider(18-59)IdentityProviderUser(4-11)
apps/backend/src/modules/users/users.controller.ts (1)
apps/backend/src/modules/users/dto/google-oauth.dto.ts (2)
OAuthTokenResponse(14-30)GoogleOAuthDto(4-12)
apps/backend/src/auth/jwt/self-issued-jwt.strategy.ts (1)
apps/backend/src/auth/jwt/jwt.service.ts (2)
Injectable(25-188)JwtPayload(9-17)
apps/backend/src/auth/jwt/jwt.service.ts (1)
apps/backend/src/auth/jwt/self-issued-jwt.strategy.ts (1)
Injectable(20-64)
apps/backend/src/auth/auth.module.ts (2)
apps/backend/src/modules/users/users.module.ts (1)
Module(12-18)apps/backend/src/app.module.ts (1)
Module(14-47)
apps/backend/src/auth/jwt/self-issued-jwt.strategy.spec.ts (1)
apps/backend/src/auth/jwt/jwt.service.ts (1)
JwtPayload(9-17)
apps/backend/src/modules/users/users.service.ts (1)
apps/backend/src/modules/users/dto/google-oauth.dto.ts (1)
OAuthTokenResponse(14-30)
🪛 dotenv-linter (4.0.0)
apps/backend/.env.example
[warning] 30-30: [UnorderedKey] The JWT_EXPIRES_IN key should go before the JWT_ISSUER key
(UnorderedKey)
🔇 Additional comments (15)
apps/backend/.env.example (1)
21-30: LGTM! Environment configuration is well-documented.The new environment variables for Google OAuth and self-issued JWT are clearly documented with helpful comments. The generate:keys script reference on Line 26 aligns with the new script in package.json.
apps/backend/src/auth/jwt/index.ts (1)
1-2: LGTM! Proper barrel export pattern.The barrel file cleanly exports the JWT-related functionality, following the established pattern for organizing exports.
Based on learnings, this aligns with the barrel export convention.
apps/backend/src/auth/providers/index.ts (1)
1-2: LGTM! Consistent barrel export pattern.The providers barrel file follows the same clean export pattern as the JWT barrel, providing a consolidated public API for authentication providers.
Based on learnings, this aligns with the barrel export convention.
apps/backend/scripts/generate-keys.sh (1)
1-42: LGTM! Well-implemented key generation script.The script properly handles key generation with appropriate security measures:
- Error handling with
set -e- Checks for existing keys before overwriting
- Correct file permissions (600 for private, 644 for public)
- Clear security warning about not committing the private key
apps/backend/package.json (2)
21-21: LGTM! Script addition aligns with key generation workflow.The new
generate:keysscript properly references the shell script added inapps/backend/scripts/generate-keys.sh.
37-38: Dependency versions are secure.Both
google-auth-library@^10.5.0andjsonwebtoken@^9.0.3have no known security vulnerabilities. The jsonwebtoken version specifically resolves prior CVEs (CVE-2022-23540 and CVE-2022-23541) that affected earlier releases.apps/backend/src/modules/users/users.controller.ts (1)
67-74: LGTM! Comprehensive API documentation.The Swagger documentation for the Google OAuth endpoint is thorough, including operation summary, success response with type, and error scenarios (400, 403).
apps/backend/src/auth/jwt/self-issued-jwt.strategy.spec.ts (1)
1-162: LGTM! Comprehensive test coverage for SelfIssuedJwtStrategy.The test suite thoroughly covers:
- Key loading from configured paths with proper fallback behavior
- Default issuer configuration
- JWT payload validation and mapping
- Role preservation across all enum values
- Special character handling in email fields
The tests use appropriate mocking for external dependencies (fs, ConfigService) and cover both happy paths and edge cases.
Based on learnings, this follows the guideline to include unit tests for all backend services.
apps/backend/src/modules/users/dto/google-oauth.dto.ts (1)
4-12: LGTM! Proper DTO validation and documentation.The
GoogleOAuthDtocorrectly uses class-validator decorators (@IsString(),@IsNotEmpty()) and includes comprehensive Swagger documentation for the idToken field.Based on learnings, this follows the guidelines for defining DTOs with validation decorators in the dto/ subdirectory.
apps/backend/src/auth/providers/google.provider.spec.ts (1)
69-201: Good test coverage for verifyToken scenarios.The tests comprehensively cover valid tokens, missing name/email/picture fields, null payload, error propagation, and providerId formatting. This aligns well with the implementation.
apps/backend/src/auth/providers/google.provider.ts (1)
45-52: Clean implementation of IdentityProviderUser mapping.The mapping correctly handles optional fields with sensible defaults: name falls back to email prefix, picture is passed through as optional, and emailVerified defaults to false. The providerId format follows the documented convention.
apps/backend/src/auth/jwt/jwt.service.spec.ts (1)
67-428: Excellent test coverage for JwtTokenService.The tests thoroughly cover:
- Key loading and availability checks
- Token generation with correct claims and algorithm
- Token verification including expiry, issuer validation, and tampering detection
- Duration parsing for various time units
This provides good confidence in the JWT implementation.
apps/backend/src/auth/jwt/jwt.service.ts (1)
9-23: Well-defined interfaces for JWT payloads.The
JwtPayloadandTokenPairinterfaces are clearly defined with appropriate fields. The payload includes all necessary claims for authorization (accountId,role) which aligns with the account isolation requirement.apps/backend/src/auth/auth.module.ts (1)
20-30: GoogleProvider null export is already properly handled with @optional().The injection site in
apps/backend/src/modules/users/users.service.ts(line 34) already uses@Optional()with explicit null typing:@Optional() private readonly googleProvider: GoogleProvider | null. The module's factory pattern correctly avoids throwing errors by returning null before instantiation, and consumers are properly prepared for the optional dependency. No changes needed.apps/backend/src/modules/users/users.service.ts (1)
5-6: Import and DI wiring for Google/JWT auth looks consistentThe new imports (
Optional,OAuthTokenResponse,GoogleProvider,JwtTokenService) and the optional constructor injections forgoogleProvider/jwtTokenServiceare wired in cleanly and align with the existing NestJS service pattern. No issues from a structure/coupling standpoint.Also applies to: 15-19, 34-36
| it('should throw error when GOOGLE_CLIENT_ID is not set', async () => { | ||
| const configWithoutClientId = { | ||
| get: jest.fn().mockReturnValue(undefined), | ||
| }; | ||
|
|
||
| await expect( | ||
| Test.createTestingModule({ | ||
| providers: [ | ||
| GoogleProvider, | ||
| { provide: ConfigService, useValue: configWithoutClientId }, | ||
| ], | ||
| }).compile(), | ||
| ).rejects.toThrow('GOOGLE_CLIENT_ID is required'); | ||
| }); |
There was a problem hiding this comment.
Error message assertion mismatch with implementation.
The test expects 'GOOGLE_CLIENT_ID is required' but the actual implementation in google.provider.ts throws 'GOOGLE_CLIENT_ID is required when using GoogleProvider'. This test will fail.
await expect(
Test.createTestingModule({
providers: [
GoogleProvider,
{ provide: ConfigService, useValue: configWithoutClientId },
],
}).compile(),
- ).rejects.toThrow('GOOGLE_CLIENT_ID is required');
+ ).rejects.toThrow('GOOGLE_CLIENT_ID is required when using GoogleProvider');🤖 Prompt for AI Agents
In apps/backend/src/auth/providers/google.provider.spec.ts around lines 41 to
54, the test asserts the error message 'GOOGLE_CLIENT_ID is required' but the
implementation throws 'GOOGLE_CLIENT_ID is required when using GoogleProvider';
update the test to expect the exact implemented message (or adjust to use a
partial match like expect(...).rejects.toThrow(/GOOGLE_CLIENT_ID is required
when using GoogleProvider/) so the assertion matches the thrown error string).
| @ApiProperty({ description: 'JWT refresh token' }) | ||
| refreshToken: string; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for refresh token implementation
echo "=== Searching for refresh token generation ==="
rg -nP --type=ts -C3 'refreshToken.*=|generateRefresh|createRefresh' apps/backend/src/ 2>/dev/null || true
echo -e "\n=== Searching for token refresh endpoint ==="
rg -nP --type=ts -C3 '@Post.*refresh|@Put.*refresh' apps/backend/src/ 2>/dev/null || true
echo -e "\n=== Searching for refresh token validation ==="
rg -nP --type=ts -C3 'validateRefresh|verifyRefresh' apps/backend/src/ 2>/dev/null || true
echo -e "\n=== Searching for OAuth service files ==="
fd -e ts -e tsx 'oauth|token' apps/backend/src/ 2>/dev/null | head -20
echo -e "\n=== Searching for JWT/token service implementations ==="
rg -nP --type=ts -C2 'class.*Service' apps/backend/src/modules/users/ 2>/dev/null || trueRepository: etusdigital/boilerplate
Length of output: 3735
🏁 Script executed:
#!/bin/bash
echo "=== Checking jwt.service.ts for all methods ==="
rg -nP --type=ts 'public.*(' apps/backend/src/auth/jwt/jwt.service.ts | head -30
echo -e "\n=== Searching for refresh endpoint with various patterns ==="
rg -nP --type=ts 'refresh|token.*endpoint|auth.*controller' apps/backend/src/ -i | grep -E '(Controller|@(Get|Post|Put|Delete)|refresh)' | head -20
echo -e "\n=== Checking auth controller files ==="
fd -e ts auth controller apps/backend/src/ 2>/dev/null | head -10
echo -e "\n=== Checking for JwtStrategy/validation ==="
rg -nP --type=ts 'validate|verify.*token' apps/backend/src/auth/ -C2 | head -40
echo -e "\n=== Full content of jwt.service.ts ==="
wc -l apps/backend/src/auth/jwt/jwt.service.tsRepository: etusdigital/boilerplate
Length of output: 5284
🏁 Script executed:
#!/bin/bash
echo "=== Reading jwt.service.ts ==="
cat -n apps/backend/src/auth/jwt/jwt.service.ts
echo -e "\n=== Searching for auth controller ==="
fd -e ts 'controller' apps/backend/src/ --path '*auth*' 2>/dev/null
echo -e "\n=== Searching for routes with 'refresh' ==="
rg -nP --type=ts 'refresh' apps/backend/src/ -B2 -A2 | grep -E '(refresh|@Post|@Get|Controller|Route)' | head -30Repository: etusdigital/boilerplate
Length of output: 7992
🏁 Script executed:
#!/bin/bash
echo "=== Searching for all controller files ==="
fd -e ts 'controller' apps/backend/src/ 2>/dev/null | head -20
echo -e "\n=== Checking users controller ==="
fd -e ts -e tsx users.controller apps/backend/src/ 2>/dev/null
echo -e "\n=== Searching for @Controller decorator ==="
rg -nP --type=ts '@Controller' apps/backend/src/ -A5 | head -40
echo -e "\n=== Searching for endpoint definitions with @Post/@Get ==="
rg -nP --type=ts '@(Post|Get|Put|Patch)\(' apps/backend/src/ | grep -i 'refresh\|token' | head -20
echo -e "\n=== Searching for token rotation/invalidation/revocation ==="
rg -nP --type=ts 'revok|rotati|invalidat|blacklist' apps/backend/src/ -i -C2 | head -40Repository: etusdigital/boilerplate
Length of output: 4344
🏁 Script executed:
#!/bin/bash
echo "=== Reading users.controller.ts ==="
cat -n apps/backend/src/modules/users/users.controller.ts
echo -e "\n=== Searching for refresh-related methods in users.service ==="
rg -nP --type=ts 'refresh|token' apps/backend/src/modules/users/users.service.ts -C3 | head -50Repository: etusdigital/boilerplate
Length of output: 6715
Implement token refresh endpoint to utilize generated refresh tokens.
While refresh token generation is properly implemented in JwtTokenService.generateRefreshToken() with correct 7-day expiry and validation exists via verifyToken(), the implementation is incomplete:
- Missing refresh endpoint: No endpoint exists to exchange a refresh token for a new access token. Clients receive a refresh token from
/users/oauth/googlebut have no way to use it. - Incomplete refresh token payload: The refresh token only contains
subandtypefields (line 122 in jwt.service.ts), lackingaccountIdandroleneeded to generate a properly scoped new access token. - No token rotation strategy: Add a
POST /users/refreshendpoint that validates the refresh token and returns a new access token/refresh token pair.
🤖 Prompt for AI Agents
apps/backend/src/modules/users/dto/google-oauth.dto.ts lines 18-19: the review
calls out that there is no /users/refresh endpoint and the refresh token payload
lacks accountId and role, so: update JwtTokenService.generateRefreshToken to
include accountId and role (alongside sub and type), ensure verifyToken can
validate that payload; add a new POST /users/refresh controller action that
accepts a refresh-token DTO, calls JwtTokenService.verifyToken, checks token
type is refresh and not expired/revoked, then issues a new access token and a
new refresh token (using the accountId/role from the verified payload);
implement token rotation by revoking the used refresh token (store a refresh
token id/jti or increment a tokenVersion on the user record) before returning
the new pair; update DTOs to accept/return the refresh token and new tokens
accordingly and add appropriate error handling for invalid/expired/revoked
tokens.
| @Post('/oauth/google') | ||
| @UseGuards() // Remove JWT guard for this endpoint | ||
| @UsePipes(new ValidationPipe()) | ||
| @ApiOperation({ summary: 'Authenticate with Google OAuth' }) | ||
| @ApiResponse({ | ||
| status: 201, | ||
| description: 'Successfully authenticated with Google', | ||
| type: OAuthTokenResponse, | ||
| }) | ||
| @ApiResponse({ status: 400, description: 'Invalid Google token.' }) | ||
| @ApiResponse({ status: 403, description: 'User not found or not allowed.' }) | ||
| async googleOAuth( | ||
| @Body() googleOAuthDto: GoogleOAuthDto, | ||
| ): Promise<OAuthTokenResponse> { | ||
| return await this.usersService.loginWithGoogle(googleOAuthDto.idToken); | ||
| } |
There was a problem hiding this comment.
Critical: Empty @UseGuards() does not bypass class-level guards.
Line 65 uses @UseGuards() with no arguments, but this does not bypass the class-level guards defined on Line 29 (AuthGuard('jwt') and RolesGuard). The Google OAuth endpoint will still require a valid JWT, which defeats its purpose as a public authentication endpoint.
Solution: Implement a @Public() decorator or move the guards to individual methods.
Option 1 (Recommended): Create a @Public() decorator
Create apps/backend/src/auth/decorators/public.decorator.ts:
import { SetMetadata } from '@nestjs/common';
export const IS_PUBLIC_KEY = 'isPublic';
export const Public = () => SetMetadata(IS_PUBLIC_KEY, true);Update the JWT strategy or create a guard that checks for the IS_PUBLIC_KEY metadata and skips authentication when present.
Option 2: Move guards to method level
Remove guards from the class level and apply them individually to each protected endpoint (more verbose but explicit).
Then apply the @Public() decorator:
@Post('/oauth/google')
- @UseGuards() // Remove JWT guard for this endpoint
+ @Public()
@UsePipes(new ValidationPipe())🤖 Prompt for AI Agents
In apps/backend/src/modules/users/users.controller.ts around lines 64 to 79, the
googleOAuth endpoint uses @UseGuards() with no args which does not bypass
class-level guards; make this endpoint public by adding a Public decorator
(create apps/backend/src/auth/decorators/public.decorator.ts exporting
IS_PUBLIC_KEY and Public that sets metadata) and update the JWT/Auth guard
pipeline (or JWT strategy) to check for IS_PUBLIC_KEY metadata and skip
authentication when present; alternatively remove class-level guards and apply
AuthGuard('jwt') and RolesGuard on individual protected methods, then annotate
this method with Public to allow unauthenticated access.
| /** | ||
| * Authenticate a user via Google OAuth. | ||
| * Verifies the Google ID token, finds or creates the user, | ||
| * and returns self-issued JWT tokens. | ||
| */ | ||
| async loginWithGoogle(idToken: string): Promise<OAuthTokenResponse> { | ||
| if (!this.googleProvider) { | ||
| throw new BadRequestException( | ||
| 'Google OAuth is not configured. Set GOOGLE_CLIENT_ID in environment.', | ||
| ); | ||
| } | ||
|
|
||
| if (!this.jwtTokenService || !this.jwtTokenService.isAvailable()) { | ||
| throw new BadRequestException( | ||
| 'JWT token service is not available. Run `npm run generate:keys`.', | ||
| ); | ||
| } | ||
|
|
||
| // Verify the Google ID token | ||
| const googleUser = await this.googleProvider.verifyToken(idToken); | ||
|
|
||
| // Set transaction ID for audit | ||
| this.cls.set('transactionId', uuidv7()); | ||
|
|
||
| // Find user by provider ID or email | ||
| let user = await this.findByProviderId(googleUser.providerId); | ||
|
|
||
| if (!user) { | ||
| // Try to find by email (user might exist from Auth0 or invitation) | ||
| user = await this.userRepository.findOne({ | ||
| where: { email: googleUser.email }, | ||
| relations: ['userAccounts'], | ||
| }); | ||
| } | ||
|
|
||
| if (!user) { | ||
| throw new ForbiddenException( | ||
| 'User not found. Please contact an administrator to create your account.', | ||
| ); | ||
| } | ||
|
|
||
| // Update user status and add Google provider ID if not already present | ||
| if (user.status !== 'accepted') { | ||
| user.status = 'accepted'; | ||
| } | ||
|
|
||
| if (!user.providerIds.includes(googleUser.providerId)) { | ||
| user.addProvider(googleUser.providerId); | ||
| } | ||
|
|
||
| // Update profile image if not set | ||
| if (!user.profileImage && googleUser.picture) { | ||
| user.profileImage = googleUser.picture; | ||
| } | ||
|
|
||
| await this.userRepository.save(user); | ||
|
|
||
| // Reload user with relations | ||
| user = await this.userRepository.findOne({ | ||
| where: { id: user.id }, | ||
| relations: ['userAccounts', 'userAccounts.account'], | ||
| }); | ||
|
|
||
| if (!user) { | ||
| throw new ForbiddenException('User not found after save.'); | ||
| } | ||
|
|
||
| // Determine account and role for token | ||
| let accountId: string; | ||
| let role: Role; | ||
|
|
||
| if (user.isSuperAdmin) { | ||
| const accounts = await this.accountsService.findAll(user); | ||
| accountId = accounts[0]?.id || 'no-account'; | ||
| role = Role.ADMIN; | ||
| } else if (user.userAccounts?.length) { | ||
| accountId = user.userAccounts[0].accountId; | ||
| role = user.userAccounts[0].role; | ||
| } else { | ||
| throw new ForbiddenException('User has no account access.'); | ||
| } | ||
|
|
||
| // Generate tokens | ||
| const tokens = this.jwtTokenService.generateTokenPair(user, accountId, role); | ||
|
|
||
| return { | ||
| accessToken: tokens.accessToken, | ||
| refreshToken: tokens.refreshToken, | ||
| expiresIn: tokens.expiresIn, | ||
| user: { | ||
| id: user.id, | ||
| email: user.email || googleUser.email, | ||
| name: user.name, | ||
| }, | ||
| }; | ||
| } |
There was a problem hiding this comment.
Fix potential crash on providerIds and tighten Google login flow behaviour
The overall flow of loginWithGoogle is solid (token verification → user lookup → status/provider/profile update → token pair), but there are a few issues and improvement opportunities:
- Possible runtime error on
user.providerIds.includes(...)
If user.providerIds is ever null/undefined (e.g., legacy users, misconfigured entity defaults), this line will throw:
if (!user.providerIds.includes(googleUser.providerId)) {Safer pattern:
- if (!user.providerIds.includes(googleUser.providerId)) {
- user.addProvider(googleUser.providerId);
- }
+ if (!user.providerIds?.includes(googleUser.providerId)) {
+ user.addProvider(googleUser.providerId);
+ }This keeps the de‑duplication intent while avoiding a crash if providerIds isn’t initialized.
- Ambiguous handling of super admins with no accounts
For super admins you currently do:
const accounts = await this.accountsService.findAll(user);
accountId = accounts[0]?.id || 'no-account';
role = Role.ADMIN;Issuing tokens with a synthetic 'no-account' ID makes downstream account‑scoped logic harder to reason about and may break the “all operations must have a real account context” expectation. Prefer failing fast if no accounts are configured:
if (user.isSuperAdmin) {
const accounts = await this.accountsService.findAll(user);
- accountId = accounts[0]?.id || 'no-account';
- role = Role.ADMIN;
+ if (!accounts.length) {
+ throw new ForbiddenException('Super admin has no account access configured.');
+ }
+ accountId = accounts[0].id;
+ role = Role.ADMIN;- CLS/account context consistency with existing
loginmethod
The email/password login method sets CLS context for both transactionId and accountId (and user), while loginWithGoogle only sets transactionId. For observability and to stay aligned with the account‑isolation guideline, consider also setting user and accountId once you’ve derived them:
this.cls.set('user', { ...user, userAccounts: null });
this.cls.set('accountId', accountId);This keeps behaviour consistent regardless of auth mechanism.
- Token generation and user lookup re-query
After save(user) you re-query by id with relations. In many cases (especially when the user was found via findByProviderId, which already loads userAccounts) this second query is mostly to ensure relations. Since you only need accountId/role to issue tokens, you could avoid the extra round-trip by ensuring the initial lookup always includes userAccounts and using that instance directly. This is a non-blocking performance simplification you can do later.
- Optional: validate
googleUser.emailVerifiedand wrap provider errors
Depending on your security posture, you might want to:
- Enforce
googleUser.emailVerified === truebefore allowing login, and - Wrap
verifyTokenfailures into anUnauthorizedExceptionwith a generic message (instead of leaking raw provider errors).
These are policy choices rather than hard bugs, but worth considering for a hardened auth flow.
1 participant
Summary
Test plan
npm test -- --testPathPattern="google.provider|jwt.service|self-issued-jwt.strategy"to verify all tests pass🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.