1

I'm maintaining an ASP.NET MVC site where they are doing their own security. So they have created a class derived from AuthorizeAttribute. In the OnAuthorization, they have some reflection code that finds the method based on the action name in RouteData.

The problem that I see, is that if you have multiple action functions in the controller, that differ by only AcceptVerb, or parameters, it will possible not authorize the user:

IList<MethodInfo> methods = filterContext.Controller.GetType().GetMethods().Where(i=>i.Name == action).ToList();

foreach (MethodInfo method in methods)
{
    //get all of the  controller security properties, and check for access:
    object[] props = method.GetCustomAttributes(typeof(ControllerSecurity), false);
    foreach (ControllerSecurity prop in props)
    {
        //does the user have access to this area/action?
        authorized = security.ValidateUserForAction(prop.SecurityArea, prop.SecurityAction);

        //if we are authorized by one ControllerSecurity, then we are good.
        if (authorized)
        {
            break;
        }
    }
}

The ControllerSecurity class is an Attribute class used to decorate our controller actions, and describe the security access required for this function:

//User needs to have VIEW privileges to REPORTS area:
[ControllerSecurity("REPORTS", "VIEW")]
public ActionResult Index(){...}

There must be a better way of doing this, without rewriting the security. I would like to know with some certainty that we only check the method that will be eventually run.

I've looked through the AuthorizationContext object, and can't find anyway to reliably find the action method that will be eventually called.

Anyone have any ideas?

mlsteeves
  • 1,251
  • 2
  • 16
  • 20
  • Since the attributes are on the actions themselves, can you not just iterate over all the actions attributes until you find the one that you are actually operating in? – Nick Larsen Mar 30 '10 at 12:43
  • But if I have two `Index` actions in my controller, each with different parameters, they could each have different security attributes. (Probably wouldn't happen in practice, but I like having security code as tight as possible) – mlsteeves Mar 30 '10 at 13:04
  • More information here: http://stackoverflow.com/questions/2168942/c-custom-attributes-how-to-get-the-member-type-to-which-an-attribute-was-appl – Robert Harvey Mar 30 '10 at 14:29
  • I'm not sure I understand why you need to know which controller method is executing. I am doing something very similar to this, and I didn't need to know the controller method to make it work. If the security attribute doesn't authorize, it short-circuits the execution of the controller method anyway (a redirect occurs). – Robert Harvey Mar 30 '10 at 14:34
  • @robert -Lets say you have two Index actions, one has a security descriptor that checks for VIEW access, while the other checks for EDIT access. If the method with the VIEW attribute gets checked first, the above code might give access, even though the other index was meant to fire. End result, the user that has only VIEW access, is allowed to see something that was meant for only people with EDIT access. --- As a side note, I really don't like the way security is done in this app because its all custom, not taking advantage of stuff that is already existing. But we are tied into it now. – mlsteeves Mar 30 '10 at 14:42

1 Answers1

0

I think Mvc gives you a way to access the particular action already. I haven't tried it but try this..

object[] props = filterContext.ActionDescriptor.GetCustomAttributes(typeof(ActionMethodSelectorAttribute), false);
foreach (ActionMethodSelectorAttribute prop in props)
{
        //does the user have access to this area/action?
        authorized = security.ValidateUserForAction(prop.SecurityArea, prop.SecurityAction);

        //if we are authorized by one ControllerSecurity, then we are good.
        if (authorized)
        {
                break;
        }
}   

The thing to note is filterContext.ActionDescriptor, it exists in the AuthorizationContext that is passed in in the OnAuthorization method.

If that doesn't work, you might look into using ActionNameSelectorAttribute in your action finding code. The code you showed won't always work. Take the case of someone explicitly using ActionName["MyAction"] to define the action instead of the implied method name.

Jab
  • 13,713
  • 2
  • 42
  • 48
  • The `AuthorizationContext` object has no member `Actiondescriptor`. (http://msdn.microsoft.com/en-us/library/system.identitymodel.policy.authorizationcontext_members.aspx) – mlsteeves Mar 30 '10 at 14:45
  • Good point about the case of someone explicitly using `ActionName["MyAction"]`! – mlsteeves Mar 30 '10 at 14:46
  • 2
    That's the wrong class you linked (wrong namespace) ( http://msdn.microsoft.com/en-us/library/system.web.mvc.authorizationcontext_members(VS.100).aspx) – Jab Mar 30 '10 at 15:14
  • Although I had the wrong link initially, I still don't see `ActionDescriptor` in InteliSense....I think the one that I am using is: http://msdn.microsoft.com/en-us/library/system.web.mvc.authorizationcontext_members(v=VS.90).aspx – mlsteeves Mar 30 '10 at 16:44
  • 2
    AuthorizationContext.ActionDescriptor was accidentally left out in MVC 1 due to an oversight. It's present in MVC 2. – Levi Mar 30 '10 at 17:15