My team is currently working on a massive refactor of a medium-sized application in PHP. We are doing our best to refactor our code on a module (Orders, Users, Products) basis. The issue that I am currently wondering about has to do with handling what a user is allowed to do on our system.

Currently, this hinges on mostly on the User's Access Level to our system. Sometimes, we may have to reference their owning group to see if the higher level group is allowed to do something (place an order) and pass that permission on transitively to the user.

What I have come up with so far is a class called "Guardian". Guardian takes a User in and then has functions like "canPlaceOrder" or "canViewOrderReport". One benefit to this approach is that we are able to then see all of the actions on our system that require permissions in one place. In addition, this will allow us to separate our permission logic from our front-end and domain models.

However, I could see this not being an entirely scalable solution. Following the OOP principle that a class should be open for extension, closed for modification, I can see that as we add more permissions to our system, our Guardian class will have to add more methods as well.

I do like the benefit of having all of our permissions well defined in a human-readable way using one class, so that we always know that we are checking the correct criteria. However, I'm worried about scalability. Are there other approaches to this problem?

My question is really: What is a good way to handle this? Is my solution okay?

  • What you can do is separate out group, action, thing (subject, verb, object) and then make a general way of asking variations of can this group do this action on this thing. That lets you add actions, things and groups as you need .. but if you think about it long and hard enough enough any action becomes some part of CRUD. Place order is a create. View order report is just read. So then design the class for handling combinations of those. – Elin Mar 5 '15 at 13:52
  • You are experiencing the problem of cross cutting concerns. There are different ways to deal with it, AOP is one famous way. I don't know PHP very well, therefore I can only provide you with two links for further investigation: en.wikipedia.org/wiki/Cross-cutting_concern and dsheiko.com/weblog/aspect-oriented-software-development-and-php – valenterry Mar 5 '15 at 15:10
  • @valenterry I know enough of other languages to be able to follow along. How might one accomplish this in a language of your choosing? I think this is a question that could benefit people from different languages other than PHP. I appreciate the help though. I am reading into AOP right now. – xJoshWalker Mar 5 '15 at 15:53
  • @elin Thanks for your reply. I do understand your point, and don't have too much to add to it. I prefer to have my permissions code human readable. As much as I could breakdown "placeOrder" and "viewReport" as different CRUD operations, I like to have my functions closely match the way that people in our organization speak about our domain. That way, when they say "What criteria does a user need to have to PLACE AN ORDER" we can go search for "canPlaceAnOrder". That way our domain lines up with our internal business workings as much as possible – xJoshWalker Mar 5 '15 at 15:58
up vote 0 down vote accepted

there are several good ways.

You could implement open-closed by always using a factory to get the Guardian and returning a subtype of guardian that inherits the previous version. New functionality could cast to the most recent subtype.

You could also implement open-closed by just ... Adding methods! And knowing not to touch others. While this is easier to screw up, yes you have to know what you are doing. Unfortunately a dev who doesn't know what they are doing will screw up the previously mentioned method (more commonly quoted as the way to implement open-closed).

You could also make a list of 'permission' basetype. Wireup/startup would populate the list with objects. Then you could just add inherited permissions. To query, the newest code could search the list for its subtype, and call it.

For example in .net you could even make a method (weak on the php here but don't try to compile this either): bool GetPermission() { return (from p in _permissionList.OfType).First(); }

Usage: can_do_this=GetPermission().NeedManagerApproval();

And... there are many other ways.

Yes, this is an aspect/crosscutting concern. But...

I think a bad way would be one that involves adding an AOP framework to your project

  • This is basically my thought process. I still think you might run into a maintainability concern, as now you have 13 subtypes of Guardian, that aren't necessarily a sub type but instead a super set of the original guardian class. I think that mostly, we might be thinking along the lines of "don't touch the methods already defined unless they need to change" approach – xJoshWalker Mar 9 '15 at 13:32

The guardian shold have a

public boolean checkPermission(String permissionTag, int userID);

method.

That way the guardian doesn't have to change when new permissions come to exist.

Usage

if (guardian.checkPermission("PLACE_ORDER",loggedUser)) {
    // place order
}
  • 1
    The only thing I don't necessarily like about this approach is that you then need to know the permissionTag that you would be checking for. This might mean that you would have to search the internals of the guardian class in order to know how to use it which I'm not totally sure I like – xJoshWalker Mar 5 '15 at 14:32

Your solution is okay.

Security and accessibility are pervasive in your system so having to add methods as permissions are added is workable.

Using character strings to identify permissions is okay in a PHP program since you won't ever find out that your code is wrong until run-time anyway. Character strings can be composed as well:

if ( guardian.checkPermissions(["PLACE_ORDER", "MODIFY_ORDER"]) ) {
  // place a modified order
}

My concern with your solution would be the overhead of maintaining user permissions, depending on how many users you have and the turnover of users.

Evey time you get a new user you will need to add a whole stack of permissions "canViewProduct", "canViewOrders", "canPlaceOrder" etc. etc. etc.

In larger shops this is usually handle by splitting the problem in two.

You create "roles" e.g. "manager", "retailCustomer", "wholeSaleCustomer", "badCreditCustomer".

You then assign the atomic permisions to these roles.

When you get a new user you simply assign them one or two roles. When you decide you need to restrict "badCreditCustomers" to cash on delivery only you simply remove the "canOrderOnCredit" from that role.

Your Answer

 

By clicking "Post Your Answer", you acknowledge that you have read our updated terms of service, privacy policy and cookie policy, and that your continued use of the website is subject to these policies.

Not the answer you're looking for? Browse other questions tagged or ask your own question.