diff --git a/AUTHORS b/AUTHORS index 88ba4ed877..3a3842f89b 100644 --- a/AUTHORS +++ b/AUTHORS @@ -335,6 +335,7 @@ answer newbie questions, and generally made Django that much better: Armin Ronacher Daniel Roseman Brian Rosner + Rozza Oliver Rutherfurd ryankanno Manuel Saelices diff --git a/django/contrib/admin/sites.py b/django/contrib/admin/sites.py index 26e935e7fb..a83d8ec4d9 100644 --- a/django/contrib/admin/sites.py +++ b/django/contrib/admin/sites.py @@ -269,7 +269,7 @@ class AdminSite(object): return self.root(request, request.path.split(self.root_path)[-1]) else: request.session.delete_test_cookie() - return http.HttpResponseRedirect(request.path) + return http.HttpResponseRedirect(request.get_full_path()) else: return self.display_login_form(request, ERROR_MESSAGE) login = never_cache(login) @@ -341,7 +341,7 @@ class AdminSite(object): context = { 'title': _('Log in'), - 'app_path': request.path, + 'app_path': request.get_full_path(), 'post_data': post_data, 'error_message': error_message, 'root_path': self.root_path, diff --git a/django/contrib/admin/views/decorators.py b/django/contrib/admin/views/decorators.py index cf0cd704c2..4b36e3ffa2 100644 --- a/django/contrib/admin/views/decorators.py +++ b/django/contrib/admin/views/decorators.py @@ -28,7 +28,7 @@ def _display_login_form(request, error_message=''): post_data = _encode_post_data({}) return render_to_response('admin/login.html', { 'title': _('Log in'), - 'app_path': request.path, + 'app_path': request.get_full_path(), 'post_data': post_data, 'error_message': error_message }, context_instance=template.RequestContext(request)) @@ -84,7 +84,7 @@ def staff_member_required(view_func): if '@' in username: # Mistakenly entered e-mail address instead of username? Look it up. users = list(User.objects.filter(email=username)) - if len(users) == 1: + if len(users) == 1 and users[0].check_password(password): message = _("Your e-mail address is not your username. Try '%s' instead.") % users[0].username else: # Either we cannot find the user, or if more than 1 @@ -106,7 +106,7 @@ def staff_member_required(view_func): return view_func(request, *args, **kwargs) else: request.session.delete_test_cookie() - return http.HttpResponseRedirect(request.path) + return http.HttpResponseRedirect(request.get_full_path()) else: return _display_login_form(request, ERROR_MESSAGE) diff --git a/tests/regressiontests/admin_views/tests.py b/tests/regressiontests/admin_views/tests.py index c7a8a2eeb8..cc91494bba 100644 --- a/tests/regressiontests/admin_views/tests.py +++ b/tests/regressiontests/admin_views/tests.py @@ -152,6 +152,13 @@ class AdminViewPermissionsTest(TestCase): # Login.context is a list of context dicts we just need to check the first one. self.assert_(login.context[0].get('error_message')) + def testLoginSuccessfullyRedirectsToOriginalUrl(self): + request = self.client.get('/test_admin/admin/') + self.failUnlessEqual(request.status_code, 200) + query_string = "the-answer=42" + login = self.client.post('/test_admin/admin/', self.super_login, QUERY_STRING = query_string ) + self.assertRedirects(login, '/test_admin/admin/?%s' % query_string) + def testAddView(self): """Test add view restricts access and actually adds items.""" @@ -363,3 +370,114 @@ class AdminViewStringPrimaryKeyTest(TestCase): response = self.client.get('/test_admin/admin/admin_views/modelwithstringprimarykey/%s/delete/' % quote(self.pk)) should_contain = """%s""" % (quote(self.pk), escape(self.pk)) self.assertContains(response, should_contain) + +class SecureViewTest(TestCase): + fixtures = ['admin-views-users.xml'] + + def setUp(self): + # login POST dicts + self.super_login = {'post_data': _encode_post_data({}), + LOGIN_FORM_KEY: 1, + 'username': 'super', + 'password': 'secret'} + self.super_email_login = {'post_data': _encode_post_data({}), + LOGIN_FORM_KEY: 1, + 'username': 'super@example.com', + 'password': 'secret'} + self.super_email_bad_login = {'post_data': _encode_post_data({}), + LOGIN_FORM_KEY: 1, + 'username': 'super@example.com', + 'password': 'notsecret'} + self.adduser_login = {'post_data': _encode_post_data({}), + LOGIN_FORM_KEY: 1, + 'username': 'adduser', + 'password': 'secret'} + self.changeuser_login = {'post_data': _encode_post_data({}), + LOGIN_FORM_KEY: 1, + 'username': 'changeuser', + 'password': 'secret'} + self.deleteuser_login = {'post_data': _encode_post_data({}), + LOGIN_FORM_KEY: 1, + 'username': 'deleteuser', + 'password': 'secret'} + self.joepublic_login = {'post_data': _encode_post_data({}), + LOGIN_FORM_KEY: 1, + 'username': 'joepublic', + 'password': 'secret'} + + def tearDown(self): + self.client.logout() + + def test_secure_view_shows_login_if_not_logged_in(self): + "Ensure that we see the login form" + response = self.client.get('/test_admin/admin/secure-view/' ) + self.assertTemplateUsed(response, 'admin/login.html') + + def test_secure_view_login_successfully_redirects_to_original_url(self): + request = self.client.get('/test_admin/admin/secure-view/') + self.failUnlessEqual(request.status_code, 200) + query_string = "the-answer=42" + login = self.client.post('/test_admin/admin/secure-view/', self.super_login, QUERY_STRING = query_string ) + self.assertRedirects(login, '/test_admin/admin/secure-view/?%s' % query_string) + + def test_staff_member_required_decorator_works_as_per_admin_login(self): + """ + Make sure only staff members can log in. + + Successful posts to the login page will redirect to the orignal url. + Unsuccessfull attempts will continue to render the login page with + a 200 status code. + """ + # Super User + request = self.client.get('/test_admin/admin/secure-view/') + self.failUnlessEqual(request.status_code, 200) + login = self.client.post('/test_admin/admin/secure-view/', self.super_login) + self.assertRedirects(login, '/test_admin/admin/secure-view/') + self.assertFalse(login.context) + self.client.get('/test_admin/admin/logout/') + + # Test if user enters e-mail address + request = self.client.get('/test_admin/admin/secure-view/') + self.failUnlessEqual(request.status_code, 200) + login = self.client.post('/test_admin/admin/secure-view/', self.super_email_login) + self.assertContains(login, "Your e-mail address is not your username") + # only correct passwords get a username hint + login = self.client.post('/test_admin/admin/secure-view/', self.super_email_bad_login) + self.assertContains(login, "Usernames cannot contain the '@' character") + new_user = User(username='jondoe', password='secret', email='super@example.com') + new_user.save() + # check to ensure if there are multiple e-mail addresses a user doesn't get a 500 + login = self.client.post('/test_admin/admin/secure-view/', self.super_email_login) + self.assertContains(login, "Usernames cannot contain the '@' character") + + # Add User + request = self.client.get('/test_admin/admin/secure-view/') + self.failUnlessEqual(request.status_code, 200) + login = self.client.post('/test_admin/admin/secure-view/', self.adduser_login) + self.assertRedirects(login, '/test_admin/admin/secure-view/') + self.assertFalse(login.context) + self.client.get('/test_admin/admin/logout/') + + # Change User + request = self.client.get('/test_admin/admin/secure-view/') + self.failUnlessEqual(request.status_code, 200) + login = self.client.post('/test_admin/admin/secure-view/', self.changeuser_login) + self.assertRedirects(login, '/test_admin/admin/secure-view/') + self.assertFalse(login.context) + self.client.get('/test_admin/admin/logout/') + + # Delete User + request = self.client.get('/test_admin/admin/secure-view/') + self.failUnlessEqual(request.status_code, 200) + login = self.client.post('/test_admin/admin/secure-view/', self.deleteuser_login) + self.assertRedirects(login, '/test_admin/admin/secure-view/') + self.assertFalse(login.context) + self.client.get('/test_admin/admin/logout/') + + # Regular User should not be able to login. + request = self.client.get('/test_admin/admin/secure-view/') + self.failUnlessEqual(request.status_code, 200) + login = self.client.post('/test_admin/admin/secure-view/', self.joepublic_login) + self.failUnlessEqual(login.status_code, 200) + # Login.context is a list of context dicts we just need to check the first one. + self.assert_(login.context[0].get('error_message')) diff --git a/tests/regressiontests/admin_views/urls.py b/tests/regressiontests/admin_views/urls.py index e556812a45..4e5da48e13 100644 --- a/tests/regressiontests/admin_views/urls.py +++ b/tests/regressiontests/admin_views/urls.py @@ -1,7 +1,9 @@ from django.conf.urls.defaults import * from django.contrib import admin +import views urlpatterns = patterns('', (r'^admin/doc/', include('django.contrib.admindocs.urls')), + (r'^admin/secure-view/$', views.secure_view), (r'^admin/(.*)', admin.site.root), ) diff --git a/tests/regressiontests/admin_views/views.py b/tests/regressiontests/admin_views/views.py new file mode 100644 index 0000000000..f1c7889d56 --- /dev/null +++ b/tests/regressiontests/admin_views/views.py @@ -0,0 +1,6 @@ +from django.contrib.admin.views.decorators import staff_member_required +from django.http import HttpResponse + +def secure_view(request): + return HttpResponse('') +secure_view = staff_member_required(secure_view) \ No newline at end of file