From 95df996542ef9bda06b38d1640460808b6c5851d Mon Sep 17 00:00:00 2001 From: MueezKhan246 Date: Tue, 4 Jun 2024 00:03:06 +0500 Subject: [PATCH 1/9] feat: added is_in_provisioning_admin_group permission class --- enterprise/api/v1/permissions.py | 15 +++++++++ .../pending_enterprise_customer_admin_user.py | 3 +- tests/test_enterprise/api/constants.py | 2 ++ tests/test_enterprise/api/test_views.py | 33 ++++++++++--------- 4 files changed, 37 insertions(+), 16 deletions(-) diff --git a/enterprise/api/v1/permissions.py b/enterprise/api/v1/permissions.py index a90d7cfdb2..74ee12a075 100644 --- a/enterprise/api/v1/permissions.py +++ b/enterprise/api/v1/permissions.py @@ -18,3 +18,18 @@ def has_permission(self, request, view): request.user.groups.filter(name__in=request.query_params.getlist('permissions')).exists() ) ) + + +class IsInProvisioningAdminGroup(permissions.BasePermission): + """ + Grant access to those users only who are part of the license provisiioning django group + """ + ALLOWED_API_GROUPS = ['provisioning-admins-group'] + message = 'Access denied: You do not have the necessary permissions to access this.' + + def has_permission(self, request, view): + return ( + super().has_permission(request, view) and ( + request.user.groups.filter(name__in=self.ALLOWED_API_GROUPS).exists() + ) + ) diff --git a/enterprise/api/v1/views/pending_enterprise_customer_admin_user.py b/enterprise/api/v1/views/pending_enterprise_customer_admin_user.py index 700be516b4..c289d1e3a5 100644 --- a/enterprise/api/v1/views/pending_enterprise_customer_admin_user.py +++ b/enterprise/api/v1/views/pending_enterprise_customer_admin_user.py @@ -7,6 +7,7 @@ from enterprise import models from enterprise.api.v1 import serializers +from enterprise.api.v1.permissions import IsInProvisioningAdminGroup from enterprise.api.v1.views.base_views import EnterpriseReadWriteModelViewSet from enterprise.logging import getEnterpriseLogger @@ -21,7 +22,7 @@ class PendingEnterpriseCustomerAdminUserViewSet(EnterpriseReadWriteModelViewSet) queryset = models.PendingEnterpriseCustomerAdminUser.objects.all() filter_backends = (filters.OrderingFilter, DjangoFilterBackend) serializer_class = serializers.PendingEnterpriseCustomerAdminUserSerializer - permission_classes = (permissions.IsAuthenticated, permissions.IsAdminUser) + permission_classes = (permissions.IsAuthenticated, IsInProvisioningAdminGroup) FIELDS = ( 'enterprise_customer', 'user_email', diff --git a/tests/test_enterprise/api/constants.py b/tests/test_enterprise/api/constants.py index e7cf071dc0..6213f0cba3 100644 --- a/tests/test_enterprise/api/constants.py +++ b/tests/test_enterprise/api/constants.py @@ -11,3 +11,5 @@ (' ', "https://idp1.example.com"), # pylint: disable=line-too-long ('', "https://example.com") # pylint: disable=line-too-long ] + +PROVISIONING_ADMINS_GROUP = "provisioning-admins-group" diff --git a/tests/test_enterprise/api/test_views.py b/tests/test_enterprise/api/test_views.py index 9d2c8514c8..c673f7335e 100644 --- a/tests/test_enterprise/api/test_views.py +++ b/tests/test_enterprise/api/test_views.py @@ -27,7 +27,7 @@ from testfixtures import LogCapture from django.conf import settings -from django.contrib.auth.models import Permission +from django.contrib.auth.models import Permission, Group from django.core.cache import cache from django.test import override_settings from django.utils import timezone @@ -107,7 +107,7 @@ ) from test_utils.fake_enterprise_api import get_default_branding_object -from .constants import FAKE_SSO_METADATA_XML_WITH_ENTITY_ID +from .constants import FAKE_SSO_METADATA_XML_WITH_ENTITY_ID, PROVISIONING_ADMINS_GROUP Application = get_application_model() fake = Faker() @@ -859,21 +859,21 @@ def setUp(self): enterprise_customer=self.enterprise_customer, user_email='test@example.com' ) + self.staff_user = factories.UserFactory(is_staff=True, is_active=True) - def setup_admin_user(self, is_staff=True): + def setup_provisioning_admin_permission(self): """ - Creates an admin user and logs them in + Grants provisioning admin permissions to the staff user for testing purposes. """ - client_username = 'client_username' - self.client.logout() - self.create_user(username=client_username, password=TEST_PASSWORD, is_staff=is_staff) - self.client.login(username=client_username, password=TEST_PASSWORD) + allowed_group = Group.objects.create(name=PROVISIONING_ADMINS_GROUP) + self.staff_user.groups.add(allowed_group) + self.client.force_authenticate(user=self.staff_user) def test_post_pending_enterprise_customer_admin_user_creation(self): """ Make sure service users can post new PendingEnterpriseCustomerAdminUsers. """ - self.setup_admin_user(True) + self.setup_provisioning_admin_permission() data = { 'enterprise_customer': self.enterprise_customer.uuid, @@ -890,7 +890,6 @@ def test_post_pending_enterprise_customer_unauthorized_user(self): """ Make sure unauthorized users can't post PendingEnterpriseCustomerAdminUsers. """ - self.setup_admin_user(False) data = { 'enterprise_customer': self.enterprise_customer.uuid, @@ -916,6 +915,7 @@ def test_delete_pending_enterprise_customer_admin_user(self): """ Test deleting a pending enterprise customer admin user. """ + self.setup_provisioning_admin_permission() url = reverse('pending-enterprise-admin-detail', kwargs={'pk': self.pending_admin_user.id}) response = self.client.delete(settings.TEST_SERVER + url) @@ -927,6 +927,7 @@ def test_get_pending_enterprise_customer_admin_user(self): """ Test retrieving a pending enterprise customer admin user. """ + self.setup_provisioning_admin_permission() url = reverse('pending-enterprise-admin-detail', kwargs={'pk': self.pending_admin_user.id}) response = self.client.get(url) @@ -942,6 +943,7 @@ def test_patch_pending_enterprise_customer_admin_user(self): """ Test updating a pending enterprise customer admin user's email. """ + self.setup_provisioning_admin_permission() data = { 'user_email': 'updated@example.com' } @@ -958,6 +960,7 @@ def test_patch_pending_enterprise_customer_admin_user_existing_admin(self): """ Test updating a pending enterprise customer admin user with an email that already has admin permissions. """ + self.setup_provisioning_admin_permission() SystemWideEnterpriseUserRoleAssignment.objects.create( role=admin_role(), user=self.user, @@ -982,7 +985,7 @@ def test_patch_pending_admin_user_with_existing_email(self): Test patching a pending enterprise customer admin user with an email that already exists for the same enterprise customer, expecting a validation error. """ - + self.setup_provisioning_admin_permission() new_user_email = 'newtest@example.com' PendingEnterpriseCustomerAdminUser.objects.create( enterprise_customer=self.enterprise_customer, @@ -998,7 +1001,7 @@ def test_patch_pending_admin_user_with_existing_email(self): self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) - error_message = response.json().get('non_field_errors', [])[0] + error_message = response.json()[0] expected_message = 'A pending user with this email and enterprise customer already exists.' self.assertEqual(error_message, expected_message) @@ -1006,7 +1009,7 @@ def test_validate_existing_admin_user(self): """ Test validation error when creating a pending admin user with an email that already has admin permissions. """ - self.setup_admin_user(True) + self.setup_provisioning_admin_permission() SystemWideEnterpriseUserRoleAssignment.objects.create( role=admin_role(), @@ -1031,7 +1034,7 @@ def test_validate_duplicate_user(self): Test validation error when creating a pending admin user that already exists. """ - self.setup_admin_user(True) + self.setup_provisioning_admin_permission() data = { 'enterprise_customer': self.enterprise_customer.uuid, @@ -1043,7 +1046,7 @@ def test_validate_duplicate_user(self): response = self.client.post(settings.TEST_SERVER + PENDING_ENTERPRISE_CUSTOMER_ADMIN_LIST_ENDPOINT, data=data) self.assertEqual(response.status_code, 400) - error_message = response.json().get('non_field_errors', []) + error_message = response.json()[0] expected_message = "A pending user with this email and enterprise customer already exists." self.assertIn(expected_message, error_message) From b217bb270cef6deb765590b8982717f78e857e20 Mon Sep 17 00:00:00 2001 From: MueezKhan246 Date: Tue, 4 Jun 2024 00:03:27 +0500 Subject: [PATCH 2/9] feat: updated exception handling in serializer --- enterprise/api/v1/serializers.py | 30 ++++++++++++++++-------------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/enterprise/api/v1/serializers.py b/enterprise/api/v1/serializers.py index 73557da62d..cbf64bb317 100644 --- a/enterprise/api/v1/serializers.py +++ b/enterprise/api/v1/serializers.py @@ -14,6 +14,7 @@ from rest_framework.fields import empty from rest_framework.settings import api_settings +from django.db import IntegrityError, transaction from django.contrib import auth from django.contrib.sites.models import Site from django.core import exceptions as django_exceptions @@ -1644,20 +1645,6 @@ def validate(self, attrs): user_email = attrs.get('user_email', instance.user_email if instance else None) enterprise_customer = attrs.get('enterprise_customer', instance.enterprise_customer if instance else None) - if instance: - existing_instances = PendingEnterpriseCustomerAdminUser.objects.filter( - user_email=user_email, - enterprise_customer=enterprise_customer - ).exclude(id=instance.id) - else: - existing_instances = PendingEnterpriseCustomerAdminUser.objects.filter( - user_email=user_email, - enterprise_customer=enterprise_customer - ) - - if existing_instances.exists(): - raise serializers.ValidationError('A pending user with this email and enterprise customer already exists.') - admin_instance = SystemWideEnterpriseUserRoleAssignment.objects.filter( role__name=ENTERPRISE_ADMIN_ROLE, user__email=user_email, enterprise_customer=enterprise_customer ) @@ -1669,6 +1656,21 @@ def validate(self, attrs): return attrs + def save(self): + """ + Attempts to save the pending enterprise customer admin user data while handling potential integrity errors. + """ + try: + with transaction.atomic(): + return super().save() + except IntegrityError: + raise serializers.ValidationError('A pending user with this email and enterprise customer already exists.') + except Exception as e: + error_message = f"An unexpected error occurred while saving PendingEnterpriseCustomerAdminUser: {e}" + data = self.validated_data + LOGGER.error(error_message, extra={'data': data}) + raise serializers.ValidationError('An unexpected error occurred. Please try again later.') + class AnalyticsSummarySerializer(serializers.Serializer): """ From 44552827c8bd0f3fc88318bb4e2bb3999d36049a Mon Sep 17 00:00:00 2001 From: MueezKhan246 Date: Tue, 4 Jun 2024 00:04:48 +0500 Subject: [PATCH 3/9] refactor: updating build version --- CHANGELOG.rst | 5 +++++ enterprise/__init__.py | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index b46174234f..bf6ec7aaa5 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -17,6 +17,11 @@ Unreleased ---------- * nothing unreleased +[4.19.7] +-------- +* feat: added is_in_provisioning_admin_group permission class +* feat: updated exception handling in serializer + [4.19.6] -------- * feat: allowing for group members removal endpoint to support remove all diff --git a/enterprise/__init__.py b/enterprise/__init__.py index accdb2eddc..4e617e328b 100644 --- a/enterprise/__init__.py +++ b/enterprise/__init__.py @@ -2,4 +2,4 @@ Your project description goes here. """ -__version__ = "4.19.6" +__version__ = "4.19.7" From 348a35ba14d1a0b12c3c0ead232c5a93ba07895a Mon Sep 17 00:00:00 2001 From: MueezKhan246 Date: Tue, 4 Jun 2024 14:53:22 +0500 Subject: [PATCH 4/9] fix: update serializer to fix pipeline --- enterprise/api/v1/serializers.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/enterprise/api/v1/serializers.py b/enterprise/api/v1/serializers.py index cbf64bb317..c0bd5f0073 100644 --- a/enterprise/api/v1/serializers.py +++ b/enterprise/api/v1/serializers.py @@ -1656,15 +1656,17 @@ def validate(self, attrs): return attrs - def save(self): + def save(self, **kwargs): """ Attempts to save the pending enterprise customer admin user data while handling potential integrity errors. """ try: with transaction.atomic(): - return super().save() - except IntegrityError: - raise serializers.ValidationError('A pending user with this email and enterprise customer already exists.') + return super().save(**kwargs) + except IntegrityError as exc: + raise serializers.ValidationError( + 'A pending user with this email and enterprise customer already exists.' + ) from exc except Exception as e: error_message = f"An unexpected error occurred while saving PendingEnterpriseCustomerAdminUser: {e}" data = self.validated_data From ec82071c8acfbea01b95f2bbb509de228d9ce1a5 Mon Sep 17 00:00:00 2001 From: MueezKhan246 Date: Tue, 4 Jun 2024 14:57:22 +0500 Subject: [PATCH 5/9] fix: made correction in changelog file --- CHANGELOG.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 03a2c83c04..7a9a2caf7e 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -43,7 +43,7 @@ Unreleased [4.19.2] -------- -* rt: added endpoint for pending_enterprise_customer_admin_user. +* feat: added endpoint for pending_enterprise_customer_admin_user. [4.19.1] -------- From ab8b41b1a0991e0f8d9ab94ea6c19b1d4fba454d Mon Sep 17 00:00:00 2001 From: MueezKhan246 Date: Tue, 4 Jun 2024 15:15:19 +0500 Subject: [PATCH 6/9] fix: sorted imports correctly --- enterprise/api/v1/serializers.py | 2 +- tests/test_enterprise/api/test_views.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/enterprise/api/v1/serializers.py b/enterprise/api/v1/serializers.py index c0bd5f0073..c19a44b53b 100644 --- a/enterprise/api/v1/serializers.py +++ b/enterprise/api/v1/serializers.py @@ -14,10 +14,10 @@ from rest_framework.fields import empty from rest_framework.settings import api_settings -from django.db import IntegrityError, transaction from django.contrib import auth from django.contrib.sites.models import Site from django.core import exceptions as django_exceptions +from django.db import IntegrityError, transaction from django.utils.translation import gettext_lazy as _ from enterprise import models, utils # pylint: disable=cyclic-import diff --git a/tests/test_enterprise/api/test_views.py b/tests/test_enterprise/api/test_views.py index c673f7335e..7978bddd68 100644 --- a/tests/test_enterprise/api/test_views.py +++ b/tests/test_enterprise/api/test_views.py @@ -27,7 +27,7 @@ from testfixtures import LogCapture from django.conf import settings -from django.contrib.auth.models import Permission, Group +from django.contrib.auth.models import Group, Permission from django.core.cache import cache from django.test import override_settings from django.utils import timezone From 804f9c7cb6726d2c4a84945a867d3a641f531a6a Mon Sep 17 00:00:00 2001 From: MueezKhan246 Date: Tue, 4 Jun 2024 21:15:50 +0500 Subject: [PATCH 7/9] test: updating post_pending_enterprise_customer_unauthorized_user --- tests/test_enterprise/api/test_views.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/test_enterprise/api/test_views.py b/tests/test_enterprise/api/test_views.py index 7978bddd68..9edfb83ddd 100644 --- a/tests/test_enterprise/api/test_views.py +++ b/tests/test_enterprise/api/test_views.py @@ -888,7 +888,8 @@ def test_post_pending_enterprise_customer_admin_user_creation(self): def test_post_pending_enterprise_customer_unauthorized_user(self): """ - Make sure unauthorized users can't post PendingEnterpriseCustomerAdminUsers. + Make sure unauthorized users, not added to IsInProvisioningAdminGroup, + can't post PendingEnterpriseCustomerAdminUsers. """ data = { @@ -898,6 +899,9 @@ def test_post_pending_enterprise_customer_unauthorized_user(self): response = self.client.post(settings.TEST_SERVER + PENDING_ENTERPRISE_CUSTOMER_ADMIN_LIST_ENDPOINT, data=data) assert response.status_code == 403 + error_message = response.json()['detail'] + expected_message = "Access denied: You do not have the necessary permissions to access this." + self.assertIn(expected_message, error_message) def test_post_pending_enterprise_customer_user_logged_out(self): """ From df0a5e2b2944ae7add8cfe70986581c032a87c22 Mon Sep 17 00:00:00 2001 From: MueezKhan246 Date: Tue, 4 Jun 2024 21:27:52 +0500 Subject: [PATCH 8/9] refactor: updated build description in changelog file --- CHANGELOG.rst | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 7a9a2caf7e..7facade82c 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -18,8 +18,7 @@ Unreleased * nothing unreleased [4.19.8] -* feat: updated exception handling in serializer -* feat: added is_in_provisioning_admin_group permission class +* chore: updated permission class ``IsInProvisioningAdminGroup`` for ``PendingEnterpriseCustomerAdminUser`` viewpoint and handled exceptions in serializer. [4.19.7] -------- From f9329b92771d3993c23d936ae6350d2befb29a97 Mon Sep 17 00:00:00 2001 From: MueezKhan246 Date: Tue, 4 Jun 2024 21:34:56 +0500 Subject: [PATCH 9/9] refactor: removing trailing whitespace --- tests/test_enterprise/api/test_views.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_enterprise/api/test_views.py b/tests/test_enterprise/api/test_views.py index 9edfb83ddd..e9f128f70a 100644 --- a/tests/test_enterprise/api/test_views.py +++ b/tests/test_enterprise/api/test_views.py @@ -888,7 +888,7 @@ def test_post_pending_enterprise_customer_admin_user_creation(self): def test_post_pending_enterprise_customer_unauthorized_user(self): """ - Make sure unauthorized users, not added to IsInProvisioningAdminGroup, + Make sure unauthorized users, not added to IsInProvisioningAdminGroup, can't post PendingEnterpriseCustomerAdminUsers. """