Skip to content

Commit

Permalink
Merge pull request #282 from openedx/pwnage101/ENT-7680-2
Browse files Browse the repository at this point in the history 
fix: learner-facing assignments endpoints broken
  • Loading branch information
@pwnage101
pwnage101 authored Sep 29, 2023
2 parents 9532ad0 + 5863ad9 commit 887ce7e
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 887ce7e

Please sign in to comment.