[Bug] ClaimActions not being invoked

[tonytilbo]

Which version of Microsoft Identity Web are you using?
E.g. Microsoft Identity Web 1.2.0

Where is the issue?

• Web app
  • Sign-in users
  • Sign-in users and call web APIs
• Web API
  • Protected web APIs (validating tokens)
  • Protected web APIs (validating scopes)
  • Protected web APIs call downstream web APIs
• Token cache serialization
  • In-memory caches
  • Session caches
  • Distributed caches
• Other (please describe)
  This is a new app or an experiment.

Repro
We are attempting to use a custom ClaimAction that will do some additional mapping of JWT claims to custom claims on the identity.

public class CustomClaimAction : ClaimAction
{
	public CustomClaimAction(string claimType, string valueType): base (claimType, valueType) { }
        public override void Run(JsonElement userData, ClaimsIdentity identity, string issuer) 
                                  => throw new System.NotImplementedException();
}

services.AddAuthentication(JwtBearerDefaults.AuthenticationScheme)
  .AddMicrosoftIdentityWebApi(jwt => { }, options =>
  {
      options.TenantId = Configuration["AzureAd:TenantId"];
      options.ClientId = Configuration["AzureAd:ClientId"];
      options.Domain = Configuration["AzureAd:Domain"];
      options.Instance = Configuration["AzureAd:Instance"];

       options.ClaimActions.Add(new CustomClaimAction (ClaimTypes.Role, ClaimValueTypes.String));
 });

Expected behavior
I expect the Run method of the CustomClaimAction to be invoked.

Actual behavior
Method is not called.

[jmprieur]

@tonytilbo thanks for the heads-up.

This property is normally an OpenIdConnect property (for a web app, and acting on the IDToken claims), whereas here you have a web API, but indeed, given it's surfaced in MicrosoftIdentityOptions, we should probably make it work for the Access token claims as well.

Out of curiosity, what is your scenario? what are you trying to achieve?

[tonytilbo]

We are looking to check for "groups" claims in the incoming access token and maps these based on some external configuration to a set of roles which would be added to the ClaimsIdentity so we can manage access to the API used role based authorization.

[jmprieur]

@tonytilbo. Thanks for the update.
Alternatively, until we get this working, you could have a look at this sample: https://github.com/Azure-Samples/active-directory-aspnetcore-webapp-openidconnect-v2/blob/7cfdb74dc595a04d95e351c8037a9433fccee0c7/5-WebApp-AuthZ/5-2-Groups/Startup.cs#L51.

[tonytilbo]

Thanks, will take a look.

[gunnim]

@jmprieur afaics MergedOptions.UpdateMergedOptionsFromMicrosoftIdentityOptions is not mapping the configured claims here

https://github.com/AzureAD/microsoft-identity-web/blob/master/src/Microsoft.Identity.Web/WebAppExtensions/MicrosoftIdentityWebAppAuthenticationBuilderExtensions.cs#L299

[NickDarvey]

I'm trying to achieve something different but I believe it might be the same cause (or should be considered when this is being fixed).

I want to clear the default ClaimActions as described here.

authn.AddMicrosoftIdentityWebApp(fun o -> o.ClaimActions.Clear ())

However, the default ClaimActions remain and are removing claims.

[warrenbuckley]

Any update on this @jmprieur or @jennyf19 please ?

Current Approach

memberAuthenticationBuilder.AddMicrosoftIdentityWebApp(options =>
{
    // TODO: Read from Config (AppSettings/Env Variables)
    options.Instance = "https://myb2c.b2clogin.com/";
    options.ClientId = "085ad022-XXXXXX";
    options.Domain = "myb2c.onmicrosoft.com";
    options.CallbackPath = "/signin-oidc";
    options.SignUpSignInPolicyId = "B2C_1_MVP_SignupAndSignIn";

    // Dont need to use in end - as Signup and SignIn has a self serve password reset option
    options.ResetPasswordPolicyId = "B2C_1_MVP_ResetPassword";

    // Do we defitely need them to edit their profile
    // We need to sync it again back to the local member if they did change email or name
    options.EditProfilePolicyId = "B2C_1_MVP_ProfileEdit";

    // The Claims we get from Azure B2C need to be remapped to new claims
    // That Umbraco is expecting in order to create the local linked member
    // Using MapUniqueJsonKey ensures if claim already exists it will not override it
    // TODO: Not sure why this simplier method/approach is not working...
    options.ClaimActions.MapJsonKey(ClaimTypes.Email, "WARREN_emails");
    options.ClaimActions.MapJsonKey(ClaimTypes.Name, "WARREN_name");


    // When we verify the token back from Azure B2C
    options.Events.OnTokenValidated = context =>
    {
        // This code gets the claims in the token
        // And re-assigns email & name to new claims to make Umbraco happy with this intergration
        ClaimsPrincipal? principal = context.Principal;
        if (principal is null)
        {
            throw new InvalidOperationException("No claims found.. :(");
        }

        var claims = principal.Claims.ToList();

        Claim? email = claims.SingleOrDefault(x => x.Type == "emails");
        if (email is not null)
        {
            claims.Add(new Claim(ClaimTypes.Email, email.Value));
        }

        Claim? name = claims.SingleOrDefault(x => x.Type == "name");
        if (name is not null)
        {
            claims.Add(new Claim(ClaimTypes.Name, name.Value));
        }

        // Override with the updated name & email claims that Umbraco wants to be happy
        var authenticationType = principal.Identity?.AuthenticationType;
        context.Principal = new ClaimsPrincipal(new ClaimsIdentity(claims, authenticationType));

        return Task.CompletedTask;
    };
}, 
openIdConnectScheme: memberAuthenticationBuilder.SchemeForMembers(UmbracoIdMemberExternalLoginProviderOptions.SchemeName));

I am using the event OnTokenValidated to find the existing name and email claim and add new claims that Umbraco CMS is expecting when creating a member and logging in with that.

I would much prefer to use the simpler approach of using ClaimActions to do this then the current block of code in OnTokenValidated to achieve the same thing.

[jmprieur]

Fixed