Add section for name and role claims configuration for Blazor with Identity Server#18278
Merged
Add section for name and role claims configuration for Blazor with Identity Server#18278
Conversation
mkArtakMSFT
reviewed
May 17, 2020
mkArtakMSFT
approved these changes
May 17, 2020
Collaborator
Author
|
UPDATE ... A Javier fix is on the way. I'll institute it for the PR and then merge this by EOD. |
Collaborator
Author
|
FIX IS IN – https://github.com/javiercn/BlazorAuthRoles I'll get this updated and merged shortly. |
guardrex
added a commit
that referenced
this pull request
May 18, 2020
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #17317
Fixes #17649
Thanks @caleblloyd, @AlbertoPa, @hakenr, and @denmitchell.
Internal Review Topic (links to Name and role claim with API authorization section) ... and see the following new section here on Profile Service.
Some of the ask on #17317 was worked on prior PRs. The parts that pertain to Identity Server are on this PR.
The guidance on the PR only supports ONE role per authenticated user. I ran into a problem trying to use the factory pattern for multiple roles. If all you want to do is support one role per user right now, then the PR is fine for now, and we can ignore what follows here 👇.
If you want to know what's failing ........
No factory
👍 RESOLVED ... Single role without a factory works. The app receives a role claim with a string role value. The PR covers this scenario.
Factory pattern
For multiple roles in a role claim ...
... a factory that parses the JSON works, but this isn't what we want to show. See: #17649 (comment) for @hakenr's approach.
When a factory is implemented without directly parsing JSON (the code for the factory is commented out on the PR to hold it for now) ...
👍 RESOLVED ... When the user has no assigned role (no
roleclaim),account.Rolesin the factory isnulland so the factory throws. To resolve it, theRolesproperty inCustomUserAccountis set to an empty string array:... let me know if you prefer an alternate approach.
👍 RESOLVED ... When the user has two or more roles ...
... the factory works and creates a separate role claim for each role. 🎸
💥 BROKEN 💥 ... When the user only has one assigned role in a string (not an array) ...
... the factory approach breaks with ...