From 6eaea58c9a4e527d28fb17f8aa9343c2cdd82889 Mon Sep 17 00:00:00 2001
From: Zacharis278 <zhancock@edx.org>
Date: Wed, 31 Jul 2024 10:46:51 -0400
Subject: [PATCH] feat: fix auth issue

---
 edx_exams/apps/api/v1/tests/test_views.py | 27 +++++++++++++++++
 edx_exams/apps/api/v1/views.py            | 37 +++++++++++++----------
 2 files changed, 48 insertions(+), 16 deletions(-)

diff --git a/edx_exams/apps/api/v1/tests/test_views.py b/edx_exams/apps/api/v1/tests/test_views.py
index e3f0b2ce..6be0f561 100644
--- a/edx_exams/apps/api/v1/tests/test_views.py
+++ b/edx_exams/apps/api/v1/tests/test_views.py
@@ -1900,6 +1900,18 @@ def test_delete_not_found(self):
         response = self.request_api('delete', self.user, self.exam.course_id, allowance_id=19)
         self.assertEqual(response.status_code, 404)
 
+    def test_delete_not_allowed(self):
+        """
+        Test that 404 is returned if the allowance is not for the authorized course
+        """
+        other_course_id = 'course-v1:edx+another+course'
+        allowance = StudentAllowanceFactory.create(
+            exam=ExamFactory.create(course_id=other_course_id),
+            user=self.user,
+        )
+        response = self.request_api('delete', self.user, self.exam.course_id, allowance_id=allowance.id)
+        self.assertEqual(response.status_code, 404)
+
     def test_post_allowances(self):
         """
         Test that the endpoint creates allowances for the given request data
@@ -1958,3 +1970,18 @@ def test_post_invalid_missing_user(self):
         ]
         response = self.request_api('post', self.user, self.exam.course_id, data=request_data)
         self.assertEqual(response.status_code, 400)
+
+    def test_post_unauthorized_course(self):
+        """
+        Test that an error is returned when atttempting to create an allowance for
+        an exam in a different course.
+        """
+        other_course_id = 'course-v1:edx+another+course'
+        other_course_exam = ExamFactory.create(course_id=other_course_id)
+
+        request_data = [
+            {'exam_id': self.exam.id, 'username': self.user.username, 'extra_time_mins': 45},
+            {'exam_id': other_course_exam.id, 'username': self.user.username, 'extra_time_mins': 45},
+        ]
+        response = self.request_api('post', self.user, self.exam.course_id, data=request_data)
+        self.assertEqual(response.status_code, 400)
diff --git a/edx_exams/apps/api/v1/views.py b/edx_exams/apps/api/v1/views.py
index 00645bb5..20451c05 100644
--- a/edx_exams/apps/api/v1/views.py
+++ b/edx_exams/apps/api/v1/views.py
@@ -828,14 +828,12 @@ def get(self, request, course_id):
         allowances = StudentAllowance.get_allowances_for_course(course_id)
         return Response(AllowanceSerializer(allowances, many=True).data)
 
-    def delete(self, request, course_id, allowance_id):  # pylint: disable=unused-argument
+    def delete(self, request, course_id, allowance_id):
         """
         HTTP DELETE handler. Deletes all allowances for a course.
-
-        TODO: both this and the POST endpoint should be limiting operations by course_id
         """
         try:
-            StudentAllowance.objects.get(id=allowance_id).delete()
+            StudentAllowance.objects.get(id=allowance_id, exam__course_id=course_id).delete()
         except StudentAllowance.DoesNotExist:
             return Response(
                 status=status.HTTP_404_NOT_FOUND,
@@ -844,7 +842,7 @@ def delete(self, request, course_id, allowance_id):  # pylint: disable=unused-ar
 
         return Response(status=status.HTTP_204_NO_CONTENT)
 
-    def post(self, request, course_id):  # pylint: disable=unused-argument
+    def post(self, request, course_id):
         """
         HTTP POST handler. Creates allowances based on the given list.
         """
@@ -856,18 +854,25 @@ def post(self, request, course_id):  # pylint: disable=unused-argument
             # We expect the number of allowances in each request to be small. Should they increase,
             # we should not query within the loop, and instead refactor this to optimize
             # the DB calls.
-            allowance_objects = [
-                StudentAllowance(
-                    user=(
-                        User.objects.get(username=allowance['username'])
-                        if allowance.get('username')
-                        else User.objects.get(email=allowance['email'])
-                    ),
-                    exam=Exam.objects.get(id=allowance['exam_id']),
-                    extra_time_mins=allowance['extra_time_mins']
+            try:
+                allowance_objects = [
+                    StudentAllowance(
+                        user=(
+                            User.objects.get(username=allowance['username'])
+                            if allowance.get('username')
+                            else User.objects.get(email=allowance['email'])
+                        ),
+                        exam=Exam.objects.get(id=allowance['exam_id'], course_id=course_id),
+                        extra_time_mins=allowance['extra_time_mins']
+                    )
+                    for allowance in allowances
+                ]
+            except Exam.DoesNotExist:
+                return Response(
+                    status=status.HTTP_400_BAD_REQUEST,
+                    data={'detail': 'Exam does not exist'}
                 )
-                for allowance in allowances
-            ]
+
             StudentAllowance.objects.bulk_create(
                 allowance_objects,
                 update_conflicts=True,