From fdf20093e0f8cd064673aa1597c20727ed4dd2a0 Mon Sep 17 00:00:00 2001 From: Tim Graham Date: Thu, 12 Feb 2015 12:14:19 -0500 Subject: [PATCH] Fixed #24334 -- Allowed admin password reset to work with non-digit custom user model primary key. Thanks Loic for help and Simon for review. --- django/contrib/auth/admin.py | 20 +++++++++--- tests/auth_tests/test_views.py | 37 ++++++++++++++++++++++ tests/auth_tests/urls_custom_user_admin.py | 21 ++++++++++++ 3 files changed, 74 insertions(+), 4 deletions(-) create mode 100644 tests/auth_tests/urls_custom_user_admin.py diff --git a/django/contrib/auth/admin.py b/django/contrib/auth/admin.py index f9eee5d7b8..eb865a1c93 100644 --- a/django/contrib/auth/admin.py +++ b/django/contrib/auth/admin.py @@ -2,6 +2,7 @@ from django.conf import settings from django.conf.urls import url from django.contrib import admin, messages from django.contrib.admin.options import IS_POPUP_VAR +from django.contrib.admin.utils import unquote from django.contrib.auth import update_session_auth_hash from django.contrib.auth.forms import ( AdminPasswordChangeForm, UserChangeForm, UserCreationForm, @@ -11,9 +12,9 @@ from django.core.exceptions import PermissionDenied from django.core.urlresolvers import reverse from django.db import transaction from django.http import Http404, HttpResponseRedirect -from django.shortcuts import get_object_or_404 from django.template.response import TemplateResponse from django.utils.decorators import method_decorator +from django.utils.encoding import force_text from django.utils.html import escape from django.utils.translation import ugettext, ugettext_lazy as _ from django.views.decorators.csrf import csrf_protect @@ -80,7 +81,7 @@ class UserAdmin(admin.ModelAdmin): def get_urls(self): return [ - url(r'^(\d+)/password/$', self.admin_site.admin_view(self.user_change_password), name='auth_user_password_change'), + url(r'^(.+)/password/$', self.admin_site.admin_view(self.user_change_password), name='auth_user_password_change'), ] + super(UserAdmin, self).get_urls() def lookup_allowed(self, lookup, value): @@ -124,7 +125,12 @@ class UserAdmin(admin.ModelAdmin): def user_change_password(self, request, id, form_url=''): if not self.has_change_permission(request): raise PermissionDenied - user = get_object_or_404(self.get_queryset(request), pk=id) + user = self.get_object(request, unquote(id)) + if user is None: + raise Http404(_('%(name)s object with primary key %(key)r does not exist.') % { + 'name': force_text(self.model._meta.verbose_name), + 'key': escape(id), + }) if request.method == 'POST': form = self.change_password_form(user, request.POST) if form.is_valid(): @@ -135,7 +141,13 @@ class UserAdmin(admin.ModelAdmin): messages.success(request, msg) update_session_auth_hash(request, form.user) return HttpResponseRedirect( - reverse('%s:auth_user_change' % self.admin_site.name, args=(user.pk,)) + reverse( + '%s:auth_%s_change' % ( + self.admin_site.name, + user._meta.model_name, + ), + args=(user.pk,), + ) ) else: form = self.change_password_form(user) diff --git a/tests/auth_tests/test_views.py b/tests/auth_tests/test_views.py index ec991153a0..85177dab28 100644 --- a/tests/auth_tests/test_views.py +++ b/tests/auth_tests/test_views.py @@ -18,6 +18,7 @@ from django.contrib.sessions.middleware import SessionMiddleware from django.contrib.sites.requests import RequestSite from django.core import mail from django.core.urlresolvers import NoReverseMatch, reverse, reverse_lazy +from django.db import connection from django.http import HttpRequest, QueryDict from django.middleware.csrf import CsrfViewMiddleware from django.test import ( @@ -30,6 +31,7 @@ from django.utils.http import urlquote from django.utils.six.moves.urllib.parse import ParseResult, urlparse from django.utils.translation import LANGUAGE_SESSION_KEY +from .models import UUIDUser from .settings import AUTH_TEMPLATES @@ -902,3 +904,38 @@ class ChangelistTests(AuthViewsTestCase): self.assertEqual(row.user_id, self.admin.pk) self.assertEqual(row.object_id, str(u.pk)) self.assertEqual(row.change_message, 'Changed password.') + + def test_password_change_bad_url(self): + response = self.client.get(reverse('auth_test_admin:auth_user_password_change', args=('foobar',))) + self.assertEqual(response.status_code, 404) + + +@override_settings( + AUTH_USER_MODEL='auth.UUIDUser', + ROOT_URLCONF='auth_tests.urls_custom_user_admin', +) +class UUIDUserTests(TestCase): + + def test_admin_password_change(self): + u = UUIDUser.objects.create_superuser(username='uuid', email='foo@bar.com', password='test') + self.assertTrue(self.client.login(username='uuid', password='test')) + + user_change_url = reverse('custom_user_admin:auth_uuiduser_change', args=(u.pk,)) + response = self.client.get(user_change_url) + self.assertEqual(response.status_code, 200) + + password_change_url = reverse('custom_user_admin:auth_user_password_change', args=(u.pk,)) + response = self.client.get(password_change_url) + self.assertEqual(response.status_code, 200) + + # A LogEntry is created with pk=1 which breaks a FK constraint on MySQL + with connection.constraint_checks_disabled(): + response = self.client.post(password_change_url, { + 'password1': 'password1', + 'password2': 'password1', + }) + self.assertRedirects(response, user_change_url) + row = LogEntry.objects.latest('id') + self.assertEqual(row.user_id, 1) # harcoded in CustomUserAdmin.log_change() + self.assertEqual(row.object_id, str(u.pk)) + self.assertEqual(row.change_message, 'Changed password.') diff --git a/tests/auth_tests/urls_custom_user_admin.py b/tests/auth_tests/urls_custom_user_admin.py new file mode 100644 index 0000000000..02389de4f2 --- /dev/null +++ b/tests/auth_tests/urls_custom_user_admin.py @@ -0,0 +1,21 @@ +from django.conf.urls import include, url +from django.contrib import admin +from django.contrib.auth import get_user_model +from django.contrib.auth.admin import UserAdmin +from django.contrib.auth.urls import urlpatterns + +site = admin.AdminSite(name='custom_user_admin') + + +class CustomUserAdmin(UserAdmin): + def log_change(self, request, object, message): + # LogEntry.user column doesn't get altered to expect a UUID, so set an + # integer manually to avoid causing an error. + request.user.pk = 1 + super(CustomUserAdmin, self).log_change(request, object, message) + +site.register(get_user_model(), CustomUserAdmin) + +urlpatterns += [ + url(r'^admin/', include(site.urls)), +]