Add support for non-integer primary key user models

Simplify URL and view structure. Add support for multiple PK types
based on URL pattern as well as natural key support via URL patterns.

Changes:

* Deprecate `HIJACK_URL_ALLOWED_ATTRIBUTES` setting favoring `HIJACK_USER_URL_PATTERN`.
* Deprecate URL names `login_with_id`, `login_with_username` and `login_with_email` favoring `acquire`.
* Deprecate URL name `release_hijack` favoring `release`.
* Deprecate views `login_with_id`, `login_with_username` and `login_with_email` favoring `release_user_view`.
* Deprecate view `release_hijack` favoring `release_user_view`.

Close #196
Close #183
Close #184
Close #198
Close #175

docs/configuration.md

@@ -81,9 +81,14 @@ MIDDLEWARE_CLASSES = (
 Hide or display the yellow notification bar show to hijackers. Default: `True`.
 ## `HIJACK_USE_BOOTSTRAP`
 Whether a Bootstrap-optimized notification bar is used. Default: `False`.
-## `HIJACK_URL_ALLOWED_ATTRIBUTES`
-User attributes by which a user can be hijacked over a URL. Default: `('user_id', 'email', 'username')`.
-May be changed to a subset of the default value.
+## `HIJACK_USER_URL_PATTERN`
+User model with a custom primary that is not a subclass of either
+`InterField`, `UUIDField` or `SlugField`, you will need to provide a regex URL pattern
+(compatible with `re_path`) that is valid for your primary key. Default: `None`
+This setting may also be used to hijack users based on other fields than the primary key, e.G.:
+```python
+HIJACK_USER_URL_PATTERN = r'^acquire/(?P<username>\w+)/$'  # hijack a user based on the username
+```
 ## `HIJACK_AUTHORIZE_STAFF`
 Whether staff members are allowed to hijack. Default: `False`.
 ## `HIJACK_AUTHORIZE_STAFF_TO_HIJACK_STAFF`

hijack/checks.py

@@ -24,6 +24,7 @@ def check_legacy_settings(app_configs, **kwargs):
     return warnings
 
 
+# @TODO: HIJACK_URL_ALLOWED_ATTRIBUTES is deprecated, remove with next major release.
 def check_url_allowed_attributes(app_configs, **kwargs):
     errors = []
     required_attributes = [

hijack/settings.py

@@ -1,4 +1,6 @@
 # -*- coding: utf-8 -*-
+import warnings
+
 from django.conf import settings as django_settings
 
 SETTINGS = (
@@ -7,6 +9,11 @@
         'default': True,
         'legacy_name': 'HIJACK_NOTIFY_ADMIN',
     },
+    {
+        'name': 'HIJACK_USER_URL_PATTERN',
+        'default': None,
+        'legacy_name': None,
+    },
     {
         'name': 'HIJACK_URL_ALLOWED_ATTRIBUTES',
         'default': ('user_id', 'email', 'username'),
@@ -54,6 +61,12 @@
     },
 )
 
+if hasattr(django_settings, 'HIJACK_URL_ALLOWED_ATTRIBUTES'):
+    warnings.warn(
+        'The "HIJACK_URL_ALLOWED_ATTRIBUTES" setting has be deprecated in favor of "HIJACK_USER_URL_PATTERN".',
+        DeprecationWarning
+    )
+
 for setting in SETTINGS:
     if setting['legacy_name']:
         default = getattr(django_settings, setting['legacy_name'], setting['default'])

hijack/templates/hijack/notifications.html

@@ -5,7 +5,7 @@
 <div id="hijacked-warning" class="hijacked-warning hijacked-warning-default">
     <span style="padding-top: 4px; float: left;"><b>{% blocktrans with user=request.user%}You are currently working on behalf of {{ user }}.{% endblocktrans %}</b></span>
     <div class="hijacked-warning-controls hijacked-warning-controls-pull-right">
-        <form action="{% url 'hijack:release_hijack' %}" method="POST">
+        <form action="{% url 'hijack:release' %}" method="POST">
             {% csrf_token %}
             <button class="django-hijack-button-default">{% blocktrans with user=request.user %}release {{ user }}{% endblocktrans %}</button>
         </form>

hijack/templates/hijack/notifications_bootstrap.html

@@ -4,7 +4,7 @@
 <div id="hijacked-warning" class="alert hijacked-warning hijacked-warning-bootstrap" role="alert">
     {% blocktrans with user=request.user%}You are currently working on behalf of {{ user }}.{% endblocktrans %}
     <div class="hijacked-warning-controls">
-        <form action="{% url 'hijack:release_hijack' %}" method="POST">
+        <form action="{% url 'hijack:release' %}" method="POST">
             {% csrf_token %}
             <button type="submit" class="btn btn-default btn-sm django-hijack-button-bootstrap">{% blocktrans with user=request.user %}release {{ user }}{% endblocktrans %}</button>
         </form>

hijack/tests/test_checks.py

@@ -29,31 +29,6 @@ def test_check_legacy_settings(self):
                 ]
                 self.assertEqual(warnings, expected_warnings)
 
-        def test_check_url_allowed_attributes(self):
-            errors = checks.check_url_allowed_attributes(HijackConfig)
-            self.assertFalse(errors)
-
-            with SettingsOverride(hijack_settings, HIJACK_URL_ALLOWED_ATTRIBUTES=('username',)):
-                errors = checks.check_url_allowed_attributes(HijackConfig)
-                self.assertFalse(errors)
-
-            with SettingsOverride(hijack_settings, HIJACK_URL_ALLOWED_ATTRIBUTES=('username', 'email')):
-                errors = checks.check_url_allowed_attributes(HijackConfig)
-                self.assertFalse(errors)
-
-            with SettingsOverride(hijack_settings, HIJACK_URL_ALLOWED_ATTRIBUTES=('other',)):
-                errors = checks.check_url_allowed_attributes(HijackConfig)
-                expected_errors = [
-                    Error(
-                        'Setting HIJACK_URL_ALLOWED_ATTRIBUTES needs to be '
-                        'subset of (user_id, email, username)',
-                        hint=None,
-                        obj=hijack_settings.HIJACK_URL_ALLOWED_ATTRIBUTES,
-                        id='hijack.E001',
-                    )
-                ]
-                self.assertEqual(errors, expected_errors)
-
         def test_check_custom_authorization_check_importable(self):
             errors = checks.check_custom_authorization_check_importable(HijackConfig)
             self.assertFalse(errors)

hijack/tests/test_hijack.py

@@ -1,7 +1,7 @@
 # -*- coding: utf-8 -*-
 from django.conf import settings
 from django.contrib.auth.models import User
-from django.test import TestCase, Client, RequestFactory
+from django.test import TestCase, Client, RequestFactory, override_settings
 from django.contrib.auth.models import AnonymousUser
 
 from compat import import_string, unquote_plus
@@ -44,10 +44,10 @@ def tearDown(self):
         self.client.logout()
 
     def _hijack(self, user):
-        return self.client.post('/hijack/%d/' % user.id, follow=True)
+        return self.client.post(reverse('hijack:acquire', args=[user.pk]), follow=True)
 
     def _release_hijack(self):
-        response = self.client.post('/hijack/release-hijack/', follow=True)
+        response = self.client.post(reverse('hijack:release'), follow=True)
         self.assertEqual(response.status_code, 200)
         self.assertFalse('hijacked-warning' in str(response.content))
         return response
@@ -64,9 +64,9 @@ def tearDown(self):
     def test_basic_hijack(self):
         client = Client()
         client.login(username=self.superuser_username, password=self.superuser_password)
-        hijacked_response = client.post('/hijack/%d/' % self.user.id, follow=True)
+        hijacked_response = client.post(reverse('hijack:acquire', args=[self.user.pk]), follow=True)
         self.assertEqual(hijacked_response.status_code, 200)
-        hijack_released_response = client.post('/hijack/release-hijack/', follow=True)
+        hijack_released_response = client.post(reverse('hijack:release'), follow=True)
         self.assertEqual(hijack_released_response.status_code, 200)
         client.logout()
 
@@ -83,35 +83,15 @@ def assertHijackPermissionDenied(self, response):
 
     def test_hijack_urls(self):
         self.assertEqual('/hijack/disable-hijack-warning/', reverse('hijack:disable_hijack_warning'))
-        self.assertEqual('/hijack/release-hijack/', reverse('hijack:release_hijack'))
-        self.assertEqual('/hijack/1/', reverse('hijack:login_with_id', args=[1]))
-        self.assertEqual('/hijack/2/', reverse('hijack:login_with_id', kwargs={'user_id': 2}))
-        self.assertEqual('/hijack/username/bob/', reverse('hijack:login_with_username', args=['bob']))
-        self.assertEqual('/hijack/username/bob_too/', reverse('hijack:login_with_username', kwargs={'username': 'bob_too'}))
-        self.assertEqual('/hijack/email/bob@bobsburgers.com/', unquote_plus(reverse('hijack:login_with_email', args=['bob@bobsburgers.com'])))
-        self.assertEqual('/hijack/email/bob_too@bobsburgers.com/', unquote_plus(reverse('hijack:login_with_email', kwargs={'email': 'bob_too@bobsburgers.com'})))
-
-    def test_hijack_url_user_id(self):
-        response = self.client.post('/hijack/%d/' % self.user.id, follow=True)
-        self.assertHijackSuccess(response)
-        self._release_hijack()
-        response = self.client.post('/hijack/%s/' % self.user.username, follow=True)
-        self.assertEqual(response.status_code, 400)
-        response = self.client.post('/hijack/-1/', follow=True)
-        self.assertEqual(response.status_code, 404)
-
-    def test_hijack_url_username(self):
-        response = self.client.post('/hijack/username/%s/' % self.user_username, follow=True)
-        self.assertHijackSuccess(response)
-        self._release_hijack()
-        response = self.client.post('/hijack/username/dfjakhdl/', follow=True)
-        self.assertEqual(response.status_code, 404)
+        self.assertEqual('/hijack/release/', reverse('hijack:release'))
+        self.assertEqual('/hijack/acquire/1/', reverse('hijack:acquire', args=[1]))
+        self.assertEqual('/hijack/acquire/2/', reverse('hijack:acquire', kwargs={'pk': 2}))
 
-    def test_hijack_url_email(self):
-        response = self.client.post('/hijack/email/%s/' % self.user_email, follow=True)
+    def test_hijack_acquire_user(self):
+        response = self.client.post(reverse('hijack:acquire', args=[self.user.pk]), follow=True)
         self.assertHijackSuccess(response)
         self._release_hijack()
-        response = self.client.post('/hijack/email/dfjak@hdl.com/', follow=True)
+        response = self.client.post(reverse('hijack:acquire', args=[1337]), follow=True)  # doe not exist
         self.assertEqual(response.status_code, 404)
 
     def test_hijack_permission_denied(self):
@@ -125,7 +105,7 @@ def test_hijack_permission_denied(self):
         self.assertHijackPermissionDenied(response)
 
     def test_release_before_hijack(self):
-        response = self.client.post('/hijack/release-hijack/', follow=True)
+        response = self.client.post(reverse('hijack:release'), follow=True)
         self.assertHijackPermissionDenied(response)
 
     def test_last_login_time_not_changed(self):
@@ -171,8 +151,6 @@ def test_permissions(self):
     def test_settings(self):
         self.assertTrue(hasattr(hijack_settings, 'HIJACK_DISPLAY_WARNING'))
         self.assertTrue(hijack_settings.HIJACK_DISPLAY_WARNING)
-        self.assertTrue(hasattr(hijack_settings, 'HIJACK_URL_ALLOWED_ATTRIBUTES'))
-        self.assertEqual(hijack_settings.HIJACK_URL_ALLOWED_ATTRIBUTES, ('user_id', 'email', 'username'))
         self.assertTrue(hasattr(hijack_settings, 'HIJACK_AUTHORIZE_STAFF'))
         self.assertFalse(hijack_settings.HIJACK_AUTHORIZE_STAFF)
         self.assertTrue(hasattr(hijack_settings, 'HIJACK_AUTHORIZE_STAFF_TO_HIJACK_STAFF'))
@@ -240,11 +218,9 @@ def test_is_authorized_staff_authorized_to_hijack_staff(self):
     def test_disallow_get_requests(self):
         self.assertFalse(hijack_settings.HIJACK_ALLOW_GET_REQUESTS)
         protected_urls = [
-            '/hijack/{}/'.format(self.user.id),
-            '/hijack/email/{}/'.format(self.user_email),
-            '/hijack/username/{}/'.format(self.user_username),
+            f'/hijack/acquire/{self.user.id}/',
             '/hijack/disable-hijack-warning/',
-            '/hijack/release-hijack/',
+            '/hijack/release/',
         ]
         for protected_url in protected_urls:
             self.assertEqual(self.client.get(protected_url, follow=True).status_code, 405,

hijack/tests/test_urls.py

@@ -0,0 +1,49 @@
+import pytest
+from django.db import models
+
+from hijack.urls import _get_user_url_pattern
+
+
+def test__get_user_url_pattern__int_pk_url():
+    assert _get_user_url_pattern().pattern.match("acquire/123/")
+    assert not _get_user_url_pattern().pattern.match(
+        "acquire/f545f538-ece0-4883-93d0-c55a13df60a4/"
+    )
+    assert not _get_user_url_pattern().pattern.match("acquire/user-slug/")
+
+
+def test__get_user_url_pattern__uuid_pk_url():
+    assert _get_user_url_pattern(pk=models.UUIDField()).pattern.match(
+        "acquire/f545f538-ece0-4883-93d0-c55a13df60a4/"
+    )
+    assert not _get_user_url_pattern(pk=models.UUIDField()).pattern.match(
+        "acquire/123/"
+    )
+    assert not _get_user_url_pattern(pk=models.UUIDField()).pattern.match(
+        "acquire/user-slug/"
+    )
+
+
+def test__get_user_url_pattern__slug_pk_url():
+    assert _get_user_url_pattern(pk=models.SlugField()).pattern.match(
+        "acquire/user-slug/"
+    )
+
+
+def test__get_user_url_pattern__custom_pattern():
+    assert _get_user_url_pattern(pattern=r"^acquire/(?P<username>\w+)/$").pattern.match(
+        "acquire/spiderman/"
+    )
+    assert not _get_user_url_pattern(
+        pattern=r"^acquire/(?P<username>\w+)/$"
+    ).pattern.match("acquire/spider-man/")
+
+
+def test__get_user_url_pattern__raise_not_implemented_error():
+    with pytest.raises(NotImplementedError) as e:
+        _get_user_url_pattern(pk=models.CharField())
+    assert (
+        "User model's primary key type CharField is not supported."
+        " Please provide your own HIJACK_USER_URL_PATTERN setting."
+        in str(e)
+    )

hijack/tests/test_views.py

@@ -0,0 +1,41 @@
+from unittest.mock import MagicMock
+
+import pytest
+
+from hijack import views
+
+
+def test_login_with_id(rf, monkeypatch):
+    request = rf.get("/")
+    acquire_user_view = MagicMock()
+    monkeypatch.setattr("hijack.views.acquire_user_view", acquire_user_view)
+    with pytest.deprecated_call():
+        views.login_with_id(request, 123)
+    acquire_user_view.assert_called_once_with(request, 123)
+
+
+def test_login_with_email(rf, monkeypatch):
+    request = rf.get("/")
+    acquire_user_view = MagicMock()
+    monkeypatch.setattr("hijack.views.acquire_user_view", acquire_user_view)
+    with pytest.deprecated_call():
+        views.login_with_email(request, "spiderman@averngers.com")
+    acquire_user_view.assert_called_once_with(request, "spiderman@averngers.com")
+
+
+def test_login_with_username(rf, monkeypatch):
+    request = rf.get("/")
+    acquire_user_view = MagicMock()
+    monkeypatch.setattr("hijack.views.acquire_user_view", acquire_user_view)
+    with pytest.deprecated_call():
+        views.login_with_username(request, "spiderman")
+    acquire_user_view.assert_called_once_with(request, "spiderman")
+
+
+def test_release_hijack(rf, monkeypatch):
+    request = rf.get("/")
+    release_user_view = MagicMock()
+    monkeypatch.setattr("hijack.views.release_user_view", release_user_view)
+    with pytest.deprecated_call():
+        views.release_hijack(request)
+    release_user_view.assert_called_once_with(request)

hijack/urls.py

@@ -1,15 +1,38 @@
 from compat import url
+from django.contrib.auth import get_user_model
+from django.db import models
+from django.urls import path, re_path
 
 from hijack import settings as hijack_settings
 from hijack import views
 
-app_name = 'hijack'
+
+def _get_user_url_pattern(pk=None, pattern=None):
+    pk = pk or get_user_model()._meta.pk
+    pattern = pattern or hijack_settings.HIJACK_USER_URL_PATTERN
+
+    if pattern:
+        return re_path(
+            pattern, views.acquire_user_view, name="acquire"
+        )
+    elif isinstance(pk, (models.IntegerField, models.AutoField)):
+        return path(f"acquire/<int:pk>/", views.acquire_user_view, name="acquire")
+    elif isinstance(pk, models.UUIDField):
+        return path(f"acquire/<uuid:pk>/", views.acquire_user_view, name="acquire")
+    elif isinstance(pk, models.SlugField):
+        return path(f"acquire/<slug:pk>/", views.acquire_user_view, name="acquire")
+    else:
+        raise NotImplementedError(
+            f"User model's primary key type {type(pk).__qualname__} is not supported."
+            f" Please provide your own HIJACK_USER_URL_PATTERN setting."
+        )
+
+
+app_name = "hijack"
 urlpatterns = [
-    url(
-        r'^release-hijack/$',
-        views.release_hijack,
-        name='release_hijack'
-    )
+    _get_user_url_pattern(),
+    path("release/", views.release_user_view, name="release"),
+    path("release/", views.release_hijack, name="release_hijack"),
 ]
 
 if hijack_settings.HIJACK_DISPLAY_WARNING: