-
-
Notifications
You must be signed in to change notification settings - Fork 363
fix(Layout): authorization failure in virtual directory mode #6086
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
Conversation
Co-Authored-By: kid5791 <[email protected]>
Co-Authored-By: dejan <[email protected]>
# Conflicts: # src/BootstrapBlazor/BootstrapBlazor.csproj
Reviewer's GuideThis PR introduces a new extension to normalize navigation paths by removing query strings and fragments, updates the Layout component to use it for authorization in virtual directory scenarios, and adds unit tests to validate the new logic. Sequence Diagram: Layout Component Authorization Flow with URL NormalizationsequenceDiagram
participant L as LayoutComponent
participant NM as NavigationManager
participant RTF as RouteTableFactory
participant AuthH as AuthorizationHandler
L->>NM: Call ToBaseRelativePathWithoutQueryAndFragment()
activate NM
NM-->>L: returns normalizedUrl
deactivate NM
L->>RTF: Create(AdditionalAssemblies, normalizedUrl)
activate RTF
RTF-->>L: returns routeContext
deactivate RTF
L->>AuthH: routeContext.Handler.IsAuthorizedAsync(...)
activate AuthH
AuthH-->>L: returns isAuthenticated
deactivate AuthH
Class Diagram: Navigation Path Normalization FeatureclassDiagram
class NavigationManagerExtensions {
+static ToBaseRelativePathWithoutQueryAndFragment(NavigationManager navigationManager) string
}
class Layout {
<<Blazor Component>>
#OnInitializedAsync() void
-NavigationManager Navigation
}
Layout ..> NavigationManagerExtensions : uses
Layout *-- NavigationManager : aggregation
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #6086 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 701 701
Lines 30956 30970 +14
Branches 4378 4380 +2
=========================================
+ Hits 30956 30970 +14 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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.
Hey @ArgoZhang - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟡 General issues: 1 issue found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
@sourcery-ai review |
|
@sourcery-ai review |
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.
Hey @ArgoZhang - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟡 Testing: 1 issue found
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
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.
Hey @ArgoZhang - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Link issues
fixes #6055
Summary By Copilot
Regression?
Risk
Verification
Packaging changes reviewed?
☑️ Self Check before Merge
Summary by Sourcery
Add a utility method to NavigationManager to strip query strings and fragments, update layout logic to use it for route matching, and add tests to fix authorization failures in virtual directory mode.
Bug Fixes:
Enhancements:
Tests: