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")