From 5863ad90bf4e8f65591b6ffa4e3720e398938806 Mon Sep 17 00:00:00 2001 From: Troy Sankey Date: Thu, 28 Sep 2023 16:46:34 -0400 Subject: [PATCH] fix: learner-facing assignments endpoints broken My last PR was missing some unit tests (added in this commit) which led to me missing the fact that learner-facing assignments endpoints were broken due to misconfigured ViewSet permissions. ENT-7680 --- .../api/v1/tests/test_assignment_views.py | 152 +++++++++++++++++- .../views/content_assignments/assignments.py | 16 +- 2 files changed, 162 insertions(+), 6 deletions(-) diff --git a/enterprise_access/apps/api/v1/tests/test_assignment_views.py b/enterprise_access/apps/api/v1/tests/test_assignment_views.py index 4f43931b..6fe0552d 100644 --- a/enterprise_access/apps/api/v1/tests/test_assignment_views.py +++ b/enterprise_access/apps/api/v1/tests/test_assignment_views.py @@ -231,6 +231,60 @@ def test_admin_assignment_readwrite_views_unauthorized_forbidden(self, role_cont assert response.status_code == expected_response_code +@ddt.ddt +class TestAssignmentsUnauthorizedCRUD(CRUDViewTestMixin, APITest): + """ + Tests Authentication and Permission checking for Learner-facing LearnerContentAssignment CRUD views. + """ + @ddt.data( + # A role that's not mapped to any feature perms will get you a 403. + ( + {'system_wide_role': 'some-other-role', 'context': str(TEST_ENTERPRISE_UUID)}, + status.HTTP_403_FORBIDDEN, + ), + # A good learner role, but in a context/customer we're not aware of, gets you a 403. + ( + {'system_wide_role': SYSTEM_ENTERPRISE_LEARNER_ROLE, 'context': str(uuid4())}, + status.HTTP_403_FORBIDDEN, + ), + # A good learner role, AND a real context/customer but just the wrong one, gets you a 403. + ( + {'system_wide_role': SYSTEM_ENTERPRISE_LEARNER_ROLE, 'context': str(TEST_OTHER_ENTERPRISE_UUID)}, + status.HTTP_403_FORBIDDEN, + ), + # No JWT based auth, no soup for you. + ( + None, + status.HTTP_401_UNAUTHORIZED, + ), + ) + @ddt.unpack + def test_assignment_views_unauthorized_forbidden(self, role_context_dict, expected_response_code): + """ + Tests that we get expected 40x responses for all of the learner-facing views. + """ + # Set the JWT-based auth that we'll use for every request + if role_context_dict: + self.set_jwt_cookie([role_context_dict]) + + detail_kwargs = { + 'assignment_configuration_uuid': str(TEST_ASSIGNMENT_CONFIG_UUID), + 'uuid': str(self.requester_assignment_accepted.uuid), + } + detail_url = reverse('api:v1:assignments-detail', kwargs=detail_kwargs) + + # Test views that need CONTENT_ASSIGNMENT_READ_PERMISSION: + + # GET/retrieve endpoint: + response = self.client.get(detail_url) + assert response.status_code == expected_response_code + + # GET/list endpoint: + request_params = {'enterprise_customer_uuid': str(TEST_ENTERPRISE_UUID)} + response = self.client.get(ADMIN_ASSIGNMENTS_LIST_ENDPOINT, request_params) + assert response.status_code == expected_response_code + + @ddt.ddt class TestAdminAssignmentAuthorizedCRUD(CRUDViewTestMixin, APITest): """ @@ -281,14 +335,13 @@ def test_list(self, role_context_dict): Test that the list view returns a 200 response code and the expected (list) results of serialization. It should also allow system-wide admins and operators. - This also tests that only AssignmentConfigs of the requested customer are returned. + This also tests that only Assignment in the requested AssignmentConfiguration are returned. """ # Set the JWT-based auth that we'll use for every request. self.set_jwt_cookie([role_context_dict]) # Send a list request for all Assignments for the main test customer. - request_params = {'enterprise_customer_uuid': str(TEST_ENTERPRISE_UUID)} - response = self.client.get(ADMIN_ASSIGNMENTS_LIST_ENDPOINT, request_params) + response = self.client.get(ADMIN_ASSIGNMENTS_LIST_ENDPOINT) # Only the Assignments for the main customer is returned, and not that of the other customer. expected_assignments_for_enterprise_customer = LearnerContentAssignment.objects.filter( @@ -355,3 +408,96 @@ def test_cancel_non_cancelable_returns_422(self): # Check that the assignment state was NOT updated, and the state is still accepted. self.assignment_allocated_post_link.refresh_from_db() assert self.assignment_accepted.state == LearnerContentAssignmentStateChoices.ACCEPTED + + +@ddt.ddt +class TestAssignmentAuthorizedCRUD(CRUDViewTestMixin, APITest): + """ + Test the Learner-facing Assignment API views while successfully authenticated/authorized. + """ + @ddt.data( + # A good learner role, and with a context matching the main testing customer. + {'system_wide_role': SYSTEM_ENTERPRISE_LEARNER_ROLE, 'context': str(TEST_ENTERPRISE_UUID)}, + # A good admin role, and with a context matching the main testing customer. + {'system_wide_role': SYSTEM_ENTERPRISE_ADMIN_ROLE, 'context': str(TEST_ENTERPRISE_UUID)}, + # A good operator role, and with a context matching the main testing customer. + {'system_wide_role': SYSTEM_ENTERPRISE_OPERATOR_ROLE, 'context': str(TEST_ENTERPRISE_UUID)}, + ) + def test_retrieve(self, role_context_dict): + """ + Test that the retrieve view returns a 200 response code and the expected results of serialization. + """ + # Set the JWT-based auth that we'll use for every request. + self.set_jwt_cookie([role_context_dict]) + + # Setup and call the retrieve endpoint. + detail_kwargs = { + 'assignment_configuration_uuid': str(TEST_ASSIGNMENT_CONFIG_UUID), + 'uuid': str(self.requester_assignment_accepted.uuid), + } + detail_url = reverse('api:v1:assignments-detail', kwargs=detail_kwargs) + response = self.client.get(detail_url) + + assert response.status_code == status.HTTP_200_OK + assert response.json() == { + 'uuid': str(self.requester_assignment_accepted.uuid), + 'assignment_configuration': str(self.requester_assignment_accepted.assignment_configuration.uuid), + 'content_key': self.requester_assignment_accepted.content_key, + 'content_quantity': self.requester_assignment_accepted.content_quantity, + 'last_notification_at': None, + 'learner_email': self.requester_assignment_accepted.learner_email, + 'lms_user_id': self.requester_assignment_accepted.lms_user_id, + 'state': LearnerContentAssignmentStateChoices.ACCEPTED, + 'transaction_uuid': str(self.requester_assignment_accepted.transaction_uuid), + } + + def test_retrieve_other_assignment_not_found(self): + """ + Tests that we get expected 40x responses when learner A attempts to retrieve learner B's assignment. + """ + self.set_jwt_cookie([{ + 'system_wide_role': SYSTEM_ENTERPRISE_LEARNER_ROLE, + 'context': str(TEST_ENTERPRISE_UUID) + }]) + + detail_kwargs = { + 'assignment_configuration_uuid': str(TEST_ASSIGNMENT_CONFIG_UUID), + 'uuid': str(self.assignment_accepted.uuid), + } + detail_url = reverse('api:v1:assignments-detail', kwargs=detail_kwargs) + + # GET/retrieve endpoint: + response = self.client.get(detail_url) + assert response.status_code == status.HTTP_404_NOT_FOUND + + @ddt.data( + # A good learner role, and with a context matching the main testing customer. + {'system_wide_role': SYSTEM_ENTERPRISE_LEARNER_ROLE, 'context': str(TEST_ENTERPRISE_UUID)}, + # A good admin role, and with a context matching the main testing customer. + {'system_wide_role': SYSTEM_ENTERPRISE_ADMIN_ROLE, 'context': str(TEST_ENTERPRISE_UUID)}, + # A good operator role, and with a context matching the main testing customer. + {'system_wide_role': SYSTEM_ENTERPRISE_OPERATOR_ROLE, 'context': str(TEST_ENTERPRISE_UUID)}, + ) + def test_list(self, role_context_dict): + """ + Test that the list view returns a 200 response code and the expected (list) results of serialization.. + + This also tests that only Assignments for the requesting user are returned. + """ + # Set the JWT-based auth that we'll use for every request. + self.set_jwt_cookie([role_context_dict]) + + # Send a list request for all Assignments for the requesting user. + response = self.client.get(ASSIGNMENTS_LIST_ENDPOINT) + + # Only Assignments that match the following qualifications are returned: + # 1. Assignment is for the requesting user. + # 2. Assignment is in the requested AssignementConfiguration. + expected_assignments_for_requester = [ + self.requester_assignment_accepted, + self.requester_assignment_cancelled, + self.requester_assignment_errored, + ] + expected_assignment_uuids = {assignment.uuid for assignment in expected_assignments_for_requester} + actual_assignment_uuids = {UUID(assignment['uuid']) for assignment in response.json()['results']} + assert actual_assignment_uuids == expected_assignment_uuids diff --git a/enterprise_access/apps/api/v1/views/content_assignments/assignments.py b/enterprise_access/apps/api/v1/views/content_assignments/assignments.py index 692df592..df260e11 100644 --- a/enterprise_access/apps/api/v1/views/content_assignments/assignments.py +++ b/enterprise_access/apps/api/v1/views/content_assignments/assignments.py @@ -8,7 +8,7 @@ from edx_rest_framework_extensions.auth.jwt.authentication import JwtAuthentication from rest_framework import authentication, mixins, permissions, status, viewsets -from enterprise_access.apps.api import filters, serializers +from enterprise_access.apps.api import filters, serializers, utils from enterprise_access.apps.api.v1.views.utils import PaginationWithPageCount from enterprise_access.apps.content_assignments.models import LearnerContentAssignment from enterprise_access.apps.core.constants import CONTENT_ASSIGNMENT_LEARNER_READ_PERMISSION @@ -18,6 +18,16 @@ CONTENT_ASSIGNMENT_CRUD_API_TAG = 'Content Assignment CRUD' +def assignment_permission_fn(request, *args, assignment_configuration_uuid=None, **kwargs): + """ + Helper to use with @permission_required on all endpoints. + + Args: + assignment_configuration_uuid (str): UUID representing a LearnerContentAssignment object. + """ + return utils.get_assignment_config_customer_uuid(assignment_configuration_uuid) + + class LearnerContentAssignmentViewSet( mixins.ListModelMixin, mixins.RetrieveModelMixin, @@ -65,7 +75,7 @@ def get_queryset(self): status.HTTP_404_NOT_FOUND: None, }, ) - @permission_required(CONTENT_ASSIGNMENT_LEARNER_READ_PERMISSION) + @permission_required(CONTENT_ASSIGNMENT_LEARNER_READ_PERMISSION, fn=assignment_permission_fn) def retrieve(self, request, *args, uuid=None, **kwargs): """ Retrieves a single ``LearnerContentAssignment`` record by uuid, if assigned to the requesting user for this @@ -77,7 +87,7 @@ def retrieve(self, request, *args, uuid=None, **kwargs): tags=[CONTENT_ASSIGNMENT_CRUD_API_TAG], summary='List content assignments.', ) - @permission_required(CONTENT_ASSIGNMENT_LEARNER_READ_PERMISSION) + @permission_required(CONTENT_ASSIGNMENT_LEARNER_READ_PERMISSION, fn=assignment_permission_fn) def list(self, request, *args, **kwargs): """ Lists ``LearnerContentAssignment`` records assigned to the requesting user for the given assignment