Move AuthZ policy types back into Policy and rejigger AddAuthorization

[HaoK]

Old AddAuthorization => AddAuthorizationCore() [In base AuthZ package)
New AddAuthorization() [In Policy package, calls AddPolicyEvaluator and AddAuthorizationCore)
base AuthZ now targets netstandard 2.0

[rynowak]

This looks roughly like what I expected to see 😆 - looks like there's still some stuff to update.

[HaoK]

So looks like there as a new IAllowAnonymous type that was added to Http Abstractions, can we flip this around and have this type live in Auth Z and have Http.Abstractions depend on AuthZ?

https://github.com/aspnet/AspNetCore/blob/84da613d2c03b6f1c0fa3c01828923ec3415d525/src/Http/Http.Abstractions/src/IAllowAnonymous.cs

[HaoK]

Tests look green finally, making this a real PR instead of Draft

[rynowak]

This makes me go hmmm. Is this because of the attributes? What in this project is using them?

[HaoK]

Yeah we moved IAuthorizeData and IAllowAnonymous back to Authorization

[rynowak]

What depends on those types in Http.Abstractions? Was it like this before?

@davidfowl @Tratcher do we want this dependency here?

[HaoK]

I think it comes from routing: https://github.com/aspnet/AspNetCore/blob/23efa15112ae2e7c9bf26e75f7173463b9d9a056/src/Http/Routing/src/EndpointMiddleware.cs#L45

[Tratcher]

I'd want to see the graph. Dependencies don't matter as much as the used to, but we still need to avoid a rats nest.

[HaoK]

AuthZ doesn't bring much, and nothing new, it just adds Options/Logging.Abstractions which are already referenced

[rynowak]

That would dictate routing referencing auth though, not http.abstractions

[HaoK]

I'll try moving the ref to routing

[Tratcher]

Much better

[davidfowl]

So looks like there as a new IAllowAnonymous type that was added to Http Abstractions, can we flip this around and have this type live in Auth Z and have Http.Abstractions depend on AuthZ?

I have to look in more detail but at first glance this doesn’t seem like a good idea. I’d like to keep the HttpAbstractions assembly near the bottom of the dependency graph

[davidfowl]

Feels like there should be an auth abstractions assembly no?

[Tratcher]

@davidfowl why bother with new assemblies when everything is in the shared runtime together?

[davidfowl]

@davidfowl why bother with new assemblies when everything is in the shared runtime together?

Future proofing. I've heard this sentiment from @rynowak as well but we should not take how we ship today as a sign that we want to remove assembly boundaries unless it makes sense to do so from a layering POV. As far as I see it, we're not going to a place where we have a single assembly even though we ship that way for the most part (when using the shared framework). There are many benefits of having actual boundaries generally:

• It helps to validate dependency boundaries
• It might make assembly trimming easier (when everything can't just reference everything)
• It's part of future proofing (we ship a shared framework and version everything together right now but we may not always ship this way).

TL;DR I don't want us to think we can't add new assemblies just because we're shipping a shared framework. If it's the "right thing to do" then lets just do it.

[rynowak]

TL;DR I don't want us to think we can't add new assemblies just because we're shipping a shared framework. If it's the "right thing to do" then lets just do it.

The problem is that the right thing to do is usually in the context of reacting to a single change or in the context of today's requirements. It makes it hard to judge. To a certain extent in this area we have to try and anticipate our future requirements OR just try and decouple things to the max.

---

Today we're responding to the requirement of needing to decouple the auth attributes from HTTP.

I see a few different options:

• Create an Microsoft.AspNetCore.Authorization.Metadata assembly for these attributes to live in
• Create an Microsoft.AspNetCore.Metadata assembly for any metadata attributes that belong at the bottom of our layering like CORS or Auth. These types are just metadata and use simple types without coupling to an implementation
• Get rid of the dependency between routing and auth, and use an analyzer for this

I didn't call these .Abstractions because I think that's a different thing.

---

Option 1 or 2 both go after the idea that we want to make loosely coupled systems that can interoperate with declarative metadata. Put another way, there's no conflict with having multiple systems that care about the same data.

Option 1 means that we'd do this as a tactical thing for solving this particular problem, and we could do it again in some other area if we choose.

Option 2 means that we'd want to make this choice in other areas - and we'd start designing for it. We'd end up with a single assembly for our top-level metadata rather than a few different ones.

Option 3 would be to punt on this. The only reason that auth and routing are together today is because we wanted some kind of safeguard in place for our security-related scenarios.

---

I've already discussed option 2 in the past with @davidfowl so I know that he's not sold on it 😆

[halter73]

Create an Microsoft.AspNetCore.Metadata assembly

I kinda like the sound of option 2. At least that makes it extremely obvious where any metadata attributes should go.

[HaoK]

Tests are all green now, should I merge and file a tracking issue for the final home of where these attributes should live?

[dougbu]

base AuthZ now targets netstandard 2.0

What's driving this part of the change?

[rynowak]

What's driving this part of the change?

We need to use it in Blazor.

[natemcmaster]

Remind me again…why doesn’t Blazor work for APIs that target netcoreapp3.0?

[rynowak]

WebAssembly is netstandard2.0. YES it is.

This is probably a point-in-time thing before .NET 5 - but we need to keep shipping releases of the wasm profile of Blazor that align with 3.0

[HaoK]

@aspnet-hello triage https://dev.azure.com/dnceng/public/_build/results?buildId=191784

[aspnet-hello]

This comment was made automatically. If there is a problem contact [email protected].

I've triaged the above build. I've created/commented on the following issue(s)
https://github.com/aspnet/AspNetCore-Internal/issues/2479
https://github.com/aspnet/AspNetCore-Internal/issues/2329
https://github.com/aspnet/AspNetCore-Internal/issues/2465
https://github.com/aspnet/AspNetCore-Internal/issues/2408
https://github.com/aspnet/AspNetCore-Internal/issues/2348
https://github.com/aspnet/AspNetCore-Internal/issues/2422

[SteveSandersonMS]

Does this make policy evaluation runnable on netstandard2.0? It looks like, unlike the proposal here, the policy evaluation is still in a netcoreapp3.0 assembly after this change, so wouldn't be usable in WebAssembly.

In case it's unclear, we do want client-side Blazor to have the same notion of policies as server-side ASP.NET Core. The only part that would be different is that, on the client, there is no HttpContext hence pulling out the non-HttpContext-attached parts into an underlying layer in this proposal.

I don't mind doing a follow-up PR to split the policy implementation in two, but that only makes sense if we have the right new assembly to put the shared (netstandard2.0+netcoreapp3.0) code, and it doesn't look like the new Metadata assembly is the right place for it given the description in the PR.

Marking as "request changes" just to be sure we talk about this before this is merged. Hope that's OK!

[HaoK]

@SteveSandersonMS you don't actually need a policy evaluator, that's the thing that binds Http authentication to authorization. For blazor, you should be able to directly use the AuthZ core package that is targeting netstandard2.0 to evaluate policies by using IAuthorizationService

[HaoK]

The naming isn't great, but basically the policy package was added as a way for SignalR and MVC to reuse similar logic that was doing http authentication and authorization, its really closer to Authorization.Http conceptually. If all you need is authorization and policies, that is all possible via the extension methods like:

https://github.com/aspnet/AspNetCore/blob/master/src/Security/Authorization/Core/src/AuthorizationServiceExtensions.cs#L103

[SteveSandersonMS]

Thanks for clarifying, @HaoK! That makes sense now.

You're right - I just tried out the code in this PR and was able to define and run policies under WebAssembly.