Re-implement forgot password

app/main/dao/users_dao.py

@@ -64,11 +64,6 @@ def is_email_unique(email_address):
             raise ex
 
 
-def request_password_reset(user):
-    user.state = 'request_password_reset'
-    user_api_client.update_user(user)
-
-
 def send_verify_code(user_id, code_type, to):
     return user_api_client.send_verify_code(user_id, code_type, to)
 

app/main/views/forgot_password.py

@@ -1,25 +1,22 @@
 from flask import (
     render_template,
-    flash
 )
+from notifications_python_client.errors import HTTPError
 
 from app.main import main
-from app.main.dao import users_dao
 from app.main.forms import ForgotPasswordForm
-from app.notify_client.sender import send_change_password_email
+from app import user_api_client
 
 
 @main.route('/forgot-password', methods=['GET', 'POST'])
 def forgot_password():
-
     form = ForgotPasswordForm()
     if form.validate_on_submit():
-        if not users_dao.is_email_unique(form.email_address.data):
+        try:
-            user = users_dao.get_user_by_email(form.email_address.data)
+            user_api_client.send_reset_password_url(form.email_address.data)
-            users_dao.request_password_reset(user)
+        except HTTPError as e:
-            send_change_password_email(form.email_address.data)
+            if e.status_code != 404:
-            return render_template('views/password-reset-sent.html')
+                raise e
-        else:
+        return render_template('views/password-reset-sent.html')
-            return render_template('views/password-reset-sent.html')
 
     return render_template('views/forgot-password.html', form=form)

app/main/views/new_password.py

@@ -1,22 +1,33 @@
-from flask import (render_template, url_for, redirect, flash, session)
+import json
+
+from flask import (render_template, url_for, redirect, flash, session, current_app, abort)
+from itsdangerous import SignatureExpired
 
 from app.main import main
 from app.main.dao import users_dao
 from app.main.forms import NewPasswordForm
-from app.notify_client.sender import check_token
+from datetime import datetime
 
 
 @main.route('/new-password/<path:token>', methods=['GET', 'POST'])
 def new_password(token):
-    email_address = check_token(token)
+    from utils.url_safe_token import check_token
-    if not email_address:
+    try:
+        token_data = check_token(token, current_app.config['SECRET_KEY'], current_app.config['DANGEROUS_SALT'],
+                                 current_app.config['TOKEN_MAX_AGE_SECONDS'])
+    except SignatureExpired:
         flash('The link in the email we sent you has expired. Enter your email address to resend.')
         return redirect(url_for('.forgot_password'))
 
+    email_address = json.loads(token_data)['email']
     user = users_dao.get_user_by_email(email_address=email_address)
-    if user and user.state != 'request_password_reset':
+    # TODO: what should this be??
-        flash('The link in the email we sent you has already been used.')
+    if not user:
-        return redirect(url_for('.index'))
+        abort(404, 'user not found')
+    if user.password_changed_at and datetime.strptime(user.password_changed_at, '%Y-%m-%d %H:%M:%S.%f') > \
+            datetime.strptime(json.loads(token_data)['created_at'], '%Y-%m-%d %H:%M:%S.%f'):
+        flash('The link in the email has already been used')
+        return redirect(url_for('main.index'))
 
     form = NewPasswordForm()
 
@@ -26,7 +37,6 @@ def new_password(token):
             'id': user.id,
             'email': user.email_address,
             'password': form.new_password.data}
-        users_dao.activate_user(user)
         return redirect(url_for('main.two_factor'))
     else:
         return render_template('views/new-password.html', token=token, form=form, user=user)

app/notify_client/user_api_client.py

@@ -102,4 +102,9 @@ class UserApiClient(BaseAPIClient):
     def set_user_permissions(self, user_id, service_id, permissions):
         data = [{'permission': x} for x in permissions]
         endpoint = '/user/{}/service/{}/permission'.format(user_id, service_id)
-        resp = self.post(endpoint, data=data)
+        self.post(endpoint, data=data)
+
+    def send_reset_password_url(self, email_address):
+        endpoint = '/user/reset-password'
+        data = {'email': email_address}
+        self.post(endpoint, data=data)

config.py

@@ -39,9 +39,9 @@ class Config(object):
     ADMIN_CLIENT_SECRET = os.getenv('ADMIN_CLIENT_SECRET')
 
     WTF_CSRF_ENABLED = True
-    SECRET_KEY = 'secret-key'
+    SECRET_KEY = 'dev-notify-secret-key'
     HTTP_PROTOCOL = 'http'
-    DANGEROUS_SALT = 'itsdangeroussalt'
+    DANGEROUS_SALT = 'dev-notify-salt'
     TOKEN_MAX_AGE_SECONDS = 3600
 
     MAX_CONTENT_LENGTH = 10 * 1024 * 1024  # 10mb

tests/app/main/views/test_forgot_password.py

@@ -9,13 +9,10 @@ def test_should_render_forgot_password(app_):
                in response.get_data(as_text=True)
 
 
-def test_should_redirect_to_password_reset_sent_and_state_updated(
+def test_should_redirect_to_password_reset_sent_for_valid_email(
-    app_,
+        app_,
-    api_user_active,
+        api_user_active,
-    mock_get_user_by_email,
+        mock_reset_user_password):
-    mock_update_user,
-    mock_send_email
-):
     with app_.test_request_context():
         response = app_.test_client().post(
             url_for('.forgot_password'),
@@ -24,23 +21,4 @@ def test_should_redirect_to_password_reset_sent_and_state_updated(
         assert (
             'You have been sent an email containing a link'
             ' to reset your password.') in response.get_data(as_text=True)
-        assert mock_send_email.call_count == 1
+        mock_reset_user_password.assert_called_once_with(api_user_active.email_address)
-
-
-def test_should_redirect_to_password_reset_sent_for_non_existant_email_address(
-    app_,
-    api_user_active,
-    mock_dont_get_user_by_email,
-    mock_update_user,
-    mock_send_email
-):
-    with app_.test_request_context():
-        response = app_.test_client().post(
-            url_for('.forgot_password'),
-            data={'email_address': 'nope@example.gov.uk'})
-        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)
-        mock_dont_get_user_by_email.assert_called_once_with('nope@example.gov.uk')
-        assert not mock_send_email.called

tests/app/main/views/test_new_password.py

@@ -1,9 +1,8 @@
+import json
+from datetime import datetime
+
 from flask import url_for
-
+from utils.url_safe_token import generate_token
-from app.main.dao import users_dao
-from app.main.encryption import check_hash
-from app.notify_client.sender import generate_token
-
 import pytest
 
 
@@ -14,59 +13,62 @@ def test_should_render_new_password_template(app_,
                                              mock_get_user_by_email_request_password_reset):
     with app_.test_request_context():
         with app_.test_client() as client:
-            token = generate_token(api_user_active.email_address)
+            data = json.dumps({'email': api_user_active.email_address, 'created_at': str(datetime.now())})
+            token = generate_token(data, app_.config['SECRET_KEY'],
+                                   app_.config['DANGEROUS_SALT'])
         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)
 
 
-@pytest.mark.skipif(True, reason='Password reset no implemented')
+def test_should_return_404_when_email_address_does_not_exist(app_, mock_get_user_by_email_not_found):
-def test_should_render_new_password_template_with_message_of_bad_token(app_,
-                                                                       mock_get_user_by_email):
     with app_.test_request_context():
         with app_.test_client() as client:
-            token = generate_token('no_user@d.gov.uk')
+            data = json.dumps({'email': 'no_user@d.gov.uk', 'created_at': str(datetime.now())})
+            token = generate_token(data, app_.config['SECRET_KEY'], app_.config['DANGEROUS_SALT'])
         response = client.get(url_for('.new_password', token=token))
-        assert response.status_code == 200
+        assert response.status_code == 404
-        assert 'Message about email address does not exist. Some one needs to figure out the words here.' in \
-               response.get_data(as_text=True)
 
 
-@pytest.mark.skipif(True, reason='Password reset no implemented')
 def test_should_redirect_to_two_factor_when_password_reset_is_successful(app_,
                                                                          mock_get_user_by_email_request_password_reset,
-                                                                         mock_login):
+                                                                         mock_login,
+                                                                         mock_send_verify_code):
     with app_.test_request_context():
         with app_.test_client() as client:
             user = mock_get_user_by_email_request_password_reset.return_value
-            token = generate_token(user.email_address)
+            data = json.dumps({'email': user.email_address, 'created_at': str(datetime.now())})
+            token = generate_token(data, app_.config['SECRET_KEY'], app_.config['DANGEROUS_SALT'])
         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)
+        mock_get_user_by_email_request_password_reset.assert_called_once_with(user.email_address)
+
+
+def test_should_redirect_index_if_user_has_already_changed_password(app_,
+                                                                    mock_get_user_by_email_user_changed_password,
+                                                                    mock_login,
+                                                                    mock_send_verify_code):
+    with app_.test_request_context():
+        with app_.test_client() as client:
+            user = mock_get_user_by_email_user_changed_password.return_value
+            data = json.dumps({'email': user.email_address, 'created_at': str(datetime.now())})
+            token = generate_token(data, app_.config['SECRET_KEY'], app_.config['DANGEROUS_SALT'])
+        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)
+        mock_get_user_by_email_user_changed_password.assert_called_once_with(user.email_address)
 
 
 def test_should_redirect_to_forgot_password_with_flash_message_when_token_is_expired(
-    app_, mock_get_user_by_email_request_password_reset, mock_login
+        app_, mock_get_user_by_email_request_password_reset, mock_login
 ):
     with app_.test_request_context():
         with app_.test_client() as client:
             app_.config['TOKEN_MAX_AGE_SECONDS'] = -1000
             user = mock_get_user_by_email_request_password_reset.return_value
-            token = generate_token(user.email_address)
+            token = generate_token(user.email_address, app_.config['SECRET_KEY'], app_.config['DANGEROUS_SALT'])
         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)
         app_.config['TOKEN_MAX_AGE_SECONDS'] = 3600
-
-
-@pytest.mark.skipif(True, reason='Password reset no implemented')
-def test_should_redirect_to_forgot_pass_when_user_active_should_be_request_passw_reset(
-    app_, mock_get_user_by_email_request_password_reset, mock_login
-):
-    with app_.test_request_context():
-        with app_.test_client() as client:
-            user = mock_get_user_by_email_request_password_reset.return_value
-            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)

tests/conftest.py

@@ -1,5 +1,5 @@
 import uuid
-from datetime import date
+from datetime import date, datetime, timedelta
 import pytest
 
 from app import create_app
@@ -268,9 +268,27 @@ def api_user_request_password_reset():
                  'password': 'somepassword',
                  'email_address': 'test@user.gov.uk',
                  'mobile_number': '+4412341234',
-                 'state': 'request_password_reset',
+                 'state': 'active',
                  'failed_login_count': 5,
-                 'permissions': {}
+                 'permissions': {},
+                 'password_changed_at': None
+                 }
+    user = User(user_data)
+    return user
+
+
+@pytest.fixture(scope='function')
+def api_user_changed_password():
+    from app.notify_client.user_api_client import User
+    user_data = {'id': 555,
+                 'name': 'Test User',
+                 'password': 'somepassword',
+                 'email_address': 'test@user.gov.uk',
+                 'mobile_number': '+4412341234',
+                 'state': 'active',
+                 'failed_login_count': 5,
+                 'permissions': {},
+                 'password_changed_at': str(datetime.now() + timedelta(minutes=1))
                  }
     user = User(user_data)
     return user
@@ -345,6 +363,13 @@ def mock_get_user_by_email_request_password_reset(mocker, api_user_request_passw
         return_value=api_user_request_password_reset)
 
 
+@pytest.fixture(scope='function')
+def mock_get_user_by_email_user_changed_password(mocker, api_user_changed_password):
+    return mocker.patch(
+        'app.user_api_client.get_user_by_email',
+        return_value=api_user_changed_password)
+
+
 @pytest.fixture(scope='function')
 def mock_get_user_by_email_locked(mocker, api_user_locked):
     return mocker.patch(
@@ -382,14 +407,6 @@ def mock_verify_password(mocker):
         side_effect=_verify_password)
 
 
-@pytest.fixture(scope='function')
-def mock_password_reset(mocker, api_user_active):
-
-    def _reset(email):
-        api_user_active.state = 'request_password_reset'
-    return mocker.patch('app.main.dao.users_dao.request_password_reset', side_effect=_reset)
-
-
 @pytest.fixture(scope='function')
 def mock_update_user(mocker):
 
@@ -645,3 +662,8 @@ def mock_add_user_to_service(mocker, service_one, api_user_active):
 @pytest.fixture(scope='function')
 def mock_set_user_permissions(mocker):
     return mocker.patch('app.user_api_client.set_user_permissions', return_value=None)
+
+
+@pytest.fixture(scope='function')
+def mock_reset_user_password(mocker):
+    return mocker.patch('app.user_api_client.send_reset_password_url', return_value=None)