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

Co-Authored-By: Alexandr Artemyev <mogost@gmail.com>

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

@@ -23,6 +23,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 = ["user_id", "email", "username"]

hijack/settings.py

@@ -1,3 +1,5 @@
+import warnings
+
 from django.conf import settings as django_settings
 
 SETTINGS = (
@@ -6,6 +8,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"),
@@ -53,6 +60,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

@@ -4,7 +4,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

@@ -3,7 +3,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

@@ -21,53 +21,24 @@ 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)
+        def test_check_custom_authorization_check_importable(self):
+            errors = checks.check_custom_authorization_check_importable(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",
+            with SettingsOverride(
+                hijack_settings, HIJACK_AUTHORIZATION_CHECK="not.importable"
+            ):
+                expected_errors = [
+                    Error(
+                        "Setting HIJACK_AUTHORIZATION_CHECK cannot be imported",
+                        hint=None,
+                        obj="not.importable",
+                        id="hijack.E002",
+                    )
+                ]
+                errors = checks.check_custom_authorization_check_importable(
+                    HijackConfig
                 )
-            ]
-            self.assertEqual(errors, expected_errors)
-
-    def test_check_custom_authorization_check_importable(self):
-        errors = checks.check_custom_authorization_check_importable(HijackConfig)
-        self.assertFalse(errors)
-        with SettingsOverride(
-            hijack_settings, HIJACK_AUTHORIZATION_CHECK="not.importable"
-        ):
-            expected_errors = [
-                Error(
-                    "Setting HIJACK_AUTHORIZATION_CHECK cannot be imported",
-                    hint=None,
-                    obj="not.importable",
-                    id="hijack.E002",
-                )
-            ]
-            errors = checks.check_custom_authorization_check_importable(HijackConfig)
-            self.assertEqual(errors, expected_errors)
+                self.assertEqual(errors, expected_errors)
 
     def test_hijack_decorator_importable(self):
         errors = checks.check_hijack_decorator_importable(HijackConfig)

hijack/tests/test_hijack.py

@@ -1,5 +1,3 @@
-from urllib.parse import unquote_plus
-
 import pytest
 from django.conf import settings
 from django.contrib.auth.models import AnonymousUser, User
@@ -48,10 +46,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
@@ -67,9 +65,11 @@ 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()
 
@@ -88,57 +88,21 @@ 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/release/", reverse("hijack:release"))
+        self.assertEqual("/hijack/acquire/1/", reverse("hijack:acquire", args=[1]))
         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"},
-                )
-            ),
+            "/hijack/acquire/2/", reverse("hijack:acquire", kwargs={"pk": 2})
         )
 
-    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):
+    def test_hijack_acquire_user(self):
         response = self.client.post(
-            "/hijack/username/%s/" % self.user_username, follow=True
+            reverse("hijack:acquire", args=[self.user.pk]), follow=True
         )
         self.assertHijackSuccess(response)
         self._release_hijack()
-        response = self.client.post("/hijack/username/dfjakhdl/", follow=True)
-        self.assertEqual(response.status_code, 404)
-
-    def test_hijack_url_email(self):
-        response = self.client.post("/hijack/email/%s/" % self.user_email, 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):
@@ -154,7 +118,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):
@@ -206,11 +170,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(
@@ -290,11 +249,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(

hijack/tests/test_urls.py

@@ -0,0 +1,48 @@
+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,48 @@
+from unittest.mock import MagicMock
+
+import pytest
+from django.contrib.auth import get_user_model
+
+from hijack import views
+
+
+def test_login_with_id(rf, db, admin_user, monkeypatch):
+    request = rf.post("/")
+    request.user = admin_user
+    login_user = MagicMock()
+    monkeypatch.setattr("hijack.views.login_user", login_user)
+    user = get_user_model().objects.create(pk=123)
+    with pytest.deprecated_call():
+        views.login_with_id(request, 123)
+    login_user.assert_called_once_with(request, user)
+
+
+def test_login_with_email(rf, db, admin_user, monkeypatch):
+    request = rf.post("/")
+    request.user = admin_user
+    login_user = MagicMock()
+    monkeypatch.setattr("hijack.views.login_user", login_user)
+    user = get_user_model().objects.create(email="spiderman@averngers.com")
+    with pytest.deprecated_call():
+        views.login_with_email(request, "spiderman@averngers.com")
+    login_user.assert_called_once_with(request, user)
+
+
+def test_login_with_username(rf, db, admin_user, monkeypatch):
+    request = rf.post("/")
+    request.user = admin_user
+    login_user = MagicMock()
+    monkeypatch.setattr("hijack.views.login_user", login_user)
+    user = get_user_model().objects.create(username="spiderman")
+    with pytest.deprecated_call():
+        views.login_with_username(request, "spiderman")
+    login_user.assert_called_once_with(request, user)
+
+
+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)