From 7f59688f9b79923122fc84457e1c667bd0dbbc4e Mon Sep 17 00:00:00 2001 From: Kyle McCormick Date: Thu, 17 Dec 2020 18:13:51 -0500 Subject: [PATCH] Course Blocks API: Return 403 instead of 500 for unknown courses (#25906) Anonymous users (like crawlers) requesting non-existent courses should get 403, just like logged-in users do. They instead raised a ValueError. --- lms/djangoapps/course_api/blocks/permissions.py | 8 ++++---- lms/djangoapps/course_api/blocks/tests/test_views.py | 7 ++++++- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/lms/djangoapps/course_api/blocks/permissions.py b/lms/djangoapps/course_api/blocks/permissions.py index 391ede90cf..6e32403fc2 100644 --- a/lms/djangoapps/course_api/blocks/permissions.py +++ b/lms/djangoapps/course_api/blocks/permissions.py @@ -8,6 +8,7 @@ from lms.djangoapps.courseware.access import has_access from lms.djangoapps.courseware.access_response import AccessResponse from lms.djangoapps.courseware.access_utils import ACCESS_DENIED, ACCESS_GRANTED, check_public_access from lms.djangoapps.courseware.courses import get_course +from openedx.core.djangoapps.content.course_overviews.models import CourseOverview from common.djangoapps.student.models import CourseEnrollment from common.djangoapps.student.roles import CourseStaffRole from xmodule.course_module import COURSE_VISIBILITY_PUBLIC @@ -39,15 +40,14 @@ def can_access_self_blocks(requesting_user: User, course_key: CourseKey) -> Acce ) if user_is_enrolled_or_staff: return ACCESS_GRANTED - try: - return is_course_public(course_key) - except ValueError: - return ACCESS_DENIED + return is_course_public(course_key) def is_course_public(course_key: CourseKey) -> AccessResponse: """ This checks if a course is publicly accessible or not. """ + if not CourseOverview.course_exists(course_key): + return ACCESS_DENIED course = get_course(course_key, depth=0) return check_public_access(course, [COURSE_VISIBILITY_PUBLIC]) diff --git a/lms/djangoapps/course_api/blocks/tests/test_views.py b/lms/djangoapps/course_api/blocks/tests/test_views.py index 71410c64fe..ccdf3a0c9c 100644 --- a/lms/djangoapps/course_api/blocks/tests/test_views.py +++ b/lms/djangoapps/course_api/blocks/tests/test_views.py @@ -395,7 +395,7 @@ class TestBlocksView(SharedModuleStoreTestCase): self.verify_response_with_requested_fields(response) -class TestBlocksInCourseView(TestBlocksView): +class TestBlocksInCourseView(TestBlocksView): # pylint: disable=test-inherits-tests """ Test class for BlocksInCourseView """ @@ -414,3 +414,8 @@ class TestBlocksInCourseView(TestBlocksView): def test_non_existent_course(self): self.verify_response(403, params={'course_id': str(CourseLocator('non', 'existent', 'course'))}) + + def test_non_existent_course_anonymous(self): + self.client.logout() + self.query_params['username'] = '' + self.verify_response(403, params={'course_id': str(CourseLocator('non', 'existent', 'course'))})