From 60b44d20568b08239e1007e1c7a3a129a485e204 Mon Sep 17 00:00:00 2001 From: Aseem Date: Wed, 5 Aug 2020 12:21:03 -0500 Subject: [PATCH 1/2] feat: Redirect admin users to setup TOTP When TOTP is required on an admin view and a user does not have a TOTP device configured, redirect them to the TOTP setup view. --- .gitignore | 2 + CHANGELOG.md | 56 ++++++++++++++- docs/class-reference.rst | 4 +- docs/installation.rst | 3 +- example/urls.py | 4 +- requirements_dev.txt | 1 - tests/test_admin.py | 118 ++++++++++++++++++++++--------- tests/urls_admin.py | 5 +- tests/urls_otp_admin.py | 11 --- two_factor/admin.py | 149 +++++++++++++++++++++++++++++++-------- 10 files changed, 270 insertions(+), 83 deletions(-) delete mode 100644 tests/urls_otp_admin.py diff --git a/.gitignore b/.gitignore index 7cd853fbb..80de2fd45 100644 --- a/.gitignore +++ b/.gitignore @@ -10,3 +10,5 @@ example/settings_private.py .eggs/ .idea/ + +venv/ diff --git a/CHANGELOG.md b/CHANGELOG.md index 7cde32a7d..1380752f2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,11 +1,16 @@ +# Change Log ## Unreleased ### Added - Enforcing a redirect to setup of otp device when none available for user [#550](https://github.com/jazzband/django-two-factor-auth/pull/500) + +### Changed + ## 1.14.0 ### Added + - Python 3.10 support - The setup view got a new `secret_key` context variable to be able to display that key elsewhere than in the QR code. @@ -15,6 +20,7 @@ and used to communicate the second factor token by email. ### Changed + - BREAKING: The phone capability moved to a plugins folder, so if you use that capability and want to keep it, you should add `two_factor.plugins.phonenumber` line in your `INSTALLED_APPS` setting. Additionally, as the `two_factor` @@ -30,15 +36,18 @@ with a dark theme. ### Removed + - Python 3.5 and 3.6 support ## 1.13.2 ### Added + - Translations for new languages: Hausa, Japanese, Vietnamese - Django 4.0 support ### Changed + - Suppressed default_app_config warning on Django 3.2+ - qrcode dependency limit upped to 7.99 and django-phonenumber-field to 7 - When validating a TOTP after scanning the QR code, allow a time drift of +/-1 instead of just -1 @@ -46,21 +55,25 @@ ## 1.13.1 ### Add + - Support Twilio Messaging Service SID - Add autofocus, autocomplete one-time-code and inputmode numeric to token input fields ### Changed + - Change "Back to Profile" to "Back to Account Security" ## 1.13.0 ### Added + - User can request that two-factor authentication be skipped the next time they log in on that particular device - Django 3.1 support - SMS message can now be customised by using a template ### Changed + - Simplified `re_path()` to `path()` in URLConf - Templates are now based on Bootstrap 4. - `DisableView` now checks user has verified before disabling two-factor on @@ -68,129 +81,168 @@ - Inline CSS has been replaced to allow stricter Content Security Policies. ### Removed + - Upper limit on django-otp dependency - Obsolete IE<9 workarounds - Workarounds for older versions of django-otp ## 1.12.1 - 2020-07-08 -*No code changes for this version* +_No code changes for this version_ ## 1.12 - 2020-07-08 + ### Added + - It is possible to set a timeout between a user authenticiating in the `LoginView` and them needing to re-authenticate. By default this is 10 minutes. ### Removed + - The final step in the `LoginView` no longer re-validates a user's credentials. - Django 1.11 support. ### Changed + - Security Fix: `LoginView` no longer stores credentials in plaintext in the session store. ## 1.11.0 - 2020-03-13 + ### Added -*Nothing has been added for this version* +_Nothing has been added for this version_ ### Removed + - MiddlewareMixin - Python 3.4 support - Django 2.1 support - `mock` dependency ### Changed + - `extra_requires` are now listed in lowercase. This is to workaround a bug in `pip`. - Use `trimmed` option on `blocktrans` to avoid garbage newlines in translations. - `random_hex` from `django_otp` 0.8.0 will always return a `str`, don't try to decode it. ## 1.10.0 - 2019-12-13 + ### Added + - Support for Django 3.0. - Optionally install full or light phonenumbers library. ### Removed + - Python 2 support. ### Changed + - Updated translations. ## 1.9.1 - 2019-07-07 + ### Changed + - 1.9.0 got pushed with incorrect changelog, no other changes. ## 1.9.0 - 2019-07-07 + ### Added + - Support for Django 2.2. - Ability to create `PhoneDevice` from Django admin. - Support for Python 3.7. ## 1.8.0 - 2018-08-03 + ### Added + - Support for Django 2.1. - Support for QRcode library up to 6. - Translation: Romanian. ### Changed + - Replace `ValidationError` with `SuspiciousOperation` in views. - Change the wording in 2FA disable template. - Updated translations. ## 1.7.0 - 2017-12-19 + ### Added + - Support for Django 2.0. ### Removed + - Django <1.11 support. ### Changed + - Do not list phone method if it is not supported (#225). - Pass request kwarg to authentication form (#227). ## 1.6.2 - 2017-07-29 + ### Fixed + - Twilio client 6.0 usage (#211). ### Changed + - Updated translation: Russian. ## 1.6.1 - 2017-05-11 + ### Added + - Support Twilio client 6.0 (#203). ### Fixed + - `redirect_to` after successful login (#204) ### Changed + - Updated translation: Norwegian Bokmål ## 1.6.0 - 2017-04-08 + ### Added + - Support for Django 1.11 (#188). ### Removed + - Django 1.9 support. ### Fixed + - Allow setting `LOGIN_REDIRECT_URL` to a URL (#192). - `DisableView` should also take `success_url` parameter (#187). ## 1.5.0 - 2017-01-04 + ### Added + - Django 1.10’s MIDDLEWARE support. - Allow `success_url` overrides from `urls.py`. - Autofocus token input during authentication. - Translations: Polish, Italian, Hungarian, Finnish and Danish. ### Removed + - Dropped Python 3.2 and 3.3 support. ### Changed + - Renamed `redirect_url` properties to `success_url` to be consistent with Django. ### Fixed + - Allow Firefox users to enter backup tokens (#177). - Allow multiple requests for QR code (#99). - Don't add phone number without gateway (#92). diff --git a/docs/class-reference.rst b/docs/class-reference.rst index 6f2332475..213247254 100644 --- a/docs/class-reference.rst +++ b/docs/class-reference.rst @@ -3,8 +3,8 @@ Class Reference Admin Site ---------- -.. autoclass:: two_factor.admin.AdminSiteOTPRequired -.. autoclass:: two_factor.admin.AdminSiteOTPRequiredMixin +.. autoclass:: two_factor.admin.TwoFactorAdminSite +.. autoclass:: two_factor.admin.TwoFactorAdminSiteMixin Decorators ---------- diff --git a/docs/installation.rst b/docs/installation.rst index 5ae666176..51989e0b8 100644 --- a/docs/installation.rst +++ b/docs/installation.rst @@ -66,9 +66,10 @@ Add the routes to your project url configuration: .. code-block:: python from two_factor.urls import urlpatterns as tf_urls + from two_factor.admin import TwoFactorAdminSite urlpatterns = [ path('', include(tf_urls)), - ... + path('admin', TwoFactorAdminSite().urls) ] .. warning:: diff --git a/example/urls.py b/example/urls.py index 8e7531f09..63258bfe0 100644 --- a/example/urls.py +++ b/example/urls.py @@ -1,8 +1,8 @@ from django.conf import settings -from django.contrib import admin from django.contrib.auth.views import LogoutView from django.urls import include, path +from two_factor.admin import TwoFactorAdminSite from two_factor.gateways.twilio.urls import urlpatterns as tf_twilio_urls from two_factor.urls import urlpatterns as tf_urls @@ -39,7 +39,7 @@ path('', include(tf_urls)), path('', include(tf_twilio_urls)), path('', include('user_sessions.urls', 'user_sessions')), - path('admin/', admin.site.urls), + path('admin/', TwoFactorAdminSite().urls), ] if settings.DEBUG: diff --git a/requirements_dev.txt b/requirements_dev.txt index a03692141..9ce66fe8d 100644 --- a/requirements_dev.txt +++ b/requirements_dev.txt @@ -14,7 +14,6 @@ django-bootstrap-form django-user-sessions # Testing - coverage flake8 tox diff --git a/tests/test_admin.py b/tests/test_admin.py index 57d11225f..d29e8dd95 100644 --- a/tests/test_admin.py +++ b/tests/test_admin.py @@ -1,5 +1,7 @@ -from django.conf import settings -from django.shortcuts import resolve_url + +from unittest import mock + +from django.shortcuts import reverse from django.test import TestCase from django.test.utils import override_settings @@ -9,7 +11,7 @@ @override_settings(ROOT_URLCONF='tests.urls_admin') -class AdminPatchTest(TestCase): +class TwoFactorAdminSiteTest(UserMixin, TestCase): def setUp(self): patch_admin() @@ -19,50 +21,98 @@ def tearDown(self): def test(self): response = self.client.get('/admin/', follow=True) - redirect_to = '%s?next=/admin/' % resolve_url(settings.LOGIN_URL) - self.assertRedirects(response, redirect_to) - - @override_settings(LOGIN_URL='two_factor:login') - def test_named_url(self): - response = self.client.get('/admin/', follow=True) - redirect_to = '%s?next=/admin/' % resolve_url(settings.LOGIN_URL) + redirect_to = '%s?next=/admin/' % reverse('admin:login') self.assertRedirects(response, redirect_to) @override_settings(ROOT_URLCONF='tests.urls_admin') -class AdminSiteTest(UserMixin, TestCase): - - def setUp(self): - super().setUp() - self.user = self.create_superuser() - self.login_user() +class AdminPatchTest(TestCase): + """ + otp_admin is admin console that needs OTP for access. + Only admin users (is_staff and is_active) + with OTP can access it. + """ + + def test_anonymous_get_admin_index_redirects_to_admin_login(self): + index_url = reverse('admin:index') + login_url = reverse('admin:login') + response = self.client.get(index_url, follow=True) + redirect_to = '%s?next=%s' % (login_url, index_url) + self.assertRedirects(response, redirect_to) - def test_default_admin(self): - response = self.client.get('/admin/') + def test_anonymous_get_admin_logout_redirects_to_admin_index(self): + # see: django.tests.admin_views.test_client_logout_url_can_be_used_to_login + index_url = reverse('admin:index') + logout_url = reverse('admin:logout') + response = self.client.get(logout_url) + self.assertEqual( + response.status_code, 302 + ) + self.assertEqual(response.get('Location'), index_url) + + def test_anonymous_get_admin_login(self): + login_url = reverse('admin:login') + response = self.client.get(login_url, follow=True) self.assertEqual(response.status_code, 200) -@override_settings(ROOT_URLCONF='tests.urls_otp_admin') -class OTPAdminSiteTest(UserMixin, TestCase): - - def setUp(self): - super().setUp() + def test_is_staff_not_verified_not_setup_get_admin_index_redirects_to_setup(self): + """ + admins without MFA setup should be redirected to the setup page. + """ + index_url = reverse('admin:index') + setup_url = reverse('two_factor:setup') self.user = self.create_superuser() self.login_user() - - def test_otp_admin_without_otp(self): - response = self.client.get('/otp_admin/', follow=True) - redirect_to = '%s?next=/otp_admin/' % resolve_url(settings.LOGIN_URL) + response = self.client.get(index_url, follow=True) + redirect_to = '%s?next=%s' % (setup_url, index_url) self.assertRedirects(response, redirect_to) - @override_settings(LOGIN_URL='two_factor:login') - def test_otp_admin_without_otp_named_url(self): - response = self.client.get('/otp_admin/', follow=True) - redirect_to = '%s?next=/otp_admin/' % resolve_url(settings.LOGIN_URL) + def test_is_staff_not_verified_not_setup_get_admin_login_redirects_to_setup(self): + index_url = reverse('admin:index') + login_url = reverse('admin:login') + setup_url = reverse('two_factor:setup') + self.user = self.create_superuser() + self.login_user() + response = self.client.get(login_url, follow=True) + redirect_to = '%s?next=%s' % (setup_url, index_url) self.assertRedirects(response, redirect_to) - def test_otp_admin_with_otp(self): - self.enable_otp() + def test_is_staff_is_verified_get_admin_index(self): + index_url = reverse('admin:index') + self.user = self.create_superuser() + self.enable_otp(self.user) self.login_user() - response = self.client.get('/otp_admin/') + response = self.client.get(index_url) self.assertEqual(response.status_code, 200) + + def test_is_staff_is_verified_get_admin_password_change(self): + password_change_url = reverse('admin:password_change') + self.user = self.create_superuser() + self.enable_otp(self.user) + self.login_user() + response = self.client.get(password_change_url) + self.assertEqual(response.status_code, 200) + + def test_is_staff_is_verified_get_admin_login_redirects_to_admin_index(self): + login_url = reverse('admin:login') + index_url = reverse('admin:index') + self.user = self.create_superuser() + self.enable_otp(self.user) + self.login_user() + response = self.client.get(login_url) + self.assertEqual(response.get('Location'), index_url) + + @mock.patch('two_factor.views.core.signals.user_verified.send') + def test_valid_login(self, mock_signal): + login_url = reverse('admin:login') + self.user = self.create_user() + self.enable_otp(self.user) + data = {'auth-username': 'bouke@example.com', + 'auth-password': 'secret', + 'login_view-current_step': 'auth'} + response = self.client.post(login_url, data=data) + self.assertEqual(response.status_code, 200) + + # No signal should be fired for non-verified user logins. + self.assertFalse(mock_signal.called) diff --git a/tests/urls_admin.py b/tests/urls_admin.py index 64a073faa..881215767 100644 --- a/tests/urls_admin.py +++ b/tests/urls_admin.py @@ -1,8 +1,9 @@ -from django.contrib import admin from django.urls import path +from two_factor.admin import TwoFactorAdminSite + from .urls import urlpatterns urlpatterns += [ - path('admin/', admin.site.urls), + path('admin/', TwoFactorAdminSite().urls), ] diff --git a/tests/urls_otp_admin.py b/tests/urls_otp_admin.py deleted file mode 100644 index 32210cb68..000000000 --- a/tests/urls_otp_admin.py +++ /dev/null @@ -1,11 +0,0 @@ -from django.urls import path - -from two_factor.admin import AdminSiteOTPRequired - -from .urls import urlpatterns - -otp_admin_site = AdminSiteOTPRequired() - -urlpatterns += [ - path('otp_admin/', otp_admin_site.urls), -] diff --git a/two_factor/admin.py b/two_factor/admin.py index 7d9b29513..7abcb4a38 100644 --- a/two_factor/admin.py +++ b/two_factor/admin.py @@ -1,20 +1,22 @@ from django.conf import settings +import warnings +from functools import update_wrapper + from django.contrib.admin import AdminSite from django.contrib.auth import REDIRECT_FIELD_NAME from django.contrib.auth.views import redirect_to_login +from django.http import HttpResponseRedirect from django.shortcuts import resolve_url +from django.urls import reverse +from django.utils.decorators import method_decorator +from django.utils.http import url_has_allowed_host_and_scheme +from django.utils.translation import gettext as _ +from django.views.decorators.cache import never_cache +from django.views.decorators.csrf import csrf_protect -from .utils import monkeypatch_method - -try: - from django.utils.http import url_has_allowed_host_and_scheme -except ImportError: - from django.utils.http import ( - is_safe_url as url_has_allowed_host_and_scheme, - ) +from .utils import default_device - -class AdminSiteOTPRequiredMixin: +class TwoFactorAdminSiteMixin: """ Mixin for enforcing OTP verified staff users. @@ -22,50 +24,141 @@ class AdminSiteOTPRequiredMixin: use :meth:`has_permission` in order to secure those views. """ + def has_admin_permission(self, request): + return super().has_permission(request) + def has_permission(self, request): """ Returns True if the given HttpRequest has permission to view *at least one* page in the admin site. """ - if not super().has_permission(request): - return False - return request.user.is_verified() - + return self.has_admin_permission(request) and request.user.is_verified() + + def has_mfa_setup(self, request): + otp_device = default_device(request.user) + return otp_device is not None + + def redirect_to_mfa_setup(self, request): + # Already logged-in, but they did not login with MFA and do not + # have MFA enabled on their account. We're going to redirect them + # to the MFA setup. + + # TODO: Add redirect_to functionality to MFA setup. + # TODO: Add message indicating why the user was directed or setup and MFA required + # interstitial page to explain to the user they need to setup MFA. + setup_url = reverse('two_factor:setup') + next = request.GET.get(REDIRECT_FIELD_NAME, reverse('admin:index')) + + # the name redirect_to_login is a little misleading, the function actually + # redirects to the specified view setting the REDIRECT_FIELD_NAME to the + # next value. We're not logging in here, we're just sending the user to the + # MFA setup screen. + return redirect_to_login(next, setup_url) + + @method_decorator(never_cache) def login(self, request, extra_context=None): """ Redirects to the site login page for the given HttpRequest. """ redirect_to = request.POST.get(REDIRECT_FIELD_NAME, request.GET.get(REDIRECT_FIELD_NAME)) - if not redirect_to or not url_has_allowed_host_and_scheme(url=redirect_to, allowed_hosts=[request.get_host()]): redirect_to = resolve_url(settings.LOGIN_REDIRECT_URL) - return redirect_to_login(redirect_to) + + def admin_view(self, view, cacheable=False): + """ + Decorator to create an admin view attached to this ``AdminSite``. This + wraps the view and provides permission checking by calling + ``self.has_permission``. + + You'll want to use this from within ``AdminSite.get_urls()``: + + class MyAdminSite(AdminSite): + + def get_urls(self): + from django.urls import path -class AdminSiteOTPRequired(AdminSiteOTPRequiredMixin, AdminSite): + urls = super().get_urls() + urls += [ + path('my_view/', self.admin_view(some_view)) + ] + return urls + + By default, admin_views are marked non-cacheable using the + ``never_cache`` decorator. If the view can be safely cached, set + cacheable=True. + """ + def inner(request, *args, **kwargs): + if self.has_admin_permission(request) and not self.has_mfa_setup(request): + return self.redirect_to_mfa_setup(request) + + if not self.has_permission(request): + if request.path == reverse('admin:logout', current_app=self.name): + index_path = reverse('admin:index', current_app=self.name) + return HttpResponseRedirect(index_path) + + # Inner import to prevent django.contrib.admin (app) from + # importing django.contrib.auth.models.User (unrelated model). + from django.contrib.auth.views import redirect_to_login + return redirect_to_login( + request.get_full_path(), + reverse('%s:login' % self.name, current_app=self.name) + ) + return view(request, *args, **kwargs) + + if not cacheable: + inner = never_cache(inner) + # We add csrf_protect here so this function can be used as a utility + # function for any view, without having to repeat 'csrf_protect'. + if not getattr(view, 'csrf_exempt', False): + inner = csrf_protect(inner) + return update_wrapper(inner, view) + + +class TwoFactorAdminSite(TwoFactorAdminSiteMixin, AdminSite): """ - AdminSite enforcing OTP verified staff users. + AdminSite with MFA Support. """ pass -def patch_admin(): - @monkeypatch_method(AdminSite) - def login(self, request, extra_context=None): - """ - Redirects to the site login page for the given HttpRequest. - """ - redirect_to = request.POST.get(REDIRECT_FIELD_NAME, request.GET.get(REDIRECT_FIELD_NAME)) +class AdminSiteOTPRequiredMixin(TwoFactorAdminSiteMixin): + warnings.warn('AdminSiteOTPRequiredMixin is deprecated by TwoFactorAdminSiteMixin, please update.', + category=DeprecationWarning) + pass - if not redirect_to or not url_has_allowed_host_and_scheme(url=redirect_to, allowed_hosts=[request.get_host()]): - redirect_to = resolve_url(settings.LOGIN_REDIRECT_URL) - return redirect_to_login(redirect_to) +class AdminSiteOTPRequired(TwoFactorAdminSite): + warnings.warn('AdminSiteOTPRequired is deprecated by TwoFactorAdminSite, please update.', + category=DeprecationWarning) + pass + + +def patch_admin(): + warnings.warn('two-factor admin patching will be removed, use TwoFactorAdminSite or TwoFactorAdminSiteMixin.', + category=DeprecationWarning) + # overrides + setattr(AdminSite, 'login', TwoFactorAdminSiteMixin.login) + setattr(AdminSite, 'admin_view', TwoFactorAdminSiteMixin.admin_view) + setattr(AdminSite, 'has_permission', TwoFactorAdminSiteMixin.has_permission) + # additions + setattr(AdminSite, 'has_admin_permission', original_has_permission) + setattr(AdminSite, 'has_mfa_setup', TwoFactorAdminSiteMixin.has_mfa_setup) + setattr(AdminSite, 'redirect_to_mfa_setup', TwoFactorAdminSiteMixin.redirect_to_mfa_setup) def unpatch_admin(): + warnings.warn('django-two-factor admin patching is deprecated, use TwoFactorAdminSite or TwoFactorAdminSiteMixin.', + category=DeprecationWarning) + # we really only need unpatching in our tests so this can be a noop. + # overrides setattr(AdminSite, 'login', original_login) + setattr(AdminSite, 'admin_view', original_admin_view) + setattr(AdminSite, 'has_permission', original_has_permission) + # NOTE: this unpatching doesn't really work, but becuase it just patches in our mixin it isn't harmful. original_login = AdminSite.login +original_admin_view = AdminSite.admin_view +original_has_permission = AdminSite.has_permission From 4c432a927bccc2dd890643b632a44b8171832518 Mon Sep 17 00:00:00 2001 From: Darrel O'Pry Date: Fri, 6 May 2022 23:49:35 -0400 Subject: [PATCH 2/2] feat: remove monkey patching --- CHANGELOG.md | 26 ++++++++++++++++++++++++++ docs/configuration.rst | 9 +-------- tests/test_admin.py | 18 ------------------ two_factor/admin.py | 30 ------------------------------ two_factor/apps.py | 6 ------ 5 files changed, 27 insertions(+), 62 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1380752f2..193d5bac6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,31 @@ ### Changed +### Removed + +- Admin Monkey Patching + + The Admin UI will not longer be automatically patched. The `TwoFactorSiteAdmin` will need to be explicitly + configured in urls.py. + + ```py + # urls.py + from django.urls import path + from two_factor.admin import TwoFactorAdminSite + url_patterns = [ + path('admin/', TwoFactorAdminSite().urls), + ] + ``` + + Custom admin sites can extend `TwoFactorSiteAdmin` or `TwoFactorSideAdminMixin` to inherit the behavior. + + ```py + # admin.py + class MyCustomAdminSite(TwoFactorSiteAdminMixin, AdminSite): + # implement your customizations here. + pass + ``` + ## 1.14.0 @@ -35,6 +60,7 @@ - The QR code now always uses a white background to support pages displayed with a dark theme. + ### Removed - Python 3.5 and 3.6 support diff --git a/docs/configuration.rst b/docs/configuration.rst index 738a833c0..aafa36494 100644 --- a/docs/configuration.rst +++ b/docs/configuration.rst @@ -4,13 +4,6 @@ Configuration General Settings ---------------- -``TWO_FACTOR_PATCH_ADMIN`` (default: ``True``) - Whether the Django admin is patched to use the default login view. - - .. warning:: - The admin currently does not enforce one-time passwords being set for - admin users. - ``LOGIN_URL`` Should point to the login view provided by this application as described in setup. This login view handles password authentication followed by a one-time @@ -123,7 +116,7 @@ Next, add additional urls to your config: # urls.py from two_factor.gateways.twilio.urls import urlpatterns as tf_twilio_urls - + urlpatterns = [ path('', include(tf_twilio_urls)), ... diff --git a/tests/test_admin.py b/tests/test_admin.py index d29e8dd95..13c9c7a74 100644 --- a/tests/test_admin.py +++ b/tests/test_admin.py @@ -5,28 +5,11 @@ from django.test import TestCase from django.test.utils import override_settings -from two_factor.admin import patch_admin, unpatch_admin - from .utils import UserMixin @override_settings(ROOT_URLCONF='tests.urls_admin') class TwoFactorAdminSiteTest(UserMixin, TestCase): - - def setUp(self): - patch_admin() - - def tearDown(self): - unpatch_admin() - - def test(self): - response = self.client.get('/admin/', follow=True) - redirect_to = '%s?next=/admin/' % reverse('admin:login') - self.assertRedirects(response, redirect_to) - - -@override_settings(ROOT_URLCONF='tests.urls_admin') -class AdminPatchTest(TestCase): """ otp_admin is admin console that needs OTP for access. Only admin users (is_staff and is_active) @@ -55,7 +38,6 @@ def test_anonymous_get_admin_login(self): response = self.client.get(login_url, follow=True) self.assertEqual(response.status_code, 200) - def test_is_staff_not_verified_not_setup_get_admin_index_redirects_to_setup(self): """ admins without MFA setup should be redirected to the setup page. diff --git a/two_factor/admin.py b/two_factor/admin.py index 7abcb4a38..62fa11b1a 100644 --- a/two_factor/admin.py +++ b/two_factor/admin.py @@ -43,7 +43,6 @@ def redirect_to_mfa_setup(self, request): # have MFA enabled on their account. We're going to redirect them # to the MFA setup. - # TODO: Add redirect_to functionality to MFA setup. # TODO: Add message indicating why the user was directed or setup and MFA required # interstitial page to explain to the user they need to setup MFA. setup_url = reverse('two_factor:setup') @@ -133,32 +132,3 @@ class AdminSiteOTPRequired(TwoFactorAdminSite): warnings.warn('AdminSiteOTPRequired is deprecated by TwoFactorAdminSite, please update.', category=DeprecationWarning) pass - - -def patch_admin(): - warnings.warn('two-factor admin patching will be removed, use TwoFactorAdminSite or TwoFactorAdminSiteMixin.', - category=DeprecationWarning) - # overrides - setattr(AdminSite, 'login', TwoFactorAdminSiteMixin.login) - setattr(AdminSite, 'admin_view', TwoFactorAdminSiteMixin.admin_view) - setattr(AdminSite, 'has_permission', TwoFactorAdminSiteMixin.has_permission) - # additions - setattr(AdminSite, 'has_admin_permission', original_has_permission) - setattr(AdminSite, 'has_mfa_setup', TwoFactorAdminSiteMixin.has_mfa_setup) - setattr(AdminSite, 'redirect_to_mfa_setup', TwoFactorAdminSiteMixin.redirect_to_mfa_setup) - - -def unpatch_admin(): - warnings.warn('django-two-factor admin patching is deprecated, use TwoFactorAdminSite or TwoFactorAdminSiteMixin.', - category=DeprecationWarning) - # we really only need unpatching in our tests so this can be a noop. - # overrides - setattr(AdminSite, 'login', original_login) - setattr(AdminSite, 'admin_view', original_admin_view) - setattr(AdminSite, 'has_permission', original_has_permission) - # NOTE: this unpatching doesn't really work, but becuase it just patches in our mixin it isn't harmful. - - -original_login = AdminSite.login -original_admin_view = AdminSite.admin_view -original_has_permission = AdminSite.has_permission diff --git a/two_factor/apps.py b/two_factor/apps.py index e627f2a8f..41e7dff34 100644 --- a/two_factor/apps.py +++ b/two_factor/apps.py @@ -1,12 +1,6 @@ from django.apps import AppConfig -from django.conf import settings class TwoFactorConfig(AppConfig): name = 'two_factor' verbose_name = "Django Two Factor Authentication" - - def ready(self): - if getattr(settings, 'TWO_FACTOR_PATCH_ADMIN', True): - from .admin import patch_admin - patch_admin()