Strawman for new authorization packages that can be shared between server and client #9997
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… server-side ASP.NET Core.
SteveSandersonMS
added
area-mvc
HaoK
reviewed
May 6, 2019
|
|
||
| namespace Microsoft.AspNetCore.Authorization.Policy | ||
| { | ||
| public class CommonPolicyEvaluator : ICommonPolicyEvaluator |
There was a problem hiding this comment.
Do we still need a PolicyEvaluator in the common layer? It was only introduced to tie authN + authZ together, I thought on the client side there's no authentication needed, so wouldn't the base IAuthorizationService be sufficient?
There was a problem hiding this comment.
Yes, we do want to evaluate policies in client-side code.
I thought on the client side there's no authentication needed
That's not correct. Although authentication isn't enforced there (all true enforcement is server-side), we still want policies to exist as a way of informing the client about what the user is going to be allowed to do.
|
We seem to use Core everywhere, maybe Authorization.Core instead? |
|
Superseded by #10021. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Our goal is that the same authorization attributes, DI services, and classes should be usable both in server-side ASP.NET Core (without any changes to customer code) and in client-side Blazor.
To make that possible, we have to introduce at least one new package where we can put things that should be common to both server and client. Restrictions include:
netstandard2.0Http.Abstractions(orHttpContextin particular)This PR moves the majority of classes out of
Microsoft.AspNetCore.Authorizationand puts them in a new package,Microsoft.AspNetCore.Authorization.Common. I'm not in any way in love with that name. I'm happy to call it anything else if someone has a preference. It just has to mean "authorization things that work both on the server and on the client".This shouldn't be a breaking change, since
Microsoft.AspNetCore.Authorizationnow references the.Commonpackage, so all existing types continue to work (if you rebuild your app). I haven't set up actual type forwarders for people who reference things using reflection. Not sure if we would do that. I also haven't moved any tests since this is just a draft.Similarly I created
Microsoft.AspNetCore.Authorization.Common.Abstractionsto hold the two interfaces we need that used to be inHttp.Abstractions.I haven't built anything end-to-end with this yet so don't even know if it meets our needs. Currently this is just seeking feedback on (a) the whole idea of moving all these things around, and (b) the package naming.