Aspnetcore: Authorize Attribute should have a flag to Override previous Authorize Attributes Controller - Method

[xrkolovos]

The Problem

[Authorize(policy: "SuperAdmins")]
public class SettingsController{

     .... Save, Update, Delete, Other Stuff  ...

     [Authorize(policy: "EveryOne")]
     public ... GetSettingA(){
     }
}

This GetSettingA is not allowed because SuperAdmins policy is also applied.
The Workaround for this is to create 2 Controllers. This Workaround is not nice because there are 2 different urls, in swagger 2 sections for frontend people to look, 2 files for backend people.

[Authorize(policy: "SuperAdmins")]
public class SettingsController{
     .... Save, Update, Delete, Other Stuff  ...
}
[Authorize(policy: "EveryOne")]
public class SettingsForAllController{

     [Authorize(policy: "EveryOne")]
     public ... GetSettingA(){
     }
}

The Proposition

A better approche

[Authorize(policy: "SuperAdmins")]
public class SettingsController{

     .... Save, Update, Delete, Other Stuff  ...

     [Authorize(policy: "EveryOne", OverideAllOtherPolicies: true)]
or
     [Authorize(policy: "EveryOne", CheckOnlyThisPolicy: true)]
or
    Something else that does the job
     public ... GetSettingA(){
     }
}

[xrkolovos]

An other strange workaround using IAllowAnonymous

https://stackoverflow.com/questions/52729492/net-core-override-controller-level-authorize-attribute-for-a-specific-action#answer-52735146

[blowdart]

Well the workaround would be to apply the loosest policy at the highest level and then narrow as you go down;

[Authorize(Policy: "Everyone")]
public class SettingsController{

     public ... GetSettingA(){
     }

     [Authorize(Policy: "SuperAdmins")]
     public ... SaveUpdateDeleteOtherStuff(){
     }
}

Is the objection to this that you want to be more restrictive by default, or is it around the duplication of the SuperAdmins attribute, or something else?

[xrkolovos]

If there inheritance in the Controller it's not that simple. Crud operations are inherited from a Base Controller. So that's not possible.

[francescomazzurco]

I have been struggling with same problem recently. So far, I managed to implement a custom ActionFilterAttribute together with a custom IFilterProvider that allow to override, for a single method, the policy applied on the whole controller. Basically, it takes advantage of the fact that filters applied on the controller have Scope = 10 while filters applied on a single method have Scope = 30. So this would solve the example you showed in the question, but would not allow to override a method-specific policy inherited from a base controller. I can provide a sample code if you wish.

When dealing with inheritance, things get more tricky. With the same idea (i.e. implementing a custom IFilterProvider), you could always override the policy inherited from a base controller. However, to my knowledge it is not possible to have a flag that allows you choose when to do so. I would be happy if someone proved me wrong.

[xrkolovos]

Hello, it would be nice to see your impementation. Sounds interesting

[francescomazzurco]

Here it is. The attribute is very simple:

public class OverrideFilter : ActionFilterAttribute
{
    public Type Type { get; set; }
}

Then the IFilterProvider, which we give Order = 1 so that it is executed first:

public class OverrideFilterProvider : IFilterProvider
{
    public int Order => 1;

    public void OnProvidersExecuted(FilterProviderContext context) { }

    public void OnProvidersExecuting(FilterProviderContext context)
    {
        if (context.ActionContext.ActionDescriptor.FilterDescriptors != null)
        {
            //Check whether the method has any OverrideFilter
            var overrideFilters = context.Results.Where(filterItem => filterItem.Filter is OverrideFilter).ToList();
            foreach (var overrideFilter in overrideFilters)
            {
                //Remove the filters of the corresponding type, but with smaller scope
                context.Results.RemoveAll(filterItem => filterItem.Descriptor.Filter.GetType() == ((OverrideFilter)overrideFilter.Filter).Type 
                && filterItem.Descriptor.Scope < overrideFilter.Descriptor.Scope);
            }
        }
    }
}

Finally we inject it as follows (I am not sure this is the right wat to inject it, but it works);

services.TryAddEnumerable(ServiceDescriptor.Singleton<IFilterProvider, OverrideFilterProvider>());

Note that so far authorization has not been mentioned. This approach would work for any filter. In order to use it for authorization, we create a new attribute:

public class OverrideAuthorization : OverrideFilter
{
    public OverrideAuthorization()
    {
        Type = typeof(AuthorizeFilter);
    }
}

And then apply it as follows:

[Authorize(Policy = "ControllerPolicy")
public class MyController : Controller
{
    [OverrideAuthorization]
    [Authorize(Policy = "ActionPolicy")]
    public IActionResult MyAction()
    {
        //Only ActionPolicy will be applied, while ControllerPolicy will be ignored
    }
}

INHERITANCE

If you try to debug the OverrideFilterProvider, you will see that Context.Results contains two items of type AuthorizeFilter, one with Scope = 10, which contains all policies applied on the controller level, and another with Scope = 30 with all policies applied on the action level. If you are working with inheritance, the list will go from the most derived class to the most base class. For instance, in this scenario:

public class ParentController : Controller
{
    [Authorize(Policy = "ParentPolicy")]
    public virtual IActionResult MyAction()
    {
        …
    }
}

public class ChildController : ParentController
{
    [Authorize(Policy = "ChildPolicy")]
    public override IActionResult MyAction()
    {
        …
    }
}

The final AuthorizationPolicy will have a IReadOnlyList<IAuthorizationRequirement> of length 2 with the first item being "ChildPolicy" and the second "ParentPolicy". Hence, theoretically in the IFilterProvider you could reconstruct the AuthorizationPolicy by keeping only the first element of the sequence. However, clearly this is not a clean solution, with plenty of flaws. First, you would have to do it always without discrimination. Second, you would have to stick with one policy only, but what if you wanted to apply "ChildPolicy1" and "ChildPolicy2"? Finally, it relies too much on the implementation details inside AspNetCore.Identity, and there's no guarantee it will be the same in future releases.

[blowdart]

@rynowak @HaoK

This is interesting, because you would start with the most secure option, and then punch holes, which is arguable more secure. So I have no problem with an override parameter, as long as it's false by default, but it'd be implemented within MVC and the auth pieces.

[mkArtakMSFT]

Thanks @blowdart. Up to you to decide what you want to do here.

[blowdart]

It's a valid request, I'd like to see it implemented, so it's up to MVC when :)

[rynowak]

This should be part of the AuthorizationPolicy.CombineAsync.

MVC doesn't have knowledge of how auth policies are grouped/filtered/combined today. We provide all of the attribute data to the that method and then we run the policy that comes out. It feels like a really dangerous thing to layer something like this on top of the existing source of truth.

[HaoK]

So commenting on the original issue at the top, another way would be to have the base "SuperAdmin" policy/requirement handler logic look at what other requirements are in the authZ request.

For example for Get, there would be both SuperAdmin + Everyone requirements, the SuperAdmin handler just would need to pass if the Everyone requirement was also requested, it can do this by looking at all the requirements in the authZ context.

The everyone requirement would just basically always pass, its effectively equivalent to AllowAnonymous specifically for the SuperAdmin policy.

[blowdart]

That approach is not exactly discoverable though. An attribute parameter would be

[Eilon]

Closing because we are not planning to add support for this feature.

[xrkolovos]

that's a pity. Can you write the reasons for your decision?

[Eilon]

We believed there are sufficient alternatives, such as:

• Using separate controllers for separate access levels. It can be dangerous to have a "super admin controller" and then have some actions on it be allowed for non-super admins. It might be a better security practice to separate those out.
• Some alternatives were described earlier in this issue to get this type of behavior to work, if you choose to

[xrkolovos]

For the first one, I will provide a project explaining why you can end up having this issue. The main reason is, doing things with generics and inheritance so you can reduce code.
For the second one, I don't thing the alternatives are good code examples. There are ways of hacking the mvc framework. Especially the IAllowAnonymous implementation.

[RredCat]

In my mind, the current implementation is inconsistent a bit.
Check out code below:

[Authorize(Roles ="Admin")]
public class HomeController : Controller{
    [Authorize] public IActionResult Index()=>View();
    [AllowAnonymous] public IActionResult Privacy()=>View();
}

When I call Index, the more strict policy is applied. (Admin user is required).
But when I call Privacy a 'closest' policy is applied. (Anonymous could view it).

[xrkolovos]

@RredCat You are right. That's a same thing.