From 6696426dbc8a1430b3a82a9236eddae4f8575561 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Mon, 4 Jan 2016 14:00:39 +0000 Subject: [PATCH 01/18] Add endpoints for forgot-password. --- app/main/__init__.py | 3 +-- app/main/dao/users_dao.py | 10 ++++++++ app/main/forms.py | 14 +++++++++--- app/main/views/__init__.py | 14 ++++++++++++ app/main/views/forgot_password.py | 21 +++++++++++++++++ app/main/views/index.py | 24 ++++++++++++++++++++ app/templates/views/forgot-password.html | 14 +++++++----- app/templates/views/password-reset-sent.html | 18 +++++++++++++++ tests/app/main/dao/test_users_dao.py | 22 ++++++++++++++++++ tests/app/main/views/test_forgot_password.py | 16 +++++++++++++ 10 files changed, 145 insertions(+), 11 deletions(-) create mode 100644 app/main/views/forgot_password.py create mode 100644 app/templates/views/password-reset-sent.html create mode 100644 tests/app/main/views/test_forgot_password.py diff --git a/app/main/__init__.py b/app/main/__init__.py index 9354e06db..2c62f8207 100644 --- a/app/main/__init__.py +++ b/app/main/__init__.py @@ -2,8 +2,7 @@ from flask import Blueprint main = Blueprint('main', __name__) - from app.main.views import ( index, sign_in, sign_out, register, two_factor, verify, sms, add_service, - code_not_received, jobs, dashboard, templates, service_settings + code_not_received, jobs, dashboard, templates, service_settings, forgot_password ) diff --git a/app/main/dao/users_dao.py b/app/main/dao/users_dao.py index ea065e9d6..f0237719a 100644 --- a/app/main/dao/users_dao.py +++ b/app/main/dao/users_dao.py @@ -1,3 +1,5 @@ +from datetime import datetime + from app import db, login_manager from app.models import User from app.main.encryption import hashpw @@ -53,3 +55,11 @@ def update_mobile_number(id, mobile_number): user.mobile_number = mobile_number db.session.add(user) db.session.commit() + + +def update_password(id, password): + user = get_user_by_id(id) + user.password = hashpw(password) + user.password_changed_at = datetime.now() + db.session.add(user) + db.session.commit() diff --git a/app/main/forms.py b/app/main/forms.py index 5b7230477..a9be2364d 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -1,10 +1,7 @@ -from datetime import datetime from flask_wtf import Form from wtforms import StringField, PasswordField, ValidationError from wtforms.validators import DataRequired, Email, Length, Regexp -from app.main.dao import verify_codes_dao -from app.main.encryption import check_hash from app.main.validators import Blacklist, ValidateUserCodes @@ -123,3 +120,14 @@ class AddServiceForm(Form): def validate_service_name(self, a): if self.service_name.data in self.service_names: raise ValidationError('Service name already exists') + + +class ForgotPassword(Form): + email_address = StringField('Email address', + validators=[Length(min=5, max=255), + DataRequired(message='Email cannot be empty'), + Email(message='Please enter a valid email address'), + Regexp(regex=gov_uk_email, message='Please enter a gov.uk email address') + ]) + + diff --git a/app/main/views/__init__.py b/app/main/views/__init__.py index e092ba59c..f145200ba 100644 --- a/app/main/views/__init__.py +++ b/app/main/views/__init__.py @@ -31,3 +31,17 @@ def send_email_code(user_id, email): raise AdminApiClientException('Exception when sending email.') return email_code + + +def send_change_password_email(email): + code = create_verify_code() + link_to_change_password = 'thelink' + code + # TODO needs an expiry date to check? + try: + admin_api_client.send_email(email_address=email, + from_str='notify@digital.cabinet-office.gov.uk', + message=link_to_change_password, + subject='Verification code', + token=admin_api_client.auth_token) + except: + raise AdminApiClientException('Exception when sending email.') diff --git a/app/main/views/forgot_password.py b/app/main/views/forgot_password.py new file mode 100644 index 000000000..ece51e59e --- /dev/null +++ b/app/main/views/forgot_password.py @@ -0,0 +1,21 @@ +from flask import render_template, jsonify + +from app.main import main +from app.main.forms import ForgotPassword +from app.main.views import send_change_password_email + + +@main.route('/forgot-password', methods=['GET']) +def render_forgot_my_password(): + return render_template('views/forgot-password.html', form=ForgotPassword()) + + +@main.route('/forgot-password', methods=['POST']) +def change_password(): + form = ForgotPassword() + if form.validate_on_submit(): + send_change_password_email(form.email_address) + + return 'You have been sent an email with a link to change your password' + else: + return jsonify(form.errors), 400 diff --git a/app/main/views/index.py b/app/main/views/index.py index dec87cf89..33d8f7e7d 100644 --- a/app/main/views/index.py +++ b/app/main/views/index.py @@ -39,6 +39,21 @@ def forgotpassword(): return render_template('views/forgot-password.html') +@main.route("/jobs") +def showjobs(): + return render_template('views/jobs.html') + + +@main.route("/jobs/job") +def showjob(): + return render_template('views/job.html') + + +@main.route("/jobs/job/notification") +def shownotification(): + return render_template('views/notification.html') + + @main.route("/new-password") def newpassword(): return render_template('views/new-password.html') @@ -62,3 +77,12 @@ def apikeys(): @main.route("/verification-not-received") def verificationnotreceived(): return render_template('views/verification-not-received.html') + +@main.route("/manage-templates") +def managetemplates(): + return render_template('views/manage-templates.html') + + +@main.route("/edit-template") +def edittemplate(): + return render_template('views/edit-template.html') diff --git a/app/templates/views/forgot-password.html b/app/templates/views/forgot-password.html index 5ca218fb4..6db3a81b9 100644 --- a/app/templates/views/forgot-password.html +++ b/app/templates/views/forgot-password.html @@ -12,15 +12,17 @@ GOV.UK Notify

If you have forgotten your password, we can send you an email to create a new password.

-

- - +

+ {{ form.hidden_tag() }} +

+ + {{ form.email_address(class="form-control-2-3", autocomplete="off") }}
+ Your email address must end in .gov.uk

- -

- Send email +

+
diff --git a/app/templates/views/password-reset-sent.html b/app/templates/views/password-reset-sent.html new file mode 100644 index 000000000..c51c206d3 --- /dev/null +++ b/app/templates/views/password-reset-sent.html @@ -0,0 +1,18 @@ +{% extends "admin_template.html" %} + +{% block page_title %} +GOV.UK Notify | +{% endblock %} + +{% block content %} + +
+
+

GOV.UK Notify

+ +

You have been sent an email containing a url to reset your password.

+ +
+
+ +{% endblock %} \ No newline at end of file diff --git a/tests/app/main/dao/test_users_dao.py b/tests/app/main/dao/test_users_dao.py index 75affe58d..ae6579c63 100644 --- a/tests/app/main/dao/test_users_dao.py +++ b/tests/app/main/dao/test_users_dao.py @@ -3,6 +3,7 @@ from datetime import datetime import pytest import sqlalchemy +from app.main.encryption import check_hash from app.models import User from app.main.dao import users_dao @@ -161,3 +162,24 @@ def test_should_update_email_address(notifications_admin, notifications_admin_db users_dao.update_email_address(user.id, 'new_email@testit.gov.uk') updated = users_dao.get_user_by_id(user.id) assert updated.email_address == 'new_email@testit.gov.uk' + + +def test_should_update_password(notifications_admin, notifications_admin_db, notify_db_session): + user = User(name='Update Email', + password='somepassword', + email_address='test@it.gov.uk', + mobile_number='+441234123412', + created_at=datetime.now(), + role_id=1, + state='active') + start = datetime.now() + users_dao.insert_user(user) + + saved = users_dao.get_user_by_id(user.id) + assert check_hash('somepassword', saved.password) + assert saved.password_changed_at is None + users_dao.update_password(saved.id, 'newpassword') + updated = users_dao.get_user_by_id(user.id) + assert check_hash('newpassword', updated.password) + assert updated.password_changed_at < datetime.now() + assert updated.password_changed_at > start diff --git a/tests/app/main/views/test_forgot_password.py b/tests/app/main/views/test_forgot_password.py new file mode 100644 index 000000000..95f0cce2a --- /dev/null +++ b/tests/app/main/views/test_forgot_password.py @@ -0,0 +1,16 @@ +from flask import current_app + + +def test_should_render_forgot_password(notifications_admin, notifications_admin_db, notify_db_session): + response = notifications_admin.test_client().get('/forgot-password') + assert response.status_code == 200 + assert 'If you have forgotten your password, we can send you an email to create a new password.' \ + in response.get_data(as_text=True) + + +def test_should_return_400_when_email_is_invalid(notifications_admin, notifications_admin_db, notify_db_session): + response = notifications_admin.test_client().post('/forgot-password', + data={'email_address': 'not_a_valid_email'}) + x = current_app._get_current_object() + assert response.status_code == 400 + assert 'Please enter a valid email address' in response.get_data(as_text=True) From 2cb896fa813bdec1d1fd6cd8824b85d5aeadfb73 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Tue, 5 Jan 2016 17:52:09 +0000 Subject: [PATCH 02/18] Completion of forgot-password endpoints. Start implementation for new-password endpoints. Created PasswordResetToken model ToDo: create and save token, send valid url to user, check validity of token, update user's password, redirect to /two-factor. --- app/main/__init__.py | 2 +- app/main/dao/password_reset_token_dao.py | 14 ++++++++ app/main/dao/users_dao.py | 6 ++++ app/main/forms.py | 10 +++++- app/main/views/__init__.py | 10 ++++-- app/main/views/forgot_password.py | 23 +++++------- app/main/views/new_password.py | 16 +++++++++ app/models.py | 7 ++++ app/templates/views/forgot-password.html | 6 +--- app/templates/views/signin.html | 2 +- .../versions/80_password_reset_token.py | 35 +++++++++++++++++++ .../app/main/dao/test_password_reset_token.py | 16 +++++++++ tests/app/main/dao/test_users_dao.py | 25 +++++++++++-- tests/app/main/test_forgot_password_form.py | 13 +++++++ tests/app/main/views/test_forgot_password.py | 35 +++++++++++++++---- 15 files changed, 187 insertions(+), 33 deletions(-) create mode 100644 app/main/dao/password_reset_token_dao.py create mode 100644 app/main/views/new_password.py create mode 100644 migrations/versions/80_password_reset_token.py create mode 100644 tests/app/main/dao/test_password_reset_token.py create mode 100644 tests/app/main/test_forgot_password_form.py diff --git a/app/main/__init__.py b/app/main/__init__.py index 2c62f8207..3d3ab5cde 100644 --- a/app/main/__init__.py +++ b/app/main/__init__.py @@ -4,5 +4,5 @@ main = Blueprint('main', __name__) from app.main.views import ( index, sign_in, sign_out, register, two_factor, verify, sms, add_service, - code_not_received, jobs, dashboard, templates, service_settings, forgot_password + code_not_received, jobs, dashboard, templates, service_settings, forgot_password, new_password ) diff --git a/app/main/dao/password_reset_token_dao.py b/app/main/dao/password_reset_token_dao.py new file mode 100644 index 000000000..f3f537aec --- /dev/null +++ b/app/main/dao/password_reset_token_dao.py @@ -0,0 +1,14 @@ +from datetime import datetime, timedelta + +from app import db +from app.models import PasswordResetToken + + +def insert(token): + token.expiry_date = datetime.now() + timedelta(hours=1) + db.session.add(token) + db.session.commit() + + +def get_token(token): + return PasswordResetToken.query.filter_by(token=token).first() \ No newline at end of file diff --git a/app/main/dao/users_dao.py b/app/main/dao/users_dao.py index f0237719a..8329d057d 100644 --- a/app/main/dao/users_dao.py +++ b/app/main/dao/users_dao.py @@ -1,5 +1,7 @@ from datetime import datetime +from sqlalchemy.orm import load_only + from app import db, login_manager from app.models import User from app.main.encryption import hashpw @@ -63,3 +65,7 @@ def update_password(id, password): user.password_changed_at = datetime.now() db.session.add(user) db.session.commit() + + +def find_all_email_address(): + return [x.email_address for x in User.query.options(load_only("email_address")).all()] diff --git a/app/main/forms.py b/app/main/forms.py index a9be2364d..eaf97ede2 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -122,7 +122,11 @@ class AddServiceForm(Form): raise ValidationError('Service name already exists') -class ForgotPassword(Form): +class ForgotPasswordForm(Form): + def __init__(self, email_addresses, *args, **kargs): + self.email_addresses = email_addresses + super(ForgotPasswordForm, self).__init__(*args, **kargs) + email_address = StringField('Email address', validators=[Length(min=5, max=255), DataRequired(message='Email cannot be empty'), @@ -130,4 +134,8 @@ class ForgotPassword(Form): Regexp(regex=gov_uk_email, message='Please enter a gov.uk email address') ]) + def validate_email_address(self, a): + if self.email_address.data not in self.email_addresses: + raise ValidationError('Please enter the email address that you registered with') + diff --git a/app/main/views/__init__.py b/app/main/views/__init__.py index f145200ba..b109d5465 100644 --- a/app/main/views/__init__.py +++ b/app/main/views/__init__.py @@ -1,4 +1,9 @@ +import traceback +import uuid from random import randint + +from flask import url_for + from app import admin_api_client from app.main.exceptions import AdminApiClientException from app.main.dao import verify_codes_dao @@ -34,14 +39,13 @@ def send_email_code(user_id, email): def send_change_password_email(email): - code = create_verify_code() - link_to_change_password = 'thelink' + code - # TODO needs an expiry date to check? try: + link_to_change_password = url_for('.new_password', token=str(uuid.uuid4())) admin_api_client.send_email(email_address=email, from_str='notify@digital.cabinet-office.gov.uk', message=link_to_change_password, subject='Verification code', token=admin_api_client.auth_token) except: + traceback.print_exc() raise AdminApiClientException('Exception when sending email.') diff --git a/app/main/views/forgot_password.py b/app/main/views/forgot_password.py index ece51e59e..8cc706dd9 100644 --- a/app/main/views/forgot_password.py +++ b/app/main/views/forgot_password.py @@ -1,21 +1,16 @@ -from flask import render_template, jsonify +from flask import render_template from app.main import main -from app.main.forms import ForgotPassword +from app.main.dao import users_dao +from app.main.forms import ForgotPasswordForm from app.main.views import send_change_password_email -@main.route('/forgot-password', methods=['GET']) -def render_forgot_my_password(): - return render_template('views/forgot-password.html', form=ForgotPassword()) - - -@main.route('/forgot-password', methods=['POST']) -def change_password(): - form = ForgotPassword() +@main.route('/forgot-password', methods=['GET', 'POST']) +def forgot_password(): + form = ForgotPasswordForm(users_dao.find_all_email_address()) if form.validate_on_submit(): - send_change_password_email(form.email_address) - - return 'You have been sent an email with a link to change your password' + send_change_password_email(form.email_address.data) + return render_template('views/password-reset-sent.html') else: - return jsonify(form.errors), 400 + return render_template('views/forgot-password.html', form=form) diff --git a/app/main/views/new_password.py b/app/main/views/new_password.py new file mode 100644 index 000000000..8a0ef77bb --- /dev/null +++ b/app/main/views/new_password.py @@ -0,0 +1,16 @@ +from flask import request + +from app.main import main + + +@main.route('/new-password/', methods=['GET', 'POST']) +def new_password(): + # Validate token + token = request.args.get('token') + # get password token (better name) + # is it expired + # add NewPasswordForm + # update password + # create password_token table (id, token, user_id, expiry_date + + return 'Got here' diff --git a/app/models.py b/app/models.py index 3608f040f..cff7d18a6 100644 --- a/app/models.py +++ b/app/models.py @@ -111,6 +111,13 @@ class Service(db.Model): return filter_null_value_fields(serialized) +class PasswordResetToken(db.Model): + id = db.Column(db.Integer, primary_key=True) + token = db.Column(db.String, unique=True, index=True, nullable=False) + user_id = db.Column(db.Integer, db.ForeignKey('users.id'), unique=False, nullable=False) + expiry_date = db.Column(db.DateTime, nullable=False) + + def filter_null_value_fields(obj): return dict( filter(lambda x: x[1] is not None, obj.items()) diff --git a/app/templates/views/forgot-password.html b/app/templates/views/forgot-password.html index 6db3a81b9..d7b9fc1c0 100644 --- a/app/templates/views/forgot-password.html +++ b/app/templates/views/forgot-password.html @@ -14,11 +14,7 @@ GOV.UK Notify
{{ form.hidden_tag() }} -

- - {{ form.email_address(class="form-control-2-3", autocomplete="off") }}
- Your email address must end in .gov.uk -

+ {{ render_field(form.email_address, class='form-control-2-3') }}

diff --git a/app/templates/views/signin.html b/app/templates/views/signin.html index a399f0196..870b61755 100644 --- a/app/templates/views/signin.html +++ b/app/templates/views/signin.html @@ -18,7 +18,7 @@ Sign in {{ render_field(form.email_address, class='form-control-2-3') }} {{ render_field(form.password, class='form-control-2-3') }}

- Forgotten password? + Forgotten password?

{{ page_footer("Continue") }}
diff --git a/migrations/versions/80_password_reset_token.py b/migrations/versions/80_password_reset_token.py new file mode 100644 index 000000000..5d2da6ad0 --- /dev/null +++ b/migrations/versions/80_password_reset_token.py @@ -0,0 +1,35 @@ +"""empty message + +Revision ID: 80_password_reset_token +Revises: 70_unique_email +Create Date: 2016-01-05 17:47:46.395959 + +""" + +# revision identifiers, used by Alembic. +revision = '80_password_reset_token' +down_revision = '70_unique_email' + +from alembic import op +import sqlalchemy as sa + + +def upgrade(): + ### commands auto generated by Alembic - please adjust! ### + op.create_table('password_reset_token', + sa.Column('id', sa.Integer(), nullable=False), + sa.Column('token', sa.String(), nullable=False), + sa.Column('user_id', sa.Integer(), nullable=False), + sa.Column('expiry_date', sa.DateTime(), nullable=False), + sa.ForeignKeyConstraint(['user_id'], ['users.id'], ), + sa.PrimaryKeyConstraint('id') + ) + op.create_index(op.f('ix_password_reset_token_token'), 'password_reset_token', ['token'], unique=True) + ### end Alembic commands ### + + +def downgrade(): + ### commands auto generated by Alembic - please adjust! ### + op.drop_index(op.f('ix_password_reset_token_token'), table_name='password_reset_token') + op.drop_table('password_reset_token') + ### end Alembic commands ### diff --git a/tests/app/main/dao/test_password_reset_token.py b/tests/app/main/dao/test_password_reset_token.py new file mode 100644 index 000000000..59f40b91b --- /dev/null +++ b/tests/app/main/dao/test_password_reset_token.py @@ -0,0 +1,16 @@ +import uuid + +from app.main.dao import password_reset_token_dao +from app.models import PasswordResetToken +from tests.app.main import create_test_user + + +def test_should_insert_and_return_token(notifications_admin, notifications_admin_db, notify_db_session): + user = create_test_user('active') + token_id = uuid.uuid4() + reset_token = PasswordResetToken(token=str(token_id), + user_id=user.id) + + password_reset_token_dao.insert(reset_token) + saved_token = password_reset_token_dao.get_token(str(token_id)) + assert saved_token.token == str(token_id) diff --git a/tests/app/main/dao/test_users_dao.py b/tests/app/main/dao/test_users_dao.py index ae6579c63..e608cb9d3 100644 --- a/tests/app/main/dao/test_users_dao.py +++ b/tests/app/main/dao/test_users_dao.py @@ -1,8 +1,6 @@ from datetime import datetime - import pytest import sqlalchemy - from app.main.encryption import check_hash from app.models import User from app.main.dao import users_dao @@ -183,3 +181,26 @@ def test_should_update_password(notifications_admin, notifications_admin_db, not assert check_hash('newpassword', updated.password) assert updated.password_changed_at < datetime.now() assert updated.password_changed_at > start + + +def test_should_return_list_of_all_email_addresses(notifications_admin, notifications_admin_db, notify_db_session): + first = User(name='First Person', + password='somepassword', + email_address='first@it.gov.uk', + mobile_number='+441234123412', + created_at=datetime.now(), + role_id=1, + state='active') + second = User(name='Second Person', + password='somepassword', + email_address='second@it.gov.uk', + mobile_number='+441234123412', + created_at=datetime.now(), + role_id=1, + state='active') + users_dao.insert_user(first) + users_dao.insert_user(second) + + email_addresses = users_dao.get_all_users() + expected = [first.email_address, second.email_address] + assert expected == [x.email_address for x in email_addresses] diff --git a/tests/app/main/test_forgot_password_form.py b/tests/app/main/test_forgot_password_form.py new file mode 100644 index 000000000..e4ae6aa19 --- /dev/null +++ b/tests/app/main/test_forgot_password_form.py @@ -0,0 +1,13 @@ +from werkzeug.datastructures import MultiDict + +from app.main.forms import ForgotPasswordForm + + +def test_should_return_validation_error_if_email_address_does_not_exist(notifications_admin, + notifications_admin_db, + notify_db_session): + with notifications_admin.test_request_context(): + form = ForgotPasswordForm(['first@it.gov.uk', 'second@it.gov.uk'], + formdata=MultiDict([('email_address', 'not_found@it.gov.uk')])) + form.validate() + assert {'email_address': ['Please enter the email address that you registered with']} == form.errors diff --git a/tests/app/main/views/test_forgot_password.py b/tests/app/main/views/test_forgot_password.py index 95f0cce2a..54a66952f 100644 --- a/tests/app/main/views/test_forgot_password.py +++ b/tests/app/main/views/test_forgot_password.py @@ -1,4 +1,6 @@ -from flask import current_app +import uuid + +from tests.app.main import create_test_user def test_should_render_forgot_password(notifications_admin, notifications_admin_db, notify_db_session): @@ -8,9 +10,30 @@ def test_should_render_forgot_password(notifications_admin, notifications_admin_ in response.get_data(as_text=True) -def test_should_return_400_when_email_is_invalid(notifications_admin, notifications_admin_db, notify_db_session): +def test_should_have_validate_error_when_email_does_not_exist(notifications_admin, + notifications_admin_db, + notify_db_session): + create_test_user('active') response = notifications_admin.test_client().post('/forgot-password', - data={'email_address': 'not_a_valid_email'}) - x = current_app._get_current_object() - assert response.status_code == 400 - assert 'Please enter a valid email address' in response.get_data(as_text=True) + data={'email_address': 'email_does_not@exist.gov.uk'}) + assert response.status_code == 200 + assert 'Please enter the email address that you registered with' in response.get_data(as_text=True) + + +def test_should_redirect_to_password_reset_sent(notifications_admin, + notifications_admin_db, + mocker, + notify_db_session, + ): + _set_up_mocker(mocker) + create_test_user('active') + response = notifications_admin.test_client().post('/forgot-password', + data={'email_address': 'test@user.gov.uk'}) + + assert response.status_code == 200 + assert 'You have been sent an email containing a url to reset your password.' in response.get_data(as_text=True) + + +def _set_up_mocker(mocker): + mocker.patch("app.admin_api_client.send_sms") + mocker.patch("app.admin_api_client.send_email") From 39970144b6efd30a5f8e13a4cc5ae6fecbc42fcd Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Tue, 5 Jan 2016 17:55:50 +0000 Subject: [PATCH 03/18] fix code style --- app/main/dao/password_reset_token_dao.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/app/main/dao/password_reset_token_dao.py b/app/main/dao/password_reset_token_dao.py index f3f537aec..8421f023a 100644 --- a/app/main/dao/password_reset_token_dao.py +++ b/app/main/dao/password_reset_token_dao.py @@ -1,5 +1,4 @@ from datetime import datetime, timedelta - from app import db from app.models import PasswordResetToken @@ -11,4 +10,4 @@ def insert(token): def get_token(token): - return PasswordResetToken.query.filter_by(token=token).first() \ No newline at end of file + return PasswordResetToken.query.filter_by(token=token).first() From f94966154db18617788c6d737eefb7a2fa97f979 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Wed, 6 Jan 2016 10:55:47 +0000 Subject: [PATCH 04/18] Fix code style. Remove duplicate endpoints --- app/main/views/index.py | 31 ------------------------------- 1 file changed, 31 deletions(-) diff --git a/app/main/views/index.py b/app/main/views/index.py index 33d8f7e7d..777dfa789 100644 --- a/app/main/views/index.py +++ b/app/main/views/index.py @@ -1,6 +1,4 @@ from flask import render_template -from flask_login import login_required - from app.main import main @@ -34,31 +32,6 @@ def checkemail(): return render_template('views/check-email.html') -@main.route("/forgot-password") -def forgotpassword(): - return render_template('views/forgot-password.html') - - -@main.route("/jobs") -def showjobs(): - return render_template('views/jobs.html') - - -@main.route("/jobs/job") -def showjob(): - return render_template('views/job.html') - - -@main.route("/jobs/job/notification") -def shownotification(): - return render_template('views/notification.html') - - -@main.route("/new-password") -def newpassword(): - return render_template('views/new-password.html') - - @main.route("/user-profile") def userprofile(): return render_template('views/user-profile.html') @@ -74,10 +47,6 @@ def apikeys(): return render_template('views/api-keys.html') -@main.route("/verification-not-received") -def verificationnotreceived(): - return render_template('views/verification-not-received.html') - @main.route("/manage-templates") def managetemplates(): return render_template('views/manage-templates.html') From b5901a1ac7e247974bf1509ec08cad59ce80afab Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Wed, 6 Jan 2016 17:37:07 +0000 Subject: [PATCH 05/18] New-password endpoints are implemented. There should be a better way to validate the token. --- app/main/dao/password_reset_token_dao.py | 12 +++-- app/main/forms.py | 7 +++ app/main/views/__init__.py | 10 +++-- app/main/views/forgot_password.py | 3 +- app/main/views/new_password.py | 35 ++++++++++----- app/templates/views/new-password.html | 11 +++-- .../app/main/dao/test_password_reset_token.py | 12 ++--- tests/app/main/views/test_forgot_password.py | 6 +-- tests/app/main/views/test_new_password.py | 44 +++++++++++++++++++ 9 files changed, 104 insertions(+), 36 deletions(-) create mode 100644 tests/app/main/views/test_new_password.py diff --git a/app/main/dao/password_reset_token_dao.py b/app/main/dao/password_reset_token_dao.py index 8421f023a..c43cc92bd 100644 --- a/app/main/dao/password_reset_token_dao.py +++ b/app/main/dao/password_reset_token_dao.py @@ -3,9 +3,15 @@ from app import db from app.models import PasswordResetToken -def insert(token): - token.expiry_date = datetime.now() + timedelta(hours=1) - db.session.add(token) +def insert(token, user_id): + password_reset_token = PasswordResetToken(token=token, + user_id=user_id, + expiry_date=datetime.now() + timedelta(hours=1)) + insert_token(password_reset_token) + + +def insert_token(password_reset_token): + db.session.add(password_reset_token) db.session.commit() diff --git a/app/main/forms.py b/app/main/forms.py index eaf97ede2..3652f5708 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -1,3 +1,5 @@ +from datetime import datetime +from flask import session from flask_wtf import Form from wtforms import StringField, PasswordField, ValidationError @@ -139,3 +141,8 @@ class ForgotPasswordForm(Form): raise ValidationError('Please enter the email address that you registered with') +class NewPasswordForm(Form): + new_password = StringField('Create a password', + validators=[DataRequired(message='Please enter your password'), + Length(10, 255, message='Password must be at least 10 characters'), + Blacklist(message='That password is blacklisted, too common')]) diff --git a/app/main/views/__init__.py b/app/main/views/__init__.py index b109d5465..88a894ee8 100644 --- a/app/main/views/__init__.py +++ b/app/main/views/__init__.py @@ -6,7 +6,7 @@ from flask import url_for from app import admin_api_client from app.main.exceptions import AdminApiClientException -from app.main.dao import verify_codes_dao +from app.main.dao import verify_codes_dao, password_reset_token_dao def create_verify_code(): @@ -38,13 +38,15 @@ def send_email_code(user_id, email): return email_code -def send_change_password_email(email): +def send_change_password_email(email, user_id): try: - link_to_change_password = url_for('.new_password', token=str(uuid.uuid4())) + reset_password_token = str(uuid.uuid4()).replace('-', '') + link_to_change_password = url_for('.new_password', token=reset_password_token, _external=True) + password_reset_token_dao.insert(reset_password_token, user_id) admin_api_client.send_email(email_address=email, from_str='notify@digital.cabinet-office.gov.uk', message=link_to_change_password, - subject='Verification code', + subject='Reset password for GOV.UK Notify', token=admin_api_client.auth_token) except: traceback.print_exc() diff --git a/app/main/views/forgot_password.py b/app/main/views/forgot_password.py index 8cc706dd9..2d21eff09 100644 --- a/app/main/views/forgot_password.py +++ b/app/main/views/forgot_password.py @@ -10,7 +10,8 @@ from app.main.views import send_change_password_email def forgot_password(): form = ForgotPasswordForm(users_dao.find_all_email_address()) if form.validate_on_submit(): - send_change_password_email(form.email_address.data) + user = users_dao.get_user_by_email(form.email_address.data) + send_change_password_email(form.email_address.data, user.id) return render_template('views/password-reset-sent.html') else: return render_template('views/forgot-password.html', form=form) diff --git a/app/main/views/new_password.py b/app/main/views/new_password.py index 8a0ef77bb..c1296b1c2 100644 --- a/app/main/views/new_password.py +++ b/app/main/views/new_password.py @@ -1,16 +1,29 @@ -from flask import request +from datetime import datetime + +from flask import (Markup, render_template, url_for, redirect) from app.main import main +from app.main.dao import (password_reset_token_dao, users_dao) +from app.main.forms import NewPasswordForm +from app.main.views import send_sms_code -@main.route('/new-password/', methods=['GET', 'POST']) -def new_password(): - # Validate token - token = request.args.get('token') - # get password token (better name) - # is it expired - # add NewPasswordForm - # update password - # create password_token table (id, token, user_id, expiry_date +@main.route('/new-password/', methods=['GET', 'POST']) +def new_password(token): + form = NewPasswordForm() + if form.validate_on_submit(): + password_reset_token = password_reset_token_dao.get_token(str(Markup.escape(token))) + if not valid_token(password_reset_token): + form.new_password.errors.append('token is invalid') # Is there a better way + return render_template('views/new-password.html', form=form) + else: + users_dao.update_password(password_reset_token.user_id, form.new_password.data) + user = users_dao.get_user_by_id(password_reset_token.user_id) + send_sms_code(user.id, user.mobile_number) + return redirect(url_for('main.render_two_factor')) + else: + return render_template('views/new-password.html', toke=token, form=form) - return 'Got here' + +def valid_token(token): + return token and datetime.now() <= token.expiry_date diff --git a/app/templates/views/new-password.html b/app/templates/views/new-password.html index a1e312642..d3f3c827b 100644 --- a/app/templates/views/new-password.html +++ b/app/templates/views/new-password.html @@ -12,15 +12,18 @@ GOV.UK Notify

You can now create a new password for your account.

-

- -
+

+ {{ form.hidden_tag() }} +

+ {{ render_field(form.new_password, class="form-control-1-4", type="password") }} Your password must have at least 10 characters

- Continue +

+ +
diff --git a/tests/app/main/dao/test_password_reset_token.py b/tests/app/main/dao/test_password_reset_token.py index 59f40b91b..ebc52fd39 100644 --- a/tests/app/main/dao/test_password_reset_token.py +++ b/tests/app/main/dao/test_password_reset_token.py @@ -1,16 +1,12 @@ import uuid from app.main.dao import password_reset_token_dao -from app.models import PasswordResetToken from tests.app.main import create_test_user def test_should_insert_and_return_token(notifications_admin, notifications_admin_db, notify_db_session): user = create_test_user('active') - token_id = uuid.uuid4() - reset_token = PasswordResetToken(token=str(token_id), - user_id=user.id) - - password_reset_token_dao.insert(reset_token) - saved_token = password_reset_token_dao.get_token(str(token_id)) - assert saved_token.token == str(token_id) + token_id = str(uuid.uuid4()) + password_reset_token_dao.insert(token=token_id, user_id=user.id) + saved_token = password_reset_token_dao.get_token(token_id) + assert saved_token.token == token_id diff --git a/tests/app/main/views/test_forgot_password.py b/tests/app/main/views/test_forgot_password.py index 54a66952f..7e3f46e20 100644 --- a/tests/app/main/views/test_forgot_password.py +++ b/tests/app/main/views/test_forgot_password.py @@ -1,5 +1,3 @@ -import uuid - from tests.app.main import create_test_user @@ -23,8 +21,7 @@ def test_should_have_validate_error_when_email_does_not_exist(notifications_admi def test_should_redirect_to_password_reset_sent(notifications_admin, notifications_admin_db, mocker, - notify_db_session, - ): + notify_db_session): _set_up_mocker(mocker) create_test_user('active') response = notifications_admin.test_client().post('/forgot-password', @@ -35,5 +32,4 @@ def test_should_redirect_to_password_reset_sent(notifications_admin, def _set_up_mocker(mocker): - mocker.patch("app.admin_api_client.send_sms") mocker.patch("app.admin_api_client.send_email") diff --git a/tests/app/main/views/test_new_password.py b/tests/app/main/views/test_new_password.py new file mode 100644 index 000000000..eeb9510b8 --- /dev/null +++ b/tests/app/main/views/test_new_password.py @@ -0,0 +1,44 @@ +from datetime import datetime, timedelta + +from app.main.dao import password_reset_token_dao, users_dao +from app.models import PasswordResetToken +from tests.app.main import create_test_user +from app.main.encryption import check_hash + + +def test_should_render_new_password_template(notifications_admin, notifications_admin_db, notify_db_session): + with notifications_admin.test_request_context(): + with notifications_admin.test_client() as client: + user = create_test_user('active') + password_reset_token_dao.insert('some_token', user.id) + response = client.get('/new-password/some_token') + assert response.status_code == 200 + assert ' You can now create a new password for your account.' in response.get_data(as_text=True) + + +def test_should_redirect_to_two_factor_when_password_reset_is_successful(notifications_admin, notifications_admin_db, + notify_db_session): + with notifications_admin.test_request_context(): + with notifications_admin.test_client() as client: + user = create_test_user('active') + password_reset_token_dao.insert('some_token', user.id) + response = client.post('/new-password/some_token', + data={'new_password': 'a-new_password'}) + assert response.status_code == 302 + assert response.location == 'http://localhost/two-factor' + saved_user = users_dao.get_user_by_id(user.id) + assert check_hash('a-new_password', saved_user.password) + + +def test_should_return_validation_error_that_token_is_expired(notifications_admin, notifications_admin_db, + notify_db_session): + with notifications_admin.test_request_context(): + with notifications_admin.test_client() as client: + user = create_test_user('active') + expired_token = PasswordResetToken(id=1, token='some_token', user_id=user.id, + expiry_date=datetime.now() + timedelta(hours=-2)) + password_reset_token_dao.insert_token(expired_token) + response = client.post('/new-password/some_token', + data={'new_password': 'a-new_password'}) + assert response.status_code == 200 + assert 'token is invalid' in response.get_data(as_text=True) From 8057a138a87a10861828ededd1b031dc91a3688c Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Thu, 7 Jan 2016 10:24:33 +0000 Subject: [PATCH 06/18] Update two_factor redirect endpoint --- app/main/views/new_password.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/main/views/new_password.py b/app/main/views/new_password.py index c1296b1c2..0c479d59f 100644 --- a/app/main/views/new_password.py +++ b/app/main/views/new_password.py @@ -20,7 +20,7 @@ def new_password(token): users_dao.update_password(password_reset_token.user_id, form.new_password.data) user = users_dao.get_user_by_id(password_reset_token.user_id) send_sms_code(user.id, user.mobile_number) - return redirect(url_for('main.render_two_factor')) + return redirect(url_for('main.two_factor')) else: return render_template('views/new-password.html', toke=token, form=form) From a860f713d236d2f55ffa5ed5b5a8175adb615413 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Thu, 7 Jan 2016 17:13:49 +0000 Subject: [PATCH 07/18] Implementation of the new_password endpoint. Found a way to create the token that does not need to persist it to the database. This requires proper error messages, written by people who speak menglis good. --- app/assets/stylesheets/main.scss | 1 + app/main/dao/password_reset_token_dao.py | 19 ------- app/main/dao/users_dao.py | 5 +- app/main/forms.py | 9 ---- app/main/views/__init__.py | 28 ++++++++--- app/main/views/forgot_password.py | 14 +++--- app/main/views/new_password.py | 20 ++++---- app/models.py | 7 --- app/templates/admin_template.html | 16 +++++- app/templates/components/form-field.html | 2 +- config.py | 1 + .../versions/80_password_reset_token.py | 35 ------------- .../app/main/dao/test_password_reset_token.py | 12 ----- tests/app/main/dao/test_users_dao.py | 2 +- tests/app/main/test_forgot_password_form.py | 13 ----- tests/app/main/views/test_forgot_password.py | 10 ---- tests/app/main/views/test_new_password.py | 49 ++++++++++--------- 17 files changed, 87 insertions(+), 156 deletions(-) delete mode 100644 app/main/dao/password_reset_token_dao.py delete mode 100644 migrations/versions/80_password_reset_token.py delete mode 100644 tests/app/main/dao/test_password_reset_token.py delete mode 100644 tests/app/main/test_forgot_password_form.py diff --git a/app/assets/stylesheets/main.scss b/app/assets/stylesheets/main.scss index 9beb5dd10..7a0a95feb 100644 --- a/app/assets/stylesheets/main.scss +++ b/app/assets/stylesheets/main.scss @@ -20,6 +20,7 @@ @import '../govuk_elements/public/sass/elements/details'; @import '../govuk_elements/public/sass/elements/elements-typography'; @import '../govuk_elements/public/sass/elements/forms'; +@import '../govuk_elements/public/sass/elements/forms/form-validation'; @import '../govuk_elements/public/sass/elements/forms/form-block-labels'; @import '../govuk_elements/public/sass/elements/icons'; @import '../govuk_elements/public/sass/elements/layout'; diff --git a/app/main/dao/password_reset_token_dao.py b/app/main/dao/password_reset_token_dao.py deleted file mode 100644 index c43cc92bd..000000000 --- a/app/main/dao/password_reset_token_dao.py +++ /dev/null @@ -1,19 +0,0 @@ -from datetime import datetime, timedelta -from app import db -from app.models import PasswordResetToken - - -def insert(token, user_id): - password_reset_token = PasswordResetToken(token=token, - user_id=user_id, - expiry_date=datetime.now() + timedelta(hours=1)) - insert_token(password_reset_token) - - -def insert_token(password_reset_token): - db.session.add(password_reset_token) - db.session.commit() - - -def get_token(token): - return PasswordResetToken.query.filter_by(token=token).first() diff --git a/app/main/dao/users_dao.py b/app/main/dao/users_dao.py index 8329d057d..64e306b79 100644 --- a/app/main/dao/users_dao.py +++ b/app/main/dao/users_dao.py @@ -59,12 +59,13 @@ def update_mobile_number(id, mobile_number): db.session.commit() -def update_password(id, password): - user = get_user_by_id(id) +def update_password(email, password): + user = get_user_by_email(email) user.password = hashpw(password) user.password_changed_at = datetime.now() db.session.add(user) db.session.commit() + return user def find_all_email_address(): diff --git a/app/main/forms.py b/app/main/forms.py index 3652f5708..46321303d 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -24,7 +24,6 @@ verify_code = '^\d{5}$' class RegisterUserForm(Form): - def __init__(self, existing_email_addresses, existing_mobile_numbers, *args, **kwargs): self.existing_emails = existing_email_addresses self.existing_mobiles = existing_mobile_numbers @@ -125,10 +124,6 @@ class AddServiceForm(Form): class ForgotPasswordForm(Form): - def __init__(self, email_addresses, *args, **kargs): - self.email_addresses = email_addresses - super(ForgotPasswordForm, self).__init__(*args, **kargs) - email_address = StringField('Email address', validators=[Length(min=5, max=255), DataRequired(message='Email cannot be empty'), @@ -136,10 +131,6 @@ class ForgotPasswordForm(Form): Regexp(regex=gov_uk_email, message='Please enter a gov.uk email address') ]) - def validate_email_address(self, a): - if self.email_address.data not in self.email_addresses: - raise ValidationError('Please enter the email address that you registered with') - class NewPasswordForm(Form): new_password = StringField('Create a password', diff --git a/app/main/views/__init__.py b/app/main/views/__init__.py index 88a894ee8..d85bbfbc9 100644 --- a/app/main/views/__init__.py +++ b/app/main/views/__init__.py @@ -1,12 +1,11 @@ import traceback -import uuid from random import randint -from flask import url_for +from flask import url_for, current_app from app import admin_api_client +from app.main.dao import verify_codes_dao from app.main.exceptions import AdminApiClientException -from app.main.dao import verify_codes_dao, password_reset_token_dao def create_verify_code(): @@ -38,11 +37,9 @@ def send_email_code(user_id, email): return email_code -def send_change_password_email(email, user_id): +def send_change_password_email(email): try: - reset_password_token = str(uuid.uuid4()).replace('-', '') - link_to_change_password = url_for('.new_password', token=reset_password_token, _external=True) - password_reset_token_dao.insert(reset_password_token, user_id) + link_to_change_password = url_for('.new_password', token=generate_token(email), _external=True) admin_api_client.send_email(email_address=email, from_str='notify@digital.cabinet-office.gov.uk', message=link_to_change_password, @@ -51,3 +48,20 @@ def send_change_password_email(email, user_id): except: traceback.print_exc() raise AdminApiClientException('Exception when sending email.') + + +def generate_token(email): + from itsdangerous import TimestampSigner + signer = TimestampSigner(current_app.config['SECRET_KEY']) + return signer.sign(email).decode('utf8') + + +def check_token(token): + from itsdangerous import TimestampSigner, SignatureExpired + signer = TimestampSigner(current_app.config['SECRET_KEY']) + try: + email = signer.unsign(token, max_age=current_app.config['TOKEN_MAX_AGE_SECONDS']) + return email + except SignatureExpired as e: + current_app.logger.info('token expired %s' % e) + return None diff --git a/app/main/views/forgot_password.py b/app/main/views/forgot_password.py index 2d21eff09..8413d7270 100644 --- a/app/main/views/forgot_password.py +++ b/app/main/views/forgot_password.py @@ -1,5 +1,4 @@ -from flask import render_template - +from flask import render_template, flash from app.main import main from app.main.dao import users_dao from app.main.forms import ForgotPasswordForm @@ -8,10 +7,13 @@ from app.main.views import send_change_password_email @main.route('/forgot-password', methods=['GET', 'POST']) def forgot_password(): - form = ForgotPasswordForm(users_dao.find_all_email_address()) + form = ForgotPasswordForm() if form.validate_on_submit(): - user = users_dao.get_user_by_email(form.email_address.data) - send_change_password_email(form.email_address.data, user.id) - return render_template('views/password-reset-sent.html') + if users_dao.get_user_by_email(form.email_address.data): + send_change_password_email(form.email_address.data) + return render_template('views/password-reset-sent.html') + else: + flash('The email address is not recognized. Try again.') + return render_template('views/forgot-password.html', form=form) else: return render_template('views/forgot-password.html', form=form) diff --git a/app/main/views/new_password.py b/app/main/views/new_password.py index 0c479d59f..4be020e74 100644 --- a/app/main/views/new_password.py +++ b/app/main/views/new_password.py @@ -1,26 +1,26 @@ from datetime import datetime -from flask import (Markup, render_template, url_for, redirect) +from flask import (render_template, url_for, redirect, flash, current_app) from app.main import main -from app.main.dao import (password_reset_token_dao, users_dao) +from app.main.dao import users_dao from app.main.forms import NewPasswordForm -from app.main.views import send_sms_code +from app.main.views import send_sms_code, check_token @main.route('/new-password/', methods=['GET', 'POST']) def new_password(token): form = NewPasswordForm() if form.validate_on_submit(): - password_reset_token = password_reset_token_dao.get_token(str(Markup.escape(token))) - if not valid_token(password_reset_token): - form.new_password.errors.append('token is invalid') # Is there a better way - return render_template('views/new-password.html', form=form) - else: - users_dao.update_password(password_reset_token.user_id, form.new_password.data) - user = users_dao.get_user_by_id(password_reset_token.user_id) + email_address = check_token(token) + if email_address: + user = users_dao.update_password(email_address.decode('utf-8'), form.new_password.data) send_sms_code(user.id, user.mobile_number) return redirect(url_for('main.two_factor')) + else: + flash('expired token request again') + current_app.logger.info('we got here') + return redirect(url_for('.forgot_password')) else: return render_template('views/new-password.html', toke=token, form=form) diff --git a/app/models.py b/app/models.py index cff7d18a6..3608f040f 100644 --- a/app/models.py +++ b/app/models.py @@ -111,13 +111,6 @@ class Service(db.Model): return filter_null_value_fields(serialized) -class PasswordResetToken(db.Model): - id = db.Column(db.Integer, primary_key=True) - token = db.Column(db.String, unique=True, index=True, nullable=False) - user_id = db.Column(db.Integer, db.ForeignKey('users.id'), unique=False, nullable=False) - expiry_date = db.Column(db.DateTime, nullable=False) - - def filter_null_value_fields(obj): return dict( filter(lambda x: x[1] is not None, obj.items()) diff --git a/app/templates/admin_template.html b/app/templates/admin_template.html index 4613ca442..a03e47612 100644 --- a/app/templates/admin_template.html +++ b/app/templates/admin_template.html @@ -43,6 +43,7 @@ {% endblock %} + {% set global_header_text = "GOV.UK Notify" %} @@ -53,9 +54,20 @@ {% endif %} {% block content %} -
+
+ {% with messages = get_flashed_messages() %} + {% if messages %} +
+
    + {% for message in messages %} +
  • {{ message }}
    • + {% endfor %} +
+
+ {% endif %} + {% endwith %} {% block fullwidth_content %}{% endblock %} -
+
{% endblock %} {% block body_end %} diff --git a/app/templates/components/form-field.html b/app/templates/components/form-field.html index 643099498..691b4c579 100644 --- a/app/templates/components/form-field.html +++ b/app/templates/components/form-field.html @@ -2,7 +2,7 @@
{{ field.label }}
{{ field(**kwargs)|safe }} {% if field.errors %} -
    +
      {% for error in field.errors %}
    • {{ error }}
      • {% endfor %} diff --git a/config.py b/config.py index d53cf17f9..40f4a58c2 100644 --- a/config.py +++ b/config.py @@ -29,6 +29,7 @@ class Config(object): SECRET_KEY = 'secret-key' HTTP_PROTOCOL = 'http' DANGEROUS_SALT = 'itsdangeroussalt' + TOKEN_MAX_AGE_SECONDS = 86400 class Development(Config): diff --git a/migrations/versions/80_password_reset_token.py b/migrations/versions/80_password_reset_token.py deleted file mode 100644 index 5d2da6ad0..000000000 --- a/migrations/versions/80_password_reset_token.py +++ /dev/null @@ -1,35 +0,0 @@ -"""empty message - -Revision ID: 80_password_reset_token -Revises: 70_unique_email -Create Date: 2016-01-05 17:47:46.395959 - -""" - -# revision identifiers, used by Alembic. -revision = '80_password_reset_token' -down_revision = '70_unique_email' - -from alembic import op -import sqlalchemy as sa - - -def upgrade(): - ### commands auto generated by Alembic - please adjust! ### - op.create_table('password_reset_token', - sa.Column('id', sa.Integer(), nullable=False), - sa.Column('token', sa.String(), nullable=False), - sa.Column('user_id', sa.Integer(), nullable=False), - sa.Column('expiry_date', sa.DateTime(), nullable=False), - sa.ForeignKeyConstraint(['user_id'], ['users.id'], ), - sa.PrimaryKeyConstraint('id') - ) - op.create_index(op.f('ix_password_reset_token_token'), 'password_reset_token', ['token'], unique=True) - ### end Alembic commands ### - - -def downgrade(): - ### commands auto generated by Alembic - please adjust! ### - op.drop_index(op.f('ix_password_reset_token_token'), table_name='password_reset_token') - op.drop_table('password_reset_token') - ### end Alembic commands ### diff --git a/tests/app/main/dao/test_password_reset_token.py b/tests/app/main/dao/test_password_reset_token.py deleted file mode 100644 index ebc52fd39..000000000 --- a/tests/app/main/dao/test_password_reset_token.py +++ /dev/null @@ -1,12 +0,0 @@ -import uuid - -from app.main.dao import password_reset_token_dao -from tests.app.main import create_test_user - - -def test_should_insert_and_return_token(notifications_admin, notifications_admin_db, notify_db_session): - user = create_test_user('active') - token_id = str(uuid.uuid4()) - password_reset_token_dao.insert(token=token_id, user_id=user.id) - saved_token = password_reset_token_dao.get_token(token_id) - assert saved_token.token == token_id diff --git a/tests/app/main/dao/test_users_dao.py b/tests/app/main/dao/test_users_dao.py index e608cb9d3..60d5dcbc8 100644 --- a/tests/app/main/dao/test_users_dao.py +++ b/tests/app/main/dao/test_users_dao.py @@ -176,7 +176,7 @@ def test_should_update_password(notifications_admin, notifications_admin_db, not saved = users_dao.get_user_by_id(user.id) assert check_hash('somepassword', saved.password) assert saved.password_changed_at is None - users_dao.update_password(saved.id, 'newpassword') + users_dao.update_password(saved.email_address, 'newpassword') updated = users_dao.get_user_by_id(user.id) assert check_hash('newpassword', updated.password) assert updated.password_changed_at < datetime.now() diff --git a/tests/app/main/test_forgot_password_form.py b/tests/app/main/test_forgot_password_form.py deleted file mode 100644 index e4ae6aa19..000000000 --- a/tests/app/main/test_forgot_password_form.py +++ /dev/null @@ -1,13 +0,0 @@ -from werkzeug.datastructures import MultiDict - -from app.main.forms import ForgotPasswordForm - - -def test_should_return_validation_error_if_email_address_does_not_exist(notifications_admin, - notifications_admin_db, - notify_db_session): - with notifications_admin.test_request_context(): - form = ForgotPasswordForm(['first@it.gov.uk', 'second@it.gov.uk'], - formdata=MultiDict([('email_address', 'not_found@it.gov.uk')])) - form.validate() - assert {'email_address': ['Please enter the email address that you registered with']} == form.errors diff --git a/tests/app/main/views/test_forgot_password.py b/tests/app/main/views/test_forgot_password.py index 7e3f46e20..aea89d869 100644 --- a/tests/app/main/views/test_forgot_password.py +++ b/tests/app/main/views/test_forgot_password.py @@ -8,16 +8,6 @@ def test_should_render_forgot_password(notifications_admin, notifications_admin_ in response.get_data(as_text=True) -def test_should_have_validate_error_when_email_does_not_exist(notifications_admin, - notifications_admin_db, - notify_db_session): - create_test_user('active') - response = notifications_admin.test_client().post('/forgot-password', - data={'email_address': 'email_does_not@exist.gov.uk'}) - assert response.status_code == 200 - assert 'Please enter the email address that you registered with' in response.get_data(as_text=True) - - def test_should_redirect_to_password_reset_sent(notifications_admin, notifications_admin_db, mocker, diff --git a/tests/app/main/views/test_new_password.py b/tests/app/main/views/test_new_password.py index eeb9510b8..aa01d411e 100644 --- a/tests/app/main/views/test_new_password.py +++ b/tests/app/main/views/test_new_password.py @@ -1,19 +1,13 @@ -from datetime import datetime, timedelta - -from app.main.dao import password_reset_token_dao, users_dao -from app.models import PasswordResetToken -from tests.app.main import create_test_user +from app.main.dao import users_dao from app.main.encryption import check_hash +from app.main.views import generate_token +from tests.app.main import create_test_user def test_should_render_new_password_template(notifications_admin, notifications_admin_db, notify_db_session): - with notifications_admin.test_request_context(): - with notifications_admin.test_client() as client: - user = create_test_user('active') - password_reset_token_dao.insert('some_token', user.id) - response = client.get('/new-password/some_token') - assert response.status_code == 200 - assert ' You can now create a new password for your account.' in response.get_data(as_text=True) + response = notifications_admin.test_client().get('/new-password/some_token') + assert response.status_code == 200 + assert ' You can now create a new password for your account.' in response.get_data(as_text=True) def test_should_redirect_to_two_factor_when_password_reset_is_successful(notifications_admin, notifications_admin_db, @@ -21,8 +15,8 @@ def test_should_redirect_to_two_factor_when_password_reset_is_successful(notific with notifications_admin.test_request_context(): with notifications_admin.test_client() as client: user = create_test_user('active') - password_reset_token_dao.insert('some_token', user.id) - response = client.post('/new-password/some_token', + token = generate_token(user.email_address) + response = client.post('/new-password/{}'.format(token), data={'new_password': 'a-new_password'}) assert response.status_code == 302 assert response.location == 'http://localhost/two-factor' @@ -30,15 +24,26 @@ def test_should_redirect_to_two_factor_when_password_reset_is_successful(notific assert check_hash('a-new_password', saved_user.password) -def test_should_return_validation_error_that_token_is_expired(notifications_admin, notifications_admin_db, - notify_db_session): +def test_should_redirect_to_forgot_password_with_flash_message_when_token_is_expired(notifications_admin, + notifications_admin_db, + notify_db_session): with notifications_admin.test_request_context(): with notifications_admin.test_client() as client: + notifications_admin.config['TOKEN_MAX_AGE_SECONDS'] = -1000 user = create_test_user('active') - expired_token = PasswordResetToken(id=1, token='some_token', user_id=user.id, - expiry_date=datetime.now() + timedelta(hours=-2)) - password_reset_token_dao.insert_token(expired_token) - response = client.post('/new-password/some_token', + token = generate_token(user.email_address) + response = client.post('/new-password/{}'.format(token), data={'new_password': 'a-new_password'}) - assert response.status_code == 200 - assert 'token is invalid' in response.get_data(as_text=True) + assert response.status_code == 302 + assert response.location == 'http://localhost/forgot-password' + notifications_admin.config['TOKEN_MAX_AGE_SECONDS'] = 86400 + + +def test_should_return_500_error_page_when_email_addres_does_not_exist(notifications_admin, notifications_admin_db, + notify_db_session): + with notifications_admin.test_request_context(): + with notifications_admin.test_client() as client: + token = generate_token('doesnotexist@it.gov.uk') + response = client.post('/new-password/{}'.format(token), + data={'new_password': 'a-new_password'}) + assert response.status_code == 500 From 35619553c8b72875a6d165706cdb1efd03364481 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Thu, 7 Jan 2016 17:41:21 +0000 Subject: [PATCH 08/18] Added NoDataFoundException --- app/main/dao/users_dao.py | 14 +++++++++----- app/main/exceptions.py | 7 +++++-- tests/app/main/views/test_new_password.py | 17 ++++++++++++----- 3 files changed, 26 insertions(+), 12 deletions(-) diff --git a/app/main/dao/users_dao.py b/app/main/dao/users_dao.py index 64e306b79..94510704c 100644 --- a/app/main/dao/users_dao.py +++ b/app/main/dao/users_dao.py @@ -3,6 +3,7 @@ from datetime import datetime from sqlalchemy.orm import load_only from app import db, login_manager +from app.main.exceptions import NoDataFoundException from app.models import User from app.main.encryption import hashpw @@ -61,11 +62,14 @@ def update_mobile_number(id, mobile_number): def update_password(email, password): user = get_user_by_email(email) - user.password = hashpw(password) - user.password_changed_at = datetime.now() - db.session.add(user) - db.session.commit() - return user + if user: + user.password = hashpw(password) + user.password_changed_at = datetime.now() + db.session.add(user) + db.session.commit() + return user + else: + raise NoDataFoundException('The user does not exist') def find_all_email_address(): diff --git a/app/main/exceptions.py b/app/main/exceptions.py index 45f0515c0..7b09af0eb 100644 --- a/app/main/exceptions.py +++ b/app/main/exceptions.py @@ -1,5 +1,8 @@ - - class AdminApiClientException(Exception): def __init__(self, message): self.value = message + + +class NoDataFoundException(Exception): + def __init__(self, message): + self.value = message diff --git a/tests/app/main/views/test_new_password.py b/tests/app/main/views/test_new_password.py index aa01d411e..9b645644d 100644 --- a/tests/app/main/views/test_new_password.py +++ b/tests/app/main/views/test_new_password.py @@ -1,5 +1,8 @@ +from pytest import fail + from app.main.dao import users_dao from app.main.encryption import check_hash +from app.main.exceptions import NoDataFoundException from app.main.views import generate_token from tests.app.main import create_test_user @@ -39,11 +42,15 @@ def test_should_redirect_to_forgot_password_with_flash_message_when_token_is_exp notifications_admin.config['TOKEN_MAX_AGE_SECONDS'] = 86400 -def test_should_return_500_error_page_when_email_addres_does_not_exist(notifications_admin, notifications_admin_db, - notify_db_session): +def test_should_return_raise_no_data_found_exception_when_email_address_does_not_exist(notifications_admin, + notifications_admin_db, + notify_db_session): with notifications_admin.test_request_context(): with notifications_admin.test_client() as client: token = generate_token('doesnotexist@it.gov.uk') - response = client.post('/new-password/{}'.format(token), - data={'new_password': 'a-new_password'}) - assert response.status_code == 500 + try: + client.post('/new-password/{}'.format(token), + data={'new_password': 'a-new_password'}) + fail('Expected NoDataFoundException') + except NoDataFoundException: + pass From eb78a778085617e515e7d45e8688e900f09b836e Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Thu, 7 Jan 2016 17:54:23 +0000 Subject: [PATCH 09/18] Added the mock for the admin_api_client --- tests/app/main/views/test_new_password.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/tests/app/main/views/test_new_password.py b/tests/app/main/views/test_new_password.py index 9b645644d..adf5961c6 100644 --- a/tests/app/main/views/test_new_password.py +++ b/tests/app/main/views/test_new_password.py @@ -13,8 +13,10 @@ def test_should_render_new_password_template(notifications_admin, notifications_ assert ' You can now create a new password for your account.' in response.get_data(as_text=True) -def test_should_redirect_to_two_factor_when_password_reset_is_successful(notifications_admin, notifications_admin_db, - notify_db_session): +def test_should_redirect_to_two_factor_when_password_reset_is_successful(notifications_admin, + notifications_admin_db, + notify_db_session, + mocker): with notifications_admin.test_request_context(): with notifications_admin.test_client() as client: user = create_test_user('active') @@ -54,3 +56,7 @@ def test_should_return_raise_no_data_found_exception_when_email_address_does_not fail('Expected NoDataFoundException') except NoDataFoundException: pass + + +def _set_up_mocker(mocker): + mocker.patch("app.admin_api_client.send_sms") From f951b364aca35f21f5a41a331333c3808660b167 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Thu, 7 Jan 2016 18:02:33 +0000 Subject: [PATCH 10/18] Actually use the mock in the test --- tests/app/main/views/test_new_password.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/app/main/views/test_new_password.py b/tests/app/main/views/test_new_password.py index adf5961c6..d6721b1c5 100644 --- a/tests/app/main/views/test_new_password.py +++ b/tests/app/main/views/test_new_password.py @@ -17,6 +17,7 @@ def test_should_redirect_to_two_factor_when_password_reset_is_successful(notific notifications_admin_db, notify_db_session, mocker): + _set_up_mocker(mocker) with notifications_admin.test_request_context(): with notifications_admin.test_client() as client: user = create_test_user('active') From ceb78f49b4e8aec9aa4f119ba43933b6277325a3 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Fri, 8 Jan 2016 12:00:52 +0000 Subject: [PATCH 11/18] Take out the Canadian politeness. Make the error message more consistent. Extracted common fields for the forms. --- app/main/forms.py | 110 +++++++++++------------ app/main/views/forgot_password.py | 2 +- tests/app/main/views/test_add_service.py | 2 +- tests/app/main/views/test_register.py | 4 +- 4 files changed, 58 insertions(+), 60 deletions(-) diff --git a/app/main/forms.py b/app/main/forms.py index 46321303d..96980aeb5 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -1,28 +1,61 @@ -from datetime import datetime -from flask import session - from flask_wtf import Form from wtforms import StringField, PasswordField, ValidationError from wtforms.validators import DataRequired, Email, Length, Regexp from app.main.validators import Blacklist, ValidateUserCodes +def email_address(): + gov_uk_email \ + = "(^[^@^\\s]+@[^@^\\.^\\s]+(\\.[^@^\\.^\\s]*)*.gov.uk)" + return StringField('Email address', validators=[ + Length(min=5, max=255), + DataRequired(message='Email cannot be empty'), + Email(message='Enter a valid email address'), + Regexp(regex=gov_uk_email, message='Enter a gov.uk email address')]) + + +def mobile_number(): + mobile_number_regex = "^\\+44[\\d]{10}$" + return StringField('Mobile phone number', + validators=[DataRequired(message='Mobile number can not be empty'), + Regexp(regex=mobile_number_regex, message='Enter a +44 mobile number')]) + + +def password(): + return PasswordField('Create a password', + 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')]) + + +def sms_code(): + verify_code = '^\d{5}$' + return StringField('Text message confirmation code', + validators=[DataRequired(message='Text message confirmation code can not be empty'), + Regexp(regex=verify_code, + message='Text message confirmation code must be 5 digits'), + ValidateUserCodes(code_type='sms')]) + + +def email_code(): + verify_code = '^\d{5}$' + return StringField("Email confirmation code", + validators=[DataRequired(message='Email confirmation code can not be empty'), + Regexp(regex=verify_code, message='Email confirmation code must be 5 digits'), + ValidateUserCodes(code_type='email')]) + + class LoginForm(Form): email_address = StringField('Email address', validators=[ Length(min=5, max=255), DataRequired(message='Email cannot be empty'), - Email(message='Please enter a valid email address') + Email(message='Enter a valid email address') ]) password = PasswordField('Password', validators=[ - DataRequired(message='Please enter your password') + DataRequired(message='Enter your password') ]) -gov_uk_email = "(^[^@^\\s]+@[^@^\\.^\\s]+(\\.[^@^\\.^\\s]*)*.gov.uk)" -mobile_number = "^\\+44[\\d]{10}$" -verify_code = '^\d{5}$' - - class RegisterUserForm(Form): def __init__(self, existing_email_addresses, existing_mobile_numbers, *args, **kwargs): self.existing_emails = existing_email_addresses @@ -31,19 +64,9 @@ class RegisterUserForm(Form): name = StringField('Full name', validators=[DataRequired(message='Name can not be empty')]) - email_address = StringField('Email address', validators=[ - Length(min=5, max=255), - DataRequired(message='Email cannot be empty'), - Email(message='Please enter a valid email address'), - Regexp(regex=gov_uk_email, message='Please enter a gov.uk email address') - ]) - mobile_number = StringField('Mobile phone number', - validators=[DataRequired(message='Please enter your mobile number'), - Regexp(regex=mobile_number, message='Please enter a +44 mobile number')]) - password = PasswordField('Create a password', - validators=[DataRequired(message='Please enter your password'), - Length(10, 255, message='Password must be at least 10 characters'), - Blacklist(message='That password is blacklisted, too common')]) + email_address = email_address() + mobile_number = mobile_number() + password = password() def validate_email_address(self, field): # Validate email address is unique. @@ -59,7 +82,6 @@ class RegisterUserForm(Form): class TwoFactorForm(Form): - def __init__(self, user_codes, *args, **kwargs): ''' Keyword arguments: @@ -69,13 +91,10 @@ class TwoFactorForm(Form): self.user_codes = user_codes super(TwoFactorForm, self).__init__(*args, **kwargs) - sms_code = StringField('sms code', validators=[DataRequired(message='Enter verification code'), - Regexp(regex=verify_code, message='Code must be 5 digits'), - ValidateUserCodes(code_type='sms')]) + sms_code = sms_code() class VerifyForm(Form): - def __init__(self, user_codes, *args, **kwargs): ''' Keyword arguments: @@ -85,29 +104,16 @@ class VerifyForm(Form): self.user_codes = user_codes super(VerifyForm, self).__init__(*args, **kwargs) - sms_code = StringField("Text message confirmation code", - validators=[DataRequired(message='SMS code can not be empty'), - Regexp(regex=verify_code, message='Code must be 5 digits'), - ValidateUserCodes(code_type='sms')]) - email_code = StringField("Email confirmation code", - validators=[DataRequired(message='Email code can not be empty'), - Regexp(regex=verify_code, message='Code must be 5 digits'), - ValidateUserCodes(code_type='email')]) + sms_code = sms_code() + email_code = email_code() class EmailNotReceivedForm(Form): - email_address = StringField('Email address', validators=[ - Length(min=5, max=255), - DataRequired(message='Email cannot be empty'), - Email(message='Please enter a valid email address'), - Regexp(regex=gov_uk_email, message='Please enter a gov.uk email address') - ]) + email_address = email_address() class TextNotReceivedForm(Form): - mobile_number = StringField('Mobile phone number', validators=[ - DataRequired(message='Please enter your mobile number'), - Regexp(regex=mobile_number, message='Please enter a +44 mobile number')]) + mobile_number = mobile_number() class AddServiceForm(Form): @@ -116,7 +122,7 @@ class AddServiceForm(Form): super(AddServiceForm, self).__init__(*args, **kwargs) service_name = StringField(validators=[ - DataRequired(message='Please enter your service name')]) + DataRequired(message='Service name can not be empty')]) def validate_service_name(self, a): if self.service_name.data in self.service_names: @@ -124,16 +130,8 @@ class AddServiceForm(Form): class ForgotPasswordForm(Form): - email_address = StringField('Email address', - validators=[Length(min=5, max=255), - DataRequired(message='Email cannot be empty'), - Email(message='Please enter a valid email address'), - Regexp(regex=gov_uk_email, message='Please enter a gov.uk email address') - ]) + email_address = email_address() class NewPasswordForm(Form): - new_password = StringField('Create a password', - validators=[DataRequired(message='Please enter your password'), - Length(10, 255, message='Password must be at least 10 characters'), - Blacklist(message='That password is blacklisted, too common')]) + new_password = password() diff --git a/app/main/views/forgot_password.py b/app/main/views/forgot_password.py index 8413d7270..0488afac3 100644 --- a/app/main/views/forgot_password.py +++ b/app/main/views/forgot_password.py @@ -13,7 +13,7 @@ def forgot_password(): send_change_password_email(form.email_address.data) return render_template('views/password-reset-sent.html') else: - flash('The email address is not recognized. Try again.') + flash('The email address is not recognized. Enter the password you registered with.') return render_template('views/forgot-password.html', form=form) else: return render_template('views/forgot-password.html', form=form) diff --git a/tests/app/main/views/test_add_service.py b/tests/app/main/views/test_add_service.py index 5f199ac63..02506ace3 100644 --- a/tests/app/main/views/test_add_service.py +++ b/tests/app/main/views/test_add_service.py @@ -39,4 +39,4 @@ def test_should_return_form_errors_when_service_name_is_empty(notifications_admi client.post('/two-factor', data={'sms_code': '12345'}) response = client.post('/add-service', data={}) assert response.status_code == 200 - assert 'Please enter your service name' in response.get_data(as_text=True) + assert 'Service name can not be empty' in response.get_data(as_text=True) diff --git a/tests/app/main/views/test_register.py b/tests/app/main/views/test_register.py index 666e9f86d..cdf021adc 100644 --- a/tests/app/main/views/test_register.py +++ b/tests/app/main/views/test_register.py @@ -31,7 +31,7 @@ def test_process_register_returns_400_when_mobile_number_is_invalid(notification 'password': 'validPassword!'}) assert response.status_code == 200 - assert 'Please enter a +44 mobile number' in response.get_data(as_text=True) + assert 'Enter a +44 mobile number' in response.get_data(as_text=True) def test_should_return_400_when_email_is_not_gov_uk(notifications_admin, @@ -46,7 +46,7 @@ def test_should_return_400_when_email_is_not_gov_uk(notifications_admin, 'password': 'validPassword!'}) assert response.status_code == 200 - assert 'Please enter a gov.uk email address' in response.get_data(as_text=True) + assert 'Enter a gov.uk email address' in response.get_data(as_text=True) def test_should_add_verify_codes_on_session(notifications_admin, notifications_admin_db, mocker, notify_db_session): From c858869a523349c6eeecc8a157948b3badea2deb Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Fri, 8 Jan 2016 15:12:14 +0000 Subject: [PATCH 12/18] Removed exceptions, found a better way to handle them. Refactored the forms so that fields like email_address can be used in multiple forms. Refactored form validation so that a query function is passed into the form to be run, this way the form is not exposed to the dao layer and the query is more efficient. This PR still requires some frontend attention. Will work with Chris to update the templates. --- app/main/dao/users_dao.py | 16 +++----- app/main/exceptions.py | 8 ---- app/main/forms.py | 20 ++++----- app/main/views/__init__.py | 43 +++++++------------- app/main/views/forgot_password.py | 10 ++--- app/main/views/new_password.py | 30 ++++++-------- app/main/views/register.py | 17 ++------ app/templates/views/forgot-password.html | 2 +- app/templates/views/new-password.html | 4 ++ tests/app/main/dao/test_users_dao.py | 2 +- tests/app/main/test_validators.py | 5 +-- tests/app/main/views/test_forgot_password.py | 18 ++++++++ tests/app/main/views/test_new_password.py | 42 +++++++++---------- 13 files changed, 96 insertions(+), 121 deletions(-) delete mode 100644 app/main/exceptions.py diff --git a/app/main/dao/users_dao.py b/app/main/dao/users_dao.py index 94510704c..3e198395b 100644 --- a/app/main/dao/users_dao.py +++ b/app/main/dao/users_dao.py @@ -3,7 +3,6 @@ from datetime import datetime from sqlalchemy.orm import load_only from app import db, login_manager -from app.main.exceptions import NoDataFoundException from app.models import User from app.main.encryption import hashpw @@ -60,16 +59,11 @@ def update_mobile_number(id, mobile_number): db.session.commit() -def update_password(email, password): - user = get_user_by_email(email) - if user: - user.password = hashpw(password) - user.password_changed_at = datetime.now() - db.session.add(user) - db.session.commit() - return user - else: - raise NoDataFoundException('The user does not exist') +def update_password(user, password): + user.password = hashpw(password) + user.password_changed_at = datetime.now() + db.session.add(user) + db.session.commit() def find_all_email_address(): diff --git a/app/main/exceptions.py b/app/main/exceptions.py deleted file mode 100644 index 7b09af0eb..000000000 --- a/app/main/exceptions.py +++ /dev/null @@ -1,8 +0,0 @@ -class AdminApiClientException(Exception): - def __init__(self, message): - self.value = message - - -class NoDataFoundException(Exception): - def __init__(self, message): - self.value = message diff --git a/app/main/forms.py b/app/main/forms.py index 96980aeb5..038dbacea 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -57,9 +57,8 @@ class LoginForm(Form): class RegisterUserForm(Form): - def __init__(self, existing_email_addresses, existing_mobile_numbers, *args, **kwargs): + def __init__(self, existing_email_addresses, *args, **kwargs): self.existing_emails = existing_email_addresses - self.existing_mobiles = existing_mobile_numbers super(RegisterUserForm, self).__init__(*args, **kwargs) name = StringField('Full name', @@ -70,16 +69,9 @@ class RegisterUserForm(Form): def validate_email_address(self, field): # Validate email address is unique. - if field.data in self.existing_emails: + if self.existing_emails(field.data): raise ValidationError('Email address already exists') - def validate_mobile_number(self, field): - # Validate mobile number is unique - # Code to re-added later - # if field.data in self.existing_mobiles: - # raise ValidationError('Mobile number already exists') - pass - class TwoFactorForm(Form): def __init__(self, user_codes, *args, **kwargs): @@ -132,6 +124,14 @@ class AddServiceForm(Form): class ForgotPasswordForm(Form): email_address = email_address() + def __init__(self, q, *args, **kwargs): + self.query_function = q + super(ForgotPasswordForm, self).__init__(*args, *kwargs) + + def validate_email_address(self, a): + if not self.query_function(a.data): + raise ValidationError('The email address is not recognized. Enter the password you registered with.') + class NewPasswordForm(Form): new_password = password() diff --git a/app/main/views/__init__.py b/app/main/views/__init__.py index d85bbfbc9..a76b4feb4 100644 --- a/app/main/views/__init__.py +++ b/app/main/views/__init__.py @@ -1,11 +1,9 @@ -import traceback from random import randint from flask import url_for, current_app from app import admin_api_client from app.main.dao import verify_codes_dao -from app.main.exceptions import AdminApiClientException def create_verify_code(): @@ -14,40 +12,30 @@ def create_verify_code(): def send_sms_code(user_id, mobile_number): sms_code = create_verify_code() - try: - verify_codes_dao.add_code(user_id=user_id, code=sms_code, code_type='sms') - admin_api_client.send_sms(mobile_number=mobile_number, message=sms_code, token=admin_api_client.auth_token) - except: - raise AdminApiClientException('Exception when sending sms.') + verify_codes_dao.add_code(user_id=user_id, code=sms_code, code_type='sms') + admin_api_client.send_sms(mobile_number=mobile_number, message=sms_code, token=admin_api_client.auth_token) + return sms_code def send_email_code(user_id, email): email_code = create_verify_code() - try: - verify_codes_dao.add_code(user_id=user_id, code=email_code, code_type='email') - admin_api_client.send_email(email_address=email, - from_str='notify@digital.cabinet-office.gov.uk', - message=email_code, - subject='Verification code', - token=admin_api_client.auth_token) - except: - raise AdminApiClientException('Exception when sending email.') - + verify_codes_dao.add_code(user_id=user_id, code=email_code, code_type='email') + admin_api_client.send_email(email_address=email, + from_str='notify@digital.cabinet-office.gov.uk', + message=email_code, + subject='Verification code', + token=admin_api_client.auth_token) return email_code def send_change_password_email(email): - try: - link_to_change_password = url_for('.new_password', token=generate_token(email), _external=True) - admin_api_client.send_email(email_address=email, - from_str='notify@digital.cabinet-office.gov.uk', - message=link_to_change_password, - subject='Reset password for GOV.UK Notify', - token=admin_api_client.auth_token) - except: - traceback.print_exc() - raise AdminApiClientException('Exception when sending email.') + link_to_change_password = url_for('.new_password', token=generate_token(email), _external=True) + admin_api_client.send_email(email_address=email, + from_str='notify@digital.cabinet-office.gov.uk', + message=link_to_change_password, + subject='Reset password for GOV.UK Notify', + token=admin_api_client.auth_token) def generate_token(email): @@ -64,4 +52,3 @@ def check_token(token): return email except SignatureExpired as e: current_app.logger.info('token expired %s' % e) - return None diff --git a/app/main/views/forgot_password.py b/app/main/views/forgot_password.py index 0488afac3..eae320df1 100644 --- a/app/main/views/forgot_password.py +++ b/app/main/views/forgot_password.py @@ -7,13 +7,9 @@ from app.main.views import send_change_password_email @main.route('/forgot-password', methods=['GET', 'POST']) def forgot_password(): - form = ForgotPasswordForm() + form = ForgotPasswordForm(users_dao.get_user_by_email) if form.validate_on_submit(): - if users_dao.get_user_by_email(form.email_address.data): - send_change_password_email(form.email_address.data) - return render_template('views/password-reset-sent.html') - else: - flash('The email address is not recognized. Enter the password you registered with.') - return render_template('views/forgot-password.html', form=form) + send_change_password_email(form.email_address.data) + return render_template('views/password-reset-sent.html') else: return render_template('views/forgot-password.html', form=form) diff --git a/app/main/views/new_password.py b/app/main/views/new_password.py index 4be020e74..9e0af8355 100644 --- a/app/main/views/new_password.py +++ b/app/main/views/new_password.py @@ -1,6 +1,4 @@ -from datetime import datetime - -from flask import (render_template, url_for, redirect, flash, current_app) +from flask import (render_template, url_for, redirect, flash) from app.main import main from app.main.dao import users_dao @@ -10,20 +8,18 @@ from app.main.views import send_sms_code, check_token @main.route('/new-password/', methods=['GET', 'POST']) def new_password(token): + email_address = check_token(token) + if not email_address: + flash('The token we sent you has expired. Enter your email address to try again.') + return redirect(url_for('.forgot_password')) + + user = users_dao.get_user_by_email(email_address=email_address.decode('utf-8')) + form = NewPasswordForm() + if form.validate_on_submit(): - email_address = check_token(token) - if email_address: - user = users_dao.update_password(email_address.decode('utf-8'), form.new_password.data) - send_sms_code(user.id, user.mobile_number) - return redirect(url_for('main.two_factor')) - else: - flash('expired token request again') - current_app.logger.info('we got here') - return redirect(url_for('.forgot_password')) + users_dao.update_password(user, form.new_password.data) + send_sms_code(user.id, user.mobile_number) + return redirect(url_for('main.two_factor')) else: - return render_template('views/new-password.html', toke=token, form=form) - - -def valid_token(token): - return token and datetime.now() <= token.expiry_date + return render_template('views/new-password.html', token=token, form=form, user=user) diff --git a/app/main/views/register.py b/app/main/views/register.py index 2e1c7f309..9eef37a4a 100644 --- a/app/main/views/register.py +++ b/app/main/views/register.py @@ -1,12 +1,9 @@ from datetime import datetime, timedelta -from flask import render_template, redirect, jsonify, session -from sqlalchemy.exc import SQLAlchemyError +from flask import render_template, redirect, session from app.main import main from app.main.dao import users_dao -from app.main.encryption import hashpw -from app.main.exceptions import AdminApiClientException from app.main.forms import RegisterUserForm from app.main.views import send_sms_code, send_email_code from app.models import User @@ -15,16 +12,8 @@ from app.models import User # TODO how do we handle duplicate unverifed email addresses? # malicious or otherwise. @main.route('/register', methods=['GET', 'POST']) -def process_register(): - try: - existing_emails, existing_mobiles = zip( - *[(x.email_address, x.mobile_number) for x in - users_dao.get_all_users()]) - except ValueError: - # Value error is raised if the db is empty. - existing_emails, existing_mobiles = [], [] - - form = RegisterUserForm(existing_emails, existing_mobiles) +def register(): + form = RegisterUserForm(users_dao.get_user_by_email) if form.validate_on_submit(): user = User(name=form.name.data, diff --git a/app/templates/views/forgot-password.html b/app/templates/views/forgot-password.html index d7b9fc1c0..4d36f2a86 100644 --- a/app/templates/views/forgot-password.html +++ b/app/templates/views/forgot-password.html @@ -16,7 +16,7 @@ GOV.UK Notify {{ form.hidden_tag() }} {{ render_field(form.email_address, class='form-control-2-3') }}

      - +

      diff --git a/app/templates/views/new-password.html b/app/templates/views/new-password.html index d3f3c827b..580325410 100644 --- a/app/templates/views/new-password.html +++ b/app/templates/views/new-password.html @@ -8,6 +8,7 @@ GOV.UK Notify
      + {% if user %}

      Create a new password

      You can now create a new password for your account.

      @@ -24,6 +25,9 @@ GOV.UK Notify

      + {% else %} + Message about email address does not exist. Some one needs to figure out the words here. + {% endif %}
      diff --git a/tests/app/main/dao/test_users_dao.py b/tests/app/main/dao/test_users_dao.py index 60d5dcbc8..387653b26 100644 --- a/tests/app/main/dao/test_users_dao.py +++ b/tests/app/main/dao/test_users_dao.py @@ -176,7 +176,7 @@ def test_should_update_password(notifications_admin, notifications_admin_db, not saved = users_dao.get_user_by_id(user.id) assert check_hash('somepassword', saved.password) assert saved.password_changed_at is None - users_dao.update_password(saved.email_address, 'newpassword') + users_dao.update_password(saved, 'newpassword') updated = users_dao.get_user_by_id(user.id) assert check_hash('newpassword', updated.password) assert updated.password_changed_at < datetime.now() diff --git a/tests/app/main/test_validators.py b/tests/app/main/test_validators.py index d85159f51..512f0350a 100644 --- a/tests/app/main/test_validators.py +++ b/tests/app/main/test_validators.py @@ -1,10 +1,9 @@ -from pytest import fail - +from app.main.dao import users_dao from app.main.forms import RegisterUserForm def test_should_raise_validation_error_for_password(notifications_admin): - form = RegisterUserForm([], []) + form = RegisterUserForm(users_dao.get_user_by_email) form.name.data = 'test' form.email_address.data = 'teset@example.gov.uk' form.mobile_number.data = '+441231231231' diff --git a/tests/app/main/views/test_forgot_password.py b/tests/app/main/views/test_forgot_password.py index aea89d869..688960791 100644 --- a/tests/app/main/views/test_forgot_password.py +++ b/tests/app/main/views/test_forgot_password.py @@ -1,3 +1,6 @@ +from flask import url_for + +from app.main.views import generate_token from tests.app.main import create_test_user @@ -21,5 +24,20 @@ def test_should_redirect_to_password_reset_sent(notifications_admin, assert 'You have been sent an email containing a url to reset your password.' in response.get_data(as_text=True) +def test_should_redirect_to_forgot_password_with_flash_message_when_token_is_expired(notifications_admin, + notifications_admin_db, + notify_db_session): + with notifications_admin.test_request_context(): + with notifications_admin.test_client() as client: + notifications_admin.config['TOKEN_MAX_AGE_SECONDS'] = -1000 + user = create_test_user('active') + token = generate_token(user.email_address) + response = client.post('/new-password/{}'.format(token), + data={'new_password': 'a-new_password'}) + assert response.status_code == 302 + assert response.location == url_for('.forgot_password', _external=True) + notifications_admin.config['TOKEN_MAX_AGE_SECONDS'] = 86400 + + def _set_up_mocker(mocker): mocker.patch("app.admin_api_client.send_email") diff --git a/tests/app/main/views/test_new_password.py b/tests/app/main/views/test_new_password.py index d6721b1c5..74a560c4a 100644 --- a/tests/app/main/views/test_new_password.py +++ b/tests/app/main/views/test_new_password.py @@ -1,16 +1,30 @@ -from pytest import fail +from flask import url_for from app.main.dao import users_dao from app.main.encryption import check_hash -from app.main.exceptions import NoDataFoundException from app.main.views import generate_token from tests.app.main import create_test_user def test_should_render_new_password_template(notifications_admin, notifications_admin_db, notify_db_session): - response = notifications_admin.test_client().get('/new-password/some_token') - assert response.status_code == 200 - assert ' You can now create a new password for your account.' in response.get_data(as_text=True) + with notifications_admin.test_request_context(): + with notifications_admin.test_client() as client: + user = create_test_user('active') + token = generate_token(user.email_address) + response = client.get('/new-password/{}'.format(token)) + assert response.status_code == 200 + assert ' You can now create a new password for your account.' in response.get_data(as_text=True) + + +def test_should_render_new_password_template_with_message_of_bad_token(notifications_admin, notifications_admin_db, + notify_db_session): + with notifications_admin.test_request_context(): + with notifications_admin.test_client() as client: + token = generate_token('no_user@d.gov.uk') + response = client.get('/new-password/{}'.format(token)) + assert response.status_code == 200 + assert 'Message about email address does not exist. Some one needs to figure out the words here.' in \ + response.get_data(as_text=True) def test_should_redirect_to_two_factor_when_password_reset_is_successful(notifications_admin, @@ -25,7 +39,7 @@ def test_should_redirect_to_two_factor_when_password_reset_is_successful(notific response = client.post('/new-password/{}'.format(token), data={'new_password': 'a-new_password'}) assert response.status_code == 302 - assert response.location == 'http://localhost/two-factor' + assert response.location == url_for('.two_factor', _external=True) saved_user = users_dao.get_user_by_id(user.id) assert check_hash('a-new_password', saved_user.password) @@ -41,23 +55,9 @@ def test_should_redirect_to_forgot_password_with_flash_message_when_token_is_exp response = client.post('/new-password/{}'.format(token), data={'new_password': 'a-new_password'}) assert response.status_code == 302 - assert response.location == 'http://localhost/forgot-password' + assert response.location == url_for('.forgot_password', _external=True) notifications_admin.config['TOKEN_MAX_AGE_SECONDS'] = 86400 -def test_should_return_raise_no_data_found_exception_when_email_address_does_not_exist(notifications_admin, - notifications_admin_db, - notify_db_session): - with notifications_admin.test_request_context(): - with notifications_admin.test_client() as client: - token = generate_token('doesnotexist@it.gov.uk') - try: - client.post('/new-password/{}'.format(token), - data={'new_password': 'a-new_password'}) - fail('Expected NoDataFoundException') - except NoDataFoundException: - pass - - def _set_up_mocker(mocker): mocker.patch("app.admin_api_client.send_sms") From 9ca2f2017f60a36578b558073e32020b38e02016 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Fri, 8 Jan 2016 15:38:47 +0000 Subject: [PATCH 13/18] Fix argument --- app/main/forms.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/main/forms.py b/app/main/forms.py index 038dbacea..808f55246 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -126,7 +126,7 @@ class ForgotPasswordForm(Form): def __init__(self, q, *args, **kwargs): self.query_function = q - super(ForgotPasswordForm, self).__init__(*args, *kwargs) + super(ForgotPasswordForm, self).__init__(*args, **kwargs) def validate_email_address(self, a): if not self.query_function(a.data): From 677f8891b2ceebdf2a11d88948cbf33438de3640 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Fri, 8 Jan 2016 15:55:31 +0000 Subject: [PATCH 14/18] Fix layout and wrong word --- app/main/forms.py | 2 +- app/templates/views/password-reset-sent.html | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/main/forms.py b/app/main/forms.py index 808f55246..816817778 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -130,7 +130,7 @@ class ForgotPasswordForm(Form): def validate_email_address(self, a): if not self.query_function(a.data): - raise ValidationError('The email address is not recognized. Enter the password you registered with.') + raise ValidationError('The email address is not recognised. Enter the email address you registered with.') class NewPasswordForm(Form): diff --git a/app/templates/views/password-reset-sent.html b/app/templates/views/password-reset-sent.html index c51c206d3..42c5e0954 100644 --- a/app/templates/views/password-reset-sent.html +++ b/app/templates/views/password-reset-sent.html @@ -4,7 +4,7 @@ GOV.UK Notify | {% endblock %} -{% block content %} +{% block fullwidth_content %}
      @@ -15,4 +15,4 @@ GOV.UK Notify |
      -{% endblock %} \ No newline at end of file +{% endblock %} From f7373ee5fc2e99c0445ab3259b629085748f1283 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Fri, 8 Jan 2016 16:47:34 +0000 Subject: [PATCH 15/18] Fix wording Changed forgot-password so that it does not expose to the user that the email address does not exist. --- app/main/forms.py | 8 -------- app/main/views/forgot_password.py | 11 +++++++---- app/main/views/new_password.py | 2 +- app/templates/views/password-reset-sent.html | 2 +- tests/app/main/views/test_forgot_password.py | 14 +++++--------- 5 files changed, 14 insertions(+), 23 deletions(-) diff --git a/app/main/forms.py b/app/main/forms.py index 816817778..e06595ba4 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -124,14 +124,6 @@ class AddServiceForm(Form): class ForgotPasswordForm(Form): email_address = email_address() - def __init__(self, q, *args, **kwargs): - self.query_function = q - super(ForgotPasswordForm, self).__init__(*args, **kwargs) - - def validate_email_address(self, a): - if not self.query_function(a.data): - raise ValidationError('The email address is not recognised. Enter the email address you registered with.') - class NewPasswordForm(Form): new_password = password() diff --git a/app/main/views/forgot_password.py b/app/main/views/forgot_password.py index eae320df1..a47285983 100644 --- a/app/main/views/forgot_password.py +++ b/app/main/views/forgot_password.py @@ -1,4 +1,4 @@ -from flask import render_template, flash +from flask import render_template, flash, current_app from app.main import main from app.main.dao import users_dao from app.main.forms import ForgotPasswordForm @@ -7,9 +7,12 @@ from app.main.views import send_change_password_email @main.route('/forgot-password', methods=['GET', 'POST']) def forgot_password(): - form = ForgotPasswordForm(users_dao.get_user_by_email) + form = ForgotPasswordForm() if form.validate_on_submit(): - send_change_password_email(form.email_address.data) - return render_template('views/password-reset-sent.html') + if users_dao.get_user_by_email(form.email_address.data): + send_change_password_email(form.email_address.data) + return render_template('views/password-reset-sent.html') + else: + current_app.logger.info('The email address used does not exist.') else: return render_template('views/forgot-password.html', form=form) diff --git a/app/main/views/new_password.py b/app/main/views/new_password.py index 9e0af8355..984c0535c 100644 --- a/app/main/views/new_password.py +++ b/app/main/views/new_password.py @@ -10,7 +10,7 @@ from app.main.views import send_sms_code, check_token def new_password(token): email_address = check_token(token) if not email_address: - flash('The token we sent you has expired. Enter your email address to try again.') + flash('The link in the email we sent you has expired. Enter your email address to resend.') return redirect(url_for('.forgot_password')) user = users_dao.get_user_by_email(email_address=email_address.decode('utf-8')) diff --git a/app/templates/views/password-reset-sent.html b/app/templates/views/password-reset-sent.html index 42c5e0954..6d12e559c 100644 --- a/app/templates/views/password-reset-sent.html +++ b/app/templates/views/password-reset-sent.html @@ -10,7 +10,7 @@ GOV.UK Notify |

      GOV.UK Notify

      -

      You have been sent an email containing a url to reset your password.

      +

      You have been sent an email containing a link to reset your password.

      diff --git a/tests/app/main/views/test_forgot_password.py b/tests/app/main/views/test_forgot_password.py index 688960791..fc1ae2d5e 100644 --- a/tests/app/main/views/test_forgot_password.py +++ b/tests/app/main/views/test_forgot_password.py @@ -15,13 +15,13 @@ def test_should_redirect_to_password_reset_sent(notifications_admin, notifications_admin_db, mocker, notify_db_session): - _set_up_mocker(mocker) - create_test_user('active') + mocker.patch("app.admin_api_client.send_email") + user = create_test_user('active') response = notifications_admin.test_client().post('/forgot-password', - data={'email_address': 'test@user.gov.uk'}) - + data={'email_address': user.email_address}) assert response.status_code == 200 - assert 'You have been sent an email containing a url to reset your password.' in response.get_data(as_text=True) + assert 'You have been sent an email containing a link to reset your password.' in response.get_data( + as_text=True) def test_should_redirect_to_forgot_password_with_flash_message_when_token_is_expired(notifications_admin, @@ -37,7 +37,3 @@ def test_should_redirect_to_forgot_password_with_flash_message_when_token_is_exp assert response.status_code == 302 assert response.location == url_for('.forgot_password', _external=True) notifications_admin.config['TOKEN_MAX_AGE_SECONDS'] = 86400 - - -def _set_up_mocker(mocker): - mocker.patch("app.admin_api_client.send_email") From 806c584b03376448cc98a356d6919a64334eb485 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Mon, 11 Jan 2016 10:44:04 +0000 Subject: [PATCH 16/18] Reset password token is valid for 1 hour --- config.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config.py b/config.py index 40f4a58c2..a56f16824 100644 --- a/config.py +++ b/config.py @@ -29,7 +29,7 @@ class Config(object): SECRET_KEY = 'secret-key' HTTP_PROTOCOL = 'http' DANGEROUS_SALT = 'itsdangeroussalt' - TOKEN_MAX_AGE_SECONDS = 86400 + TOKEN_MAX_AGE_SECONDS = 120 class Development(Config): From 0c1592905f2f794742eef058b661b5a0efcc3348 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Mon, 11 Jan 2016 12:06:01 +0000 Subject: [PATCH 17/18] Update sign_out to go to the index page not sign-in --- app/main/views/sign_out.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/main/views/sign_out.py b/app/main/views/sign_out.py index d734916c8..fe1e47329 100644 --- a/app/main/views/sign_out.py +++ b/app/main/views/sign_out.py @@ -9,4 +9,4 @@ from app.main import main @login_required def sign_out(): logout_user() - return redirect(url_for('main.sign_in')) + return redirect(url_for('main.index')) From bb1db0c345ace6d5214b0a498e342bf4ba1d6bf8 Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Mon, 11 Jan 2016 12:06:52 +0000 Subject: [PATCH 18/18] When the user request a reset password link, the user.state is set to request_password_reset. Which means the user will only be able to reset their password, and not sign-in. Once the user resets the password the user state is set to active once more. If the link is used a second time they will be redirected to the index page with a message that the link in the email is not longer valid. --- app/main/dao/users_dao.py | 8 ++++-- app/main/views/forgot_password.py | 1 + app/main/views/new_password.py | 3 ++ config.py | 2 +- tests/app/main/dao/test_users_dao.py | 14 +++++++++ tests/app/main/views/test_forgot_password.py | 10 ++++--- tests/app/main/views/test_new_password.py | 30 ++++++++++++++------ tests/app/main/views/test_sign_out.py | 2 +- 8 files changed, 53 insertions(+), 17 deletions(-) diff --git a/app/main/dao/users_dao.py b/app/main/dao/users_dao.py index 3e198395b..f45d2a95d 100644 --- a/app/main/dao/users_dao.py +++ b/app/main/dao/users_dao.py @@ -62,9 +62,13 @@ def update_mobile_number(id, mobile_number): def update_password(user, password): user.password = hashpw(password) user.password_changed_at = datetime.now() + user.state = 'active' db.session.add(user) db.session.commit() -def find_all_email_address(): - return [x.email_address for x in User.query.options(load_only("email_address")).all()] +def request_password_reset(email): + user = get_user_by_email(email) + user.state = 'request_password_reset' + db.session.add(user) + db.session.commit() diff --git a/app/main/views/forgot_password.py b/app/main/views/forgot_password.py index a47285983..0e5c81bda 100644 --- a/app/main/views/forgot_password.py +++ b/app/main/views/forgot_password.py @@ -10,6 +10,7 @@ def forgot_password(): form = ForgotPasswordForm() if form.validate_on_submit(): if users_dao.get_user_by_email(form.email_address.data): + users_dao.request_password_reset(form.email_address.data) send_change_password_email(form.email_address.data) return render_template('views/password-reset-sent.html') else: diff --git a/app/main/views/new_password.py b/app/main/views/new_password.py index 984c0535c..c1b0e1e04 100644 --- a/app/main/views/new_password.py +++ b/app/main/views/new_password.py @@ -14,6 +14,9 @@ def new_password(token): return redirect(url_for('.forgot_password')) user = users_dao.get_user_by_email(email_address=email_address.decode('utf-8')) + if user and user.state != 'request_password_reset': + flash('The link in the email we sent you has already been used.') + return redirect(url_for('.index')) form = NewPasswordForm() diff --git a/config.py b/config.py index a56f16824..0f03f3792 100644 --- a/config.py +++ b/config.py @@ -29,7 +29,7 @@ class Config(object): SECRET_KEY = 'secret-key' HTTP_PROTOCOL = 'http' DANGEROUS_SALT = 'itsdangeroussalt' - TOKEN_MAX_AGE_SECONDS = 120 + TOKEN_MAX_AGE_SECONDS = 120000 class Development(Config): diff --git a/tests/app/main/dao/test_users_dao.py b/tests/app/main/dao/test_users_dao.py index 387653b26..885d9ab34 100644 --- a/tests/app/main/dao/test_users_dao.py +++ b/tests/app/main/dao/test_users_dao.py @@ -204,3 +204,17 @@ def test_should_return_list_of_all_email_addresses(notifications_admin, notifica email_addresses = users_dao.get_all_users() expected = [first.email_address, second.email_address] assert expected == [x.email_address for x in email_addresses] + + +def test_should_update_state_to_request_password_reset(notifications_admin, notifications_admin_db, notify_db_session): + user = User(name='Requesting Password Resest', + password='somepassword', + email_address='request@new_password.gov.uk', + mobile_number='+441234123412', + created_at=datetime.now(), + role_id=1, + state='active') + users_dao.insert_user(user) + users_dao.request_password_reset(user.email_address) + saved = users_dao.get_user_by_email(user.email_address) + assert saved.state == 'request_password_reset' diff --git a/tests/app/main/views/test_forgot_password.py b/tests/app/main/views/test_forgot_password.py index fc1ae2d5e..c7dac0177 100644 --- a/tests/app/main/views/test_forgot_password.py +++ b/tests/app/main/views/test_forgot_password.py @@ -1,5 +1,6 @@ from flask import url_for +from app.main.dao import users_dao from app.main.views import generate_token from tests.app.main import create_test_user @@ -11,10 +12,10 @@ def test_should_render_forgot_password(notifications_admin, notifications_admin_ in response.get_data(as_text=True) -def test_should_redirect_to_password_reset_sent(notifications_admin, - notifications_admin_db, - mocker, - notify_db_session): +def test_should_redirect_to_password_reset_sent_and_state_updated(notifications_admin, + notifications_admin_db, + mocker, + notify_db_session): mocker.patch("app.admin_api_client.send_email") user = create_test_user('active') response = notifications_admin.test_client().post('/forgot-password', @@ -22,6 +23,7 @@ def test_should_redirect_to_password_reset_sent(notifications_admin, assert response.status_code == 200 assert 'You have been sent an email containing a link to reset your password.' in response.get_data( as_text=True) + assert users_dao.get_user_by_id(user.id).state == 'request_password_reset' def test_should_redirect_to_forgot_password_with_flash_message_when_token_is_expired(notifications_admin, diff --git a/tests/app/main/views/test_new_password.py b/tests/app/main/views/test_new_password.py index 74a560c4a..4f1aeba6b 100644 --- a/tests/app/main/views/test_new_password.py +++ b/tests/app/main/views/test_new_password.py @@ -9,9 +9,9 @@ from tests.app.main import create_test_user def test_should_render_new_password_template(notifications_admin, notifications_admin_db, notify_db_session): with notifications_admin.test_request_context(): with notifications_admin.test_client() as client: - user = create_test_user('active') + user = create_test_user('request_password_reset') token = generate_token(user.email_address) - response = client.get('/new-password/{}'.format(token)) + response = client.get(url_for('.new_password', token=token)) assert response.status_code == 200 assert ' You can now create a new password for your account.' in response.get_data(as_text=True) @@ -20,8 +20,9 @@ def test_should_render_new_password_template_with_message_of_bad_token(notificat notify_db_session): with notifications_admin.test_request_context(): with notifications_admin.test_client() as client: + create_test_user('request_password_reset') token = generate_token('no_user@d.gov.uk') - response = client.get('/new-password/{}'.format(token)) + response = client.get(url_for('.new_password', token=token)) assert response.status_code == 200 assert 'Message about email address does not exist. Some one needs to figure out the words here.' in \ response.get_data(as_text=True) @@ -34,14 +35,14 @@ def test_should_redirect_to_two_factor_when_password_reset_is_successful(notific _set_up_mocker(mocker) with notifications_admin.test_request_context(): with notifications_admin.test_client() as client: - user = create_test_user('active') + user = create_test_user('request_password_reset') token = generate_token(user.email_address) - response = client.post('/new-password/{}'.format(token), - data={'new_password': 'a-new_password'}) + response = client.post(url_for('.new_password', token=token), data={'new_password': 'a-new_password'}) assert response.status_code == 302 assert response.location == url_for('.two_factor', _external=True) saved_user = users_dao.get_user_by_id(user.id) assert check_hash('a-new_password', saved_user.password) + assert saved_user.state == 'active' def test_should_redirect_to_forgot_password_with_flash_message_when_token_is_expired(notifications_admin, @@ -50,14 +51,25 @@ def test_should_redirect_to_forgot_password_with_flash_message_when_token_is_exp with notifications_admin.test_request_context(): with notifications_admin.test_client() as client: notifications_admin.config['TOKEN_MAX_AGE_SECONDS'] = -1000 - user = create_test_user('active') + user = create_test_user('request_password_reset') token = generate_token(user.email_address) - response = client.post('/new-password/{}'.format(token), - data={'new_password': 'a-new_password'}) + response = client.post(url_for('.new_password', token=token), data={'new_password': 'a-new_password'}) assert response.status_code == 302 assert response.location == url_for('.forgot_password', _external=True) notifications_admin.config['TOKEN_MAX_AGE_SECONDS'] = 86400 +def test_should_redirect_to_forgot_password_when_user_is_active_should_be_request_password_reset(notifications_admin, + notifications_admin_db, + notify_db_session): + with notifications_admin.test_request_context(): + with notifications_admin.test_client() as client: + user = create_test_user('active') + token = generate_token(user.email_address) + response = client.post(url_for('.new_password', token=token), data={'new_password': 'a-new_password'}) + assert response.status_code == 302 + assert response.location == url_for('.index', _external=True) + + def _set_up_mocker(mocker): mocker.patch("app.admin_api_client.send_sms") diff --git a/tests/app/main/views/test_sign_out.py b/tests/app/main/views/test_sign_out.py index 05fc8804d..31ccb055e 100644 --- a/tests/app/main/views/test_sign_out.py +++ b/tests/app/main/views/test_sign_out.py @@ -40,4 +40,4 @@ def test_sign_out_user(notifications_admin, response = client.get(url_for('main.sign_out')) assert response.status_code == 302 assert response.location == url_for( - 'main.sign_in', _external=True) + 'main.index', _external=True)