Skip to content

Commit

Permalink
Remove hierarchy relations filtering from facility user permissions t…
Browse files Browse the repository at this point in the history
…o reduce performance issues on larger collection hierarchies (> 100)
  • Loading branch information
rtibbles committed Mar 2, 2021
1 parent baac052 commit 19f78f4
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 2 deletions.
23 changes: 22 additions & 1 deletion kolibri/core/auth/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
from django.core.exceptions import ValidationError
from django.db import models
from django.db.models.query import F
from django.db.models.query import Q
from django.db.utils import IntegrityError
from django.utils.encoding import python_2_unicode_compatible
from morango.models import Certificate
Expand Down Expand Up @@ -649,6 +650,26 @@ def validate_birth_year(value):
raise ValidationError(error)


class FacilityUserRoleBasedPermissionsForCoach(RoleBasedPermissions):
def readable_by_user_filter(self, user, queryset):
if user.is_anonymous():
return queryset.none()
roles = user.roles.filter(kind__in=self.can_be_read_by)

if not roles:
return queryset.none()

filter = Q()

for role in roles:
filter |= Q(
Q(memberships__collection_id=role.collection_id)
| Q(facility_id=role.collection_id)
)

return queryset.filter(filter)


@python_2_unicode_compatible
class FacilityUser(KolibriAbstractBaseUser, AbstractFacilityDataModel):
"""
Expand All @@ -664,7 +685,7 @@ class FacilityUser(KolibriAbstractBaseUser, AbstractFacilityDataModel):
# FacilityUser can be read and written by a facility admin
admin = IsAdminForOwnFacility()
# FacilityUser can be read by admin or coach, and updated by admin, but not created/deleted by non-facility admin
role = RoleBasedPermissions(
role = FacilityUserRoleBasedPermissionsForCoach(
target_field=".",
can_be_created_by=(), # we can't check creation permissions by role, as user doesn't exist yet
can_be_read_by=(role_kinds.ADMIN, role_kinds.COACH),
Expand Down
4 changes: 3 additions & 1 deletion kolibri/core/auth/test/test_roles_and_membership.py
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,9 @@ def test_admin_can_read_learner_object(self):
self.assertTrue(self.admin.can_read(self.learner))

def test_learner_is_in_list_of_readable_objects(self):
self.assertIn(
# This should not be present as we are no longer supporting classroom membership
# solely by virtue of being a member of a group in that class.
self.assertNotIn(
self.learner, self.admin.filter_readable(FacilityUser.objects.all())
)

Expand Down

0 comments on commit 19f78f4

Please sign in to comment.