aspnet / AspNetKatana

Microsoft's OWIN implementation, the Katana project
Apache License 2.0
968 stars 334 forks source link

Shared Cookie between ASP.NET and ASP.NET Core not reading custom claims #535

Open Schoof-T opened 3 months ago

Schoof-T commented 3 months ago

Hi

We have set up cookie sharing between my ASP.NET and ASP.NET Core application. I did this using the guidance found here:

This all works great, but we have now noticed that custom claims we are adding in our ASP.NET Core application are not being read/remembered in the ASP.NET application.

We are adding the custom claim in ASP.NET Core as follows:

        .AddOpenIdConnect("oidc", options =>
        {
           ....
            options.Events = new OpenIdConnectEvents
            {
                OnTokenValidated = async (ctx) =>
                {
                    var claims = new List<Claim>();
                    var claim = new Claim("NEWCLAIM", "TEST");
                    claims.Add(claim);

                    var appIdentity = new ClaimsIdentity(claims);
                    ctx.Principal.AddIdentity(appIdentity);

                    return;
                }
            };
        });

And in ASP.NET we are reading the claims as follows:

            var principal = Thread.CurrentPrincipal as ClaimsPrincipal;
            var identity =  principal?.Identity as ClaimsIdentity;
            var claim = identity?.Claims.FirstOrDefault(x => x.Type == "NEWCLAIM");

However, the 'NEWCLAIM' is missing. All the other default claims we get from the identity provider are there.

Any ideas, something we might be missing?

Thanks!

Tratcher commented 3 months ago

You're adding the new claim to a second identity, but you're only searching the first identity for claims later. Try searching ClaimsPrincipal.Claims to see claims from all identities.

Schoof-T commented 3 months ago

You're adding the new claim to a second identity, but you're only searching the first identity for claims later. Try searching ClaimsPrincipal.Claims to see claims from all identities.

Unfortunately it's not in there either.

            var principal = Thread.CurrentPrincipal as ClaimsPrincipal;
            var claim = principal ?.Claims.FirstOrDefault(x => x.Type == "NEWCLAIM");

I also only see one identity.

Schoof-T commented 3 months ago

It seems that when I add the claim directly to the current identity, instead of adding a new identity to the principal, it does work.

I'm not sure why in all the microsoft docs I was finding to add a second identity then. I even found articles stating that adding directly to the current identity doesn't even work.

        .AddOpenIdConnect("oidc", options =>
        {
           ....
            options.Events = new OpenIdConnectEvents
            {
                OnTokenValidated = async (ctx) =>
                {
                    var claims = new List<Claim>();
                    var claim = new Claim("NEWCLAIM", "TEST");
                    claims.Add(claim);

                    (ctx.Principal.Identity as ClaimsIdentity).AddClaims(claims);

                    return;
                }
            };
        });

I also found documentation from Microsoft saying I should be adding custom claims through the IClaimsTransformation.

public class MyClaimsTransformation() : IClaimsTransformation
{
    public async Task<ClaimsPrincipal> TransformAsync(ClaimsPrincipal principal)
    {
        if (!principal.HasClaim(claim => claim.Type == "NEWCLAIM"))
        {
            ((ClaimsIdentity)principal.Identity).AddClaim(new Claim("NEWCLAIM", "TEST"));
        }

        return principal;
    }
}

I did try that, but that doesn't work in either cases. The custom claim is not persisted. It's not even persisted through different calls to the TransformAsync method.

I feel like I'm mising something, or doing something wrong here. But I can't put my finger on it.

Tratcher commented 3 months ago

Ah, the old interop serializer only supports one identity, so that explains that part. https://github.com/dotnet/aspnetcore/blob/9f359891a031f52b597da1799aa0813b19dbc03c/src/Security/Interop/src/AspNetTicketSerializer.cs#L43-L57

Schoof-T commented 3 months ago

Great! So it's safe to use it like I'm using it now then?