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

OnTokenValidated Invoked Twice when using the delegate form of AddIdentityWebApp #2456

Closed
DaleyKD opened this issue Sep 8, 2023 · 3 comments · Fixed by #2489
Closed

OnTokenValidated Invoked Twice when using the delegate form of AddIdentityWebApp #2456

DaleyKD opened this issue Sep 8, 2023 · 3 comments · Fixed by #2489
Labels
bug Something isn't working P1

Comments

@DaleyKD
Copy link

DaleyKD commented Sep 8, 2023

Microsoft.Identity.Web Library

Microsoft.Identity.Web

Microsoft.Identity.Web version

2.13.4

Web app

Sign-in users and call web APIs

Web API

Protected web APIs (validating scopes/roles)

Token cache serialization

In-memory caches

Description

Based upon multiple examples on how to call MS Graph after signing in, I would hope that OnTokenValidated is only called once. However, it is called twice.

Reproduction steps

See code.

Error message

No response

Id Web logs

No response

Relevant code snippets

internal static IServiceCollection AddAuthentication(this IServiceCollection services, IConfiguration configuration, bool isDev)
{
    var apiScopes = configuration.GetSection("MyApi:Scopes").Get<string[]>() ?? Array.Empty<string>();
    var graphScopes = configuration["MicrosoftGraph:Scopes"]?.Split(' ') ?? Array.Empty<string>();

    // Add services to the container.
    var webApiAuthenticationBuilder = services
        .AddAuthentication(OpenIdConnectDefaults.AuthenticationScheme)
        .AddMicrosoftIdentityWebApp(options =>
        {
            configuration.Bind("AzureAd", options);

            var previousOnTokenValidatedHandler = options.Events.OnTokenValidated;
            options.Events.OnTokenValidated = ctx => GetGraphUserPhotoAsync(ctx, graphScopes, previousOnTokenValidatedHandler);
        })
        .EnableTokenAcquisitionToCallDownstreamApi(apiScopes)
        .AddMicrosoftGraph(configuration.GetSection("MicrosoftGraph"))
        .AddDownstreamApi("MyApi", configuration.GetSection("MyApi"));

    if (isDev)
    {
        webApiAuthenticationBuilder.AddInMemoryTokenCaches();
    }
    else
    {
        webApiAuthenticationBuilder.AddDistributedTokenCaches();
    }

    return services;
}

private static async Task GetGraphUserPhotoAsync(TokenValidatedContext context, IEnumerable<string> graphScopes, Func<TokenValidatedContext, Task> previousOnTokenValidatedHandler)
{
    // Let Microsoft.Identity.Web process the token
    await previousOnTokenValidatedHandler(context).ConfigureAwait(false);

    var consentHandler = context.HttpContext.RequestServices.GetRequiredService<MicrosoftIdentityConsentAndConditionalAccessHandler>();
    var tokenAcquisition = context.HttpContext.RequestServices.GetRequiredService<ITokenAcquisition>();
    var graphClient = new GraphServiceClient(new DelegateAuthenticationProvider(async request =>
    {
        var token = await tokenAcquisition.GetAccessTokenForUserAsync(graphScopes, user: context.Principal).ConfigureAwait(false);
        request.Headers.Authorization = new AuthenticationHeaderValue("Bearer", token);
    }));

    // Get user information from Graph
    try
    {
        var user = await graphClient.Me.Request().Select(u => new { u.DisplayName, u.Mail, u.UserPrincipalName, u.CompanyName }).GetAsync().ConfigureAwait(false);
        context.Principal?.AddUserGraphInfo(user);

        // Get the user's photo
        try
        {
            var photo = await graphClient.Me.Photos["48x48"].Content.Request().GetAsync().ConfigureAwait(false);
            context.Principal?.AddUserGraphPhoto(photo);
        }
        catch (ServiceException svcEx)
        {
            if (svcEx.IsMatch("ErrorItemNotFound") || svcEx.IsMatch("ImageNotFound") || svcEx.IsMatch("ConsumerPhotoIsNotSupported"))
            {
                context.Principal?.AddUserGraphPhoto(null);
            }
            else
            {
                consentHandler.HandleException(svcEx);
            }
        }
    }
    catch (Exception ex)
    {
        consentHandler.HandleException(ex);
    }
}

Regression

No response

Expected behavior

OnTokenValidated should only be invoked once.

@DaleyKD
Copy link
Author

DaleyKD commented Sep 21, 2023

It appears that this IS a bug still. ALL examples that load something from Graph OnTokenValidated do this, and it runs twice.

I am to get around it by doing the following:

services.Configure<MicrosoftIdentityOptions>(OpenIdConnectDefaults.AuthenticationScheme, options =>
{
    var previousOnTokenValidatedHandler = options.Events.OnTokenValidated;
    options.Events.OnTokenValidated = ctx => GetGraphUserPhotoAsync(ctx, graphScopes, previousOnTokenValidatedHandler);
    options.Events.OnAuthenticationFailed = OnAuthenticationFailed;
    options.Events.OnRemoteFailure = OnRemoteFailure;
});

@jmprieur jmprieur added bug Something isn't working P1 and removed question Further information is requested labels Sep 21, 2023
@jmprieur jmprieur changed the title OnTokenValidated Invoked Twice OnTokenValidated Invoked Twice when using the delegate form of AddIdentityWebApp Sep 21, 2023
@jmprieur
Copy link
Collaborator

@jennyf19 @JoshLozensky @westin-m
I added it to the board

@mvromer
Copy link

mvromer commented Sep 28, 2023

I'm seeing something similar in MIW 2.14.0 in my ASP.NET Core app where I'm trying to subscribe to OnRedirectToIdentityProvider. I'm passing an Action<MicrosoftIdentityOptions> in the call to AddMicrosoftIdentityWebApp. Later, I'm also calling EnableTokenAcquisitionToCallDownstreamApi. I threw the following in my Action delegate:

Console.WriteLine("\nConfiguring MIW options\n");

Sure enough, when I run the app locally, I see "Configuring MIW options" twice in the output. I think this is related to #1796. It looks like the configure delegate gets added first here during the call to AddMicrosoftIdentityWeb:

builder.Services.Configure(openIdConnectScheme, configureMicrosoftIdentityOptions); // <---- First Configure
builder.Services.AddSingleton<IMergedOptionsStore, MergedOptionsStore>();
builder.Services.AddHttpClient();

Then, when EnableTokenAcquisitionToCallDownstreamApi, the delegate gets applied a second time here:

// Ensure that configuration options for MSAL.NET, HttpContext accessor and the Token acquisition service
// (encapsulating MSAL.NET) are available through dependency injection
services.Configure(openIdConnectScheme, configureMicrosoftIdentityOptions);  // <---- Second Configure

Seems like there should be some sort of check to avoid the configure delegate from getting applied twice when enabling downstream token acquisition? When I apply a pattern like the one documented here for subscribing to OIDC events, it'll result in the OIDC event handlers getting invoked multiple times.

jmprieur added a commit that referenced this issue Oct 1, 2023
@jmprieur jmprieur mentioned this issue Oct 1, 2023
jmprieur added a commit that referenced this issue Oct 1, 2023
* Fix build on .NET FW after #2480

* Fix the "The referenced project is targeted to a different framework family" warnings
by changing the order of the frameworks (older to newer) as advised in https://www.primordialcode.com/blog/post/referenced-project-targeted-different-framework-family

* Fixes #2456

* Addressing comments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working P1
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants