Merge pull request #1162 from alphagov/fix-user-login-flow

Fix user login flow

app/main/views/new_password.py

@@ -30,6 +30,7 @@ def new_password(token):
 
     if form.validate_on_submit():
         user_api_client.send_verify_code(user.id, 'sms', user.mobile_number)
+        user_api_client.reset_failed_login_count(user.id)
         session['user_details'] = {
             'id': user.id,
             'email': user.email_address,

app/main/views/two_factor.py

@@ -31,10 +31,7 @@ def two_factor():
             services = service_api_client.get_active_services({'user_id': str(user_id)}).get('data', [])
             # Check if coming from new password page
             if 'password' in session['user_details']:
-                user_api_client.update_password(user.id, password=session['user_details']['password'])
-            if user.is_locked():
-                form.sms_code.errors.append('Code not found')
-                return render_template('views/two-factor.html', form=form)
+                user = user_api_client.update_password(user.id, password=session['user_details']['password'])
             activated_user = user_api_client.activate_user(user)
             login_user(activated_user, remember=True)
         finally:

app/notify_client/user_api_client.py

@@ -75,6 +75,11 @@ class UserApiClient(NotifyAdminAPIClient):
         user_data = self.post(url, data=data)
         return User(user_data['data'], max_failed_login_count=self.max_failed_login_count)
 
+    def reset_failed_login_count(self, user_id):
+        url = "/user/{}/reset-failed-login-count".format(user_id)
+        user_data = self.post(url, data={})
+        return User(user_data['data'], max_failed_login_count=self.max_failed_login_count)
+
     def update_password(self, user_id, password):
         data = {"_password": password}
         url = "/user/{}/update-password".format(user_id)

tests/app/main/views/test_new_password.py

@@ -38,6 +38,7 @@ def test_should_redirect_to_two_factor_when_password_reset_is_successful(
     mock_get_user_by_email_request_password_reset,
     mock_login,
     mock_send_verify_code,
+    mock_reset_failed_login_count
 ):
     user = mock_get_user_by_email_request_password_reset.return_value
     data = json.dumps({'email': user.email_address, 'created_at': str(datetime.utcnow())})
@@ -54,6 +55,7 @@ def test_should_redirect_index_if_user_has_already_changed_password(
     mock_get_user_by_email_user_changed_password,
     mock_login,
     mock_send_verify_code,
+    mock_reset_failed_login_count
 ):
     user = mock_get_user_by_email_user_changed_password.return_value
     data = json.dumps({'email': user.email_address, 'created_at': str(datetime.utcnow())})

tests/app/main/views/test_two_factor.py

@@ -187,7 +187,7 @@ def test_two_factor_returns_error_when_user_is_locked(
     client,
     api_user_locked,
     mock_get_locked_user,
-    mock_check_verify_code,
+    mock_check_verify_code_code_not_found,
     mock_get_services_with_one_service
 ):
     with client.session_transaction() as session:

tests/conftest.py

@@ -1377,6 +1377,11 @@ def mock_update_whitelist(mocker):
     )
 
 
+@pytest.fixture(scope='function')
+def mock_reset_failed_login_count(mocker):
+    return mocker.patch('app.user_api_client.reset_failed_login_count')
+
+
 @pytest.fixture(scope='function')
 def client(app_):
     with app_.test_request_context(), app_.test_client() as client: