From 8fa0bc07df85f4bcc1cfcc70c29d179a7cef147b Mon Sep 17 00:00:00 2001 From: Karl Hobley Date: Mon, 2 Jun 2014 13:27:39 +0100 Subject: [PATCH 1/3] Made login view redirect already logged in users to dashboard. Fixes #25 --- .../account/password_reset/complete.html | 2 +- .../templates/wagtailadmin/login.html | 2 +- .../tests/test_account_management.py | 1 - wagtail/wagtailadmin/urls.py | 10 ++------- wagtail/wagtailadmin/views/account.py | 22 ++++++++++++++++++- 5 files changed, 25 insertions(+), 12 deletions(-) diff --git a/wagtail/wagtailadmin/templates/wagtailadmin/account/password_reset/complete.html b/wagtail/wagtailadmin/templates/wagtailadmin/account/password_reset/complete.html index c0cf872e25..3dc8272b07 100644 --- a/wagtail/wagtailadmin/templates/wagtailadmin/account/password_reset/complete.html +++ b/wagtail/wagtailadmin/templates/wagtailadmin/account/password_reset/complete.html @@ -13,6 +13,6 @@ {% block furniture %}

{% trans "Password change successful" %}

-

{% trans "Login" %}

+

{% trans "Login" %}

{% endblock %} \ No newline at end of file diff --git a/wagtail/wagtailadmin/templates/wagtailadmin/login.html b/wagtail/wagtailadmin/templates/wagtailadmin/login.html index 012dd32f89..22682500f7 100644 --- a/wagtail/wagtailadmin/templates/wagtailadmin/login.html +++ b/wagtail/wagtailadmin/templates/wagtailadmin/login.html @@ -20,7 +20,7 @@ {% endif %} -
+ {% csrf_token %}

{% trans "Sign in to Wagtail" %}

diff --git a/wagtail/wagtailadmin/tests/test_account_management.py b/wagtail/wagtailadmin/tests/test_account_management.py index 95d54d7d93..63c70779b2 100644 --- a/wagtail/wagtailadmin/tests/test_account_management.py +++ b/wagtail/wagtailadmin/tests/test_account_management.py @@ -49,7 +49,6 @@ class TestAuthentication(TestCase): self.assertTrue('_auth_user_id' in self.client.session) self.assertEqual(self.client.session['_auth_user_id'], User.objects.get(username='test').id) - @unittest.expectedFailure # See: https://github.com/torchbox/wagtail/issues/25 def test_already_logged_in_redirect(self): """ This tests that a user who is already logged in is automatically diff --git a/wagtail/wagtailadmin/urls.py b/wagtail/wagtailadmin/urls.py index 8fbf2f6e77..806240c5c5 100644 --- a/wagtail/wagtailadmin/urls.py +++ b/wagtail/wagtailadmin/urls.py @@ -5,15 +5,8 @@ from wagtail.wagtailadmin.forms import LoginForm, PasswordResetForm from wagtail.wagtailadmin.views import account, chooser, home, pages, tags, userbar from wagtail.wagtailadmin import hooks -urlpatterns = [ - url( - r'^login/$', 'django.contrib.auth.views.login', { - 'template_name': 'wagtailadmin/login.html', - 'authentication_form': LoginForm, - 'extra_context': {'show_password_reset': getattr(settings, 'WAGTAIL_PASSWORD_MANAGEMENT_ENABLED', True)}, - }, name='wagtailadmin_login' - ), +urlpatterns = [ # Password reset url( r'^password_reset/$', 'django.contrib.auth.views.password_reset', { @@ -81,6 +74,7 @@ urlpatterns += [ url(r'^tag-autocomplete/$', tags.autocomplete, name='wagtailadmin_tag_autocomplete'), + url(r'^login/$', account.login, name='wagtailadmin_login'), url(r'^account/$', account.account, name='wagtailadmin_account'), url(r'^account/change_password/$', account.change_password, name='wagtailadmin_account_change_password'), url(r'^logout/$', account.logout, name='wagtailadmin_logout'), diff --git a/wagtail/wagtailadmin/views/account.py b/wagtail/wagtailadmin/views/account.py index 8479ea6b0e..c5e461f55c 100644 --- a/wagtail/wagtailadmin/views/account.py +++ b/wagtail/wagtailadmin/views/account.py @@ -3,8 +3,13 @@ from django.shortcuts import render, redirect from django.contrib import messages from django.contrib.auth.forms import SetPasswordForm from django.contrib.auth.decorators import permission_required -from django.contrib.auth.views import logout as auth_logout +from django.contrib.auth.views import logout as auth_logout, login as auth_login from django.utils.translation import ugettext as _ +from django.views.decorators.debug import sensitive_post_parameters +from django.views.decorators.cache import never_cache + +from wagtail.wagtailadmin import forms + @permission_required('wagtailadmin.access_admin') def account(request): @@ -37,6 +42,21 @@ def change_password(request): }) +@sensitive_post_parameters() +@never_cache +def login(request): + if request.user.is_authenticated(): + return redirect('wagtailadmin_home') + else: + return auth_login(request, + template_name='wagtailadmin/login.html', + authentication_form=forms.LoginForm, + extra_context={ + 'show_password_reset': getattr(settings, 'WAGTAIL_PASSWORD_MANAGEMENT_ENABLED', True), + }, + ) + + def logout(request): response = auth_logout(request, next_page = 'wagtailadmin_login') From b35f6d9409e4327d955f0d4b3569f25accdc4a8a Mon Sep 17 00:00:00 2001 From: Karl Hobley Date: Fri, 6 Jun 2014 12:52:00 +0100 Subject: [PATCH 2/3] Added test to check that wagtailadmin behaves correctly when LOGIN_URL is not set --- runtests.py | 1 + .../tests/test_account_management.py | 37 ++++++++++++++++++- 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/runtests.py b/runtests.py index 346729653f..cc9c6e6586 100755 --- a/runtests.py +++ b/runtests.py @@ -103,6 +103,7 @@ if not settings.configured: WAGTAILSEARCH_BACKENDS=WAGTAILSEARCH_BACKENDS, WAGTAIL_SITE_NAME='Test Site', LOGIN_REDIRECT_URL = 'wagtailadmin_home', + LOGIN_URL='wagtailadmin_login', ) diff --git a/wagtail/wagtailadmin/tests/test_account_management.py b/wagtail/wagtailadmin/tests/test_account_management.py index 0a864aaccc..932ac71a8e 100644 --- a/wagtail/wagtailadmin/tests/test_account_management.py +++ b/wagtail/wagtailadmin/tests/test_account_management.py @@ -67,7 +67,7 @@ class TestAuthentication(TestCase, WagtailTestUtils): """ This tests that the user can logout """ - # Get logout page page + # Get logout page response = self.client.get(reverse('wagtailadmin_logout')) # Check that the user was redirected to the login page @@ -77,6 +77,41 @@ class TestAuthentication(TestCase, WagtailTestUtils): # Check that the user was logged out self.assertFalse('_auth_user_id' in self.client.session) + def test_not_logged_in_redirect(self): + """ + This tests that a not logged in user is redirected to the + login page + """ + # Logout + self.client.logout() + + # Get dashboard + response = self.client.get(reverse('wagtailadmin_home')) + + # Check that the user was redirected to the login page and that next was set correctly + self.assertEqual(response.status_code, 302) + self.assertURLEqual(response.url, reverse('wagtailadmin_login') + '?next=' + reverse('wagtailadmin_home')) + + @unittest.expectedFailure + def test_not_logged_in_redirect_default_settings(self): + """ + This does the same as the above test but checks that it + redirects to the correct place when the user has not set + the LOGIN_URL setting correctly + """ + # Logout + self.client.logout() + + # Get dashboard with default LOGIN_URL setting + with self.settings(LOGIN_URL='django.contrib.auth.views.login'): + response = self.client.get(reverse('wagtailadmin_home')) + + # Check that the user was redirected to the login page and that next was set correctly + # Note: The user will be redirected to 'django.contrib.auth.views.login' but + # this must be the same URL as 'wagtailadmin_login' + self.assertEqual(response.status_code, 302) + self.assertURLEqual(response.url, reverse('wagtailadmin_login') + '?next=' + reverse('wagtailadmin_home')) + class TestAccountSection(TestCase, WagtailTestUtils): """ From 45a10979c67dc35c7e64222192b8c0c9496993e5 Mon Sep 17 00:00:00 2001 From: Karl Hobley Date: Fri, 6 Jun 2014 12:59:07 +0100 Subject: [PATCH 3/3] Fixed issue with 'django.contrib.auth.views.login' not being reversed properly --- wagtail/wagtailadmin/tests/test_account_management.py | 1 - wagtail/wagtailadmin/urls.py | 7 +++++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/wagtail/wagtailadmin/tests/test_account_management.py b/wagtail/wagtailadmin/tests/test_account_management.py index 932ac71a8e..3553b8fba5 100644 --- a/wagtail/wagtailadmin/tests/test_account_management.py +++ b/wagtail/wagtailadmin/tests/test_account_management.py @@ -92,7 +92,6 @@ class TestAuthentication(TestCase, WagtailTestUtils): self.assertEqual(response.status_code, 302) self.assertURLEqual(response.url, reverse('wagtailadmin_login') + '?next=' + reverse('wagtailadmin_home')) - @unittest.expectedFailure def test_not_logged_in_redirect_default_settings(self): """ This does the same as the above test but checks that it diff --git a/wagtail/wagtailadmin/urls.py b/wagtail/wagtailadmin/urls.py index 806240c5c5..819caa92c8 100644 --- a/wagtail/wagtailadmin/urls.py +++ b/wagtail/wagtailadmin/urls.py @@ -84,6 +84,13 @@ urlpatterns += [ ] +# This is here to make sure that 'django.contrib.auth.views.login' is reversed correctly +# It must be placed after 'wagtailadmin_login' to prevent this from being used +urlpatterns += [ + url(r'^login/$', 'django.contrib.auth.views.login'), +] + + # Import additional urlpatterns from any apps that define a register_admin_urls hook for fn in hooks.get_hooks('register_admin_urls'): urls = fn()