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

[Bug] HttpContext.GetTokenAsync("access_token") returns always null when using EnableTokenAcquisitionToCallDownstreamApi() #929

Closed
1 of 8 tasks
berhir opened this issue Feb 5, 2021 · 10 comments
Assignees
Labels
enhancement New feature or request fixed
Milestone

Comments

@berhir
Copy link

berhir commented Feb 5, 2021

Which version of Microsoft Identity Web are you using?
Microsoft.Identity.Web 1.5.1

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)

Is this a new or an existing app?
This is a new app

Repro

public void ConfigureServices(IServiceCollection services)
{
    // Adds Microsoft Identity platform (AAD v2.0) support to protect this API
    services.AddAuthentication(JwtBearerDefaults.AuthenticationScheme)
                .AddMicrosoftIdentityWebApi(Configuration)
                .EnableTokenAcquisitionToCallDownstreamApi()
                .AddInMemoryTokenCaches();
    // ...
}

public void Configure(IApplicationBuilder app, IWebHostEnvironment env)
{
    // ...

    app.Use(async (context, next) =>
    {
        var token = await context.GetTokenAsync("access_token");
        // token is always null here

        await next();
    });

    // ...
}

Expected behavior
When JwtBearerOptions.SaveToken is true, context.GetTokenAsync("access_token") returns the access token used in the request.

Actual behavior
context.GetTokenAsync("access_token") returns always null when EnableTokenAcquisitionToCallDownstreamApi() is used.
When I remove EnableTokenAcquisitionToCallDownstreamApi(), I get the access token as expected.

Possible solution
I debugged the code, and the problem is in MicrosoftIdentityWebApiAuthenticationBuilder.cs:

options.Events.OnTokenValidated = async context =>
{
    await onTokenValidatedHandler(context).ConfigureAwait(false);
    context.HttpContext.StoreTokenUsedToCallWebAPI(context.SecurityToken as JwtSecurityToken);
    context.Success();
};

I don't know why context.Success(); it's needed, but when I remove it, everything works as expected.

@berhir
Copy link
Author

berhir commented Feb 6, 2021

This is my current workaround for this problem:

public void ConfigureServices(IServiceCollection services)
{
    // Adds Microsoft Identity platform (AAD v2.0) support to protect this API
    var authBuilder = services.AddAuthentication(JwtBearerDefaults.AuthenticationScheme)
                .AddMicrosoftIdentityWebApi(configuration);

    // get JwtBearerOptions before calling EnableTokenAcquisitionToCallDownstreamApi()
    using var sp = services.BuildServiceProvider();
    var jwtOptionsSnapshot = sp.GetRequiredService<IOptionsSnapshot<JwtBearerOptions>>();
    var jwtOptions = jwtOptionsSnapshot.Get(JwtBearerDefaults.AuthenticationScheme);

    authBuilder.EnableTokenAcquisitionToCallDownstreamApi()
               .AddMicrosoftGraph()
               .AddInMemoryTokenCaches();

    // apply config from MicrosoftIdentityWebApiAuthenticationBuilder.cs without "context.Success();"
    services.AddOptions<JwtBearerOptions>(JwtBearerDefaults.AuthenticationScheme)
        .Configure<IServiceProvider>((options, serviceProvider) =>
        {
            var onTokenValidatedHandler = jwtOptions.Events.OnTokenValidated;

            options.Events.OnTokenValidated = async context =>
            {
                await onTokenValidatedHandler(context).ConfigureAwait(false);
                context.HttpContext.Items.Add("JwtSecurityTokenUsedToCallWebAPI", context.SecurityToken as JwtSecurityToken);
            };
        });

    // ...
}

@jmprieur jmprieur added the enhancement New feature or request label Feb 7, 2021
@jordykrul
Copy link

It works fine for me when I use ITokenAcquisition. Use the methode GetAccessTokenForUserAsync for example. This will get you the AccessToken. Maybe you could use this for the time being.

@berhir
Copy link
Author

berhir commented Feb 12, 2021

@jordykrul Thank you for the suggestion. But I am using a library that is using context.GetTokenAsync("access_token") internally and I don't have access to the code. And the library should work also without Microsoft.Identity.Web and therefore it can't use ITokenAcquisition.

@jmprieur
Copy link
Collaborator

@berhir.

  1. We'll add the support to get the access token from the context.
  2. For your work around, I think that this should work:
public void ConfigureServices(IServiceCollection services)
{
    // Adds Microsoft Identity platform (AAD v2.0) support to protect this API
    services.AddAuthentication(JwtBearerDefaults.AuthenticationScheme)
                .AddMicrosoftIdentityWebApi(configuration);
                .EnableTokenAcquisitionToCallDownstreamApi()
                .AddMicrosoftGraph()
                .AddInMemoryTokenCaches();

}

services.Configure<JwtBearerOptions>(JwtBearerDefaults.AuthenticationScheme, options =>
{
  options.Events.OnTokenValidated = async context =>
  {
     context.HttpContext.StoreTokenUsedToCallWebAPI(context.SecurityToken as JwtSecurityToken);
  }
});

to override the Microsoft.Identity.Web implementation (assuming you did not have any OnTokenValidated event handler.

@jmprieur
Copy link
Collaborator

@jennyf19
We should not call context.Success() in the OnTokenValidated Jwt event handler, so that the token is also available in ASP.NET Core previous ways. Do you remember if this is something Yordan asked us to do?

@ryanfalzon
Copy link
Contributor

@jmprieur we are facing the same issue and have noticed that this has not yet been fixed. We created the PR for the change you suggested where we remove the context.Success(): #1387

@jennyf19
Copy link
Collaborator

@Tratcher can we get your thoughts on this? We include context.Success(); because we have successfully processed the access token sent to the web API, so we wouldn't want to hit a failure. Do you see any issue with removing it? thanks.

@Tratcher
Copy link

Remove it. Success() means you've taken care of everything and want to disable any further processing. In this case the only further processing you're skipping is the part where we save the access token for later use.
https://github.com/dotnet/aspnetcore/blob/61fc66cada5d72e76513918aee515846f2923b4e/src/Security/Authentication/JwtBearer/src/JwtBearerHandler.cs#L155-L161

@jennyf19
Copy link
Collaborator

great. thanks for confirming @Tratcher appreciate it.

cc: @jmprieur

@jennyf19 jennyf19 added this to the 1.16 milestone Aug 19, 2021
@jennyf19 jennyf19 added the fixed label Aug 19, 2021
@jennyf19
Copy link
Collaborator

Included in 1.16 release

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request fixed
Projects
None yet
Development

No branches or pull requests

6 participants