From f40e4a7567d05c774c0854a3d603d3f52b6e1185 Mon Sep 17 00:00:00 2001 From: Christian Schudoma Date: Fri, 29 Oct 2021 16:51:50 +0200 Subject: [PATCH 01/32] starting to address issue #478 --- .../alembic/versions/20211029_98911b881230.py | 37 +++++++++++++++++++ datameta/models/__init__.py | 3 +- datameta/models/db.py | 11 ++++++ datameta/security/__init__.py | 35 ++++++++++++++++-- 4 files changed, 81 insertions(+), 5 deletions(-) create mode 100644 datameta/alembic/versions/20211029_98911b881230.py diff --git a/datameta/alembic/versions/20211029_98911b881230.py b/datameta/alembic/versions/20211029_98911b881230.py new file mode 100644 index 00000000..77f705ce --- /dev/null +++ b/datameta/alembic/versions/20211029_98911b881230.py @@ -0,0 +1,37 @@ +"""added failed_login tracking + +Revision ID: 98911b881230 +Revises: 7fdc829db18d +Create Date: 2021-10-29 15:01:29.844727 + +""" +from alembic import op +import sqlalchemy as sa +from sqlalchemy.dialects import postgresql + +# revision identifiers, used by Alembic. +revision = '98911b881230' +down_revision = '7fdc829db18d' +branch_labels = None +depends_on = None + + +def upgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.create_table( + 'loginattempts', + sa.Column('id', sa.Integer(), nullable=False), + sa.Column('uuid', postgresql.UUID(as_uuid=True), nullable=False), + sa.Column('user_id', sa.Integer(), nullable=False), + sa.Column('timestamp', sa.DateTime(), nullable=False), + sa.ForeignKeyConstraint(['user_id'], ['users.id'], name=op.f('fk_loginattempts_user_id_users')), + sa.PrimaryKeyConstraint('id', name=op.f('pk_loginattempts')), + sa.UniqueConstraint('uuid', name=op.f('uq_loginattempts_uuid')) + ) + # ### end Alembic commands ### + + +def downgrade(): + # ### commands auto generated by Alembic - please adjust! ### + op.drop_table('loginattempts') + # ### end Alembic commands ### diff --git a/datameta/models/__init__.py b/datameta/models/__init__.py index cf3ba155..57db7a37 100644 --- a/datameta/models/__init__.py +++ b/datameta/models/__init__.py @@ -35,7 +35,8 @@ ApiKey, DownloadToken, Service, - ServiceExecution + ServiceExecution, + LoginAttempt ) # run configure_mappers after defining all of the models to ensure diff --git a/datameta/models/db.py b/datameta/models/db.py index 935f9d18..4c4c18dc 100644 --- a/datameta/models/db.py +++ b/datameta/models/db.py @@ -90,6 +90,17 @@ class User(Base): apikeys = relationship('ApiKey', back_populates='user') services = relationship('Service', secondary=user_service_table, back_populates='users') service_executions = relationship('ServiceExecution', back_populates='user') + login_attempts = relationship("LoginAttempt", back_populates='user') + + +class LoginAttempt(Base): + __tablename__ = 'loginattempts' + id = Column(Integer, primary_key=True) + uuid = Column(UUID(as_uuid=True), unique=True, default=uuid.uuid4, nullable=False) + user_id = Column(Integer, ForeignKey('users.id'), nullable=False) + timestamp = Column(DateTime, nullable=False) + # Relationships + user = relationship('User', back_populates='login_attempts') class ApiKey(Base): diff --git a/datameta/security/__init__.py b/datameta/security/__init__.py index da22fa98..9fce43f3 100644 --- a/datameta/security/__init__.py +++ b/datameta/security/__init__.py @@ -19,7 +19,7 @@ from string import ascii_letters, digits from sqlalchemy import and_ -from ..models import User, ApiKey, PasswordToken, Session +from ..models import User, ApiKey, PasswordToken, Session, LoginAttempt import bcrypt import hashlib @@ -103,16 +103,43 @@ def check_password_by_hash(pw, hashed_pw): def hash_token(token): """Hash a token and return the unsalted hash""" - hashed_token = hashlib.sha256(token.encode('utf-8')).hexdigest() + hashed_token = hashlib.sha256(token.encode('utf-8')).hexdigest() return hashed_token +def register_failed_login_attempt(db, user): + """ Registers a failed login attempt and disables user if this has happened too often in the last hour.""" + BLOCK_AFTER_N_FAILED_LOGINS = 5 + + now = datetime.now() + last_hour = now - timedelta(hours=1) + db.add(LoginAttempt(user_id=user.id, timestamp=now)) + + failed_logins = sum( + 1 + for attempt in db.query(LoginAttempt).filter(LoginAttempt.user_id==user.id).all() + if attempt.timestamp - last_hour <= timedelta(hours=1) + ) + + log.debug(f"FAILED LOGINS {failed_logins}") + if failed_logins >= BLOCK_AFTER_N_FAILED_LOGINS: + log.debug("BLOCKED!") + user.enabled = False + + return None + + def get_user_by_credentials(request, email: str, password: str): """Check a combination of email and password, returns a user object if valid""" + db = request.dbsession user = db.query(User).filter(and_(User.email == email, User.enabled.is_(True))).one_or_none() - if user and check_password_by_hash(password, user.pwhash): - return user + if user: + if check_password_by_hash(password, user.pwhash): + return user + + register_failed_login_attempt(db, user) + return None From b58787d9ea16caca9cd2968cc88a4d99b413d20e Mon Sep 17 00:00:00 2001 From: Christian Schudoma Date: Fri, 29 Oct 2021 16:52:01 +0200 Subject: [PATCH 02/32] version bump -> 1.1.0 --- setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.py b/setup.py index 91ab6366..e2a8bd83 100644 --- a/setup.py +++ b/setup.py @@ -60,7 +60,7 @@ setup( name = 'datameta', - version = '1.0.5', + version = '1.1.0', description = 'DataMeta - submission server for data and associated metadata', long_description = README + '\n\n' + CHANGES, author = 'Leon Kuchenbecker', From 0ead5ef954a9e84a8d399cb9ef679b81e0adb976 Mon Sep 17 00:00:00 2001 From: Christian Schudoma Date: Fri, 29 Oct 2021 16:53:01 +0200 Subject: [PATCH 03/32] ConnectionRefusedError are now caught when trying to send email --- datameta/email/__init__.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/datameta/email/__init__.py b/datameta/email/__init__.py index 5665bc86..d09bba17 100644 --- a/datameta/email/__init__.py +++ b/datameta/email/__init__.py @@ -42,4 +42,7 @@ def send(recipients, subject, template, values, bcc=None, rec_header_only=False) message = template.format(**values) # Send the message - __smtp.sendMessage(__smtp_from, recipients, subject, message, bcc=bcc, rec_header_only=rec_header_only) + try: + __smtp.sendMessage(__smtp_from, recipients, subject, message, bcc=bcc, rec_header_only=rec_header_only) + except ConnectionRefusedError: + pass From 4af403f56374238b6c069b18b8ae1b67432acf3c Mon Sep 17 00:00:00 2001 From: Christian Schudoma Date: Fri, 29 Oct 2021 16:53:15 +0200 Subject: [PATCH 04/32] minor fix --- datameta/defaults/appsettings.yaml | 1 - 1 file changed, 1 deletion(-) diff --git a/datameta/defaults/appsettings.yaml b/datameta/defaults/appsettings.yaml index 1cc3af7b..5eb77a78 100644 --- a/datameta/defaults/appsettings.yaml +++ b/datameta/defaults/appsettings.yaml @@ -41,7 +41,6 @@ template_welcome_token: Best regards, The Support Team - }, subject_reject: str_value : Your registration request was rejected From 8b74fcf4211bcf72d55e9a189f29e8de54125cef Mon Sep 17 00:00:00 2001 From: Christian Schudoma Date: Fri, 29 Oct 2021 17:18:50 +0200 Subject: [PATCH 05/32] pleasing flake8 --- datameta/security/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/datameta/security/__init__.py b/datameta/security/__init__.py index 9fce43f3..2bbe0cb0 100644 --- a/datameta/security/__init__.py +++ b/datameta/security/__init__.py @@ -117,7 +117,7 @@ def register_failed_login_attempt(db, user): failed_logins = sum( 1 - for attempt in db.query(LoginAttempt).filter(LoginAttempt.user_id==user.id).all() + for attempt in db.query(LoginAttempt).filter(LoginAttempt.user_id == user.id).all() if attempt.timestamp - last_hour <= timedelta(hours=1) ) From d7af75e8c7396f15a791f159e92ce4fdf262cdc1 Mon Sep 17 00:00:00 2001 From: Christian Schudoma Date: Thu, 13 Jan 2022 18:16:39 +0100 Subject: [PATCH 06/32] implemented suggestions from code review * added informative log warnings when login attempt fails and/or user is blocked * moved maximum number of allowed failed login attempts to appsettings:security_max_failed_logins * failed login attempts are now cleared when user logs in successfully --- datameta/defaults/appsettings.yaml | 3 +++ datameta/security/__init__.py | 24 +++++++++++++++--------- 2 files changed, 18 insertions(+), 9 deletions(-) diff --git a/datameta/defaults/appsettings.yaml b/datameta/defaults/appsettings.yaml index 5eb77a78..7c106113 100644 --- a/datameta/defaults/appsettings.yaml +++ b/datameta/defaults/appsettings.yaml @@ -84,3 +84,6 @@ logo_html: user_agreement: str_value: "" + +security_max_failed_logins: + int_value: 5 \ No newline at end of file diff --git a/datameta/security/__init__.py b/datameta/security/__init__.py index 2bbe0cb0..852c2fce 100644 --- a/datameta/security/__init__.py +++ b/datameta/security/__init__.py @@ -20,6 +20,7 @@ from sqlalchemy import and_ from ..models import User, ApiKey, PasswordToken, Session, LoginAttempt +from ..settings import get_setting import bcrypt import hashlib @@ -109,33 +110,38 @@ def hash_token(token): def register_failed_login_attempt(db, user): """ Registers a failed login attempt and disables user if this has happened too often in the last hour.""" - BLOCK_AFTER_N_FAILED_LOGINS = 5 - now = datetime.now() - last_hour = now - timedelta(hours=1) + now = datetime.utcnow() + max_allowed_failed_logins = get_setting(db, "security_max_failed_logins") db.add(LoginAttempt(user_id=user.id, timestamp=now)) - failed_logins = sum( - 1 + n_failed_logins = sum( + now - attempt.timestamp <= timedelta(hours=1) for attempt in db.query(LoginAttempt).filter(LoginAttempt.user_id == user.id).all() - if attempt.timestamp - last_hour <= timedelta(hours=1) ) - log.debug(f"FAILED LOGINS {failed_logins}") - if failed_logins >= BLOCK_AFTER_N_FAILED_LOGINS: - log.debug("BLOCKED!") + log.warning(f"FAILED LOGIN ATTEMPT USER id={user.id} n={n_failed_logins} within one hour.") + if n_failed_logins >= max_allowed_failed_logins: + log.warning(f"BLOCKED USER id={user.id} reason={n_failed_logins} failed login attempts within one hour.") user.enabled = False return None def get_user_by_credentials(request, email: str, password: str): + """Check a combination of email and password, returns a user object if valid""" db = request.dbsession user = db.query(User).filter(and_(User.email == email, User.enabled.is_(True))).one_or_none() if user: if check_password_by_hash(password, user.pwhash): + + failed_attempts = db.query(LoginAttempt).join(User).filter(and_( + User.id == user.id, + User.enabled.is_(True))).all() + + [db.delete(attempt) for attempt in failed_attempts] return user register_failed_login_attempt(db, user) From 5d9c8e4d4174e402c7c4d3979815576010a9efad Mon Sep 17 00:00:00 2001 From: Christian Schudoma Date: Fri, 14 Jan 2022 16:30:33 +0100 Subject: [PATCH 07/32] added get_db() to FixtureManager in order to be able to check database from a test --- tests/integration/fixtures/__init__.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/integration/fixtures/__init__.py b/tests/integration/fixtures/__init__.py index c9e02435..d8df4ac3 100644 --- a/tests/integration/fixtures/__init__.py +++ b/tests/integration/fixtures/__init__.py @@ -58,6 +58,10 @@ def __init__(self, session_factory, storage_path): self.session_factory = session_factory self.storage_path = storage_path + def get_db(self): + with transaction.manager: + return get_tm_session(self.session_factory, transaction.manager) + def _create_fixture(self, db, fixture_set: str, fixture_name: str, fixture: dict, database_insert=True): """Creates a fixture based on the provided dictionary""" models = import_module('datameta.models') From a1bc4da53b446215925064967c8c93b0493540ce Mon Sep 17 00:00:00 2001 From: Christian Schudoma Date: Fri, 14 Jan 2022 16:33:23 +0100 Subject: [PATCH 08/32] added db-flush after dealing with invalid login attempt, renamed security_max_failed_login_attempts setting --- datameta/defaults/appsettings.yaml | 4 ++-- datameta/security/__init__.py | 5 +++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/datameta/defaults/appsettings.yaml b/datameta/defaults/appsettings.yaml index 7c106113..8c2a1c8d 100644 --- a/datameta/defaults/appsettings.yaml +++ b/datameta/defaults/appsettings.yaml @@ -85,5 +85,5 @@ user_agreement: str_value: "" -security_max_failed_logins: - int_value: 5 \ No newline at end of file +security_max_failed_login_attempts: + int_value: 5 diff --git a/datameta/security/__init__.py b/datameta/security/__init__.py index 852c2fce..3e153898 100644 --- a/datameta/security/__init__.py +++ b/datameta/security/__init__.py @@ -112,7 +112,7 @@ def register_failed_login_attempt(db, user): """ Registers a failed login attempt and disables user if this has happened too often in the last hour.""" now = datetime.utcnow() - max_allowed_failed_logins = get_setting(db, "security_max_failed_logins") + max_allowed_failed_logins = get_setting(db, "security_max_failed_login_attempts") db.add(LoginAttempt(user_id=user.id, timestamp=now)) n_failed_logins = sum( @@ -122,8 +122,9 @@ def register_failed_login_attempt(db, user): log.warning(f"FAILED LOGIN ATTEMPT USER id={user.id} n={n_failed_logins} within one hour.") if n_failed_logins >= max_allowed_failed_logins: - log.warning(f"BLOCKED USER id={user.id} reason={n_failed_logins} failed login attempts within one hour.") user.enabled = False + log.warning(f"BLOCKED USER id={user.id} enabled={user.enabled} reason={n_failed_logins} failed login attempts within one hour.") + db.flush() return None From d9a13271ade3e2e5d0946c32b4208f405036d1f6 Mon Sep 17 00:00:00 2001 From: Christian Schudoma Date: Fri, 14 Jan 2022 16:33:59 +0100 Subject: [PATCH 09/32] added login_tracking integration test --- tests/integration/test_login_tracking.py | 56 ++++++++++++++++++++++++ 1 file changed, 56 insertions(+) create mode 100644 tests/integration/test_login_tracking.py diff --git a/tests/integration/test_login_tracking.py b/tests/integration/test_login_tracking.py new file mode 100644 index 00000000..11aab4a7 --- /dev/null +++ b/tests/integration/test_login_tracking.py @@ -0,0 +1,56 @@ +# Copyright 2021 Universität Tübingen, DKFZ and EMBL for the German Human Genome-Phenome Archive (GHGA) +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# https://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +"""Testing tracking of failed login attempts +""" +from parameterized import parameterized + +from . import BaseIntegrationTest + +from datameta.models import User +from datameta.settings import get_setting + + +class TestLoginTracking(BaseIntegrationTest): + + def setUp(self): + super().setUp() + self.fixture_manager.load_fixtureset('groups') + self.fixture_manager.load_fixtureset('users') + + @parameterized.expand([ + # TEST_NAME EXEC_USER SUCCESS? EXPECTED_OUTCOME + ("login_fail_with_block" , "user_a", False , (302, "login", False)), + ("login_success_after_n-1", "user_a", True , (302, "home", True)), + ]) + def test_login_tracking(self, _, executing_user: str, success: bool, expected_outcome): + + user = self.fixture_manager.get_fixture('users', executing_user) + n_attempts = get_setting(self.fixture_manager.get_db(), "security_max_failed_login_attempts") + + for attempt in range(1, n_attempts + 1): + + form = self.testapp.get("/login").forms[0] + form["input_email"] = user.email + form["input_password"] = user.password + ("_somenonsense" if attempt < n_attempts or not success else "") + response = form.submit("form.submitted") + + form = self.testapp.get("/login").forms[0] + form["input_email"] = user.email + form["input_password"] = user.password + response = form.submit("form.submitted") + + user_db = self.fixture_manager.get_db().query(User).filter(User.email == user.email).one_or_none() + + assert (response.status_int, response.location.split("/")[-1], user_db.enabled) == expected_outcome From a57fe632c694f929632af0e2ce7493663e5e6089 Mon Sep 17 00:00:00 2001 From: Christian Schudoma Date: Wed, 2 Feb 2022 10:57:00 +0100 Subject: [PATCH 10/32] refactored appsettings:put -> settings.set_setting --- datameta/api/appsettings.py | 41 +++-------------------------- datameta/settings.py | 52 +++++++++++++++++++++++++++++++++++-- 2 files changed, 54 insertions(+), 39 deletions(-) diff --git a/datameta/api/appsettings.py b/datameta/api/appsettings.py index d22aff01..4e926785 100644 --- a/datameta/api/appsettings.py +++ b/datameta/api/appsettings.py @@ -23,7 +23,7 @@ from .. import resource, security, errors from ..security import authz from ..resource import resource_by_id -from ..settings import get_setting_value_type +from ..settings import get_setting_value_type, set_setting import datetime @@ -89,43 +89,10 @@ def put(request: Request): raise HTTPForbidden() target_setting = resource_by_id(db, ApplicationSetting, settings_id) - - temp, value_type = get_setting_value_type(target_setting) value = request.openapi_validated.body["value"] - if value_type == 'int': - try: - value_int = int(value) - except ValueError: - raise errors.get_validation_error(["You have to provide an integer."]) - - target_setting.int_value = value_int - - elif value_type == 'string': - target_setting.str_value = value - - elif value_type == 'float': - try: - value_float = float(value) - except ValueError: - raise errors.get_validation_error(["You have to provide a float."]) - - target_setting.float_value = value_float - - elif value_type == 'date': - try: - datetime.datetime.strptime(value, "%Y-%m-%d") - except ValueError: - raise errors.get_validation_error(["The date value has to specified in the form '%Y-%m-%d'."]) - - target_setting.date_value = value - - elif value_type == 'time': - try: - datetime.datetime.strptime(value, "%H:%M:%S") - except ValueError: - raise errors.get_validation_error(["The time value has to specified in the form '%H:%M:%S'."]) - - target_setting.date_value = value + error = set_setting(db, target_setting.key, value) + if error is not None: + raise errors.get_validation_error([error]) return HTTPNoContent() diff --git a/datameta/settings.py b/datameta/settings.py index cc45f88c..639c0c07 100644 --- a/datameta/settings.py +++ b/datameta/settings.py @@ -12,11 +12,13 @@ # See the License for the specific language governing permissions and # limitations under the License. -import transaction -from .models import ApplicationSetting, get_engine, get_tm_session, get_session_factory +import datetime import pkgutil import yaml +import transaction +from .models import ApplicationSetting, get_engine, get_tm_session, get_session_factory + import logging log = logging.getLogger(__name__) @@ -65,6 +67,52 @@ def get_setting(db, name): return None +VALUE_CASTS = { + "int": { + "function": int, + "error_msg": "You have to provide an integer.", + "target": "int_value" + }, + "string": { + "function": lambda value: value, + "error_msg": "", + "target": "str_value" + }, + "float": { + "function": float, + "error_msg": "You have to provide an integer.", + "target": "float_value" + }, + "date": { + "function": lambda value: datetime.datetime.strptime(value, "%Y-%m-%d"), + "error_msg": "The date value has to specified in the form '%Y-%m-%d'.", + "target": "date_value" + }, + "time": { + "function": lambda value: datetime.datetime.strptime(value, "%H:%M:%S"), + "error_msg": "The time value has to specified in the form '%H:%M:%S'.", + "target": "date_value" + } +} + + +def set_setting(db, name, value): + target_setting = db.query(ApplicationSetting).filter(ApplicationSetting.key == name).one_or_none() + _, value_type = get_setting_value_type(target_setting) + + cast_params = VALUE_CASTS.get(value_type) + if cast_params is not None: + + try: + casted_value = cast_params["function"](value) + except ValueError: + return cast_params["error_msg"] + + setattr(target_setting, cast_params["target"], casted_value) + + return None + + def includeme(config): """Initializes the default values for applications settings""" # Load the app setting defaults from the packaged yaml file From 21e8474422ddbb2b8c8c9914d3bceefe9a5aa129 Mon Sep 17 00:00:00 2001 From: Christian Schudoma Date: Wed, 2 Feb 2022 10:58:28 +0100 Subject: [PATCH 11/32] revised revision message --- datameta/alembic/versions/20211029_98911b881230.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/datameta/alembic/versions/20211029_98911b881230.py b/datameta/alembic/versions/20211029_98911b881230.py index 77f705ce..6bb27cf3 100644 --- a/datameta/alembic/versions/20211029_98911b881230.py +++ b/datameta/alembic/versions/20211029_98911b881230.py @@ -1,4 +1,4 @@ -"""added failed_login tracking +"""added failed_login_attempt tracking Revision ID: 98911b881230 Revises: 7fdc829db18d From 0c2533afb3302bf2a4e48b9d0c8c54f7a4fefbcb Mon Sep 17 00:00:00 2001 From: Christian Schudoma Date: Wed, 2 Feb 2022 10:59:13 +0100 Subject: [PATCH 12/32] cosmetics --- datameta/settings.py | 1 + 1 file changed, 1 insertion(+) diff --git a/datameta/settings.py b/datameta/settings.py index 639c0c07..5caeb22e 100644 --- a/datameta/settings.py +++ b/datameta/settings.py @@ -101,6 +101,7 @@ def set_setting(db, name, value): _, value_type = get_setting_value_type(target_setting) cast_params = VALUE_CASTS.get(value_type) + if cast_params is not None: try: From 0f0f63680ae65eb04b20dc91d4f7c636a5707097 Mon Sep 17 00:00:00 2001 From: Christian Schudoma Date: Thu, 3 Feb 2022 19:27:12 +0100 Subject: [PATCH 13/32] fixed: date_value -> time_value --- datameta/settings.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/datameta/settings.py b/datameta/settings.py index 5caeb22e..97cacf05 100644 --- a/datameta/settings.py +++ b/datameta/settings.py @@ -91,12 +91,13 @@ def get_setting(db, name): "time": { "function": lambda value: datetime.datetime.strptime(value, "%H:%M:%S"), "error_msg": "The time value has to specified in the form '%H:%M:%S'.", - "target": "date_value" + "target": "time_value" # @lkuchenb: this was date_value in the original code - on purpose? } } def set_setting(db, name, value): + target_setting = db.query(ApplicationSetting).filter(ApplicationSetting.key == name).one_or_none() _, value_type = get_setting_value_type(target_setting) From 17ba9025dc69a518f742f87a5a8cd6b616a86f22 Mon Sep 17 00:00:00 2001 From: Christian Schudoma Date: Thu, 3 Feb 2022 19:29:42 +0100 Subject: [PATCH 14/32] added set_application_setting() function to BaseIntegrationTest --- tests/integration/__init__.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/tests/integration/__init__.py b/tests/integration/__init__.py index 40bf0376..9fc81da4 100644 --- a/tests/integration/__init__.py +++ b/tests/integration/__init__.py @@ -9,7 +9,8 @@ from datameta.models import ( get_engine, - get_session_factory + get_session_factory, + get_tm_session ) from datameta.models.meta import Base @@ -22,6 +23,8 @@ from .utils import get_auth_header +from datameta.settings import set_setting + class BaseIntegrationTest(unittest.TestCase): """Base TestCase to inherit from""" @@ -70,6 +73,11 @@ def tearDown(self): del self.testapp self.storage_path_obj.cleanup() + def set_application_setting(self, name, value): + with transaction.manager: + db = get_tm_session(self.session_factory, transaction.manager) + set_setting(db, name, value) + def apikey_auth(self, user: holders.UserFixture) -> dict: apikey = self.fixture_manager.get_fixture('apikeys', user.site_id) return get_auth_header(apikey.value_plain) From c7c454f3adac8a35891ac8a4690345f191ea136c Mon Sep 17 00:00:00 2001 From: Christian Schudoma Date: Thu, 3 Feb 2022 19:38:32 +0100 Subject: [PATCH 15/32] refactored test to use BaseIntegrationTest.set_application_setting() --- tests/integration/test_login_tracking.py | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/tests/integration/test_login_tracking.py b/tests/integration/test_login_tracking.py index 11aab4a7..6fd32092 100644 --- a/tests/integration/test_login_tracking.py +++ b/tests/integration/test_login_tracking.py @@ -18,9 +18,6 @@ from . import BaseIntegrationTest -from datameta.models import User -from datameta.settings import get_setting - class TestLoginTracking(BaseIntegrationTest): @@ -30,17 +27,17 @@ def setUp(self): self.fixture_manager.load_fixtureset('users') @parameterized.expand([ - # TEST_NAME EXEC_USER SUCCESS? EXPECTED_OUTCOME - ("login_fail_with_block" , "user_a", False , (302, "login", False)), - ("login_success_after_n-1", "user_a", True , (302, "home", True)), + # TEST_NAME EXEC_USER SUCCESS? EXPECTED_OUTCOME + ("login_failure_with_block" , "user_a" , False , (302, "login", False)), + ("login_success_after_n-1" , "user_a" , True , (302, "home", True)), ]) def test_login_tracking(self, _, executing_user: str, success: bool, expected_outcome): user = self.fixture_manager.get_fixture('users', executing_user) - n_attempts = get_setting(self.fixture_manager.get_db(), "security_max_failed_login_attempts") + n_attempts = 5 + self.set_application_setting("security_max_failed_login_attempts", n_attempts) for attempt in range(1, n_attempts + 1): - form = self.testapp.get("/login").forms[0] form["input_email"] = user.email form["input_password"] = user.password + ("_somenonsense" if attempt < n_attempts or not success else "") @@ -51,6 +48,6 @@ def test_login_tracking(self, _, executing_user: str, success: bool, expected_ou form["input_password"] = user.password response = form.submit("form.submitted") - user_db = self.fixture_manager.get_db().query(User).filter(User.email == user.email).one_or_none() + db_user = self.fixture_manager.get_fixture_db("users", executing_user) - assert (response.status_int, response.location.split("/")[-1], user_db.enabled) == expected_outcome + assert (response.status_int, response.location.split("/")[-1], db_user.enabled) == expected_outcome From f4f20a903012f98f9964d8b292fd1e77d5198b8a Mon Sep 17 00:00:00 2001 From: Christian Schudoma Date: Thu, 3 Feb 2022 19:40:15 +0100 Subject: [PATCH 16/32] removed get_db() function --- tests/integration/fixtures/__init__.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/tests/integration/fixtures/__init__.py b/tests/integration/fixtures/__init__.py index d8df4ac3..c9e02435 100644 --- a/tests/integration/fixtures/__init__.py +++ b/tests/integration/fixtures/__init__.py @@ -58,10 +58,6 @@ def __init__(self, session_factory, storage_path): self.session_factory = session_factory self.storage_path = storage_path - def get_db(self): - with transaction.manager: - return get_tm_session(self.session_factory, transaction.manager) - def _create_fixture(self, db, fixture_set: str, fixture_name: str, fixture: dict, database_insert=True): """Creates a fixture based on the provided dictionary""" models = import_module('datameta.models') From 973eac450745c93e828dd278ae36b7b32565ef4e Mon Sep 17 00:00:00 2001 From: Christian Schudoma Date: Thu, 3 Feb 2022 19:41:04 +0100 Subject: [PATCH 17/32] pleasing flake8 --- datameta/api/appsettings.py | 1 - 1 file changed, 1 deletion(-) diff --git a/datameta/api/appsettings.py b/datameta/api/appsettings.py index 4e926785..d08d5560 100644 --- a/datameta/api/appsettings.py +++ b/datameta/api/appsettings.py @@ -24,7 +24,6 @@ from ..security import authz from ..resource import resource_by_id from ..settings import get_setting_value_type, set_setting -import datetime @dataclass From cf0a5eac255bbca87b0b80a1b25264fc0ac696f4 Mon Sep 17 00:00:00 2001 From: Christian Schudoma Date: Thu, 3 Feb 2022 19:53:23 +0100 Subject: [PATCH 18/32] removed unneccessary list-comprehension --- datameta/security/__init__.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/datameta/security/__init__.py b/datameta/security/__init__.py index 96b93bae..a7f9fbc3 100644 --- a/datameta/security/__init__.py +++ b/datameta/security/__init__.py @@ -166,7 +166,9 @@ def get_user_by_credentials(request, email: str, password: str): User.id == user.id, User.enabled.is_(True))).all() - [db.delete(attempt) for attempt in failed_attempts] + for attempt in failed_attempts: + db.delete(attempt) + return user register_failed_login_attempt(db, user) From 9ec9b461b662305cf909b787ea384646bbfdd76b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C3=A9on=20Kuchenbecker?= Date: Thu, 17 Feb 2022 12:01:18 +0100 Subject: [PATCH 19/32] Refactor login attempts deletion --- datameta/models/db.py | 2 +- datameta/security/__init__.py | 9 +-------- 2 files changed, 2 insertions(+), 9 deletions(-) diff --git a/datameta/models/db.py b/datameta/models/db.py index 4c4c18dc..68730565 100644 --- a/datameta/models/db.py +++ b/datameta/models/db.py @@ -90,7 +90,7 @@ class User(Base): apikeys = relationship('ApiKey', back_populates='user') services = relationship('Service', secondary=user_service_table, back_populates='users') service_executions = relationship('ServiceExecution', back_populates='user') - login_attempts = relationship("LoginAttempt", back_populates='user') + login_attempts = relationship("LoginAttempt", back_populates='user', cascade="all, delete-orphan") class LoginAttempt(Base): diff --git a/datameta/security/__init__.py b/datameta/security/__init__.py index a7f9fbc3..9f4840b7 100644 --- a/datameta/security/__init__.py +++ b/datameta/security/__init__.py @@ -161,14 +161,7 @@ def get_user_by_credentials(request, email: str, password: str): user = db.query(User).filter(and_(User.email == email, User.enabled.is_(True))).one_or_none() if user: if check_password_by_hash(password, user.pwhash): - - failed_attempts = db.query(LoginAttempt).join(User).filter(and_( - User.id == user.id, - User.enabled.is_(True))).all() - - for attempt in failed_attempts: - db.delete(attempt) - + user.login_attempts.clear() return user register_failed_login_attempt(db, user) From 3da759fdc9275284b058aac91326a27557d6ed86 Mon Sep 17 00:00:00 2001 From: Christian Schudoma Date: Thu, 17 Feb 2022 16:21:25 +0100 Subject: [PATCH 20/32] Apply suggestions from code review Co-authored-by: Leon Kuchenbecker --- datameta/api/appsettings.py | 9 +++++---- datameta/settings.py | 7 +++++-- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/datameta/api/appsettings.py b/datameta/api/appsettings.py index d08d5560..d0b9a3ce 100644 --- a/datameta/api/appsettings.py +++ b/datameta/api/appsettings.py @@ -23,7 +23,7 @@ from .. import resource, security, errors from ..security import authz from ..resource import resource_by_id -from ..settings import get_setting_value_type, set_setting +from ..settings import get_setting_value_type, set_setting, SettingUpdateError @dataclass @@ -90,8 +90,9 @@ def put(request: Request): target_setting = resource_by_id(db, ApplicationSetting, settings_id) value = request.openapi_validated.body["value"] - error = set_setting(db, target_setting.key, value) - if error is not None: - raise errors.get_validation_error([error]) + try: + error = set_setting(db, target_setting.key, value) + except SettingUpdateError as e: + raise errors.get_validation_error(str(e)) return HTTPNoContent() diff --git a/datameta/settings.py b/datameta/settings.py index 97cacf05..479a4489 100644 --- a/datameta/settings.py +++ b/datameta/settings.py @@ -67,6 +67,9 @@ def get_setting(db, name): return None +class SettingUpdateError(RuntimeError): + pass + VALUE_CASTS = { "int": { "function": int, @@ -91,7 +94,7 @@ def get_setting(db, name): "time": { "function": lambda value: datetime.datetime.strptime(value, "%H:%M:%S"), "error_msg": "The time value has to specified in the form '%H:%M:%S'.", - "target": "time_value" # @lkuchenb: this was date_value in the original code - on purpose? + "target": "time_value" } } @@ -108,7 +111,7 @@ def set_setting(db, name, value): try: casted_value = cast_params["function"](value) except ValueError: - return cast_params["error_msg"] + raise SettingUpdateError(cast_params["error_msg"]) setattr(target_setting, cast_params["target"], casted_value) From ca0a62d8eb9da0f027e723de6f84aa4d8bfb5000 Mon Sep 17 00:00:00 2001 From: Christian Schudoma Date: Thu, 17 Feb 2022 16:23:24 +0100 Subject: [PATCH 21/32] pleasing flake8 --- datameta/api/appsettings.py | 2 +- datameta/settings.py | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/datameta/api/appsettings.py b/datameta/api/appsettings.py index d0b9a3ce..267b6f47 100644 --- a/datameta/api/appsettings.py +++ b/datameta/api/appsettings.py @@ -91,7 +91,7 @@ def put(request: Request): value = request.openapi_validated.body["value"] try: - error = set_setting(db, target_setting.key, value) + set_setting(db, target_setting.key, value) except SettingUpdateError as e: raise errors.get_validation_error(str(e)) diff --git a/datameta/settings.py b/datameta/settings.py index 479a4489..b2ca3e7b 100644 --- a/datameta/settings.py +++ b/datameta/settings.py @@ -70,6 +70,7 @@ def get_setting(db, name): class SettingUpdateError(RuntimeError): pass + VALUE_CASTS = { "int": { "function": int, From 218500adddac1ff0125f2fc779f1da67284e5554 Mon Sep 17 00:00:00 2001 From: Christian Schudoma Date: Thu, 17 Feb 2022 17:00:08 +0100 Subject: [PATCH 22/32] pleasing mypy --- datameta/api/appsettings.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/datameta/api/appsettings.py b/datameta/api/appsettings.py index 267b6f47..73ed3fe1 100644 --- a/datameta/api/appsettings.py +++ b/datameta/api/appsettings.py @@ -93,6 +93,6 @@ def put(request: Request): try: set_setting(db, target_setting.key, value) except SettingUpdateError as e: - raise errors.get_validation_error(str(e)) + raise errors.get_validation_error([str(e)]) return HTTPNoContent() From e2fb375ad225c713b132fbeda4eeb7f4eb5f2cb0 Mon Sep 17 00:00:00 2001 From: Christian Schudoma Date: Fri, 18 Feb 2022 10:48:39 +0100 Subject: [PATCH 23/32] added failed login attempt tracking to expired apikey and group-mismatch scenarios --- datameta/security/__init__.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/datameta/security/__init__.py b/datameta/security/__init__.py index 9f4840b7..a0e7b02b 100644 --- a/datameta/security/__init__.py +++ b/datameta/security/__init__.py @@ -208,7 +208,9 @@ def revalidate_user(request): )).one_or_none() if apikey is not None: if check_expiration(apikey.expires): + register_failed_login_attempt(request.dbsession, apikey.user) raise HTTPUnauthorized() + apikey.user.login_attempts.clear() return apikey.user else: raise HTTPUnauthorized() @@ -223,10 +225,13 @@ def revalidate_user(request): )).one_or_none() # Check if the user still exists and their group hasn't changed if user is None or user.group_id != request.session['user_gid']: + if user.group_id != request.session['user_gid']: + register_failed_login_attempt(request.dbsession, user) request.session.invalidate() raise HTTPUnauthorized() request.session['site_admin'] = user.site_admin request.session['group_admin'] = user.group_admin + user.login_attempts.clear() return user From 7bf43bf12b267fa8c2801b30a377935cd1487295 Mon Sep 17 00:00:00 2001 From: Christian Schudoma Date: Mon, 28 Feb 2022 16:57:32 +0100 Subject: [PATCH 24/32] refactor: revalidate_user now uses new transaction for login attempt tracking --- datameta/security/__init__.py | 74 ++++++++++++++++++++++++----------- 1 file changed, 51 insertions(+), 23 deletions(-) diff --git a/datameta/security/__init__.py b/datameta/security/__init__.py index a0e7b02b..abbc1de8 100644 --- a/datameta/security/__init__.py +++ b/datameta/security/__init__.py @@ -22,6 +22,7 @@ from ..models import User, ApiKey, PasswordToken, Session, LoginAttempt from ..settings import get_setting +from pyramid_tm import create_tm import bcrypt import hashlib import logging @@ -132,23 +133,29 @@ def hash_token(token): return hashed_token -def register_failed_login_attempt(db, user): +def register_failed_login_attempt(request, user): """ Registers a failed login attempt and disables user if this has happened too often in the last hour.""" + db = request.dbsession + txn = create_tm(request) + now = datetime.utcnow() max_allowed_failed_logins = get_setting(db, "security_max_failed_login_attempts") - db.add(LoginAttempt(user_id=user.id, timestamp=now)) + db.add(LoginAttempt(user_id=user.id, timestamp=now)) n_failed_logins = sum( now - attempt.timestamp <= timedelta(hours=1) for attempt in db.query(LoginAttempt).filter(LoginAttempt.user_id == user.id).all() ) + db.flush() log.warning(f"FAILED LOGIN ATTEMPT USER id={user.id} n={n_failed_logins} within one hour.") if n_failed_logins >= max_allowed_failed_logins: user.enabled = False log.warning(f"BLOCKED USER id={user.id} enabled={user.enabled} reason={n_failed_logins} failed login attempts within one hour.") - db.flush() + + txn.commit() + txn.begin() return None @@ -161,10 +168,11 @@ def get_user_by_credentials(request, email: str, password: str): user = db.query(User).filter(and_(User.email == email, User.enabled.is_(True))).one_or_none() if user: if check_password_by_hash(password, user.pwhash): + log.warning(f"CLEARING FAILED LOGIN ATTEMPTS FOR gubc USER {user}") user.login_attempts.clear() return user - register_failed_login_attempt(db, user) + register_failed_login_attempt(request, user) return None @@ -194,47 +202,67 @@ def get_password_reset_token(db: Session, token: str): )).one_or_none() -def revalidate_user(request): - """Revalidate the currently logged in user and return the corresponding user object. On failure, - raise a 401""" +def revalidate_user_token_based(request, token): db = request.dbsession - # Check for token based auth - token = get_bearer_token(request) - if token is not None: - token_hash = hash_token(token) - apikey = db.query(ApiKey).join(User).filter(and_( - ApiKey.value == token_hash, - User.enabled.is_(True) - )).one_or_none() - if apikey is not None: - if check_expiration(apikey.expires): - register_failed_login_attempt(request.dbsession, apikey.user) - raise HTTPUnauthorized() - apikey.user.login_attempts.clear() - return apikey.user + + token_hash = hash_token(token) + apikey = db.query(ApiKey).join(User).filter(and_( + ApiKey.value == token_hash, + User.enabled.is_(True) + )).one_or_none() + + if apikey is not None: + apikey_expired = check_expiration(apikey.expires) + user = apikey.user + + if apikey_expired: + register_failed_login_attempt(request, user) else: - raise HTTPUnauthorized() + log.warning(f"CLEARING FAILED LOGIN ATTEMPTS FOR APIKEY.USER {user.id}") + user.login_attempts.clear() + return user + + raise HTTPUnauthorized() + +def revalidate_user_session_based(request): # Check for session based auth if 'user_uid' not in request.session: request.session.invalidate() raise HTTPUnauthorized() + user = request.dbsession.query(User).filter(and_( User.id == request.session['user_uid'], User.enabled.is_(True) )).one_or_none() + # Check if the user still exists and their group hasn't changed if user is None or user.group_id != request.session['user_gid']: if user.group_id != request.session['user_gid']: - register_failed_login_attempt(request.dbsession, user) + register_failed_login_attempt(request, user) request.session.invalidate() raise HTTPUnauthorized() + request.session['site_admin'] = user.site_admin request.session['group_admin'] = user.group_admin + + log.warning(f"CLEARING FAILED LOGIN ATTEMPTS FOR USER {user}") user.login_attempts.clear() return user +def revalidate_user(request): + """Revalidate the currently logged in user and return the corresponding user object. On failure, + raise a 401""" + + # Check for token based auth + token = get_bearer_token(request) + if token is not None: + return revalidate_user_token_based(request, token) + + return revalidate_user_session_based(request) + + def revalidate_user_or_login(request): """Revalidate and return the currently logged in user, on failure redirect to the login page""" try: From 95833ebf0ecfbb4f336ad6a7b0aeb1be9ed7e4a0 Mon Sep 17 00:00:00 2001 From: Christian Schudoma Date: Mon, 28 Feb 2022 16:58:11 +0100 Subject: [PATCH 25/32] added test for apikey-based login tracking --- tests/integration/test_login_tracking.py | 36 +++++++++++++++++++++++- 1 file changed, 35 insertions(+), 1 deletion(-) diff --git a/tests/integration/test_login_tracking.py b/tests/integration/test_login_tracking.py index 6fd32092..42bbdac3 100644 --- a/tests/integration/test_login_tracking.py +++ b/tests/integration/test_login_tracking.py @@ -17,6 +17,8 @@ from parameterized import parameterized from . import BaseIntegrationTest +from datameta.api import base_url +from .utils import get_auth_header class TestLoginTracking(BaseIntegrationTest): @@ -25,13 +27,14 @@ def setUp(self): super().setUp() self.fixture_manager.load_fixtureset('groups') self.fixture_manager.load_fixtureset('users') + self.fixture_manager.load_fixtureset('apikeys') @parameterized.expand([ # TEST_NAME EXEC_USER SUCCESS? EXPECTED_OUTCOME ("login_failure_with_block" , "user_a" , False , (302, "login", False)), ("login_success_after_n-1" , "user_a" , True , (302, "home", True)), ]) - def test_login_tracking(self, _, executing_user: str, success: bool, expected_outcome): + def test_login_tracking_via_login(self, _, executing_user: str, success: bool, expected_outcome): user = self.fixture_manager.get_fixture('users', executing_user) n_attempts = 5 @@ -51,3 +54,34 @@ def test_login_tracking(self, _, executing_user: str, success: bool, expected_ou db_user = self.fixture_manager.get_fixture_db("users", executing_user) assert (response.status_int, response.location.split("/")[-1], db_user.enabled) == expected_outcome + + @parameterized.expand([ + # TEST_NAME EXEC_USER SUCCESS? EXPECTED_OUTCOME + ("login_failure_with_block" , "user_a" , False , (401, False)), + ("login_success_after_n-1" , "user_a" , True , (200, True)), + ]) + def test_login_tracking_via_apikey(self, _, executing_user: str, success: bool, expected_outcome): + + n_attempts = 5 + self.set_application_setting("security_max_failed_login_attempts", n_attempts) + + for attempt in range(1, n_attempts + 1): + failed_attempt = attempt < n_attempts or not success + apikey = self.fixture_manager.get_fixture('apikeys', executing_user + ("_expired" if failed_attempt else "")) + + self.testapp.get( + f"{base_url}/rpc/whoami", + headers=get_auth_header(apikey.value_plain), + status=401 if failed_attempt else 200 + ) + + apikey = self.fixture_manager.get_fixture('apikeys', executing_user) + response = self.testapp.get( + f"{base_url}/rpc/whoami", + status=expected_outcome[0], + headers=get_auth_header(apikey.value_plain) + ) + + db_user = self.fixture_manager.get_fixture_db("users", executing_user) + + assert (response.status_int, db_user.enabled) == expected_outcome From 6597d01b4397b8eb7627c0e6a8f4fbdd0ecf1bd8 Mon Sep 17 00:00:00 2001 From: Christian Schudoma Date: Mon, 28 Feb 2022 17:04:13 +0100 Subject: [PATCH 26/32] minor: whitespace --- datameta/security/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/datameta/security/__init__.py b/datameta/security/__init__.py index a6f2797e..39409d83 100644 --- a/datameta/security/__init__.py +++ b/datameta/security/__init__.py @@ -168,7 +168,7 @@ def register_failed_login_attempt(request, user): """ Registers a failed login attempt and disables user if this has happened too often in the last hour.""" db = request.dbsession - txn = create_tm(request) + txn = create_tm(request) now = datetime.utcnow() max_allowed_failed_logins = get_setting(db, "security_max_failed_login_attempts") From 475753b7185201092bf12ad4f4b2dafb33760e7e Mon Sep 17 00:00:00 2001 From: Christian Schudoma Date: Mon, 28 Feb 2022 17:06:25 +0100 Subject: [PATCH 27/32] corrected down_revision --- datameta/alembic/versions/20211029_98911b881230.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/datameta/alembic/versions/20211029_98911b881230.py b/datameta/alembic/versions/20211029_98911b881230.py index 6bb27cf3..bcd99797 100644 --- a/datameta/alembic/versions/20211029_98911b881230.py +++ b/datameta/alembic/versions/20211029_98911b881230.py @@ -11,7 +11,7 @@ # revision identifiers, used by Alembic. revision = '98911b881230' -down_revision = '7fdc829db18d' +down_revision = '6a9f0bec38e6' branch_labels = None depends_on = None From 0afd7aace13efbb982e99ae0ec01f30deeb50709 Mon Sep 17 00:00:00 2001 From: Christian Schudoma Date: Mon, 28 Feb 2022 17:08:11 +0100 Subject: [PATCH 28/32] added missing comma --- datameta/models/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/datameta/models/__init__.py b/datameta/models/__init__.py index ffa193a4..236a1513 100644 --- a/datameta/models/__init__.py +++ b/datameta/models/__init__.py @@ -36,7 +36,7 @@ DownloadToken, Service, ServiceExecution, - LoginAttempt + LoginAttempt, UsedPassword ) From aa5cdde0a347cf0333f21b3edea2a25c33010a07 Mon Sep 17 00:00:00 2001 From: Christian Schudoma Date: Mon, 28 Feb 2022 17:08:21 +0100 Subject: [PATCH 29/32] added missing comma --- datameta/models/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/datameta/models/__init__.py b/datameta/models/__init__.py index 236a1513..5d717fa0 100644 --- a/datameta/models/__init__.py +++ b/datameta/models/__init__.py @@ -37,7 +37,7 @@ Service, ServiceExecution, LoginAttempt, - UsedPassword + UsedPassword, ) # run configure_mappers after defining all of the models to ensure From 84e54b1ed20b3621cb96d6949501092c1efcf170 Mon Sep 17 00:00:00 2001 From: Christian Schudoma Date: Thu, 3 Mar 2022 16:03:39 +0100 Subject: [PATCH 30/32] remedied remaining issues from 7bf43bf --- datameta/security/__init__.py | 46 ++++++++++++++++++++--------------- 1 file changed, 26 insertions(+), 20 deletions(-) diff --git a/datameta/security/__init__.py b/datameta/security/__init__.py index 39409d83..cd0a3582 100644 --- a/datameta/security/__init__.py +++ b/datameta/security/__init__.py @@ -12,21 +12,22 @@ # See the License for the specific language governing permissions and # limitations under the License. -from pyramid.httpexceptions import HTTPFound, HTTPUnauthorized +import bcrypt +import hashlib +import logging +import secrets + from datetime import datetime, timedelta -from typing import Optional from random import choice from string import ascii_letters, digits, punctuation +from typing import Optional + +from pyramid.httpexceptions import HTTPFound, HTTPUnauthorized from sqlalchemy import and_ from ..models import User, ApiKey, PasswordToken, Session, UsedPassword, LoginAttempt from ..settings import get_setting -from pyramid_tm import create_tm -import bcrypt -import hashlib -import logging -import secrets log = logging.getLogger(__name__) @@ -164,30 +165,25 @@ def hash_token(token): return hashed_token -def register_failed_login_attempt(request, user): +def register_failed_login_attempt(db, user): """ Registers a failed login attempt and disables user if this has happened too often in the last hour.""" - db = request.dbsession - txn = create_tm(request) - now = datetime.utcnow() max_allowed_failed_logins = get_setting(db, "security_max_failed_login_attempts") db.add(LoginAttempt(user_id=user.id, timestamp=now)) + db.flush() n_failed_logins = sum( now - attempt.timestamp <= timedelta(hours=1) for attempt in db.query(LoginAttempt).filter(LoginAttempt.user_id == user.id).all() ) - db.flush() log.warning(f"FAILED LOGIN ATTEMPT USER id={user.id} n={n_failed_logins} within one hour.") if n_failed_logins >= max_allowed_failed_logins: - user.enabled = False + db.query(User).filter(user.id == User.id).update({User.enabled: False}) + db.flush() log.warning(f"BLOCKED USER id={user.id} enabled={user.enabled} reason={n_failed_logins} failed login attempts within one hour.") - txn.commit() - txn.begin() - return None @@ -203,7 +199,7 @@ def get_user_by_credentials(request, email: str, password: str): user.login_attempts.clear() return user - register_failed_login_attempt(request, user) + register_failed_login_attempt(db, user) return None @@ -247,7 +243,11 @@ def revalidate_user_token_based(request, token): user = apikey.user if apikey_expired: - register_failed_login_attempt(request, user) + request.tm.abort() + request.tm.begin() + register_failed_login_attempt(db, user) + request.tm.commit() + request.tm.begin() else: log.warning(f"CLEARING FAILED LOGIN ATTEMPTS FOR APIKEY.USER {user.id}") user.login_attempts.clear() @@ -262,7 +262,9 @@ def revalidate_user_session_based(request): request.session.invalidate() raise HTTPUnauthorized() - user = request.dbsession.query(User).filter(and_( + db = request.dbsession + + user = db.query(User).filter(and_( User.id == request.session['user_uid'], User.enabled.is_(True) )).one_or_none() @@ -270,7 +272,11 @@ def revalidate_user_session_based(request): # Check if the user still exists and their group hasn't changed if user is None or user.group_id != request.session['user_gid']: if user.group_id != request.session['user_gid']: - register_failed_login_attempt(request, user) + request.tm.abort() + request.tm.begin() + register_failed_login_attempt(db, user) + request.tm.commit() + request.tm.begin() request.session.invalidate() raise HTTPUnauthorized() From 8c3018818ecd4e6845444de86c529a6ffbba5f91 Mon Sep 17 00:00:00 2001 From: Christian Schudoma Date: Fri, 4 Mar 2022 09:55:57 +0100 Subject: [PATCH 31/32] split password/token-related security functions into security.pwdz and security.tokenz --- datameta/api/apikeys.py | 6 +- datameta/api/download.py | 3 +- datameta/api/password.py | 10 +- datameta/api/ui/admin.py | 3 +- datameta/api/ui/forgot.py | 5 +- datameta/api/upload.py | 5 +- datameta/scripts/initialize_db.py | 3 +- datameta/security/__init__.py | 168 ++---------------------------- datameta/security/pwdz.py | 99 ++++++++++++++++++ datameta/security/tokenz.py | 107 +++++++++++++++++++ datameta/storage.py | 11 +- datameta/views/setpass.py | 6 +- 12 files changed, 243 insertions(+), 183 deletions(-) create mode 100644 datameta/security/pwdz.py create mode 100644 datameta/security/tokenz.py diff --git a/datameta/api/apikeys.py b/datameta/api/apikeys.py index bb7cb580..e95c8042 100644 --- a/datameta/api/apikeys.py +++ b/datameta/api/apikeys.py @@ -18,7 +18,7 @@ from typing import Optional, List from .. import models from .. import security -from ..security import authz +from ..security import authz, tokenz from pyramid.httpexceptions import HTTPOk, HTTPUnauthorized, HTTPForbidden from ..resource import resource_by_id, get_identifier from ..errors import get_validation_error @@ -95,8 +95,8 @@ def generate_api_key(request: Request, user: models.User, label: str, expires: O """ Generate API Token and store unsalted hash in db. """ - token = security.generate_token() - token_hash = security.hash_token(token) + token = tokenz.generate_token() + token_hash = tokenz.hash_token(token) db = request.dbsession apikey = models.ApiKey( diff --git a/datameta/api/download.py b/datameta/api/download.py index 40821e48..9489088f 100644 --- a/datameta/api/download.py +++ b/datameta/api/download.py @@ -18,6 +18,7 @@ from pyramid.response import FileResponse from datetime import datetime from .. import security, models, storage +from ..security import tokenz from ..resource import get_identifier from .files import access_file_by_user from sqlalchemy import and_ @@ -75,7 +76,7 @@ def download_by_token(request: Request) -> HTTPOk: Usage: /download/{download_token} """ token = request.matchdict['token'] - hashed_token = security.hash_token(token) + hashed_token = tokenz.hash_token(token) # get download token from db db = request.dbsession diff --git a/datameta/api/password.py b/datameta/api/password.py index 6c80c566..5259fa38 100644 --- a/datameta/api/password.py +++ b/datameta/api/password.py @@ -17,7 +17,7 @@ from pyramid.view import view_config from .. import security, errors -from ..security import authz +from ..security import authz, tokenz, pwdz from ..models import User from datetime import datetime @@ -41,7 +41,7 @@ def put(request): token = None if request_id == '0': # Try to find the token - token = security.get_password_reset_token(db, request_credential) + token = tokenz.get_password_reset_token(db, request_credential) if token is None: raise HTTPNotFound() # 404 Token not found @@ -64,16 +64,16 @@ def put(request): raise HTTPForbidden() # 403 User ID not found, hidden from the user intentionally # Only changing the user's own password is allowed - if not authz.update_user_password(auth_user, target_user) or not security.check_password_by_hash(request_credential, auth_user.pwhash): + if not authz.update_user_password(auth_user, target_user) or not pwdz.check_password_by_hash(request_credential, auth_user.pwhash): raise HTTPForbidden() # 403 Not authorized to change this user's password # Verify the password quality - error = security.verify_password(db, auth_user.id, request_newPassword) + error = pwdz.verify_password(db, auth_user.id, request_newPassword) if error: raise errors.get_validation_error([error]) # Set the new password - auth_user.pwhash = security.register_password(db, auth_user.id, request_newPassword) + auth_user.pwhash = pwdz.register_password(db, auth_user.id, request_newPassword) # Delete the password token if any if token: diff --git a/datameta/api/ui/admin.py b/datameta/api/ui/admin.py index 18e83edd..d902db9d 100644 --- a/datameta/api/ui/admin.py +++ b/datameta/api/ui/admin.py @@ -18,6 +18,7 @@ from ...models import User, Group, RegRequest from ...settings import get_setting from ... import email, security, siteid, errors +from ...security import tokenz from ...resource import resource_by_id, get_identifier from sqlalchemy.exc import IntegrityError @@ -98,7 +99,7 @@ def v_admin_put_request(request): db.delete(reg_req) # Obtain a new token - _, clear_token = security.get_new_password_reset_token(db, new_user) + _, clear_token = tokenz.get_new_password_reset_token(db, new_user) # Generate the token url token_url = request.route_url('setpass', token = clear_token) diff --git a/datameta/api/ui/forgot.py b/datameta/api/ui/forgot.py index dcdd0d8b..602cf89b 100644 --- a/datameta/api/ui/forgot.py +++ b/datameta/api/ui/forgot.py @@ -14,7 +14,8 @@ from pyramid.view import view_config -from ... import security, email +from ... import email +from ...security import tokenz from ...settings import get_setting import re @@ -51,7 +52,7 @@ def v_forgot_api(request): return { 'success' : False, 'error' : 'MALFORMED_EMAIL' } try: - db_token_obj, clear_token = security.get_new_password_reset_token_from_email(db, req_email) + db_token_obj, clear_token = tokenz.get_new_password_reset_token_from_email(db, req_email) except KeyError: # User not found log.debug(f"DURING RECOVERY TOKEN REQUEST: USER COULD NOT BE RESOLVED FROM EMAIL: {req_email}") diff --git a/datameta/api/upload.py b/datameta/api/upload.py index 0925e4f2..770b64d2 100644 --- a/datameta/api/upload.py +++ b/datameta/api/upload.py @@ -18,7 +18,8 @@ import webob import logging from datetime import datetime -from .. import resource, models, storage, security +from .. import resource, models, storage +from ..security import tokenz log = logging.getLogger(__name__) @@ -53,7 +54,7 @@ def post(request) -> HTTPNoContent: db_file = resource.resource_by_id(db, models.File, req_file_id) # Validate access token match - db_file = db_file if db_file and req_token and db_file.upload_expires > datetime.now() and db_file.access_token == security.hash_token(req_token) else None + db_file = db_file if db_file and req_token and db_file.upload_expires > datetime.now() and db_file.access_token == tokenz.hash_token(req_token) else None if db_file is None: # Includes token mismatch raise HTTPNotFound(json=None) diff --git a/datameta/scripts/initialize_db.py b/datameta/scripts/initialize_db.py index 6fc56787..e6c2eb8a 100644 --- a/datameta/scripts/initialize_db.py +++ b/datameta/scripts/initialize_db.py @@ -18,7 +18,8 @@ from pyramid.paster import bootstrap, setup_logging from sqlalchemy.exc import OperationalError -from ..security import register_password, hash_token +from ..security.pwdz import register_password +from ..security.tokenz import hash_token from ..models import User, Group, MetaDatum, DateTimeMode, ApiKey diff --git a/datameta/security/__init__.py b/datameta/security/__init__.py index cd0a3582..7b19fec6 100644 --- a/datameta/security/__init__.py +++ b/datameta/security/__init__.py @@ -12,99 +12,23 @@ # See the License for the specific language governing permissions and # limitations under the License. -import bcrypt -import hashlib import logging -import secrets from datetime import datetime, timedelta -from random import choice -from string import ascii_letters, digits, punctuation from typing import Optional from pyramid.httpexceptions import HTTPFound, HTTPUnauthorized from sqlalchemy import and_ -from ..models import User, ApiKey, PasswordToken, Session, UsedPassword, LoginAttempt +from ..models import User, ApiKey, LoginAttempt from ..settings import get_setting -log = logging.getLogger(__name__) - - -def register_password(db, user_id, password): - """ Hashes an accepted password string, adds the hash to the user's password hash history. - - Returns: - - the hashed password - """ - hashed_pw = hash_password(password) - db.add(UsedPassword(user_id=user_id, pwhash=hashed_pw)) - - return hashed_pw - - -def is_used_password(db, user_id, password): - """ Checks a password string against a user's password hash history. - - Returns: - - True, if password has been used before by the user - - False, otherwise - """ - - used_passwords = db.query(UsedPassword).filter(UsedPassword.user_id == user_id).all() - for pw in used_passwords: - if check_password_by_hash(password, pw.pwhash): - return True - - return False - - -def generate_token(): - return "".join(choice(ascii_letters + digits) for _ in range(64) ) - - -def get_new_password_reset_token(db: Session, user: User, expires=None): - """Clears all password recovery tokens for user identified by the supplied - email address, generates a new one and returns it. - - Returns: - - PasswordToken object (with hashed token value) - - unhashed token value - """ - - # Delete existing tokens - db.query(PasswordToken).filter(PasswordToken.user_id == user.id).delete() - - # Create new token value - clear_token = secrets.token_urlsafe(40) - - # Add hashed token to db - db_token_obj = PasswordToken( - user=user, - value=hash_token(clear_token), - expires=expires if expires else datetime.now() + timedelta(minutes=10) - ) - db.add(db_token_obj) - - return db_token_obj, clear_token - - -def get_new_password_reset_token_from_email(db: Session, email: str): - """Clears all password recovery tokens for user identified by the supplied - email address, generates a new one and returns it. +from . import tokenz, pwdz - Returns: - - PasswordToken with hashed token value - - cleartext token for user notification - Raises: - KeyError - if user not found/not enabled""" - user = db.query(User).filter(User.enabled.is_(True), User.email == email).one_or_none() +from .tokenz import * +from .pwdz import * - # User not found or disabled - if not user: - raise KeyError(f"Could not find active user with email={email}") - - return get_new_password_reset_token(db, user) +log = logging.getLogger(__name__) def check_expiration(expiration_datetime: Optional[datetime]): @@ -114,57 +38,6 @@ def check_expiration(expiration_datetime: Optional[datetime]): return expiration_datetime is not None and datetime.now() >= expiration_datetime -def verify_password(db, user_id, password): - if is_used_password(db, user_id, password): - return "The password has already been used." - - pw_min_length = get_setting(db, "security_password_minimum_length") - pw_min_ucase = get_setting(db, "security_password_minimum_uppercase_characters") - pw_min_lcase = get_setting(db, "security_password_minimum_lowercase_characters") - pw_min_digits = get_setting(db, "security_password_minimum_digits") - pw_min_punctuation = get_setting(db, "security_password_minimum_punctuation_characters") - - if len(password) < pw_min_length: - return f"The password must be at least {pw_min_length} characters long." - - alphas = [c for c in password if c.isalpha()] - digits = [c for c in password if c.isdigit()] - puncs = [c for c in password if c in punctuation] - n_upper = sum(c.isupper() for c in alphas) - n_lower = len(alphas) - n_upper - - if n_upper < pw_min_ucase or n_lower < pw_min_lcase or len(digits) < pw_min_digits or len(puncs) < pw_min_punctuation: - return f"Password must contain at least {pw_min_ucase} uppercase characters, " \ - f"{pw_min_lcase} lowercase character(s), " \ - f"{pw_min_punctuation} punctuation mark(s), and " \ - f"{pw_min_digits} digit(s)." - - invalid_chars = set(password).difference(alphas).difference(digits).difference(puncs) - - if invalid_chars: - return f"Invalid password characters: {','.join(invalid_chars)}" - - return None - - -def hash_password(pw): - """Hash a password and return the salted hash""" - pwhash = bcrypt.hashpw(pw.encode('utf8'), bcrypt.gensalt()) - return pwhash.decode('utf8') - - -def check_password_by_hash(pw, hashed_pw): - """Check a password against a salted hash""" - expected_hash = hashed_pw.encode('utf8') - return bcrypt.checkpw(pw.encode('utf8'), expected_hash) - - -def hash_token(token): - """Hash a token and return the unsalted hash""" - hashed_token = hashlib.sha256(token.encode('utf-8')).hexdigest() - return hashed_token - - def register_failed_login_attempt(db, user): """ Registers a failed login attempt and disables user if this has happened too often in the last hour.""" @@ -194,7 +67,7 @@ def get_user_by_credentials(request, email: str, password: str): db = request.dbsession user = db.query(User).filter(and_(User.email == email, User.enabled.is_(True))).one_or_none() if user: - if check_password_by_hash(password, user.pwhash): + if pwdz.check_password_by_hash(password, user.pwhash): log.warning(f"CLEARING FAILED LOGIN ATTEMPTS FOR gubc USER {user}") user.login_attempts.clear() return user @@ -204,35 +77,10 @@ def get_user_by_credentials(request, email: str, password: str): return None -def get_bearer_token(request): - """Extracts a Bearer authentication token from the request and returns it if present, None - otherwise.""" - auth = request.headers.get("Authorization") - if auth is not None: - try: - method, content = auth.split(" ") - if method == "Bearer": - return content - except Exception: - pass - return None - - -def get_password_reset_token(db: Session, token: str): - """Tries to find the corresponding password reset token in the database. - Returns the token only if it can be found and if the corresponding user is - enabled, otherwise no checking is performed, most importantly expiration - checks are not performed""" - return db.query(PasswordToken).join(User).filter(and_( - PasswordToken.value == hash_token(token), - User.enabled.is_(True) - )).one_or_none() - - def revalidate_user_token_based(request, token): db = request.dbsession - token_hash = hash_token(token) + token_hash = tokenz.hash_token(token) apikey = db.query(ApiKey).join(User).filter(and_( ApiKey.value == token_hash, User.enabled.is_(True) @@ -293,7 +141,7 @@ def revalidate_user(request): raise a 401""" # Check for token based auth - token = get_bearer_token(request) + token = tokenz.get_bearer_token(request) if token is not None: return revalidate_user_token_based(request, token) diff --git a/datameta/security/pwdz.py b/datameta/security/pwdz.py new file mode 100644 index 00000000..413a5dfe --- /dev/null +++ b/datameta/security/pwdz.py @@ -0,0 +1,99 @@ +# Copyright 2021 Universität Tübingen, DKFZ and EMBL for the German Human Genome-Phenome Archive (GHGA) +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# https://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import bcrypt +import logging + +from string import punctuation + +from pyramid.httpexceptions import HTTPFound, HTTPUnauthorized +from sqlalchemy import and_ + +from ..models import UsedPassword +from ..settings import get_setting + +log = logging.getLogger(__name__) + + +def register_password(db, user_id, password): + """ Hashes an accepted password string, adds the hash to the user's password hash history. + + Returns: + - the hashed password + """ + hashed_pw = hash_password(password) + db.add(UsedPassword(user_id=user_id, pwhash=hashed_pw)) + + return hashed_pw + + +def is_used_password(db, user_id, password): + """ Checks a password string against a user's password hash history. + + Returns: + - True, if password has been used before by the user + - False, otherwise + """ + + used_passwords = db.query(UsedPassword).filter(UsedPassword.user_id == user_id).all() + for pw in used_passwords: + if check_password_by_hash(password, pw.pwhash): + return True + + return False + + +def verify_password(db, user_id, password): + if is_used_password(db, user_id, password): + return "The password has already been used." + + pw_min_length = get_setting(db, "security_password_minimum_length") + pw_min_ucase = get_setting(db, "security_password_minimum_uppercase_characters") + pw_min_lcase = get_setting(db, "security_password_minimum_lowercase_characters") + pw_min_digits = get_setting(db, "security_password_minimum_digits") + pw_min_punctuation = get_setting(db, "security_password_minimum_punctuation_characters") + + if len(password) < pw_min_length: + return f"The password must be at least {pw_min_length} characters long." + + alphas = [c for c in password if c.isalpha()] + digits = [c for c in password if c.isdigit()] + puncs = [c for c in password if c in punctuation] + n_upper = sum(c.isupper() for c in alphas) + n_lower = len(alphas) - n_upper + + if n_upper < pw_min_ucase or n_lower < pw_min_lcase or len(digits) < pw_min_digits or len(puncs) < pw_min_punctuation: + return f"Password must contain at least {pw_min_ucase} uppercase characters, " \ + f"{pw_min_lcase} lowercase character(s), " \ + f"{pw_min_punctuation} punctuation mark(s), and " \ + f"{pw_min_digits} digit(s)." + + invalid_chars = set(password).difference(alphas).difference(digits).difference(puncs) + + if invalid_chars: + return f"Invalid password characters: {','.join(invalid_chars)}" + + return None + + +def hash_password(pw): + """Hash a password and return the salted hash""" + pwhash = bcrypt.hashpw(pw.encode('utf8'), bcrypt.gensalt()) + return pwhash.decode('utf8') + + +def check_password_by_hash(pw, hashed_pw): + """Check a password against a salted hash""" + expected_hash = hashed_pw.encode('utf8') + return bcrypt.checkpw(pw.encode('utf8'), expected_hash) diff --git a/datameta/security/tokenz.py b/datameta/security/tokenz.py new file mode 100644 index 00000000..e6594ceb --- /dev/null +++ b/datameta/security/tokenz.py @@ -0,0 +1,107 @@ +# Copyright 2021 Universität Tübingen, DKFZ and EMBL for the German Human Genome-Phenome Archive (GHGA) +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# https://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import hashlib +import logging +import secrets + +from datetime import datetime, timedelta +from random import choice +from string import ascii_letters, digits, punctuation + +from sqlalchemy import and_ + +from ..models import User, PasswordToken, Session + + +log = logging.getLogger(__name__) + + +def generate_token(): + return "".join(choice(ascii_letters + digits) for _ in range(64) ) + + +def get_new_password_reset_token(db: Session, user: User, expires=None): + """Clears all password recovery tokens for user identified by the supplied + email address, generates a new one and returns it. + + Returns: + - PasswordToken object (with hashed token value) + - unhashed token value + """ + + # Delete existing tokens + db.query(PasswordToken).filter(PasswordToken.user_id == user.id).delete() + + # Create new token value + clear_token = secrets.token_urlsafe(40) + + # Add hashed token to db + db_token_obj = PasswordToken( + user=user, + value=hash_token(clear_token), + expires=expires if expires else datetime.now() + timedelta(minutes=10) + ) + db.add(db_token_obj) + + return db_token_obj, clear_token + + +def get_new_password_reset_token_from_email(db: Session, email: str): + """Clears all password recovery tokens for user identified by the supplied + email address, generates a new one and returns it. + + Returns: + - PasswordToken with hashed token value + - cleartext token for user notification + Raises: + KeyError - if user not found/not enabled""" + user = db.query(User).filter(User.enabled.is_(True), User.email == email).one_or_none() + + # User not found or disabled + if not user: + raise KeyError(f"Could not find active user with email={email}") + + return get_new_password_reset_token(db, user) + + +def hash_token(token): + """Hash a token and return the unsalted hash""" + hashed_token = hashlib.sha256(token.encode('utf-8')).hexdigest() + return hashed_token + + +def get_bearer_token(request): + """Extracts a Bearer authentication token from the request and returns it if present, None + otherwise.""" + auth = request.headers.get("Authorization") + if auth is not None: + try: + method, content = auth.split(" ") + if method == "Bearer": + return content + except Exception: + pass + return None + + +def get_password_reset_token(db: Session, token: str): + """Tries to find the corresponding password reset token in the database. + Returns the token only if it can be found and if the corresponding user is + enabled, otherwise no checking is performed, most importantly expiration + checks are not performed""" + return db.query(PasswordToken).join(User).filter(and_( + PasswordToken.value == hash_token(token), + User.enabled.is_(True) + )).one_or_none() diff --git a/datameta/storage.py b/datameta/storage.py index d2a19d5f..70717481 100644 --- a/datameta/storage.py +++ b/datameta/storage.py @@ -19,7 +19,8 @@ from datetime import datetime, timedelta from pyramid.request import Request from typing import Optional -from . import security, models +from . import models +from .security import tokenz from .api import base_url log = logging.getLogger(__name__) @@ -77,8 +78,8 @@ def create_and_annotate_storage(request, db_file): # Currently, only local storage is supported db_file.storage_uri = f"file://{db_file.uuid}__{db_file.checksum}" - token = security.generate_token() - db_file.access_token = security.hash_token(token) + token = tokenz.generate_token() + db_file.access_token = tokenz.hash_token(token) # Create empty file open(get_local_storage_path(request, db_file.storage_uri), 'w').close() @@ -162,8 +163,8 @@ def _get_download_url_local(request: Request, db_file: models.File, expires_afte if expires_after is None: expires_after = 1 - token = security.generate_token() - token_hash = security.hash_token(token) + token = tokenz.generate_token() + token_hash = tokenz.hash_token(token) expires = datetime.utcnow() + timedelta(minutes = float(expires_after)) db = request.dbsession diff --git a/datameta/views/setpass.py b/datameta/views/setpass.py index 8cd2013d..240ebb4a 100644 --- a/datameta/views/setpass.py +++ b/datameta/views/setpass.py @@ -14,7 +14,7 @@ from pyramid.view import view_config -from .. import security +from ..security import tokenz from ..api.ui.forgot import send_forgot_token import datetime @@ -23,14 +23,14 @@ @view_config(route_name='setpass', renderer='../templates/setpass.pt') def v_setpass(request): # Validate token - dbtoken = security.get_password_reset_token(request.dbsession, request.matchdict['token']) + dbtoken = tokenz.get_password_reset_token(request.dbsession, request.matchdict['token']) unknown_token = dbtoken is None expired_token = dbtoken is not None and dbtoken.expires < datetime.datetime.now() # Token expired? Send new one. if expired_token: - db_token_obj, clear_token = security.get_new_password_reset_token(request.dbsession, dbtoken.user) + db_token_obj, clear_token = tokenz.get_new_password_reset_token(request.dbsession, dbtoken.user) send_forgot_token(request, db_token_obj, clear_token) return { From fb4a31b0310002b946d716498f04758d24b57c16 Mon Sep 17 00:00:00 2001 From: Christian Schudoma Date: Fri, 4 Mar 2022 10:20:40 +0100 Subject: [PATCH 32/32] pleasing flake8 --- datameta/security/__init__.py | 23 +++++++++++++++++------ datameta/security/pwdz.py | 3 --- datameta/security/tokenz.py | 2 +- 3 files changed, 18 insertions(+), 10 deletions(-) diff --git a/datameta/security/__init__.py b/datameta/security/__init__.py index 7b19fec6..3fcb5f87 100644 --- a/datameta/security/__init__.py +++ b/datameta/security/__init__.py @@ -23,10 +23,21 @@ from ..models import User, ApiKey, LoginAttempt from ..settings import get_setting -from . import tokenz, pwdz +from .pwdz import ( # noqa: F401 + register_password, + is_used_password, + verify_password, + hash_password, + check_password_by_hash + ) -from .tokenz import * -from .pwdz import * +from .tokenz import ( # noqa: F401 + generate_token, + get_new_password_reset_token, + get_new_password_reset_token_from_email, + hash_token, get_bearer_token, + get_password_reset_token + ) log = logging.getLogger(__name__) @@ -67,7 +78,7 @@ def get_user_by_credentials(request, email: str, password: str): db = request.dbsession user = db.query(User).filter(and_(User.email == email, User.enabled.is_(True))).one_or_none() if user: - if pwdz.check_password_by_hash(password, user.pwhash): + if check_password_by_hash(password, user.pwhash): log.warning(f"CLEARING FAILED LOGIN ATTEMPTS FOR gubc USER {user}") user.login_attempts.clear() return user @@ -80,7 +91,7 @@ def get_user_by_credentials(request, email: str, password: str): def revalidate_user_token_based(request, token): db = request.dbsession - token_hash = tokenz.hash_token(token) + token_hash = hash_token(token) apikey = db.query(ApiKey).join(User).filter(and_( ApiKey.value == token_hash, User.enabled.is_(True) @@ -141,7 +152,7 @@ def revalidate_user(request): raise a 401""" # Check for token based auth - token = tokenz.get_bearer_token(request) + token = get_bearer_token(request) if token is not None: return revalidate_user_token_based(request, token) diff --git a/datameta/security/pwdz.py b/datameta/security/pwdz.py index 413a5dfe..c0ae1697 100644 --- a/datameta/security/pwdz.py +++ b/datameta/security/pwdz.py @@ -17,9 +17,6 @@ from string import punctuation -from pyramid.httpexceptions import HTTPFound, HTTPUnauthorized -from sqlalchemy import and_ - from ..models import UsedPassword from ..settings import get_setting diff --git a/datameta/security/tokenz.py b/datameta/security/tokenz.py index e6594ceb..4c1c1882 100644 --- a/datameta/security/tokenz.py +++ b/datameta/security/tokenz.py @@ -18,7 +18,7 @@ from datetime import datetime, timedelta from random import choice -from string import ascii_letters, digits, punctuation +from string import ascii_letters, digits from sqlalchemy import and_