From 78a1b835fa9164f356f7baacc0ba8fe3992982a0 Mon Sep 17 00:00:00 2001
From: Dillon Dumesnil <ddumesnil@edx.org>
Date: Wed, 1 Apr 2020 13:45:37 -0700
Subject: [PATCH] Update the LMS courses API for staff permissions

We want to allow staff to see all courses in the LMS.
This changes the behavior from staff being treated like
an AnonymousUser (unless an username query parameter is
provided) to being treated like staff.

I also added in some tests for the other paths in this
function that did not seem to be tested.
---
 lms/djangoapps/course_api/api.py            |  6 ++++-
 lms/djangoapps/course_api/tests/test_api.py | 26 +++++++++++++++------
 2 files changed, 24 insertions(+), 8 deletions(-)

diff --git a/lms/djangoapps/course_api/api.py b/lms/djangoapps/course_api/api.py
index 8963a27805..245a8d31a2 100644
--- a/lms/djangoapps/course_api/api.py
+++ b/lms/djangoapps/course_api/api.py
@@ -38,9 +38,13 @@ def get_effective_user(requesting_user, target_username):
     """
     if target_username == requesting_user.username:
         return requesting_user
+    # This is the default behavior if username is not specified as a query parameter
+    # which is why the is_staff check is happening inside of here.
     elif target_username == '':
+        if requesting_user.is_staff:
+            return requesting_user
         return AnonymousUser()
-    elif can_view_courses_for_username(requesting_user, target_username):
+    elif target_username and can_view_courses_for_username(requesting_user, target_username):
         return User.objects.get(username=target_username)
     else:
         raise PermissionDenied()
diff --git a/lms/djangoapps/course_api/tests/test_api.py b/lms/djangoapps/course_api/tests/test_api.py
index dda51d7e7c..f7c6bdd9d5 100644
--- a/lms/djangoapps/course_api/tests/test_api.py
+++ b/lms/djangoapps/course_api/tests/test_api.py
@@ -46,7 +46,7 @@ class CourseDetailTestMixin(CourseApiTestMixin):
     """
     ENABLED_SIGNALS = ['course_published']
 
-    def _make_api_call(self, requesting_user, target_user, course_key):
+    def _make_api_call(self, requesting_user, target_username, course_key):
         """
         Call the `course_detail` api endpoint to get information on the course
         identified by `course_key`.
@@ -54,7 +54,7 @@ class CourseDetailTestMixin(CourseApiTestMixin):
         request = Request(self.request_factory.get('/'))
         request.user = requesting_user
         with check_mongo_calls(0):
-            return course_detail(request, target_user.username, course_key)
+            return course_detail(request, target_username, course_key)
 
 
 class TestGetCourseDetail(CourseDetailTestMixin, SharedModuleStoreTestCase):
@@ -71,25 +71,37 @@ class TestGetCourseDetail(CourseDetailTestMixin, SharedModuleStoreTestCase):
         cls.staff_user = cls.create_user('staff', is_staff=True)
 
     def test_get_existing_course(self):
-        course = self._make_api_call(self.honor_user, self.honor_user, self.course.id)
+        course = self._make_api_call(self.honor_user, self.honor_user.username, self.course.id)
+        self.verify_course(course)
+
+    def test_get_existing_course_as_anonymous_user(self):
+        course = self._make_api_call(self.honor_user, '', self.course.id)
         self.verify_course(course)
 
     def test_get_nonexistent_course(self):
         course_key = CourseKey.from_string(u'edX/toy/nope')
         with self.assertRaises(Http404):
-            self._make_api_call(self.honor_user, self.honor_user, course_key)
+            self._make_api_call(self.honor_user, self.honor_user.username, course_key)
 
     def test_hidden_course_for_honor(self):
         with self.assertRaises(Http404):
-            self._make_api_call(self.honor_user, self.honor_user, self.hidden_course.id)
+            self._make_api_call(self.honor_user, self.honor_user.username, self.hidden_course.id)
 
     def test_hidden_course_for_staff(self):
-        course = self._make_api_call(self.staff_user, self.staff_user, self.hidden_course.id)
+        course = self._make_api_call(self.staff_user, self.staff_user.username, self.hidden_course.id)
+        self.verify_course(course, course_id=u'edX/hidden/2012_Fall')
+
+    def test_hidden_course_for_staff_no_target_username(self):
+        course = self._make_api_call(self.staff_user, '', self.hidden_course.id)
         self.verify_course(course, course_id=u'edX/hidden/2012_Fall')
 
     def test_hidden_course_for_staff_as_honor(self):
         with self.assertRaises(Http404):
-            self._make_api_call(self.staff_user, self.honor_user, self.hidden_course.id)
+            self._make_api_call(self.staff_user, self.honor_user.username, self.hidden_course.id)
+
+    def test_permission_denied(self):
+        with self.assertRaises(PermissionDenied):
+            self._make_api_call(self.staff_user, None, self.hidden_course.id)
 
 
 class CourseListTestMixin(CourseApiTestMixin):