Search code examples
oopdesign-patternsclass-design

Tricky class design issue


I'm working on implementing a class for managing user permissions on my website.

For example: employees can view customer records but nothing else, employers can view customers as well as manage employees, and admins can do both those things as well as manage employers.

So far, what I've got is this:

  • I've stored a list of permissions, e.g addCustomer, delCustomer, etc. Each permission is linked to a list of the user roles which are allowed to do that action.

  • I've got a simple permissions class built. I'm using it something like this:

    if ($permissions->can('addCustomer')) echo " Add Customer "; else echo 'Not allowed to add customers';

However the tricky part is that in some places, I need to be more specific. For example: a customer has got the permission: readMsgs which allows him to read the messages between himself and an employee. However, if he has that permission, then he can simply change the url from:

site.com/messages/read/100

to

site.com/messages/read/101

And read message # 101 as well, which might be between another customer and employee. A customer shouldn't be able to read anyone's messages except himself.

Similarly, a customer has got the editCustomer permission, which allows him to edit his own profile by going to:

site.com/customers/99

(where 99 is his customer id)

But if he goes to site.com/customers/100

He should not be allowed to access that page.

How can I solve this problem? Ideally I'd like to be able to pass on an id to the permissions class. E.g:

if (! $permissions->can('readMsg', $msgId))
   echo 'not allowed';

if (! $permissions->can('editCustomer', $requestedCustomerId))
   echo 'not allowed';

Any ideas how I'd have to restructure my class structure to allow the above kind of thing?


Solution

  • I would be more granular in my taxonomy of permissions (e.g., "readOwnMsgs" vs. "readAnyMsg"). This would elaborate your permission-checking code (e.g., site.com/messages/read/### goes something along the lines of "proceed if canReadAnyMsg or if canReadOwnMsg and message author is current user"), suggesting that this logic should be encapsulated in separate classes broken down by resource type or whatever other circumstances might have an effect on contextual information required to make such decisions.