Authorization middleware

[JamesNK]

https://github.com/aspnet/Routing/issues/537

Will add tests in a different PR

[JamesNK]

Although the logic in the authz middleware is very similar to the authz filter, they are different. For example, how each gets the policy, how each denies unauthorized requests.

I don't think there is anything to share between them that not is already shared.

[HaoK]

Code looks fine, do we want consider just having this live in the Authorization.Policy package that the evaluator lives in? There isn't very much in that package today, and since this middleware depends on that, it doesn't seem like a terrible fit...

[JamesNK]

I chose not to because Authorization.Policy has no dependency on Http, and the namespace for every file is Authorization.Policy. It feels like an odd fit to put middleware in it.

[HaoK]

It does though since I created it basically to be able to use Authorization and Http together, the reason you don't see http in the namespaces, is due to authentication not having it even though its lives in httpabstractions:

https://github.com/aspnet/HttpAbstractions/tree/master/src/Microsoft.AspNetCore.Authentication.Abstractions

Also for example, the main PolicyEvaluator AuthenticateAsync method takes httpContext as a parameter:

https://github.com/aspnet/HttpAbstractions/tree/master/src/Microsoft.AspNetCore.Authentication.Abstractions

[JamesNK]

Ah ok. I didn't follow down the transitive dependencies.

That makes me feel better about it. I still think it is an odd fit but happy to be outvoted. Its not a hill I'll die on 😄

[Tratcher]

I'll read through this tomorrow, but I do want you to try to replace this sample as an acceptance test.

[Tratcher]

https://github.com/aspnet/Security/issues/1734

[JamesNK]

🆙 📅

The remaining ask is a policy that runs even if there is no endpoint matched, or there is no IAuthorizeData on the endpoint.

Authorization today has the concept of DefaultPolicy, but that refers to the policy used when there is IAuthorizeData with no policy name on an endpoint.

What about adding the idea of GlobalPolicy? It runs on all requests that pass through AuthorizationMiddleware (and should add it to the MVC authz filter for consistency). If the request has an endpoint with policies then they are evaluated together with the global policy. The default GlobalPolicy is no op.

Questions:

• Does GlobalPolicy live on AuthorizationOptions?
• If the policy lives on AuthorizationOptions, should it be passed as an argument to the middleware? Will allow different globals in different branches
• If AuthorizationOptions is passed to the middleware, should ALL the options on it be local to the route? That could allow "Policy1" to mean one thing in one instance of the middleware, and something else in another instance.

Alternative suggestions welcome!

[HaoK]

Just to clarify the meaning of GlobalPolicy, this is basically a required policy that always must be satisfied, as compared to DefaultPolicy which is only used if no policy is specified... If that's the case I think I prefer a name closer to RequiredPolicy since its always applied no matter what?

[HaoK]

Also if we want to introduce something like this, maybe it would make more sense to bake in at the lowest level so imperative calls to IAuthorizationService.Authorize would pick it up as well, instead of something that only middleware/endpoints could take advantage of (this would align with exposing it in AuthorizationOptions)

[HaoK]

Basically the default authZ service implementation would just always include the requirements from the Global/Required policy in addition to any other requirements passed in for evaluation.

[JamesNK]

If that's the case I think I prefer a name closer to RequiredPolicy since its always applied no matter what?

Ok!

Also if we want to introduce something like this, maybe it would make more sense to bake in at the lowest level so imperative calls to IAuthorizationService.Authorize would pick it up as well

Good idea! I think it would feel cleanest if it was exposed from IAuthorizationPolicyProvider: IAuthorizationPolicyProvider.GetRequiredPolicyAsync(). The middleware already takes a policy provider so customizing the required policy per middleware would be simple enough. On the other hand it would be a breaking change for 3.0. The alternative option is to read it directly from AuthorizationOptions.

[HaoK]

That's what major versions changes are for :) Yeah I agree it should just behave just like DefaultPolicy so it should get a method on the policy provider and we can just have the default policy provider read it from the options just like with the default policy.

[JamesNK]

Updated with RequiredPolicy + tests.

[JamesNK]

@natemcmaster @ryanbrandenburg Hi. Could one of y'all enable feature branch builds for https://dotnet.myget.org/feed/aspnetcore-dev/package/nuget/Microsoft.AspNetCore.Authorization.Policy ?

I'd like to use a package over here: https://github.com/aspnet/AuthSamples

[natemcmaster]

Could one of y'all enable feature branch builds for

done

[JamesNK]

Two open questions to resolve:

1. Should there be a UseAuthorization overload that takes AuthorizationOptions? And if so, should the options completely replace the options configured globally?
2. Do we still want RequiredPolicy as a new concept? I have updated the static files auth sample so that it is no longer nessessary and I think the new approach is better.

I don't have a strong opinion either way about adding or removing these.

[HaoK]

If you don't need either of these anymore, maybe leaving them out to start is best. AuthenticationMiddleware is analogous to the AuthZMiddleware with its options + scheme provider and it doesn't have an overload that lets you replace the options, also its pretty easy to implement required policy in a custom IAuthorizationService even if we don't bake it in.

[JamesNK]

🆙 📅