From 792c9b0c8d71c456a2f913a7294ce6ec3e55ea21 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 | 16 ++++++-------- .../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(+), 15 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..9f54f230f 100644 --- a/enterprise_catalog/apps/api/v1/tests/test_views.py +++ b/enterprise_catalog/apps/api/v1/tests/test_views.py @@ -9,7 +9,6 @@ import ddt import pytz from django.conf import settings -from django.contrib.auth.models import Group from django.db import IntegrityError from django.utils.http import urlencode from django.utils.text import slugify @@ -32,7 +31,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 +183,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 +263,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) @@ -325,10 +323,9 @@ def test_patch_provisioning_admins(self): """ Verify the viewset handles patching an enterprise catalog """ - 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 +387,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 +477,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 +534,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 +556,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..9d8b872f4 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_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_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_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..53183d674 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 = 'enterprise_catalog_provisioning_admin' 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_PROVISIONING_ADMIN_ACCESS = 'catalog.has_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..38df46700 100644 --- a/enterprise_catalog/apps/catalog/rules.py +++ b/enterprise_catalog/apps/catalog/rules.py @@ -16,8 +16,10 @@ ACCESS_TO_ALL_ENTERPRISES_TOKEN, ENTERPRISE_CATALOG_ADMIN_ROLE, ENTERPRISE_CATALOG_LEARNER_ROLE, + ENTERPRISE_CATALOG_PROVISIONING_ADMIN, PERMISSION_HAS_ADMIN_ACCESS, PERMISSION_HAS_LEARNER_ACCESS, + PERMISSION_HAS_PROVISIONING_ADMIN_ACCESS, ) from enterprise_catalog.apps.catalog.models import ( EnterpriseCatalogRoleAssignment, @@ -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. + + 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, context) + + +rules.add_perm( + PERMISSION_HAS_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..280acb0e9 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, ) 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, + ] } # Set the query batch size with which existing content metadata records