Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Questions about a Custom Security setup for MVC3 using overridden AuthorizeAttribute, thread safety, ChildActions and caching

So after searching for a robust security solution for my MVC3 app, I came across this blog post by Rick Anderson. It details a WhiteList approach where a custom implementation of AuthorizeAttribute is applied as a Global Filter, and you decorate actions/controllers you wish to allow Anonymous access to using a dummy attribute AllowAnonymousAttribute (I say dummy because there is no logic inside of AllowAnonymousAttribute, it's just an empty attribute class)

bool allowAnnonymous = filterContext.ActionDescriptor.IsDefined(typeof(AllowAnonymousAttribute), true)
|| filterContext.ActionDescriptor.ControllerDescriptor.IsDefined(typeof(AllowAnonymousAttribute), true);
if (allowAnnonymous) return;

This (along with other recommendations for security mentioned on his blog like HTTPS) gives me a secure by default model whereby I don't have to apply a security check to every single action, and remember to also add it to future feature additions.

First Part of Question

Now, I'm not using the Users/Roles properties on the AuthorizeAttribute, I need to grab that stuff from a database. To me that's something that would be in AuthorizeCore, since it's sole responsability is to return a true false, does the user have access. However I have a problem, AuthorizeCore has to be thread safe based off my reading of the source for the AuthorizeAttribute class, and I am not sure the best way to go about accessing my database to determine user permissions and adhere to that. My app is using IoC and currently letting my IoC container inject my repository handling all that to the constructor of the AuthorizeAttribute, but by doing this and then accessing it within AuthorizeCore, am I not causing problems with thread safety? Or will an IoC implementation and the MVC3 DependencyResolver I'm using to provide the parameter to my custom AuthorizeAttribute constructor handle the thread safety adequately? Note, my repositories are using a UnitOfWork pattern that includes my nHibernate SessionFactory as a contructor to the repository and the Unit of Work class is provided from my IoC container, implemented by StructureMap using the line below, am I correct in thinking the scope used here would handle threading concerns?

For<IUnitOfWork>().HybridHttpOrThreadLocalScoped().Use<UnitOfWork>();

Second Part of Question

My data model (and thus security model) is setup so that my primary business objects are all defined is such a way that it's one large hierarchy model, and that when I check for permissions I look in that hierarchy model for where the user's account was defined and grant access to everything underneath it by default. The secondary permissions check is the one that uses Administrative defined business logic permissions like can users in role X access the Delete Widget functionality. For this I'm using the Route data and pulling out the Controller and Action names, and use them in conjunction with details from the current users Principal details to hit my database to resolve the permissions for this request. However this logic is being repeated for each ChildAction used on the page as well, but because I'm using the Controller and Action names from the Route data, I'm not actually getting the Child Action information. It stays as the parent action name, not the child action since the child action is not being executed via a URL request. This is causes redundant security checks on my database for the details of the Parent Action and needless resource hits. In researching this I decided to simply bypass the security check for Child actions and rely on the parent action for this.

bool bypassChildAction = filterContext.ActionDescriptor.IsDefined(typeof (ChildActionOnlyAttribute), true) || filterContext.IsChildAction;
if (bypassChildAction) return;

Does it make sense to do this, and if so/not, why? In my mind, if the Action is decorated with ChildActionOnlyAttribute, it's inaccessible publicly via a URL anyway. And if it's being executed as a Child Action but is not exclusively a child action, I can bypass the security check just for this execution since the parent action will handle the permissions. Would you ever have a situation where you would need restrict access to a child action? Knowing that child actions are typically very small partial views I don't anticipate this being a problem but I also saw reference to a line in the default implementation of OnAuthorization outlining some concerns over caching. Does anyone know if that affects my proposed solution?

Summary concerns:

  • Multi-threading concerns for accessing user permission from database within AuthorizeCore
  • Security concerns over bypassing Authorization check for child actions
  • Caching concerns for Child Actions combines with previous point

Any opinions or help with these aspects would be greatly appreciated!

like image 708
Nick Albrecht Avatar asked May 20 '11 15:05

Nick Albrecht


1 Answers

Heya Yarx - Part 1- cache all permissions for the user upon login. Multi threaded access then is not an issue as your AuthorizeCore simply gets the roles from the cache which at that time can be considered read only.

Part 2: Again going to point 1 above : ) - if your security checks are so heavy, why not load all permissions for a user upon login and cache them. Upon hitting your child actions you can demand the permissions and at that time check the cache for them.

There is definitely a better way to handle this that isn't as heavy. If you are hitting the db multiple times in a single request solely for permissions, you need to either cache your permission set via some mechanism (custom or implementing another claims based system, etc)

I'm not 100% following your mechanism though for authorization based on the route. You mentioned that you are pulling info from the route - can you give an example here?

It absolutely makes sense to protect your child actions. What if two views call Html.Action - one specifically as an admin and the other is erroneously copy and pasted into another view? Your child actions should always be protected, don't assume they are ok since they are only called from another view.

Also if you cannot cache the entire tree for a user, you can certainly cache the security checks in the first call to AuthorizeCore. Subsequent calls would simply check for ex. cached roles - if there then it uses them, otherwise look to the database.

like image 103
Adam Tuliper Avatar answered Sep 18 '22 14:09

Adam Tuliper