Repository navigation
refactor(Layout): update authentication state on navigation - #8505
Conversation
Reviewer's GuideRefactors Layout navigation authorization to keep rendered content synchronized with the latest authorization result, including recovery to authorized content, non-redirect unauthorized rendering, and safe handling of missing or removed callbacks; adds focused bUnit coverage for these scenarios. Sequence diagram for Layout authorization on navigationsequenceDiagram
participant Navigation
participant Layout
participant OnAuthorizing
participant UI
Navigation->>Layout: LocationChanged(location)
alt OnAuthorizing is null
Layout-->>Layout: return
else callback exists
Layout->>OnAuthorizing: OnAuthorizing(location)
OnAuthorizing-->>Layout: auth
alt auth is true
opt _authenticated is false
Layout->>Layout: StateHasChanged()
Layout->>UI: Render authorized content
end
else IsAutoNavigateWhenNotAuthorize is true
Layout->>Navigation: NavigateTo(NotAuthorizeUrl, true)
else _authenticated is true
Layout->>Layout: StateHasChanged()
Layout->>UI: Render NotAuthorized template
end
end
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 3 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="src/BootstrapBlazor/Components/Layout/Layout.razor.cs" line_range="739-744" />
<code_context>
+ InvokeAsync(async () =>
{
- InvokeAsync(async () =>
+ var auth = await OnAuthorizing(e.Location);
+ if (auth)
{
- var auth = await OnAuthorizing(e.Location);
- if (!auth && IsAutoNavigateWhenNotAuthorize)
</code_context>
<issue_to_address>
**Protected routes render as authorized**
When onAuthorizing returns true for a route whose framework authorization check denies the user, `Navigation_LocationChanged` sets `_authenticated` from `OnAuthorizing` alone, bypassing the route handler's `IsAuthorizedAsync` result, so the layout renders `Main` for a route the user cannot access.
Combine the callback result with the destination route's framework authorization result before setting `_authenticated`.
Also at `src/BootstrapBlazor/Components/Layout/Layout.razor.cs:745`.
</issue_to_address>
### Comment 2
<location path="src/BootstrapBlazor/Components/Layout/Layout.razor.cs" line_range="737" />
<code_context>
+ }
- if (OnAuthorizing != null)
+ InvokeAsync(async () =>
{
- InvokeAsync(async () =>
</code_context>
<issue_to_address>
**Stale checks overwrite current authorization**
When multiple navigations overlap and an earlier `OnAuthorizing` call completes after a later one, `Navigation_LocationChanged` applies each result to the shared `_authenticated` state and may redirect based on the stale location, so the layout shows the wrong content for the current page.
Before applying a result or redirecting, verify that its location is still current.
Also at `src/BootstrapBlazor/Components/Layout/Layout.razor.cs:739-758`.
</issue_to_address>
### Comment 3
<location path="src/BootstrapBlazor/Components/Layout/Layout.razor.cs" line_range="739" />
<code_context>
+ InvokeAsync(async () =>
{
- InvokeAsync(async () =>
+ var auth = await OnAuthorizing(e.Location);
+ if (auth)
{
</code_context>
<issue_to_address>
**Failed checks retain access**
When the authorization callback hangs or throws after the layout was previously authorized, `_authenticated` is not cleared before `OnAuthorizing` is awaited, and the dispatched task has no failure handling. If the callback stalls or throws during a dependency failure, the previous authorized state remains active and the layout continues rendering protected content.
Fail closed while authorization is pending or fails, and handle callback exceptions and timeouts explicitly.
</issue_to_address>Sourcery assessment
Needs a human reviewer. 3 findings to address first, and the navigation callback now controls whether the layout renders its main content or the unauthorized template, so an incorrect or stale authorization result could expose protected UI or redirect users incorrectly. Reverting prevents the new behavior, but any access or content exposure that occurred while it was active cannot be undone.
Blocking findings: src/BootstrapBlazor/Components/Layout/Layout.razor.cs:744, src/BootstrapBlazor/Components/Layout/Layout.razor.cs:737, src/BootstrapBlazor/Components/Layout/Layout.razor.cs:739
| var auth = await OnAuthorizing(e.Location); | ||
| if (auth) | ||
| { | ||
| var auth = await OnAuthorizing(e.Location); | ||
| if (!auth && IsAutoNavigateWhenNotAuthorize) | ||
| // 当前地址已授权时恢复 UI 状态 | ||
| if (!_authenticated) | ||
| { |
There was a problem hiding this comment.
🔴 Critical · Protected routes render as authorized
When onAuthorizing returns true for a route whose framework authorization check denies the user, Navigation_LocationChanged sets _authenticated from OnAuthorizing alone, bypassing the route handler's IsAuthorizedAsync result, so the layout renders Main for a route the user cannot access.
Combine the callback result with the destination route's framework authorization result before setting _authenticated.
Also at src/BootstrapBlazor/Components/Layout/Layout.razor.cs:745.
Prompt for AI agents
In `src/BootstrapBlazor/Components/Layout/Layout.razor.cs` at lines 739-744:
**Protected routes render as authorized**
When onAuthorizing returns true for a route whose framework authorization check denies the user, `Navigation_LocationChanged` sets `_authenticated` from `OnAuthorizing` alone, bypassing the route handler's `IsAuthorizedAsync` result, so the layout renders `Main` for a route the user cannot access.
Combine the callback result with the destination route's framework authorization result before setting `_authenticated`.
Also at `src/BootstrapBlazor/Components/Layout/Layout.razor.cs:745`.| } | ||
|
|
||
| if (OnAuthorizing != null) | ||
| InvokeAsync(async () => |
There was a problem hiding this comment.
🔴 Critical · Stale checks overwrite current authorization
When multiple navigations overlap and an earlier OnAuthorizing call completes after a later one, Navigation_LocationChanged applies each result to the shared _authenticated state and may redirect based on the stale location, so the layout shows the wrong content for the current page.
Before applying a result or redirecting, verify that its location is still current.
Also at src/BootstrapBlazor/Components/Layout/Layout.razor.cs:739-758.
Prompt for AI agents
In `src/BootstrapBlazor/Components/Layout/Layout.razor.cs` at line 737:
**Stale checks overwrite current authorization**
When multiple navigations overlap and an earlier `OnAuthorizing` call completes after a later one, `Navigation_LocationChanged` applies each result to the shared `_authenticated` state and may redirect based on the stale location, so the layout shows the wrong content for the current page.
Before applying a result or redirecting, verify that its location is still current.
Also at `src/BootstrapBlazor/Components/Layout/Layout.razor.cs:739-758`.| InvokeAsync(async () => | ||
| { | ||
| InvokeAsync(async () => | ||
| var auth = await OnAuthorizing(e.Location); |
There was a problem hiding this comment.
🔴 Critical · Failed checks retain access
When the authorization callback hangs or throws after the layout was previously authorized, _authenticated is not cleared before OnAuthorizing is awaited, and the dispatched task has no failure handling. If the callback stalls or throws during a dependency failure, the previous authorized state remains active and the layout continues rendering protected content.
Fail closed while authorization is pending or fails, and handle callback exceptions and timeouts explicitly.
Prompt for AI agents
In `src/BootstrapBlazor/Components/Layout/Layout.razor.cs` at line 739:
**Failed checks retain access**
When the authorization callback hangs or throws after the layout was previously authorized, `_authenticated` is not cleared before `OnAuthorizing` is awaited, and the dispatched task has no failure handling. If the callback stalls or throws during a dependency failure, the previous authorized state remains active and the layout continues rendering protected content.
Fail closed while authorization is pending or fails, and handle callback exceptions and timeouts explicitly.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8505 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 777 777
Lines 35160 35175 +15
=========================================
+ Hits 35160 35175 +15
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Link issues
fixes #8504
Summary By Copilot
Regression?
Risk
Verification
Packaging changes reviewed?
☑️ Self Check before Merge
Summary by Sourcery
Keep layout authentication state synchronized with navigation and render the correct authorized or unauthorized content.
Bug Fixes:
Enhancements:
Tests: