Skip to content

Commit

Permalink
feat: remove monkey patching
Browse files Browse the repository at this point in the history
  • Loading branch information
dopry committed Jun 8, 2022
1 parent ee91b53 commit d76311c
Show file tree
Hide file tree
Showing 6 changed files with 28 additions and 63 deletions.
26 changes: 26 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,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

Expand Down Expand Up @@ -38,6 +63,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
Expand Down
9 changes: 1 addition & 8 deletions docs/configuration.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)),
...
Expand Down
18 changes: 0 additions & 18 deletions tests/test_admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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.
Expand Down
30 changes: 0 additions & 30 deletions two_factor/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,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')
Expand Down Expand Up @@ -159,32 +158,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
6 changes: 0 additions & 6 deletions two_factor/apps.py
Original file line number Diff line number Diff line change
@@ -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()
2 changes: 1 addition & 1 deletion two_factor/views/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -394,7 +394,7 @@ def dispatch(self, request, *args, **kwargs):

@class_view_decorator(never_cache)
@class_view_decorator(login_required)
class SetupView(SuccessURLAllowedHostsMixin, IdempotentSessionWizardView):
class SetupView(RedirectURLMixin, IdempotentSessionWizardView):
"""
View for handling OTP setup using a wizard.
Expand Down

0 comments on commit d76311c

Please sign in to comment.