Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
mpaulosky
added
squad
There was a problem hiding this comment.
Pull request overview
Fixes role extraction in the Web UI’s Auth0 authentication state provider so admin users reliably receive ClaimTypes.Role claims (and therefore see admin-only navigation) when Auth0 emits roles in different formats.
Changes:
- Updated
Auth0AuthenticationStateProviderto extract roles from a custom namespaced claim (comma-separated, JSON array, or single value) and from standard"roles"claims. - Added unit tests covering multiple role formats, whitespace/duplicates, and error scenarios.
- Expanded
docs/auth0-setup.mdwith role setup, Post-Login Action configuration, and troubleshooting steps.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
src/Web/Services/Auth0AuthenticationStateProvider.cs |
Adds multi-format role extraction + additional logging around claims/roles. |
tests/Web.Tests.Unit/Services/Auth0AuthenticationStateProviderTests.cs |
New unit test coverage for role parsing and edge cases. |
tests/Web.Tests.Unit/GlobalUsings.cs |
Adds Microsoft.Extensions.Logging for the new tests. |
docs/auth0-setup.md |
Documents Auth0 roles + Post-Login Action setup and troubleshooting. |
You can also share your feedback on Copilot code review. Take the survey.
| private static void AddRoleIfNotExists(ClaimsIdentity identity, string role) | ||
| { | ||
| string trimmedRole = role.Trim(); | ||
| if (!string.IsNullOrEmpty(trimmedRole) && !identity.HasClaim(ClaimTypes.Role, trimmedRole)) | ||
| { | ||
| identity.AddClaim(new Claim(ClaimTypes.Role, trimmedRole)); | ||
| } |
There was a problem hiding this comment.
AddRoleIfNotExists assumes role is non-null and calls Trim(). When roles are sourced from JsonSerializer.Deserialize<string[]>(...), the array can contain null elements (e.g., ["Admin", null]), which would cause a NullReferenceException during role extraction. Consider treating roles as string? and skipping null/whitespace entries before trimming/adding.
| _logger.LogInformation("User roles extracted: {Roles}", string.Join(", ", finalRoles)); | ||
| } | ||
| else | ||
| { | ||
| _logger.LogWarning("No roles found for authenticated user. Check Auth0 Post-Login Action configuration."); |
There was a problem hiding this comment.
GetAuthenticationStateAsync is called frequently in a Blazor app; logging at Information/Warning on every call (especially the "No roles found" warning) can create significant log noise and may page on normal scenarios (users without roles, misconfigured dev environments). Consider downgrading these to Debug/Trace, or only warning when a roles claim is present but cannot be parsed / is unexpectedly empty.
| _logger.LogInformation("User roles extracted: {Roles}", string.Join(", ", finalRoles)); | |
| } | |
| else | |
| { | |
| _logger.LogWarning("No roles found for authenticated user. Check Auth0 Post-Login Action configuration."); | |
| _logger.LogDebug("User roles extracted: {Roles}", string.Join(", ", finalRoles)); | |
| } | |
| else | |
| { | |
| _logger.LogDebug("No roles found for authenticated user. Check Auth0 Post-Login Action configuration."); |
| var result = await _sut.GetAuthenticationStateAsync(); | ||
|
|
||
| // Assert | ||
| result.User.Identity!.IsAuthenticated.Should().BeTrue(); |
There was a problem hiding this comment.
This test claims invalid JSON is handled gracefully, but it doesn't assert the expected outcome (e.g., that no role claims were added). Adding an assertion that ClaimTypes.Role claims are empty would better lock in the behavior and catch regressions.
| result.User.Identity!.IsAuthenticated.Should().BeTrue(); | |
| result.User.Identity!.IsAuthenticated.Should().BeTrue(); | |
| result.User.Claims.Where(c => c.Type == ClaimTypes.Role).Should().BeEmpty(); |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #110 +/- ##
==========================================
+ Coverage 54.10% 55.52% +1.41%
==========================================
Files 124 124
Lines 2717 2826 +109
Branches 287 313 +26
==========================================
+ Hits 1470 1569 +99
+ Misses 1032 1027 -5
- Partials 215 230 +15
🚀 New features to boost your workflow:
|
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Address reviewer feedback from PR #110: - Handle null elements in deserialized role arrays (null safety) - Downgrade role logging from Info/Warning to Debug (reduce log noise) - Add assertion for empty roles on invalid JSON in test - Fix OnTokenValidated event to extract roles from custom namespace claim - Update NavMenu to link profile page from greeting - Clean up Profile page layout and move to /user/profile Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Summary
Fixes admin users not seeing admin-only menu items despite having the Admin role in Auth0.
Changes
Testing
Closes #107