feat: remove enterprise dashboard context imports (ENT-11569)#38094
feat: remove enterprise dashboard context imports (ENT-11569)#38094
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| 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
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 6 days ago
In general, to fix untrusted URL redirection, any URL derived from user input (directly or indirectly) must be validated against an allowlist or using a robust “is safe redirect” check before being passed to redirect(). In a Django context, this typically means ensuring that the target is either a relative URL or, if absolute, has an allowed host and scheme.
For this specific case, the simplest, least-invasive fix is to validate redirect_url at the point where it is used. We already import is_safe_login_or_logout_redirect in common/djangoapps/student/helpers.py; we can use that here by importing it in management.py and checking redirect_url just before calling redirect(redirect_url). If redirect_url is not safe, we should ignore it and fall back to the dashboard, preserving existing behavior for legitimate values while preventing open redirects.
Concretely:
- In
common/djangoapps/student/views/management.py, add an import foris_safe_login_or_logout_redirectfromopenedx.core.djangoapps.user_authn.utils. - In the
activate_accountview, just before constructingresponse = redirect(redirect_url) if redirect_url else redirect('dashboard'), add a conditional that resetsredirect_urltoNoneifredirect_urlis not considered safe byis_safe_login_or_logout_redirect. This leaves all other logic intact, including the use ofredirect_urlin theparams['next']when redirecting to the AuthN microfrontend.
This ensures that even if get_next_url_for_login_page or get_redirect_url_with_host were ever modified or misused to allow unsafe values, the final redirect in activate_account remains protected.
| @@ -40,6 +40,7 @@ | ||
| from eventtracking import tracker | ||
| # Note that this lives in LMS, so this dependency should be refactored. | ||
| from opaque_keys import InvalidKeyError | ||
| from openedx.core.djangoapps.user_authn.utils import is_safe_login_or_logout_redirect | ||
| from opaque_keys.edx.keys import CourseKey | ||
| from rest_framework.decorators import api_view, authentication_classes, permission_classes | ||
| from rest_framework.permissions import IsAuthenticated | ||
| @@ -697,6 +698,9 @@ | ||
| url_path = '/login?{}'.format(urllib.parse.urlencode(params)) | ||
| return redirect(settings.AUTHN_MICROFRONTEND_URL + url_path) | ||
|
|
||
| if redirect_url and not is_safe_login_or_logout_redirect(redirect_url, request): | ||
| redirect_url = None | ||
|
|
||
| response = redirect(redirect_url) if redirect_url else redirect('dashboard') | ||
| if show_account_activation_popup: | ||
| response.delete_cookie( |
pwnage101
force-pushed
the
pwnage101/ENT-11569
branch
from
87089ed to
4637f70
Compare
pwnage101
left a comment
pwnage101
left a comment
There was a problem hiding this comment.
Note for implementer/reviewer: looks like isort got ran with the wrong parameters. at least fix that.
1 participant
feat: remove enterprise dashboard context imports (ENT-11569)
ENT-11569