From fe57074dabb6ca9a8ddbbfa3fb0c19ef8f833c0a Mon Sep 17 00:00:00 2001 From: Awais Qureshi Date: Tue, 1 Mar 2022 17:38:36 +0500 Subject: [PATCH] feat!: Remove django-admin default login (#29876) * feat!: Remove django-admin default login --- .../contentstore/tests/test_admin.py | 32 +++++++++++++++++++ cms/djangoapps/contentstore/views/public.py | 17 +++++++++- .../course_creators/tests/test_admin.py | 15 --------- cms/templates/admin/base_site.html | 9 +++++- cms/urls.py | 3 ++ lms/templates/admin/base_site.html | 9 +++++- .../djangoapps/user_authn/config/waffle.py | 15 ++++++++- .../core/djangoapps/user_authn/views/login.py | 7 ++-- openedx/core/tests/test_admin_view.py | 13 +++++++- 9 files changed, 98 insertions(+), 22 deletions(-) create mode 100644 cms/djangoapps/contentstore/tests/test_admin.py diff --git a/cms/djangoapps/contentstore/tests/test_admin.py b/cms/djangoapps/contentstore/tests/test_admin.py new file mode 100644 index 0000000000..d0a306ba11 --- /dev/null +++ b/cms/djangoapps/contentstore/tests/test_admin.py @@ -0,0 +1,32 @@ +""" +Tests that verify that the admin view loads. + +This is not inside a django app because it is a global property of the system. +""" +import ddt +from django.test import TestCase +from django.urls import reverse +from edx_toggles.toggles.testutils import override_waffle_flag + +from openedx.core.djangoapps.user_authn.config.waffle import ADMIN_AUTH_REDIRECT_TO_LMS + + +@ddt.ddt +class TestAdminView(TestCase): + """ + Tests of the admin view. + """ + @override_waffle_flag(ADMIN_AUTH_REDIRECT_TO_LMS, True) + @ddt.data('/admin/', '/admin/login', reverse('admin:login')) + def test_admin_login_redirect(self, admin_url): + """Admin login will redirect towards the site login page.""" + response = self.client.get(admin_url, follow=True) + assert any('/login/edx-oauth2/?next=' in r[0] for r in response.redirect_chain) + + def test_admin_login_default(self): + """Without flag Admin login will redirect towards the admin default login page.""" + response = self.client.get('/admin/', follow=True) + assert response.status_code == 200 + self.assertIn('/admin/login/?next=/admin/', response.redirect_chain[0]) + assert len(response.redirect_chain) == 1 + assert response.template_name == ['admin/login.html'] diff --git a/cms/djangoapps/contentstore/views/public.py b/cms/djangoapps/contentstore/views/public.py index 4c5cebdb33..823bcd4154 100644 --- a/cms/djangoapps/contentstore/views/public.py +++ b/cms/djangoapps/contentstore/views/public.py @@ -7,12 +7,17 @@ from django.conf import settings from django.shortcuts import redirect from urllib.parse import quote_plus # lint-amnesty, pylint: disable=wrong-import-order from waffle.decorators import waffle_switch +from django.contrib import admin from common.djangoapps.edxmako.shortcuts import render_to_response +from openedx.core.djangoapps.user_authn.config.waffle import ADMIN_AUTH_REDIRECT_TO_LMS from ..config import waffle -__all__ = ['register_redirect_to_lms', 'login_redirect_to_lms', 'howitworks', 'accessibility'] +__all__ = [ + 'register_redirect_to_lms', 'login_redirect_to_lms', 'howitworks', 'accessibility', + 'redirect_to_lms_login_for_admin', +] def register_redirect_to_lms(request): @@ -39,6 +44,16 @@ def login_redirect_to_lms(request): return redirect(login_url) +def redirect_to_lms_login_for_admin(request): + """ + This view redirect the admin/login url to the site's login page. + """ + if ADMIN_AUTH_REDIRECT_TO_LMS.is_enabled(): + return redirect('/login?next=/admin') + else: + return admin.site.login(request) + + def _build_next_param(request): """ Returns the next param to be used with login or register. """ next_url = request.GET.get('next') diff --git a/cms/djangoapps/course_creators/tests/test_admin.py b/cms/djangoapps/course_creators/tests/test_admin.py index 60c7f0691f..c747b92075 100644 --- a/cms/djangoapps/course_creators/tests/test_admin.py +++ b/cms/djangoapps/course_creators/tests/test_admin.py @@ -176,18 +176,3 @@ class CourseCreatorAdminTest(TestCase): self.request.user = self.user self.assertFalse(self.creator_admin.has_change_permission(self.request)) - - def test_rate_limit_login(self): - with mock.patch.dict('django.conf.settings.FEATURES', {'ENABLE_CREATOR_GROUP': True}): - post_params = {'username': self.user.username, 'password': 'wrong_password'} - # try logging in 30 times, the default limit in the number of failed - # login attempts in one 5 minute period before the rate gets limited - for _ in range(30): - response = self.client.post('/admin/login/', post_params) - self.assertEqual(response.status_code, 200) - - response = self.client.post('/admin/login/', post_params) - # Since we are using the default rate limit behavior, we are - # expecting this to return a 403 error to indicate that there have - # been too many attempts - self.assertEqual(response.status_code, 403) diff --git a/cms/templates/admin/base_site.html b/cms/templates/admin/base_site.html index 953b42d2bf..dd10b68f5f 100644 --- a/cms/templates/admin/base_site.html +++ b/cms/templates/admin/base_site.html @@ -1,5 +1,6 @@ {% extends "admin/base.html" %} {% load i18n admin_urls %} +{% load waffle_tags %} {% block title %}{{ title }} | {{ site_title|default:_('Django site admin') }}{% endblock %} {% block branding %}

{{ site_header|default:_('Django administration') }}

@@ -15,5 +16,11 @@ {% trans 'Documentation' as tmsg %} {{tmsg|force_escape}} / {% endif %} {% endif %} - {% trans 'Log out' as tmsg %} {{tmsg|force_escape}} + + {% flag "user_authn.admin_auth_redirect_to_lms" %} + {% trans 'Log out' as tmsg %} {{tmsg|force_escape}} + {% else %} + {% trans 'Log out' as tmsg %} {{tmsg|force_escape}} + {% endflag %} + {% endblock %} diff --git a/cms/urls.py b/cms/urls.py index 9748c00919..3facddf0bf 100644 --- a/cms/urls.py +++ b/cms/urls.py @@ -230,6 +230,9 @@ if settings.FEATURES.get('ENABLE_SERVICE_STATUS'): if not settings.FEATURES.get('ENABLE_CHANGE_USER_PASSWORD_ADMIN'): urlpatterns.append(re_path(r'^admin/auth/user/\d+/password/$', handler404)) urlpatterns.append(path('admin/password_change/', handler404)) +urlpatterns.append( + path('admin/login/', contentstore_views.redirect_to_lms_login_for_admin, name='redirect_to_lms_login_for_admin') +) urlpatterns.append(path('admin/', admin.site.urls)) # enable entrance exams diff --git a/lms/templates/admin/base_site.html b/lms/templates/admin/base_site.html index 4b36642f6b..285d275282 100644 --- a/lms/templates/admin/base_site.html +++ b/lms/templates/admin/base_site.html @@ -1,5 +1,6 @@ {% extends "admin/base.html" %} {% load i18n admin_urls %} +{% load waffle_tags %} {% block title %}{{ title }} | {{ site_title|default:_('Django site admin') }}{% endblock %} {% block branding %}

{{ site_header|default:_('Django administration') }}

@@ -15,5 +16,11 @@ {% trans 'Documentation' as tmsg%}{{tmsg|force_escape}} / {% endif %} {% endif %} - {% trans 'Log out' as tmsg%}{{tmsg|force_escape}} + + {% flag "user_authn.admin_auth_redirect_to_lms" %} + {% trans 'Log out' as tmsg%}{{tmsg|force_escape}} + {% else %} + {% trans 'Log out' as tmsg%}{{tmsg|force_escape}} + {% endflag %} + {% endblock %} diff --git a/openedx/core/djangoapps/user_authn/config/waffle.py b/openedx/core/djangoapps/user_authn/config/waffle.py index dc409d4eaa..c58b81869d 100644 --- a/openedx/core/djangoapps/user_authn/config/waffle.py +++ b/openedx/core/djangoapps/user_authn/config/waffle.py @@ -3,7 +3,7 @@ Waffle flags and switches for user authn. """ -from edx_toggles.toggles import LegacyWaffleSwitch, LegacyWaffleSwitchNamespace +from edx_toggles.toggles import LegacyWaffleSwitch, LegacyWaffleSwitchNamespace, WaffleFlag _WAFFLE_NAMESPACE = 'user_authn' _WAFFLE_SWITCH_NAMESPACE = LegacyWaffleSwitchNamespace(name=_WAFFLE_NAMESPACE, log_prefix='UserAuthN: ') @@ -37,3 +37,16 @@ ENABLE_PWNED_PASSWORD_API = LegacyWaffleSwitch( 'enable_pwned_password_api', __name__ ) + + +# .. toggle_name: ADMIN_AUTH_REDIRECT_TO_LMS +# .. toggle_implementation: WaffleFlag +# .. toggle_default: False +# .. toggle_description: Set this to True if you want to redirect cms-admin login to lms login. +# In case of logout it will use lms logout also. +# .. toggle_use_cases: open_edx +# .. toggle_creation_date: 2022-02-08 +# .. toggle_target_removal_date: None +ADMIN_AUTH_REDIRECT_TO_LMS = WaffleFlag( # lint-amnesty, pylint: disable=toggle-missing-annotation + "user_authn.admin_auth_redirect_to_lms", module_name=__name__ +) diff --git a/openedx/core/djangoapps/user_authn/views/login.py b/openedx/core/djangoapps/user_authn/views/login.py index a6691dc7ac..7ad123c9eb 100644 --- a/openedx/core/djangoapps/user_authn/views/login.py +++ b/openedx/core/djangoapps/user_authn/views/login.py @@ -43,7 +43,10 @@ from common.djangoapps.util.password_policy_validators import normalize_password from openedx.core.djangoapps.password_policy import compliance as password_policy_compliance from openedx.core.djangoapps.safe_sessions.middleware import mark_user_change_as_expected from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers -from openedx.core.djangoapps.user_authn.config.waffle import ENABLE_LOGIN_USING_THIRDPARTY_AUTH_ONLY +from openedx.core.djangoapps.user_authn.config.waffle import ( + ENABLE_LOGIN_USING_THIRDPARTY_AUTH_ONLY, + ADMIN_AUTH_REDIRECT_TO_LMS +) from openedx.core.djangoapps.user_authn.cookies import get_response_with_refreshed_jwt_cookies, set_logged_in_cookies from openedx.core.djangoapps.user_authn.exceptions import AuthFailedError from openedx.core.djangoapps.user_authn.toggles import ( @@ -655,7 +658,7 @@ def redirect_to_lms_login(request): This view redirect the admin/login url to the site's login page if waffle switch is on otherwise returns the admin site's login view. """ - if ENABLE_LOGIN_USING_THIRDPARTY_AUTH_ONLY.is_enabled(): + if ENABLE_LOGIN_USING_THIRDPARTY_AUTH_ONLY.is_enabled() or ADMIN_AUTH_REDIRECT_TO_LMS.is_enabled(): return redirect('/login?next=/admin') else: return admin.site.login(request) diff --git a/openedx/core/tests/test_admin_view.py b/openedx/core/tests/test_admin_view.py index 5e84fd0012..b0a70ac965 100644 --- a/openedx/core/tests/test_admin_view.py +++ b/openedx/core/tests/test_admin_view.py @@ -6,8 +6,9 @@ This is not inside a django app because it is a global property of the system. from django.test import Client, TestCase from django.urls import reverse -from edx_toggles.toggles.testutils import override_waffle_switch +from edx_toggles.toggles.testutils import override_waffle_switch, override_waffle_flag from common.djangoapps.student.tests.factories import UserFactory, TEST_PASSWORD +from openedx.core.djangoapps.user_authn.config.waffle import ADMIN_AUTH_REDIRECT_TO_LMS from openedx.core.djangoapps.user_authn.views.login import ENABLE_LOGIN_USING_THIRDPARTY_AUTH_ONLY @@ -43,6 +44,16 @@ class TestAdminView(TestCase): response = self.client.get(reverse('admin:login')) assert response.url == '/login?next=/admin' assert response.status_code == 302 + + with override_waffle_flag(ADMIN_AUTH_REDIRECT_TO_LMS, True): + response = self.client.get(reverse('admin:login')) + assert response.url == '/login?next=/admin' + assert response.status_code == 302 + with override_waffle_switch(ENABLE_LOGIN_USING_THIRDPARTY_AUTH_ONLY, False): response = self.client.get(reverse('admin:login')) assert response.template_name == ['admin/login.html'] + + with override_waffle_flag(ADMIN_AUTH_REDIRECT_TO_LMS, False): + response = self.client.get(reverse('admin:login')) + assert response.template_name == ['admin/login.html']