From b5901a1ac7e247974bf1509ec08cad59ce80afab Mon Sep 17 00:00:00 2001 From: Rebecca Law Date: Wed, 6 Jan 2016 17:37:07 +0000 Subject: [PATCH] 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)