remove salt generation from users handler

It causes issues, because users handler is operating with service user,
but writtinng salt requires root privileges
This commit is contained in:
Evgenii Alekseev 2023-06-05 05:21:46 +03:00
parent 17f5f41e36
commit 2922bb9d72
4 changed files with 26 additions and 176 deletions

View File

@ -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

View File

@ -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

View File

@ -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:
"""

View File

@ -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