Merge pull request #30 from edx/iahmad/ENT-2501-Updated-JWT-user-role…

…-claim-for-multiple-enterprises

ENT-2501: Updated utils for user with multiple contexts

CHANGELOG.rst

@@ -12,6 +12,11 @@ Change Log
 .. There should always be an "Unreleased" section for changes pending release.
 
 
+[1.0.4] - 2019-12-17
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+* Updated utils for user with multiple contexts.
+
 [1.0.3] - 2019-09-12
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 

edx_rbac/__init__.py

@@ -4,6 +4,6 @@
 
 from __future__ import absolute_import, unicode_literals
 
-__version__ = '1.0.3'
+__version__ = '1.0.4'
 
 default_app_config = 'edx_rbac.apps.EdxRbacConfig'  # pylint: disable=invalid-name

edx_rbac/utils.py

@@ -8,6 +8,7 @@
 
 from django.apps import apps
 from django.conf import settings
+from six import string_types
 
 ALL_ACCESS_CONTEXT = '*'
 
@@ -54,16 +55,22 @@ def user_has_access_via_database(user, role_name, role_assignment_class, context
     """
     Check if there is a role assignment for a given user and role.
 
-    The role object itself is found via the role_name
+    The role object itself is found via the role_name. The role_assignment_class's get_context() method can return a
+    single context string which could be an ALL_ACCESS_CONTEXT or, incase of multiple user contexts, a list of strings.
+    The context argument is evaluated against the context(s) received from the role_assignment_class while accounting
+    for the ALL_ACCESS_CONTEXT to grant access.
     """
     try:
         role_assignment = role_assignment_class.objects.get(user=user, role__name=role_name)
     except role_assignment_class.DoesNotExist:
         return False
 
     if context:
-        return role_assignment.get_context() in (context, ALL_ACCESS_CONTEXT)
-
+        context_in_database = role_assignment.get_context()
+        if isinstance(context_in_database, string_types):
+            return context_in_database in (context, ALL_ACCESS_CONTEXT)
+        else:  # Multiple context case
+            return context in context_in_database or ALL_ACCESS_CONTEXT in context_in_database
     return True
 
 
@@ -84,6 +91,16 @@ def create_role_auth_claim_for_user(user):
             SystemWideConcreteUserRoleAssignment
         ]
     """
+    def append_role_auth_claim(role_string, context=None):
+        """
+        Append the formatted auth claim for a role and context.
+        """
+        if context:
+            contextual_role = '{}:{}'.format(role_string, context)
+            role_auth_claim.append(contextual_role)
+        else:
+            role_auth_claim.append(role_string)
+
     role_auth_claim = []
     for system_role_loc in settings.SYSTEM_WIDE_ROLE_CLASSES:
         # location can either be a module or a django model
@@ -99,6 +116,11 @@ def create_role_auth_claim_for_user(user):
 
         for role_string, context in role_func(user):
             if context:
-                role_string = '{}:{}'.format(role_string, context)
-            role_auth_claim.append(role_string)
+                if isinstance(context, string_types):
+                    append_role_auth_claim(role_string, context)
+                else:
+                    for item in context:
+                        append_role_auth_claim(role_string, item)
+            else:
+                append_role_auth_claim(role_string)
     return role_auth_claim

test_settings.py

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

tests/models.py

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

tests/test_utils.py

@@ -14,7 +14,12 @@
     request_user_has_implicit_access_via_jwt,
     user_has_access_via_database
 )
-from tests.models import ConcreteUserRole, ConcreteUserRoleAssignment, ConcreteUserRoleAssignmentNoContext
+from tests.models import (
+    ConcreteUserRole,
+    ConcreteUserRoleAssignment,
+    ConcreteUserRoleAssignmentMultipleContexts,
+    ConcreteUserRoleAssignmentNoContext
+)
 
 
 class TestUtils(TestCase):
@@ -207,10 +212,31 @@ def test_user_has_access_via_database_with_context(self):
             'a-test-context'
         )
 
+    def test_user_with_multiple_contexts_has_access_via_database(self):
+        """
+        Access check should return true if RoleAssignment exists for user with multiple contexts.
+        """
+        ConcreteUserRoleAssignmentMultipleContexts.objects.create(
+            user=self.user,
+            role=self.role
+        )
+        assert user_has_access_via_database(
+            self.user,
+            'coupon-manager',
+            ConcreteUserRoleAssignmentMultipleContexts,
+            'a-test-context'
+        )
+        assert user_has_access_via_database(
+            self.user,
+            'coupon-manager',
+            ConcreteUserRoleAssignmentMultipleContexts,
+            'a-second-test-context'
+        )
+
     def test_user_has_access_via_database_with_all_access_context(self):
         """
         Access check should return true if RoleAssignment exists for user.
-        This case handles checking if the role assignement has `ALL_ACCESS_CONTEXT` context.
+        This case handles checking if the role assignment has `ALL_ACCESS_CONTEXT` context.
         """
         ConcreteUserRoleAssignment.objects.create(
             user=self.user,
@@ -225,10 +251,30 @@ def test_user_has_access_via_database_with_all_access_context(self):
                 'some_context'
             )
 
+    def test_user_with_multiple_contexts_has_access_via_database_with_all_access_context(self):
+        """
+        Access check should return true if the correct RoleAssignment exists for user.
+        This case handles checking if the role assignment has `ALL_ACCESS_CONTEXT` as part of multiple contexts.
+        """
+        ConcreteUserRoleAssignmentMultipleContexts.objects.create(
+            user=self.user,
+            role=self.role
+        )
+
+        with patch(
+                'tests.models.ConcreteUserRoleAssignmentMultipleContexts.get_context',
+                return_value=[u'some_context', ALL_ACCESS_CONTEXT]
+        ):
+            assert user_has_access_via_database(
+                self.user,
+                'coupon-manager',
+                ConcreteUserRoleAssignmentMultipleContexts,
+                'a_context_bypassed_by_all_access_context'
+            )
+
     def test_user_has_no_access_via_database_with_context(self):
         """
-        Access check should return false if RoleAssignment does not exist for user.
-        This case handles checking if the context matches.
+        Access check should return false if the right RoleAssignment context does not exist for user.
         """
         ConcreteUserRoleAssignment.objects.create(
             user=self.user,
@@ -242,6 +288,23 @@ def test_user_has_no_access_via_database_with_context(self):
             'not_the_right_context'
         )
 
+    def test_user_with_multiple_contexts_has_no_access_via_database(self):
+        """
+        Access check should return false if the right RoleAssignment context does not exist for user with multiple
+        contexts.
+        """
+        ConcreteUserRoleAssignmentMultipleContexts.objects.create(
+            user=self.user,
+            role=self.role
+        )
+
+        assert not user_has_access_via_database(
+            self.user,
+            'coupon-manager',
+            ConcreteUserRoleAssignmentMultipleContexts,
+            'not_the_right_context'
+        )
+
     def test_user_has_no_access_via_database_no_context(self):
         """
         Access check should return false if RoleAssignment does not exist for user.
@@ -276,3 +339,22 @@ def test_create_role_auth_claim_for_user(self):
         ]
         actual_claim = create_role_auth_claim_for_user(self.user)
         assert expected_claim == actual_claim
+
+    def test_create_role_auth_claim_for_user_with_multiple_contexts(self):
+        """
+        Helper function should create a list of strings based on the roles
+        associated with the user with multiple contexts.
+        """
+        ConcreteUserRoleAssignmentMultipleContexts.objects.create(
+            user=self.user,
+            role=self.role
+        )
+
+        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