Skip to content

Commit

Permalink
fixes #271
Browse files Browse the repository at this point in the history
  • Loading branch information
scholtalbers authored and brianmay committed Mar 16, 2016
1 parent 77292c3 commit 7049a57
Show file tree
Hide file tree
Showing 5 changed files with 97 additions and 12 deletions.
20 changes: 11 additions & 9 deletions guardian/shortcuts.py
Original file line number Diff line number Diff line change
Expand Up @@ -508,10 +508,12 @@ def get_objects_for_user(user, perms, klass=None, use_groups=True, any_perm=Fals
if len(codenames):
user_obj_perms_queryset = user_obj_perms_queryset.filter(
permission__codename__in=codenames)
direct_fields = ['content_object__pk', 'permission__codename']
generic_fields = ['object_pk', 'permission__codename']
if user_model.objects.is_generic():
fields = ['object_pk', 'permission__codename']
user_fields = generic_fields
else:
fields = ['content_object__pk', 'permission__codename']
user_fields = direct_fields

if use_groups:
group_model = get_group_obj_perms_model(queryset.model)
Expand All @@ -525,12 +527,12 @@ def get_objects_for_user(user, perms, klass=None, use_groups=True, any_perm=Fals
})
groups_obj_perms_queryset = group_model.objects.filter(**group_filters)
if group_model.objects.is_generic():
fields = ['object_pk', 'permission__codename']
group_fields = generic_fields
else:
fields = ['content_object__pk', 'permission__codename']
group_fields = direct_fields
if not any_perm and len(codenames) and not has_global_perms:
user_obj_perms = user_obj_perms_queryset.values_list(*fields)
groups_obj_perms = groups_obj_perms_queryset.values_list(*fields)
user_obj_perms = user_obj_perms_queryset.values_list(*user_fields)
groups_obj_perms = groups_obj_perms_queryset.values_list(*group_fields)
data = list(user_obj_perms) + list(groups_obj_perms)
# sorting/grouping by pk (first in result tuple)
keyfunc = lambda t: t[0]
Expand All @@ -545,16 +547,16 @@ def get_objects_for_user(user, perms, klass=None, use_groups=True, any_perm=Fals

if not any_perm and len(codenames) > 1:
counts = user_obj_perms_queryset.values(
fields[0]).annotate(object_pk_count=Count(fields[0]))
user_fields[0]).annotate(object_pk_count=Count(user_fields[0]))
user_obj_perms_queryset = counts.filter(
object_pk_count__gte=len(codenames))

values = user_obj_perms_queryset.values_list(fields[0], flat=True)
values = user_obj_perms_queryset.values_list(user_fields[0], flat=True)
if user_model.objects.is_generic():
values = list(values)
q = Q(pk__in=values)
if use_groups:
values = groups_obj_perms_queryset.values_list(fields[0], flat=True)
values = groups_obj_perms_queryset.values_list(group_fields[0], flat=True)
if group_model.objects.is_generic():
values = list(values)
q |= Q(pk__in=values)
Expand Down
46 changes: 46 additions & 0 deletions guardian/testapp/migrations/0007_auto_20160309_0245.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
# -*- coding: utf-8 -*-
# Generated by Django 1.9.1 on 2016-03-09 02:45
from __future__ import unicode_literals

from django.conf import settings
import django.contrib.auth.models
import django.core.validators
from django.db import migrations, models
import django.db.models.deletion


class Migration(migrations.Migration):

dependencies = [
('testapp', '0006_auto_20160221_1054'),
]

operations = [
migrations.CreateModel(
name='ReverseMixed',
fields=[
('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
('name', models.CharField(max_length=128, unique=True)),
],
),
migrations.CreateModel(
name='ReverseMixedUserObjectPermission',
fields=[
('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
('content_object', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='testapp.ReverseMixed')),
('permission', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='auth.Permission')),
],
options={
'abstract': False,
},
),
migrations.AddField(
model_name='reversemixeduserobjectpermission',
name='user',
field=models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to=settings.AUTH_USER_MODEL),
),
migrations.AlterUniqueTogether(
name='reversemixeduserobjectpermission',
unique_together=set([('user', 'permission', 'content_object')]),
),
]
15 changes: 15 additions & 0 deletions guardian/testapp/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,21 @@ def __unicode__(self):
return self.name


class ReverseMixedUserObjectPermission(UserObjectPermissionBase):
content_object = models.ForeignKey('ReverseMixed')


class ReverseMixed(models.Model):
"""
Model for tests obj perms checks with generic group object permissions model
and generic group object permissions model.
"""
name = models.CharField(max_length=128, unique=True)

def __unicode__(self):
return self.name


class LogEntryWithGroup(LogEntry):
group = models.ForeignKey('auth.Group', null=True, blank=True)

Expand Down
5 changes: 3 additions & 2 deletions guardian/testapp/tests/test_core.py
Original file line number Diff line number Diff line change
Expand Up @@ -90,11 +90,12 @@ def test_cache_for_queries_count(self):
checker.has_perm("change_group", new_group)
self.assertEqual(len(connection.queries), query_count + 2)

# Checking for permission for other model should spawn 3 queries
# Checking for permission for other model should spawn 4 queries
# every added direct relation adds one more query..
# (again: content type and actual permissions for the object...
query_count = len(connection.queries)
checker.has_perm("change_user", self.user)
self.assertEqual(len(connection.queries), query_count + 3)
self.assertEqual(len(connection.queries), query_count + 4)

finally:
settings.DEBUG = False
Expand Down
23 changes: 22 additions & 1 deletion guardian/testapp/tests/test_direct_rel.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
from __future__ import unicode_literals
from guardian.testapp.models import Mixed
from guardian.testapp.models import Mixed, ReverseMixed
from guardian.testapp.models import Project
from guardian.testapp.models import ProjectGroupObjectPermission
from guardian.testapp.models import ProjectUserObjectPermission
Expand Down Expand Up @@ -200,6 +200,7 @@ def setUp(self):
self.group = Group.objects.create(name='admins')
self.joe.groups.add(self.group)
self.mixed = Mixed.objects.create(name='Foobar')
self.reverse_mixed = ReverseMixed.objects.create(name='Foobar')

def test_get_users_with_perms_plus_groups(self):
User.objects.create_user('john', 'john@foobar.com', 'john')
Expand All @@ -214,3 +215,23 @@ def test_get_users_with_perms_plus_groups(self):
self.joe: ['add_mixed', 'change_mixed'],
jane: ['change_mixed'],
})
result = get_objects_for_user(self.joe, 'testapp.add_mixed')
self.assertEqual(sorted(p.pk for p in result),
sorted([self.mixed.pk]))

def test_get_users_with_perms_plus_groups_reverse_mixed(self):
User.objects.create_user('john', 'john@foobar.com', 'john')
jane = User.objects.create_user('jane', 'jane@foobar.com', 'jane')
group = Group.objects.create(name='devs')
self.joe.groups.add(group)
assign_perm('add_reversemixed', self.joe, self.reverse_mixed)
assign_perm('change_reversemixed', group, self.reverse_mixed)
assign_perm('change_reversemixed', jane, self.reverse_mixed)
self.assertEqual(get_users_with_perms(self.reverse_mixed, attach_perms=True),
{
self.joe: ['add_reversemixed', 'change_reversemixed'],
jane: ['change_reversemixed'],
})
result = get_objects_for_user(self.joe, 'testapp.add_reversemixed')
self.assertEqual(sorted(p.pk for p in result),
sorted([self.reverse_mixed.pk]))

0 comments on commit 7049a57

Please sign in to comment.