From 0cd5600163ecca9bfee6dfb9fb0c83bc7ae26168 Mon Sep 17 00:00:00 2001 From: ReAnzu Date: Sat, 24 Feb 2018 23:45:00 -0600 Subject: [PATCH 1/7] Changed password setup to use libsodium and argon2id * Regular SHA256 hashing for passwords is inadequate as modern GPU's can hash generate billions of hashes per second. * Added code to auto migrate old passwords to the new password_hash if the existing password_hash matches either of the legacy password generation schemes (SHA1 or SHA256). * Added migration to support new password_hash format length * Added auth tests --- server/requirements.txt | 1 + server/szurubooru/func/auth.py | 39 ++++++++++++++----- server/szurubooru/migrations/env.py | 10 ++++- ...increase_password_hash_max_field_length.py | 30 ++++++++++++++ server/szurubooru/model/user.py | 2 +- server/szurubooru/tests/conftest.py | 6 +-- server/szurubooru/tests/func/test_auth.py | 37 ++++++++++++++++++ 7 files changed, 110 insertions(+), 15 deletions(-) create mode 100644 server/szurubooru/migrations/versions/9ef1a1643c2a_increase_password_hash_max_field_length.py create mode 100644 server/szurubooru/tests/func/test_auth.py diff --git a/server/requirements.txt b/server/requirements.txt index 2cd15ec1..7cc47868 100644 --- a/server/requirements.txt +++ b/server/requirements.txt @@ -11,3 +11,4 @@ scipy>=0.18.1 elasticsearch>=5.0.0 elasticsearch-dsl>=5.0.0 scikit-image>=0.12 +pynacl>=1.2.1 \ No newline at end of file diff --git a/server/szurubooru/func/auth.py b/server/szurubooru/func/auth.py index 25c991c4..d2bb69ac 100644 --- a/server/szurubooru/func/auth.py +++ b/server/szurubooru/func/auth.py @@ -1,8 +1,11 @@ import hashlib import random from collections import OrderedDict -from szurubooru import config, model, errors +from nacl.exceptions import InvalidkeyError + +from szurubooru import config, model, errors, db from szurubooru.func import util +from nacl.pwhash import argon2id, verify RANK_MAP = OrderedDict([ @@ -17,7 +20,14 @@ RANK_MAP = OrderedDict([ def get_password_hash(salt: str, password: str) -> str: - ''' Retrieve new-style password hash. ''' + """ Retrieve argon2id password hash.""" + return argon2id.str( + (config.config['secret'] + salt + password).encode('utf8') + ).decode('utf8') + + +def get_sha256_legacy_password_hash(salt: str, password: str) -> str: + """ Retrieve old-style sha256 password hash.""" digest = hashlib.sha256() digest.update(config.config['secret'].encode('utf8')) digest.update(salt.encode('utf8')) @@ -25,8 +35,8 @@ def get_password_hash(salt: str, password: str) -> str: return digest.hexdigest() -def get_legacy_password_hash(salt: str, password: str) -> str: - ''' Retrieve old-style password hash. ''' +def get_sha1_legacy_password_hash(salt: str, password: str) -> str: + """ Retrieve old-style sha1 password hash.""" digest = hashlib.sha1() digest.update(b'1A2/$_4xVa') digest.update(salt.encode('utf8')) @@ -47,11 +57,22 @@ def create_password() -> str: def is_valid_password(user: model.User, password: str) -> bool: assert user salt, valid_hash = user.password_salt, user.password_hash - possible_hashes = [ - get_password_hash(salt, password), - get_legacy_password_hash(salt, password) - ] - return valid_hash in possible_hashes + + try: + return verify(user.password_hash.encode('utf8'), + (config.config['secret'] + salt + password).encode('utf8')) + except InvalidkeyError: + possible_hashes = [ + get_sha256_legacy_password_hash(salt, password), + get_sha1_legacy_password_hash(salt, password) + ] + if valid_hash in possible_hashes: + # Convert the user password hash to the new hash + user.password_hash = get_password_hash(salt, password) + db.session.commit() + return True + + return False def has_privilege(user: model.User, privilege_name: str) -> bool: diff --git a/server/szurubooru/migrations/env.py b/server/szurubooru/migrations/env.py index 7065a69e..61b20f4e 100644 --- a/server/szurubooru/migrations/env.py +++ b/server/szurubooru/migrations/env.py @@ -35,7 +35,11 @@ def run_migrations_offline(): ''' url = alembic_config.get_main_option('sqlalchemy.url') alembic.context.configure( - url=url, target_metadata=target_metadata, literal_binds=True) + url=url, + target_metadata=target_metadata, + literal_binds=True, + compare_type=True + ) with alembic.context.begin_transaction(): alembic.context.run_migrations() @@ -56,7 +60,9 @@ def run_migrations_online(): with connectable.connect() as connection: alembic.context.configure( connection=connection, - target_metadata=target_metadata) + target_metadata=target_metadata, + compare_type=True + ) with alembic.context.begin_transaction(): alembic.context.run_migrations() diff --git a/server/szurubooru/migrations/versions/9ef1a1643c2a_increase_password_hash_max_field_length.py b/server/szurubooru/migrations/versions/9ef1a1643c2a_increase_password_hash_max_field_length.py new file mode 100644 index 00000000..137b5911 --- /dev/null +++ b/server/szurubooru/migrations/versions/9ef1a1643c2a_increase_password_hash_max_field_length.py @@ -0,0 +1,30 @@ +''' +Alter the password_hash field to work with larger output. Particularly libsodium output for greater password security. + +Revision ID: 9ef1a1643c2a +Created at: 2018-02-24 23:00:32.848575 +''' + +import sqlalchemy as sa +from alembic import op + + +revision = '9ef1a1643c2a' +down_revision = '02ef5f73f4ab' +branch_labels = None +depends_on = None + + +def upgrade(): + + op.alter_column('user', 'password_hash', + existing_type=sa.VARCHAR(length=64), + type_=sa.Unicode(length=128), + existing_nullable=False) + + +def downgrade(): + op.alter_column('user', 'password_hash', + existing_type=sa.Unicode(length=128), + type_=sa.VARCHAR(length=64), + existing_nullable=False) diff --git a/server/szurubooru/model/user.py b/server/szurubooru/model/user.py index dd7c0629..d355a143 100644 --- a/server/szurubooru/model/user.py +++ b/server/szurubooru/model/user.py @@ -23,7 +23,7 @@ class User(Base): last_login_time = sa.Column('last_login_time', sa.DateTime) version = sa.Column('version', sa.Integer, default=1, nullable=False) name = sa.Column('name', sa.Unicode(50), nullable=False, unique=True) - password_hash = sa.Column('password_hash', sa.Unicode(64), nullable=False) + password_hash = sa.Column('password_hash', sa.Unicode(128), nullable=False) password_salt = sa.Column('password_salt', sa.Unicode(32)) email = sa.Column('email', sa.Unicode(64), nullable=True) rank = sa.Column('rank', sa.Unicode(32), nullable=False) diff --git a/server/szurubooru/tests/conftest.py b/server/szurubooru/tests/conftest.py index e71f9609..aa26b731 100644 --- a/server/szurubooru/tests/conftest.py +++ b/server/szurubooru/tests/conftest.py @@ -115,11 +115,11 @@ def config_injector(): @pytest.fixture def user_factory(): - def factory(name=None, rank=model.User.RANK_REGULAR, email='dummy'): + def factory(name=None, rank=model.User.RANK_REGULAR, email='dummy', password_salt=None, password_hash=None): user = model.User() user.name = name or get_unique_name() - user.password_salt = 'dummy' - user.password_hash = 'dummy' + user.password_salt = password_salt or 'dummy' + user.password_hash = password_hash or 'dummy' user.email = email user.rank = rank user.creation_time = datetime(1997, 1, 1) diff --git a/server/szurubooru/tests/func/test_auth.py b/server/szurubooru/tests/func/test_auth.py new file mode 100644 index 00000000..9bc78d7a --- /dev/null +++ b/server/szurubooru/tests/func/test_auth.py @@ -0,0 +1,37 @@ +from szurubooru.func import auth +import pytest + + +@pytest.fixture(autouse=True) +def inject_config(config_injector): + config_injector({'secret': 'testSecret'}) + + +def test_get_password_hash(): + salt, password = ('testSalt', 'pass') + result = auth.get_password_hash(salt, password) + assert result + hash_parts = list(filter(lambda e: e is not None and e != '', result.split('$'))) + assert len(hash_parts) == 5 + assert hash_parts[0] == 'argon2id' + + +def test_get_sha256_legacy_password_hash(): + salt, password = ('testSalt', 'pass') + result = auth.get_sha256_legacy_password_hash(salt, password) + assert result == '2031ac9631353ac9303719a7f808a24f79aa1d71712c98523e4bb4cce579428a' + + +def test_get_sha1_legacy_password_hash(): + salt, password = ('testSalt', 'pass') + result = auth.get_sha1_legacy_password_hash(salt, password) + assert result == '1eb1f953d9be303a1b54627e903e6124cfb1245b' + + +def test_is_valid_password_auto_upgrades_user_password_hash_on_success_of_legacy_hash(user_factory): + salt, password = ('testSalt', 'pass') + legacy_password_hash = auth.get_sha256_legacy_password_hash(salt, password) + user = user_factory(password_salt=salt, password_hash=legacy_password_hash) + result = auth.is_valid_password(user, password) + assert result is True + assert user.password_hash != legacy_password_hash From 3276662c39c5dad1db9e0b1f686d3d5a8970ddae Mon Sep 17 00:00:00 2001 From: ReAnzu Date: Thu, 1 Mar 2018 02:24:15 -0600 Subject: [PATCH 2/7] Address pull request comments * Added column password_revision. This field will default to 0, which all passwords will have till they're updated. After that each password hash method has a revision. * appeased pycodestyle --- server/szurubooru/func/auth.py | 37 +++++++++++-------- server/szurubooru/func/users.py | 8 +++- ...increase_password_hash_max_field_length.py | 30 --------------- ...pdate_user_table_for_hardened_passwords.py | 34 +++++++++++++++++ server/szurubooru/model/user.py | 2 + server/szurubooru/tests/conftest.py | 6 ++- server/szurubooru/tests/func/test_auth.py | 24 +++++++----- server/szurubooru/tests/func/test_users.py | 6 ++- 8 files changed, 87 insertions(+), 60 deletions(-) delete mode 100644 server/szurubooru/migrations/versions/9ef1a1643c2a_increase_password_hash_max_field_length.py create mode 100644 server/szurubooru/migrations/versions/9ef1a1643c2a_update_user_table_for_hardened_passwords.py diff --git a/server/szurubooru/func/auth.py b/server/szurubooru/func/auth.py index d2bb69ac..55b4db39 100644 --- a/server/szurubooru/func/auth.py +++ b/server/szurubooru/func/auth.py @@ -1,3 +1,5 @@ +from typing import Tuple + import hashlib import random from collections import OrderedDict @@ -5,7 +7,7 @@ from nacl.exceptions import InvalidkeyError from szurubooru import config, model, errors, db from szurubooru.func import util -from nacl.pwhash import argon2id, verify +from nacl import pwhash RANK_MAP = OrderedDict([ @@ -19,29 +21,29 @@ RANK_MAP = OrderedDict([ ]) -def get_password_hash(salt: str, password: str) -> str: - """ Retrieve argon2id password hash.""" - return argon2id.str( +def get_password_hash(salt: str, password: str) -> Tuple[str, int]: + ''' Retrieve argon2id password hash. ''' + return pwhash.argon2id.str( (config.config['secret'] + salt + password).encode('utf8') - ).decode('utf8') + ).decode('utf8'), 3 -def get_sha256_legacy_password_hash(salt: str, password: str) -> str: - """ Retrieve old-style sha256 password hash.""" +def get_sha256_legacy_password_hash(salt: str, password: str) -> Tuple[str, int]: + ''' Retrieve old-style sha256 password hash. ''' digest = hashlib.sha256() digest.update(config.config['secret'].encode('utf8')) digest.update(salt.encode('utf8')) digest.update(password.encode('utf8')) - return digest.hexdigest() + return digest.hexdigest(), 2 -def get_sha1_legacy_password_hash(salt: str, password: str) -> str: - """ Retrieve old-style sha1 password hash.""" +def get_sha1_legacy_password_hash(salt: str, password: str) -> Tuple[str, int]: + ''' Retrieve old-style sha1 password hash. ''' digest = hashlib.sha1() digest.update(b'1A2/$_4xVa') digest.update(salt.encode('utf8')) digest.update(password.encode('utf8')) - return digest.hexdigest() + return digest.hexdigest(), 1 def create_password() -> str: @@ -59,16 +61,19 @@ def is_valid_password(user: model.User, password: str) -> bool: salt, valid_hash = user.password_salt, user.password_hash try: - return verify(user.password_hash.encode('utf8'), - (config.config['secret'] + salt + password).encode('utf8')) + return pwhash.verify( + user.password_hash.encode('utf8'), + (config.config['secret'] + salt + password).encode('utf8')) except InvalidkeyError: possible_hashes = [ - get_sha256_legacy_password_hash(salt, password), - get_sha1_legacy_password_hash(salt, password) + get_sha256_legacy_password_hash(salt, password)[0], + get_sha1_legacy_password_hash(salt, password)[0] ] if valid_hash in possible_hashes: # Convert the user password hash to the new hash - user.password_hash = get_password_hash(salt, password) + new_hash, revision = get_password_hash(salt, password) + user.password_hash = new_hash + user.password_revision = revision db.session.commit() return True diff --git a/server/szurubooru/func/users.py b/server/szurubooru/func/users.py index ba6f67f2..2b744e28 100644 --- a/server/szurubooru/func/users.py +++ b/server/szurubooru/func/users.py @@ -243,7 +243,9 @@ def update_user_password(user: model.User, password: str) -> None: raise InvalidPasswordError( 'Password must satisfy regex %r.' % password_regex) user.password_salt = auth.create_password() - user.password_hash = auth.get_password_hash(user.password_salt, password) + hash, revision = auth.get_password_hash(user.password_salt, password) + user.password_hash = hash + user.password_revision = revision def update_user_email(user: model.User, email: str) -> None: @@ -308,5 +310,7 @@ def reset_user_password(user: model.User) -> str: assert user password = auth.create_password() user.password_salt = auth.create_password() - user.password_hash = auth.get_password_hash(user.password_salt, password) + hash, revision = auth.get_password_hash(user.password_salt, password) + user.password_hash = hash + user.password_revision = revision return password diff --git a/server/szurubooru/migrations/versions/9ef1a1643c2a_increase_password_hash_max_field_length.py b/server/szurubooru/migrations/versions/9ef1a1643c2a_increase_password_hash_max_field_length.py deleted file mode 100644 index 137b5911..00000000 --- a/server/szurubooru/migrations/versions/9ef1a1643c2a_increase_password_hash_max_field_length.py +++ /dev/null @@ -1,30 +0,0 @@ -''' -Alter the password_hash field to work with larger output. Particularly libsodium output for greater password security. - -Revision ID: 9ef1a1643c2a -Created at: 2018-02-24 23:00:32.848575 -''' - -import sqlalchemy as sa -from alembic import op - - -revision = '9ef1a1643c2a' -down_revision = '02ef5f73f4ab' -branch_labels = None -depends_on = None - - -def upgrade(): - - op.alter_column('user', 'password_hash', - existing_type=sa.VARCHAR(length=64), - type_=sa.Unicode(length=128), - existing_nullable=False) - - -def downgrade(): - op.alter_column('user', 'password_hash', - existing_type=sa.Unicode(length=128), - type_=sa.VARCHAR(length=64), - existing_nullable=False) diff --git a/server/szurubooru/migrations/versions/9ef1a1643c2a_update_user_table_for_hardened_passwords.py b/server/szurubooru/migrations/versions/9ef1a1643c2a_update_user_table_for_hardened_passwords.py new file mode 100644 index 00000000..4ae9bf2b --- /dev/null +++ b/server/szurubooru/migrations/versions/9ef1a1643c2a_update_user_table_for_hardened_passwords.py @@ -0,0 +1,34 @@ +''' +Alter the password_hash field to work with larger output. +Particularly libsodium output for greater password security. + +Revision ID: 9ef1a1643c2a +Created at: 2018-02-24 23:00:32.848575 +''' + +import sqlalchemy as sa +from alembic import op + + +revision = '9ef1a1643c2a' +down_revision = '02ef5f73f4ab' +branch_labels = None +depends_on = None + + +def upgrade(): + op.alter_column('user', 'password_hash', + existing_type=sa.VARCHAR(length=64), + type_=sa.Unicode(length=128), + existing_nullable=False) + op.add_column('user', sa.Column('password_revision', + sa.SmallInteger(), + nullable=False)) + + +def downgrade(): + op.alter_column('user', 'password_hash', + existing_type=sa.Unicode(length=128), + type_=sa.VARCHAR(length=64), + existing_nullable=False) + op.drop_column('user', 'password_revision') diff --git a/server/szurubooru/model/user.py b/server/szurubooru/model/user.py index d355a143..be95b0b1 100644 --- a/server/szurubooru/model/user.py +++ b/server/szurubooru/model/user.py @@ -25,6 +25,8 @@ class User(Base): name = sa.Column('name', sa.Unicode(50), nullable=False, unique=True) password_hash = sa.Column('password_hash', sa.Unicode(128), nullable=False) password_salt = sa.Column('password_salt', sa.Unicode(32)) + password_revision = sa.Column('password_revision', sa.SmallInteger, + default=0, nullable=False) email = sa.Column('email', sa.Unicode(64), nullable=True) rank = sa.Column('rank', sa.Unicode(32), nullable=False) avatar_style = sa.Column( diff --git a/server/szurubooru/tests/conftest.py b/server/szurubooru/tests/conftest.py index aa26b731..44ded329 100644 --- a/server/szurubooru/tests/conftest.py +++ b/server/szurubooru/tests/conftest.py @@ -115,7 +115,11 @@ def config_injector(): @pytest.fixture def user_factory(): - def factory(name=None, rank=model.User.RANK_REGULAR, email='dummy', password_salt=None, password_hash=None): + def factory(name=None, + rank=model.User.RANK_REGULAR, + email='dummy', + password_salt=None, + password_hash=None): user = model.User() user.name = name or get_unique_name() user.password_salt = password_salt or 'dummy' diff --git a/server/szurubooru/tests/func/test_auth.py b/server/szurubooru/tests/func/test_auth.py index 9bc78d7a..5d0955ad 100644 --- a/server/szurubooru/tests/func/test_auth.py +++ b/server/szurubooru/tests/func/test_auth.py @@ -9,29 +9,35 @@ def inject_config(config_injector): def test_get_password_hash(): salt, password = ('testSalt', 'pass') - result = auth.get_password_hash(salt, password) + result, revision = auth.get_password_hash(salt, password) assert result - hash_parts = list(filter(lambda e: e is not None and e != '', result.split('$'))) + assert revision == 3 + hash_parts = list( + filter(lambda e: e is not None and e != '', result.split('$'))) assert len(hash_parts) == 5 assert hash_parts[0] == 'argon2id' def test_get_sha256_legacy_password_hash(): salt, password = ('testSalt', 'pass') - result = auth.get_sha256_legacy_password_hash(salt, password) - assert result == '2031ac9631353ac9303719a7f808a24f79aa1d71712c98523e4bb4cce579428a' + result, revision = auth.get_sha256_legacy_password_hash(salt, password) + hash = '2031ac9631353ac9303719a7f808a24f79aa1d71712c98523e4bb4cce579428a' + assert result == hash + assert revision == 2 def test_get_sha1_legacy_password_hash(): salt, password = ('testSalt', 'pass') - result = auth.get_sha1_legacy_password_hash(salt, password) + result, revision = auth.get_sha1_legacy_password_hash(salt, password) assert result == '1eb1f953d9be303a1b54627e903e6124cfb1245b' + assert revision == 1 -def test_is_valid_password_auto_upgrades_user_password_hash_on_success_of_legacy_hash(user_factory): +def test_is_valid_password_auto_upgrades_user_password_hash(user_factory): salt, password = ('testSalt', 'pass') - legacy_password_hash = auth.get_sha256_legacy_password_hash(salt, password) - user = user_factory(password_salt=salt, password_hash=legacy_password_hash) + hash, revision = auth.get_sha256_legacy_password_hash(salt, password) + user = user_factory(password_salt=salt, password_hash=hash) result = auth.is_valid_password(user, password) assert result is True - assert user.password_hash != legacy_password_hash + assert user.password_hash != hash + assert user.password_revision > revision diff --git a/server/szurubooru/tests/func/test_users.py b/server/szurubooru/tests/func/test_users.py index 76c52c8b..55061276 100644 --- a/server/szurubooru/tests/func/test_users.py +++ b/server/szurubooru/tests/func/test_users.py @@ -320,10 +320,11 @@ def test_update_user_password(user_factory, config_injector): with patch('szurubooru.func.auth.create_password'), \ patch('szurubooru.func.auth.get_password_hash'): auth.create_password.return_value = 'salt' - auth.get_password_hash.return_value = 'hash' + auth.get_password_hash.return_value = ('hash', 3) users.update_user_password(user, 'a') assert user.password_salt == 'salt' assert user.password_hash == 'hash' + assert user.password_revision == 3 def test_update_user_email_with_too_long_string(user_factory): @@ -447,7 +448,8 @@ def test_reset_user_password(user_factory): patch('szurubooru.func.auth.get_password_hash'): user = user_factory() auth.create_password.return_value = 'salt' - auth.get_password_hash.return_value = 'hash' + auth.get_password_hash.return_value = ('hash', 3) users.reset_user_password(user) assert user.password_salt == 'salt' assert user.password_hash == 'hash' + assert user.password_revision == 3 From 76c5c202b11e4f703ffe54fd37f9237684e25ff3 Mon Sep 17 00:00:00 2001 From: ReAnzu Date: Fri, 2 Mar 2018 22:47:22 -0600 Subject: [PATCH 3/7] Updated migration * Modified migration to do an in place update of the password revision based on the hash length --- ...pdate_user_table_for_hardened_passwords.py | 49 ++++++++++++++++++- 1 file changed, 48 insertions(+), 1 deletion(-) diff --git a/server/szurubooru/migrations/versions/9ef1a1643c2a_update_user_table_for_hardened_passwords.py b/server/szurubooru/migrations/versions/9ef1a1643c2a_update_user_table_for_hardened_passwords.py index 4ae9bf2b..ce3b2e95 100644 --- a/server/szurubooru/migrations/versions/9ef1a1643c2a_update_user_table_for_hardened_passwords.py +++ b/server/szurubooru/migrations/versions/9ef1a1643c2a_update_user_table_for_hardened_passwords.py @@ -7,6 +7,8 @@ Created at: 2018-02-24 23:00:32.848575 ''' import sqlalchemy as sa +import sqlalchemy.ext.declarative +import sqlalchemy.orm.session from alembic import op @@ -15,6 +17,34 @@ down_revision = '02ef5f73f4ab' branch_labels = None depends_on = None +Base = sa.ext.declarative.declarative_base() + + +class User(Base): + __tablename__ = 'user' + + AVATAR_GRAVATAR = 'gravatar' + + user_id = sa.Column('id', sa.Integer, primary_key=True) + creation_time = sa.Column('creation_time', sa.DateTime, nullable=False) + last_login_time = sa.Column('last_login_time', sa.DateTime) + version = sa.Column('version', sa.Integer, default=1, nullable=False) + name = sa.Column('name', sa.Unicode(50), nullable=False, unique=True) + password_hash = sa.Column('password_hash', sa.Unicode(128), nullable=False) + password_salt = sa.Column('password_salt', sa.Unicode(32)) + password_revision = sa.Column('password_revision', sa.SmallInteger, + default=0, nullable=False) + email = sa.Column('email', sa.Unicode(64), nullable=True) + rank = sa.Column('rank', sa.Unicode(32), nullable=False) + avatar_style = sa.Column( + 'avatar_style', sa.Unicode(32), nullable=False, + default=AVATAR_GRAVATAR) + + __mapper_args__ = { + 'version_id_col': version, + 'version_id_generator': False, + } + def upgrade(): op.alter_column('user', 'password_hash', @@ -23,7 +53,24 @@ def upgrade(): existing_nullable=False) op.add_column('user', sa.Column('password_revision', sa.SmallInteger(), - nullable=False)) + nullable=True, + default=0)) + + session = sa.orm.session.Session(bind=op.get_bind()) + if session.query(User).count() >= 0: + for user in session.query(User).all(): + password_hash_length = len(user.password_hash) + if password_hash_length == 40: + user.password_revision = 1 + elif password_hash_length == 64: + user.password_revision = 2 + else: + user.password_revision = 3 + session.flush() + session.commit() + + op.alter_column('user', 'password_revision', + existing_nullable=True, nullable=False) def downgrade(): From bedc03b18f4325dbd599068dd4b5e9073a9e5cb8 Mon Sep 17 00:00:00 2001 From: ReAnzu Date: Fri, 2 Mar 2018 22:50:20 -0600 Subject: [PATCH 4/7] Revision 3 can't exist yet, default to 0 for other lengths. --- .../9ef1a1643c2a_update_user_table_for_hardened_passwords.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/szurubooru/migrations/versions/9ef1a1643c2a_update_user_table_for_hardened_passwords.py b/server/szurubooru/migrations/versions/9ef1a1643c2a_update_user_table_for_hardened_passwords.py index ce3b2e95..33aa9c93 100644 --- a/server/szurubooru/migrations/versions/9ef1a1643c2a_update_user_table_for_hardened_passwords.py +++ b/server/szurubooru/migrations/versions/9ef1a1643c2a_update_user_table_for_hardened_passwords.py @@ -65,7 +65,7 @@ def upgrade(): elif password_hash_length == 64: user.password_revision = 2 else: - user.password_revision = 3 + user.password_revision = 0 session.flush() session.commit() From 9c13c6ae562a392c04c97d431bd8a9a488471266 Mon Sep 17 00:00:00 2001 From: ReAnzu Date: Wed, 7 Mar 2018 19:32:28 -0600 Subject: [PATCH 5/7] Resolved code formatting change requests --- server/szurubooru/func/auth.py | 4 +-- server/szurubooru/func/users.py | 6 ++-- server/szurubooru/migrations/env.py | 6 ++-- ...pdate_user_table_for_hardened_passwords.py | 36 +++++++++++-------- server/szurubooru/model/user.py | 4 +-- server/szurubooru/tests/conftest.py | 11 +++--- 6 files changed, 37 insertions(+), 30 deletions(-) diff --git a/server/szurubooru/func/auth.py b/server/szurubooru/func/auth.py index 55b4db39..c9740fe0 100644 --- a/server/szurubooru/func/auth.py +++ b/server/szurubooru/func/auth.py @@ -1,13 +1,11 @@ from typing import Tuple - import hashlib import random from collections import OrderedDict +from nacl import pwhash from nacl.exceptions import InvalidkeyError - from szurubooru import config, model, errors, db from szurubooru.func import util -from nacl import pwhash RANK_MAP = OrderedDict([ diff --git a/server/szurubooru/func/users.py b/server/szurubooru/func/users.py index 2b744e28..5fe9f2ca 100644 --- a/server/szurubooru/func/users.py +++ b/server/szurubooru/func/users.py @@ -243,8 +243,10 @@ def update_user_password(user: model.User, password: str) -> None: raise InvalidPasswordError( 'Password must satisfy regex %r.' % password_regex) user.password_salt = auth.create_password() - hash, revision = auth.get_password_hash(user.password_salt, password) - user.password_hash = hash + password_hash, revision = auth.get_password_hash( + user.password_salt, + password) + user.password_hash = password_hash user.password_revision = revision diff --git a/server/szurubooru/migrations/env.py b/server/szurubooru/migrations/env.py index 61b20f4e..59d031f6 100644 --- a/server/szurubooru/migrations/env.py +++ b/server/szurubooru/migrations/env.py @@ -38,8 +38,7 @@ def run_migrations_offline(): url=url, target_metadata=target_metadata, literal_binds=True, - compare_type=True - ) + compare_type=True) with alembic.context.begin_transaction(): alembic.context.run_migrations() @@ -61,8 +60,7 @@ def run_migrations_online(): alembic.context.configure( connection=connection, target_metadata=target_metadata, - compare_type=True - ) + compare_type=True) with alembic.context.begin_transaction(): alembic.context.run_migrations() diff --git a/server/szurubooru/migrations/versions/9ef1a1643c2a_update_user_table_for_hardened_passwords.py b/server/szurubooru/migrations/versions/9ef1a1643c2a_update_user_table_for_hardened_passwords.py index 33aa9c93..084f196e 100644 --- a/server/szurubooru/migrations/versions/9ef1a1643c2a_update_user_table_for_hardened_passwords.py +++ b/server/szurubooru/migrations/versions/9ef1a1643c2a_update_user_table_for_hardened_passwords.py @@ -47,14 +47,17 @@ class User(Base): def upgrade(): - op.alter_column('user', 'password_hash', - existing_type=sa.VARCHAR(length=64), - type_=sa.Unicode(length=128), - existing_nullable=False) - op.add_column('user', sa.Column('password_revision', - sa.SmallInteger(), - nullable=True, - default=0)) + op.alter_column( + 'user', + 'password_hash', + existing_type=sa.VARCHAR(length=64), + type_=sa.Unicode(length=128), + existing_nullable=False) + op.add_column('user', sa.Column( + 'password_revision', + sa.SmallInteger(), + nullable=True, + default=0)) session = sa.orm.session.Session(bind=op.get_bind()) if session.query(User).count() >= 0: @@ -69,13 +72,18 @@ def upgrade(): session.flush() session.commit() - op.alter_column('user', 'password_revision', - existing_nullable=True, nullable=False) + op.alter_column( + 'user', + 'password_revision', + existing_nullable=True, + nullable=False) def downgrade(): - op.alter_column('user', 'password_hash', - existing_type=sa.Unicode(length=128), - type_=sa.VARCHAR(length=64), - existing_nullable=False) + op.alter_column( + 'user', + 'password_hash', + existing_type=sa.Unicode(length=128), + type_=sa.VARCHAR(length=64), + existing_nullable=False) op.drop_column('user', 'password_revision') diff --git a/server/szurubooru/model/user.py b/server/szurubooru/model/user.py index be95b0b1..39c5a91b 100644 --- a/server/szurubooru/model/user.py +++ b/server/szurubooru/model/user.py @@ -25,8 +25,8 @@ class User(Base): name = sa.Column('name', sa.Unicode(50), nullable=False, unique=True) password_hash = sa.Column('password_hash', sa.Unicode(128), nullable=False) password_salt = sa.Column('password_salt', sa.Unicode(32)) - password_revision = sa.Column('password_revision', sa.SmallInteger, - default=0, nullable=False) + password_revision = sa.Column( + 'password_revision', sa.SmallInteger, default=0, nullable=False) email = sa.Column('email', sa.Unicode(64), nullable=True) rank = sa.Column('rank', sa.Unicode(32), nullable=False) avatar_style = sa.Column( diff --git a/server/szurubooru/tests/conftest.py b/server/szurubooru/tests/conftest.py index 44ded329..db7806e8 100644 --- a/server/szurubooru/tests/conftest.py +++ b/server/szurubooru/tests/conftest.py @@ -115,11 +115,12 @@ def config_injector(): @pytest.fixture def user_factory(): - def factory(name=None, - rank=model.User.RANK_REGULAR, - email='dummy', - password_salt=None, - password_hash=None): + def factory( + name=None, + rank=model.User.RANK_REGULAR, + email='dummy', + password_salt=None, + password_hash=None): user = model.User() user.name = name or get_unique_name() user.password_salt = password_salt or 'dummy' From 5a585cb01d73e968ace0dc8f017fe65f5c61a6f0 Mon Sep 17 00:00:00 2001 From: ReAnzu Date: Wed, 7 Mar 2018 19:55:38 -0600 Subject: [PATCH 6/7] Missed name shadowing issue --- server/szurubooru/func/users.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/server/szurubooru/func/users.py b/server/szurubooru/func/users.py index 5fe9f2ca..012debca 100644 --- a/server/szurubooru/func/users.py +++ b/server/szurubooru/func/users.py @@ -244,8 +244,7 @@ def update_user_password(user: model.User, password: str) -> None: 'Password must satisfy regex %r.' % password_regex) user.password_salt = auth.create_password() password_hash, revision = auth.get_password_hash( - user.password_salt, - password) + user.password_salt, password) user.password_hash = password_hash user.password_revision = revision @@ -312,7 +311,8 @@ def reset_user_password(user: model.User) -> str: assert user password = auth.create_password() user.password_salt = auth.create_password() - hash, revision = auth.get_password_hash(user.password_salt, password) - user.password_hash = hash + password_hash, revision = auth.get_password_hash( + user.password_salt, password) + user.password_hash = password_hash user.password_revision = revision return password From 72ff93784b95b57fbfbfcd55b726e89979f6448f Mon Sep 17 00:00:00 2001 From: ReAnzu Date: Thu, 8 Mar 2018 16:36:23 -0600 Subject: [PATCH 7/7] server/migrations: Address code review comments --- .../9ef1a1643c2a_update_user_table_for_hardened_passwords.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/szurubooru/migrations/versions/9ef1a1643c2a_update_user_table_for_hardened_passwords.py b/server/szurubooru/migrations/versions/9ef1a1643c2a_update_user_table_for_hardened_passwords.py index 084f196e..38057728 100644 --- a/server/szurubooru/migrations/versions/9ef1a1643c2a_update_user_table_for_hardened_passwords.py +++ b/server/szurubooru/migrations/versions/9ef1a1643c2a_update_user_table_for_hardened_passwords.py @@ -32,8 +32,8 @@ class User(Base): name = sa.Column('name', sa.Unicode(50), nullable=False, unique=True) password_hash = sa.Column('password_hash', sa.Unicode(128), nullable=False) password_salt = sa.Column('password_salt', sa.Unicode(32)) - password_revision = sa.Column('password_revision', sa.SmallInteger, - default=0, nullable=False) + password_revision = sa.Column( + 'password_revision', sa.SmallInteger, default=0, nullable=False) email = sa.Column('email', sa.Unicode(64), nullable=True) rank = sa.Column('rank', sa.Unicode(32), nullable=False) avatar_style = sa.Column(