fix: where pages could not be refreshed when login expired #4204
Conversation
|
WalkthroughThe recent updates enhance the authentication and access management system across multiple components. Key changes include extending the validity of access tokens from 2 hours to 1 day, refining the logic that determines login expiration based on access checks, and introducing a new state to track whether access permissions have been verified. These adjustments streamline user session management and improve overall system robustness. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant AuthStore
participant AccessStore
User->>AuthStore: Login attempt
AuthStore-->>User: Returns access token
User->>AccessStore: Check access state
AccessStore-->>User: Access check completed
User->>AuthStore: Refresh token if needed
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 as PR comments)
Additionally, you can add 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: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (12)
- apps/backend-mock/utils/jwt-utils.ts (1 hunks)
- apps/web-antd/src/api/request.ts (1 hunks)
- apps/web-antd/src/router/guard.ts (2 hunks)
- apps/web-ele/src/api/request.ts (1 hunks)
- apps/web-ele/src/router/guard.ts (2 hunks)
- apps/web-naive/src/api/request.ts (1 hunks)
- apps/web-naive/src/router/guard.ts (2 hunks)
- docs/src/guide/essentials/server.md (1 hunks)
- packages/@core/preferences/src/config.ts (1 hunks)
- packages/stores/src/modules/access.ts (3 hunks)
- playground/src/api/request.ts (1 hunks)
- playground/src/router/guard.ts (3 hunks)
Additional comments not posted (17)
apps/backend-mock/utils/jwt-utils.ts (1)
17-17: Consider the security implications of extending token expiration.The access token expiration time has been extended from 2 hours to 1 day. While this change improves user convenience by reducing the frequency of re-authentication, it may also increase security risks if tokens are compromised. Ensure that this change aligns with your security policies and consider implementing additional security measures, such as token revocation or monitoring for unusual activity.
packages/stores/src/modules/access.ts (3)
28-28: LGTM! EnsureisAccessCheckedis utilized correctly.The addition of
isAccessCheckedto track access verification status is a good enhancement. Make sure that this property is correctly used throughout the application to control access-related logic.
56-58: LGTM! Verify the usage ofsetIsAccessChecked.The
setIsAccessCheckedmethod is a useful addition for updating the access check status. Ensure that it is invoked appropriately in the application flow to maintain accurate state management.
75-75: LGTM! Default state initialization is appropriate.The default initialization of
isAccessCheckedtofalseensures that access permissions are not assumed to be verified upon store initialization. This is a good practice for ensuring security and correctness in access management.packages/@core/preferences/src/config.ts (1)
21-21: Consider the impact on user experience when changingloginExpiredMode.The
loginExpiredModehas been changed from'modal'to'page'. This shift to redirecting users to a dedicated page can provide a clearer user experience but may also disrupt the user's workflow. Ensure that this change aligns with the overall user experience strategy and consider user feedback on this interaction.apps/web-naive/src/api/request.ts (1)
33-36: LGTM! Verify the logic for settingloginExpired.The addition of the
isAccessCheckedcondition ensures thatloginExpiredis only set when appropriate. This change can help prevent premature session expiration handling. Verify that this logic is correctly integrated into the application flow.apps/web-antd/src/api/request.ts (1)
34-37: LGTM! Verify the logic for settingloginExpired.The updated condition for setting
loginExpiredenhances the robustness of session management. Ensure that this logic is correctly applied and tested within the application to avoid unintended session handling.playground/src/api/request.ts (1)
34-37: Approved: Refined condition for setting login expired state.The changes ensure that the login expired state is only set after access checks are completed, preventing premature state changes related to user authentication.
However, verify that the
accessStore.isAccessCheckedis correctly set in all relevant scenarios.apps/web-ele/src/api/request.ts (1)
34-37: Approved: Refined condition for setting login expired state.The changes ensure that the login expired state is only set after access checks are completed, preventing premature state changes related to user authentication.
However, verify that the
accessStore.isAccessCheckedis correctly set in all relevant scenarios.Verification successful
Verification Successful:
accessStore.isAccessCheckedis correctly set in relevant scenarios.The property
isAccessCheckedis actively used and set within the codebase, confirming that the condition in the review comment is valid. The code ensures thataccessStore.isAccessCheckedis managed appropriately.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that `accessStore.isAccessChecked` is correctly set in all relevant scenarios. # Test: Search for all occurrences of `setIsAccessChecked` to ensure it's set appropriately. rg --type javascript --word-regexp 'setIsAccessChecked'Length of output: 90
Script:
#!/bin/bash # Description: Verify that `accessStore.isAccessChecked` is correctly set in all relevant scenarios. # Find all JavaScript files and search for `setIsAccessChecked` within them. fd --extension js --exec rg --word-regexp 'setIsAccessChecked'Length of output: 62
Script:
#!/bin/bash # Description: Search for occurrences of `isAccessChecked` to verify its usage and setting. # Find all JavaScript files and search for `isAccessChecked` within them. fd --extension js --exec rg --word-regexp 'isAccessChecked'Length of output: 59
Script:
#!/bin/bash # Description: Search for occurrences of `accessStore` to understand its structure and usage. # Find all JavaScript files and search for `accessStore` within them. fd --extension js --exec rg --word-regexp 'accessStore'Length of output: 55
Script:
#!/bin/bash # Description: Search for occurrences of `accessStore` in both JavaScript and TypeScript files to understand its structure and usage. # Find all JavaScript and TypeScript files and search for `accessStore` within them. fd --extension js --extension ts --exec rg --word-regexp 'accessStore'Length of output: 4444
apps/web-naive/src/router/guard.ts (2)
115-115: Approved: Set access check flag after route generation.Setting
isAccessCheckedto true after generating access menus and routes ensures subsequent calls recognize completed access checks.
96-96: Approved: Streamlined access check logic.The changes replace the
accessRoutesvariable withisAccessChecked, improving clarity and efficiency in access management.However, verify that
accessStore.isAccessCheckedis correctly set after access checks are performed.Verification successful
Verified:
accessStore.isAccessCheckedis correctly set after access checks.The method
setIsAccessCheckedis used to update theisAccessCheckedproperty in several parts of the codebase, confirming its proper implementation.
- Calls to
setIsAccessCheckedfound in:
playground/src/router/guard.tsapps/web-naive/src/router/guard.tsapps/web-ele/src/router/guard.tsapps/web-antd/src/router/guard.tsScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that `accessStore.isAccessChecked` is correctly set after access checks. # Test: Search for all occurrences of `setIsAccessChecked` to ensure it's set appropriately. rg --type javascript --word-regexp 'setIsAccessChecked'Length of output: 90
Script:
#!/bin/bash # Description: Search for occurrences of `isAccessChecked` to verify where it is set in the codebase. rg 'isAccessChecked'Length of output: 949
Script:
#!/bin/bash # Description: Search for calls to `setIsAccessChecked` to verify its usage in the codebase. rg 'setIsAccessChecked'Length of output: 413
playground/src/router/guard.ts (2)
115-115: Approved: Set access check flag after route generation.Setting
isAccessCheckedto true after generating access menus and routes ensures subsequent calls recognize completed access checks.
95-95: Approved: Simplified access validation logic.The changes use
isAccessCheckedfor access validation, enhancing clarity and efficiency.However, verify that
accessStore.isAccessCheckedis correctly set after access checks are performed.Verification successful
Verification Successful:
accessStore.isAccessCheckedis correctly set.The
setIsAccessChecked(true)method is invoked inplayground/src/router/guard.ts, confirming that theisAccessCheckedproperty is set appropriately after access checks. No further action is needed.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that `accessStore.isAccessChecked` is correctly set after access checks. # Test: Search for all occurrences of `setIsAccessChecked` to ensure it's set appropriately. rg --type javascript --word-regexp 'setIsAccessChecked'Length of output: 90
Script:
#!/bin/bash # Adjust the search to find occurrences of `setIsAccessChecked` without specifying a file type. rg --word-regexp 'setIsAccessChecked'Length of output: 427
apps/web-antd/src/router/guard.ts (2)
116-116: Approved: Set access check flag after route generation.Setting
isAccessCheckedto true after generating access menus and routes ensures subsequent calls recognize completed access checks.
96-96: Approved: Simplified access validation logic.The changes use
isAccessCheckedfor access validation, enhancing clarity and efficiency.However, verify that
accessStore.isAccessCheckedis correctly set after access checks are performed.Verification successful
Access Check Confirmation:
isAccessCheckedis Set CorrectlyThe
isAccessCheckedflag is set usingsetIsAccessChecked(true)inapps/web-antd/src/router/guard.ts, confirming that it is correctly updated after access checks. This ensures that the access validation logic functions as intended.
setIsAccessCheckedis defined inpackages/stores/src/modules/access.ts.- It is used in
apps/web-antd/src/router/guard.tsand other files, indicating consistent application.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that `accessStore.isAccessChecked` is correctly set after access checks. # Test: Search for all occurrences of `setIsAccessChecked` to ensure it's set appropriately. rg --type javascript --word-regexp 'setIsAccessChecked'Length of output: 90
Script:
#!/bin/bash # Description: Verify that `accessStore.isAccessChecked` is correctly set after access checks. # Test: Search for all occurrences of `setIsAccessChecked` to ensure it's set appropriately. rg 'setIsAccessChecked'Length of output: 413
apps/web-ele/src/router/guard.ts (1)
Line range hint
96-116: LGTM! VerifyisAccessCheckedusage throughout the codebase.The change to use
isAccessCheckedsimplifies the logic and is a good improvement. Ensure that this property is correctly used and updated in all relevant parts of the codebase.Verification successful
Usage of
isAccessCheckedis correct and consistent across the codebase.The
setIsAccessCheckedmethod is appropriately called after setting routes and menus, ensuring the access state is updated correctly. No issues found with the current implementation.
apps/web-ele/src/router/guard.ts: Properly callssetIsAccessChecked(true)after setting routes and menus.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of `isAccessChecked` across the codebase. # Test: Search for all occurrences of `isAccessChecked`. Expect: Correct usage and updates. rg --type ts 'isAccessChecked'Length of output: 889
Script:
#!/bin/bash # Description: Verify if `setIsAccessChecked` is called after setting routes and menus. # Test: Search for occurrences of `setIsAccessChecked` in the codebase. rg --type ts 'setIsAccessChecked'Length of output: 423
docs/src/guide/essentials/server.md (1)
196-199: LGTM! Verify the conditions for login expiration logic.The addition of
accessStore.isAccessCheckedto the login expiration logic is a sensible improvement. Ensure that this condition aligns with the intended behavior across the application.Verification successful
Verification Successful: Consistent Use of
isAccessCheckedandloginExpiredModeThe conditions for login expiration logic, involving
accessStore.isAccessCheckedandpreferences.app.loginExpiredMode, are consistently applied across different modules and applications. This ensures that access checks are completed before any login expiration logic is executed, aligning with the intended behavior.
accessStore.isAccessCheckedis used in multiple locations to ensure access checks are completed.preferences.app.loginExpiredModeis consistently checked for the 'modal' mode.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the conditions used in login expiration logic. # Test: Search for all occurrences of `loginExpiredMode` and `isAccessChecked`. Expect: Correct usage and logic. rg --type ts 'loginExpiredMode|isAccessChecked'Length of output: 1406
Description
fixed #4188
Type of change
Please delete options that are not relevant.
pnpm-lock.yamlunless you introduce a new test example.Checklist
pnpm run docs:devcommand.pnpm test.feat:,fix:,perf:,docs:, orchore:.Summary by CodeRabbit
New Features
Enhancements
isAccessCheckedproperty to track access permissions more effectively.Bug Fixes