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?
4
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
}
2
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
1
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.