fix: return False from has_role() when role_names is the empty list (#4541)

* fix: return False from has_role() when role_names is the empty list

* chore: add comments clarifying the effect of Q()

ietf/ietfauth/tests.py

@@ -34,6 +34,7 @@ import debug                            # pyflakes:ignore
 from ietf.group.factories import GroupFactory, RoleFactory
 from ietf.group.models import Group, Role, RoleName
 from ietf.ietfauth.htpasswd import update_htpasswd_file
+from ietf.ietfauth.utils import has_role
 from ietf.mailinglists.models import Subscribed
 from ietf.meeting.factories import MeetingFactory
 from ietf.nomcom.factories import NomComFactory
@@ -1003,3 +1004,11 @@ class OpenIDConnectTests(TestCase):
             # handler, causing later logging to become visible even if that wasn't intended.
             # Fail here if that happens.
             self.assertEqual(logging.root.handlers, [])
+
+
+class UtilsTests(TestCase):
+    def test_has_role_empty_role_names(self):
+        """has_role is False if role_names is empty"""
+        role = RoleFactory(name_id='secr', group__acronym='secretariat')
+        self.assertTrue(has_role(role.person.user, ['Secretariat']), 'Test is broken')
+        self.assertFalse(has_role(role.person.user, []), 'has_role() should return False when role_name is empty')

ietf/ietfauth/utils.py

@@ -93,7 +93,7 @@ def has_role(user, role_names, *args, **kwargs):
             "Robot": Q(person=person, name="robot", group__acronym="secretariat"),
             }
 
-        filter_expr = Q()
+        filter_expr = Q(pk__in=[])  # ensure empty set is returned if no other terms are added
         for r in role_names:
             filter_expr |= role_qs[r]
 
@@ -164,7 +164,7 @@ def is_authorized_in_doc_stream(user, doc):
             docman_roles = GroupFeatures.objects.get(type_id="ietf").docman_roles
         group_req = Q(group__acronym=doc.stream.slug)
     else:
-        group_req = Q()
+        group_req = Q()  # no group constraint for other cases
 
     return Role.objects.filter(Q(name__in=docman_roles, person__user=user) & group_req).exists()
 

ietf/ipr/views.py

@@ -187,7 +187,7 @@ def ajax_search(request):
     if not q:
         objs = IprDisclosureBase.objects.none()
     else:
-        query = Q()
+        query = Q()  # all objects returned if no other terms in the queryset
         for t in q:
             query &= Q(title__icontains=t)
 

ietf/person/views.py

@@ -31,7 +31,7 @@ def ajax_select2_search(request, model_name):
     if not q:
         objs = model.objects.none()
     else:
-        query = Q()
+        query = Q()  # all objects returned if no other terms in the queryset
         for t in q:
             if model == Email:
                 query &= Q(person__alias__name__icontains=t) | Q(address__icontains=t)

ietf/review/utils.py

@@ -147,7 +147,7 @@ def extract_review_assignment_data(teams=None, reviewers=None, time_from=None, t
     if ordering is None:
         ordering = []
 
-    filters = Q()
+    filters = Q()  # all objects returned if no other terms in the queryset
 
     if teams:
         filters &= Q(review_request__team__in=teams)