Skip to content
Open
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
21 changes: 1 addition & 20 deletions common/djangoapps/student/views/dashboard.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,12 +58,7 @@
from openedx.core.djangolib.markup import HTML, Text
from openedx.features.content_type_gating.models import ContentTypeGatingConfig
from openedx.features.course_duration_limits.access import get_user_course_duration, get_user_course_expiration_date
from openedx.features.enterprise_support.api import (
get_dashboard_consent_notification,
get_enterprise_learner_portal_context,
)
from openedx.features.enterprise_support.utils import is_enterprise_learner
from xmodule.modulestore.django import modulestore # pylint: disable=wrong-import-order
from xmodule.modulestore.django import modulestore # lint-amnesty, pylint: disable=wrong-import-order

log = logging.getLogger("edx.student")

Expand Down Expand Up @@ -619,8 +614,6 @@ def student_dashboard(request): # pylint: disable=too-many-statements
link_end=HTML("</a>"),
)

enterprise_message = get_dashboard_consent_notification(request, user, course_enrollments)

recovery_email_message = recovery_email_activation_message = None
if is_secondary_email_feature_enabled():
try:
Expand All @@ -647,10 +640,6 @@ def student_dashboard(request): # pylint: disable=too-many-statements
)
)

# Disable lookup of Enterprise consent_required_course due to ENT-727
# Will re-enable after fixing WL-1315
consent_required_courses = set()

# Account activation message
account_activation_messages = [
message for message in messages.get_messages(request) if 'account-activation' in message.tags
Expand Down Expand Up @@ -801,8 +790,6 @@ def student_dashboard(request): # pylint: disable=too-many-statements
context = {
'urls': urls,
'programs_data': programs_data,
'enterprise_message': enterprise_message,
'consent_required_courses': consent_required_courses,
'enrollment_message': enrollment_message,
'redirect_message': Text(redirect_message),
'account_activation_messages': account_activation_messages,
Expand Down Expand Up @@ -852,14 +839,8 @@ def student_dashboard(request): # pylint: disable=too-many-statements
'course_info': get_dashboard_course_info(user, course_enrollments),
# TODO START: clean up as part of REVEM-199 (END)
'disable_unenrollment': disable_unenrollment,
# TODO: clean when experiment(Merchandise 2U LOBs - Dashboard) would be stop. [VAN-1097]
'is_enterprise_user': is_enterprise_learner(user),
}

# Include enterprise learner portal metadata and messaging
enterprise_learner_portal_context = get_enterprise_learner_portal_context(request)
context.update(enterprise_learner_portal_context)

context_from_plugins = get_plugins_view_context(
ProjectType.LMS,
COURSE_DASHBOARD_PLUGIN_VIEW_NAME,
Expand Down
7 changes: 2 additions & 5 deletions common/djangoapps/student/views/management.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@
)
from django.views.decorators.http import ( # pylint: disable=unused-import
require_GET,
require_http_methods, # noqa: F401
require_POST,
)
from edx_ace import ace
Expand Down Expand Up @@ -113,8 +112,7 @@
from openedx.core.lib.api.authentication import BearerAuthenticationAllowInactiveUser
from openedx.features.course_experience.url_helpers import make_learning_mfe_courseware_url
from openedx.features.discounts.applicability import FIRST_PURCHASE_DISCOUNT_OVERRIDE_FLAG
from openedx.features.enterprise_support.utils import is_enterprise_learner
from xmodule.modulestore.django import modulestore # pylint: disable=wrong-import-order
from xmodule.modulestore.django import modulestore # lint-amnesty, pylint: disable=wrong-import-order

log = logging.getLogger("edx.student")

Expand Down Expand Up @@ -235,7 +233,6 @@
message_context = generate_activation_email_context(user, user_registration)
message_context.update({
'confirm_activation_link': _get_activation_confirmation_link(message_context['key'], redirect_url),
'is_enterprise_learner': is_enterprise_learner(user),
'is_first_purchase_discount_overridden': FIRST_PURCHASE_DISCOUNT_OVERRIDE_FLAG.is_enabled(),
'route_enabled': route_enabled,
'routed_user': user.username,
Expand Down Expand Up @@ -708,7 +705,7 @@
url_path = '/login?{}'.format(urllib.parse.urlencode(params)) # noqa: UP032
return redirect(settings.AUTHN_MICROFRONTEND_URL + url_path)

response = redirect(redirect_url) if redirect_url and is_enterprise_learner(request.user) else redirect('dashboard')
response = redirect(redirect_url) if redirect_url else redirect('dashboard')

Check warning

Code scanning / CodeQL

URL redirection from remote source Medium

Untrusted URL redirection depends on a
user-provided value
.

Copilot Autofix

AI 1 day ago

To fix this without changing intended functionality, add a final allowlist-style safety validation for redirect_url in activate_account before it is used in redirect(...). If the URL is not safe, fall back to dashboard behavior (existing default). Use Django’s built-in url_has_allowed_host_and_scheme, allowing only the current host and requiring HTTPS according to request security.

Best single change:

  • File: common/djangoapps/student/views/management.py
  • Region: activate_account, around lines 687–709 (where redirect_url is set and consumed).
  • Edits needed:
    1. Add import: from django.utils.http import url_has_allowed_host_and_scheme
    2. After computing redirect_url, validate it:
      • If present and unsafe, set redirect_url = None
    3. Keep existing logic and fallback redirect('dashboard') unchanged.

This preserves behavior for legitimate internal redirects and blocks untrusted external targets.

Suggested changeset 1
common/djangoapps/student/views/management.py

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/common/djangoapps/student/views/management.py b/common/djangoapps/student/views/management.py
--- a/common/djangoapps/student/views/management.py
+++ b/common/djangoapps/student/views/management.py
@@ -24,6 +24,7 @@
 from django.shortcuts import redirect
 from django.template.context_processors import csrf
 from django.urls import reverse
+from django.utils.http import url_has_allowed_host_and_scheme
 from django.utils.translation import gettext as _
 from django.views.decorators.csrf import (  # pylint: disable=unused-import  # noqa: F401
     csrf_exempt,
@@ -698,6 +699,13 @@
         ):
             redirect_url = get_redirect_url_with_host(root_login_url, redirect_to)
 
+    if redirect_url and not url_has_allowed_host_and_scheme(
+        redirect_url,
+        allowed_hosts={request.get_host()},
+        require_https=request.is_secure(),
+    ):
+        redirect_url = None
+
     if should_redirect_to_authn_microfrontend() and not request.user.is_authenticated:
         params = {'account_activation_status': activation_message_type}
         if redirect_url:
EOF
@@ -24,6 +24,7 @@
from django.shortcuts import redirect
from django.template.context_processors import csrf
from django.urls import reverse
from django.utils.http import url_has_allowed_host_and_scheme
from django.utils.translation import gettext as _
from django.views.decorators.csrf import ( # pylint: disable=unused-import # noqa: F401
csrf_exempt,
@@ -698,6 +699,13 @@
):
redirect_url = get_redirect_url_with_host(root_login_url, redirect_to)

if redirect_url and not url_has_allowed_host_and_scheme(
redirect_url,
allowed_hosts={request.get_host()},
require_https=request.is_secure(),
):
redirect_url = None

if should_redirect_to_authn_microfrontend() and not request.user.is_authenticated:
params = {'account_activation_status': activation_message_type}
if redirect_url:
Copilot is powered by AI and may make mistakes. Always verify output.
if show_account_activation_popup:
response.delete_cookie(
settings.SHOW_ACTIVATE_CTA_POPUP_COOKIE_NAME,
Expand Down
3 changes: 1 addition & 2 deletions common/djangoapps/third_party_auth/pipeline.py
Original file line number Diff line number Diff line change
Expand Up @@ -1009,8 +1009,7 @@ def get_username(strategy, details, backend, user=None, *args, **kwargs): # pyl
slug_func = lambda val: val

if is_auto_generated_username_enabled() and details.get('username') is None:
# Lazy import to avoid circular dependency
from openedx.core.djangoapps.user_authn.views.utils import get_auto_generated_username
from openedx.core.djangoapps.user_authn.views.utils import get_auto_generated_username # pylint: disable=import-outside-toplevel # noqa: I001
username = get_auto_generated_username(details)
else:
if email_as_username and details.get('email'):
Expand Down
4 changes: 4 additions & 0 deletions lms/envs/common.py
Original file line number Diff line number Diff line change
Expand Up @@ -3293,6 +3293,10 @@ def _should_send_certificate_events(settings):
"fail_silently": True,
"pipeline": ["enterprise.filters.accounts.AccountSettingsReadOnlyFieldsStep"],
},
"org.openedx.learning.dashboard.render.started.v1": {
"fail_silently": True,
"pipeline": ["enterprise.filters.dashboard.DashboardContextEnricher"],
},
}

############################## Miscellaneous ###############################
Expand Down
3 changes: 1 addition & 2 deletions lms/templates/dashboard.html
Original file line number Diff line number Diff line change
Expand Up @@ -220,10 +220,9 @@
is_course_voucher_refundable = (session_id in enrolled_courses_voucher_refundable)
course_requirements = courses_requirements_not_met.get(session_id)
related_programs = inverted_programs.get(str(entitlement.course_uuid if is_unfulfilled_entitlement else session_id))
show_consent_link = (session_id in consent_required_courses)
resume_button_url = resume_button_urls[dashboard_index]
%>
<%include file='dashboard/_dashboard_course_listing.html' args='course_overview=course_overview, course_card_index=dashboard_index, enrollment=enrollment, enrollments_fbe_is_on=enrollments_fbe_is_on, is_unfulfilled_entitlement=is_unfulfilled_entitlement, is_fulfilled_entitlement=is_fulfilled_entitlement, entitlement=entitlement, entitlement_session=entitlement_session, entitlement_available_sessions=entitlement_available_sessions, entitlement_expiration_date=entitlement_expiration_date, entitlement_expired_at=entitlement_expired_at, show_courseware_link=show_courseware_link, cert_status=cert_status, can_refund_entitlement=can_refund_entitlement, can_unenroll=can_unenroll, credit_status=credit_status, show_email_settings=show_email_settings, course_mode_info=course_mode_info, is_paid_course=is_paid_course, is_course_voucher_refundable=is_course_voucher_refundable, course_requirements=course_requirements, dashboard_index=dashboard_index, share_settings=share_settings, user=user, related_programs=related_programs, display_course_modes_on_dashboard=display_course_modes_on_dashboard, show_consent_link=show_consent_link, enterprise_customer_name=enterprise_customer_name, resume_button_url=resume_button_url, partner_managed_enrollment=partner_managed_enrollment' />
<%include file='dashboard/_dashboard_course_listing.html' args='course_overview=course_overview, course_card_index=dashboard_index, enrollment=enrollment, enrollments_fbe_is_on=enrollments_fbe_is_on, is_unfulfilled_entitlement=is_unfulfilled_entitlement, is_fulfilled_entitlement=is_fulfilled_entitlement, entitlement=entitlement, entitlement_session=entitlement_session, entitlement_available_sessions=entitlement_available_sessions, entitlement_expiration_date=entitlement_expiration_date, entitlement_expired_at=entitlement_expired_at, show_courseware_link=show_courseware_link, cert_status=cert_status, can_refund_entitlement=can_refund_entitlement, can_unenroll=can_unenroll, credit_status=credit_status, show_email_settings=show_email_settings, course_mode_info=course_mode_info, is_paid_course=is_paid_course, is_course_voucher_refundable=is_course_voucher_refundable, course_requirements=course_requirements, dashboard_index=dashboard_index, share_settings=share_settings, user=user, related_programs=related_programs, display_course_modes_on_dashboard=display_course_modes_on_dashboard, resume_button_url=resume_button_url, partner_managed_enrollment=partner_managed_enrollment' />
% endfor
% if show_load_all_courses_link:
<br/>
Expand Down
6 changes: 1 addition & 5 deletions lms/templates/dashboard/_dashboard_course_listing.html
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<%page args="course_overview, enrollment, entitlement, entitlement_session, course_card_index, enrollments_fbe_is_on, is_unfulfilled_entitlement, is_fulfilled_entitlement, entitlement_available_sessions, entitlement_expiration_date, entitlement_expired_at, show_courseware_link, cert_status, can_refund_entitlement, can_unenroll, credit_status, show_email_settings, course_mode_info, is_paid_course, is_course_voucher_refundable, course_requirements, dashboard_index, share_settings, related_programs, display_course_modes_on_dashboard, show_consent_link, enterprise_customer_name, resume_button_url, partner_managed_enrollment" expression_filter="h"/>
<%page args="course_overview, enrollment, entitlement, entitlement_session, course_card_index, enrollments_fbe_is_on, is_unfulfilled_entitlement, is_fulfilled_entitlement, entitlement_available_sessions, entitlement_expiration_date, entitlement_expired_at, show_courseware_link, cert_status, can_refund_entitlement, can_unenroll, credit_status, show_email_settings, course_mode_info, is_paid_course, is_course_voucher_refundable, course_requirements, dashboard_index, share_settings, related_programs, display_course_modes_on_dashboard, resume_button_url, partner_managed_enrollment" expression_filter="h"/>

<%!
import datetime
Expand Down Expand Up @@ -370,10 +370,6 @@ <h3 class="course-title" id="course-title-${enrollment.course_id}">
<%include file="_dashboard_credit_info.html" args="credit_status=credit_status"/>
% endif

% if show_consent_link:
<%include file="_dashboard_show_consent.html" args="course_overview=course_overview, course_target=course_target, enrollment=enrollment, enterprise_customer_name=enterprise_customer_name"/>
%endif

% if display_course_upgrade:
<div class="message message-upsell has-actions is-shown">
<div class="wrapper-extended">
Expand Down
25 changes: 0 additions & 25 deletions lms/templates/dashboard/_dashboard_show_consent.html

This file was deleted.

1 change: 0 additions & 1 deletion openedx/core/djangoapps/user_authn/tests/test_tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ def test_ComposeEmail(self):
assert self.msg.context['routed_user_email'] == self.student.email
assert self.msg.context['routed_profile_name'] == ''
assert self.msg.context['registration_flow'] is False
assert self.msg.context['is_enterprise_learner'] is False
assert self.msg.context['is_first_purchase_discount_overridden'] is False

@mock.patch('time.sleep', mock.Mock(return_value=None))
Expand Down
23 changes: 0 additions & 23 deletions openedx/features/enterprise_support/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -713,29 +713,6 @@ def consent_needed_for_course(request, user, course_id, enrollment_exists=False)
return True


@enterprise_is_enabled(otherwise=set())
def get_consent_required_courses(user, course_ids):
"""
Returns a set of course_ids that require consent
Note that this function makes use of the Enterprise models directly instead of using the API calls
"""
result = set()
enterprise_learner = EnterpriseCustomerUser.objects.filter(user_id=user.id).first()
if not enterprise_learner or not enterprise_learner.enterprise_customer:
return result

enterprise_uuid = enterprise_learner.enterprise_customer.uuid
data_sharing_consent = DataSharingConsent.objects.filter(username=user.username,
course_id__in=course_ids,
enterprise_customer__uuid=enterprise_uuid)

for consent in data_sharing_consent:
if consent.consent_required():
result.add(consent.course_id)

return result


@enterprise_is_enabled(otherwise='')
def get_enterprise_consent_url(request, course_id, user=None, return_to=None, enrollment_exists=False, source='lms'):
"""
Expand Down
33 changes: 0 additions & 33 deletions openedx/features/enterprise_support/tests/test_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@
enterprise_enabled,
get_active_enterprise_customer_user,
get_consent_notification_data,
get_consent_required_courses,
get_dashboard_consent_notification,
get_data_sharing_consents,
get_enterprise_consent_url,
Expand Down Expand Up @@ -533,38 +532,6 @@ def test_consent_needed_for_course_when_consent_is_required(
)
)

@httpretty.activate
@mock.patch('enterprise.models.EnterpriseCustomer.catalog_contains_course')
def test_get_consent_required_courses(self, mock_catalog_contains_course):
mock_catalog_contains_course.return_value = True
user = UserFactory()
enterprise_customer_user = EnterpriseCustomerUserFactory(user_id=user.id)

course_id = 'fake-course'
data_sharing_consent = DataSharingConsent(
course_id=course_id,
enterprise_customer=enterprise_customer_user.enterprise_customer,
username=user.username,
granted=False
)
data_sharing_consent.save()
consent_required = get_consent_required_courses(user, [course_id])
assert course_id in consent_required

# now grant consent and call our method again
data_sharing_consent.granted = True
data_sharing_consent.save()
consent_required = get_consent_required_courses(user, [course_id])
assert course_id not in consent_required

def test_consent_not_required_for_non_enterprise_user(self):
user = UserFactory()
course_id = 'fake-course'

consent_required_courses = get_consent_required_courses(user, [course_id])

assert set() == consent_required_courses

@mock.patch('openedx.features.enterprise_support.api.create_jwt_for_user')
def test_fetch_enterprise_learner_data_unauthenticated(self, mock_jwt_builder):
api_client = self._assert_api_client_with_user(EnterpriseApiClient, mock_jwt_builder)
Expand Down
2 changes: 1 addition & 1 deletion requirements/constraints.txt
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ django-stubs<6
# The team that owns this package will manually bump this package rather than having it pulled in automatically.
# This is to allow them to better control its deployment and to do it in a process that works better
# for them.
edx-enterprise==8.0.2
edx-enterprise==8.0.3

# Date: 2023-07-26
# Our legacy Sass code is incompatible with anything except this ancient libsass version.
Expand Down
2 changes: 1 addition & 1 deletion requirements/edx/base.txt
Original file line number Diff line number Diff line change
Expand Up @@ -483,7 +483,7 @@ edx-drf-extensions==10.6.0
# enterprise-integrated-channels
# openedx-authz
# openedx-core
edx-enterprise==8.0.2
edx-enterprise==8.0.3
# via
# -c requirements/constraints.txt
# -r requirements/edx/kernel.in
Expand Down
2 changes: 1 addition & 1 deletion requirements/edx/development.txt
Original file line number Diff line number Diff line change
Expand Up @@ -756,7 +756,7 @@ edx-drf-extensions==10.6.0
# enterprise-integrated-channels
# openedx-authz
# openedx-core
edx-enterprise==8.0.2
edx-enterprise==8.0.3
# via
# -c requirements/constraints.txt
# -r requirements/edx/doc.txt
Expand Down
2 changes: 1 addition & 1 deletion requirements/edx/doc.txt
Original file line number Diff line number Diff line change
Expand Up @@ -573,7 +573,7 @@ edx-drf-extensions==10.6.0
# enterprise-integrated-channels
# openedx-authz
# openedx-core
edx-enterprise==8.0.2
edx-enterprise==8.0.3
# via
# -c requirements/constraints.txt
# -r requirements/edx/base.txt
Expand Down
2 changes: 1 addition & 1 deletion requirements/edx/testing.txt
Original file line number Diff line number Diff line change
Expand Up @@ -589,7 +589,7 @@ edx-drf-extensions==10.6.0
# enterprise-integrated-channels
# openedx-authz
# openedx-core
edx-enterprise==8.0.2
edx-enterprise==8.0.3
# via
# -c requirements/constraints.txt
# -r requirements/edx/base.txt
Expand Down
Loading