Copy access token scopes from the ticket properties to the user claims

[kevinchalet]

The introspection middleware will also need to be updated to store the audiences extracted from the introspection response in the final authentication ticket.

[MonkeyJamboree]

#18 should store the audiences in the ticket properties to help facilitate this.

[MonkeyJamboree]

Just wondering - what's the reason for copying scopes to the user claims?

[kevinchalet]

Just wondering - what's the reason for copying scopes to the user claims?

Making them accessible from the authorization block, which is claims-based (the authentication properties are not flowed). Doing that allows creating scope-restricted policies: https://github.com/aspnet-contrib/AspNet.Security.OpenIdConnect.Samples/blob/master/samples/Mvc/Mvc.Server/Startup.cs#L31-L38.

FWIW, I'm not a huge fan of mixing "user attributes" and "protocol claims" (that's why I liked SAML, which made the separation cleaner), but that's exactly how JWT work and what it encourages.

The JWT middleware adds the scopes in the ClaimsPrincipal, so I think it makes sense to do a similar thing in the validation middleware, by copying the scopes from the properties to the claims.

[MonkeyJamboree]

I suppose that makes sense, though maybe that's more of an underlying problem with the authorization block. Where does AddPolicy come from, exactly? I wasn't able to find it from quick glances at the aspnet/Security or aspnet/Identity repos and I'd like to take a look at the code for the API.

After thinking it over more, I suppose it makes sense to be in-line with other middleware for consistency, and ClaimsPrincipal does seem to mostly flow along with how JWT works (everything is a claim).

The JWT middleware likely follows that approach because of the design of JWT rather than some other reason. When it comes to OpenID Connect and OAuth, there isn't anything that dictates the token formats.

Just some ruminations and thoughts here about how it's all being handled at present, really.

[kevinchalet]

I suppose that makes sense, though maybe that's more of an underlying problem with the authorization block.

Agreed. Sadly, it's very unlikely to change, unless you find a way to convince @blowdart 😄

Where does AddPolicy come from, exactly? I wasn't able to find it from quick glances at the aspnet/Security or aspnet/Identity repos and I'd like to take a look at the code for the API.

https://github.com/aspnet/Security/blob/dev/src/Microsoft.AspNetCore.Authorization/AuthorizationOptions.cs#L26

[MonkeyJamboree]

After looking at the authorization block, it actually seems to be mostly sufficiently abstracted out. The major problem appears to be that IAuthorizationService and DefaultAuthorizationService require a ClaimsPrincipal instead of something more all-encompassing like a HttpContext or an AuthenticationTicket.

Until it's changed, I guess we'll just have to deal with ClaimsPrincipal or something, despite the underlying stuff being well-abstracted such that you shouldn't need one 👎

[blowdart]

It's not going to change. We wouldn't return an HttpContext because, well, it's not limited to Http. We used to return a ticket, but we had lots of requests to change it as well.

[MonkeyJamboree]

@blowdart what are you talking about? The Authorization block returns boolean values based on decisions made from the requirements. The requirements themselves should have the dependencies (such as a ClaimsPrincipal or list of role strings or HttpContext), not the entire Authorization block. Perhaps generics could help in this regard instead of tying to any one specific dependency for the IAuthorizationService and the handlers?

Also, I believe the place to debate this would be on aspnet/Security#785 but since you replied here I thought I would as well.

[kevinchalet]

Done: 6442dab.