From 313311d0087bcb640060f369c8e52f4be4e726a6 Mon Sep 17 00:00:00 2001 From: Roberto Rosario Date: Wed, 17 Apr 2019 02:00:28 -0400 Subject: [PATCH] Use Django's new class based authentication views Signed-off-by: Roberto Rosario --- HISTORY.rst | 2 + docs/releases/3.2.rst | 2 + mayan/apps/authentication/tests/test_views.py | 78 ++++--- mayan/apps/authentication/urls.py | 45 ++-- mayan/apps/authentication/views.py | 197 ++++++++---------- 5 files changed, 156 insertions(+), 168 deletions(-) diff --git a/HISTORY.rst b/HISTORY.rst index 22642f8946..33455a562d 100644 --- a/HISTORY.rst +++ b/HISTORY.rst @@ -87,6 +87,8 @@ * Add keyword arguments to messages uses. * Add keyword arguments to the reverse use in views. * Add MERCs 5 and 6. +* Update authentication function views to use Django's new class + based authentication views. 3.1.11 (2019-04-XX) =================== diff --git a/docs/releases/3.2.rst b/docs/releases/3.2.rst index 7ebec03ef3..86a0b7abea 100644 --- a/docs/releases/3.2.rst +++ b/docs/releases/3.2.rst @@ -119,6 +119,8 @@ Other changes * Add keyword arguments to messages uses. * Add keyword arguments to the reverse use in views. * Add MERCs 5 and 6. +* Update authentication function views to use Django's new class + based authentication views. Removals -------- diff --git a/mayan/apps/authentication/tests/test_views.py b/mayan/apps/authentication/tests/test_views.py index 3cafefc98e..34540e30e3 100644 --- a/mayan/apps/authentication/tests/test_views.py +++ b/mayan/apps/authentication/tests/test_views.py @@ -1,15 +1,17 @@ from __future__ import absolute_import, unicode_literals +from furl import furl + from django.conf import settings from django.core import mail from django.test import override_settings from django.urls import reverse +from django.utils.http import urlunquote_plus from mayan.apps.common.tests import GenericViewTestCase from mayan.apps.smart_settings.classes import Namespace from mayan.apps.user_management.tests.literals import ( - TEST_ADMIN_EMAIL, TEST_ADMIN_PASSWORD, TEST_USER_PASSWORD_EDITED, - TEST_ADMIN_USERNAME + TEST_ADMIN_EMAIL, TEST_ADMIN_PASSWORD, TEST_ADMIN_USERNAME ) from ..settings import setting_maximum_session_length @@ -21,70 +23,83 @@ class UserLoginTestCase(GenericViewTestCase): """ Test that users can login via the supported authentication methods """ + authenticated_url = reverse(viewname='common:home') + # Unquote directly until furl 2.1.0 is released which will include + # the tostr() argument query_dont_quote=True + # TODO: Remove after release and update to furl 2.1.0 + authentication_url = urlunquote_plus( + furl( + path=reverse(settings.LOGIN_URL), args={ + 'next': authenticated_url + } + ).tostr() + ) + auto_login_user = False + def setUp(self): super(UserLoginTestCase, self).setUp() Namespace.invalidate_cache_all() + def _request_authenticated_view(self): + return self.get(path=self.authenticated_url) + @override_settings(AUTHENTICATION_LOGIN_METHOD='username') - def test_normal_behavior(self): - response = self.get(viewname='documents:document_list') + def test_non_authenticated_request(self): + response = self._request_authenticated_view() self.assertRedirects( - response, - 'http://testserver/authentication/login/?next=/documents/list/' + response=response, expected_url=self.authentication_url ) @override_settings(AUTHENTICATION_LOGIN_METHOD='username') def test_username_login(self): - logged_in = self.client.login( + logged_in = self.login( username=TEST_ADMIN_USERNAME, password=TEST_ADMIN_PASSWORD ) self.assertTrue(logged_in) - response = self.get(viewname='documents:document_list') + response = self._request_authenticated_view() # We didn't get redirected to the login URL self.assertEqual(response.status_code, 200) @override_settings(AUTHENTICATION_LOGIN_METHOD='email') def test_email_login(self): with self.settings(AUTHENTICATION_BACKENDS=(TEST_EMAIL_AUTHENTICATION_BACKEND,)): - logged_in = self.client.login( + logged_in = self.login( username=TEST_ADMIN_USERNAME, password=TEST_ADMIN_PASSWORD ) self.assertFalse(logged_in) - logged_in = self.client.login( + logged_in = self.login( email=TEST_ADMIN_EMAIL, password=TEST_ADMIN_PASSWORD ) self.assertTrue(logged_in) - response = self.get(viewname='documents:document_list') + response = self._request_authenticated_view() # We didn't get redirected to the login URL self.assertEqual(response.status_code, 200) @override_settings(AUTHENTICATION_LOGIN_METHOD='username') def test_username_login_via_views(self): - response = self.get(viewname='documents:document_list') + response = self._request_authenticated_view() self.assertRedirects( - response, - 'http://testserver/authentication/login/?next=/documents/list/' + response=response, expected_url=self.authentication_url ) response = self.post( - data={ + viewname=settings.LOGIN_URL, data={ 'username': TEST_ADMIN_USERNAME, 'password': TEST_ADMIN_PASSWORD - }, viewname=settings.LOGIN_URL + } ) - response = self.get(viewname='documents:document_list') + response = self._request_authenticated_view() # We didn't get redirected to the login URL self.assertEqual(response.status_code, 200) @override_settings(AUTHENTICATION_LOGIN_METHOD='email') def test_email_login_via_views(self): with self.settings(AUTHENTICATION_BACKENDS=(TEST_EMAIL_AUTHENTICATION_BACKEND,)): - response = self.get(viewname='documents:document_list') + response = self._request_authenticated_view() self.assertRedirects( - response, - 'http://testserver/authentication/login/?next=/documents/list/' + response=response, expected_url=self.authentication_url ) response = self.post( @@ -94,7 +109,7 @@ class UserLoginTestCase(GenericViewTestCase): ) self.assertEqual(response.status_code, 200) - response = self.get(viewname='documents:document_list') + response = self._request_authenticated_view() # We didn't get redirected to the login URL self.assertEqual(response.status_code, 200) @@ -108,7 +123,7 @@ class UserLoginTestCase(GenericViewTestCase): }, follow=True ) - response = self.get(viewname='documents:document_list') + response = self._request_authenticated_view() self.assertEqual(response.status_code, 200) self.assertEqual( @@ -127,7 +142,7 @@ class UserLoginTestCase(GenericViewTestCase): }, follow=True ) - response = self.get(viewname='documents:document_list') + response = self._request_authenticated_view() self.assertEqual(response.status_code, 200) self.assertTrue(self.client.session.get_expire_at_browser_close()) @@ -143,7 +158,7 @@ class UserLoginTestCase(GenericViewTestCase): }, follow=True ) - response = self.get(viewname='documents:document_list') + response = self._request_authenticated_view() self.assertEqual(response.status_code, 200) self.assertEqual( @@ -163,7 +178,7 @@ class UserLoginTestCase(GenericViewTestCase): } ) - response = self.get(viewname='documents:document_list') + response = self._request_authenticated_view() self.assertEqual(response.status_code, 200) self.assertTrue(self.client.session.get_expire_at_browser_close()) @@ -184,26 +199,27 @@ class UserLoginTestCase(GenericViewTestCase): response = self.post( viewname='authentication:password_reset_confirm_view', args=uid_token[-3:-1], data={ - 'new_password1': TEST_USER_PASSWORD_EDITED, - 'new_password2': TEST_USER_PASSWORD_EDITED, + 'new_password1': TEST_ADMIN_PASSWORD, + 'new_password2': TEST_ADMIN_PASSWORD, } ) self.assertEqual(response.status_code, 302) self.login( - username=TEST_ADMIN_USERNAME, password=TEST_USER_PASSWORD_EDITED + username=TEST_ADMIN_USERNAME, password=TEST_ADMIN_PASSWORD ) - response = self.get(viewname='documents:document_list') + response = self._request_authenticated_view() self.assertEqual(response.status_code, 200) def test_username_login_redirect(self): TEST_REDIRECT_URL = reverse(viewname='common:about_view') response = self.post( - viewname=settings.LOGIN_URL, query={'next': TEST_REDIRECT_URL}, - data={ + path='{}?next={}'.format( + reverse(settings.LOGIN_URL), TEST_REDIRECT_URL + ), data={ 'username': TEST_ADMIN_USERNAME, 'password': TEST_ADMIN_PASSWORD, 'remember_me': False diff --git a/mayan/apps/authentication/urls.py b/mayan/apps/authentication/urls.py index fc2faa5bf1..d48ac1ffb7 100644 --- a/mayan/apps/authentication/urls.py +++ b/mayan/apps/authentication/urls.py @@ -1,43 +1,44 @@ from __future__ import unicode_literals -from django.conf import settings from django.conf.urls import url -from django.contrib.auth.views import logout from .views import ( - login_view, password_change_done, password_change_view, - password_reset_complete_view, password_reset_confirm_view, - password_reset_done_view, password_reset_view + MayanLoginView, MayanLogoutView, MayanPasswordChangeDoneView, + MayanPasswordChangeView, MayanPasswordResetCompleteView, + MayanPasswordResetConfirmView, MayanPasswordResetDoneView, + MayanPasswordResetView ) urlpatterns = [ - url(regex=r'^login/$', view=login_view, name='login_view'), + url(regex=r'^login/$', view=MayanLoginView.as_view(), name='login_view'), url( - regex=r'^password/change/done/$', view=password_change_done, - name='password_change_done' + regex=r'^logout/$', view=MayanLogoutView.as_view(), name='logout_view' ), url( - regex=r'^password/change/$', view=password_change_view, + regex=r'^password/change/done/$', + view=MayanPasswordChangeDoneView.as_view(), name='password_change_done' + ), + url( + regex=r'^password/change/$', view=MayanPasswordChangeView.as_view(), name='password_change_view' ), url( - regex=r'^logout/$', view=logout, kwargs={'next_page': settings.LOGIN_REDIRECT_URL}, - name='logout_view' - ), - url( - regex=r'^password/reset/$', view=password_reset_view, name='password_reset_view' - ), - url( - regex=r'^password/reset/confirm/(?P[0-9A-Za-z_\-]+)/(?P[0-9A-Za-z]{1,13}-[0-9A-Za-z]{1,20})/$', - view=password_reset_confirm_view, name='password_reset_confirm_view' - ), - url( - regex=r'^password/reset/complete/$', view=password_reset_complete_view, + regex=r'^password/reset/complete/$', + view=MayanPasswordResetCompleteView.as_view(), name='password_reset_complete_view' ), url( - regex=r'^password/reset/done/$', view=password_reset_done_view, + regex=r'^password/reset/confirm/(?P[0-9A-Za-z_\-]+)/(?P[0-9A-Za-z]{1,13}-[0-9A-Za-z]{1,20})/$', + view=MayanPasswordResetConfirmView.as_view(), + name='password_reset_confirm_view' + ), + url( + regex=r'^password/reset/done/$', view=MayanPasswordResetDoneView.as_view(), name='password_reset_done_view' ), + url( + regex=r'^password/reset/$', view=MayanPasswordResetView.as_view(), + name='password_reset_view' + ) ] diff --git a/mayan/apps/authentication/views.py b/mayan/apps/authentication/views.py index acc3e15c5e..d6d7232d8f 100644 --- a/mayan/apps/authentication/views.py +++ b/mayan/apps/authentication/views.py @@ -1,19 +1,17 @@ from __future__ import absolute_import, unicode_literals -from django.conf import settings from django.contrib import messages -from django.contrib.auth import REDIRECT_FIELD_NAME from django.contrib.auth.views import ( - login, password_change, password_reset, password_reset_confirm, - password_reset_complete, password_reset_done + LoginView, LogoutView, PasswordChangeDoneView, PasswordChangeView, + PasswordResetCompleteView, PasswordResetConfirmView, PasswordResetDoneView, + PasswordResetView ) from django.http import HttpResponseRedirect -from django.shortcuts import redirect, resolve_url -from django.urls import reverse -from django.utils.http import is_safe_url +from django.shortcuts import redirect +from django.urls import reverse, reverse_lazy from django.utils.translation import ugettext_lazy as _ -from stronghold.decorators import public +from stronghold.views import StrongholdPublicMixin import mayan from mayan.apps.common.settings import ( @@ -24,143 +22,112 @@ from .forms import EmailAuthenticationForm, UsernameAuthenticationForm from .settings import setting_login_method, setting_maximum_session_length -@public -def login_view(request): +class MayanLoginView(StrongholdPublicMixin, LoginView): """ Control how the use is to be authenticated, options are 'email' and 'username' """ - success_url_allowed_hosts = set() - kwargs = {'template_name': 'authentication/login.html'} + extra_context = { + 'appearance_type': 'plain' + } + template_name = 'authentication/login.html' + redirect_authenticated_user = True - if setting_login_method.value == 'email': - kwargs['authentication_form'] = EmailAuthenticationForm - else: - kwargs['authentication_form'] = UsernameAuthenticationForm + def form_valid(self, form): + result = super(MayanLoginView, self).form_valid(form=form) + remember_me = form.cleaned_data.get('remember_me') - allowed_hosts = {request.get_host()} - allowed_hosts.update(success_url_allowed_hosts) + # remember_me values: + # True - long session + # False - short session + # None - Form has no remember_me value and we let the session + # expiration default. - redirect_to = request.POST.get( - REDIRECT_FIELD_NAME, request.GET.get(REDIRECT_FIELD_NAME, '') - ) - - url_is_safe = is_safe_url( - url=redirect_to, - allowed_hosts=allowed_hosts, - require_https=request.is_secure(), - ) - - url = redirect_to if url_is_safe else '' - - if not request.user.is_authenticated: - extra_context = { - 'appearance_type': 'plain', - REDIRECT_FIELD_NAME: url or resolve_url(settings.LOGIN_REDIRECT_URL) - } - - result = login(request, extra_context=extra_context, **kwargs) - if request.method == 'POST': - form = kwargs['authentication_form'](request, data=request.POST) - if form.is_valid(): - if form.cleaned_data['remember_me']: - request.session.set_expiry( - setting_maximum_session_length.value - ) - else: - request.session.set_expiry(0) - return result - else: - return HttpResponseRedirect(resolve_url(settings.LOGIN_REDIRECT_URL)) - - -def password_change_view(request): - """ - Password change wrapper for better control - """ - extra_context = {'title': _('Current user password change')} - - if request.user.user_options.block_password_change: - messages.error( - request, _( - 'Changing the password is not allowed for this account.' + if remember_me is True: + self.request.session.set_expiry( + setting_maximum_session_length.value ) + elif remember_me is False: + self.request.session.set_expiry(0) + + return result + + def get_form_class(self): + if setting_login_method.value == 'email': + return EmailAuthenticationForm + else: + return UsernameAuthenticationForm + + +class MayanLogoutView(LogoutView): + """No current change or overrides, left here for future expansion""" + + +class MayanPasswordChangeDoneView(PasswordChangeDoneView): + def dispatch(self, *args, **kwargs): + messages.success( + message=_('Your password has been successfully changed.'), + request=self.request ) - return HttpResponseRedirect(reverse(setting_home_view.view)) - - return password_change( - request, extra_context=extra_context, - template_name='appearance/generic_form.html', - post_change_redirect=reverse('authentication:password_change_done'), - ) + return redirect(to='common:current_user_details') -def password_change_done(request): - """ - View called when the new user password has been accepted - """ - messages.success( - request, _('Your password has been successfully changed.') - ) - return redirect('common:current_user_details') +class MayanPasswordChangeView(PasswordChangeView): + extra_context = {'title': _('Current user password change')} + success_url = reverse_lazy(viewname='authentication:password_change_done') + template_name = 'appearance/generic_form.html' + + def dispatch(self, *args, **kwargs): + if self.request.user.user_options.block_password_change: + messages.error( + message=_( + 'Changing the password is not allowed for this account.' + ), request=self.request + ) + return HttpResponseRedirect( + redirect_to=reverse(viewname=setting_home_view.view) + ) + + return super(MayanPasswordChangeView, self).dispatch(*args, **kwargs) -@public -def password_reset_complete_view(request): +class MayanPasswordResetCompleteView(StrongholdPublicMixin, PasswordResetCompleteView): extra_context = { 'appearance_type': 'plain' } - - return password_reset_complete( - request, extra_context=extra_context, - template_name='authentication/password_reset_complete.html' - ) + template_name = 'authentication/password_reset_complete.html' -@public -def password_reset_confirm_view(request, uidb64=None, token=None): +class MayanPasswordResetConfirmView(StrongholdPublicMixin, PasswordResetConfirmView): extra_context = { 'appearance_type': 'plain' } - - return password_reset_confirm( - request, extra_context=extra_context, - template_name='authentication/password_reset_confirm.html', - post_reset_redirect=reverse( - 'authentication:password_reset_complete_view' - ), uidb64=uidb64, token=token + success_url = reverse_lazy( + viewname='authentication:password_reset_complete_view' ) + template_name = 'authentication/password_reset_confirm.html' -@public -def password_reset_done_view(request): +class MayanPasswordResetDoneView(StrongholdPublicMixin, PasswordResetDoneView): extra_context = { 'appearance_type': 'plain' } - - return password_reset_done( - request, extra_context=extra_context, - template_name='authentication/password_reset_done.html' - ) + template_name = 'authentication/password_reset_done.html' -@public -def password_reset_view(request): +class MayanPasswordResetView(StrongholdPublicMixin, PasswordResetView): + email_template_name = 'authentication/password_reset_email.html' extra_context = { 'appearance_type': 'plain' } - - return password_reset( - request, extra_context=extra_context, - email_template_name='authentication/password_reset_email.html', - extra_email_context={ - 'project_title': setting_project_title.value, - 'project_website': setting_project_url.value, - 'project_copyright': mayan.__copyright__, - 'project_license': mayan.__license__, - }, subject_template_name='authentication/password_reset_subject.txt', - template_name='authentication/password_reset_form.html', - post_reset_redirect=reverse( - 'authentication:password_reset_done_view' - ) + extra_email_context = { + 'project_copyright': mayan.__copyright__, + 'project_license': mayan.__license__, + 'project_title': setting_project_title.value, + 'project_website': setting_project_url.value + } + subject_template_name = 'authentication/password_reset_subject.txt' + success_url = reverse_lazy( + viewname='authentication:password_reset_done_view' ) + template_name = 'authentication/password_reset_form.html'