From 6c8bfdc5b01e4b3221c168076f4fc61df2ff513e Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Fri, 19 Mar 2021 14:11:37 +0000 Subject: [PATCH] Refactor failed login count MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We don’t vary this between different environments so it doesn’t need to be in the config. I was trying to look up what this value was and found it a bit confusing that it was spread across multiple places. --- app/config.py | 1 - app/models/user.py | 7 ++++--- tests/app/models/test_user.py | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/app/config.py b/app/config.py index b881a06c5..8c126547a 100644 --- a/app/config.py +++ b/app/config.py @@ -71,7 +71,6 @@ class Config(object): EMAIL_2FA_EXPIRY_SECONDS = 1800 # 30 Minutes HEADER_COLOUR = '#81878b' # mix(govuk-colour("dark-grey"), govuk-colour("mid-grey")) HTTP_PROTOCOL = 'http' - MAX_FAILED_LOGIN_COUNT = 10 NOTIFY_APP_NAME = 'admin' NOTIFY_LOG_LEVEL = 'DEBUG' PERMANENT_SESSION_LIFETIME = 20 * 60 * 60 # 20 hours diff --git a/app/models/user.py b/app/models/user.py index 8335cdc4d..831465ec7 100644 --- a/app/models/user.py +++ b/app/models/user.py @@ -1,4 +1,4 @@ -from flask import abort, current_app, request, session +from flask import abort, request, session from flask_login import AnonymousUserMixin, UserMixin, login_user, logout_user from notifications_python_client.errors import HTTPError from notifications_utils.timezones import utc_string_to_aware_gmt_datetime @@ -27,6 +27,8 @@ def _get_org_id_from_view_args(): class User(JSONModel, UserMixin): + MAX_FAILED_LOGIN_COUNT = 10 + ALLOWED_PROPERTIES = { 'id', 'name', @@ -45,7 +47,6 @@ class User(JSONModel, UserMixin): def __init__(self, _dict): super().__init__(_dict) self.permissions = _dict.get('permissions', {}) - self.max_failed_login_count = current_app.config['MAX_FAILED_LOGIN_COUNT'] self._platform_admin = _dict['platform_admin'] @classmethod @@ -261,7 +262,7 @@ class User(JSONModel, UserMixin): @property def locked(self): - return self.failed_login_count >= self.max_failed_login_count + return self.failed_login_count >= self.MAX_FAILED_LOGIN_COUNT @property def email_domain(self): diff --git a/tests/app/models/test_user.py b/tests/app/models/test_user.py index ec7224e34..f01cb8551 100644 --- a/tests/app/models/test_user.py +++ b/tests/app/models/test_user.py @@ -33,12 +33,12 @@ def test_user(app_): assert user.state == 'pending' # user has ten failed logins before being locked - assert user.max_failed_login_count == app_.config['MAX_FAILED_LOGIN_COUNT'] == 10 + assert user.MAX_FAILED_LOGIN_COUNT == 10 assert user.failed_login_count == 0 assert user.locked is False # set failed logins to threshold - user.failed_login_count = app_.config['MAX_FAILED_LOGIN_COUNT'] + user.failed_login_count = 10 assert user.locked is True with pytest.raises(TypeError):