-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: ✨ add mutation signIn of AuthResolver #82
Conversation
WalkthroughThis pull request introduces several new files and configurations for the Changes
Possibly related PRs
Warning Rate limit exceeded@zhumeisongsong has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 13 minutes and 22 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 9
🧹 Outside diff range and nitpick comments (3)
libs/auth/interface-adapters/src/lib/resolver/auth.resolver.spec.ts (3)
31-49
: Add test cases for error scenariosWhile the happy path is tested, consider adding tests for:
- AuthService throwing exceptions
- Invalid input validation
- Rate limiting scenarios
Example additional test:
it('should handle AuthService errors', async () => { const signInInput: SignInInputDto = { email: '[email protected]', password: 'wrong', }; jest.spyOn(authService, 'signIn').mockRejectedValue( new UnauthorizedException('Invalid credentials') ); await expect(resolver.signIn(signInInput)).rejects.toThrow( UnauthorizedException ); });
33-36
: Consider using test data factoriesInstead of inline test data, consider using a test data factory for SignInInputDto to improve test maintainability.
Example factory:
const createSignInInput = (override?: Partial<SignInInputDto>): SignInInputDto => ({ email: '[email protected]', password: 'password123', ...override, });
1-50
: Security considerations for authentication implementationPlease ensure the following security measures are implemented:
- Rate limiting to prevent brute force attacks
- Password hashing in AuthService
- Secure token generation with appropriate expiration
- HTTPS-only cookie for token storage
- CSRF protection
Consider implementing a rate limiting guard:
@Injectable() class AuthRateLimitGuard implements CanActivate { private keyPrefix = 'login_attempts'; constructor(private readonly redis: Redis) {} async canActivate(context: ExecutionContext): Promise<boolean> { const client = context.getArgByIndex(2).req.ip; const key = `${this.keyPrefix}:${client}`; // Implement rate limiting logic } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (23)
libs/auth/application/README.md
(1 hunks)libs/auth/application/eslint.config.js
(1 hunks)libs/auth/application/jest.config.ts
(1 hunks)libs/auth/application/project.json
(1 hunks)libs/auth/application/src/index.ts
(1 hunks)libs/auth/application/src/lib/auth.service.spec.ts
(1 hunks)libs/auth/application/src/lib/auth.service.ts
(1 hunks)libs/auth/application/tsconfig.json
(1 hunks)libs/auth/application/tsconfig.lib.json
(1 hunks)libs/auth/application/tsconfig.spec.json
(1 hunks)libs/auth/interface-adapters/README.md
(1 hunks)libs/auth/interface-adapters/eslint.config.js
(1 hunks)libs/auth/interface-adapters/jest.config.ts
(1 hunks)libs/auth/interface-adapters/project.json
(1 hunks)libs/auth/interface-adapters/src/index.ts
(1 hunks)libs/auth/interface-adapters/src/lib/auth.module.ts
(1 hunks)libs/auth/interface-adapters/src/lib/dto/sign-in-input.dto.ts
(1 hunks)libs/auth/interface-adapters/src/lib/resolver/auth.resolver.spec.ts
(1 hunks)libs/auth/interface-adapters/src/lib/resolver/auth.resolver.ts
(1 hunks)libs/auth/interface-adapters/tsconfig.json
(1 hunks)libs/auth/interface-adapters/tsconfig.lib.json
(1 hunks)libs/auth/interface-adapters/tsconfig.spec.json
(1 hunks)tsconfig.base.json
(2 hunks)
✅ Files skipped from review due to trivial changes (16)
- libs/auth/application/README.md
- libs/auth/interface-adapters/eslint.config.js
- libs/auth/application/project.json
- libs/auth/interface-adapters/src/index.ts
- libs/auth/application/tsconfig.spec.json
- libs/auth/interface-adapters/README.md
- libs/auth/application/src/index.ts
- libs/auth/application/jest.config.ts
- libs/auth/application/eslint.config.js
- libs/auth/interface-adapters/tsconfig.spec.json
- libs/auth/interface-adapters/tsconfig.lib.json
- libs/auth/interface-adapters/jest.config.ts
- libs/auth/interface-adapters/tsconfig.json
- libs/auth/interface-adapters/project.json
- libs/auth/application/tsconfig.json
- libs/auth/application/tsconfig.lib.json
🧰 Additional context used
🪛 Biome (1.9.4)
libs/auth/application/src/lib/auth.service.ts
[error] 2-2: This constructor is unnecessary.
Unsafe fix: Remove the unnecessary constructor.
(lint/complexity/noUselessConstructor)
🪛 eslint
libs/auth/application/src/lib/auth.service.ts
[error] 2-2: Unexpected empty constructor.
(@typescript-eslint/no-empty-function)
🔇 Additional comments (3)
tsconfig.base.json (2)
18-19
: LGTM! Verify the new module paths exist.
The new path aliases follow the project's established naming conventions and structure.
Let's verify that the referenced module entry points exist:
✅ Verification successful
Path aliases are correctly configured and files exist
Both module entry points exist at the specified paths:
- libs/auth/application/src/index.ts
- libs/auth/interface-adapters/src/index.ts
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that the new auth module entry points exist
# Check if the auth module entry points exist
for path in "libs/auth/application/src/index.ts" "libs/auth/interface-adapters/src/index.ts"; do
if [ ! -f "$path" ]; then
echo "❌ Missing file: $path"
exit 1
else
echo "✅ Found: $path"
fi
done
Length of output: 508
32-33
: Verify aws-cognito imports still work after reordering.
While the reordering doesn't affect TypeScript's path resolution, let's ensure existing imports aren't broken.
✅ Verification successful
Let me try another search pattern to verify the imports, as the previous command had an issue with the file type.
No issues found with aws-cognito path mapping
The verification shows that:
- The aws-cognito entry point exists and correctly exports the module and service
- The module structure is intact with proper TypeScript files
- No direct imports of "aws-cognito" were found, suggesting the path mapping is likely used in build configuration only
The reordering in tsconfig.base.json is safe as the path mapping is valid and the module structure is correct.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for aws-cognito imports in the codebase
# Search for aws-cognito imports
echo "Checking for aws-cognito imports:"
rg -t ts -t tsx "from ['\"]aws-cognito['\"]" -l
# Verify the target path exists
if [ -f "libs/shared/infrastructure/aws-cognito/src/index.ts" ]; then
echo "✅ aws-cognito entry point exists"
else
echo "❌ aws-cognito entry point is missing"
fi
Length of output: 298
Script:
#!/bin/bash
# Description: Check for aws-cognito imports with corrected file type and expanded search
# Search for aws-cognito imports with both possible quote types
echo "Checking for aws-cognito imports:"
rg --type ts "from ['\"]aws-cognito['\"]"
# List all TypeScript files that might use aws-cognito
echo -e "\nListing files in the aws-cognito directory:"
fd -e ts . libs/shared/infrastructure/aws-cognito/src
# Check the content of the entry point
echo -e "\nChecking index.ts content:"
cat libs/shared/infrastructure/aws-cognito/src/index.ts
Length of output: 761
libs/auth/interface-adapters/src/lib/auth.module.ts (1)
1-9
: Verify JWT module configuration
Ensure proper JWT module configuration for token generation and validation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
libs/auth/interface-adapters/src/lib/auth.module.ts (1)
8-8
: Consider exporting AuthServiceThe module currently doesn't export any providers. If other modules need to use authentication functionality, consider exporting
AuthService
.providers: [AuthResolver, AuthService], - exports: [], + exports: [AuthService],
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
libs/auth/interface-adapters/src/lib/auth.module.ts
(1 hunks)
🔇 Additional comments (2)
libs/auth/interface-adapters/src/lib/auth.module.ts (2)
6-9
:
Add required authentication dependencies
The module is missing essential dependencies for a proper authentication setup.
The module should include:
- JWT module for token handling
- Configuration module for managing auth settings
- (Optional) Passport module if using passport strategies
@Module({
+ imports: [
+ JwtModule.register({
+ secret: process.env.JWT_SECRET,
+ signOptions: { expiresIn: '1h' },
+ }),
+ ConfigModule.forRoot(),
+ ],
providers: [AuthResolver, AuthService],
exports: [],
})
1-1
: Verify path alias configuration
The import path @auth/application
needs to be properly configured in the TypeScript configuration.
✅ Verification successful
Path alias @auth/application
is properly configured
The import path @auth/application
is correctly configured in the project:
- Mapped in
tsconfig.base.json
tolibs/auth/application/src/index.ts
- Used consistently across multiple files in the auth module
- Directory structure exists and matches the configuration
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify path alias configuration in tsconfig files
# Check if the path alias is configured in tsconfig
fd tsconfig.*.json -x cat {} \; | jq -r 'select(.compilerOptions.paths."@auth/*" != null) | .compilerOptions.paths."@auth/*"'
# Verify the actual path exists
fd -t d "auth/application"
Length of output: 160
Script:
#!/bin/bash
# Let's try a different approach to verify the path alias and module structure
# First, let's check the content of tsconfig.base.json directly
echo "=== Checking tsconfig.base.json ==="
cat tsconfig.base.json
# Then, let's look for any auth-related directories
echo -e "\n=== Looking for auth-related directories ==="
fd -t d auth
# Finally, let's check if there are any imports using @auth/application in the codebase
echo -e "\n=== Checking usage of @auth/application ==="
rg "@auth/application" --type ts
Length of output: 1968
🎉 This PR is included in version 1.5.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Related: #71
Summary by CodeRabbit
Release Notes
New Features
AuthService
for handling user authentication, including asignIn
method.AuthResolver
for GraphQL integration, enabling authentication operations via GraphQL.SignInInputDto
for structured input during sign-in requests.Documentation
auth-application
andauth-interface-adapters
libraries, providing setup and testing instructions.Tests
AuthService
andAuthResolver
, enhancing test coverage and reliability.Configuration
auth-application
andauth-interface-adapters
libraries, ensuring proper compilation and testing setup.