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

get_users_with_perms(obj, attach_perms=True, with_superusers=False) seems inconsistent #327

Closed
kewama opened this issue Jun 9, 2015 · 3 comments

Comments

@kewama
Copy link

kewama commented Jun 9, 2015

Hi,

Using django-guardian 1.2.5.

When a superuser has no specific permission for a particular object, the get_users_with_perms shortcut works as expected:

>>> from guardian.shortcuts import *
>>> from vrt.testnet.models import Testnet
>>> from django.contrib.auth import get_user_model
>>> u = get_user_model().objects.get(username='aschultz')
>>> u.is_superuser
True
>>> t = Testnet.objects.get(pk=4)
>>> get_users_with_perms(t, attach_perms=True, with_superusers=False, with_group_users=False)
{}

But if we grant that user a particular permission to that object, get_users_with_perms then reports that the user has all permissions for it:

>>> assign_perm('access_testnet', u, t)
<UserObjectPermission: snapshot-4 | aschultz | access_testnet>
>>> get_users_with_perms(t, attach_perms=True, with_superusers=False, with_group_users=False)
{<User: aschultz>: [u'access_testnet', u'add_testnet', u'change_testnet', u'create_snapshot', u'delete_testnet', u'manage_testnet']}

This seems a surprising behavior. I would have expected this instead:

>>> get_users_with_perms(t, attach_perms=True, with_superusers=False, with_group_users=False)
{<User: aschultz>: [u'access_testnet']}

Thanks,
Kevin

@brianmay
Copy link
Contributor

There seems to be a bit of confusion in the code regarding the global permissions. When we get the user list we only check for object permissions. Then when we retrieve the permissions we merge the global permissions and the object permissions.

We need to be a bit more consistent as to if we use the global permissions or not.

@brianmay
Copy link
Contributor

Have added the 'API change' label to this, as fixing this is going to change the API which people may be relying on. So fixing should perhaps be done in a major release.

@brianmay
Copy link
Contributor

Moving to #380.

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

No branches or pull requests

2 participants