From bba58352e07c36df3ebdcfa0fb17d1f8e58fc5c3 Mon Sep 17 00:00:00 2001 From: Evgeniy Alekseev Date: Mon, 28 Nov 2022 00:02:53 +0200 Subject: [PATCH] do not invoke configuration write in case if no salt or user was written --- src/ahriman/application/ahriman.py | 6 +- src/ahriman/application/handlers/users.py | 20 +++--- .../handlers/test_handler_users.py | 63 ++++++++++++++++--- 3 files changed, 70 insertions(+), 19 deletions(-) diff --git a/src/ahriman/application/ahriman.py b/src/ahriman/application/ahriman.py index a9455241..52f25940 100644 --- a/src/ahriman/application/ahriman.py +++ b/src/ahriman/application/ahriman.py @@ -760,7 +760,7 @@ def _set_shell_parser(root: SubParserAction) -> argparse.ArgumentParser: Returns: argparse.ArgumentParser: created argument parser """ - parser = root.add_parser("shell", help="envoke python shell", + parser = root.add_parser("shell", help="invoke python shell", description="drop into python shell while having created application", formatter_class=_formatter) parser.add_argument("code", help="instead of dropping into shell, just execute the specified code", nargs="?") @@ -782,6 +782,9 @@ def _set_user_add_parser(root: SubParserAction) -> argparse.ArgumentParser: parser = root.add_parser("user-add", help="create or update user", description="update user for web services with the given password and role. " "In case if password was not entered it will be asked interactively", + epilog="In case of first run (i.e. if password salt is not set yet) or if ``as-service`` " + "flag is supplied, this action requires root privileges because it performs write " + "to filesystem configuration.", formatter_class=_formatter) parser.add_argument("username", help="username for web service") parser.add_argument("--as-service", help="add user as service user", action="store_true") @@ -830,7 +833,6 @@ def _set_user_remove_parser(root: SubParserAction) -> argparse.ArgumentParser: description="remove user from the user mapping and update the configuration", formatter_class=_formatter) parser.add_argument("username", help="username for web service") - parser.add_argument("-s", "--secure", help="set file permissions to user-only", action="store_true") parser.set_defaults(handler=handlers.Users, action=Action.Remove, architecture=[""], lock=None, report=False, # nosec password="", quiet=True, unsafe=True) return parser diff --git a/src/ahriman/application/handlers/users.py b/src/ahriman/application/handlers/users.py index bf8a5b4b..0188e01f 100644 --- a/src/ahriman/application/handlers/users.py +++ b/src/ahriman/application/handlers/users.py @@ -21,7 +21,7 @@ import argparse import getpass from pathlib import Path -from typing import Type +from typing import Optional, Tuple, Type from ahriman.application.handlers import Handler from ahriman.core.configuration import Configuration @@ -55,12 +55,13 @@ class Users(Handler): database = SQLite.load(configuration) if args.action == Action.Update: - salt = Users.get_salt(configuration) + old_salt, salt = Users.get_salt(configuration) user = Users.user_create(args) - auth_configuration = Users.configuration_get(configuration.include) + if old_salt is None or args.as_service: + auth_configuration = Users.configuration_get(configuration.include) + Users.configuration_create(auth_configuration, user, salt, args.as_service, args.secure) - Users.configuration_create(auth_configuration, user, salt, args.as_service, args.secure) database.user_update(user.hash_password(salt)) elif args.action == Action.List: users = database.user_list(args.username, args.role) @@ -100,7 +101,7 @@ class Users(Handler): Returns: Configuration: configuration instance. In case if there are local settings they will be loaded """ - target = include_path / "auth.ini" + target = include_path / "00-auth.ini" configuration = Configuration() configuration.load(target) @@ -124,7 +125,7 @@ class Users(Handler): path.chmod(0o600) @staticmethod - def get_salt(configuration: Configuration, salt_length: int = 20) -> str: + def get_salt(configuration: Configuration, salt_length: int = 20) -> Tuple[Optional[str], str]: """ get salt from configuration or create new string @@ -133,11 +134,12 @@ class Users(Handler): salt_length(int, optional): salt length (Default value = 20) Returns: - str: current salt + Tuple[Optional[str], str]: tuple containing salt from configuration if any and actual salt which must be + used for password hash """ if salt := configuration.get("auth", "salt", fallback=None): - return salt - return User.generate_password(salt_length) + return salt, salt + return None, User.generate_password(salt_length) @staticmethod def user_create(args: argparse.Namespace) -> User: diff --git a/tests/ahriman/application/handlers/test_handler_users.py b/tests/ahriman/application/handlers/test_handler_users.py index 0990b6c0..486ddd43 100644 --- a/tests/ahriman/application/handlers/test_handler_users.py +++ b/tests/ahriman/application/handlers/test_handler_users.py @@ -45,7 +45,55 @@ def test_run(args: argparse.Namespace, configuration: Configuration, database: S get_auth_configuration_mock = mocker.patch("ahriman.application.handlers.Users.configuration_get") create_configuration_mock = mocker.patch("ahriman.application.handlers.Users.configuration_create") create_user_mock = mocker.patch("ahriman.application.handlers.Users.user_create", return_value=user) - get_salt_mock = mocker.patch("ahriman.application.handlers.Users.get_salt", return_value="salt") + get_salt_mock = mocker.patch("ahriman.application.handlers.Users.get_salt", return_value=("salt", "salt")) + update_mock = mocker.patch("ahriman.core.database.SQLite.user_update") + + Users.run(args, "x86_64", configuration, report=False, unsafe=False) + get_auth_configuration_mock.assert_not_called() + create_configuration_mock.assert_not_called() + create_user_mock.assert_called_once_with(args) + get_salt_mock.assert_called_once_with(configuration) + update_mock.assert_called_once_with(user) + + +def test_run_empty_salt(args: argparse.Namespace, configuration: Configuration, database: SQLite, + mocker: MockerFixture) -> None: + """ + must create configuration if salt was not set + """ + args = _default_args(args) + user = User(username=args.username, password=args.password, access=args.role) + mocker.patch("ahriman.core.database.SQLite.load", return_value=database) + mocker.patch("ahriman.models.user.User.hash_password", return_value=user) + get_auth_configuration_mock = mocker.patch("ahriman.application.handlers.Users.configuration_get") + create_configuration_mock = mocker.patch("ahriman.application.handlers.Users.configuration_create") + create_user_mock = mocker.patch("ahriman.application.handlers.Users.user_create", return_value=user) + get_salt_mock = mocker.patch("ahriman.application.handlers.Users.get_salt", return_value=(None, "salt")) + update_mock = mocker.patch("ahriman.core.database.SQLite.user_update") + + Users.run(args, "x86_64", configuration, report=False, unsafe=False) + get_auth_configuration_mock.assert_called_once_with(configuration.include) + create_configuration_mock.assert_called_once_with(pytest.helpers.anyvar(int), pytest.helpers.anyvar(int), + pytest.helpers.anyvar(int), args.as_service, args.secure) + create_user_mock.assert_called_once_with(args) + get_salt_mock.assert_called_once_with(configuration) + update_mock.assert_called_once_with(user) + + +def test_run_service_user(args: argparse.Namespace, configuration: Configuration, database: SQLite, + mocker: MockerFixture) -> None: + """ + must create configuration if as service argument is provided + """ + args = _default_args(args) + args.as_service = True + user = User(username=args.username, password=args.password, access=args.role) + mocker.patch("ahriman.core.database.SQLite.load", return_value=database) + mocker.patch("ahriman.models.user.User.hash_password", return_value=user) + get_auth_configuration_mock = mocker.patch("ahriman.application.handlers.Users.configuration_get") + create_configuration_mock = mocker.patch("ahriman.application.handlers.Users.configuration_create") + create_user_mock = mocker.patch("ahriman.application.handlers.Users.user_create", return_value=user) + get_salt_mock = mocker.patch("ahriman.application.handlers.Users.get_salt", return_value=("salt", "salt")) update_mock = mocker.patch("ahriman.core.database.SQLite.user_update") Users.run(args, "x86_64", configuration, report=False, unsafe=False) @@ -116,10 +164,8 @@ def test_configuration_create(configuration: Configuration, user: User, mocker: write_mock.assert_called_once_with(configuration, False) -def test_configuration_create_with_plain_password( - configuration: Configuration, - user: User, - mocker: MockerFixture) -> None: +def test_configuration_create_with_plain_password(configuration: Configuration, user: User, + mocker: MockerFixture) -> None: """ must set plain text password and user for the service """ @@ -142,7 +188,7 @@ def test_configuration_get(mocker: MockerFixture) -> None: read_mock = mocker.patch("ahriman.core.configuration.Configuration.read") assert Users.configuration_get(Path("path")) - read_mock.assert_called_once_with(Path("path") / "auth.ini") + read_mock.assert_called_once_with(Path("path") / "00-auth.ini") def test_configuration_write(configuration: Configuration, mocker: MockerFixture) -> None: @@ -185,7 +231,7 @@ def test_get_salt_read(configuration: Configuration) -> None: """ must read salt from configuration """ - assert Users.get_salt(configuration) == "salt" + assert Users.get_salt(configuration) == ("salt", "salt") def test_get_salt_generate(configuration: Configuration) -> None: @@ -194,8 +240,9 @@ def test_get_salt_generate(configuration: Configuration) -> None: """ configuration.remove_option("auth", "salt") - salt = Users.get_salt(configuration, 16) + old_salt, salt = Users.get_salt(configuration, 16) assert salt + assert old_salt is None assert len(salt) == 16