Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
81 changes: 25 additions & 56 deletions lms/djangoapps/course_home_api/course_metadata/tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
from django.test.utils import override_settings
from django.urls import reverse
from edx_toggles.toggles.testutils import override_waffle_flag
from openedx_filters.learning.filters import CoursewareAccessChecksRequested

from common.djangoapps.course_modes.models import CourseMode
from common.djangoapps.student.models import CourseEnrollment
Expand All @@ -26,10 +27,6 @@
COURSEWARE_MICROFRONTEND_PROGRESS_MILESTONES_STREAK_CELEBRATION,
)
from openedx.core.djangoapps.discussions.models import DiscussionsConfiguration
from openedx.features.enterprise_support.tests.factories import (
EnterpriseCourseEnrollmentFactory,
EnterpriseCustomerUserFactory,
)


@ddt.ddt
Expand Down Expand Up @@ -162,7 +159,7 @@ def test_catalog_visibility_none_staff_gets_200(self):
'enroll_user': True,
'instructor_role': False,
'masquerade_role': None,
'dsc_required': False,
'filter_denies_access': False,
'expect_course_access': True,
'error_code': None,
},
Expand All @@ -171,7 +168,7 @@ def test_catalog_visibility_none_staff_gets_200(self):
'enroll_user': False,
'instructor_role': False,
'masquerade_role': None,
'dsc_required': False,
'filter_denies_access': False,
'expect_course_access': False,
'error_code': 'enrollment_required'
},
Expand All @@ -180,7 +177,7 @@ def test_catalog_visibility_none_staff_gets_200(self):
'enroll_user': False,
'instructor_role': True,
'masquerade_role': None,
'dsc_required': False,
'filter_denies_access': False,
'expect_course_access': True,
'error_code': None
},
Expand All @@ -189,32 +186,32 @@ def test_catalog_visibility_none_staff_gets_200(self):
'enroll_user': False,
'instructor_role': True,
'masquerade_role': 'student',
'dsc_required': False,
'filter_denies_access': False,
'expect_course_access': True,
'error_code': None
},
{
# Data sharing Consent required learners should Not have access.
# Learners denied by an access-checks pipeline step should NOT have access.
'enroll_user': True,
'instructor_role': False,
'masquerade_role': None,
'dsc_required': True,
'filter_denies_access': True,
'expect_course_access': False,
'error_code': 'data_sharing_access_required'
'error_code': 'access_denied_by_filter'
},
{
# Data sharing Consent required staff should Not have access.
# Staff denied by an access-checks pipeline step should NOT have access.
'enroll_user': True,
'instructor_role': True,
'masquerade_role': None,
'dsc_required': True,
'filter_denies_access': True,
'expect_course_access': False,
'error_code': 'data_sharing_access_required'
'error_code': 'access_denied_by_filter'
}
)
@ddt.unpack
def test_course_access(
self, enroll_user, instructor_role, masquerade_role, dsc_required, expect_course_access, error_code
self, enroll_user, instructor_role, masquerade_role, filter_denies_access, expect_course_access, error_code
):
"""
Test that course_access is calculated correctly based on
Expand All @@ -227,51 +224,23 @@ def test_course_access(
if masquerade_role:
self.update_masquerade(role=masquerade_role)

consent_url = 'dump/consent/url' if dsc_required else None
with patch('openedx.features.enterprise_support.api.get_enterprise_consent_url', return_value=consent_url):
if filter_denies_access:
mock_side_effect = CoursewareAccessChecksRequested.PreventCoursewareAccess(
message='Access denied by a courseware access-checks pipeline step',
error_code='access_denied_by_filter',
developer_message='https://example.com/redirect',
user_message='You are not allowed to access this course',
)
with patch(
'openedx_filters.learning.filters.CoursewareAccessChecksRequested.run_filter',
side_effect=mock_side_effect,
):
response = self.client.get(self.url)
else:
response = self.client.get(self.url)

self._assert_course_access_response(response, expect_course_access, error_code)

@ddt.data(True, False)
def test_course_access_with_correct_active_enterprise(self, instructor_role):
"""
Test that course_access is calculated correctly based on
access to MFE and access to the course itself.
"""
if instructor_role:
CourseInstructorRole(self.course.id).add_users(self.user)

# Test with no EnterpriseCourseEnrollment
course_enrollment = CourseEnrollment.enroll(self.user, self.course.id, 'audit')
response = self.client.get(self.url)
self._assert_course_access_response(response, True, None)

# Test with EnterpriseCourseEnrollment and having correct active enterprise
course = course_enrollment.course
enterprise_customer_user = EnterpriseCustomerUserFactory(user_id=self.user.id)
EnterpriseCourseEnrollmentFactory(enterprise_customer_user=enterprise_customer_user, course_id=course.id)
response = self.client.get(self.url)
self._assert_course_access_response(response, True, None)

# Test with incorrect active enterprise
enterprise_customer_user_2 = EnterpriseCustomerUserFactory(user_id=self.user.id, active=True)
enterprise_customer_user.refresh_from_db()
assert not enterprise_customer_user.active
assert enterprise_customer_user_2.active
response = self.client.get(self.url)
self._assert_course_access_response(response, False, 'incorrect_active_enterprise')

# test when no active enterprise at all (ideally this should never happen)
enterprise_customer_user_2.active = False
enterprise_customer_user_2.save()
enterprise_customer_user.refresh_from_db()
enterprise_customer_user_2.refresh_from_db()
assert not enterprise_customer_user.active
assert not enterprise_customer_user_2.active
response = self.client.get(self.url)
self._assert_course_access_response(response, False, 'incorrect_active_enterprise')

@patch.dict("django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True})
@ddt.data(True, False)
def test_discussion_tab_visible(self, visible):
Expand Down
2 changes: 1 addition & 1 deletion lms/djangoapps/course_home_api/course_metadata/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ def get(self, request, *args, **kwargs):
'load',
check_if_enrolled=True,
check_if_authenticated=True,
apply_enterprise_checks=True,
apply_priority_access_checks=True,
)

_, request.user = setup_masquerade(
Expand Down
11 changes: 6 additions & 5 deletions lms/djangoapps/course_wiki/middleware.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,13 @@
from django.http import Http404
from django.shortcuts import redirect
from django.utils.deprecation import MiddlewareMixin
from openedx_filters.learning.filters import CoursewareViewStarted
from wiki.models import reverse

from common.djangoapps.student.models import CourseEnrollment
from lms.djangoapps.courseware.access import has_access
from lms.djangoapps.courseware.courses import get_course_overview_with_access, get_course_with_access
from openedx.core.lib.request_utils import course_id_from_url
from openedx.features.enterprise_support.api import get_enterprise_consent_url
from xmodule.modulestore.django import modulestore


Expand Down Expand Up @@ -96,10 +96,11 @@ def process_view(self, request, view_func, view_args, view_kwargs): # pylint: d
# we'll redirect them to the course about page
return redirect('about_course', str(course_id))

# If we need enterprise data sharing consent for this course, then redirect to the form.
consent_url = get_enterprise_consent_url(request, str(course_id), source='WikiAccessMiddleware')
if consent_url:
return redirect(consent_url)
# If a plugin requires a redirect for this course, redirect now.
try:
CoursewareViewStarted.run_filter(course_key=course_id, view_name='WikiAccessMiddleware')
except CoursewareViewStarted.RedirectToUrl as exc:
return redirect(exc.redirect_to)

# set the course onto here so that the wiki template can show the course navigation
request.course = course
Expand Down
34 changes: 20 additions & 14 deletions lms/djangoapps/course_wiki/tests/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,17 @@
from unittest.mock import patch

from django.urls import reverse
from openedx_filters.learning.filters import CoursewareViewStarted

from lms.djangoapps.courseware.tests.tests import LoginEnrollmentTestCase
from openedx.features.course_experience.url_helpers import make_learning_mfe_courseware_url
from openedx.features.enterprise_support.tests.mixins.enterprise import EnterpriseTestConsentRequired
from xmodule.modulestore.tests.django_utils import (
ModuleStoreTestCase, # pylint: disable=wrong-import-order
)
from xmodule.modulestore.tests.factories import CourseFactory # pylint: disable=wrong-import-order


class WikiRedirectTestCase(EnterpriseTestConsentRequired, LoginEnrollmentTestCase, ModuleStoreTestCase):
class WikiRedirectTestCase(LoginEnrollmentTestCase, ModuleStoreTestCase):
"""
Tests for wiki course redirection.
"""
Expand Down Expand Up @@ -205,27 +205,33 @@ def test_create_wiki_with_long_course_id(self):
assert resp.status_code == 200

@patch.dict("django.conf.settings.FEATURES", {'ALLOW_WIKI_ROOT_ACCESS': True})
@patch('openedx.features.enterprise_support.api.enterprise_customer_for_request')
def test_consent_required(self, mock_enterprise_customer_for_request):
@patch('openedx_filters.learning.filters.CoursewareViewStarted.run_filter')
def test_filter_redirect(self, mock_run_filter):
"""
Test that enterprise data sharing consent is required when enabled for the various courseware views.
Test that wiki views redirect when the CoursewareViewStarted filter provides a URL.
"""
# ENT-924: Temporary solution to replace sensitive SSO usernames.
mock_enterprise_customer_for_request.return_value = None
redirect_url = 'http://example.com/redirect'
mock_run_filter.side_effect = CoursewareViewStarted.RedirectToUrl(message="redirect", redirect_to=redirect_url)

# Public wikis can be accessed by non-enrolled users, and so direct access is not gated by the consent page
# Public wikis can be accessed by non-enrolled users, and so direct access is not gated by the redirect
course = CourseFactory.create()
course.allow_public_wiki_access = False
course.save()

# However, for private wikis, enrolled users must pass through the consent gate
# However, for private wikis, enrolled users must pass through the filter redirect gate
# (Unenrolled users are redirected to course/about)
course_id = str(course.id)
self.login(self.student, self.password)
self.enroll(course)

for (url, status_code) in (
(reverse('course_wiki', kwargs={'course_id': course_id}), 302),
(f'/courses/{course_id}/wiki/', 200),
):
self.verify_consent_required(self.client, url, status_code=status_code) # pylint: disable=no-value-for-parameter
# The course_wiki view is decorated with courseware_view_hooks which calls the filter
url = reverse('course_wiki', kwargs={'course_id': course_id})
response = self.client.get(url)
assert response.status_code == 302
assert response['Location'] == redirect_url

# The wiki middleware (/courses/.../wiki/) also calls the filter
url = f'/courses/{course_id}/wiki/'
response = self.client.get(url)
assert response.status_code == 302
assert response['Location'] == redirect_url
4 changes: 2 additions & 2 deletions lms/djangoapps/course_wiki/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,10 @@
from wiki.models import Article, URLPath

from lms.djangoapps.course_wiki.utils import course_wiki_slug
from lms.djangoapps.courseware.decorators import courseware_view_hooks
from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers
from openedx.core.djangolib.markup import Text
from openedx.core.lib.courses import get_course_by_id
from openedx.features.enterprise_support.api import data_sharing_consent_required

log = logging.getLogger(__name__)

Expand All @@ -31,7 +31,7 @@ def root_create(request):
return redirect('wiki:get', path=root.path)


@data_sharing_consent_required
@courseware_view_hooks
def course_wiki_redirect(request, course_id, wiki_path=""):
"""
This redirects to whatever page on the wiki that the course designates
Expand Down
71 changes: 21 additions & 50 deletions lms/djangoapps/courseware/access_response.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
"""
This file contains all the classes used by has_access for error handling
"""

from datetime import datetime

from django.utils.translation import gettext as _

Expand Down Expand Up @@ -124,46 +124,35 @@ class StartDateError(AccessError):
Access denied because the course has not started yet and the user
is not staff
"""
def __init__(self, start_date, display_error_to_user=True):
def __init__(
self,
start_date: datetime,
display_error_to_user=True,
error_code_override: str | None = None,
developer_message_override: str | None = None,
user_message_override: str | None = None,
):
"""
Arguments:
start_date: The future start date of the course, used for messaging.
display_error_to_user: If True, display this error to users in the UI.
error_code_override: Optional override error_code.
developer_message_override: Optional override developer_message.
user_message_override: Optional override user_message.
"""
error_code = "course_not_started"
error_code = error_code_override or "course_not_started"
if start_date == DEFAULT_START_DATE:
developer_message = "Course has not started"
user_message = _("Course has not started")
else:
developer_message = f"Course does not start until {start_date}"
user_message = _("Course does not start until {}" # pylint: disable=translation-of-non-string
.format(f"{start_date:%B %d, %Y}"))
super().__init__(
error_code,
developer_message,
user_message if display_error_to_user else None
)

# Use override message if available.
developer_message = developer_message_override or developer_message
user_message = user_message_override or user_message

class StartDateEnterpriseLearnerError(AccessError):
"""
Access denied because the course has not started yet and the user is not staff. Use this error when this user is
also an enterprise learner and enrolled in the requested course.
"""
def __init__(self, start_date, display_error_to_user=True):
"""
Arguments:
display_error_to_user: If True, display this error to users in the UI.
"""
error_code = "course_not_started_enterprise_learner"
if start_date == DEFAULT_START_DATE:
developer_message = "Course has not started, and the learner is enrolled via an enterprise subsidy."
user_message = _("Course has not started")
else:
developer_message = (
f"Course does not start until {start_date}, and the learner is enrolled via an enterprise subsidy."
)
user_message = _("Course does not start until {}" # pylint: disable=translation-of-non-string
.format(f"{start_date:%B %d, %Y}"))
super().__init__(
error_code,
developer_message,
Expand Down Expand Up @@ -270,31 +259,13 @@ def __init__(self):
super().__init__(error_code, developer_message, user_message)


class IncorrectActiveEnterpriseAccessError(AccessError):
"""
Access denied because the user must login with correct enterprise.
class PriorityAccessFiltersError(AccessError):
"""
def __init__(self, enrollment_enterprise_name, active_enterprise_name):
error_code = "incorrect_active_enterprise"
developer_message = "User active enterprise should be same as EnterpriseCourseEnrollment enterprise."
user_message = _("You are enrolled in this course with '{enrollment_enterprise_name}'. However, you are "
"currently logged in as a '{active_enterprise_name}' user. Please log in with "
"'{enrollment_enterprise_name}' to access this course.")
user_message = user_message.format(
enrollment_enterprise_name=enrollment_enterprise_name, active_enterprise_name=active_enterprise_name
)
super().__init__(error_code, developer_message, user_message)

Access denied by a plugin via the CoursewareAccessChecksRequested filter.

class DataSharingConsentRequiredAccessError(AccessError):
Priority — non-bypassable by staff. The error_code, developer_message,
and user_message are supplied by the pipeline step that denied access.
"""
Access denied because the user must give Data sharing consent before access it.
"""
def __init__(self, consent_url):
error_code = "data_sharing_access_required"
developer_message = consent_url
user_message = _("You must give Data Sharing Consent for the course")
super().__init__(error_code, developer_message, user_message)


class AuthenticationRequiredAccessError(AccessError):
Expand Down
Loading
Loading