Search code examples
asp.net-coremicrosoft-identity-web

Microsoft.Identity.Web OnTokenValidated event doesn't seem to play nicely with async


I've been struggling with the Func that I attach to OnTokenValidated not doing what it should be.

After lots of trying different things:

services.Configure<MicrosoftIdentityOptions>(options =>
{
    options.Events ??= new OpenIdConnectEvents();
    options.Events.OnTokenValidated += context =>
    {
        context.Principal.AddIdentity(new ClaimsIdentity(new List<Claim> {new Claim("Foo", "Bar")}));
        return Task.CompletedTask;
    };
}

works as expected and the claim "Foo" with value "Bar" is added to the identity.

but, as soon as I await a Task within the code and make the func async, the additional claim does not get added to the signed in Identity:

services.Configure<MicrosoftIdentityOptions>(options =>
{
    options.Events ??= new OpenIdConnectEvents();
    options.Events.OnTokenValidated += async context =>
    {
        var someService = context.HttpContext.RequestServices.GetRequiredService<ISomeService>();
        var someValue = await someService.SomeMethod();
        context.Principal.AddIdentity(new ClaimsIdentity(new List<Claim> {new Claim("Foo", "Bar")}));
    };
}

does not work. To be clear, it doesn't error, but after the method completes, the additional claim does not exist on the identity...

As far as I can tell, by elimating various things, it's the await which is making it break, yet the definition of OnTokenValidated is:

Func<TokenValidatedContext,Task> OnTokenValidated

So it seems to expect handlers to be async?


Update: 2022-02-07 Taking this even further:

OnTicketReceived = ctx =>
{
    throw new AuthenticationException("Sorry, you cannot log in");
}

causes the login to fail due to the exception being thrown, whereas

OnTicketReceived = async ctx =>
{
    throw new AuthenticationException("Sorry, you cannot log in");
}

does not work - the exception being thrown does not affect the login, the user is logged in correctly despite an exception being thrown. It appears as though the code exection has already moved on as if something somewhere in the stack is not awaiting... yet looking at the code on github I can't find anywhere that an async method in the stack isn't awaited


Update 2022-02-09 Example: https://github.com/VaticanUK/msal_so_fail_example

I have taken one of the official MS examples (available here: https://github.com/Azure-Samples/active-directory-aspnetcore-webapp-openidconnect-v2) and added the customisation of MicrosoftIdentityOptions in Startup.cs: (https://github.com/VaticanUK/msal_so_fail_example/blob/master/Startup.cs#L40) using the pattern shown in the documentation: https://github.com/AzureAD/microsoft-identity-web/wiki/customization#customization-in-the-startupcs

To run the example, you need to create an app in Azure and set your client ID in appsettings.json.

As-is, the example will work, by which I mean login will fail (since there is no async keyword):

Exception thrown, and login aborted

If you add the async keyword, the exception will not prevent login: Exception thrown, but login succeeded


Solution

  • I raised the above on the MSAL Github account and after chatting to one of the contributors, the answer is that in the registration of the Options that contains the event handler it needs to be registered as:

    services.Configure<MicrosoftIdentityOptions>(OpenIdConnectDefaults.AuthenticationScheme, options =>
    

    rather than:

    services.Configure<MicrosoftIdentityOptions>(options =>
    

    so the full registration that works is:

    services.Configure<MicrosoftIdentityOptions>(OpenIdConnectDefaults.AuthenticationScheme, options =>
    {
        options.Events ??= new OpenIdConnectEvents();
        options.Events.OnTokenValidated += async context =>
        {
            var someService = context.HttpContext.RequestServices.GetRequiredService<ISomeService>();
            var someValue = await someService.SomeMethod();
            context.Principal.AddIdentity(new ClaimsIdentity(new List<Claim> {new Claim("Foo", "Bar")}));
        };
    }
    

    At the moment, I don't understand how this fixes the issue, or why not having the authentication scheme as a name for the registration causes it to only work synchronously, but it's fixed my problem so I'll probably try and get to the bottom of it out of intellectual curiosity, but the above fixes the immediate issue!