feat(WebClientServer): add UserName property#7601
Conversation
Reviewer's GuideAdds propagation of the currently authenticated user’s name into ConnectionHub’s ClientInfo via AuthenticationStateProvider, and covers it with a unit test plus a new UserName property on ClientInfo. Sequence diagram for propagating authenticated user name into ClientInfosequenceDiagram
actor Browser
participant ConnectionHub
participant AuthenticationStateProvider
participant ConnectionService
Browser->>ConnectionHub: Register connection
ConnectionHub->>AuthenticationStateProvider: GetAuthenticationStateAsync()
AuthenticationStateProvider-->>ConnectionHub: AuthenticationState with User.Identity
ConnectionHub->>ConnectionHub: Build ClientInfo with Ip, City, Engine
ConnectionHub->>ConnectionHub: Set client.UserName if identity.IsAuthenticated
ConnectionHub->>ConnectionService: AddOrUpdate(client)
Updated class diagram for ConnectionHub and ClientInfoclassDiagram
class ConnectionHub {
-IOptions~BootstrapBlazorOptions~ BootstrapBlazorOptions
-AuthenticationStateProvider AuthenticationStateProvider
-IIpLocatorProvider _ipLocatorProvider
-ThrottleOptions _throttleOptions
}
class ClientInfo {
string Ip
string City
string Engine
string UserName
}
class ConnectionService {
+AddOrUpdate(client ClientInfo)
}
ConnectionHub --> ClientInfo : populates
ConnectionHub --> ConnectionService : uses
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 1 issue, and left some high level feedback:
- Injecting
AuthenticationStateProviderdirectly intoConnectionHubmakes the component depend on the auth subsystem being registered; consider falling back gracefully (e.g., making the dependency optional or usingTryGetService) so apps without authentication don’t fail at startup. - In
ConnectionHub.Callback, the authentication state is fetched on every callback; if callbacks are frequent, consider caching the username for the connection or otherwise minimizing repeatedGetAuthenticationStateAsynccalls to reduce overhead.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Injecting `AuthenticationStateProvider` directly into `ConnectionHub` makes the component depend on the auth subsystem being registered; consider falling back gracefully (e.g., making the dependency optional or using `TryGetService`) so apps without authentication don’t fail at startup.
- In `ConnectionHub.Callback`, the authentication state is fetched on every callback; if callbacks are frequent, consider caching the username for the connection or otherwise minimizing repeated `GetAuthenticationStateAsync` calls to reduce overhead.
## Individual Comments
### Comment 1
<location> `src/BootstrapBlazor/Components/ConnectionHub/ConnectionHub.cs:89-91` </location>
<code_context>
}
+
+ var state = await AuthenticationStateProvider.GetAuthenticationStateAsync();
+ var identity = state.User.Identity;
+ if (identity is { IsAuthenticated: true })
+ {
+ client.UserName = identity.Name;
+ }
</code_context>
<issue_to_address>
**question (bug_risk):** Clarify behavior when a previously authenticated user becomes anonymous to avoid stale usernames.
Right now `client.UserName` is only set when `identity is { IsAuthenticated: true }`. If the user later becomes unauthenticated (logout, timeout), the previous username will remain, so an anonymous client may still appear as a named user.
If that’s undesirable, clear `client.UserName` in the `else` branch when the identity is not authenticated. If you do want to preserve the last known user, double-check that all consumers of `ClientInfo` are prepared for that behavior.
</issue_to_address>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.
Pull request overview
This PR adds a UserName property to the ClientInfo class that automatically captures the currently logged-in user's name from the authentication state. This enhancement allows tracking which user is associated with each client connection in the ConnectionHub system.
Changes:
- Added
UserNameproperty toClientInfoclass with bilingual XML documentation - Modified
ConnectionHubcomponent to populateUserNamefromAuthenticationStateProvider - Added test coverage for the authenticated user scenario with a mock authentication state provider
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/BootstrapBlazor/Services/WebClientService.cs | Added nullable UserName string property to ClientInfo class |
| src/BootstrapBlazor/Components/ConnectionHub/ConnectionHub.cs | Injected AuthenticationStateProvider and added logic to retrieve and set the authenticated user's name in the Callback method |
| test/UnitTest/Services/ConnectionHubTest.cs | Added mock AuthenticationStateProvider class and test assertions to verify UserName is correctly populated when a user is authenticated |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7601 +/- ##
========================================
Coverage ? 100.00%
========================================
Files ? 749
Lines ? 33001
Branches ? 4580
========================================
Hits ? 33001
Misses ? 0
Partials ? 0
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Link issues
fixes #7600
Summary By Copilot
Regression?
Risk
Verification
Packaging changes reviewed?
☑️ Self Check before Merge
Summary by Sourcery
Associate connected clients with the currently authenticated user and expose their username in connection tracking.
New Features:
Enhancements:
Tests: