Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Future AuthZ improvements list #4670

Open
HaoK opened this issue Aug 10, 2017 · 46 comments
Open

Future AuthZ improvements list #4670

HaoK opened this issue Aug 10, 2017 · 46 comments
Labels
affected-medium This issue impacts approximately half of our customers area-auth Includes: Authn, Authz, OAuth, OIDC, Bearer enhancement This issue represents an ask for new feature or an enhancement to an existing one Needs: Design This issue requires design work before implementating. severity-major This label is used by an internal tool
Milestone

Comments

@HaoK
Copy link
Member

HaoK commented Aug 10, 2017

This issue will track all the various Authorization improvements we are looking at for 2.1.

Some initial thoughts:

From @davidfowl

We've had a bunch of feedback about our authz system with respect to flowing context from the authorize attribute to the authorization handler:

Today the Authorize attribute supports has enough metadata to describe the policy but it doesn't let you specify the resource (via IAuthorizeData). I think if we solve that, it might solve a bulk of the issues since people will be able to write custom attributes that flow the relevant context to the authorization handler.

Today that's only possible when doing imperative authz. I was thinking something like:

public interface IAuthorizeResource
{
    object Resource { get; }
}

If the attribute implemented this, we would flow that as the resource to the handler. This way you implement permissions of whatever you please via resources.


From @rynowak

Look into providing overriding semantics in MVC, maybe a marker interface: IAuthorizeMetadata. Any attributes that implement that interface on the endpoint could be flowed. Then it’s up to developers to build whatever they want. (Also look into flowing single objects vs many objects + requirements VS resources)

We will also look into making it possible to specify requirements via Attributes similar to imperative AuthZ so you don't have to preconstruct policies for attributes.

Misc other improvements:

@HaoK HaoK self-assigned this Aug 10, 2017
@rynowak
Copy link
Member

rynowak commented Aug 23, 2017

My additional thoughts on this. We're looking at making the concept of an endpoint available at a low level from the dispatcher. Endpoints will have arbitrary metadata in an ordered list. This is the equivalent to what MVC does today by passing the action descriptor.

We'll probably also have a concept of active metadata which are metadata items that have been hydrated with the request context.

These might be useful/necessary building blocks for these things.


Additionally in the case of MVC, the current [Authorize] has some surprising behaviors compared to other filters. All authorize filters will execute rather than supporting overriding. Additionally, [AllowAnonymous] cannot be overridden and turns off all authorize attributes. These might be consistent with the past, but they don't really fit with the zen of MVC filters.

@HaoK
Copy link
Member Author

HaoK commented Aug 24, 2017

Interesting, well if we have a new endpoint concept with a clean slate for metadata, we should probably just expose all of the basic building blocks

  1. Building the ClaimsPrincipal, need to end up with 0 or more authentication schemes. If none are specified, we continue to use the default user aka httpContext.User, otherwise we build a merged user via authenticating all of the schemes. We could manifest this with a series of [Authenticate(scheme)] attributes.

  2. Specifying the Authorization requirements, today the metadata only targets policies, but we could have [Authorize(typeof(MyRequirement), typeof(MyOtherRequirement)]

Some examples:

[Authorize]

Would behave the same as today (using some default policy which relies on context.User, and still only does RequireAuthenticatedUser by default

[Authenticate("Bearer")]
[Authorize]

Would use the Bearer scheme and only require that it exists.

[Authenticate("Bearer", "Cookie")]
[Authorize(typeof(RequireAdminClaim)]

Would use the merged Bearer Cookie schemes and require an admin claim.

@Tratcher
Copy link
Member

Which challenge would you issue?

@HaoK
Copy link
Member Author

HaoK commented Aug 24, 2017

All of them, its not any different than today

@Tratcher
Copy link
Member

Challenging both Bearer and Cookie is nonsensical.

@HaoK
Copy link
Member Author

HaoK commented Aug 24, 2017

Not disagreeing, but today if you have a policy that has Bearer and Cookies, that's what it does today as well

@HaoK
Copy link
Member Author

HaoK commented Aug 24, 2017

Unless we want to eliminate being able to specify multiple schemes entirely... that's an option that would simplify things...

@davidfowl
Copy link
Member

Not disagreeing, but today if you have a policy that has Bearer and Cookies, that's what it does today as well

It does? What happens as a result? Don't they stomp on each other?

@HaoK
Copy link
Member Author

HaoK commented Aug 24, 2017

Yeah they do, last one wins, multiple auth schemes in policies has always been a bit weird

@Tratcher
Copy link
Member

Some are compatible like Bearer, Basic, and Windows.

@Eilon
Copy link
Member

Eilon commented Feb 8, 2018

@HaoK please log new issues for any 2.1.0 proposals.

@Eilon Eilon unassigned HaoK Feb 8, 2018
@HaoK HaoK changed the title 2.1 AuthZ improvements 2.2 AuthZ improvements Feb 9, 2018
@hmvs
Copy link

hmvs commented May 14, 2018

@HaoK Is it going to be a part of 2.1 release?

@hmvs
Copy link

hmvs commented May 14, 2018

I want to have ability to return custom json if specific policy failed. for example
403
{
"errorType": "PrivalcyPolicyAcceptRequired",
"errorMessage": "You token doesn't have PolicyAccepted claim"
}

@HaoK
Copy link
Member Author

HaoK commented May 14, 2018

No the error authZ improvements didn't make it into 2.1, I'll try to get them in 2.2

@spottedmahn
Copy link

I assume that is why the title changed to 2.2 😜⁉

@HaoK
Copy link
Member Author

HaoK commented May 14, 2018

Yup, I've got some cycles this week so I'll try to prototype something

@HaoK
Copy link
Member Author

HaoK commented May 14, 2018

So rough idea of some initial AuthZ improvements I've got at the top of my head for 2.2

  • For errors, maybe some kind of scoped IAuthorizationErrorService, that the authZ services will report errors to with appropriate context. This should enable apps to write an appropriate detailed authorization error.

  • UseAuthorize("policyName") - middleware equivalent of [Authorize("policyName")], would behave similarly, and also allow sideffect of setting httpContext.User if AuthenticationScheme is specified in the policy. TBD on what to do for failure, redirect to some access denied page which is able to display the appropriate context seems ideal.

@davidfowl
Copy link
Member

I think we should close this issue and revisit this in 2.2 planning so we can scope it down.

@HaoK
Copy link
Member Author

HaoK commented May 14, 2018

Close or leave it open for planning?

@hmvs
Copy link

hmvs commented May 15, 2018

Have you considered adding some way how we can access the list of policies which is run and which is failed. Also I thought it could be a helpful to have overload of method context.Fail(); with some object which can be accessed later.

@natelaff
Copy link

Is this what currently prevents something such as the ability to show a specific page when a specific policy fails. For example, when a policy fails, redirect to a page that shows how to get that particular policy? (i.e. paying for a new membership level).

@Eilon Eilon changed the title 2.2 AuthZ improvements Future AuthZ improvements list Jul 17, 2018
@aspnet-hello aspnet-hello transferred this issue from aspnet/Security Dec 13, 2018
@aspnet-hello aspnet-hello added this to the 3.0.0-preview2 milestone Dec 13, 2018
@aspnet-hello aspnet-hello added area-auth Includes: Authn, Authz, OAuth, OIDC, Bearer enhancement This issue represents an ask for new feature or an enhancement to an existing one Needs: Design This issue requires design work before implementating. labels Dec 13, 2018
@JamesNK
Copy link
Member

JamesNK commented Dec 17, 2018

@HaoK @rynowak Yo, I think we can close this now that we have AuthorizationMiddleware + endpoints + AuthorizeAttribute. Is there anything left to do?

@HaoK
Copy link
Member Author

HaoK commented Dec 17, 2018

This is currently also serving as tracking several other asks we haven't done yet for AuthZ:

We moved this into backlog in triage last week so I think its safe to just leave this parked there for now

@RamunasAdamonis
Copy link

Is there any progress on custom failure message? Or any official workaround?

@rsc82
Copy link

rsc82 commented May 15, 2019

I too wonder about how to get a custom json body when ForbidResult is called.

Is there any way to get at the AuthorizationResult(s) later in the pipeline? As far as I can tell it gets swallowed as part of the PolicyEvaluator (line 84 where it calls authorizationService). The following works inside the controller action but it completely bypasses the filters and I really don't want to go this route.

//ControllerAction
public async Task<IActionResult> GetSomeThing() {
  var a = await this._authorizationService.AuthorizeAsync(User, "MyPolicy");
  if (!a.Succeeded)
  {
    return this.StatusCode(403, a.Failure.FailedRequirements);
  }
}

Just putting this out there as part of the conversation, I came across the following issue and being able to map the AuthorizationResult (and FailedRequirements) to a ProblemDetail would be amazing.
#10120

@bugnuker
Copy link

Ping! Any progress on these issues in this ticket?

I am looking for a solution to obtain the reason an AuthorizationPolicy failed, as in, what Claim or scope was missing or whatever the reason might be for the HTTP 403 when access is denied. I would like to provide a JSON body in the 403 reply with said information.

@reddogaw
Copy link

@bugnuker As far as I can tell, there's still no support for specific "reasons" why an authorization is marked as Forbidden. However, if your system is relatively simple (e.g. mine is mostly just role based), then you could write a PolicyEvaluator (as mentioned above) and look up the existing policy.Requirements to attach detail the redirection. For example:

        private class RedirectingPolicyEvaluator : PolicyEvaluator
        {
            internal static object ItemsKey = new object();

            public RedirectingPolicyEvaluator(IAuthorizationService authorization) : base(authorization)
            {
            }

            public override async Task<PolicyAuthorizationResult> AuthorizeAsync(AuthorizationPolicy policy, AuthenticateResult authenticationResult, HttpContext context, object resource)
            {
                var result = await base.AuthorizeAsync(policy, authenticationResult, context, resource);
                if (result.Forbidden && policy.Requirements.OfType<RolesAuthorizationRequirement>().Any())
                {
                    // If user is authenticated but not allowed, send them to a special error page
                    if (authenticationResult.Succeeded)
                    {
                        // Find all the role name that are required
                        var rolesRequired = policy.Requirements.OfType<RolesAuthorizationRequirement>().SelectMany(r => r.AllowedRoles).Where(role => !authenticationResult.Principal.IsInRole(role));

                        // Get the URL to the help page, with a return url that's appropriate to the request type
                        var urlHelper = context.RequestServices.GetRequiredService<IUrlHelper>();
                        var returnUrl = context.Request.GetEncodedUrl();
                        if (context.Request.IsAjaxRequest())
                        {
                            returnUrl = FindAjaxRequestReturnUrl(context, urlHelper);
                        }
                        var redirectTo = urlHelper.AbsoluteAction(nameof(HomeController.Unauthorized), "Home", new { ReturnUrl = returnUrl, RolesRequired = rolesRequired.ToArray() });

                        // Store an item in the context
                        context.Items[ItemsKey] = new ForbiddenRequestDetails { RedirectTo = redirectTo, RolesRequired = rolesRequired.ToArray() };
                    }
                }
                return result;
            }
        }

Then I wrapped/adapted the IAuthenticationService interface to override what happens for ForbidAsync in particular by looking for that ForbiddenRequestDetails in the context items:

        private class RedirectingAuthenticationService : IAuthenticationService
        {
            private readonly IAuthenticationService _adaptee;

            public RedirectingAuthenticationService(IAuthenticationService adaptee)
            {
                _adaptee = adaptee;
            }

            .... Other methods removed ...

            public Task ForbidAsync(HttpContext context, string scheme, AuthenticationProperties properties)
            {
                if (context.Items.ContainsKey(RedirectingPolicyEvaluator.ItemsKey))
                {
                    var options = context.Items[RedirectingPolicyEvaluator.ItemsKey] as ForbiddenRequestDetails;
                    var isApi = context.Request.IsAjaxRequest();
                    if (!isApi)
                    {
                        // Redirect the user
                        context.Response.Redirect(options.RedirectTo);
                        return Task.CompletedTask;
                    }
                    else
                    {
                        // Just write some extra stuff to the content body to make it easier
                        var routeData = context.GetRouteData();
                        var actionDescriptor = new ActionDescriptor();
                        var actionContext = new ActionContext(context, routeData, actionDescriptor);
                        var actionResult = new JsonResult(options) { StatusCode = (int)HttpStatusCode.Forbidden };
                        return actionResult.ExecuteResultAsync(actionContext);
                    }
                }
                return _adaptee.ForbidAsync(context, scheme, properties);
            }
     }

@TheTechArch
Copy link

TheTechArch commented Oct 11, 2019

We are also building a Asp.Net Web application with api controllers with Attribute based authorization and need a way to returm some additional information about the reason why a request was forbidden by the authorization handler The possibility to add a custom header or json body would do a lot for us.

@abowen
Copy link

abowen commented Dec 31, 2019

Also interested in returning a reason as part failing a policy.

For anyone else, there is a very dodgy workaround you can use here
aspnet/Security#1560 (comment)

@BorisTheBrave
Copy link

Returning an auth failure reason now seems to be supported thanks to #21117 ?

@ericsampson
Copy link

@HaoK @blowdart is there any design progress on allowing the construction of AuthorizationAttributes that make use of resources? Instead of having to fall back on imperative resource-based authorization.
There's solutions like this article, but the potential disconnect between the self-parsed values out of ActionArguments and what MVC etc ends up binding to makes it way too easy to end up with security issues.
Thanks!

@blowdart
Copy link
Contributor

No there is not. We can't have model binding before authz, because people do weird things in model binding which can have side effects, which in turn cause security issues. It's a non-starter.

@rynowak
Copy link
Member

rynowak commented Aug 16, 2020

Regarding this specifically - it's possible to access routing's info in AuthZ with the current layering. So you could figure out the id of the item being accessed (in a typical scenario) but without having run any model binding code to turn arbitrary data into arbitrary objects.

Not sure if this helps 😆

@Tratcher Tratcher added affected-medium This issue impacts approximately half of our customers severity-major This label is used by an internal tool labels Nov 10, 2020 — with ASP.NET Core Issue Ranking
@xMANIGHTx
Copy link

I've seen that permission based authorizations have finally been taken into account, that's great! May I suggest hierarchial roles (with parent/child) out of the box?
Since 1997 I find myself skipping over the built-in authorization mechanisms,and writing them from scratch, because of missing permission based authorization and parent/child node structure.
Keep up for your great work! Maybe this time migrating do ASP.NET Core I will be able to use the built in provider... this would be a breakthrough for me! Lol

@ericsampson
Copy link

@rynowak could you point me towards the doc for doing that when you have a minute sometime? Maybe I'm misunderstanding your suggestion. Thanks so much!!

@ericsampson
Copy link

@blowdart , I know you're busy as heck, but do you know what Ryan was referring to here? I can't figure out what he's referring to, or find documentation about it. Thanks so much

it's possible to access routing's info in AuthZ with the current layering. So you could figure out the id of the item being accessed (in a typical scenario) but without having run any model binding code to turn arbitrary data into arbitrary objects.

@blowdart
Copy link
Contributor

@ericsampson No idea, and he's moved on from .net. @pranavkm any idea?

@pranavkm
Copy link
Contributor

There's some discussion about it here: dotnet/AspNetCore.Docs#12564

tl,dr:

  • AuthorizationHandlerContext.Resource is an instance of Endpoint with endpoint routing. You can use it to inspect static metadata associated with the endpoint:
if (context.Resource is Endpoint endpoint)
{
    var controllerActionDescriptor = endpoint.Metadata
        .GetMetadata<ControllerActionDescriptor>();if (controllerActionDescriptor != null)
    {
        // ...
    }	
}
  • You can inject an IHttpContextAccessor in to handler to access route values.

@HaoK to verify if (2) is still the right way to go about this.

@HaoK
Copy link
Member Author

HaoK commented Feb 10, 2021

Right, with the change that passed the Endpoint as the resource, it should be easier to inspect things from the endpoint without needing to rely on the HttpContext. I'm not sure what (2) is referring to exactly?

@pranavkm
Copy link
Contributor

I think (2) is required if you were looking for the RouteValueDictionary which is available as a feature on HttpContext.

@ericsampson
Copy link

is any of this documented in the official docs?

@HaoK
Copy link
Member Author

HaoK commented Feb 10, 2021

Ah yes, due to layering with the core authz layer being lower than Http, the handlers just have get IHttpContextAccessor from DI

@tb-mtg
Copy link

tb-mtg commented Feb 11, 2021

If possible, how can the example from @pranavkm (which shows controllerActionDescriptor), also be used for razor pages, including getting the razor page handler name?

Also is there any way to determine the HttpMethod (if it was a GET or POST) via endpoint metadata as opposed using IHttpContextAccessor?

if (context.Resource is Endpoint endpoint) {

  // can PageActionDescriptor be used?
  var pageActionDescriptor = endpoint.Metadata.GetMetadata<PageActionDescriptor>();​
  if (pageActionDescriptor != null) {
    var area = pageActionDescriptor.AreaName;
    var relativePath = pageActionDescriptor.RelativePath;
    var viewEnginePath = pageActionDescriptor.ViewEnginePath;
    var hander = ???;
    var httpMethod = ???;
    // ...
  }

  // or should CompiledPageActionDescriptor be used?
  var compiledPageActionDescriptor = endpoint.Metadata.GetMetadata<CompiledPageActionDescriptor>();​
  if (compiledPageActionDescriptor != null) {
    var area = compiledPageActionDescriptor.AreaName;
    var relativePath = compiledPageActionDescriptor.RelativePath;
    var viewEnginePath = compiledPageActionDescriptor.ViewEnginePath;
    var hander = ???;
    var httpMethod = ???;
    // ...
  }

}

@jzhouw
Copy link

jzhouw commented Jun 2, 2021

AuthorizationFailure.FailedRequirements is empty even AuthorizeAsync result failed? was trying to add customization message based on failed requirements...

@GarvMS
Copy link

GarvMS commented Jul 1, 2021

@HaoK Can we add support for populating the failed requirements in case the explicit Fail() was called? We are internally using it since version 2.0.3 but since we are now migrating to .Net Core, thought of asking here if there is anything that is stopping us in terms of design or core principle of Authz?

Let me know if you want me to raise a PR first and discuss there?

@HaoK HaoK self-assigned this Sep 27, 2022
@HaoK HaoK removed their assignment Feb 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affected-medium This issue impacts approximately half of our customers area-auth Includes: Authn, Authz, OAuth, OIDC, Bearer enhancement This issue represents an ask for new feature or an enhancement to an existing one Needs: Design This issue requires design work before implementating. severity-major This label is used by an internal tool
Projects
None yet
Development

No branches or pull requests