feat(users): implement OAuth SRP binding flow

[HuanCheng65]

No description provided.

[coderabbitai]

Caution

Review failed

Failed to post review comments.

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 82c2470 and 40bd866.

⛔ Files ignored due to path filters (2)

• package.json is excluded by !**/*.json
• pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml, !**/*.yaml

📒 Files selected for processing (12)

• src/auth/auth.service.ts (2 hunks)
• src/main.ts (2 hunks)
• src/users/DTO/oauth.dto.ts (1 hunks)
• src/users/helpers/cookie.helper.spec.ts (1 hunks)
• src/users/helpers/cookie.helper.ts (1 hunks)
• src/users/users.controller.spec.ts (8 hunks)
• src/users/users.controller.ts (17 hunks)
• src/users/users.error.ts (1 hunks)
• src/users/users.service.spec.ts (19 hunks)
• src/users/users.service.ts (20 hunks)
• src/users/users.spec.ts (3 hunks)
• test/user.e2e-spec.ts (8 hunks)

🧰 Additional context used

🧬 Code Graph Analysis (8)

src/users/users.error.ts (1)

src/common/error/base-error.ts (1)

• BaseError (21-29)

src/users/users.spec.ts (1)

src/users/users.error.ts (1)

• InvalidLoginCredentialsError (284-288)

src/users/helpers/cookie.helper.ts (1)

src/users/users.controller.ts (1)

• refreshToken (406-454)

src/users/DTO/oauth.dto.ts (1)

src/common/DTO/base-response.dto.ts (1)

• BaseResponseDto (12-23)

src/users/helpers/cookie.helper.spec.ts (2)

src/users/helpers/cookie.helper.ts (1)

• CookieHelper (14-88)

src/users/users.controller.ts (1)

• refreshToken (406-454)

src/users/users.service.spec.ts (1)

src/auth/oauth/oauth.types.ts (1)

• OAuthUserInfo (8-14)

src/users/users.controller.ts (4)

src/common/interceptor/token-validate.interceptor.ts (1)

• NoAuth (35-35)

src/users/DTO/oauth.dto.ts (4)

• OAuthCreateUserRequestDto (150-173)
• OAuthSrpBindInitRequestDto (120-126)
• OAuthSrpBindInitResponseDto (129-135)
• OAuthSrpBindVerifyRequestDto (138-147)

src/auth/auth.error.ts (1)

• InvalidTokenError (19-23)

src/users/users.error.ts (2)

• UsernameNotFoundError (108-116)
• InvalidLoginCredentialsError (284-288)

src/users/users.service.ts (4)

src/users/DTO/user.dto.ts (1)

• UserDto (1-12)

src/users/users.error.ts (11)

• UserIdNotFoundError (102-106)
• InvalidLoginCredentialsError (284-288)
• InvalidEmailAddressError (12-20)
• InvalidEmailSuffixError (22-33)
• EmailSendFailedError (45-49)
• InvalidUsernameError (51-62)
• InvalidNicknameError (64-75)
• UsernameAlreadyRegisteredError (83-91)
• EmailAlreadyRegisteredError (35-43)
• SrpNotUpgradedError (217-225)
• SrpVerificationError (227-231)

src/auth/oauth/oauth.types.ts (1)

• OAuthUserInfo (8-14)

src/users/DTO/oauth.dto.ts (1)

• OAuthUserDto (40-42)

🪛 GitHub Actions: Build and Test Dev Docker

src/users/users.spec.ts

[error] 396-396: Test error: Failed to publish cache invalidation message for user ID: 1 to Redis channel 'cache:user:updated'. Reason: Redis connection failed.

src/users/users.controller.spec.ts

[error] 1291-1291: Test failure: Expected mockUsersService.createOAuthUserFromDecision to be called with 5 arguments but received 8. Mismatch in function call arguments.

🪛 GitHub Actions: Automatic Test

src/users/users.spec.ts

[error] 396-396: Redis connection failed during cache invalidation message publish to channel 'cache:user:updated'.

src/users/users.controller.spec.ts

[error] 1291-1291: Jest test failure: Expected call arguments mismatch in 'createOAuthUserFromDecision'. Expected 5 arguments but received 8.

---

[error] Multiple OAuth callback and binding errors reported by UsersController including: 'OAuth callback failed: Some OAuth error', 'OAuth binding callback failed: Service error during binding', 'OAuth binding callback failed: Invalid binding state format', 'OAuth verification failed: Verification failed', 'OAuth callback failed: Authorization code is required', 'OAuth callback failed: Failed to get user info', 'OAuth callback failed: OAuth flow error', 'OAuth binding initialization failed: Binding init failed', 'OAuth binding initialization failed: URL generation failed', 'OAuth unbinding failed: Unbind failed'.

---

[error] OAuth callback errors with invalid authorization codes reported: 'OAuth callback failed: Invalid authorization code' repeated twice.

---

[error] OAuth verification failures including 'PasswordNotMatchError' for users 'TestLegacyUser-7865507314' and 'TestLegacyUser-7949979987', and 'OAuth session not found or expired'.

---

[error] OAuth binding to existing user failed due to invalid login credentials and invalid or expired OAuth state tokens, and username already registered error for 'TestLegacyUser-4517685835'.

🪛 GitHub Actions: Automatic Test Coverage

src/users/users.spec.ts

[error] 396-396: Redis connection failed error during test: Failed to publish cache invalidation message for user ID: 1 to Redis channel 'cache:user:updated'.

src/users/users.controller.spec.ts

[error] 1291-1291: Test failure in UsersController Additional OAuth methods createOAuthUser: Expected call to createOAuthUserFromDecision with 5 arguments but received 8 arguments.

🪛 Biome (1.9.4)

src/users/helpers/cookie.helper.spec.ts

[error] 434-434: Avoid the delete operator which can impact performance.

Unsafe fix: Use an undefined assignment instead.

(lint/performance/noDelete)

src/users/users.controller.ts

[error] 1408-1409: Change to an optional chain.

Unsafe fix: Change to an optional chain.

(lint/complexity/useOptionalChain)

🔇 Additional comments (44)

src/main.ts (1)

23-24: Verify cookie secret configuration.

The cookie parser is configured with process.env.COOKIE_SECRET which could be undefined. While this will still work (unsigned cookies), ensure this is intentional for your security requirements.

Consider adding a fallback or validation:

- app.use(cookieParser(process.env.COOKIE_SECRET));
+ app.use(cookieParser(process.env.COOKIE_SECRET || undefined));

Or verify that COOKIE_SECRET is properly set in your environment configuration.

src/users/users.error.ts (1)

284-288: LGTM! Good security practice.

The generic error message prevents user enumeration attacks while providing appropriate feedback for authentication failures. The implementation correctly extends BaseError with the proper 401 status code.

src/users/DTO/oauth.dto.ts (1)

119-173: Well-structured OAuth SRP binding DTOs.

The new DTOs are properly designed with appropriate validation decorators. The restriction of passwordMode to only 'none' or 'srp' (line 172) aligns with modern security practices by removing legacy password support. All required and optional fields are correctly annotated.

src/users/helpers/cookie.helper.ts (3)

32-37: Excellent environment-aware security configuration.

The cookie options properly adapt to the environment - using secure and strict sameSite in production while allowing lax in development. This provides good security while maintaining functionality during development.

---

55-57: Good integration with cookie-parser middleware.

The static method properly leverages the cookie-parser middleware configured in main.ts, providing a clean abstraction for accessing parsed cookies.

---

69-75: Correct OAuth redirect cookie configuration.

The sameSite: 'lax' setting is essential for OAuth flows that involve cross-domain redirects. The 10-minute default expiration is reasonable for OAuth state management.

src/auth/auth.service.ts (2)

82-82: Enhanced input validation improves security.

Adding the typeof token !== 'string' check strengthens input validation by ensuring only string tokens are processed, preventing potential issues with unexpected input types.

---

228-228: Consistent validation across verify and decode methods.

The same validation logic is properly applied to both verify() and decode() methods, ensuring consistent input handling throughout the authentication service.

src/users/users.spec.ts (3)

49-49: Good security improvement!

Importing InvalidLoginCredentialsError aligns with the security enhancement to prevent user enumeration.

---

1031-1038: Consistent error handling for non-upgraded users.

The test correctly expects the generic InvalidLoginCredentialsError instead of revealing that the user exists but isn't upgraded to SRP.

---

1041-1051: Proper error handling for failed SRP verification.

The test correctly expects the generic InvalidLoginCredentialsError for failed verification, preventing attackers from distinguishing between invalid users and invalid credentials.

src/users/helpers/cookie.helper.spec.ts (3)

1-62: Well-structured test setup!

The test file has proper imports, well-organized mock functions, and clear test structure.

---

64-182: Comprehensive coverage for setRefreshTokenCookie.

The tests cover all important scenarios including environment-specific behavior, cookie base paths, and edge cases.

---

282-392: Good OAuth redirect cookie test coverage.

Tests properly verify cookie settings for OAuth flows with appropriate security settings and expiration handling.

test/user.e2e-spec.ts (5)

622-636: Excellent security improvement in login error handling!

Both tests now expect the generic InvalidLoginCredentialsError, preventing attackers from determining whether a username exists or distinguishing between authentication failures.

---

710-720: Good enumeration prevention for password recovery.

Returning success for non-existent emails prevents attackers from discovering valid email addresses through the password reset endpoint.

---

1253-1298: Well-implemented OAuth state endpoint test!

The test properly verifies the OAuth state retrieval functionality with unique test data generation to avoid conflicts.

---

1392-1400: Smart use of dynamic test data generation!

Using timestamps and random numbers for OAuth user IDs and emails prevents test conflicts when running tests in parallel or repeatedly. The email regex validation is also correctly implemented.

Also applies to: 1483-1485

---

1744-2041: Comprehensive OAuth decision flow test coverage!

Excellent coverage of OAuth binding scenarios including:

• Successful binding with password and SRP
• Invalid credentials handling
• State token validation
• Username conflicts
• Email conflicts with SRP users
• Expired token handling

The tests are well-structured and cover both happy paths and error scenarios.

src/users/users.controller.spec.ts (5)

63-66: LGTM! Mock methods align with new OAuth flow.

The addition of these mock methods properly supports testing the new OAuth SRP binding flow functionality.

---

696-734: Good security improvement to prevent user enumeration.

The refactoring to use direct Prisma queries instead of service exceptions helps prevent user enumeration attacks by returning a consistent response regardless of whether the user exists.

---

864-952: Well-structured OAuth flow handling.

The tests correctly handle all three OAuth flow outcomes: direct login, verification requirement, and user decision requirement. The redirect logic and query parameter construction are properly validated.

---

1400-1643: Comprehensive error handling coverage.

The error handling tests thoroughly cover various failure scenarios including missing authorization codes, service failures, and invalid state formats. The edge case tests for OAuth callback state handling are particularly well done.

---

1289-1297: Let’s inspect the controller spec to see how the DTO and createOAuthUser call are set up:

#!/bin/bash
# Show the test setup and expectation for createOAuthUser in the controller spec
rg -n "createOAuthUser" -C5 src/users/users.controller.spec.ts

src/users/users.service.spec.ts (7)

37-42: Good addition of bcrypt mock for security testing.

The bcrypt mock enables proper testing of the dummy password comparison security feature to prevent timing attacks.

---

589-829: Excellent test coverage for initiateOAuthFlow scenarios.

The tests comprehensively cover all OAuth flow paths including existing connections, SRP users, legacy users, and new user registration. The handling of users without email addresses is particularly well tested.

---

831-1196: Thorough validation and error handling in OAuth user creation tests.

The tests excellently cover state token validation, username/nickname generation, and comprehensive error scenarios including race conditions. The handling of users without email addresses using placeholder emails is well implemented.

---

1251-1628: Comprehensive OAuth binding tests with proper security validations.

The tests thoroughly validate all binding scenarios including credential verification, state token validation, and prevention of duplicate OAuth account linking. The restriction preventing SRP users from using the decision flow is a good security measure.

---

2085-2128: Excellent security implementation against timing attacks.

The dummy password comparison using a fixed bcrypt hash prevents timing-based user enumeration attacks. This ensures consistent response times regardless of whether the user exists or a database error occurs.

---

2064-2067: Good security validation preventing lockout.

The test properly validates that users cannot unbind their only authentication method, preventing accidental account lockout.

---

2396-2400: Good validation of username/nickname sanitization.

The test properly validates that special characters are sanitized and length limits are enforced for suggested usernames and nicknames.

src/users/users.controller.ts (5)

153-153: LGTM! Good addition of global error handling.

Adding the BaseErrorExceptionFilter at the controller level ensures consistent error handling across all endpoints.

---

219-219: Good security improvement with rate limiting.

The rate limits are well-chosen for each endpoint type, providing good protection against brute force attacks and abuse while maintaining usability.

Also applies to: 234-234, 349-349, 481-481, 916-916, 1120-1120, 1146-1146

---

1408-1409: Consider using optional chaining for cleaner code.

The static analysis tool correctly identifies an opportunity to use optional chaining here.

-      if (Array.isArray(result)) {
-        // 已有OAuth连接，直接登录成功
+      // 已有OAuth连接，直接登录成功
+      if (Array.isArray(result)) {
         const [userDto, refreshToken] = result;
         await this.handleSuccessfulOAuthRedirect(res, refreshToken, userDto);

Note: The suggestion from static analysis about optional chaining might not be directly applicable here since we're checking if result is an array, not accessing a property. The current code is actually correct as-is.

---

171-210: Excellent refactoring with the helper method.

The handleSuccessfulOAuthRedirect method effectively extracts common OAuth redirect logic, reducing code duplication and ensuring consistent behavior across all OAuth-related endpoints.

---

1297-1322: Clean implementation of OAuth state retrieval.

The endpoint properly delegates to the service layer and follows REST conventions.

src/users/users.service.ts (8)

645-648: Good performance improvement with async bcrypt!

The transition from synchronous to asynchronous bcrypt operations prevents blocking the event loop during password hashing, which improves application responsiveness.

---

704-828: Excellent solution for N+1 query problem!

The batch user DTO fetching implementation is well-designed:

• Efficiently deduplicates IDs while preserving order
• Uses Maps for O(1) lookups
• Performs minimal database queries
• Properly handles error cases with UserIdNotFoundError

This will significantly improve performance when fetching multiple users.

---

905-979: Excellent security implementation to prevent user enumeration!

The secure login method implements best practices:

• Constant-time operations regardless of user existence
• Dummy password comparison with fixed hash for timing attack prevention
• Generic error messages that don't reveal user information
• Proper error handling for database failures

This significantly improves the application's security posture.

---

1100-1167: Good security improvement to prevent email enumeration!

The method now properly prevents attackers from discovering valid email addresses:

• Always succeeds from the API perspective
• Performs actual email sending only for existing users
• Logs attempts for non-existent emails without revealing this to the caller

This follows security best practices for password reset flows.

---

1728-1789: Consistent security improvements in SRP authentication!

Both SRP methods now prevent user enumeration by returning generic InvalidLoginCredentialsError for:

• Non-existent users
• Users without SRP credentials
• Failed verifications

This maintains security consistency across all authentication methods.

---

1862-1949: Well-structured OAuth flow implementation!

The method clearly handles different scenarios:

1. Existing OAuth connections → direct login
2. Email conflicts → verification required (password or SRP)
3. New users → decision page with state token

The return type union effectively communicates the different outcomes to callers.

---

3086-3249: Well-implemented OAuth SRP binding flow!

The new methods provide a complete solution for SRP users to bind OAuth accounts:

• Proper session management with 15-minute expiration
• Consistent error handling and validation
• Prevention of duplicate bindings
• Clear separation between initialization and completion phases

This addresses the SRP user binding limitation mentioned earlier.

---

2204-2348: ```shell
#!/bin/bash

Check for changelog or documentation regarding removal of legacy password support

echo "Searching for CHANGELOG files..."
fd -HI "CHANGELOG*" .
echo

echo "Searching for references to 'legacy password' in repository..."
rg -in "legacy password" .
echo

echo "Searching for a migration guide or notes..."
rg -in "migration" .

</details>

</blockquote></details>

Walkthrough

This update introduces a comprehensive refactor and expansion of OAuth and SRP authentication flows. It adds new DTOs, controller endpoints, and service methods for OAuth user creation, binding, and SRP integration. Enhanced error handling, rate limiting, and security improvements such as prevention of user/email enumeration and batch user DTO fetching are included. Extensive new and updated tests cover these flows.

Changes

File(s)	Change Summary
src/auth/auth.service.ts	Added stricter type checking for the token parameter in verify and decode methods, throwing errors if the token is not a string.
src/main.ts	Integrated cookie-parser middleware for cookie handling in the NestJS app.
src/users/DTO/oauth.dto.ts	Added new DTOs for OAuth SRP binding and OAuth user creation, supporting new binding and registration flows.
src/users/helpers/cookie.helper.ts src/users/helpers/cookie.helper.spec.ts	Introduced CookieHelper class for consistent cookie management and comprehensive unit tests for its methods.
src/users/users.controller.ts	Major refactor: Added new OAuth endpoints (state, create, bind, SRP binding), applied global error filter, added rate limiting, improved error handling, and refactored OAuth callback logic to support multi-step flows.
src/users/users.controller.spec.ts	Refactored and expanded tests to cover new OAuth endpoints, flows, error cases, and edge scenarios.
src/users/users.error.ts	Added InvalidLoginCredentialsError for unified invalid credential handling.
src/users/users.service.ts	Major refactor: Added batch user DTO fetching, secure login to prevent enumeration, new OAuth flow methods (initiate, state, create, bind, SRP binding), improved error handling, and performance optimizations.
src/users/users.service.spec.ts	Extensively expanded and refactored tests for new OAuth and SRP flows, error cases, and login scenarios.
src/users/users.spec.ts	Updated tests to expect InvalidLoginCredentialsError instead of SRP-specific errors for SRP failures.
test/user.e2e-spec.ts	Updated and expanded e2e tests for OAuth flows, error handling, decision endpoints, and login/password recovery behaviors.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant Frontend
    participant UsersController
    participant UsersService
    participant AuthService
    participant CookieHelper

    User->>Frontend: Initiate OAuth login
    Frontend->>UsersController: /auth/oauth/callback?code=...&state=...
    UsersController->>UsersService: initiateOAuthFlow(providerId, userInfo, ip, ua)
    alt Existing OAuth user
        UsersService-->>UsersController: [OAuthUserDto, refreshToken]
        UsersController->>CookieHelper: setRefreshTokenCookie(res, refreshToken, expires)
        UsersController->>Frontend: Redirect to success page
    else Requires verification (password/SRP)
        UsersService-->>UsersController: {requiresVerification, ...}
        UsersController->>Frontend: Redirect to verification page
    else Requires user decision
        UsersService-->>UsersController: {requiresDecision, stateToken}
        UsersController->>Frontend: Redirect to decision page with stateToken
    end

    User->>Frontend: Submit decision (create/bind)
    Frontend->>UsersController: /oauth/create or /oauth/bind with stateToken, credentials
    UsersController->>UsersService: createOAuthUserFromDecision(...) or bindOAuthToExistingUser(...)
    UsersService-->>UsersController: [OAuthUserDto, refreshToken]
    UsersController->>CookieHelper: setRefreshTokenCookie(res, refreshToken, expires)
    UsersController->>Frontend: Redirect to success page

Loading

Possibly related PRs

• #386: Introduces foundational SRP authentication, which is directly extended by this PR's SRP binding and verification features.
• #448: Also refactors OAuth integration and user management, overlapping with this PR's OAuth flow enhancements and error handling.
• #451: Shares identical OAuth flow refactoring, token validation, and CookieHelper introduction, indicating a direct code-level relationship.

Suggested reviewers

• andylizf

Poem

In the warren of code, a rabbit did find
OAuth and SRP, all neatly aligned.
With cookies and tokens, and tests quite robust,
New flows for the users, in security we trust.
So hop along, friends, and give it a try—
For login and binding now reach for the sky!
🥕✨

✨ Finishing Touches

• 📝 Generate Docstrings

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

• Review comments: Directly reply to a review comment made by CodeRabbit. Example:
  • I pushed a fix in commit <commit_id>, please review it.
  • Explain this complex logic.
  • Open a follow-up GitHub issue for this discussion.
• Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
  • @coderabbitai explain this code block.
  • @coderabbitai modularize this function.
• PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
  • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
  • @coderabbitai read src/utils.ts and explain its main purpose.
  • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
  • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

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)

• @coderabbitai pause to pause the reviews on a PR.
• @coderabbitai resume to resume the paused reviews.
• @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
• @coderabbitai full review to do a full review from scratch and review all the files again.
• @coderabbitai summary to regenerate the summary of the PR.
• @coderabbitai generate docstrings to generate docstrings for this PR.
• @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
• @coderabbitai resolve resolve all the CodeRabbit review comments.
• @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
• @coderabbitai help to get help.

Other keywords and placeholders

• Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
• Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
• Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

• You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
• Please see the configuration documentation for more information.
• If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

• Visit our Documentation for detailed information on how to use CodeRabbit.
• Join our Discord Community to get help, request features, and share feedback.
• Follow us on X/Twitter for updates and announcements.