From af2591f19aaf24af5354ff27ed7e86d9d21d0116 Mon Sep 17 00:00:00 2001 From: hamzawaleed01 Date: Mon, 20 May 2024 18:17:33 +0500 Subject: [PATCH 1/4] feat: WIP - implement access control for license provisioning --- license_manager/apps/api/permissions.py | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/license_manager/apps/api/permissions.py b/license_manager/apps/api/permissions.py index 68971efa..490842a3 100644 --- a/license_manager/apps/api/permissions.py +++ b/license_manager/apps/api/permissions.py @@ -13,3 +13,20 @@ class CanRetireUser(permissions.BasePermission): def has_permission(self, request, view): return request.user.username == settings.RETIREMENT_SERVICE_WORKER_USERNAME or request.user.is_superuser + + +class CanProvisionLicenses(permissions.BasePermission): + """ + Grant access to the user retirement API for the service user, and to superusers. This mimics the + retirement permissions check in edx-platform. + """ + ALLOWED_API_GROUPS = ['license_provisioning_admins'] + message = 'User is not allowed to access the view.' + + def has_permission(self, request, view): + return ( + super().has_permission(request, view) and ( + request.user.groups.filter(name__in=self.ALLOWED_API_GROUPS).exists() + ) + ) + From cf9e7fc73d5821562238e068b364ab002cef2135 Mon Sep 17 00:00:00 2001 From: hamzawaleed01 Date: Tue, 21 May 2024 16:24:28 +0500 Subject: [PATCH 2/4] feat: add permission check in subscription plan create and partial_update actions --- license_manager/apps/api/permissions.py | 2 +- license_manager/apps/api/v1/views.py | 10 ++++++++-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/license_manager/apps/api/permissions.py b/license_manager/apps/api/permissions.py index 490842a3..d1337b74 100644 --- a/license_manager/apps/api/permissions.py +++ b/license_manager/apps/api/permissions.py @@ -25,7 +25,7 @@ class CanProvisionLicenses(permissions.BasePermission): def has_permission(self, request, view): return ( - super().has_permission(request, view) and ( + super().has_permission(self, request, view) and ( request.user.groups.filter(name__in=self.ALLOWED_API_GROUPS).exists() ) ) diff --git a/license_manager/apps/api/v1/views.py b/license_manager/apps/api/v1/views.py index 7733856c..de49c087 100644 --- a/license_manager/apps/api/v1/views.py +++ b/license_manager/apps/api/v1/views.py @@ -30,7 +30,7 @@ from license_manager.apps.api.filters import LicenseFilter from license_manager.apps.api.mixins import UserDetailsFromJwtMixin from license_manager.apps.api.models import BulkEnrollmentJob -from license_manager.apps.api.permissions import CanRetireUser +from license_manager.apps.api.permissions import CanRetireUser, CanProvisionLicenses from license_manager.apps.api.tasks import ( create_braze_aliases_task, execute_post_revocation_tasks, @@ -390,8 +390,14 @@ class SubscriptionViewSet( viewsets.GenericViewSet ): """ Viewset for Admin only read operations on SubscriptionPlans.""" - permission_required = constants.SUBSCRIPTIONS_ADMIN_ACCESS_PERMISSION allowed_roles = [constants.SUBSCRIPTIONS_ADMIN_ROLE] + permission_required = constants.SUBSCRIPTIONS_ADMIN_ACCESS_PERMISSION + + def get_permissions(self): + if self.action in ('create', 'partial_update'): + return [permissions.IsAuthenticated(), CanProvisionLicenses()] + else: + return [permission() for permission in self.permission_classes] def get_serializer_class(self): if self.action == 'create': From 8890b7b28eec8d599a36ff8363f3b7a2f910428f Mon Sep 17 00:00:00 2001 From: hamzawaleed01 Date: Wed, 22 May 2024 15:38:51 +0500 Subject: [PATCH 3/4] feat: seprate view for subscription plan creation+provisioning licenses --- license_manager/apps/api/permissions.py | 3 +- .../apps/api/v1/tests/test_views.py | 206 ++++++++++++------ license_manager/apps/api/v1/urls.py | 5 + license_manager/apps/api/v1/views.py | 61 +++--- 4 files changed, 174 insertions(+), 101 deletions(-) diff --git a/license_manager/apps/api/permissions.py b/license_manager/apps/api/permissions.py index d1337b74..b6e6a6cc 100644 --- a/license_manager/apps/api/permissions.py +++ b/license_manager/apps/api/permissions.py @@ -25,8 +25,7 @@ class CanProvisionLicenses(permissions.BasePermission): def has_permission(self, request, view): return ( - super().has_permission(self, request, view) and ( + super().has_permission(request, view) and ( request.user.groups.filter(name__in=self.ALLOWED_API_GROUPS).exists() ) ) - diff --git a/license_manager/apps/api/v1/tests/test_views.py b/license_manager/apps/api/v1/tests/test_views.py index d26f6bee..3a512404 100644 --- a/license_manager/apps/api/v1/tests/test_views.py +++ b/license_manager/apps/api/v1/tests/test_views.py @@ -23,6 +23,7 @@ generate_jwt_token, generate_unversioned_payload, ) +from django.contrib.auth.models import Group from rest_framework import status from rest_framework.test import APIClient @@ -242,14 +243,14 @@ def _subscriptions_list_request(api_client, user, enterprise_customer_uuid=None, return api_client.get(url) -def _subscription_create_request(api_client, user, params): +def _provision_license_create_request(api_client, user, params): """ Helper method that creates a SubscriptionPlan. """ if user: api_client.force_authenticate(user=user) - url = '/api/v1/subscriptions' + url = '/api/v1/provision-license' return api_client.post(url, params) @@ -264,14 +265,14 @@ def _subscription_get_request(api_client, user, subscription_uuid): return api_client.get(url) -def _subscriptions_patch_request(api_client, user, params, subscription_uuid): +def _provision_license_patch_request(api_client, user, params, subscription_uuid): """ Helper method that updates a SubscriptionPlan. """ if user: api_client.force_authenticate(user=user) - url = f'/api/v1/subscriptions/{subscription_uuid}' + url = f'/api/v1/provision-license/{subscription_uuid}' return api_client.patch(url, params) @@ -843,7 +844,7 @@ def test_subscription_plan_list_bad_enterprise_uuid_400(api_client, superuser): @pytest.mark.django_db -def test_subscription_plan_create_superuser_200(api_client, superuser, boolean_toggle): +def test_subscription_plan_create_staff_user_200(api_client, staff_user, boolean_toggle): """ Verify that the subcription POST endpoint creates new record and response includes all expected fields """ @@ -853,10 +854,11 @@ def test_subscription_plan_create_superuser_200(api_client, superuser, boolean_t ProductFactory.create_batch(1) params = _prepare_subscription_plan_payload(customer_agreement) _assign_role_via_jwt_or_db( - api_client, superuser, enterprise_customer_uuid=enterprise_customer_uuid, assign_via_jwt=boolean_toggle) - - response = _subscription_create_request( - api_client, superuser, params=params) + api_client, staff_user, enterprise_customer_uuid=enterprise_customer_uuid, assign_via_jwt=boolean_toggle) + allowed_group = Group.objects.create(name='license_provisioning_admins') + staff_user.groups.add(allowed_group) + response = _provision_license_create_request( + api_client, staff_user, params=params) assert status.HTTP_201_CREATED == response.status_code expected_fields = { @@ -888,7 +890,65 @@ def test_subscription_plan_create_superuser_200(api_client, superuser, boolean_t @pytest.mark.django_db -def test_subscription_plan_create_superuser_customer_agreement_400(api_client, superuser, boolean_toggle): +def test_subscription_plan_create_staff_user_403(api_client, staff_user, boolean_toggle): + """ + Verify that the subcription POST endpoint is accessible to authorised users only + """ + enterprise_customer_uuid = uuid4() + customer_agreement = CustomerAgreementFactory.create( + enterprise_customer_uuid=enterprise_customer_uuid) + ProductFactory.create_batch(1) + params = _prepare_subscription_plan_payload(customer_agreement) + _assign_role_via_jwt_or_db( + api_client, staff_user, enterprise_customer_uuid=enterprise_customer_uuid, assign_via_jwt=boolean_toggle) + + response = _provision_license_create_request( + api_client, staff_user, params=params) + assert status.HTTP_403_FORBIDDEN == response.status_code + + +@pytest.mark.django_db +def test_subscription_plan_update_staff_user_403(api_client, staff_user, boolean_toggle): + """ + Verify that the subcription update endpoint is accessible to authorised users only + """ + enterprise_customer_uuid = uuid4() + fake_uuid = uuid4() + customer_agreement = CustomerAgreementFactory.create( + enterprise_customer_uuid=enterprise_customer_uuid) + ProductFactory.create_batch(1) + params = _prepare_subscription_plan_payload(customer_agreement) + _assign_role_via_jwt_or_db( + api_client, staff_user, enterprise_customer_uuid=enterprise_customer_uuid, assign_via_jwt=boolean_toggle) + + response = _provision_license_patch_request( + api_client, staff_user, params=params, subscription_uuid=fake_uuid) + + assert status.HTTP_403_FORBIDDEN == response.status_code + + +@pytest.mark.django_db +def test_subscription_plan_create_staff_user_201(api_client, staff_user, boolean_toggle): + """ + Verify that the subcription update endpoint is accessible to authorised users only + """ + enterprise_customer_uuid = uuid4() + customer_agreement = CustomerAgreementFactory.create( + enterprise_customer_uuid=enterprise_customer_uuid) + ProductFactory.create_batch(1) + params = _prepare_subscription_plan_payload(customer_agreement) + allowed_group = Group.objects.create(name='license_provisioning_admins') + staff_user.groups.add(allowed_group) + + _assign_role_via_jwt_or_db( + api_client, staff_user, enterprise_customer_uuid=enterprise_customer_uuid, assign_via_jwt=boolean_toggle) + response = _provision_license_create_request( + api_client, staff_user, params=params) + assert status.HTTP_201_CREATED == response.status_code + + +@pytest.mark.django_db +def test_subscription_plan_create_staff_user_customer_agreement_400(api_client, staff_user, boolean_toggle): """ Verify that the subscription create endpoint returns error if invalid customer_agreement is given """ @@ -901,16 +961,17 @@ def test_subscription_plan_create_superuser_customer_agreement_400(api_client, s params = _prepare_subscription_plan_payload(customer_agreement) params['customer_agreement'] = invalid_customer_agreement_uuid _assign_role_via_jwt_or_db( - api_client, superuser, enterprise_customer_uuid=enterprise_customer_uuid, assign_via_jwt=boolean_toggle) - - response = _subscription_create_request( - api_client, superuser, params=params) + api_client, staff_user, enterprise_customer_uuid=enterprise_customer_uuid, assign_via_jwt=boolean_toggle) + allowed_group = Group.objects.create(name='license_provisioning_admins') + staff_user.groups.add(allowed_group) + response = _provision_license_create_request( + api_client, staff_user, params=params) assert status.HTTP_400_BAD_REQUEST == response.status_code assert response.json() == {'error': "An error occurred: Provided customer_agreement doesn't exist."} @pytest.mark.django_db -def test_subscription_plan_create_superuser_product_400(api_client, superuser, boolean_toggle): +def test_subscription_plan_create_staff_user_product_400(api_client, staff_user, boolean_toggle): """ Verify that the subscription create endpoint returns error if invalid Product is given """ @@ -922,10 +983,11 @@ def test_subscription_plan_create_superuser_product_400(api_client, superuser, b params = _prepare_subscription_plan_payload(customer_agreement) params['product'] = 2 # passing an invalid PK _assign_role_via_jwt_or_db( - api_client, superuser, enterprise_customer_uuid=enterprise_customer_uuid, assign_via_jwt=boolean_toggle) - - response = _subscription_create_request( - api_client, superuser, params=params) + api_client, staff_user, enterprise_customer_uuid=enterprise_customer_uuid, assign_via_jwt=boolean_toggle) + allowed_group = Group.objects.create(name='license_provisioning_admins') + staff_user.groups.add(allowed_group) + response = _provision_license_create_request( + api_client, staff_user, params=params) assert status.HTTP_400_BAD_REQUEST == response.status_code assert response.json() == {'error': {'product': [ @@ -933,7 +995,7 @@ def test_subscription_plan_create_superuser_product_400(api_client, superuser, b @pytest.mark.django_db -def test_subscription_plan_create_superuser_salesforce_lineitem_400(api_client, superuser, boolean_toggle): +def test_subscription_plan_create_staff_user_salesforce_lineitem_400(api_client, staff_user, boolean_toggle): """ Verify that the subscription create endpoint returns error if invalid salesforce_lineitem is given """ @@ -946,10 +1008,11 @@ def test_subscription_plan_create_superuser_salesforce_lineitem_400(api_client, # passing a invalid string params['salesforce_opportunity_line_item'] = 'foo' _assign_role_via_jwt_or_db( - api_client, superuser, enterprise_customer_uuid=enterprise_customer_uuid, assign_via_jwt=boolean_toggle) - - response = _subscription_create_request( - api_client, superuser, params=params) + api_client, staff_user, enterprise_customer_uuid=enterprise_customer_uuid, assign_via_jwt=boolean_toggle) + allowed_group = Group.objects.create(name='license_provisioning_admins') + staff_user.groups.add(allowed_group) + response = _provision_license_create_request( + api_client, staff_user, params=params) assert status.HTTP_400_BAD_REQUEST == response.status_code assert response.json() == \ @@ -957,7 +1020,7 @@ def test_subscription_plan_create_superuser_salesforce_lineitem_400(api_client, @pytest.mark.django_db -def test_subscription_plan_update_superuser_200(api_client, superuser, boolean_toggle): +def test_subscription_plan_update_staff_user_200(api_client, staff_user, boolean_toggle): """ Verify that the subscription create endpoint returns 200 on patch request """ @@ -968,13 +1031,14 @@ def test_subscription_plan_update_superuser_200(api_client, superuser, boolean_t params = _prepare_subscription_plan_payload(customer_agreement) _assign_role_via_jwt_or_db( - api_client, superuser, enterprise_customer_uuid=enterprise_customer_uuid, assign_via_jwt=boolean_toggle) - - create_response = _subscription_create_request( - api_client, superuser, params=params) + api_client, staff_user, enterprise_customer_uuid=enterprise_customer_uuid, assign_via_jwt=boolean_toggle) + allowed_group = Group.objects.create(name='license_provisioning_admins') + staff_user.groups.add(allowed_group) + create_response = _provision_license_create_request( + api_client, staff_user, params=params) params['title'] = 'bar' - patch_response = _subscriptions_patch_request( - api_client, superuser, params=params, subscription_uuid=create_response.json()['uuid']) + patch_response = _provision_license_patch_request( + api_client, staff_user, params=params, subscription_uuid=create_response.json()['uuid']) assert status.HTTP_201_CREATED == create_response.status_code assert status.HTTP_200_OK == patch_response.status_code expected_fields = { @@ -1007,7 +1071,7 @@ def test_subscription_plan_update_superuser_200(api_client, superuser, boolean_t @pytest.mark.django_db -def test_subscription_plan_update_superuser_invalid_product_id(api_client, superuser, boolean_toggle): +def test_subscription_plan_update_staff_user_invalid_product_id(api_client, staff_user, boolean_toggle): """ Verify that the subscription create endpoint returns 200 on patch request """ @@ -1018,19 +1082,20 @@ def test_subscription_plan_update_superuser_invalid_product_id(api_client, super params = _prepare_subscription_plan_payload(customer_agreement) _assign_role_via_jwt_or_db( - api_client, superuser, enterprise_customer_uuid=enterprise_customer_uuid, assign_via_jwt=boolean_toggle) - - create_response = _subscription_create_request( - api_client, superuser, params=params) + api_client, staff_user, enterprise_customer_uuid=enterprise_customer_uuid, assign_via_jwt=boolean_toggle) + allowed_group = Group.objects.create(name='license_provisioning_admins') + staff_user.groups.add(allowed_group) + create_response = _provision_license_create_request( + api_client, staff_user, params=params) params['product'] = 10 # set invalid ID - patch_response = _subscriptions_patch_request( - api_client, superuser, params=params, subscription_uuid=create_response.json()['uuid']) + patch_response = _provision_license_patch_request( + api_client, staff_user, params=params, subscription_uuid=create_response.json()['uuid']) assert status.HTTP_201_CREATED == create_response.status_code assert status.HTTP_400_BAD_REQUEST == patch_response.status_code @pytest.mark.django_db -def test_subscription_plan_update_superuser_invalid_payload(api_client, superuser, boolean_toggle): +def test_subscription_plan_update_staff_user_invalid_payload(api_client, staff_user, boolean_toggle): """ Verify that the subscription create endpoint returns 200 on patch request """ @@ -1041,20 +1106,21 @@ def test_subscription_plan_update_superuser_invalid_payload(api_client, superuse params = _prepare_subscription_plan_payload(customer_agreement) _assign_role_via_jwt_or_db( - api_client, superuser, enterprise_customer_uuid=enterprise_customer_uuid, assign_via_jwt=boolean_toggle) - - create_response = _subscription_create_request( - api_client, superuser, params=params) + api_client, staff_user, enterprise_customer_uuid=enterprise_customer_uuid, assign_via_jwt=boolean_toggle) + allowed_group = Group.objects.create(name='license_provisioning_admins') + staff_user.groups.add(allowed_group) + create_response = _provision_license_create_request( + api_client, staff_user, params=params) params['salesforce_opportunity_line_item'] = 'foo' # set invalid ID - patch_response = _subscriptions_patch_request( - api_client, superuser, params=params, subscription_uuid=create_response.json()['uuid']) + patch_response = _provision_license_patch_request( + api_client, staff_user, params=params, subscription_uuid=create_response.json()['uuid']) assert status.HTTP_201_CREATED == create_response.status_code assert status.HTTP_400_BAD_REQUEST == patch_response.status_code @pytest.mark.django_db -def test_subscription_plan_create_superuser_cataog_uuid_missing(api_client, superuser, boolean_toggle): +def test_subscription_plan_create_staff_user_cataog_uuid_missing(api_client, staff_user, boolean_toggle): """ Verify that the subscription create endpoint handles request gracefully if enterprise_catalog_uuid is missing """ @@ -1066,10 +1132,11 @@ def test_subscription_plan_create_superuser_cataog_uuid_missing(api_client, supe params = _prepare_subscription_plan_payload(customer_agreement) params['enterprise_catalog_uuid'] = None _assign_role_via_jwt_or_db( - api_client, superuser, enterprise_customer_uuid=enterprise_customer_uuid, assign_via_jwt=boolean_toggle) - - create_response = _subscription_create_request( - api_client, superuser, params=params) + api_client, staff_user, enterprise_customer_uuid=enterprise_customer_uuid, assign_via_jwt=boolean_toggle) + allowed_group = Group.objects.create(name='license_provisioning_admins') + staff_user.groups.add(allowed_group) + create_response = _provision_license_create_request( + api_client, staff_user, params=params) assert status.HTTP_201_CREATED == create_response.status_code @@ -1078,7 +1145,7 @@ def test_subscription_plan_create_superuser_cataog_uuid_missing(api_client, supe @pytest.mark.django_db -def test_subscription_plan_create_superuser_invalid_product_id(api_client, superuser, boolean_toggle): +def test_subscription_plan_create_staff_user_invalid_product_id(api_client, staff_user, boolean_toggle): """ Verify that the subscription create endpoint returns error if invalid product id is provided """ @@ -1090,10 +1157,11 @@ def test_subscription_plan_create_superuser_invalid_product_id(api_client, super params = _prepare_subscription_plan_payload(customer_agreement) params['product'] = 12 _assign_role_via_jwt_or_db( - api_client, superuser, enterprise_customer_uuid=enterprise_customer_uuid, assign_via_jwt=boolean_toggle) - - create_response = _subscription_create_request( - api_client, superuser, params=params) + api_client, staff_user, enterprise_customer_uuid=enterprise_customer_uuid, assign_via_jwt=boolean_toggle) + allowed_group = Group.objects.create(name='license_provisioning_admins') + staff_user.groups.add(allowed_group) + create_response = _provision_license_create_request( + api_client, staff_user, params=params) assert status.HTTP_400_BAD_REQUEST == create_response.status_code assert create_response.json( @@ -1101,7 +1169,7 @@ def test_subscription_plan_create_superuser_invalid_product_id(api_client, super @pytest.mark.django_db -def test_subscription_plan_create_superuser_db_integrity_error(api_client, superuser, boolean_toggle): +def test_subscription_plan_create_staff_user_db_integrity_error(api_client, staff_user, boolean_toggle): """ Verify that the subscription create endpoint returns error if invalid product id is provided """ @@ -1112,13 +1180,14 @@ def test_subscription_plan_create_superuser_db_integrity_error(api_client, super params = _prepare_subscription_plan_payload(customer_agreement) _assign_role_via_jwt_or_db( - api_client, superuser, enterprise_customer_uuid=enterprise_customer_uuid, assign_via_jwt=boolean_toggle) + api_client, staff_user, enterprise_customer_uuid=enterprise_customer_uuid, assign_via_jwt=boolean_toggle) + allowed_group = Group.objects.create(name='license_provisioning_admins') + staff_user.groups.add(allowed_group) + first_create_response = _provision_license_create_request( + api_client, staff_user, params=params) - first_create_response = _subscription_create_request( - api_client, superuser, params=params) - - second_create_response = _subscription_create_request( - api_client, superuser, params=params) + second_create_response = _provision_license_create_request( + api_client, staff_user, params=params) assert status.HTTP_201_CREATED == first_create_response.status_code assert status.HTTP_400_BAD_REQUEST == second_create_response.status_code @@ -1128,7 +1197,7 @@ def test_subscription_plan_create_superuser_db_integrity_error(api_client, super @pytest.mark.django_db -def test_subscription_plan_get_superuser_success(api_client, superuser, boolean_toggle): +def test_subscription_plan_get_staff_user_success(api_client, staff_user, boolean_toggle): """ Verify that the subscription create endpoint returns error if invalid product id is provided """ @@ -1139,14 +1208,15 @@ def test_subscription_plan_get_superuser_success(api_client, superuser, boolean_ params = _prepare_subscription_plan_payload(customer_agreement) _assign_role_via_jwt_or_db( - api_client, superuser, enterprise_customer_uuid=enterprise_customer_uuid, assign_via_jwt=boolean_toggle) - - create_response = _subscription_create_request( - api_client, superuser, params) + api_client, staff_user, enterprise_customer_uuid=enterprise_customer_uuid, assign_via_jwt=boolean_toggle) + allowed_group = Group.objects.create(name='license_provisioning_admins') + staff_user.groups.add(allowed_group) + create_response = _provision_license_create_request( + api_client, staff_user, params) created_uuid = str(create_response.json()['uuid']) retrieved_subscription = _subscription_get_request( - api_client, superuser, created_uuid) + api_client, staff_user, created_uuid) assert retrieved_subscription.json()['uuid'] == created_uuid assert status.HTTP_201_CREATED == create_response.status_code diff --git a/license_manager/apps/api/v1/urls.py b/license_manager/apps/api/v1/urls.py index 44bc8f0e..3f7f317f 100644 --- a/license_manager/apps/api/v1/urls.py +++ b/license_manager/apps/api/v1/urls.py @@ -71,6 +71,11 @@ class NestedSimpleRouter(NestedMixin, routers.SimpleRouter): viewset=views.CustomerAgreementViewSet, basename='customer-agreement', ) +router.register( + prefix=r'provision-license', + viewset=views.SubscriptionPlanProvisionViewSet, + basename='provision-license', +) subscription_router = NestedSimpleRouter( parent_router=router, diff --git a/license_manager/apps/api/v1/views.py b/license_manager/apps/api/v1/views.py index de49c087..4fe05e48 100644 --- a/license_manager/apps/api/v1/views.py +++ b/license_manager/apps/api/v1/views.py @@ -382,30 +382,10 @@ def base_queryset(self): parameters=[serializers.SubscriptionPlanQueryParamsSerializer], ), ) -class SubscriptionViewSet( - LearnerSubscriptionViewSet, - mixins.CreateModelMixin, - mixins.RetrieveModelMixin, - mixins.ListModelMixin, - viewsets.GenericViewSet -): +class SubscriptionViewSet(LearnerSubscriptionViewSet): """ Viewset for Admin only read operations on SubscriptionPlans.""" - allowed_roles = [constants.SUBSCRIPTIONS_ADMIN_ROLE] permission_required = constants.SUBSCRIPTIONS_ADMIN_ACCESS_PERMISSION - - def get_permissions(self): - if self.action in ('create', 'partial_update'): - return [permissions.IsAuthenticated(), CanProvisionLicenses()] - else: - return [permission() for permission in self.permission_classes] - - def get_serializer_class(self): - if self.action == 'create': - return serializers.SubscriptionPlanCreateSerializer - elif self.action == 'partial_update': - return serializers.SubscriptionPlanUpdateSerializer - else: - return serializers.SubscriptionPlanSerializer + allowed_roles = [constants.SUBSCRIPTIONS_ADMIN_ROLE] @property def requested_current_plan(self): @@ -430,8 +410,7 @@ def base_queryset(self): try: if self.requested_enterprise_uuid is not None: # Use the class method to get the most recent plan - current_plan = SubscriptionPlan.get_current_plan( - self.requested_enterprise_uuid) + current_plan = SubscriptionPlan.get_current_plan(self.requested_enterprise_uuid) queryset = SubscriptionPlan.objects.filter(pk=current_plan.pk) if current_plan \ else SubscriptionPlan.objects.none() else: @@ -441,6 +420,33 @@ def base_queryset(self): return queryset.order_by('-start_date') + +class SubscriptionPlanProvisionViewSet( + mixins.CreateModelMixin, + viewsets.GenericViewSet +): + """ Viewset for Admin only read operations on SubscriptionPlans.""" + authentication_classes = [JwtAuthentication, SessionAuthentication] + permission_classes = [permissions.IsAuthenticated, CanProvisionLicenses] + lookup_field = 'uuid' + + def get_serializer_class(self): + if self.action == 'create': + return serializers.SubscriptionPlanCreateSerializer + elif self.action == 'partial_update': + return serializers.SubscriptionPlanUpdateSerializer + + def get_queryset(self): + """ + Required by the `PermissionRequiredForListingMixin`. + For non-list actions, this is what's returned by `get_queryset()`. + For list actions, some non-strict subset of this is what's returned by `get_queryset()`. + """ + return SubscriptionPlan.objects.filter( + is_active=True + ).order_by('-start_date') + + def create(self, request, *args, **kwargs): """ Creates a new SubscriptionPlan record @@ -467,13 +473,6 @@ def create(self, request, *args, **kwargs): logger.exception(error) return Response({'error': 'Unknown error.'}, status=status.HTTP_400_BAD_REQUEST) - def retrieve(self, request, *args, **kwargs): - """ - Returns a single SubscriptionPlan against given uuid - """ - result = super().retrieve(request, *args, **kwargs) - return Response(data=result.data, status=status.HTTP_200_OK) - def partial_update(self, request, *args, **kwargs): """ Updates SubscriptionPlan record against given uuid From d1f2ae91c71383c177956a0f57b7ad944bf23bf7 Mon Sep 17 00:00:00 2001 From: hamzawaleed01 Date: Thu, 23 May 2024 18:56:11 +0500 Subject: [PATCH 4/4] feat: add retrieve action with some other minor code improvements --- license_manager/apps/api/permissions.py | 4 +- .../apps/api/v1/tests/constants.py | 1 + .../apps/api/v1/tests/test_views.py | 39 ++++++++++--------- license_manager/apps/api/v1/urls.py | 6 +-- license_manager/apps/api/v1/views.py | 35 ++++++++++++----- 5 files changed, 53 insertions(+), 32 deletions(-) diff --git a/license_manager/apps/api/permissions.py b/license_manager/apps/api/permissions.py index 70a5dd40..0d6bef45 100644 --- a/license_manager/apps/api/permissions.py +++ b/license_manager/apps/api/permissions.py @@ -15,11 +15,11 @@ def has_permission(self, request, view): return request.user.username == settings.RETIREMENT_SERVICE_WORKER_USERNAME or request.user.is_superuser -class CanProvisionLicenses(permissions.BasePermission): +class IsInProvisioningAdminGroup(permissions.BasePermission): """ Grant access to those users only who are part of the license provisiioning django group """ - ALLOWED_API_GROUPS = ['license_provisioning_admins'] + 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): diff --git a/license_manager/apps/api/v1/tests/constants.py b/license_manager/apps/api/v1/tests/constants.py index b1f2ba03..59b40e52 100644 --- a/license_manager/apps/api/v1/tests/constants.py +++ b/license_manager/apps/api/v1/tests/constants.py @@ -3,6 +3,7 @@ # Constants for subscriptions API tests SUBSCRIPTION_RENEWAL_DAYS_OFFSET = 500 +PROVISIONING_ADMINS_GROUP = "provisioning-admins-group" ADMIN_ROLES = { 'system_role': constants.SYSTEM_ENTERPRISE_ADMIN_ROLE, diff --git a/license_manager/apps/api/v1/tests/test_views.py b/license_manager/apps/api/v1/tests/test_views.py index a2c74172..5c034236 100644 --- a/license_manager/apps/api/v1/tests/test_views.py +++ b/license_manager/apps/api/v1/tests/test_views.py @@ -35,6 +35,7 @@ from license_manager.apps.api.v1.tests.constants import ( ADMIN_ROLES, LEARNER_ROLES, + PROVISIONING_ADMINS_GROUP, SUBSCRIPTION_RENEWAL_DAYS_OFFSET, ) from license_manager.apps.api.v1.views import ( @@ -250,7 +251,7 @@ def _provision_license_create_request(api_client, user, params): if user: api_client.force_authenticate(user=user) - url = '/api/v1/provision-license' + url = '/api/v1/provisioning-admins/subscriptions' return api_client.post(url, params) @@ -261,7 +262,7 @@ def _subscription_get_request(api_client, user, subscription_uuid): if user: api_client.force_authenticate(user=user) - url = f'/api/v1/subscriptions/{subscription_uuid}' + url = f'/api/v1/provisioning-admins/subscriptions/{subscription_uuid}' return api_client.get(url) @@ -272,7 +273,7 @@ def _provision_license_patch_request(api_client, user, params, subscription_uuid if user: api_client.force_authenticate(user=user) - url = f'/api/v1/provision-license/{subscription_uuid}' + url = f'/api/v1/provisioning-admins/subscriptions/{subscription_uuid}' return api_client.patch(url, params) @@ -855,7 +856,7 @@ def test_subscription_plan_create_staff_user_200(api_client, staff_user, boolean params = _prepare_subscription_plan_payload(customer_agreement) _assign_role_via_jwt_or_db( api_client, staff_user, enterprise_customer_uuid=enterprise_customer_uuid, assign_via_jwt=boolean_toggle) - allowed_group = Group.objects.create(name='license_provisioning_admins') + allowed_group = Group.objects.create(name=PROVISIONING_ADMINS_GROUP) staff_user.groups.add(allowed_group) response = _provision_license_create_request( api_client, staff_user, params=params) @@ -937,7 +938,7 @@ def test_subscription_plan_create_staff_user_201(api_client, staff_user, boolean enterprise_customer_uuid=enterprise_customer_uuid) ProductFactory.create_batch(1) params = _prepare_subscription_plan_payload(customer_agreement) - allowed_group = Group.objects.create(name='license_provisioning_admins') + allowed_group = Group.objects.create(name=PROVISIONING_ADMINS_GROUP) staff_user.groups.add(allowed_group) _assign_role_via_jwt_or_db( @@ -962,7 +963,7 @@ def test_subscription_plan_create_staff_user_customer_agreement_400(api_client, params['customer_agreement'] = invalid_customer_agreement_uuid _assign_role_via_jwt_or_db( api_client, staff_user, enterprise_customer_uuid=enterprise_customer_uuid, assign_via_jwt=boolean_toggle) - allowed_group = Group.objects.create(name='license_provisioning_admins') + allowed_group = Group.objects.create(name=PROVISIONING_ADMINS_GROUP) staff_user.groups.add(allowed_group) response = _provision_license_create_request( api_client, staff_user, params=params) @@ -984,7 +985,7 @@ def test_subscription_plan_create_staff_user_product_400(api_client, staff_user, params['product'] = 2 # passing an invalid PK _assign_role_via_jwt_or_db( api_client, staff_user, enterprise_customer_uuid=enterprise_customer_uuid, assign_via_jwt=boolean_toggle) - allowed_group = Group.objects.create(name='license_provisioning_admins') + allowed_group = Group.objects.create(name=PROVISIONING_ADMINS_GROUP) staff_user.groups.add(allowed_group) response = _provision_license_create_request( api_client, staff_user, params=params) @@ -1009,7 +1010,7 @@ def test_subscription_plan_create_staff_user_salesforce_lineitem_400(api_client, params['salesforce_opportunity_line_item'] = 'foo' _assign_role_via_jwt_or_db( api_client, staff_user, enterprise_customer_uuid=enterprise_customer_uuid, assign_via_jwt=boolean_toggle) - allowed_group = Group.objects.create(name='license_provisioning_admins') + allowed_group = Group.objects.create(name=PROVISIONING_ADMINS_GROUP) staff_user.groups.add(allowed_group) response = _provision_license_create_request( api_client, staff_user, params=params) @@ -1032,7 +1033,7 @@ def test_subscription_plan_update_staff_user_200(api_client, staff_user, boolean params = _prepare_subscription_plan_payload(customer_agreement) _assign_role_via_jwt_or_db( api_client, staff_user, enterprise_customer_uuid=enterprise_customer_uuid, assign_via_jwt=boolean_toggle) - allowed_group = Group.objects.create(name='license_provisioning_admins') + allowed_group = Group.objects.create(name=PROVISIONING_ADMINS_GROUP) staff_user.groups.add(allowed_group) create_response = _provision_license_create_request( api_client, staff_user, params=params) @@ -1083,7 +1084,7 @@ def test_subscription_plan_update_staff_user_invalid_product_id(api_client, staf params = _prepare_subscription_plan_payload(customer_agreement) _assign_role_via_jwt_or_db( api_client, staff_user, enterprise_customer_uuid=enterprise_customer_uuid, assign_via_jwt=boolean_toggle) - allowed_group = Group.objects.create(name='license_provisioning_admins') + allowed_group = Group.objects.create(name=PROVISIONING_ADMINS_GROUP) staff_user.groups.add(allowed_group) create_response = _provision_license_create_request( api_client, staff_user, params=params) @@ -1107,7 +1108,7 @@ def test_subscription_plan_update_staff_user_invalid_payload(api_client, staff_u params = _prepare_subscription_plan_payload(customer_agreement) _assign_role_via_jwt_or_db( api_client, staff_user, enterprise_customer_uuid=enterprise_customer_uuid, assign_via_jwt=boolean_toggle) - allowed_group = Group.objects.create(name='license_provisioning_admins') + allowed_group = Group.objects.create(name=PROVISIONING_ADMINS_GROUP) staff_user.groups.add(allowed_group) create_response = _provision_license_create_request( api_client, staff_user, params=params) @@ -1133,7 +1134,7 @@ def test_subscription_plan_create_staff_user_cataog_uuid_missing(api_client, sta params['enterprise_catalog_uuid'] = None _assign_role_via_jwt_or_db( api_client, staff_user, enterprise_customer_uuid=enterprise_customer_uuid, assign_via_jwt=boolean_toggle) - allowed_group = Group.objects.create(name='license_provisioning_admins') + allowed_group = Group.objects.create(name=PROVISIONING_ADMINS_GROUP) staff_user.groups.add(allowed_group) create_response = _provision_license_create_request( api_client, staff_user, params=params) @@ -1158,7 +1159,7 @@ def test_subscription_plan_create_staff_user_invalid_product_id(api_client, staf params['product'] = 12 _assign_role_via_jwt_or_db( api_client, staff_user, enterprise_customer_uuid=enterprise_customer_uuid, assign_via_jwt=boolean_toggle) - allowed_group = Group.objects.create(name='license_provisioning_admins') + allowed_group = Group.objects.create(name=PROVISIONING_ADMINS_GROUP) staff_user.groups.add(allowed_group) create_response = _provision_license_create_request( api_client, staff_user, params=params) @@ -1181,7 +1182,7 @@ def test_subscription_plan_create_staff_user_db_integrity_error(api_client, staf params = _prepare_subscription_plan_payload(customer_agreement) _assign_role_via_jwt_or_db( api_client, staff_user, enterprise_customer_uuid=enterprise_customer_uuid, assign_via_jwt=boolean_toggle) - allowed_group = Group.objects.create(name='license_provisioning_admins') + allowed_group = Group.objects.create(name=PROVISIONING_ADMINS_GROUP) staff_user.groups.add(allowed_group) first_create_response = _provision_license_create_request( api_client, staff_user, params=params) @@ -1209,7 +1210,7 @@ def test_subscription_plan_get_staff_user_success(api_client, staff_user, boolea params = _prepare_subscription_plan_payload(customer_agreement) _assign_role_via_jwt_or_db( api_client, staff_user, enterprise_customer_uuid=enterprise_customer_uuid, assign_via_jwt=boolean_toggle) - allowed_group = Group.objects.create(name='license_provisioning_admins') + allowed_group = Group.objects.create(name=PROVISIONING_ADMINS_GROUP) staff_user.groups.add(allowed_group) create_response = _provision_license_create_request( api_client, staff_user, params) @@ -1223,7 +1224,7 @@ def test_subscription_plan_get_staff_user_success(api_client, staff_user, boolea @pytest.mark.django_db -def test_subscription_plan_get_superuser_failure(api_client, superuser, boolean_toggle): +def test_subscription_plan_get_staff_user_failure(api_client, staff_user, boolean_toggle): """ Verify that the subscription create endpoint returns error if invalid product id is provided """ @@ -1231,9 +1232,11 @@ def test_subscription_plan_get_superuser_failure(api_client, superuser, boolean_ invalid_subscription_id = uuid4() _assign_role_via_jwt_or_db( - api_client, superuser, enterprise_customer_uuid=enterprise_customer_uuid, assign_via_jwt=boolean_toggle) + api_client, staff_user, enterprise_customer_uuid=enterprise_customer_uuid, assign_via_jwt=boolean_toggle) + allowed_group = Group.objects.create(name=PROVISIONING_ADMINS_GROUP) + staff_user.groups.add(allowed_group) response = _subscription_get_request( - api_client, superuser, invalid_subscription_id) + api_client, staff_user, invalid_subscription_id) assert status.HTTP_404_NOT_FOUND == response.status_code diff --git a/license_manager/apps/api/v1/urls.py b/license_manager/apps/api/v1/urls.py index 3f7f317f..9617c509 100644 --- a/license_manager/apps/api/v1/urls.py +++ b/license_manager/apps/api/v1/urls.py @@ -72,9 +72,9 @@ class NestedSimpleRouter(NestedMixin, routers.SimpleRouter): basename='customer-agreement', ) router.register( - prefix=r'provision-license', - viewset=views.SubscriptionPlanProvisionViewSet, - basename='provision-license', + prefix='provisioning-admins/subscriptions', + viewset=views.SubscriptionPlanProvisioningAdminViewset, + basename='provisioning-admins', ) subscription_router = NestedSimpleRouter( diff --git a/license_manager/apps/api/v1/views.py b/license_manager/apps/api/v1/views.py index 343b9d7a..99376be9 100644 --- a/license_manager/apps/api/v1/views.py +++ b/license_manager/apps/api/v1/views.py @@ -30,7 +30,10 @@ from license_manager.apps.api.filters import LicenseFilter from license_manager.apps.api.mixins import UserDetailsFromJwtMixin from license_manager.apps.api.models import BulkEnrollmentJob -from license_manager.apps.api.permissions import CanRetireUser, CanProvisionLicenses +from license_manager.apps.api.permissions import ( + CanRetireUser, + IsInProvisioningAdminGroup, +) from license_manager.apps.api.tasks import ( create_braze_aliases_task, execute_post_revocation_tasks, @@ -421,31 +424,33 @@ def base_queryset(self): return queryset.order_by('-start_date') -class SubscriptionPlanProvisionViewSet( +class SubscriptionPlanProvisioningAdminViewset( mixins.CreateModelMixin, viewsets.GenericViewSet ): - """ Viewset for Admin only read operations on SubscriptionPlans.""" + """ Viewset for Provisioning Admins write operations.""" authentication_classes = [JwtAuthentication, SessionAuthentication] - permission_classes = [permissions.IsAuthenticated, CanProvisionLicenses] + permission_classes = [permissions.IsAuthenticated, IsInProvisioningAdminGroup] lookup_field = 'uuid' + lookup_url_kwarg = 'subscription_uuid' # URL keyword for the lookup field def get_serializer_class(self): if self.action == 'create': return serializers.SubscriptionPlanCreateSerializer elif self.action == 'partial_update': return serializers.SubscriptionPlanUpdateSerializer + else: + return serializers.SubscriptionPlanSerializer def get_queryset(self): - """ - Required by the `PermissionRequiredForListingMixin`. - For non-list actions, this is what's returned by `get_queryset()`. - For list actions, some non-strict subset of this is what's returned by `get_queryset()`. - """ return SubscriptionPlan.objects.filter( is_active=True ).order_by('-start_date') + @property + def requested_subscription_uuid(self): + return self.kwargs.get('subscription_uuid') + def create(self, request, *args, **kwargs): """ Creates a new SubscriptionPlan record @@ -501,6 +506,18 @@ def partial_update(self, request, *args, **kwargs): logger.exception(error) return Response({'error': 'Unknown error.'}, status=status.HTTP_400_BAD_REQUEST) + def retrieve(self, request, *args, **kwargs): + """ + Returns a single SubscriptionPlan object against given uuid + """ + try: + subscription_plan = SubscriptionPlan.objects.get( + uuid=self.requested_subscription_uuid) + serializer = self.get_serializer(subscription_plan) + return Response(data=serializer.data, status=status.HTTP_200_OK) + except SubscriptionPlan.DoesNotExist: + return Response({'error': "No record found."}, status=status.HTTP_404_NOT_FOUND) + class LearnerLicensesViewSet( PermissionRequiredForListingMixin,