From 2c25f0f3984f89830f9f17cfe00fba343025ff5b Mon Sep 17 00:00:00 2001 From: hamzawaleed01 Date: Thu, 8 Aug 2024 13:21:18 +0500 Subject: [PATCH] feat: replace group-based permissions with role-based + update tests --- .../apps/api/v1/tests/test_views.py | 14 ++++++------- .../api/v1/views/enterprise_catalog_crud.py | 10 +++++---- enterprise_catalog/apps/catalog/constants.py | 4 +++- enterprise_catalog/apps/catalog/rules.py | 21 +++++++++++++++++++ enterprise_catalog/settings/base.py | 5 +++++ 5 files changed, 41 insertions(+), 13 deletions(-) diff --git a/enterprise_catalog/apps/api/v1/tests/test_views.py b/enterprise_catalog/apps/api/v1/tests/test_views.py index 3f09450f0..83209dc9a 100644 --- a/enterprise_catalog/apps/api/v1/tests/test_views.py +++ b/enterprise_catalog/apps/api/v1/tests/test_views.py @@ -32,7 +32,7 @@ EXEC_ED_2U_ENTITLEMENT_MODE, LEARNER_PATHWAY, PROGRAM, - PROVISIONING_ADMINS_GROUP, + SYSTEM_ENTERPRISE_PROVISIONING_ADMIN_ROLE, ) from enterprise_catalog.apps.catalog.models import ( CatalogQuery, @@ -184,7 +184,6 @@ def setUp(self): 'publish_audit_enrollment_urls': True, 'content_filter': {'content_type': 'course'}, } - self.allowed_group = Group.objects.create(name=PROVISIONING_ADMINS_GROUP) def _assert_correct_new_catalog_data(self, catalog_uuid): """ @@ -265,7 +264,7 @@ def test_detail_provisioning_admin(self): self.set_up_staff_user() self.remove_role_assignments() self.set_up_invalid_jwt_role() - self.user.groups.add(self.allowed_group) + self.set_jwt_cookie([(SYSTEM_ENTERPRISE_PROVISIONING_ADMIN_ROLE, '*')]) url = reverse('api:v1:enterprise-catalog-detail', kwargs={'uuid': self.enterprise_catalog.uuid}) response = self.client.get(url) self.assertEqual(response.status_code, status.HTTP_200_OK) @@ -328,7 +327,7 @@ def test_patch_provisioning_admins(self): self.set_up_staff_user() self.remove_role_assignments() self.set_up_invalid_jwt_role() - self.user.groups.add(self.allowed_group) + self.set_jwt_cookie([(SYSTEM_ENTERPRISE_PROVISIONING_ADMIN_ROLE, '*')]) url = reverse('api:v1:enterprise-catalog-detail', kwargs={'uuid': self.enterprise_catalog.uuid}) patch_data = {'title': 'Patch title'} response = self.client.patch(url, patch_data) @@ -390,7 +389,7 @@ def test_put_provisioning_admins(self): self.set_up_staff_user() self.remove_role_assignments() self.set_up_invalid_jwt_role() - self.user.groups.add(self.allowed_group) + self.set_jwt_cookie([(SYSTEM_ENTERPRISE_PROVISIONING_ADMIN_ROLE, '*')]) url = reverse('api:v1:enterprise-catalog-detail', kwargs={'uuid': self.enterprise_catalog.uuid}) response = self.client.put(url, self.new_catalog_data) self.assertEqual(response.status_code, status.HTTP_200_OK) @@ -480,7 +479,7 @@ def test_post_provisioning_admins(self): self.set_up_staff_user() self.remove_role_assignments() self.set_up_invalid_jwt_role() - self.user.groups.add(self.allowed_group) + self.set_jwt_cookie([(SYSTEM_ENTERPRISE_PROVISIONING_ADMIN_ROLE, '*')]) url = reverse('api:v1:enterprise-catalog-list') response = self.client.post(url, self.new_catalog_data) self.assertEqual(response.status_code, status.HTTP_201_CREATED) @@ -537,7 +536,6 @@ def setUp(self): super().setUp() self.set_up_staff_user() self.enterprise_catalog = EnterpriseCatalogFactory(enterprise_uuid=self.enterprise_uuid) - self.allowed_group = Group.objects.create(name=PROVISIONING_ADMINS_GROUP) def test_list_for_superusers(self): """ @@ -560,7 +558,7 @@ def test_list_for_provisioning_admins(self): self.set_up_staff_user() self.remove_role_assignments() self.set_up_invalid_jwt_role() - self.user.groups.add(self.allowed_group) + self.set_jwt_cookie([(SYSTEM_ENTERPRISE_PROVISIONING_ADMIN_ROLE, '*')]) url = reverse('api:v1:enterprise-catalog-list') second_enterprise_catalog = EnterpriseCatalogFactory() response = self.client.get(url) diff --git a/enterprise_catalog/apps/api/v1/views/enterprise_catalog_crud.py b/enterprise_catalog/apps/api/v1/views/enterprise_catalog_crud.py index 862af881b..43332cbd1 100644 --- a/enterprise_catalog/apps/api/v1/views/enterprise_catalog_crud.py +++ b/enterprise_catalog/apps/api/v1/views/enterprise_catalog_crud.py @@ -12,7 +12,9 @@ EnterpriseCatalogSerializer, ) from enterprise_catalog.apps.api.v1.views.base import BaseViewSet -from enterprise_catalog.apps.catalog.constants import PROVISIONING_ADMINS_GROUP +from enterprise_catalog.apps.catalog.constants import ( + PERMISSION_HAS_CATALOG_PROVISIONING_ADMIN_ACCESS, +) from enterprise_catalog.apps.catalog.models import EnterpriseCatalog from enterprise_catalog.apps.catalog.rules import ( enterprises_with_admin_access, @@ -66,8 +68,9 @@ def check_permissions(self, request): for which the `rules` predicate checks against. """ # Grant provisioning-admins access to few actions + is_provisioning_admin = self.request.user.has_perm(PERMISSION_HAS_CATALOG_PROVISIONING_ADMIN_ACCESS) if self.request_action in ('create', 'partial_update', 'update', 'retrieve', 'list') and \ - request.user.groups.filter(name=PROVISIONING_ADMINS_GROUP).exists(): + is_provisioning_admin: return if self.request_action == 'list': @@ -88,8 +91,7 @@ def get_queryset(self): """ all_catalogs = EnterpriseCatalog.objects.all().order_by('created') enterprise_customer = self.request.GET.get('enterprise_customer', False) - is_provisioning_admin = self.request.user.groups.filter( - name=PROVISIONING_ADMINS_GROUP).exists() + is_provisioning_admin = self.request.user.has_perm(PERMISSION_HAS_CATALOG_PROVISIONING_ADMIN_ACCESS) if enterprise_customer: all_catalogs = all_catalogs.filter(enterprise_uuid=enterprise_customer) diff --git a/enterprise_catalog/apps/catalog/constants.py b/enterprise_catalog/apps/catalog/constants.py index bf27d89d7..ff1d1ed73 100644 --- a/enterprise_catalog/apps/catalog/constants.py +++ b/enterprise_catalog/apps/catalog/constants.py @@ -63,16 +63,19 @@ ENTERPRISE_CATALOG_ADMIN_ROLE = 'enterprise_catalog_admin' ENTERPRISE_CATALOG_LEARNER_ROLE = 'enterprise_learner' +ENTERPRISE_CATALOG_PROVISIONING_ADMIN_ROLE = 'enterprise_catalog_provisioning_admin_role' SYSTEM_ENTERPRISE_CATALOG_ADMIN_ROLE = 'enterprise_catalog_admin' SYSTEM_ENTERPRISE_LEARNER_ROLE = 'enterprise_learner' SYSTEM_ENTERPRISE_ADMIN_ROLE = 'enterprise_admin' SYSTEM_ENTERPRISE_OPERATOR_ROLE = 'enterprise_openedx_operator' +SYSTEM_ENTERPRISE_PROVISIONING_ADMIN_ROLE = 'enterprise_provisioning_admin' ACCESS_TO_ALL_ENTERPRISES_TOKEN = '*' PERMISSION_HAS_LEARNER_ACCESS = 'catalog.has_learner_access' PERMISSION_HAS_ADMIN_ACCESS = 'catalog.has_admin_access' +PERMISSION_HAS_CATALOG_PROVISIONING_ADMIN_ACCESS = 'provisioning.has_catalog_provisioning_admin_access' DISCOVERY_COURSE_KEY_BATCH_SIZE = 50 DISCOVERY_PROGRAM_KEY_BATCH_SIZE = 50 @@ -104,7 +107,6 @@ } FORCE_INCLUSION_METADATA_TAG_KEY = 'enterprise_force_included' -PROVISIONING_ADMINS_GROUP = "provisioning_admins_group" def json_serialized_course_modes(): diff --git a/enterprise_catalog/apps/catalog/rules.py b/enterprise_catalog/apps/catalog/rules.py index d591e5339..6fb091057 100644 --- a/enterprise_catalog/apps/catalog/rules.py +++ b/enterprise_catalog/apps/catalog/rules.py @@ -16,7 +16,9 @@ ACCESS_TO_ALL_ENTERPRISES_TOKEN, ENTERPRISE_CATALOG_ADMIN_ROLE, ENTERPRISE_CATALOG_LEARNER_ROLE, + ENTERPRISE_CATALOG_PROVISIONING_ADMIN_ROLE, PERMISSION_HAS_ADMIN_ACCESS, + PERMISSION_HAS_CATALOG_PROVISIONING_ADMIN_ACCESS, PERMISSION_HAS_LEARNER_ACCESS, ) from enterprise_catalog.apps.catalog.models import ( @@ -102,6 +104,25 @@ def has_explicit_access_to_catalog_learner(user, context): ) +@rules.predicate +def has_implicit_access_to_enterprise_catalog_provisioining(user, context): # pylint: disable=unused-argument + """ + Check that if request user has implicit access to `ENTERPRISE_CATALOG_PROVISIONING_ADMIN_ROLE` role. + + Returns: + boolean: whether the request user has access or not + """ + request = crum.get_current_request() + decoded_jwt = get_decoded_jwt(request) or get_decoded_jwt_from_auth(request) + return request_user_has_implicit_access_via_jwt(decoded_jwt, ENTERPRISE_CATALOG_PROVISIONING_ADMIN_ROLE, context) + + +rules.add_perm( + PERMISSION_HAS_CATALOG_PROVISIONING_ADMIN_ACCESS, + has_implicit_access_to_enterprise_catalog_provisioining +) + + def has_access_to_all_enterprises(enterprise_ids): """ Returns true if the given set of enterprise customer ids contains the "wildcard" access identifier. diff --git a/enterprise_catalog/settings/base.py b/enterprise_catalog/settings/base.py index 04eb05f76..54dcf0288 100644 --- a/enterprise_catalog/settings/base.py +++ b/enterprise_catalog/settings/base.py @@ -11,6 +11,8 @@ SYSTEM_ENTERPRISE_CATALOG_ADMIN_ROLE, SYSTEM_ENTERPRISE_LEARNER_ROLE, SYSTEM_ENTERPRISE_OPERATOR_ROLE, + SYSTEM_ENTERPRISE_PROVISIONING_ADMIN_ROLE, + ENTERPRISE_CATALOG_PROVISIONING_ADMIN_ROLE, ) from enterprise_catalog.settings.utils import get_logger_config @@ -424,6 +426,9 @@ # Admins and learners should both be able to access catalog metadata and call the contains content endpoints SYSTEM_ENTERPRISE_LEARNER_ROLE: [ENTERPRISE_CATALOG_LEARNER_ROLE], SYSTEM_ENTERPRISE_ADMIN_ROLE: [ENTERPRISE_CATALOG_LEARNER_ROLE, ENTERPRISE_CATALOG_ADMIN_ROLE], + SYSTEM_ENTERPRISE_PROVISIONING_ADMIN_ROLE: [ + ENTERPRISE_CATALOG_PROVISIONING_ADMIN_ROLE, + ] } # Set the query batch size with which existing content metadata records