From b394a18b4e64ca3e7f0ec3f7a689adef412dc977 Mon Sep 17 00:00:00 2001
From: Adam Shimali <adam.shimali@gmail.com>
Date: Tue, 26 Jan 2016 12:32:08 +0000
Subject: [PATCH] Incrementing of failed logins happens on api side

---
 app/main/dao/users_dao.py            |  5 -----
 app/main/views/sign_in.py            | 31 +++++++++++++++++++---------
 tests/app/main/dao/test_users_dao.py |  6 ------
 3 files changed, 21 insertions(+), 21 deletions(-)

diff --git a/app/main/dao/users_dao.py b/app/main/dao/users_dao.py
index 19477fdbc..2bfed65d5 100644
--- a/app/main/dao/users_dao.py
+++ b/app/main/dao/users_dao.py
@@ -38,11 +38,6 @@ def verify_password(user, password):
     return user_api_client.verify_password(user, password)
 
 
-def increment_failed_login_count(id):
-    user = get_user_by_id(id)
-    user.failed_login_count += 1
-
-
 def activate_user(user):
     user.state = 'active'
     return user_api_client.update_user(user)
diff --git a/app/main/views/sign_in.py b/app/main/views/sign_in.py
index 35469d1b1..aa8f6834c 100644
--- a/app/main/views/sign_in.py
+++ b/app/main/views/sign_in.py
@@ -23,20 +23,31 @@ def sign_in():
     try:
         form = LoginForm()
         if form.validate_on_submit():
-            user = users_dao.get_user_by_email(form.email_address.data)
+            user = _get_and_verify_user(form.email_address.data, form.password.data)
             if user:
-                if not user.is_locked() and user.is_active() and users_dao.verify_password(user, form.password.data):
-                    send_sms_code(user.id, user.mobile_number)
-                    session['user_email'] = user.email_address
-                    return redirect(url_for('.two_factor'))
-                else:
-                    # TODO re wire this increment to api
-                    users_dao.increment_failed_login_count(user.id)
-            # Vague error message for login
-            form.password.errors.append('Username or password is incorrect')
+                send_sms_code(user.id, user.mobile_number)
+                session['user_email'] = user.email_address
+                return redirect(url_for('.two_factor'))
+            else:
+                # Vague error message for login in case of user not known, locked, inactive or password not verified
+                form.password.errors.append('Username or password is incorrect')
 
         return render_template('views/signin.html', form=form)
     except:
         import traceback
         traceback.print_exc()
         abort(500)
+
+
+def _get_and_verify_user(email_address, password):
+    user = users_dao.get_user_by_email(email_address)
+    if not user:
+        return None
+    elif user.is_locked():
+        return None
+    elif not user.is_active():
+        return None
+    elif not users_dao.verify_password(user, password):
+        return None
+    else:
+        return user
diff --git a/tests/app/main/dao/test_users_dao.py b/tests/app/main/dao/test_users_dao.py
index 5670b9e07..7cb994590 100644
--- a/tests/app/main/dao/test_users_dao.py
+++ b/tests/app/main/dao/test_users_dao.py
@@ -41,12 +41,6 @@ def test_get_all_users_calls_api(db_, db_session, mock_get_all_users_from_api):
     assert mock_get_all_users_from_api.called
 
 
-def test_increment_failed_login_count_should_increade_count_by_1(db_, db_session, mock_active_user, mock_get_user):
-    assert mock_active_user.failed_login_count == 0
-    users_dao.increment_failed_login_count(mock_active_user.id)
-    assert mock_active_user.failed_login_count == 1
-
-
 def test_user_is_active_is_false_if_state_is_inactive(db_, db_session, mock_active_user):
     assert mock_active_user.is_active() is True
     mock_active_user.state = 'inactive'