Merge pull request #506 from alphagov/reset_failed_login_count

Failed login count set to zero with password reset.

app/main/views/sign_in.py

@@ -5,7 +5,8 @@ from flask import (
     session,
     flash,
     request,
-    abort
+    abort,
+    Markup
 )
 
 from flask.ext.login import (
@@ -69,7 +70,11 @@ def sign_in():
                 else:
                     return redirect(url_for('.two_factor'))
         # Vague error message for login in case of user not known, locked, inactive or password not verified
-        flash('Username or password is incorrect')
+        flash(Markup((
+            "The email address or password you entered is incorrect."
+            " <a href={password_reset}>Forgot your password</a>?"
+            ).format(password_reset=url_for('.forgot_password'))
+        ))
 
     return render_template('views/signin.html', form=form)
 

app/main/views/two_factor.py

@@ -34,6 +34,7 @@ def two_factor():
             # Check if coming from new password page
             if 'password' in session['user_details']:
                 user.set_password(session['user_details']['password'])
+                user.reset_failed_login_count()
                 user_api_client.update_user(user)
             login_user(user, remember=True)
         finally:

app/notify_client/models.py

@@ -108,6 +108,9 @@ class User(UserMixin):
     def failed_login_count(self, num):
         self._failed_login_count += num
 
+    def reset_failed_login_count(self):
+        self._failed_login_count = 0
+
     def is_locked(self):
         return self.failed_login_count >= self.max_failed_login_count
 

tests/app/main/views/test_sign_in.py

@@ -51,7 +51,7 @@ def test_should_return_locked_out_true_when_user_is_locked(app_,
                 'email_address': 'valid@example.gov.uk',
                 'password': 'whatIsMyPassword!'})
         assert resp.status_code == 200
-        assert 'Username or password is incorrect' in resp.get_data(as_text=True)
+        assert 'The email address or password you entered is incorrect' in resp.get_data(as_text=True)
 
 
 def test_should_return_200_when_user_does_not_exist(app_, mock_get_user_by_email_not_found):
@@ -61,7 +61,7 @@ def test_should_return_200_when_user_does_not_exist(app_, mock_get_user_by_email
                 'email_address': 'notfound@gov.uk',
                 'password': 'doesNotExist!'})
     assert response.status_code == 200
-    assert 'Username or password is incorrect' in response.get_data(as_text=True)
+    assert 'The email address or password you entered is incorrect' in response.get_data(as_text=True)
 
 
 def test_should_return_redirect_when_user_is_pending(app_,

tests/app/main/views/test_two_factor.py

@@ -177,3 +177,31 @@ def test_two_factor_should_set_password_when_new_password_exists_in_session(app_
             )
             api_user_active.password = 'changedpassword'
             mock_update_user.assert_called_once_with(api_user_active)
+
+
+def test_two_factor_reset_login_count_called(app_,
+                                             api_user_locked,
+                                             mock_get_locked_user,
+                                             mock_update_user,
+                                             mock_check_verify_code,
+                                             mock_get_services_with_one_service):
+    with app_.test_request_context():
+        with app_.test_client() as client:
+            with client.session_transaction() as session:
+                new_password = "1234567890"
+                session['user_details'] = {
+                    'id': api_user_locked.id,
+                    'email': api_user_locked.email_address,
+                    'password': new_password
+                }
+            response = client.post(url_for('main.two_factor'),
+                                   data={'sms_code': '12345'})
+            assert response.status_code == 302
+            assert response.location == url_for(
+                'main.service_dashboard',
+                service_id=SERVICE_ONE_ID,
+                _external=True
+            )
+            api_user_locked.reset_failed_login_count()
+            api_user_locked.password = new_password
+            mock_update_user.assert_called_with(api_user_locked)

tests/conftest.py

@@ -406,14 +406,22 @@ def mock_register_user(mocker, api_user_pending):
 
 
 @pytest.fixture(scope='function')
-def mock_get_user(mocker, api_user_active):
+def mock_get_user(mocker, user=None):
+    if user is None:
+        user = api_user_active(fake_uuid())
+
     def _get_user(id_):
-        api_user_active.id = id_
+        user.id = id_
-        return api_user_active
+        return user
     return mocker.patch(
         'app.user_api_client.get_user', side_effect=_get_user)
 
 
+@pytest.fixture(scope='function')
+def mock_get_locked_user(mocker, api_user_locked):
+    return mock_get_user(mocker, user=api_user_locked)
+
+
 @pytest.fixture(scope='function')
 def mock_get_user_locked(mocker, api_user_locked):
     return mocker.patch(
@@ -427,14 +435,21 @@ def mock_get_user_pending(mocker, api_user_pending):
 
 
 @pytest.fixture(scope='function')
-def mock_get_user_by_email(mocker, api_user_active):
+def mock_get_user_by_email(mocker, user=None):
+    if user is None:
+        user = api_user_active(fake_uuid())
 
     def _get_user(email_address):
-        api_user_active._email_address = email_address
+        user._email_address = email_address
-        return api_user_active
+        return user
     return mocker.patch('app.user_api_client.get_user_by_email', side_effect=_get_user)
 
 
+@pytest.fixture(scope='function')
+def mock_get_locked_user_by_email(mocker, api_user_locked):
+    return mock_get_user_by_email(mocker, user=api_user_locked)
+
+
 @pytest.fixture(scope='function')
 def mock_get_user_with_permissions(mocker, api_user_active):
     def _get_user(id):