Skip to content
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

Implement Refresh Token #317

Merged
merged 17 commits into from
Sep 11, 2024
Merged

Implement Refresh Token #317

merged 17 commits into from
Sep 11, 2024

Conversation

xet-a
Copy link
Contributor

@xet-a xet-a commented Aug 25, 2024

What this PR does / why we need it?

This PR implements a Refresh Token mechanism alongside the existing Access Token authentication method. The addition of a Refresh Token enhances security and usability by allowing users to seamlessly obtain new Access Tokens without the need for reauthentication every time the Access Token expires.

The changes include:

  • Frontend:

    • Saving the Refresh Token alongside the Access Token upon successful authentication.
    • Adding an Axios interceptor to automatically call the /auth/refresh endpoint when the Access Token is detected as expired.
    • Implementing a POST request to the /auth/refresh endpoint to retrieve a new Access Token.
  • Backend:

    • Generating the Refresh Token upon user authentication.
    • Implementing a RefreshStrategy to manage Refresh Token validity and secure handling.
    • Creating a POST /auth/refresh API to generate a new Access Token using a valid Refresh Token.

Currently, the Access Token has a validity of one day, while the Refresh Token is valid for one week.

Any background context you want to provide?

The introduction of a Refresh Token aligns with best practices in API authentication and allows for more robust user sessions while minimizing the friction caused by token expiration. Careful consideration has been given to token lifecycle management and the implications for existing client applications and authentication flows.

What are the relevant tickets?

Fixes #160

Checklist

  • Added relevant tests or not required
  • Didn't break anything

Summary by CodeRabbit

  • New Features

    • Introduced support for refresh tokens to enhance user session management.
    • Implemented token refresh logic in the application to improve authentication flow.
    • Enhanced JWT configuration for better security and management of access and refresh tokens.
  • Bug Fixes

    • Improved error handling for user data retrieval and authentication states.
  • Style

    • Reorganized import statements for clarity across several components.
  • Chores

    • Updated state management in the Redux store to accommodate refresh tokens.

@xet-a xet-a added the enhancement 🌟 New feature or request label Aug 25, 2024
@xet-a xet-a requested a review from devleejb August 25, 2024 03:20
@xet-a xet-a self-assigned this Aug 25, 2024
@CLAassistant
Copy link

CLAassistant commented Aug 25, 2024

CLA assistant check
All committers have signed the CLA.

Copy link
Contributor

coderabbitai bot commented Aug 25, 2024

Walkthrough

The changes implement a refresh token mechanism for authentication in the application, enhancing token management by introducing distinct access and refresh tokens. Modifications include updates to environment variables, authentication controller logic, service methods, and client-side token handling. New methods for token generation and validation are added, ensuring a seamless authentication process and improved security.

Changes

Files Change Summary
backend/.env.development JWT_AUTH_SECRET renamed to JWT_ACCESS_TOKEN_SECRET; new variables: JWT_ACCESS_TOKEN_EXPIRATION_TIME, JWT_REFRESH_TOKEN_SECRET, JWT_REFRESH_TOKEN_EXPIRATION_TIME.
backend/src/auth/auth.controller.ts Removed usersService and jwtService; added authService; login method now returns both access and refresh tokens; new refresh method added.
backend/src/auth/auth.service.ts Added loginWithSocialProvider method for social logins; introduced getNewAccessToken method for refresh token handling.
backend/src/auth/dto/refresh-token-request.dto.ts New class for refresh token request structure with validation.
backend/src/auth/dto/refresh-token-response.dto.ts New class for refresh token response structure.
backend/src/auth/types/login-response.type.ts Added refreshToken property to LoginResponse class.
frontend/src/App.tsx Integrated Axios interceptor for handling token refresh logic; improved error management.
frontend/src/components/popovers/ProfilePopover.tsx Updated handleLogout to dispatch logout action instead of clearing access token directly.
frontend/src/hooks/api/user.ts Included logout action; updated user query error handling to log out user on failure.
frontend/src/store/authSlice.ts Updated AuthState interface to include refreshToken; added setRefreshToken reducer and logout reducer.

Assessment against linked issues

Objective Addressed Explanation
Introduce Refresh Token for authentication ( #160 )
Enhance security and usability of authentication ( #160 )
Manage token expiration and renewal process ( #160 )
Ensure compatibility with existing authentication flows ( #160 ) No specific mention of impact on existing flows.

Poem

🐇 In the meadow where tokens gleam,
Refresh and access, a seamless dream.
With each new login, a hop and a bound,
User sessions secure, joyfully found!
A dance of data, oh what a delight,
In our cozy code, everything feels right! 🌼


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?

Share
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>.
    • Generate unit testing code for this file.
    • 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 generate unit testing code for this file.
    • @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 generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @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.

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 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.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

Outside diff range, codebase verification and nitpick comments (1)
backend/src/auth/jwt-refresh.strategy.ts (1)

1-26: Consider using optional chaining in jwtFromRequest.

The current implementation of the jwtFromRequest function uses a traditional conditional check. You can simplify this by using optional chaining:

  • Update the line if (req && req.body.refreshToken) to if (req?.body?.refreshToken).

This change will make the code more concise and modern.

  • Location: backend/src/auth/jwt-refresh.strategy.ts, line 10.
Analysis chain

Implement JwtRefreshStrategy.

The implementation of JwtRefreshStrategy is comprehensive and aligns with the objectives of securely handling refresh tokens. The use of configService.get for the secret key is a good practice for secure configuration management.

However, consider the static analysis hint:

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the use of optional chaining in jwtFromRequest.

# Test: Search for the jwtFromRequest implementation. Expect: Use of optional chaining.
ast-grep --lang typescript --pattern $'jwtFromRequest: (req) => {
  if (req && req.body.refreshToken) {
    return req.body.refreshToken
  }
  return null
}'

Length of output: 158


Script:

#!/bin/bash
# Description: Search for the jwtFromRequest function and check for the use of optional chaining.

# Search for the jwtFromRequest function in the codebase.
rg 'jwtFromRequest: \(req\) => {' -A 3

Length of output: 160


Script:

#!/bin/bash
# Description: Search for the jwtFromRequest function and check for the use of optional chaining.

# Corrected search pattern for the jwtFromRequest function in the codebase.
rg 'jwtFromRequest: \(req\) => \{' -A 3

Length of output: 316

Tools
Biome

[error] 13-13: Change to an optional chain.

Unsafe fix: Change to an optional chain.

(lint/complexity/useOptionalChain)

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 2ca8d32 and fbecc95.

Files selected for processing (14)
  • backend/.env.development (1 hunks)
  • backend/src/auth/auth.controller.ts (2 hunks)
  • backend/src/auth/auth.module.ts (2 hunks)
  • backend/src/auth/auth.service.spec.ts (1 hunks)
  • backend/src/auth/auth.service.ts (1 hunks)
  • backend/src/auth/jwt-refresh.strategy.ts (1 hunks)
  • backend/src/auth/jwt.strategy.ts (1 hunks)
  • backend/src/auth/types/login-response.type.ts (1 hunks)
  • backend/src/auth/types/refresh-token-request.type.ts (1 hunks)
  • frontend/src/App.tsx (4 hunks)
  • frontend/src/components/popovers/ProfilePopover.tsx (3 hunks)
  • frontend/src/hooks/api/user.ts (2 hunks)
  • frontend/src/pages/auth/callback/Index.tsx (1 hunks)
  • frontend/src/store/authSlice.ts (2 hunks)
Additional context used
Biome
backend/src/auth/jwt-refresh.strategy.ts

[error] 13-13: Change to an optional chain.

Unsafe fix: Change to an optional chain.

(lint/complexity/useOptionalChain)

Additional comments not posted (27)
backend/src/auth/jwt.strategy.ts (1)

9-14: Approve changes in JWT strategy, but verify configuration alignment.

The changes are appropriate for distinguishing the JWT strategy. Ensure that the JWT_ACCESS_TOKEN_SECRET is consistently used across all parts of the system where JWTs are handled.

Run the following script to verify the usage of JWT_ACCESS_TOKEN_SECRET across the project:

Verification successful

JWT secret key usage is consistent across authentication components.

The JWT_ACCESS_TOKEN_SECRET is consistently used in both jwt.strategy.ts and auth.module.ts, ensuring alignment in the JWT authentication setup. No further issues were found.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the consistent use of `JWT_ACCESS_TOKEN_SECRET` across the project.

# Test: Search for the secret key usage. Expect: Consistent usage across all relevant files.
rg --type typescript "JWT_ACCESS_TOKEN_SECRET"

Length of output: 81


Script:

#!/bin/bash
# Description: Verify the consistent use of `JWT_ACCESS_TOKEN_SECRET` across the project.

# Test: Search for the secret key usage. Expect: Consistent usage across all relevant files.
rg --type ts "JWT_ACCESS_TOKEN_SECRET"

Length of output: 240

frontend/src/pages/auth/callback/Index.tsx (4)

5-5: Update imports to include setRefreshToken.

The import statement has been correctly updated to include setRefreshToken which is necessary for the new functionality.


13-14: Retrieve both accessToken and refreshToken.

The code correctly retrieves both tokens from the URL search parameters, aligning with the new authentication flow.


16-16: Redirect if tokens are missing.

This conditional check ensures that both tokens must be present, otherwise, it redirects to the home page. This is a good security practice to prevent unauthorized access.


21-22: Dispatch tokens to the Redux store.

Tokens are correctly dispatched to the Redux store, ensuring that they are available throughout the application.

frontend/src/store/authSlice.ts (4)

7-7: Add refreshToken to AuthState.

Adding refreshToken to the AuthState is necessary for managing the refresh token's state across the application.


12-12: Initialize refreshToken in the initialState.

Initializing refreshToken to null in the initialState is correct and aligns with the handling of the accessToken.


22-24: Implement setRefreshToken reducer.

The setRefreshToken function is correctly implemented to update the refreshToken state based on the action's payload.


28-28: Export setRefreshToken.

Including setRefreshToken in the exported actions allows it to be dispatched from components, which is necessary for the new functionality.

backend/src/auth/auth.module.ts (3)

6-8: Import order and new strategy addition.

The import of JwtRefreshStrategy is correctly placed and aligns with the new functionality for handling refresh tokens.


27-27: Updated providers list with new strategy.

The addition of JwtRefreshStrategy to the providers list is crucial for enabling refresh token functionality. This change is consistent with the PR objectives.


18-21: Dynamic JWT configuration.

The use of ConfigService to dynamically set JWT expiration times and secrets enhances flexibility and maintainability. Ensure that the configuration values are properly set in the environment or configuration files.

Run the following script to verify the configuration settings:

backend/src/auth/auth.service.ts (3)

10-14: Constructor enhancements with new dependencies.

The addition of JwtService and ConfigService to the constructor is essential for handling JWT operations and configuration settings. This change supports the new token management functionality.


35-44: New method for access token renewal.

The getNewAccessToken method efficiently handles the generation of a new access token using a refresh token. This method is crucial for maintaining user sessions without frequent re-authentications.


16-33: New method for GitHub authentication.

The loginWithGithub method correctly handles user authentication via GitHub and manages both access and refresh tokens. Ensure that error handling is robust, especially in user creation and token generation processes.

Run the following script to verify error handling in the loginWithGithub method:

backend/src/auth/auth.controller.ts (3)

24-25: Dependency update in constructor.

The shift to using authService centralizes authentication logic, which is a positive change for maintainability and modularity.


39-42: Updated login method logic.

The updated login method now correctly handles the redirection with both access and refresh tokens. This change enhances the client-side handling of authentication states.


47-55: New method for token refresh.

The refresh method provides a structured way to renew access tokens, which is essential for secure and continuous user sessions. Ensure that the AuthGuard is properly configured to protect this endpoint.

Run the following script to verify the AuthGuard configuration for the refresh method:

Verification successful

AuthGuard Configuration Verified for Refresh Method

The AuthGuard is correctly configured to use the "refresh" strategy in the refresh method. The implementation of JwtRefreshStrategy confirms that the necessary setup is in place to protect the endpoint.

  • backend/src/auth/jwt-refresh.strategy.ts: Implementation of JwtRefreshStrategy for the "refresh" strategy.
  • backend/src/auth/auth.controller.ts: Usage of AuthGuard("refresh") in the refresh method.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify `AuthGuard` configuration for the `refresh` method.

# Test: Search for `AuthGuard` configuration. Expect: Proper guard settings.
rg --type python 'AuthGuard' backend/src/auth/auth.controller.ts

Length of output: 95


Script:

#!/bin/bash
# Description: Verify `AuthGuard` configuration for the `refresh` method.

# Test: Search for `AuthGuard` configuration. Expect: Proper guard settings.
rg 'AuthGuard' backend/src/auth/auth.controller.ts

Length of output: 163


Script:

#!/bin/bash
# Description: Verify the implementation of the "refresh" strategy used by AuthGuard.

# Test: Search for the implementation or configuration of the "refresh" strategy.
rg 'refresh' --type typescript

Length of output: 65


Script:

#!/bin/bash
# Description: Verify the implementation of the "refresh" strategy used by AuthGuard.

# Test: Search for the implementation or configuration of the "refresh" strategy.
rg 'refresh' --glob '*.ts'

Length of output: 2320

frontend/src/hooks/api/user.ts (2)

5-5: Approved: Enhanced import for refresh token management.

The addition of setRefreshToken to the imports supports the new functionality of managing refresh tokens, aligning with the PR's objectives to improve authentication flows.


35-35: Approved: Robust error handling in useGetUserQuery.

The inclusion of setRefreshToken(null) ensures that refresh tokens are cleared alongside access tokens and user data upon encountering a query error, enhancing the security and consistency of the state management.

backend/src/auth/auth.service.spec.ts (3)

1-1: Approved: Enhanced configuration handling with ConfigModule.

The import of ConfigModule supports a more modular and manageable configuration setup, which is beneficial for maintaining scalable and clean code.


13-31: Approved: Comprehensive mocking for dependency isolation in tests.

The introduction of mocks for UsersService and JwtService allows for more controlled and isolated testing of the AuthService, ensuring that the tests are not dependent on actual implementations and can focus on behavior verification.


42-63: Approved: Thorough testing for new access token generation.

The tests for getNewAccessToken effectively verify both the successful token generation and error handling scenarios, ensuring the AuthService behaves as expected under various conditions.

frontend/src/components/popovers/ProfilePopover.tsx (2)

16-16: Approved: Enhanced import for refresh token management in logout process.

The addition of setRefreshToken to the imports supports the updated logout functionality, ensuring that refresh tokens are properly managed during user logout.


27-27: Approved: Enhanced security in logout functionality.

The update to handleLogout to include setRefreshToken(null) ensures that all session tokens are cleared upon logout, enhancing the security and consistency of the application's state management.

backend/.env.development (1)

19-22: Review of new JWT configuration variables.

The renaming of JWT_AUTH_SECRET to JWT_ACCESS_TOKEN_SECRET and the addition of JWT_ACCESS_TOKEN_EXPIRATION_TIME, JWT_REFRESH_TOKEN_SECRET, and JWT_REFRESH_TOKEN_EXPIRATION_TIME are correctly implemented and align with the PR's objectives to enhance token management.

frontend/src/App.tsx (1)

102-141: Review of Axios interceptor implementation for token management.

The implementation of the Axios interceptor to handle 401 errors and refresh tokens is well-done. It correctly identifies when to attempt a token refresh and when to clear tokens and user data. This is crucial for maintaining user sessions and enhancing security.

However, ensure that the interceptor is properly cleaned up when the component unmounts to prevent memory leaks or unintended behavior.

backend/src/auth/types/refresh-token-request.type.ts Outdated Show resolved Hide resolved
backend/src/auth/types/login-response.type.ts Show resolved Hide resolved
backend/.env.development Outdated Show resolved Hide resolved
@xet-a
Copy link
Contributor Author

xet-a commented Aug 25, 2024

I think there are still a few tasks left.
It works, but there is a problem that the error appears first and is processed.

image
First, I get a 401 error as shown in the picture when I first access. It's automatically redirected when the refresh token expires. I think it's the same as the second question, is there any other reason?

image
const res = await axios.get<GetUserResponse>("/users");
Second, when the access token expires, the 401 error comes out first, as shown in the photo, which seems to be caused by the way React Query works and the order of execution of the axios interceptor. React Query basically tries to detect and process the error immediately, which is likely to happen before the interceptor works.

Can you let me know if there is a good solution?

@devleejb
Copy link
Member

@xet-a

First, I get a 401 error as shown in the picture when I first access. It's automatically redirected when the refresh token expires. I think it's the same as the second question, is there any other reason?

I this it is because of #193.
After resolving the issue, I will merge it!

Copy link
Member

@devleejb devleejb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems to be well implemented overall. I think it just needs a few details to be adjusted.

backend/.env.development Outdated Show resolved Hide resolved
backend/src/auth/auth.controller.ts Outdated Show resolved Hide resolved
backend/src/auth/auth.controller.ts Outdated Show resolved Hide resolved
backend/src/auth/types/refresh-token-request.type.ts Outdated Show resolved Hide resolved
backend/src/auth/types/refresh-token-request.type.ts Outdated Show resolved Hide resolved
backend/src/auth/auth.service.ts Outdated Show resolved Hide resolved
backend/src/auth/auth.service.ts Outdated Show resolved Hide resolved
backend/src/auth/types/login-response.type.ts Show resolved Hide resolved
frontend/src/App.tsx Show resolved Hide resolved
frontend/src/store/authSlice.ts Show resolved Hide resolved
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between fbecc95 and e8af8f9.

Files selected for processing (1)
  • backend/.env.development (1 hunks)
Files skipped from review as they are similar to previous changes (1)
  • backend/.env.development

@xet-a xet-a requested a review from devleejb September 8, 2024 15:11
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between e8af8f9 and 587e58d.

Files selected for processing (10)
  • backend/src/auth/auth.controller.ts (2 hunks)
  • backend/src/auth/auth.service.ts (1 hunks)
  • backend/src/auth/dto/refresh-token-request.dto.ts (1 hunks)
  • backend/src/auth/dto/refresh-token-response.dto.ts (1 hunks)
  • backend/src/auth/types/login-response.type.ts (1 hunks)
  • backend/src/users/users.module.ts (1 hunks)
  • frontend/src/App.tsx (4 hunks)
  • frontend/src/components/popovers/ProfilePopover.tsx (2 hunks)
  • frontend/src/hooks/api/user.ts (2 hunks)
  • frontend/src/store/authSlice.ts (2 hunks)
Files skipped from review due to trivial changes (3)
  • backend/src/auth/dto/refresh-token-request.dto.ts
  • backend/src/auth/dto/refresh-token-response.dto.ts
  • backend/src/users/users.module.ts
Additional comments not posted (12)
backend/src/auth/types/login-response.type.ts (1)

7-8: Properly documented refreshToken.

The addition of the refreshToken property with the @ApiProperty annotation is correctly implemented and aligns with the PR objectives to enhance token management capabilities.

frontend/src/store/authSlice.ts (3)

8-8: Enhanced state management with refreshToken.

The addition of the refreshToken property in the AuthState interface and its initialization in initialState are well-implemented, enhancing the state management for authentication.

Also applies to: 13-13


23-30: Well-implemented token management reducers.

The new reducers setRefreshToken and logout are correctly implemented. The logout function effectively resets both tokens and clears the Authorization header, aligning with best practices for secure token management.


34-34: Correct export of slice actions.

The export statement for setAccessToken, setRefreshToken, and logout is correctly implemented, ensuring that these actions are available for use throughout the application.

frontend/src/hooks/api/user.ts (2)

4-5: Updated imports and added logout function.

The adjustments to the import statements and the addition of the logout function from authSlice are correctly implemented, enhancing the functionality related to user authentication and state management.


34-34: Enhanced error handling in useGetUserQuery.

The modification to use the logout function in the error handling section of useGetUserQuery is a significant improvement. This change ensures that the user is logged out, clearing session data and enhancing security when a query fails.

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

11-15: Constructor updated to include essential services.

The inclusion of JwtService and ConfigService in the constructor is a positive change, enhancing the AuthService's capability to manage JWTs and configurations effectively.


17-40: Ensure secure handling of JWT secrets.

The method loginWithSocialProvider correctly uses different secrets for access and refresh tokens, addressing previous concerns about using the same JWT secret. It's recommended to perform a security audit to ensure that the secrets are stored and handled securely.

frontend/src/components/popovers/ProfilePopover.tsx (1)

26-26: Updated logout functionality.

The handleLogout function now dispatches the logout() action, which likely encapsulates the logout process more comprehensively. Ensure that this new action is integrated correctly with the rest of the application.

Verification successful

Logout action is correctly integrated.

The logout action is defined and exported from authSlice and is imported in multiple files, including ProfilePopover.tsx, App.tsx, and user.ts. This confirms its proper integration within the application.

Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the integration of the `logout()` action.

# Test: Search for the usage of the `logout()` action in the application. Expect: Proper integration and handling.
rg --type typescript 'dispatch\\(logout\\(\\)\\)'

Length of output: 86


Script:

#!/bin/bash
# Description: Verify the integration of the `logout()` action by searching for its usage in TypeScript files.

# Find all TypeScript files and search for the usage of `dispatch(logout())`.
fd --extension ts --exec rg 'dispatch\(logout\(\)\)' {}

Length of output: 82


Script:

#!/bin/bash
# Description: Search for the definition and imports of the `logout` action.

# Search for the definition of `logout`.
rg 'function logout' --type ts

# Search for imports of `logout`.
rg 'import.*logout' --type ts

Length of output: 336


Script:

#!/bin/bash
# Description: Search for the definition of the `logout` action within the `authSlice`.

# Find the `authSlice` file and search for the `logout` action definition.
fd 'authSlice' --extension ts --exec rg 'logout' {}

Length of output: 152

backend/src/auth/auth.controller.ts (1)

25-26: Constructor streamlined with essential services.

The inclusion of AuthService and ConfigService in the constructor is a positive change, centralizing the authentication logic and reducing dependency complexity.

frontend/src/App.tsx (2)

6-10: Approved new imports for enhanced functionality.

The addition of Sentry, React Query, and Axios supports the new authentication handling and error tracking features.


Line range hint 64-138: Review of new useEffect and Axios interceptor logic.

The useEffect hook adds an Axios response interceptor to handle 401 errors related to token expiration:

  • The interceptor first checks if the error is from the /auth/refresh endpoint to avoid retry loops.
  • If not, it sets a retry flag and attempts to refresh the token using the stored refresh token.
  • If the refresh is successful, it updates the access token in both the Redux store and Axios headers and retries the failed request.
  • If the refresh fails, it triggers a logout and clears user data.

Considerations:

  • Ensure that the retry logic is robust and avoids potential infinite loops.
  • Verify that all paths (success, failure, retry) are correctly handled and tested.

This implementation enhances the application's resilience against session expiration and automates token management, aligning with the PR's objectives.

Verification successful

Retry Logic in Axios Interceptor is Robust

The Axios interceptor logic in frontend/src/App.tsx effectively handles token expiration by using a _retry flag to prevent infinite loops and ensuring that all paths (success, failure, retry) are correctly managed. This implementation aligns with best practices for handling authentication token refresh scenarios.

  • The _retry flag prevents repeated refresh attempts for the same request.
  • The interceptor correctly handles both the refresh success and failure paths.
Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the robustness of the retry logic in the Axios interceptor.

# Test: Search for potential infinite loops or unhandled paths in the interceptor logic.
rg --type typescript -A 5 $'axios.interceptors.response.use'

Length of output: 94


Script:

#!/bin/bash
# Description: Verify the robustness of the retry logic in the Axios interceptor.

# Test: Search for potential infinite loops or unhandled paths in the interceptor logic.
rg --type ts -A 10 $'axios.interceptors.response.use'

Length of output: 740

backend/src/auth/auth.service.ts Outdated Show resolved Hide resolved
backend/src/auth/auth.controller.ts Show resolved Hide resolved
Copy link
Member

@devleejb devleejb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, I recommend to create two JwtService objects like below code.
(Maybe, there are better approaches.)

auth.module.ts

@Module({
	imports: [UsersModule],
	providers: [
		AuthService,
		GithubStrategy,
		JwtStrategy,
		JwtRefreshStrategy,
		{
			provide: "JWT_ACCESS_SERVICE",
			useFactory: (configService: ConfigService) => {
				return new JwtService({
					secret: configService.get<string>("JWT_ACCESS_TOKEN_SECRET"),
					signOptions: {
						expiresIn: `${configService.get("JWT_ACCESS_TOKEN_EXPIRATION_TIME")}s`,
					},
				});
			},
			inject: [ConfigService],
		},
		{
			provide: "JWT_REFRESH_SERVICE",
			useFactory: (configService: ConfigService) => {
				return new JwtService({
					secret: configService.get<string>("JWT_REFRESH_TOKEN_SECRET"),
					signOptions: {
						expiresIn: `${configService.get("JWT_REFRESH_TOKEN_EXPIRATION_TIME")}s`,
					},
				});
			},
			inject: [ConfigService],
		},
	],
	exports: ["JWT_ACCESS_SERVICE", "JWT_REFRESH_SERVICE"],
	controllers: [AuthController],
})
export class AuthModule {}

auth.service.ts

constructor(
	private readonly usersService: UsersService,
	private readonly configService: ConfigService,
	@Inject("JWT_ACCESS_SERVICE") private readonly jwtAccessService: JwtService,
	@Inject("JWT_REFRESH_SERVICE") private readonly jwtRefreshService: JwtService
) {
        // Check whether the tokens are intended value.
	console.log("JWT_ACCESS_SERVICE", jwtAccessService);
	console.log("JWT_REFRESH_SERVICE", jwtRefreshService);
}

Also, you can create the object name constants :)
For example, @Inject(JwtInject.ACCESS).

If you have any questions, feel free to ask.

backend/src/auth/auth.service.ts Outdated Show resolved Hide resolved
backend/.env.development Outdated Show resolved Hide resolved
@xet-a xet-a requested a review from devleejb September 10, 2024 11:25
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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, codebase verification and nitpick comments (1)
backend/src/auth/auth.service.ts (1)

29-38: LGTM, but consider adding error handling.

The method getNewAccessToken correctly verifies the refresh token and uses its payload to generate a new access token. Consider adding error handling for cases where the refresh token is invalid or expired to enhance security and user experience.

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 587e58d and 9bc0506.

Files selected for processing (4)
  • backend/.env.development (1 hunks)
  • backend/src/auth/auth.module.ts (1 hunks)
  • backend/src/auth/auth.service.ts (1 hunks)
  • backend/src/utils/constants/jwt-inject.ts (1 hunks)
Files skipped from review due to trivial changes (1)
  • backend/src/utils/constants/jwt-inject.ts
Additional comments not posted (8)
backend/src/auth/auth.service.ts (2)

11-15: LGTM!

The changes to the constructor to inject separate JwtService instances for access and refresh tokens using the JwtInject constants are approved. This allows for better security and token management.


17-27: LGTM!

The loginWithSocialProvider method correctly handles social login by retrieving or creating the user and generating the necessary access and refresh tokens.

backend/src/auth/auth.module.ts (3)

19-30: LGTM!

The JwtInject.ACCESS provider correctly sets up a separate JwtService instance for access tokens with its own secret and expiration settings retrieved from the ConfigService. This allows for better control and security.


31-42: LGTM!

The JwtInject.REFRESH provider correctly sets up a separate JwtService instance for refresh tokens with its own secret and expiration settings retrieved from the ConfigService. This allows for better control and security.


44-44: LGTM!

Exporting the JwtInject.ACCESS and JwtInject.REFRESH providers in the exports array allows them to be used in other modules, which is necessary for the AuthService to function correctly.

backend/.env.development (3)

23-24: LGTM!

The refresh token secret and expiration time are correctly set. The expiration time of 1 week (604800 seconds) is reasonable for refresh tokens.


17-24: LGTM!

The removal of the JWT_AUTH_SECRET variable is correct as it has been replaced with separate secrets for access and refresh tokens (JWT_ACCESS_TOKEN_SECRET and JWT_REFRESH_TOKEN_SECRET).


17-18: LGTM!

The comments added for the JWT_ACCESS_TOKEN_SECRET, JWT_ACCESS_TOKEN_EXPIRATION_TIME, JWT_REFRESH_TOKEN_SECRET, and JWT_REFRESH_TOKEN_EXPIRATION_TIME variables provide clear explanations of their purpose and usage. They serve as helpful documentation for developers.

Also applies to: 21-22

frontend/src/App.tsx Outdated Show resolved Hide resolved
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 30c9d07 and 82752c9.

Files selected for processing (4)
  • backend/.env.development (1 hunks)
  • frontend/src/App.tsx (4 hunks)
  • frontend/src/components/popovers/ProfilePopover.tsx (2 hunks)
  • frontend/src/hooks/api/user.ts (2 hunks)
Files skipped from review as they are similar to previous changes (3)
  • backend/.env.development
  • frontend/src/components/popovers/ProfilePopover.tsx
  • frontend/src/hooks/api/user.ts
Additional comments not posted (1)
frontend/src/App.tsx (1)

64-64: LGTM!

The code changes are approved.

frontend/src/App.tsx Show resolved Hide resolved
Copy link
Member

@devleejb devleejb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your contribution.

@devleejb devleejb merged commit 5d5ed73 into main Sep 11, 2024
3 checks passed
@devleejb devleejb deleted the introduce-refresh-token branch September 11, 2024 14:45
@coderabbitai coderabbitai bot mentioned this pull request Nov 2, 2024
2 tasks
minai621 pushed a commit that referenced this pull request Nov 5, 2024
* Implement refresh token in backend

* Implement token refresh with axios interceptor

* Fix typo

* Apply lint

* Add Refresh token initialization

* Update Axios interceptor to handle token expiration and refresh logic

* Update environment variable comments for clarity

* Separate dtos related to refresh token

* Add `@ApiProperty` decorator

* Update `@ApiBody` and `@ApiResponse` type

* Separate JWT secrets for access and refresh tokens

* Add logout logic and remove duplicate call

* Update env variable comments for clarity

* Split JwtService for access and refresh tokens

* Fix interceptor logic for refresh token

* Add error config token
@coderabbitai coderabbitai bot mentioned this pull request Nov 8, 2024
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement 🌟 New feature or request
Projects
Status: Done
Status: Backlog
Development

Successfully merging this pull request may close these issues.

Introduce Refresh Token for authentication
3 participants