Skip to content

Commit

Permalink
fix: learner-facing assignments endpoints broken
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@pwnage101
pwnage101 committed Sep 28, 2023
1 parent 9532ad0 commit 5863ad9
Show file tree
Hide file tree
Showing 2 changed files with 162 additions and 6 deletions.
152 changes: 149 additions & 3 deletions enterprise_access/apps/api/v1/tests/test_assignment_views.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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):
"""
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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
16 changes: 13 additions & 3 deletions enterprise_access/apps/api/v1/views/content_assignments/assignments.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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,
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down

0 comments on commit 5863ad9

Please sign in to comment.