feat: ✨ add auth app #113
feat: ✨ add auth app #113
Conversation
WalkthroughThis pull request introduces several new configuration files and updates to support an authentication service within an application. Key changes include the addition of environment variables in the Changes
Possibly related PRs
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Outside diff range and nitpick comments (11)
apps/auth/src/app/app.service.ts (1)
1-8: LGTM, but this appears to be just a placeholder implementation.While the implementation is correct, as an authentication service, it should provide core authentication functionality.
Consider adding essential auth methods such as:
register(credentials: RegisterDto): Promise<User>login(credentials: LoginDto): Promise<AuthToken>validateToken(token: string): Promise<User>refreshToken(token: string): Promise<AuthToken>apps/auth-e2e/src/support/test-setup.ts (1)
1-10: Consider adding type safety and request timeout.The setup could benefit from additional configuration for robust e2e testing.
Consider these improvements:
import axios, { AxiosError } from 'axios'; interface TestConfig { host: string; port: number; timeout: number; } module.exports = async function () { const config: TestConfig = { host: process.env.AUTH_HOST ?? 'localhost', port: Number(process.env.AUTH_PORT ?? '15003'), timeout: Number(process.env.AUTH_TIMEOUT ?? '5000'), }; // Validate config if (isNaN(config.port)) { throw new Error('AUTH_PORT must be a valid number'); } axios.defaults.baseURL = `http://${config.host}:${config.port}`; axios.defaults.timeout = config.timeout; // Add response interceptor for better error handling axios.interceptors.response.use( response => response, (error: AxiosError) => { console.error(`Test request failed: ${error.message}`); return Promise.reject(error); } ); };apps/auth/src/app/app.controller.ts (1)
1-3: Consider adding authentication guardsAs this is an auth module, consider adding appropriate authentication/authorization guards to protect the endpoints.
Example:
import { AuthGuard } from '@nestjs/passport'; import { ApiBearerAuth } from '@nestjs/swagger'; @ApiBearerAuth() @UseGuards(AuthGuard('jwt'))apps/auth-e2e/src/support/global-setup.ts (1)
1-3: Improve type declarationConsider using a more specific type declaration for the teardown message.
-var __TEARDOWN_MESSAGE__: string; +declare global { + var __TEARDOWN_MESSAGE__: string; +}apps/auth/webpack.config.js (1)
9-18: Consider enabling optimization for production buildsWhile disabling optimization is fine for development, consider adding a conditional configuration for production builds:
new NxAppWebpackPlugin({ target: 'node', compiler: 'tsc', main: './src/main.ts', tsConfig: './tsconfig.app.json', assets: ['./src/assets'], - optimization: false, + optimization: process.env.NODE_ENV === 'production', outputHashing: 'none', generatePackageJson: true, }),apps/auth-e2e/jest.config.ts (1)
17-17: Consider adding coverage thresholdsAdd coverage thresholds to ensure maintaining test coverage standards:
coverageDirectory: '../../coverage/auth-e2e', +coverageThreshold: { + global: { + branches: 80, + functions: 80, + lines: 80, + statements: 80 + } +},apps/auth/src/app/app.controller.spec.ts (1)
1-5: Add e2e test configurationSince this is an auth service, consider adding separate e2e tests for full authentication flows.
Create a new e2e test file with scenarios like:
- User registration flow
- Login flow
- Token refresh flow
- Password reset flow
- Session management
libs/shared/config/src/lib/applications.config.ts (1)
57-65: Consider standardizing environment variable namingThe implementation looks good and follows the established pattern. However, for better consistency with other services, consider using
AUTH_APP_HOSTandAUTH_APP_PORTinstead ofAUTH_HOSTandAUTH_PORTto match the naming pattern used in the config keyauthApp.- host: process.env['AUTH_HOST'] ?? DEFAULT_HOST, - port: process.env['AUTH_PORT'] ? Number(process.env['AUTH_PORT']) : DEFAULT_PORT.auth, + host: process.env['AUTH_APP_HOST'] ?? DEFAULT_HOST, + port: process.env['AUTH_APP_PORT'] ? Number(process.env['AUTH_APP_PORT']) : DEFAULT_PORT.auth,apps/auth/src/app/app.module.ts (2)
1-18: Consider organizing imports by categoryThe imports look good but could be better organized for maintainability.
+ // Apollo import { ApolloServerPluginInlineTrace } from '@apollo/server/plugin/inlineTrace'; import { ApolloFederationDriver, ApolloFederationDriverConfig, } from '@nestjs/apollo'; + + // NestJS import { Module } from '@nestjs/common'; import { ConfigModule } from '@nestjs/config'; import { GraphQLModule } from '@nestjs/graphql'; + + // Configuration import { databaseConfig, authAppConfig, awsConfig, authConfig, } from '@shared/config'; + + // Modules import { AuthModule } from '@auth/interface-adapters'; + // Local import { AppController } from './app.controller'; import { AppService } from './app.service';
26-39: Consider additional GraphQL security measuresThe GraphQL configuration looks good, but consider adding these security enhancements:
- Query complexity limits
- Rate limiting
- Request timeout
GraphQLModule.forRoot<ApolloFederationDriverConfig>({ driver: ApolloFederationDriver, autoSchemaFile: { federation: 2, }, playground: process.env['NODE_ENV'] !== 'production', sortSchema: true, plugins: [ApolloServerPluginInlineTrace()], + // Prevent complex queries from overloading the server + complexity: { + max: 20, + }, + // Set timeout for long-running queries + context: ({ req }) => ({ + req, + timeout: 10000, + }), }),apps/gateway/src/app/app.module.ts (1)
39-42: Consider authentication flow in federated setup.Since this is adding an auth subgraph to the federation:
- Ensure proper JWT validation across services
- Consider implementing authentication directives at the gateway level
- Plan for secure service-to-service communication
- Consider rate limiting for auth-related operations
Resources:
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (26)
.env.sample(1 hunks)apps/auth-e2e/eslint.config.js(1 hunks)apps/auth-e2e/jest.config.ts(1 hunks)apps/auth-e2e/project.json(1 hunks)apps/auth-e2e/src/auth/auth.spec.ts(1 hunks)apps/auth-e2e/src/support/global-setup.ts(1 hunks)apps/auth-e2e/src/support/global-teardown.ts(1 hunks)apps/auth-e2e/src/support/test-setup.ts(1 hunks)apps/auth-e2e/tsconfig.json(1 hunks)apps/auth-e2e/tsconfig.spec.json(1 hunks)apps/auth/eslint.config.js(1 hunks)apps/auth/jest.config.ts(1 hunks)apps/auth/project.json(1 hunks)apps/auth/src/app/app.controller.spec.ts(1 hunks)apps/auth/src/app/app.controller.ts(1 hunks)apps/auth/src/app/app.module.ts(1 hunks)apps/auth/src/app/app.service.spec.ts(1 hunks)apps/auth/src/app/app.service.ts(1 hunks)apps/auth/src/main.ts(1 hunks)apps/auth/tsconfig.app.json(1 hunks)apps/auth/tsconfig.json(1 hunks)apps/auth/tsconfig.spec.json(1 hunks)apps/auth/webpack.config.js(1 hunks)apps/gateway/src/app/app.module.ts(4 hunks)libs/shared/config/src/lib/applications.config.ts(2 hunks)nx.json(1 hunks)
✅ Files skipped from review due to trivial changes (10)
- apps/auth/eslint.config.js
- apps/auth-e2e/eslint.config.js
- apps/auth-e2e/tsconfig.json
- apps/auth-e2e/src/support/global-teardown.ts
- apps/auth/tsconfig.json
- apps/auth/tsconfig.spec.json
- apps/auth/tsconfig.app.json
- apps/auth/project.json
- apps/auth/jest.config.ts
- apps/auth-e2e/tsconfig.spec.json
🔇 Additional comments (11)
apps/auth/webpack.config.js (1)
5-7: LGTM: Output path follows Nx workspace convention
The output path is correctly configured to dist/apps/auth relative to the workspace root.
apps/auth-e2e/jest.config.ts (1)
4-6: Verify existence of global setup and teardown files
Ensure that the referenced setup and teardown files exist:
✅ Verification successful
All required Jest setup files are present
The verification confirms that all referenced setup files exist in the correct locations:
apps/auth-e2e/src/support/global-setup.tsapps/auth-e2e/src/support/global-teardown.tsapps/auth-e2e/src/support/test-setup.ts
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if the global setup/teardown files exist
files=(
"apps/auth-e2e/src/support/global-setup.ts"
"apps/auth-e2e/src/support/global-teardown.ts"
"apps/auth-e2e/src/support/test-setup.ts"
)
for file in "${files[@]}"; do
if [ ! -f "$file" ]; then
echo "Missing file: $file"
fi
done
Length of output: 404
Script:
#!/bin/bash
# Let's use fd to search for these files as it's more reliable
fd -t f "global-setup.ts|global-teardown.ts|test-setup.ts" apps/auth-e2e/src/support
Length of output: 213
apps/auth-e2e/project.json (3)
4-5: LGTM: Correct project type and dependencies
The project is correctly configured as an application with an implicit dependency on the auth project.
14-14: LGTM: Proper build dependency
The e2e tests correctly depend on the auth project being built first.
11-13: 🛠️ Refactor suggestion
Reconsider allowing tests to pass with no tests
The passWithNoTests option is set to true, which might hide the fact that e2e tests are missing:
Consider setting this to false once you have your initial test suite in place to ensure e2e test coverage is maintained.
✅ Verification successful
passWithNoTests setting can be safely removed
The project already has active e2e tests in place:
apps/auth-e2e/src/auth/auth.spec.tscontains a functional test suite for the auth endpointapps/auth-e2e/src/support/test-setup.tsincludes test support code
Since there are actual tests present and running, the passWithNoTests setting is unnecessary and can be safely set to false to ensure e2e test coverage is maintained.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if there are any test files in the e2e project
test_files=$(find apps/auth-e2e/src -name "*.spec.ts" -o -name "*.test.ts")
if [ -z "$test_files" ]; then
echo "Warning: No test files found in apps/auth-e2e/src"
fi
Length of output: 162
Script:
#!/bin/bash
# Let's check the content of the test file to see if it contains actual tests
cat apps/auth-e2e/src/auth/auth.spec.ts
# Also check if there are any other test-related files
find apps/auth-e2e/src -type f -exec grep -l "test\|describe\|it(" {} \;
Length of output: 442
apps/auth/src/main.ts (1)
15-16: Verify configuration handling
Add type safety and validation for the config object.
libs/shared/config/src/lib/applications.config.ts (1)
16-19: LGTM! Port assignment follows established pattern
The auth service port (15003) is correctly assigned following the sequential pattern of existing microservices.
nx.json (1)
40-41: LGTM! Verify auth-e2e directory setup
The exclusion pattern follows the established configuration for other e2e test suites.
✅ Verification successful
Directory structure and configuration files verified
The auth-e2e directory exists at the expected location with required configuration files (jest.config.ts and project.json) in place, confirming proper e2e test setup.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify the existence of auth-e2e directory and its configuration
fd -t d "auth-e2e$" apps/
fd -t f "jest.config.ts|project.json" apps/auth-e2e/
Length of output: 153
apps/gateway/src/app/app.module.ts (3)
2-2: LGTM! Import statement follows established patterns.
The addition of authAppConfig maintains consistency with existing imports and follows the project's naming conventions.
15-15: LGTM! Configuration module setup is correct.
The authAppConfig is properly integrated into the global configuration setup, maintaining consistency with the existing pattern.
24-24: Consider adding validation and error handling for the auth configuration.
While the implementation follows the established pattern, consider these improvements for robustness:
- Add validation to ensure all required auth config properties exist
- Implement URL sanitization for the constructed endpoint
- Add error handling for configuration loading
Let's verify the auth config structure:
Consider adding validation like this:
// In the useFactory
const authAppConfig = configService.get('authApp');
if (!authAppConfig?.host || !authAppConfig?.port || !authAppConfig?.protocol) {
throw new Error('Invalid auth app configuration');
}Also applies to: 39-42
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (5)
apps/gateway-e2e/src/support/test-setup.ts (1)
7-8: LGTM! Consider documenting environment variables.The renaming of environment variables to include the
GATEWAY_prefix is a good change that prevents naming conflicts as the application grows to include multiple services.Consider adding a comment block documenting these environment variables and their purpose:
/* eslint-disable */ import axios from 'axios'; +/** + * Environment variables: + * - GATEWAY_HOST: Host address for the gateway service (default: 'localhost') + * - GATEWAY_PORT: Port number for the gateway service (default: '3333') + */ module.exports = async function () {apps/users-e2e/src/support/test-setup.ts (2)
7-9: Consider adding validation for connection parameters.While the default values are sensible, consider adding validation to ensure:
- Port number is within valid range (0-65535)
- Host format is valid
- Service is actually reachable before tests begin
const host = process.env.USERS_HOST ?? 'localhost'; const port = process.env.USERS_PORT ?? '15001'; +const portNum = parseInt(port, 10); +if (isNaN(portNum) || portNum < 0 || portNum > 65535) { + throw new Error(`Invalid port number: ${port}`); +} + axios.defaults.baseURL = `http://${host}:${port}`; + +// Verify service is reachable +try { + await axios.get('/health'); +} catch (error) { + throw new Error(`Users service not reachable at ${axios.defaults.baseURL}`); +}
7-9: Consider implementing service discovery for test environments.As the number of services grows (users, auth, etc.), managing individual host/port configurations becomes more complex. Consider implementing a service discovery mechanism or a central test configuration service.
This could involve:
- Using a service registry (e.g., Consul, Eureka)
- Implementing a test-specific service discovery mechanism
- Using Docker Compose for test environments with service name resolution
apps/tasks-e2e/src/support/test-setup.ts (2)
4-9: Consider TypeScript improvementsWhile the code is functional, we can improve type safety and maintainability.
Consider this enhanced version:
/* eslint-disable */ import axios from 'axios'; -module.exports = async function () { +interface ServiceConfig { + host: string; + port: string; +} + +export const getServiceConfig = (): ServiceConfig => ({ + host: process.env.TASKS_HOST ?? 'localhost', + port: process.env.TASKS_PORT ?? '15002', +}); + +export default async function setupAxios(): Promise<void> { - const host = process.env.TASKS_HOST ?? 'localhost'; - const port = process.env.TASKS_PORT ?? '15002'; + const { host, port } = getServiceConfig(); axios.defaults.baseURL = `http://${host}:${port}`; -}; +}This refactor:
- Uses proper TypeScript exports
- Separates config logic for better testing
- Adds type safety with interfaces
Line range hint
1-1: Remove eslint-disable commentInstead of disabling ESLint completely, consider addressing specific rules if needed.
-/* eslint-disable */ +// TODO: Address specific ESLint rules if needed
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
apps/auth-e2e/src/support/test-setup.ts(1 hunks)apps/gateway-e2e/src/support/test-setup.ts(1 hunks)apps/tasks-e2e/src/support/test-setup.ts(1 hunks)apps/users-e2e/src/support/test-setup.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/auth-e2e/src/support/test-setup.ts
🔇 Additional comments (3)
apps/gateway-e2e/src/support/test-setup.ts (1)
7-9: Verify port number consistency across services.
Since this PR introduces a new auth service alongside the gateway, let's ensure port numbers are consistently configured across services.
✅ Verification successful
Port configurations are consistent across services
The port configurations are well-organized and consistent across the codebase:
- Gateway service uses port 3333
- Users service uses port 15001
- Tasks service uses port 15002
- Auth service uses port 15003
Each service has a dedicated port range, with microservices using 15xxx range and the gateway using 3333, preventing any conflicts.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check port configurations across different services to ensure consistency
# Expected: Find all port configurations to verify they don't conflict
# Search for port configurations in environment files
echo "Checking environment files for port configurations:"
rg -n "PORT=" --type-add 'env:*.{env,env.*}' --type env
# Search for hardcoded port numbers in setup files
echo -e "\nChecking setup files for port configurations:"
fd -e ts -e js test-setup | xargs rg -n "port.*=.*\d{4}"
Length of output: 868
apps/users-e2e/src/support/test-setup.ts (1)
7-8: LGTM! Clear and consistent environment variable naming.
The change to use service-specific environment variables (USERS_HOST, USERS_PORT) improves clarity and prevents conflicts in a multi-service architecture.
apps/tasks-e2e/src/support/test-setup.ts (1)
6-7: Verify port allocation scheme across services
The default port 15002 for the tasks service suggests a planned port allocation scheme (auth:15003, tasks:15002). Let's verify this is consistent across services.
Consider documenting the port allocation scheme in a central location (e.g., README.md) to make it easier for developers to understand the service port assignments.
✅ Verification successful
Port allocation scheme is consistent across services
The verification confirms a clear and consistent port allocation scheme across the microservices:
- Users service: 15001
- Tasks service: 15002
- Auth service: 15003
This follows a logical sequential numbering pattern, indicating good architectural planning.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for port allocations across services to ensure consistency
# Expected: Find all port configurations to verify they follow the planned scheme
# Search for port configurations in environment files
echo "Checking environment files for port configurations:"
rg -n "PORT.*=.*150\d{2}" --type env
# Search for hardcoded ports in source files
echo "Checking source files for port configurations:"
rg -n "port.*=.*['\"](150\d{2})['\"]" --type ts
Length of output: 613
|
🎉 This PR is included in version 1.15.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Related #97 #71
Summary by CodeRabbit
Release Notes
New Features
authandauth-e2eapplications.Bug Fixes
Documentation
authandauth-e2eapplications added.Chores
nx.jsonto exclude specific end-to-end test files from Jest processing.