Skip to content

Commit

Permalink
Fixed #5801: admin requests with GET args now get properly bounced th…
Browse files Browse the repository at this point in the history
…rough login with those args intact. Thanks for the patch, Rozza.

git-svn-id: http://code.djangoproject.com/svn/django/trunk@8271 bcc190cf-cafb-0310-a4f2-bffc1f526a37
  • Loading branch information
jacobian committed Aug 9, 2008
1 parent 400a6b2 commit 4747347
Show file tree
Hide file tree
Showing 6 changed files with 132 additions and 5 deletions.
1 change: 1 addition & 0 deletions AUTHORS
Expand Up @@ -335,6 +335,7 @@ answer newbie questions, and generally made Django that much better:
Armin Ronacher Armin Ronacher
Daniel Roseman <http://roseman.org.uk/> Daniel Roseman <http://roseman.org.uk/>
Brian Rosner <brosner@gmail.com> Brian Rosner <brosner@gmail.com>
Rozza <ross.lawley@gmail.com>
Oliver Rutherfurd <http://rutherfurd.net/> Oliver Rutherfurd <http://rutherfurd.net/>
ryankanno ryankanno
Manuel Saelices <msaelices@yaco.es> Manuel Saelices <msaelices@yaco.es>
Expand Down
4 changes: 2 additions & 2 deletions django/contrib/admin/sites.py
Expand Up @@ -269,7 +269,7 @@ def login(self, request):
return self.root(request, request.path.split(self.root_path)[-1]) return self.root(request, request.path.split(self.root_path)[-1])
else: else:
request.session.delete_test_cookie() request.session.delete_test_cookie()
return http.HttpResponseRedirect(request.path) return http.HttpResponseRedirect(request.get_full_path())
else: else:
return self.display_login_form(request, ERROR_MESSAGE) return self.display_login_form(request, ERROR_MESSAGE)
login = never_cache(login) login = never_cache(login)
Expand Down Expand Up @@ -341,7 +341,7 @@ def display_login_form(self, request, error_message='', extra_context=None):


context = { context = {
'title': _('Log in'), 'title': _('Log in'),
'app_path': request.path, 'app_path': request.get_full_path(),
'post_data': post_data, 'post_data': post_data,
'error_message': error_message, 'error_message': error_message,
'root_path': self.root_path, 'root_path': self.root_path,
Expand Down
6 changes: 3 additions & 3 deletions django/contrib/admin/views/decorators.py
Expand Up @@ -28,7 +28,7 @@ def _display_login_form(request, error_message=''):
post_data = _encode_post_data({}) post_data = _encode_post_data({})
return render_to_response('admin/login.html', { return render_to_response('admin/login.html', {
'title': _('Log in'), 'title': _('Log in'),
'app_path': request.path, 'app_path': request.get_full_path(),
'post_data': post_data, 'post_data': post_data,
'error_message': error_message 'error_message': error_message
}, context_instance=template.RequestContext(request)) }, context_instance=template.RequestContext(request))
Expand Down Expand Up @@ -84,7 +84,7 @@ def _checklogin(request, *args, **kwargs):
if '@' in username: if '@' in username:
# Mistakenly entered e-mail address instead of username? Look it up. # Mistakenly entered e-mail address instead of username? Look it up.
users = list(User.objects.filter(email=username)) 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 message = _("Your e-mail address is not your username. Try '%s' instead.") % users[0].username
else: else:
# Either we cannot find the user, or if more than 1 # Either we cannot find the user, or if more than 1
Expand All @@ -106,7 +106,7 @@ def _checklogin(request, *args, **kwargs):
return view_func(request, *args, **kwargs) return view_func(request, *args, **kwargs)
else: else:
request.session.delete_test_cookie() request.session.delete_test_cookie()
return http.HttpResponseRedirect(request.path) return http.HttpResponseRedirect(request.get_full_path())
else: else:
return _display_login_form(request, ERROR_MESSAGE) return _display_login_form(request, ERROR_MESSAGE)


Expand Down
118 changes: 118 additions & 0 deletions tests/regressiontests/admin_views/tests.py
Expand Up @@ -152,6 +152,13 @@ def testLogin(self):
# Login.context is a list of context dicts we just need to check the first one. # Login.context is a list of context dicts we just need to check the first one.
self.assert_(login.context[0].get('error_message')) 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): def testAddView(self):
"""Test add view restricts access and actually adds items.""" """Test add view restricts access and actually adds items."""


Expand Down Expand Up @@ -363,3 +370,114 @@ def test_deleteconfirmation_link(self):
response = self.client.get('/test_admin/admin/admin_views/modelwithstringprimarykey/%s/delete/' % quote(self.pk)) response = self.client.get('/test_admin/admin/admin_views/modelwithstringprimarykey/%s/delete/' % quote(self.pk))
should_contain = """<a href="../../%s/">%s</a>""" % (quote(self.pk), escape(self.pk)) should_contain = """<a href="../../%s/">%s</a>""" % (quote(self.pk), escape(self.pk))
self.assertContains(response, should_contain) 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 &#39;@&#39; 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 &#39;@&#39; 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'))
2 changes: 2 additions & 0 deletions tests/regressiontests/admin_views/urls.py
@@ -1,7 +1,9 @@
from django.conf.urls.defaults import * from django.conf.urls.defaults import *
from django.contrib import admin from django.contrib import admin
import views


urlpatterns = patterns('', urlpatterns = patterns('',
(r'^admin/doc/', include('django.contrib.admindocs.urls')), (r'^admin/doc/', include('django.contrib.admindocs.urls')),
(r'^admin/secure-view/$', views.secure_view),
(r'^admin/(.*)', admin.site.root), (r'^admin/(.*)', admin.site.root),
) )
6 changes: 6 additions & 0 deletions 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)

0 comments on commit 4747347

Please sign in to comment.