fix: RoleCache.has_role should be case-insensitive

nelc · Aug 16, 2024 · db728cf · db728cf
1 parent e9609fb
commit db728cf
Show file tree

Hide file tree

Showing 2 changed files with 11 additions and 12 deletions.
diff --git a/common/djangoapps/student/roles.py b/common/djangoapps/student/roles.py
@@ -90,7 +90,7 @@ def has_role(self, role, course_id, org):
         return any(
             access_role.role in self.get_roles(role) and
             access_role.course_id == course_id and
-            access_role.org == org
+            access_role.org.lower() == org.lower()
             for access_role in self._roles
         )
 

diff --git a/common/djangoapps/student/tests/test_roles.py b/common/djangoapps/student/tests/test_roles.py
@@ -161,11 +161,13 @@ class RoleCacheTestCase(TestCase):  # lint-amnesty, pylint: disable=missing-clas
     NOT_IN_KEY = CourseKey.from_string('edX/toy/2013_Fall')
 
     ROLES = (
-        (CourseStaffRole(IN_KEY), ('staff', IN_KEY, 'edX')),
-        (CourseInstructorRole(IN_KEY), ('instructor', IN_KEY, 'edX')),
-        (OrgStaffRole(IN_KEY.org), ('staff', None, 'edX')),
-        (OrgInstructorRole(IN_KEY.org), ('instructor', None, 'edX')),
-        (CourseBetaTesterRole(IN_KEY), ('beta_testers', IN_KEY, 'edX')),
+        (CourseStaffRole(IN_KEY), ('staff', IN_KEY, 'edX'), 'test_id_01'),
+        (CourseStaffRole(IN_KEY), ('staff', IN_KEY, 'EDX'), 'test_id_01'),  # case-insensitive, considered the same
+        (CourseInstructorRole(IN_KEY), ('instructor', IN_KEY, 'edX'), 'test_id_02'),
+        (OrgStaffRole(IN_KEY.org), ('staff', None, 'edX'), 'test_id_03'),
+        (OrgStaffRole(IN_KEY.org), ('staff', None, 'EDX'), 'test_id_03'),  # case-insensitive, considered the same
+        (OrgInstructorRole(IN_KEY.org), ('instructor', None, 'edX'), 'test_id_04'),
+        (CourseBetaTesterRole(IN_KEY), ('beta_testers', IN_KEY, 'edX'), 'test_id_05'),
     )
 
     def setUp(self):
@@ -174,16 +176,13 @@ def setUp(self):
 
     @ddt.data(*ROLES)
     @ddt.unpack
-    def test_only_in_role(self, role, target):
+    def test_only_in_role(self, role, target, test_id):
         role.add_users(self.user)
         cache = RoleCache(self.user)
         assert cache.has_role(*target)
 
-        for other_role, other_target in self.ROLES:
-            if other_role == role:
-                continue
-
-            assert not cache.has_role(*other_target)
+        for _, other_target, other_test_id in self.ROLES:
+            assert cache.has_role(*other_target) == (test_id == other_test_id)
 
     @ddt.data(*ROLES)
     @ddt.unpack