update_user now resets failed_login_count if password is changed

until work is done to stop using PUT /user/{id} on the admin app, this
function also needs to reset failed logins, cos it's used during the
forgotten password flow

app/dao/users_dao.py

@@ -114,7 +114,6 @@ def reset_failed_login_count(user):
 
 def update_user_password(user, password):
     # reset failed login count - they've just reset their password so should be fine
-    user.failed_login_count = 0
     user.password = password
     user.password_changed_at = datetime.utcnow()
     db.session.add(user)

app/user/rest.py

@@ -59,12 +59,14 @@ def update_user(user_id):
     user_to_update = get_user_by_id(user_id=user_id)
     req_json = request.get_json()
     update_dct, errors = user_schema_load_json.load(req_json)
+    # TODO don't let password be updated in this PUT method (currently used by the forgot password flow)
     pwd = req_json.get('password', None)
-    # TODO password validation, it is already done on the admin app
-    # but would be good to have the same validation here.
-    if pwd is not None and not pwd:
-        errors.update({'password': ['Invalid data for field']})
-        raise InvalidRequest(errors, status_code=400)
+    if pwd is not None:
+        if not pwd:
+            errors.update({'password': ['Invalid data for field']})
+            raise InvalidRequest(errors, status_code=400)
+        else:
+            reset_failed_login_count(user_to_update)
     save_model_user(user_to_update, update_dict=update_dct, pwd=pwd)
     return jsonify(data=user_schema.dump(user_to_update).data), 200
 
@@ -324,6 +326,7 @@ def update_password(user_id):
     if errors:
         raise InvalidRequest(errors, status_code=400)
 
+    reset_failed_login_count(user)
     update_user_password(user, pwd)
     return jsonify(data=user_schema.dump(user).data), 200
 

tests/app/user/test_rest.py

@@ -153,6 +153,7 @@ def test_put_user(notify_api, notify_db, notify_db_session, sample_service):
         with notify_api.test_client() as client:
             assert User.query.count() == 1
             sample_user = sample_service.users[0]
+            sample_user.failed_login_count = 1
             new_email = 'new@digital.cabinet-office.gov.uk'
             data = {
                 'name': sample_user.name,
@@ -178,6 +179,8 @@ def test_put_user(notify_api, notify_db, notify_db_session, sample_service):
             assert new_email == fetched['email_address']
             assert sample_user.state == fetched['state']
             assert sorted(expected_permissions) == sorted(fetched['permissions'][str(sample_service.id)])
+            # password wasn't updated, so failed_login_count stays the same
+            assert sample_user.failed_login_count == 1
 
 
 @pytest.mark.parametrize('user_attribute, user_value', [
@@ -572,3 +575,22 @@ def test_update_user_password_resets_failed_login_count(client, sample_service):
 
     assert resp.status_code == 200
     assert user.failed_login_count == 0
+
+
+def test_update_user_resets_failed_login_count_if_updating_password(client, sample_service):
+    user = sample_service.users[0]
+    user.failed_login_count = 1
+
+    resp = client.put(
+        url_for('user.update_user', user_id=user.id),
+        data=json.dumps({
+            'name': user.name,
+            'email_address': user.email_address,
+            'mobile_number': user.mobile_number,
+            'password': 'foo'
+        }),
+        headers=[('Content-Type', 'application/json'), create_authorization_header()]
+    )
+
+    assert resp.status_code == 200
+    assert user.failed_login_count == 0