Skip to content

Commit

Permalink
Bugfix #749 Claim Actions Missing & Not Running (#1899) (#1930)
Browse files Browse the repository at this point in the history
* Add in missing ClaimActions into MergedOptions

* Add unit test to verify that the merged option for ClaimAction will appear

Co-authored-by: Warren Buckley <warren@umbraco.com>
  • Loading branch information
jennyf19 and Warren Buckley authored Oct 24, 2022
1 parent 64692a7 commit bd81458
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 0 deletions.
11 changes: 11 additions & 0 deletions src/Microsoft.Identity.Web/MergedOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,17 @@ internal static void UpdateMergedOptionsFromMicrosoftIdentityOptions(MicrosoftId
mergedOptions.BackchannelHttpHandler ??= microsoftIdentityOptions.BackchannelHttpHandler;
mergedOptions.BackchannelTimeout = microsoftIdentityOptions.BackchannelTimeout;
mergedOptions.CallbackPath = microsoftIdentityOptions.CallbackPath;

mergedOptions.ClaimActions.Clear();

foreach (var claimAction in microsoftIdentityOptions.ClaimActions)
{
if (!mergedOptions.ClaimActions.Contains(claimAction))
{
mergedOptions.ClaimActions.Add(claimAction);
}
}

if (string.IsNullOrEmpty(mergedOptions.ClaimsIssuer) && !string.IsNullOrEmpty(microsoftIdentityOptions.ClaimsIssuer))
{
mergedOptions.ClaimsIssuer = microsoftIdentityOptions.ClaimsIssuer;
Expand Down
39 changes: 39 additions & 0 deletions tests/Microsoft.Identity.Web.Test/MicrosoftIdentityOptionsTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,11 @@

using System;
using System.Globalization;
using System.Linq;
using System.Security.Claims;
using Microsoft.AspNetCore.Authentication.OAuth.Claims;
using Microsoft.AspNetCore.Authentication.OpenIdConnect;
using Microsoft.AspNetCore.Authentication.OpenIdConnect.Claims;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Options;
using Microsoft.Identity.Client;
Expand Down Expand Up @@ -102,6 +106,41 @@ public void ValidateRequiredMicrosoftIdentityOptions(
}
}

[Fact]
public void TestMergedOptions_ContainsClaimsActions()
{

_microsoftIdentityOptionsMonitor = new TestOptionsMonitor<MicrosoftIdentityOptions>(new MicrosoftIdentityOptions
{
ClaimActions =
{
new UniqueJsonKeyClaimAction(ClaimTypes.Gender, "string", "sex"),
},
});

BuildTheRequiredServices();
MergedOptions mergedOptions = _provider.GetRequiredService<IOptionsMonitor<MergedOptions>>().Get(OpenIdConnectDefaults.AuthenticationScheme);

MergedOptions.UpdateMergedOptionsFromMicrosoftIdentityOptions(_microsoftIdentityOptionsMonitor.Get(OpenIdConnectDefaults.AuthenticationScheme), mergedOptions);

// Verify that the mergedOptions.ClaimActions has claims
// It should contain some default ones along with our added one
Assert.NotEmpty(mergedOptions.ClaimActions.AsEnumerable());

// See if we can find the ClaimAction that we added
Assert.Contains(mergedOptions.ClaimActions, action => action.ClaimType == ClaimTypes.Gender);

// Select the single ClaimAction from the collection
var genderClaim = mergedOptions.ClaimActions.Single(x => x.ClaimType == ClaimTypes.Gender);

// Assert its a type of UniqueJsonKeyClaimAction
Assert.IsType<UniqueJsonKeyClaimAction>(genderClaim);

// Ensure gender has the value of sex
var jsonKeyClaim = genderClaim as UniqueJsonKeyClaimAction;
Assert.Equal(jsonKeyClaim.JsonKey, "sex");
}

private void BuildTheRequiredServices()
{
var services = new ServiceCollection();
Expand Down

0 comments on commit bd81458

Please sign in to comment.