From c94ac4266c89fa9968c4650ea1fd78417a671cfe Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Tue, 12 Jan 2016 11:25:46 +0000 Subject: [PATCH] Add confirmation of password for important changes This commit adds an extra page or field for confirming your current password when making important changes Name | Email address | Mobile number | Password ---------------------|-------------------|-------------------|------------ No password required | As second page | As second page | On same page as new password --- app/main/forms.py | 9 +++- app/main/views/user_profile.py | 43 ++++++++++++++++--- app/templates/views/signin.html | 2 +- .../views/user-profile/authenticate.html | 26 +++++++++++ .../views/user-profile/change-password.html | 27 ++++++++++++ tests/app/main/views/test_user_profile.py | 32 +++++++++++++- 6 files changed, 129 insertions(+), 10 deletions(-) create mode 100644 app/templates/views/user-profile/authenticate.html create mode 100644 app/templates/views/user-profile/change-password.html diff --git a/app/main/forms.py b/app/main/forms.py index 0fbae1bbb..5ad1d04b5 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -31,8 +31,8 @@ def mobile_number(): Regexp(regex=mobile_number_regex, message='Enter a +44 mobile number')]) -def password(): - return PasswordField('Create a password', +def password(label='Create a password'): + return PasswordField(label, validators=[DataRequired(message='Password can not be empty'), Length(10, 255, message='Password must be at least 10 characters'), Blacklist(message='That password is blacklisted, too common')]) @@ -152,6 +152,11 @@ class NewPasswordForm(Form): new_password = password() +class ChangePasswordForm(Form): + old_password = password('Current password') + new_password = password('New password') + + class CsvUploadForm(Form): file = FileField('File to upload', validators=[DataRequired( message='Please pick a file'), CsvFileValidator()]) diff --git a/app/main/views/user_profile.py b/app/main/views/user_profile.py index 19c56b10f..1b8db76cb 100644 --- a/app/main/views/user_profile.py +++ b/app/main/views/user_profile.py @@ -2,8 +2,8 @@ from flask import request, render_template, redirect, url_for from flask.ext.login import current_user from app.main import main from app.main.forms import ( - NewPasswordForm, ChangeNameForm, ChangeEmailForm, ConfirmEmailForm, - ChangeMobileNumberForm, ConfirmMobileNumberForm + ChangePasswordForm, ChangeNameForm, ChangeEmailForm, ConfirmEmailForm, + ChangeMobileNumberForm, ConfirmMobileNumberForm, ConfirmPasswordForm ) @@ -42,6 +42,22 @@ def userprofile_email(): thing='email address', form_field=form.email_address ) + elif request.method == 'POST': + return redirect(url_for('.userprofile_email_authenticate')) + + +@main.route("/user-profile/email/authenticate", methods=['GET', 'POST']) +def userprofile_email_authenticate(): + + form = ConfirmPasswordForm() + + if request.method == 'GET': + return render_template( + 'views/user-profile/authenticate.html', + thing='email address', + form=form, + back_link=url_for('.userprofile_email') + ) elif request.method == 'POST': return redirect(url_for('.userprofile_email_confirm')) @@ -74,6 +90,22 @@ def userprofile_mobile_number(): thing='mobile number', form_field=form.mobile_number ) + elif request.method == 'POST': + return redirect(url_for('.userprofile_mobile_number_authenticate')) + + +@main.route("/user-profile/mobile-number/authenticate", methods=['GET', 'POST']) +def userprofile_mobile_number_authenticate(): + + form = ConfirmPasswordForm() + + if request.method == 'GET': + return render_template( + 'views/user-profile/authenticate.html', + thing='mobile number', + form=form, + back_link=url_for('.userprofile_mobile_number_confirm') + ) elif request.method == 'POST': return redirect(url_for('.userprofile_mobile_number_confirm')) @@ -96,13 +128,12 @@ def userprofile_mobile_number_confirm(): @main.route("/user-profile/password", methods=['GET', 'POST']) def userprofile_password(): - form = NewPasswordForm() + form = ChangePasswordForm() if request.method == 'GET': return render_template( - 'views/user-profile/change.html', - thing='password', - form_field=form.new_password + 'views/user-profile/change-password.html', + form=form ) elif request.method == 'POST': return redirect(url_for('.userprofile')) diff --git a/app/templates/views/signin.html b/app/templates/views/signin.html index fe6210b0b..add765160 100644 --- a/app/templates/views/signin.html +++ b/app/templates/views/signin.html @@ -17,7 +17,7 @@ Sign in
{{ textbox(form.email_address) }} {{ textbox(form.password) }} - {{ page_footer("Continue", back_link="#", back_link_text="Forgotten password?") }} + {{ page_footer("Continue", back_link=url_for('.forgot_password'), back_link_text="Forgotten password?") }}
diff --git a/app/templates/views/user-profile/authenticate.html b/app/templates/views/user-profile/authenticate.html new file mode 100644 index 000000000..8587bcc85 --- /dev/null +++ b/app/templates/views/user-profile/authenticate.html @@ -0,0 +1,26 @@ +{% extends "withnav_template.html" %} +{% from "components/textbox.html" import textbox %} +{% from "components/page-footer.html" import page_footer %} + +{% block page_title %} +GOV.UK Notify | Service settings +{% endblock %} + +{% block maincolumn_content %} + +

Change your {{ thing }}

+ +
+
+ +
+ {{ textbox(form.password) }} + {{ page_footer( + 'Confirm', + back_link=back_link + ) }} +
+
+
+ +{% endblock %} diff --git a/app/templates/views/user-profile/change-password.html b/app/templates/views/user-profile/change-password.html new file mode 100644 index 000000000..108258bfb --- /dev/null +++ b/app/templates/views/user-profile/change-password.html @@ -0,0 +1,27 @@ +{% extends "withnav_template.html" %} +{% from "components/textbox.html" import textbox %} +{% from "components/page-footer.html" import page_footer %} + +{% block page_title %} +GOV.UK Notify | Service settings +{% endblock %} + +{% block maincolumn_content %} + +

Change your password

+ +
+
+
+ {{ textbox(form.old_password) }} + {{ textbox(form.new_password) }} + {{ page_footer( + 'Save', + back_link=url_for('.userprofile'), + back_link_text="Back to your profile" + ) }} +
+
+
+ +{% endblock %} diff --git a/tests/app/main/views/test_user_profile.py b/tests/app/main/views/test_user_profile.py index a292c271f..ae052cdd0 100644 --- a/tests/app/main/views/test_user_profile.py +++ b/tests/app/main/views/test_user_profile.py @@ -29,6 +29,21 @@ def test_should_show_email_page(notifications_admin): def test_should_redirect_after_email_change(notifications_admin): response = notifications_admin.test_client().post('/user-profile/email') + assert response.status_code == 302 + assert response.location == 'http://localhost/user-profile/email/authenticate' + + +def test_should_show_authenticate_after_email_change(notifications_admin): + response = notifications_admin.test_client().get('/user-profile/email/authenticate') + + assert 'Change your email address' in response.get_data(as_text=True) + assert 'Confirm' in response.get_data(as_text=True) + assert response.status_code == 200 + + +def test_should_redirect_after_email_change_confirm(notifications_admin): + response = notifications_admin.test_client().post('/user-profile/email/authenticate') + assert response.status_code == 302 assert response.location == 'http://localhost/user-profile/email/confirm' @@ -59,7 +74,22 @@ def test_should_redirect_after_mobile_number_change(notifications_admin): response = notifications_admin.test_client().post('/user-profile/email') assert response.status_code == 302 - assert response.location == 'http://localhost/user-profile/email/confirm' + assert response.location == 'http://localhost/user-profile/email/authenticate' + + +def test_should_show_authenticate_after_mobile_number_change(notifications_admin): + response = notifications_admin.test_client().get('/user-profile/mobile-number/authenticate') + + assert 'Change your mobile number' in response.get_data(as_text=True) + assert 'Confirm' in response.get_data(as_text=True) + assert response.status_code == 200 + + +def test_should_redirect_after_mobile_number_authenticate(notifications_admin): + response = notifications_admin.test_client().post('/user-profile/mobile-number/authenticate') + + assert response.status_code == 302 + assert response.location == 'http://localhost/user-profile/mobile-number/confirm' def test_should_show_confirm_after_mobile_number_change(notifications_admin):