Skip to content
This repository has been archived by the owner on Feb 20, 2018. It is now read-only.

User permissions #33

Open
PrimozRome opened this issue Feb 6, 2013 · 17 comments
Open

User permissions #33

PrimozRome opened this issue Feb 6, 2013 · 17 comments
Labels

Comments

@PrimozRome
Copy link
Collaborator

Is there any plan to extend warden to have permission based on users and not only roles as in current implementation?

@dre1080
Copy link
Owner

dre1080 commented Feb 6, 2013

Could you give an example of this and how it could be used?

@PrimozRome
Copy link
Collaborator Author

Now you have roles and permissions and relation between them. For example: admin can add, edit, delete users, and so on. Some projects need a little bit more advanced permissions handling on per user basis. For example:

  • USER with ID 5 can add, edit, delete articles,
  • USER with ID 10 can only add and edit articles, ...

So permissions per user, and not per role...

@dre1080
Copy link
Owner

dre1080 commented Feb 6, 2013

Don't see much use for this as its already possible by doing the following:

User with ID 5 have a role that has permission to add, edit, delete articles
User with ID 10 have a role that has permission to can only add and edit articles

So, unsure. You could maybe make a pull request and I could review it as whether its beneficial to have this.

Thanks

@PrimozRome
Copy link
Collaborator Author

Yes but if you have a heavy user driven backend, that lots of users are using and you need to restrict lots of things for some users you end up with N different roles, whereas you would need only couple of roles for access and then per user restriction on permissions...

@dre1080
Copy link
Owner

dre1080 commented Feb 7, 2013

That's true.. convinced and +1 for this.. will work on it for 2.1

Thanks for the insight

@PrimozRome
Copy link
Collaborator Author

Great, thanks for the info, looking forward on it. Take care.

@a7madgamal
Copy link

hello
in my project we need to authenticate using the following structure (top level first):

  1. Groups (a collection of users)
  2. Roles (collection of permissions assigned for groups only)
  3. permissions (RWD for predefined actions)
    i just want to know if warden can work this way and if this issue is related (should i wait for v2.1) ?
    thanks!

@PrimozRome
Copy link
Collaborator Author

Hi a7madgamal. Thanks on your input. I don't see any special reason having another layer (groups), when you already have roles which also serve as a collection of users? What's the benefit of having these groups?

@dre1080
Copy link
Owner

dre1080 commented Feb 11, 2013

@a7madgamal @PrimozRome Also don't see the need for this when you can just use roles to represent groups. Any reason for this?
Thanks

@dre1080
Copy link
Owner

dre1080 commented Feb 11, 2013

@PrimozRome Any ideas on how to go about adding permission based on users also?

@PrimozRome
Copy link
Collaborator Author

Yeaj I was thinking about how this should be done. Found a lot of resources including one very interesting article: http://lostechies.com/derickbailey/2011/05/24/dont-do-role-based-authorization-checks-do-activity-based-checks/. He completely dismisses the role-based authorization/permission system with some good examples! You can also check some comments bellow the article, you will get some nice input on this.

But if you follow above article you would end up without roles completely and end up only with users, permissions and users_permissions... If I understand it right and I guess it makes sense.

More ACL resources and design coments can be found here: http://stackoverflow.com/questions/4415663/implementing-acl-for-my-php-application.

The real question I guess here is the one being asked in above linked article. Do we really need ROLE based ACL at all? The problem of ROLE based ACL is that you eventually end up with tens and hundreds of roles, which becomes impossible to sustain. I have used the role based authorization since ever and I have end up so many roles I can not even count. I think better solution is to have User based ACL, so like users, permissions, and users_permissions... But then I guess I am not sure how you say this user is just an user, or this user is user and admin. Then ADMIN is only another permission which you grant to user?

Maybe you would leave roles in Wardem, and use them to authorize user for certain parts of the app. Like normal user, admin, moderator, ... And re-implement permissions which would be assigned to each individual user, instead to each role. Then you would have users_permissions instead of roles_permissions, everything else would almost stay the same...

@PrimozRome
Copy link
Collaborator Author

Hi dre, did you have any thoughts on this?

@ghost ghost assigned dre1080 Feb 28, 2013
@dre1080
Copy link
Owner

dre1080 commented Mar 6, 2013

@PrimozRome
Looks like a good idea,
Unfortunately, time is limiting me to go deep into this.
Could you have a go at implementing this, and send a pull request?

@ghost ghost assigned PrimozRome Mar 6, 2013
@PrimozRome
Copy link
Collaborator Author

@dre1080

I am also very busy with my ongoing projects currently, but I will try to look into this and see If I am able to do it.

Do you think it would make sense to implement it like this:

  • leave current role-based permissions system as it is
  • add additional user-based permissions system

Then you can decide whether you need to use one of them or both?

@dre1080
Copy link
Owner

dre1080 commented Apr 30, 2013

Yes I think that makes the most sense, to have both for now.
Then see which one works better before the next major release and use that better one alone for newer versions.

@PrimozRome
Copy link
Collaborator Author

I now have working implementation on users_permissions, but am still testing it. So far so good, but my implementation is only on users_permissions and I have removed roles_permissions ... Actually it's pretty simple:

  /**
   * Check if the user has permission to perform a given action on a resource.
   *
   * @param mixed $action   The action for the permission.
   * @param mixed $resource The resource for the permission.
   *
   * @return bool
   */
  public function can_user($action, $resource)
  {
    $user   = $this->current_user();
    $status = !!$user;

    if ($status) {
        $status   = false;
        $action   = (is_array($action)    ? $action : array($action));
        $resource = (is_object($resource) ? get_class($resource) : $resource);
        $resource = (is_array($resource)  ? $resource : array($resource));

        foreach ($user->permissions as $permission) {
          if ((in_array('manage', $action) || in_array($permission->action, $action)) &&
              (in_array('all', $resource)  || in_array($permission->resource, $resource)))
          {
            $status = true;
            break;
          }
        }

      /* BEFORE roles_permissions
      foreach ($user->roles as $role) {
        foreach ($role->permissions as $permission) {
          if ((in_array('manage', $action) || in_array($permission->action, $action)) &&
              (in_array('all', $resource)  || in_array($permission->resource, $resource)))
          {
            $status = true;
            break;
          }
        }
      }
      */

        $status && $this->_run_event('after_authorization');
    }

    return $status;
  }

@andreoav
Copy link
Collaborator

andreoav commented May 6, 2013

I think this can be implemented this way...
We keep role-permissions and it will work as a "minimum user permissions". In other words, if an user belongs to the "member" role, it'll have at least the permission that "member" role has.

This way we can use user-permission as a specify permission for a user, and users-permissions table will have only permission that the user needs.

If you we remove roles-permissions, you'll have to add a big list of permissions to an user when it is created...
I don't know if you agree with my idea.

@dre1080 dre1080 removed the Pending label May 6, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

4 participants