do not invoke configuration write in case if no salt or user was written

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

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)
 
+            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)
+
             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 salt, salt
-        return User.generate_password(salt_length)
+        return None, User.generate_password(salt_length)
 
     @staticmethod
     def user_create(args: argparse.Namespace) -> User:

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,9 +164,7 @@ def test_configuration_create(configuration: Configuration, user: User, mocker:
     write_mock.assert_called_once_with(configuration, False)
 
 
-def test_configuration_create_with_plain_password(
+def test_configuration_create_with_plain_password(configuration: Configuration, user: User,
-        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