Skip to content
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

Same type claims are merged when they get to the client side #39144

Closed
1 task done
3x0dv5 opened this issue Dec 21, 2021 · 7 comments
Closed
1 task done

Same type claims are merged when they get to the client side #39144

3x0dv5 opened this issue Dec 21, 2021 · 7 comments
Labels
area-blazor Includes: Blazor, Razor Components feature-blazor-wasm This issue is related to and / or impacts Blazor WebAssembly feature-blazor-wasm-auth ✔️ Resolution: Answered Resolved because the question asked by the original author has been answered. question Status: Resolved

Comments

@3x0dv5
Copy link

3x0dv5 commented Dec 21, 2021

Is there an existing issue for this?

  • I have searched the existing issues

Describe the bug

The issue can be better understood by looking at this issue: openiddict/openiddict-core#1365
The claims list in the server side behaves differently from the client/blazor side.
Somehow when the token is read the claims of the same type are merged together.

Expected Behavior

We have this content in the HttpContext.User.Claims object on the server side
image

I would expect to have the same representation on the client side and not a string with the claim values merged
image

Steps To Reproduce

I have a repo here: https://github.com/3x0dv5/openiddict-samples/tree/bug_1365

Use the sample Balosar to see the behaviour
image

When you run the application and after logging in there will be an orange button to add the sample claims. Once you click on it, logout and back in and you should see the issue.

Exceptions (if any)

No response

.NET Version

6.0.100

Anything else?

No response

@TanayParikh TanayParikh added the area-auth Includes: Authn, Authz, OAuth, OIDC, Bearer label Dec 21, 2021
@Tratcher Tratcher added the area-blazor Includes: Blazor, Razor Components label Dec 21, 2021
@pranavkm pranavkm added area-blazor Includes: Blazor, Razor Components and removed area-blazor Includes: Blazor, Razor Components labels Dec 27, 2021
@BrennanConroy BrennanConroy removed the area-auth Includes: Authn, Authz, OAuth, OIDC, Bearer label Jan 3, 2022
@javiercn
Copy link
Member

@3x0dv5 thanks for contacting us.

I think this is by design. They are two different systems and at some point the arrays get converted to individual claims on the client.

You can provide your own https://github.com/dotnet/aspnetcore/blob/main/src/Components/WebAssembly/WebAssembly.Authentication/src/Services/AccountClaimsPrincipalFactory.cs to control how the token gets transformed into a ClaimsPrincipal yourself.

@javiercn javiercn added ✔️ Resolution: Answered Resolved because the question asked by the original author has been answered. feature-blazor-wasm This issue is related to and / or impacts Blazor WebAssembly feature-blazor-wasm-auth question labels Jan 10, 2022
@ghost ghost added the Status: Resolved label Jan 10, 2022
@kevinchalet
Copy link
Contributor

I think this is by design.

Sadly, this makes things really inconsistent:

  • The JWT/OIDC handlers both use IdentityModel, where this "one JSON array = multiple CLR claims" mapping strategy was implemented many years ago.
  • For claims extraction from userinfo responses, the native JsonKeyClaimAction offered to all OAuth 2.0-based handlers in ASP.NET Core also use this strategy:

if (value.ValueKind == JsonValueKind.Array)
{
foreach (var v in value.EnumerateArray())
{
AddClaim(v.ToString()!, identity, issuer);
}
}

I encourage you to revisit this design to make the WASM authentication helpers consistent with the ASP.NET server-side components folks are used to.

@ghost
Copy link

ghost commented Jan 11, 2022

This issue has been resolved and has not had any activity for 1 day. It will be closed for housekeeping purposes.

See our Issue Management Policies for more information.

@ghost ghost closed this as completed Jan 11, 2022
@kevinchalet
Copy link
Contributor

@javiercn should this ticket be reopened to track possible future improvements?

@javiercn
Copy link
Member

@kevinchalet It's unlikely that we do something in this area.

The auth support in wasm is different from the auth support in ASP.NET Core and while its not great that the information is slightly different in both cases, it's not something that where we necessarily want to do work to homogenize across implementations.

Both server and client have mechanisms to completely control how the app deals with the generated principal (in the wasm case, this is done via a custom AccountClaimsPrincipalFactory) and users can implement one if they want to.

At this point I'm not in favor of changing the implementation (at least on the wasm side, but similar argument likely applies to server), as it will be breaking for existing customers and can have unwanted repercussions.

@kevinchalet
Copy link
Contributor

Both server and client have mechanisms to completely control how the app deals with the generated principal (in the wasm case, this is done via a custom AccountClaimsPrincipalFactory) and users can implement one if they want to.

A configurable bool option would be easier to use, IMHO, but hem 🤷🏻

At this point I'm not in favor of changing the implementation (at least on the wasm side, but similar argument likely applies to server), as it will be breaking for existing customers and can have unwanted repercussions.

Then this discrepancy should be clearly documented so that folks don't think it's an issue with the OIDC servers and token validation implementations. Otherwise both you and I will keep receiving bug reports for that 🤣

@rmencia-isv
Copy link

Security is not easy to get right, but if the tools and frameworks we rely on make it more difficult, it's only going to create more complex situations.

There are multiple places in where the Blazor Client and ASP.net are also different.

  1. Claim "emails" in Blazor wasm it would return '["myemail@whatever.com"]' and the server would return 'myemail@whatever.com'. Blazor WASM would return the exact same text as the Token, and in this case is a list of emails. The server would return a single value (the list is gone)
  2. for a B2C token that has the claim “oid”, in Blazor the claims shows as “oid”, but in aspnet core 6 it shows as “https://schemas.microsoft.com/identity/claims/objectidentifier”
  3. The same as "oid", name and given_name and familly_name and surname.

The Claims get change into something different in aspnet, so it means that for the same token, the code that reads from the list of claims needs to find them with completely different names.

There is also Blazor server. Does Blazor server load the token with the same names as wasm, or it loads the tokens as aspnet.

Also, aspnet uses naming from constants in 2 different classes classes.

Answering that "It's by design" is a common answer that doesn't help anyone. Many times, the design chosen is not the best for whatever reason. In those cases, improving the design is a good thing, even though I understand that it may be opening a can of worms and there are already a huge number of issues already in Blazor and dotnet and VS2022 open and nobody wants to add to the bucket.

It would be easier to allow a configuration setting that allows getting all the platforms the same.

@dotnet dotnet locked as resolved and limited conversation to collaborators Mar 9, 2022
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-blazor Includes: Blazor, Razor Components feature-blazor-wasm This issue is related to and / or impacts Blazor WebAssembly feature-blazor-wasm-auth ✔️ Resolution: Answered Resolved because the question asked by the original author has been answered. question Status: Resolved
Projects
None yet
Development

No branches or pull requests

8 participants