feat: return 403 with descriptive error for catalog visibility restricted courses

lms/djangoapps/course_home_api/course_metadata/tests/test_views.py

@@ -7,6 +7,7 @@
 import ddt
 from django.db import transaction
 from django.urls import reverse
+from django.test.utils import override_settings
 from edx_toggles.toggles.testutils import override_waffle_flag
 
 from common.djangoapps.course_modes.models import CourseMode
@@ -118,6 +119,42 @@ def test_streak_data_in_response(self):
                 assert celebrations['streak_length_to_celebrate'] == 3
                 assert celebrations['streak_discount_enabled'] is True
 
+    @override_settings(COURSE_ABOUT_VISIBILITY_PERMISSION='see_about_page')
+    def test_catalog_visibility_none_returns_403(self):
+        """
+        Test that a non-staff user gets a 403 response with a meaningful error
+        message when accessing a course with catalog_visibility='none'.
+        """
+        from lms.djangoapps.courseware.access_response import CatalogVisibilityError
+        from lms.djangoapps.courseware.exceptions import CourseAccessRedirect
+        error = CatalogVisibilityError()
+        exc = CourseAccessRedirect(reverse('dashboard'), error)
+        url = reverse('course-home:course-metadata', args=[self.course.id])
+        # We mock course_detail to raise CourseAccessRedirect with a
+        # CatalogVisibilityError.  We also mock set_rollback to prevent
+        # DRF from marking the test's transaction for rollback, which
+        # would cause TransactionManagementError in session middleware.
+        with patch(
+            'lms.djangoapps.course_home_api.course_metadata.views.course_detail',
+            side_effect=exc,
+        ), patch('rest_framework.views.set_rollback'):
+            response = self.client.get(url)
+        assert response.status_code == 403
+        response_data = response.json()
+        assert 'not currently accessible' in response_data['detail']
+
+    @override_settings(COURSE_ABOUT_VISIBILITY_PERMISSION='see_about_page')
+    def test_catalog_visibility_none_staff_gets_200(self):
+        """
+        Test that a staff user can still access a course with catalog_visibility='none',
+        even when COURSE_ABOUT_VISIBILITY_PERMISSION='see_about_page'.
+        Staff bypasses catalog visibility checks.
+        """
+        self.switch_to_staff()
+        url = reverse('course-home:course-metadata', args=[self.course.id])
+        response = self.client.get(url)
+        assert response.status_code == 200
+
     @ddt.data(
         # Who has access to MFE courseware?
         {

lms/djangoapps/course_home_api/course_metadata/views.py

@@ -5,6 +5,7 @@
 from django.db import transaction
 from django.utils.decorators import method_decorator
 from opaque_keys.edx.keys import CourseKey
+from rest_framework.exceptions import PermissionDenied
 from rest_framework.generics import RetrieveAPIView
 from rest_framework.response import Response
 
@@ -24,6 +25,7 @@
 from lms.djangoapps.courseware.access import has_access, has_cms_access
 from lms.djangoapps.courseware.context_processor import user_timezone_locale_prefs
 from lms.djangoapps.courseware.courses import check_course_access
+from lms.djangoapps.courseware.exceptions import CourseAccessRedirect
 from lms.djangoapps.courseware.masquerade import setup_masquerade
 from lms.djangoapps.courseware.tabs import get_course_tab_list
 
@@ -83,7 +85,14 @@ def get(self, request, *args, **kwargs):
         original_user_is_global_staff = self.request.user.is_staff
         original_user_is_staff = has_access(request.user, 'staff', course_key).has_access
 
-        course = course_detail(request, request.user.username, course_key)
+        try:
+            course = course_detail(request, request.user.username, course_key)
+        except CourseAccessRedirect as e:
+            error = e.access_error
+            raise PermissionDenied(
+                detail=error.user_message if error is not None else 'You do not have access to this course.',
+                code=error.error_code if error is not None else None,
+            ) from e
 
         # We must compute course load access *before* setting up masquerading,
         # else course staff (who are not enrolled) will not be able view

lms/djangoapps/courseware/access.py

@@ -21,6 +21,7 @@
 from xblock.core import XBlock
 
 from lms.djangoapps.courseware.access_response import (
+    CatalogVisibilityError,
     IncorrectPartitionGroupError,
     MilestoneAccessError,
     MobileAvailabilityError,
@@ -412,10 +413,14 @@ def can_see_in_catalog():
         In this case we use the catalog_visibility property on the course block
         but also allow course staff to see this.
         """
-        return (
-            _has_catalog_visibility(courselike, CATALOG_VISIBILITY_CATALOG_AND_ABOUT)
-            or _has_staff_access_to_block(user, courselike, courselike.id)
-        )
+        catalog_response = _has_catalog_visibility(courselike, CATALOG_VISIBILITY_CATALOG_AND_ABOUT)
+        if catalog_response:
+            return ACCESS_GRANTED
+        if _has_staff_access_to_block(user, courselike, courselike.id):
+            return ACCESS_GRANTED
+        # Return the typed CatalogVisibilityError so downstream handlers
+        # can provide a meaningful error message instead of a generic 404.
+        return catalog_response
 
     @function_trace('can_see_about_page')
     def can_see_about_page():
@@ -424,11 +429,17 @@ def can_see_about_page():
         In this case we use the catalog_visibility property on the course block
         but also allow course staff to see this.
         """
-        return (
-            _has_catalog_visibility(courselike, CATALOG_VISIBILITY_CATALOG_AND_ABOUT)
-            or _has_catalog_visibility(courselike, CATALOG_VISIBILITY_ABOUT)
-            or _has_staff_access_to_block(user, courselike, courselike.id)
-        )
+        both_response = _has_catalog_visibility(courselike, CATALOG_VISIBILITY_CATALOG_AND_ABOUT)
+        if both_response:
+            return ACCESS_GRANTED
+        about_response = _has_catalog_visibility(courselike, CATALOG_VISIBILITY_ABOUT)
+        if about_response:
+            return ACCESS_GRANTED
+        if _has_staff_access_to_block(user, courselike, courselike.id):
+            return ACCESS_GRANTED
+        # Return the typed CatalogVisibilityError so downstream handlers
+        # can provide a meaningful error message instead of a generic 404.
+        return both_response
 
     checkers = {
         'load': can_load,
@@ -876,7 +887,9 @@ def _has_catalog_visibility(course, visibility_type):
     """
     Returns whether the given course has the given visibility type
     """
-    return ACCESS_GRANTED if course.catalog_visibility == visibility_type else ACCESS_DENIED
+    if course.catalog_visibility == visibility_type:
+        return ACCESS_GRANTED
+    return CatalogVisibilityError()
 
 
 def _is_block_mobile_available(block):

lms/djangoapps/courseware/access_response.py

@@ -184,7 +184,7 @@ def __init__(self):
 
 class VisibilityError(AccessError):
     """
-    Access denied because the user does have the correct role to view this
+    Access denied because the user does not have the correct role to view this
     course.
     """
     def __init__(self, display_error_to_user=True):
@@ -203,6 +203,22 @@ def __init__(self, display_error_to_user=True):
         )
 
 
+class CatalogVisibilityError(AccessError):
+    """
+    Access denied because the course's catalog_visibility setting prevents
+    the user from seeing this course.
+    """
+    def __init__(self):
+        error_code = "not_visible_in_catalog"
+        developer_message = "Course catalog visibility setting prevents access"
+        user_message = _(
+            "This course is not currently accessible. "
+            "The course team has restricted access to this content. "
+            "Please contact the course team for further assistance."
+        )
+        super().__init__(error_code, developer_message, user_message)
+
+
 class MobileAvailabilityError(AccessError):
     """
     Access denied because the course is not available on mobile for the user

lms/djangoapps/courseware/courses.py

@@ -30,10 +30,11 @@
 from lms.djangoapps.courseware.access import has_access
 from lms.djangoapps.courseware.access_response import (
     AuthenticationRequiredAccessError,
+    CatalogVisibilityError,
     EnrollmentRequiredAccessError,
     MilestoneAccessError,
     OldMongoAccessError,
-    StartDateError
+    StartDateError,
 )
 from lms.djangoapps.courseware.access_utils import check_authentication, check_data_sharing_consent, check_enrollment, \
     check_correct_active_enterprise_customer, is_priority_access_error
@@ -288,6 +289,12 @@ def check_course_access_with_redirect(
         if isinstance(access_response, AuthenticationRequiredAccessError):
             raise CourseAccessRedirect(reverse('about_course', args=[str(course.id)]))
 
+        # Redirect if the course catalog visibility prevents access
+        if isinstance(access_response, CatalogVisibilityError):
+            raise CourseAccessRedirect('{dashboard_url}'.format(
+                dashboard_url=reverse('dashboard'),
+            ), access_response)
+
         # Redirect if the user must answer a survey before entering the course.
         if isinstance(access_response, SurveyRequiredAccessError):
             raise CourseAccessRedirect(reverse('course_survey', args=[str(course.id)]))

lms/djangoapps/courseware/tests/test_about.py

@@ -111,7 +111,7 @@ def test_visible_about_page_settings(self):
 
         url = reverse('about_course', args=[str(self.course_without_about.id)])
         resp = self.client.get(url)
-        assert resp.status_code == 404
+        self.assertRedirects(resp, reverse('dashboard'), fetch_redirect_response=False)
 
     @patch.dict(settings.FEATURES, {'ENABLE_MKTG_SITE': True})
     def test_logged_in_marketing(self):

lms/djangoapps/courseware/tests/test_access.py

@@ -682,6 +682,47 @@ def test__catalog_visibility(self):
         assert access._has_access_course(staff, 'see_in_catalog', course)
         assert access._has_access_course(staff, 'see_about_page', course)
 
+    def test__catalog_visibility_returns_typed_error(self):
+        """
+        Tests that when catalog_visibility is 'none', the access response
+        for non-staff users is a CatalogVisibilityError (not a bare ACCESS_DENIED),
+        so downstream handlers can provide a meaningful error message.
+        """
+        user = UserFactory.create()
+        course_id = CourseLocator('edX', 'test', '2012_Fall')
+        staff = StaffFactory.create(course_key=course_id)
+
+        course = Mock(
+            id=course_id,
+            catalog_visibility=CATALOG_VISIBILITY_NONE
+        )
+
+        # Non-staff user should get CatalogVisibilityError
+        see_in_catalog_response = access._has_access_course(user, 'see_in_catalog', course)
+        assert not see_in_catalog_response
+        assert isinstance(see_in_catalog_response, access_response.CatalogVisibilityError)
+        assert see_in_catalog_response.error_code == 'not_visible_in_catalog'
+
+        see_about_page_response = access._has_access_course(user, 'see_about_page', course)
+        assert not see_about_page_response
+        assert isinstance(see_about_page_response, access_response.CatalogVisibilityError)
+        assert see_about_page_response.error_code == 'not_visible_in_catalog'
+
+        # Staff user should still get access
+        assert access._has_access_course(staff, 'see_in_catalog', course)
+        assert access._has_access_course(staff, 'see_about_page', course)
+
+        # When visibility is 'about', see_in_catalog should return CatalogVisibilityError
+        # but see_about_page should grant access
+        course_about = Mock(
+            id=course_id,
+            catalog_visibility=CATALOG_VISIBILITY_ABOUT
+        )
+        see_in_catalog_response = access._has_access_course(user, 'see_in_catalog', course_about)
+        assert not see_in_catalog_response
+        assert isinstance(see_in_catalog_response, access_response.CatalogVisibilityError)
+        assert access._has_access_course(user, 'see_about_page', course_about)
+
     @patch.dict("django.conf.settings.FEATURES", {'ENABLE_PREREQUISITE_COURSES': True})
     @override_settings(MILESTONES_APP=True)
     def test_access_on_course_with_pre_requisites(self):

lms/djangoapps/courseware/tests/test_courses.py

@@ -42,6 +42,7 @@
 from lms.djangoapps.courseware.model_data import FieldDataCache
 from lms.djangoapps.courseware.block_render import get_block_for_descriptor
 from lms.djangoapps.courseware.courseware_access_exception import CoursewareAccessException
+from lms.djangoapps.courseware.exceptions import CourseAccessRedirect
 from openedx.core.djangolib.testing.utils import get_mock_request
 from openedx.core.lib.courses import course_image_url
 from common.djangoapps.student.tests.factories import UserFactory
@@ -87,6 +88,23 @@ def test_get_course_func_with_access_error(self, course_access_func_name):
         assert error.value.access_response.error_code == 'not_visible_to_user'
         assert not error.value.access_response.has_access
 
+    @ddt.data(GET_COURSE_WITH_ACCESS, GET_COURSE_OVERVIEW_WITH_ACCESS)
+    def test_get_course_func_with_catalog_visibility_error(self, course_access_func_name):
+        """
+        Test that accessing a course with catalog_visibility='none' via
+        'see_about_page' action raises CourseAccessRedirect with a
+        CatalogVisibilityError, not a generic CoursewareAccessException.
+        """
+        course_access_func = self.COURSE_ACCESS_FUNCS[course_access_func_name]
+        user = UserFactory.create()
+        course = CourseFactory.create(catalog_visibility='none', emit_signals=True)
+
+        with pytest.raises(CourseAccessRedirect) as error:
+            course_access_func(user, 'see_about_page', course.id)
+        assert error.value.access_error is not None
+        assert error.value.access_error.error_code == 'not_visible_in_catalog'
+        assert not error.value.access_error.has_access
+
     @ddt.data(
         (GET_COURSE_WITH_ACCESS, 1),
         (GET_COURSE_OVERVIEW_WITH_ACCESS, 0),