fix: Modifies create_role_auth_claim_for_user to return a list of…

… *unique*

(role:context) entries, so that the JWT does not become too large to fit in cookies/headers.
ENT-4357

CHANGELOG.rst

@@ -14,6 +14,13 @@ Change Log
 Unreleased
 --------------------
 
+[1.4.2] - 2021-03-22
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+* Modifies ``create_role_auth_claim_for_user`` to return a list of *unique*
+  (role:context) entries, so that the JWT does not become too large
+  to fit in cookies/headers.
+
 [1.4.1] - 2021-01-22
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 

edx_rbac/__init__.py

@@ -2,6 +2,6 @@
 Library to help managing role based access controls for django apps.
 """
 
-__version__ = '1.4.1'
+__version__ = '1.4.2'
 
 default_app_config = 'edx_rbac.apps.EdxRbacConfig'  # pylint: disable=invalid-name

edx_rbac/utils.py

@@ -130,13 +130,13 @@ def create_role_auth_claim_for_user(user):
     Create role auth claim for a given user.
 
     Takes a user, and for each RoleAssignment class specified in config as a
-    system wide jwt role associated with that user, creates a list of strings
-    denoting the role and context.
+    system wide jwt role associated with that user, creates and returns
+    a list of unique strings denoting the role and context.
 
-    Returns a list.
+    The SYSTEM_WIDE_ROLE_CLASSES setting is a list of classes whose roles (and associated contexts)
+    should be added to the JWT.
 
-    This setting is a list of classes whose roles should be added to the
-    jwt. The setting should look something like this:
+    The setting should look something like this:
 
         SYSTEM_WIDE_ROLE_CLASSES = [
             SystemWideConcreteUserRoleAssignment
@@ -148,11 +148,11 @@ def append_role_auth_claim(role_string, context=None):
         """
         if context:
             contextual_role = f'{role_string}:{context}'
-            role_auth_claim.append(contextual_role)
+            role_auth_claim.add(contextual_role)
         else:
-            role_auth_claim.append(role_string)
+            role_auth_claim.add(role_string)
 
-    role_auth_claim = []
+    role_auth_claim = set()
     for system_role_loc in settings.SYSTEM_WIDE_ROLE_CLASSES:
         # location can either be a module or a django model
         module_name, func_name = system_role_loc.rsplit('.', 1)
@@ -174,7 +174,8 @@ def append_role_auth_claim(role_string, context=None):
                         append_role_auth_claim(role_string, item)
             else:
                 append_role_auth_claim(role_string)
-    return role_auth_claim
+
+    return list(role_auth_claim)
 
 
 def is_iterable(obj):

test_settings.py

@@ -53,6 +53,7 @@ def root(*args):
 SYSTEM_WIDE_ROLE_CLASSES = [
     'tests.ConcreteUserRoleAssignment',
     'tests.ConcreteUserRoleAssignmentMultipleContexts',
+    'tests.ConcreteUserRoleAssignmentDuplicateContexts',
     'tests.test_assignments.get_assigments',
 ]
 

tests/migrations/0003_add_duplicate_concrete_role_assignment.py

@@ -0,0 +1,32 @@
+# Generated by Django 2.2.19 on 2021-03-22 15:22
+
+import django.db.models.deletion
+import django.utils.timezone
+import model_utils.fields
+from django.conf import settings
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        migrations.swappable_dependency(settings.AUTH_USER_MODEL),
+        ('tests', '0002_add_applies_to_all_contexts_field'),
+    ]
+
+    operations = [
+        migrations.CreateModel(
+            name='ConcreteUserRoleAssignmentDuplicateContexts',
+            fields=[
+                ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
+                ('created', model_utils.fields.AutoCreatedField(default=django.utils.timezone.now, editable=False, verbose_name='created')),
+                ('modified', model_utils.fields.AutoLastModifiedField(default=django.utils.timezone.now, editable=False, verbose_name='modified')),
+                ('applies_to_all_contexts', models.BooleanField(default=False, help_text='If true, indicates that the user is effectively assigned their role for any and all contexts. Defaults to False.')),
+                ('role', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='tests.ConcreteUserRole')),
+                ('user', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to=settings.AUTH_USER_MODEL)),
+            ],
+            options={
+                'abstract': False,
+            },
+        ),
+    ]

tests/models.py

@@ -42,6 +42,20 @@ def get_context(self):
         return ['a-test-context', 'a-second-test-context']
 
 
+class ConcreteUserRoleAssignmentDuplicateContexts(UserRoleAssignment):
+    """
+    Used for testing the UserRoleAssignment model when user has multiple, duplicate contexts.
+    """
+
+    role_class = ConcreteUserRole
+
+    def get_context(self):
+        """
+        Generate a list with multiple, duplicate contexts to be used in tests.
+        """
+        return ['a-test-context', 'a-second-test-context', 'a-test-context']
+
+
 class ConcreteUserRoleAssignmentNoContext(UserRoleAssignment):
     """
     Used for testing the UserRoleAssignment model without context returned.

tests/test_utils.py

@@ -25,6 +25,7 @@
 from tests.models import (
     ConcreteUserRole,
     ConcreteUserRoleAssignment,
+    ConcreteUserRoleAssignmentDuplicateContexts,
     ConcreteUserRoleAssignmentMultipleContexts,
     ConcreteUserRoleAssignmentNoContext
 )
@@ -414,6 +415,7 @@ def test_user_has_access(self, assigned_contexts, requested_contexts, expected_r
         assert expected_result == _user_has_access(assigned_contexts, requested_contexts)
 
 
+@ddt.ddt
 class TestUtilsWithDatabaseRequirements(TestCase):
     """
     TestUtilsWithDatabaseRequirements tests.
@@ -448,6 +450,12 @@ def create_user_role_assignment_multiple_contexts(self):
         with self._create_assignment(ConcreteUserRoleAssignmentMultipleContexts):
             yield
 
+    @contextmanager
+    def create_user_role_assignment_duplicate_contexts(self):
+        """ Helper to create a "Duplicate Context" assignment for this object's user and role. """
+        with self._create_assignment(ConcreteUserRoleAssignmentDuplicateContexts):
+            yield
+
     @contextmanager
     def create_user_role_assignment_no_context(self):
         """ Helper to create a "No Context" assignment for this object's user and role. """
@@ -599,19 +607,25 @@ def test_create_role_auth_claim_for_user(self):
                 'test-role2:1',
             ]
             actual_claim = create_role_auth_claim_for_user(self.user)
-            assert expected_claim == actual_claim
+            self.assertCountEqual(expected_claim, actual_claim)
 
-    def test_create_role_auth_claim_for_user_with_multiple_contexts(self):
+    @ddt.data(
+        'create_user_role_assignment_duplicate_contexts',
+        'create_user_role_assignment_multiple_contexts',
+    )
+    def test_create_role_auth_claim_for_user_with_many_contexts(self, role_assignment_context_manager_name):
         """
         Helper function should create a list of strings based on the roles
-        associated with the user with multiple contexts.
+        associated with the user with multiple contexts, and it should
+        always be a list of unique items.
         """
-        with self.create_user_role_assignment_multiple_contexts():
+        role_assignment_context_manager = getattr(self, role_assignment_context_manager_name)
+        with role_assignment_context_manager():
             expected_claim = [
                 'coupon-manager:a-test-context',
                 'coupon-manager:a-second-test-context',
                 'test-role',
                 'test-role2:1',
             ]
             actual_claim = create_role_auth_claim_for_user(self.user)
-            assert expected_claim == actual_claim
+            self.assertCountEqual(expected_claim, actual_claim)