SaveTokenAsClaim for JwtBearer

[leastprivilege]

For consistency - the JwtBearer MW should also have a SaveTokenAsClaim setting - this would make it easier to get to the token e.g. for contacting user info in claims transformation - or "delegation"

[leastprivilege]

it probably should go on AuthenticationOptions and not RemoteAuthenticationOptions

[Tratcher]

It wouldn't belong on the base AuthenticationOptions because it wouldn't apply to things like Cookies, Basic, etc.. If we do this I'd just duplicate the property for JWT rather than moving it up to AuthOptions.

[Eilon]

We're considering putting the data into the AuthenticationProperties instead of as claims. Would that make sense here?

[leastprivilege]

I thought this was meant to be a convenient way to get to the access token without having to implement an event. The token should be available as a property (or on the protocol message in OIDC) anyways.

[Tratcher]

The auth properties flow with the ClaimsPrincipal all the way back to the app, and through the cookie.

[kevinchalet]

@Eilon @Tratcher when adding SaveTokensAsClaims to the OIDC middleware, I had considered using AuthenticationProperties, as it was the best candidate for that. Sadly, retrieving authentication properties is much harder than retrieving a simple claim (not to mention that it requires using a very strange syntax).

var token = context.User.FindFirst("access_token")?.Value;

var context = new AuthenticateContext("[authentication scheme]");
await context.Authentication.AuthenticateAsync(context);

if (context.Principal == null || context.Properties == null) {
    throw new InvalidOperationException("The request is not authenticated.");
}

string token;
context.Properties.TryGetValue("access_token", out token);

[Eilon]

Putting on Backlog for now. We will revisit the popular suggestion at a later time. (Popular == not the one I made 😄 )

[Tratcher]

@vibronet recommends cutting SaveTokensAsClaims outright on the grounds that the ClaimsPrincipal (and the auth properties) go into the session cookie and that's the wrong place to save this kind of information. Moreover the lifetimes of the access and refresh tokens can be fundamentally different from the login session. Finally if there are additional tokens acquired later they would have to be handled separately. The recommended way to handle the tokens is to get them via the events and store them independently.

[kevinchalet]

I disagree.

SaveTokensAsClaims is far from being perfect, but it's an extremely convenient option to make API requests after an authentication/authorization flow (IIRC, @Eilon used it in one of his projects 😄).

Removing this option would also make retrieving the access/identity/refresh tokens from an AccountController much harder (extracting the access/refresh tokens from ExternalLoginInfo to store them in a database became a quite popular pattern).

[brockallen]

I disagree.

+1

[vibronet]

It might be convenient, but it is fundamentally broken. The identity token is retrievable from a controller via SaveSignInToken if you need it. The access and refresh token don't belong to a session, as they have completely different lifetimes and uses. Why should an expired access token linger in the ClaimsPrincipal after it is long expired? Why should a refresh token be buried in something tied to a web session when in fact it represents a consent that should be available regardless of how one access the app (think granting consent for a web app to access an API, then accessing the app through a mobile client - I should not have to get a brand new refresh token). Why should a developer have the access and refresh token acquired at sign in time handled differently than the ones acquired after sign in via normal OAuth2 code flow?
The middlewares as they exist today handle sign in/identity verificatin. Access token and refresh token acquisition at the same time as sign in are an optimization to save roundrips, not fundamental changes in the nature of a sign in. I 100% agree that we need a good mechanism to make those tokens available to the app, because that's where they are needed - but per the above treating the claims collection as a property bag is not the cleanest design IMO. Can we come out with some other place we could save those tokens, so that they are available to the app without introducing asymmetries and special cases?

[leastprivilege]

I think this is all a bit mixed up here -

this issue is about the JwtBearer middleware -

my point was mainly that there needs to be an easy way to get to the access token from within the API. You might need it in claims transformation (e.g. for the userinfo endpoint) or for delegation.
Since this token can potentially come from different source (authZ header, query string etc) - the Jwt middleware should make it available somehow.

Now if this is a claim - or some other well defined (and consistent) place that is easy to reach - I don't care.

Now for APIs that is easy since there is no session involved.

Generally speaking I agree with Kevin - SavenTokensAsClaims is not perfect but is exactly what most people end up doing anyways - tuck away the tokens in the cookie so they can get to it for calling an API, renewing and logout hints.

It doesn't necessarily need to be the claims collection - but not sure we have the time to invent another abstraction.