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.

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`
+Should you use a user model with a custom primary that isn't a subclass of either
+`InterField`, `UUIDField` or `SlugField`, you will need pro 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 PK, 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,27 +24,6 @@ def check_legacy_settings(app_configs, **kwargs):
     return warnings
 
 
-def check_url_allowed_attributes(app_configs, **kwargs):
-    errors = []
-    required_attributes = [
-        'user_id',
-        'email',
-        'username',
-    ]
-    set_attributes = hijack_settings.HIJACK_URL_ALLOWED_ATTRIBUTES
-    if not any(attribute in set_attributes for attribute in required_attributes):
-        errors.append(
-            Error(
-                'Setting HIJACK_URL_ALLOWED_ATTRIBUTES needs to be subset of (%s)'
-                    % ', '.join(required_attributes),
-                hint=None,
-                obj=set_attributes,
-                id='hijack.E001',
-            )
-        )
-    return errors
-
-
 def check_custom_authorization_check_importable(app_configs, **kwargs):
     errors = []
     authorization_check = hijack_settings.HIJACK_AUTHORIZATION_CHECK
@@ -98,7 +77,6 @@ def check_staff_authorization_settings(app_configs, **kwargs):
 def register_checks():
     for check in [
         check_legacy_settings,
-        check_url_allowed_attributes,
         check_custom_authorization_check_importable,
         check_hijack_decorator_importable,
         check_staff_authorization_settings,

hijack/settings.py

@@ -8,9 +8,9 @@
         'legacy_name': 'HIJACK_NOTIFY_ADMIN',
     },
     {
-        'name': 'HIJACK_URL_ALLOWED_ATTRIBUTES',
-        'default': ('user_id', 'email', 'username'),
-        'legacy_name': 'ALLOWED_HIJACKING_USER_ATTRIBUTES',
+        'name': 'HIJACK_USER_URL_PATTERN',
+        'default': None,
+        'legacy_name': None,
     },
     {
         'name': 'HIJACK_AUTHORIZE_STAFF',

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/urls.py

@@ -1,15 +1,32 @@
 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'
-urlpatterns = [
-    url(
-        r'^release-hijack/$',
-        views.release_hijack,
-        name='release_hijack'
+pk = get_user_model()._meta.pk
+if hijack_settings.HIJACK_USER_URL_PATTERN:
+    acquire_user_path = re_path(
+        hijack_settings.HIJACK_USER_URL_PATTERN, views.hijack_user, name="acquire"
+    )
+elif isinstance(pk, (models.IntegerField, models.AutoField)):
+    acquire_user_path = path(f"acquire/<int:pk>/", views.hijack_user, name="acquire")
+elif isinstance(pk, models.UUIDField):
+    acquire_user_path = path(f"acquire/<uuid:pk>/", views.hijack_user, name="acquire")
+elif isinstance(pk, models.SlugField):
+    acquire_user_path = path(f"acquire/<slug:pk>/", views.hijack_user, 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 = [
+    acquire_user_path,
+    path("release/", views.release_hijack, name="release"),
 ]
 
 if hijack_settings.HIJACK_DISPLAY_WARNING:
@@ -18,23 +35,3 @@
         views.disable_hijack_warning,
         name='disable_hijack_warning'
     ))
-
-hijacking_user_attributes = hijack_settings.HIJACK_URL_ALLOWED_ATTRIBUTES
-if 'email' in hijacking_user_attributes:
-    urlpatterns.append(url(
-        r'^email/(?P<email>[^@\s]+@[^@\s]+\.[^@\s]+)/$',
-        views.login_with_email,
-        name='login_with_email'
-    ))
-if 'username' in hijacking_user_attributes:
-    urlpatterns.append(url(
-        r'^username/(?P<username>.*)/$',
-        views.login_with_username,
-        name='login_with_username'
-    ))
-if 'user_id' in hijacking_user_attributes:
-    urlpatterns.append(url(
-        r'^(?P<user_id>[\w-]+)/$',
-        views.login_with_id,
-        name='login_with_id'
-    ))

hijack/views.py

@@ -1,6 +1,5 @@
 # -*- encoding: utf-8 -*-
 from django.contrib.auth.decorators import login_required
-from django.http import HttpResponseBadRequest
 from django.shortcuts import get_object_or_404
 
 from hijack.decorators import hijack_require_http_methods, hijack_decorator
@@ -12,27 +11,8 @@
 
 @hijack_decorator
 @hijack_require_http_methods
-def login_with_id(request, user_id):
-    # input(user_id) is unicode
-    try:
-        user_id = int(user_id)
-    except ValueError:
-        return HttpResponseBadRequest('user_id must be an integer value.')
-    user = get_object_or_404(get_user_model(), pk=user_id)
-    return login_user(request, user)
-
-
-@hijack_decorator
-@hijack_require_http_methods
-def login_with_email(request, email):
-    user = get_object_or_404(get_user_model(), email=email)
-    return login_user(request, user)
-
-
-@hijack_decorator
-@hijack_require_http_methods
-def login_with_username(request, username):
-    user = get_object_or_404(get_user_model(), username=username)
+def hijack_user(request, **kwargs):
+    user = get_object_or_404(get_user_model(), **kwargs)
     return login_user(request, user)