Improve bit Boilerplate sign-in process (#11113) #11114
Conversation
WalkthroughThe changes update the handling of return URLs in the sign-in flow, introduce a new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant SignInModal
participant SignInPanel
participant AuthManager
participant Server
User->>SignInModal: Opens sign-in modal
SignInModal->>SignInPanel: Passes ReturnUrl parameter
User->>SignInPanel: Submits credentials
SignInPanel->>AuthManager: Calls sign-in with credentials and GetReturnUrl()
AuthManager->>Server: Authenticates user
Server-->>AuthManager: Returns tokens
AuthManager->>AuthManager: Stores tokens (sets cookie with domain)
AuthManager-->>SignInPanel: Sign-in result
SignInPanel->>User: Navigates to GetReturnUrl()
Assessment against linked issues
Poem
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed 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)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Pull Request Overview
This PR enhances the sign-in flow by properly setting cookie domains and unifying return URL logic across components.
- Injects
AbsoluteServerAddressProviderintoAuthManager(and diagnostic modal utils) to set cookieDomaindynamically - Introduces a
GetReturnUrlhelper and adds a newReturnUrlparameter toSignInPanel, replacing direct use of query-string-only logic - Updates
SignInModaland Razor markup to pass and use the newReturnUrlparameter
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
AuthManager.cs |
Injects and uses AbsoluteServerAddressProvider for cookie domain |
SignInPanel.razor.cs |
Adds ReturnUrl param and GetReturnUrl method, updates calls |
SignInPanel.razor |
Switches link Hrefs to use GetReturnUrl() |
SignInModal.razor |
Passes new ReturnUrl parameter instead of query-string property |
AppDiagnosticModal.razor.Utils.cs |
Applies same cookie-domain injection pattern |
Comments suppressed due to low confidence (4)
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Services/AuthManager.cs:23
- [nitpick] The field name
absoluteServerAddressis a provider; consider renaming it toabsoluteServerAddressProviderfor clarity and consistency with other injected services.
[AutoInject] private AbsoluteServerAddressProvider absoluteServerAddress = default!;
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Services/AuthManager.cs:18
- The
telemetryContextinjection has been removed; verify thatITelemetryContextis no longer used in this class or restore the injection to avoid runtime errors or missing telemetry.
[AutoInject] private IAuthTokenProvider tokenProvider = default!;
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Identity/SignIn/SignInPanel.razor.cs:21
- [nitpick] Add an XML doc comment above
GetReturnUrlto explain its fallback order (parameter → query string → home page) for future maintainers.
private string GetReturnUrl() => ReturnUrl ?? ReturnUrlQueryString ?? Urls.HomePage;
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Identity/SignIn/SignInPanel.razor.cs:21
- [nitpick] Consider adding unit tests for
GetReturnUrlto cover scenarios whenReturnUrl,ReturnUrlQueryString, or neither is set, ensuring fallback logic behaves as expected.
private string GetReturnUrl() => ReturnUrl ?? ReturnUrlQueryString ?? Urls.HomePage;
...lient/Boilerplate.Client.Core/Components/Layout/Diagnostic/AppDiagnosticModal.razor.Utils.cs
Show resolved
Hide resolved
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Knowledge Base: Disabled due to Reviews > Disable Knowledge Base setting
📒 Files selected for processing (5)
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Layout/Diagnostic/AppDiagnosticModal.razor.Utils.cs(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Identity/SignIn/SignInModal.razor(1 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Identity/SignIn/SignInPanel.razor(2 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Identity/SignIn/SignInPanel.razor.cs(5 hunks)src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Services/AuthManager.cs(3 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Services/AuthManager.cs (1)
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Services/AbsoluteServerAddressProvider.cs (1)
AbsoluteServerAddressProvider(10-28)
🔇 Additional comments (12)
src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Identity/SignIn/SignInModal.razor (1)
14-14: LGTM! Parameter name updated to align with refactoring.The parameter name change from
ReturnUrlQueryStringtoReturnUrlis consistent with the new parameter introduced inSignInPaneland maintains the same functionality.src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Identity/SignIn/SignInPanel.razor (2)
74-74: LGTM! Centralized return URL handling for forgot password link.The change to use
GetReturnUrl()method instead of direct property access provides consistent return URL handling with proper fallback logic.
144-144: LGTM! Centralized return URL handling for sign up link.The change to use
GetReturnUrl()method instead of direct property access ensures consistent return URL handling throughout the component.src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Components/Pages/Identity/SignIn/SignInPanel.razor.cs (6)
21-21: LGTM! Well-designed centralized return URL logic.The
GetReturnUrl()method provides a clean fallback hierarchy: parameter → query string → default home page. This centralizes the logic and makes it easy to maintain.
23-24: LGTM! New parameter enhances flexibility.The new
ReturnUrlparameter allows for more flexible return URL handling, enabling both programmatic and query string-based return URL specification.
144-144: LGTM! Consistent return URL handling in sign-in flow.Using
GetReturnUrl()ensures the model gets the correct return URL based on the centralized logic.
188-188: LGTM! Consistent navigation after successful sign-in.Using
GetReturnUrl()for navigation ensures users are redirected to the correct location after successful authentication.
248-248: LGTM! Consistent return URL for social sign-in.Using
GetReturnUrl()ensures social sign-in redirects follow the same return URL logic as other authentication methods.
324-324: LGTM! Consistent return URL for OTP flow.Using
GetReturnUrl()ensures the OTP flow maintains consistent return URL handling throughout the authentication process.src/Templates/Boilerplate/Bit.Boilerplate/src/Client/Boilerplate.Client.Core/Services/AuthManager.cs (3)
23-23: LGTM: Proper dependency injection.The addition of
AbsoluteServerAddressProviderinjection is correctly implemented and will be used to ensure consistent cookie domain handling.
64-64: LGTM: Consistent cookie domain setting.Explicitly setting the cookie domain when storing the access token ensures proper cookie scoping and security. The implementation correctly uses the server host from the injected provider.
227-227: LGTM: Consistent cookie domain setting for removal.The cookie removal operation now properly specifies the domain, ensuring the cookie is removed from the correct domain. This mirrors the domain setting used when storing the cookie.
...lient/Boilerplate.Client.Core/Components/Layout/Diagnostic/AppDiagnosticModal.razor.Utils.cs
Show resolved
Hide resolved
1 participant
closes #11113
Summary by CodeRabbit
Bug Fixes
Refactor