diff --git a/docs/faq.rst b/docs/faq.rst index 19b6f914..3565a839 100644 --- a/docs/faq.rst +++ b/docs/faq.rst @@ -862,12 +862,13 @@ How to enable basic authorization yay -S --asdeps python-aiohttp-security python-aiohttp-session python-cryptography #. - Configure the service to enable authorization: + Configure the service to enable authorization (``salt`` can be generated as any random string): .. code-block:: ini [auth] target = configuration + salt = somerandomstring #. In order to provide access for reporting from application instances you can (recommended way) use unix sockets by configuring the following (note, that it requires ``python-requests-unixsocket`` package to be installed): @@ -933,7 +934,7 @@ How to enable OAuth authorization Configure ``oauth_provider`` and ``oauth_scopes`` in case if you would like to use different from Google provider. Scope must grant access to user email. ``web.address`` is required to make callback URL available from internet. #. - Create service user: + If you are not going to use unix socket, you also need to create service user (remember to set ``auth.salt`` option before): .. code-block:: shell diff --git a/src/ahriman/application/ahriman.py b/src/ahriman/application/ahriman.py index 47d9f40c..f02fb1dd 100644 --- a/src/ahriman/application/ahriman.py +++ b/src/ahriman/application/ahriman.py @@ -923,8 +923,6 @@ 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) 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("--key", help="optional PGP key used by this user. The private key must be imported") @@ -934,7 +932,6 @@ def _set_user_add_parser(root: SubParserAction) -> argparse.ArgumentParser: "which is in particular must be used for OAuth2 authorization type.") parser.add_argument("-r", "--role", help="user access level", type=UserAccess, choices=enum_values(UserAccess), default=UserAccess.Read) - parser.add_argument("-s", "--secure", help="set file permissions to user-only", action="store_true") parser.set_defaults(handler=handlers.Users, action=Action.Update, architecture=[""], lock=None, report=False, quiet=True) return parser diff --git a/src/ahriman/application/handlers/users.py b/src/ahriman/application/handlers/users.py index 30adce27..ce36d7a5 100644 --- a/src/ahriman/application/handlers/users.py +++ b/src/ahriman/application/handlers/users.py @@ -20,8 +20,6 @@ import argparse import getpass -from pathlib import Path - from ahriman.application.handlers import Handler from ahriman.core.configuration import Configuration from ahriman.core.database import SQLite @@ -54,13 +52,9 @@ class Users(Handler): database = SQLite.load(configuration) if args.action == Action.Update: - old_salt, salt = Users.get_salt(configuration) user = Users.user_create(args) - - if old_salt is None: - auth_configuration = Users.configuration_get(configuration.include) - Users.configuration_create(auth_configuration, salt, args.secure) - + # if password is left blank we are not going to require salt to be set + salt = configuration.get("auth", "salt") if user.password else "" database.user_update(user.hash_password(salt)) elif args.action == Action.List: users = database.user_list(args.username, args.role) @@ -70,70 +64,6 @@ class Users(Handler): elif args.action == Action.Remove: database.user_remove(args.username) - @staticmethod - def configuration_create(configuration: Configuration, salt: str, secure: bool) -> None: - """ - enable configuration if it has been disabled - - Args: - configuration(Configuration): configuration instance - salt(str): password hash salt - secure(bool): if true then set file permissions to 0o600 - """ - configuration.set_option("auth", "salt", salt) - Users.configuration_write(configuration, secure) - - @staticmethod - def configuration_get(include_path: Path) -> Configuration: - """ - create configuration instance - - Args: - include_path(Path): path to directory with configuration includes - - Returns: - Configuration: configuration instance. In case if there are local settings they will be loaded - """ - target = include_path / "00-auth.ini" - configuration = Configuration() - configuration.load(target) - - configuration.architecture = "" # not user anyway - - return configuration - - @staticmethod - def configuration_write(configuration: Configuration, secure: bool) -> None: - """ - write configuration file - - Args: - configuration(Configuration): configuration instance - secure(bool): if true then set file permissions to 0o600 - """ - path, _ = configuration.check_loaded() - with path.open("w") as ahriman_configuration: - configuration.write(ahriman_configuration) - if secure: - path.chmod(0o600) - - @staticmethod - def get_salt(configuration: Configuration, salt_length: int = 20) -> tuple[str | None, str]: - """ - get salt from configuration or create new string - - Args: - configuration(Configuration): configuration instance - salt_length(int, optional): salt length (Default value = 20) - - Returns: - tuple[str | None, 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, 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 381c1b0c..b9f31fe2 100644 --- a/tests/ahriman/application/handlers/test_handler_users.py +++ b/tests/ahriman/application/handlers/test_handler_users.py @@ -1,14 +1,15 @@ import argparse +import configparser + import pytest -from pathlib import Path from pytest_mock import MockerFixture from unittest.mock import call as MockCall from ahriman.application.handlers import Users from ahriman.core.configuration import Configuration from ahriman.core.database import SQLite -from ahriman.core.exceptions import InitializeError, PasswordError +from ahriman.core.exceptions import PasswordError from ahriman.models.action import Action from ahriman.models.user import User from ahriman.models.user_access import UserAccess @@ -31,7 +32,6 @@ def _default_args(args: argparse.Namespace) -> argparse.Namespace: args.packager = "packager" args.password = "pa55w0rd" args.role = UserAccess.Reporter - args.secure = False return args @@ -44,42 +44,44 @@ def test_run(args: argparse.Namespace, configuration: Configuration, database: S packager_id=args.packager, key=args.key) 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) - 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: +def test_run_empty_salt(args: argparse.Namespace, configuration: Configuration, mocker: MockerFixture) -> None: """ - must create configuration if salt was not set + must raise exception if salt is required, but not set """ + configuration.remove_option("auth", "salt") args = _default_args(args) user = User(username=args.username, password=args.password, access=args.role, packager_id=args.packager, key=args.key) + mocker.patch("ahriman.models.user.User.hash_password", return_value=user) + + with pytest.raises(configparser.NoOptionError): + Users.run(args, "x86_64", configuration, report=False, unsafe=False) + + +def test_run_empty_salt_without_password(args: argparse.Namespace, configuration: Configuration, database: SQLite, + mocker: MockerFixture) -> None: + """ + must skip salt option in case if password was empty + """ + configuration.remove_option("auth", "salt") + args = _default_args(args) + user = User(username=args.username, password="", access=args.role, + packager_id=args.packager, key=args.key) 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), 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) @@ -129,86 +131,6 @@ def test_run_remove(args: argparse.Namespace, configuration: Configuration, data remove_mock.assert_called_once_with(args.username) -def test_configuration_create(configuration: Configuration, mocker: MockerFixture) -> None: - """ - must correctly create configuration file - """ - mocker.patch("pathlib.Path.open") - set_mock = mocker.patch("ahriman.core.configuration.Configuration.set_option") - write_mock = mocker.patch("ahriman.application.handlers.Users.configuration_write") - - Users.configuration_create(configuration, "salt", False) - set_mock.assert_called_once_with("auth", "salt", pytest.helpers.anyvar(int)) - write_mock.assert_called_once_with(configuration, False) - - -def test_configuration_get(mocker: MockerFixture) -> None: - """ - must load configuration from filesystem - """ - mocker.patch("pathlib.Path.open") - mocker.patch("pathlib.Path.is_file", return_value=True) - read_mock = mocker.patch("ahriman.core.configuration.Configuration.read") - - assert Users.configuration_get(Path("path")) - read_mock.assert_called_once_with(Path("path") / "00-auth.ini") - - -def test_configuration_write(configuration: Configuration, mocker: MockerFixture) -> None: - """ - must write configuration - """ - mocker.patch("pathlib.Path.open") - write_mock = mocker.patch("ahriman.core.configuration.Configuration.write") - chmod_mock = mocker.patch("pathlib.Path.chmod") - - Users.configuration_write(configuration, secure=True) - write_mock.assert_called_once_with(pytest.helpers.anyvar(int)) - chmod_mock.assert_called_once_with(0o600) - - -def test_configuration_write_insecure(configuration: Configuration, mocker: MockerFixture) -> None: - """ - must write configuration without setting file permissions - """ - mocker.patch("pathlib.Path.open") - mocker.patch("ahriman.core.configuration.Configuration.write") - chmod_mock = mocker.patch("pathlib.Path.chmod") - - Users.configuration_write(configuration, secure=False) - chmod_mock.assert_not_called() - - -def test_configuration_write_not_loaded(configuration: Configuration, mocker: MockerFixture) -> None: - """ - must do nothing in case if configuration is not loaded - """ - configuration.path = None - mocker.patch("pathlib.Path.open") - - with pytest.raises(InitializeError): - Users.configuration_write(configuration, secure=True) - - -def test_get_salt_read(configuration: Configuration) -> None: - """ - must read salt from configuration - """ - assert Users.get_salt(configuration) == ("salt", "salt") - - -def test_get_salt_generate(configuration: Configuration) -> None: - """ - must generate salt if it does not exist - """ - configuration.remove_option("auth", "salt") - - old_salt, salt = Users.get_salt(configuration, 16) - assert salt - assert old_salt is None - assert len(salt) == 16 - - def test_user_create(args: argparse.Namespace, user: User) -> None: """ must create user