Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Exclusive use of Groups #20

Closed
aarcro opened this issue Aug 28, 2015 · 3 comments
Closed

Exclusive use of Groups #20

aarcro opened this issue Aug 28, 2015 · 3 comments

Comments

@aarcro
Copy link
Contributor

aarcro commented Aug 28, 2015

I think it's a mistake to assume each user is in a single group related to a single role. In 6852439 you removed the UserRole model but I'd like to see it restored.

I was expecting that 'role' was entirely separate from groups. I intend to have my Users in groups by team, and given a role 'team_member' they would be granted the permission 'change_team_documents'. I could then use the object level permissions checks to ensure that user.has_perm('change_team_documents') and doc.team in user.groups.all()

At a minimum I'd hope the use of groups could be reworked to drop the single group assumption. Possibly filter for groups that match role names, and delete only "role groups".

I'm not even sure why assign_role_to_user() removes previous rolls. Further, available_permissions doesn't actually limit the permissions that can be applied to a user, default False permissions have no effect at all. It might make more sense to just have default_permissions as a list, thus users granted multiple rows will just have the permissions from each roll.

Sorry for the novel here. If you're interested I'm happy to split the issues up and work on solutions.

@aarcro
Copy link
Contributor Author

aarcro commented Aug 28, 2015

...Actually the one use of default False available_permissions is that they will be removed when the old role is removed by assign_role_to_user which might not actually be expected.

aarcro added a commit to aarcro/django-role-permissions that referenced this issue Aug 28, 2015
@filipeximenes
Copy link
Contributor

The change from having UserRole to using Groups was due to feedbacks I got from other people.

For the sake of simplicity, I assumed Groups would only be used for managing roles, and that's why all user groups are deleted before a new one is assigned. Your approach is more correct, this library should not interfere with Groups beyond the ones it creates.

About available_permissions, they do limit permissions that can be applied. You are supposed to be using grant_permission and revoke_permission to manage then. This methods will check if they are valid for the user role. When you change users Role, the library assumes it is a fresh start on the new Role and that's why all permissions are deleted and rebuild according to the new role. Maybe all this should be better documented. I'll open an issue :)

Can you make a PR for your changes?

filipeximenes added a commit that referenced this issue Aug 28, 2015
[#20] Don't interfere with other groups
filipeximenes added a commit that referenced this issue Aug 28, 2015
* origin/master:
  [#20] Don't interfere with other groups
@filipeximenes
Copy link
Contributor

It's ready on pypi as version 0.6

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants