HttpContext.GetTokenAsync fails after Upgrade to 2.1.0

[TimHess]

Hello,
I have some code that's calling await HttpContext.GetTokenAsync("access_token"). When using Microsoft.AspNetCore.Authentication.JwtBearer version 2.0.4 it works, but upgrading to 2.1.0-preview1-final or newer results in null being returned.

I believe this is because on line 73 in Microsoft.AspNetCore.Authentication.Core/AuthenticationService.cs
return AuthenticateResult.Success(new AuthenticationTicket(transformed, result.Properties, result.Ticket.AuthenticationScheme)); result.Properties is null.

When using JwtBearer auth, result.Properties is never set - however, the properties that are expected to be there are present on result.Ticket. Changing line 73 to return AuthenticateResult.Success(new AuthenticationTicket(transformed, result.Ticket.Properties, result.Ticket.AuthenticationScheme)); appears to fix the issue for me, is there any reason not to do so?
Thanks

[Tratcher]

JwtBearer Succeeds:

Security/src/Microsoft.AspNetCore.Authentication.JwtBearer/JwtBearerHandler.cs

Lines 152 to 153 in e031eff

	tokenValidatedContext.Success();
	return tokenValidatedContext.Result;

Creates a ticket and HandleRequestResult:

Security/src/Microsoft.AspNetCore.Authentication/Events/ResultContext.cs

Line 46 in e031eff

public void Success() => Result = HandleRequestResult.Success(new AuthenticationTicket(Principal, Properties, Scheme.Name));

Stores the ticket:

Security/src/Microsoft.AspNetCore.Authentication/HandleRequestResult.cs

Line 36 in e031eff

return new HandleRequestResult() { Ticket = ticket };

Which doesn't set Properties like the base class does:
https://github.com/aspnet/HttpAbstractions/blob/bcb7bcbb7a6461204f4cefd9848e2a782ebb0a6b/src/Microsoft.AspNetCore.Authentication.Abstractions/AuthenticateResult.cs#L57

[HaoK]

Yeah looks like a bug

[TimHess]

@Tratcher Thanks for accepting the PR - is it possible to have this included in 2.1.0-RTM?

[Tratcher]

Sorry, no. 2.1 is already completed.

[davidfowl]

Is this a patch candidate? /cc @muratg @ajcvickers

[ajcvickers]

@Tratcher @HaoK Is there a workaround?

[HaoK]

The information is still there via result.Ticket.Properties, so the workaround would be to use the underlying apis instead of the top level sugar...

instead of context.GetToken("access_token") something like:

 context.AuthenticateAsync(JwtBearer.Defaults).Ticket.Properties.GetToken("access_token");

[ajcvickers]

Thanks @HaoK. So the workaround is easy. How common do we think it will be for people to hit this?

[Tratcher]

That workaround doesn't work, all of the Properties collections are empty. You'd have to go a step further and hook into an event and store the token yourself in HttpContext.Items.

[Tratcher]

Actually, isn't that token the same as the incoming Auth header value?

Security/src/Microsoft.AspNetCore.Authentication.JwtBearer/JwtBearerHandler.cs

Lines 65 to 76 in 0f18ff1

	string authorization = Request.Headers["Authorization"];
	// If no authorization header found, nothing to process further
	if (string.IsNullOrEmpty(authorization))
	{
	return AuthenticateResult.NoResult();
	}
	if (authorization.StartsWith("Bearer ", StringComparison.OrdinalIgnoreCase))
	{
	token = authorization.Substring("Bearer ".Length).Trim();
	}

Security/src/Microsoft.AspNetCore.Authentication.JwtBearer/JwtBearerHandler.cs

Line 148 in 0f18ff1

new AuthenticationToken { Name = "access_token", Value = token }

[HaoK]

I was just going by the description in the bug above: "When using JwtBearer auth, result.Properties is never set - however, the properties that are expected to be there are present on result.Ticket"

[HaoK]

Assuming that statement is correct, its there just in the wrong place

[HaoK]

RE how common this is @ajcvickers this bug has been there since 2.0, its not new in 2.1, so this is the first we've seen it, so its pretty uncommon I guess

[Tratcher]

No, I debugged it. Part of the trouble is that the ticket gets re-created and loses the Properties from the first ticket.

[HaoK]

Actually rereading the OP, this behavior is new to 2.1, let me see what changed in 2.1

[HaoK]

Can you try debugging this on 2.0.4 @Tratcher the code path looks the same to me in https://github.com/aspnet/Security/blob/2.0.4/src/Microsoft.AspNetCore.Authentication/RemoteAuthenticationResult.cs doesn't look like we changed anything other than rename the file and you tweaked the exception/error methods to include the properties

[TimHess]

For what this background is worth, this is the application I'm working on right now: https://github.com/SteeltoeOSS/Samples/tree/dev/FreddysBBQ

It uses Steeltoe Security for doing JWT on Cloud Foundry, via Microsoft.AspNetCore.Authentication.JwtBearer

I'm in the process of making sure that Steeltoe works with [asp]netcore 2.1, so the changes I'm making are almost exclusively TargetFramework and NuGet dependencies.

The code that's breaking is in GetClientAsync() inside this file https://github.com/SteeltoeOSS/Samples/blob/dev/FreddysBBQ/src/Common/Services/AbstractService.cs#L142

The scenario is that of multiple microservices, where the user's credentials are passed down the chain (in this case a customer site calls an order service which then calls a menu service)

[Tratcher]

Yes this worked in 2.0. Here's what broke it:
aspnet/HttpAbstractions@321639b#diff-7e92f7940da0e821fc452fc9950db334
AuthentiateResult.Properties change from a pass through to a local field.

[HaoK]

Ah cool, I actually looked at the PR...just in the wrong repo doh...I looked at the security half forgot there was a matching one in Http doh

[TimHess]

I was able to confirm that pulling the token out of the auth header from the incoming request works fine in my case, so I'm a lot less worried about the extension not working in 2.1.0. We don't depend on GetTokenAsync for anything other than this one sample, so we should be fine.

Thanks again!

[avaisane]

If AuthenticationProperties are not saved in cookie (e.g access_token) and/or able to get those after authentication/challenge flow, then it is a deal breaker for me. You should at least give some example how to store this to enable access it on coming requests.

[Tratcher]

@avaisane JwtBearer never did store the token in a cookie. The Auth header is sent on every request and GetTokenAsync is only a shortcut for reading the header. You can read the auth header directly.

[avaisane]

@Tratcher. Sorry, I shoild have been more precise. We are using openidconnect for web app and it is having similar problem, because accesstoken is not in place when calling GetToken. This worked on 2.0 version. For jwt bearertoken api's workaround is fine, because access_token exists on every request.

[Tratcher]

Ah, have you reproduce this with OIDC? This regression happened a year ago and I've definitely used that code path in OIDC since then...

[Tratcher]

Hmm, GetTokens works fine in the 2.1 branch in the OIDC sample. https://github.com/aspnet/Security/blob/dev/samples/OpenIdConnectSample/Startup.cs

[avaisane]

@Tratcher. Thats correct, oidc is having similar phenomenon. After updating to .net core 2.1.300 functionality break up. ma 4. kesäk. 2018 klo 20.45 Chris Ross <[email protected]> kirjoitti:

…

Ah, have you reproduce this with OIDC? This regression happened a year ago and I've definitely used that code path in OIDC since then... — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1765 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABxtIciMga2odb6BTlMvl9ZpIUTwGnrqks5t5XJGgaJpZM4UJaia> .

[Tratcher]

How are you using it? The sample linked above is working fine.

[avaisane]

@Tratcher Goosh, we did broke our own code. We implemented the header trick mentioned above into our shared accesstokenfetcher componed, which was used by APIs and Web App. Thus it bypassed previous httpContextAccessor.httpContext.GetTokenAsync() 80) Thanks anyway for you assistance!

[profet23]

This just caused me to waste a day trying to figure out why our token authentication was failing... any chance at a hot fix release of 2.1 with this fix?

[Tratcher]

@muratg for patch triage:
+This is a regression from 2.0.
-The workaround is trivial.

[muratg]

@Tratcher Could you file a bug for the patch candidate? Thanks.

[Tratcher]

Filed #1809

[plazav]

@Tratcher, even after updating to 2.2.0-preview2-35157 I still can reproduce this issue.
I am trying this example: active-directory-aspnetcore-webapp-openidconnect-v2

[Tratcher]

@plazav Comments on closed issues are not tracked, please open a new issue with the details for your scenario.

Also, this issue never affected OpenIdConnect, only JwtBearer.