feat: remove monkey patching

Author: dopry

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

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)),
         ...

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.

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

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()