Skip to content

Commit

Permalink
make auth.salt parameter optional
Browse files Browse the repository at this point in the history
Used implementation of the hasher includes salt itself, thus additional
salt is optional and can be safely (in terms of security) treat as empty
string
  • Loading branch information
arcan1s committed Aug 11, 2023
1 parent 1baf049 commit 1f2d56e
Show file tree
Hide file tree
Showing 7 changed files with 18 additions and 10 deletions.
2 changes: 1 addition & 1 deletion docs/architecture.rst
Expand Up @@ -231,7 +231,7 @@ The package provides several authorization methods: disabled, based on configura

Disabled (default) authorization provider just allows everything for everyone and does not have any specific configuration (it uses some default configuration parameters though). It also provides generic interface for derived classes.

Mapping (aka configuration) provider uses hashed passwords with salt from the database in order to authenticate users. This provider also enables user permission checking (read/write) (authorization). Thus, it defines the following methods:
Mapping (aka configuration) provider uses hashed passwords with optional salt from the database in order to authenticate users. This provider also enables user permission checking (read/write) (authorization). Thus, it defines the following methods:

* ``check_credentials`` - user password validation (authentication).
* ``verify_access`` - user permission validation (authorization).
Expand Down
2 changes: 1 addition & 1 deletion docs/configuration.rst
Expand Up @@ -54,7 +54,7 @@ Base authorization settings. ``OAuth`` provider requires ``aioauth-client`` libr
* ``max_age`` - parameter which controls both cookie expiration and token expiration inside the service, integer, optional, default is 7 days.
* ``oauth_provider`` - OAuth2 provider class name as is in ``aioauth-client`` (e.g. ``GoogleClient``, ``GithubClient`` etc), string, required in case if ``oauth`` is used.
* ``oauth_scopes`` - scopes list for OAuth2 provider, which will allow retrieving user email (which is used for checking user permissions), e.g. ``https://www.googleapis.com/auth/userinfo.email`` for ``GoogleClient`` or ``user:email`` for ``GithubClient``, space separated list of strings, required in case if ``oauth`` is used.
* ``salt`` - password hash salt, string, required in case if authorization enabled (automatically generated by ``user-add`` subcommand).
* ``salt`` - additional password hash salt, string, optional.

Authorized users are stored inside internal database, if any of external provides are used the password field for non-service users must be empty.

Expand Down
4 changes: 3 additions & 1 deletion docs/faq.rst
Expand Up @@ -870,6 +870,8 @@ How to enable basic authorization
target = configuration
salt = somerandomstring
The ``salt`` parameter is optional, but recommended.

#.
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):

Expand Down Expand Up @@ -934,7 +936,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.

#.
If you are not going to use unix socket, you also need to create service user (remember to set ``auth.salt`` option before):
If you are not going to use unix socket, you also need to create service user (remember to set ``auth.salt`` option before if required):

.. code-block:: shell
Expand Down
2 changes: 1 addition & 1 deletion src/ahriman/application/handlers/users.py
Expand Up @@ -52,7 +52,7 @@ def run(cls, args: argparse.Namespace, architecture: str, configuration: Configu
if args.action == Action.Update:
user = Users.user_create(args)
# if password is left blank we are not going to require salt to be set
salt = configuration.get("auth", "salt") if user.password else ""
salt = configuration.get("auth", "salt", fallback="") if user.password else ""
database.user_update(user.hash_password(salt))
elif args.action == Action.List:
users = database.user_list(args.username, args.role)
Expand Down
2 changes: 1 addition & 1 deletion src/ahriman/core/auth/mapping.py
Expand Up @@ -46,7 +46,7 @@ def __init__(self, configuration: Configuration, database: SQLite,
"""
Auth.__init__(self, configuration, provider)
self.database = database
self.salt = configuration.get("auth", "salt")
self.salt = configuration.get("auth", "salt", fallback="")

async def check_credentials(self, username: str | None, password: str | None) -> bool:
"""
Expand Down
7 changes: 5 additions & 2 deletions src/ahriman/core/configuration/schema.py
Expand Up @@ -93,9 +93,12 @@
"type": "string",
"oneof": [
{"allowed": ["disabled"]},
{"allowed": ["configuration", "mapping"], "dependencies": ["salt"]},
{"allowed": ["configuration", "mapping"]},
{"allowed": ["oauth"], "dependencies": [
"client_id", "client_secret", "oauth_provider", "oauth_scopes", "salt"
"client_id",
"client_secret",
"oauth_provider",
"oauth_scopes",
]},
],
},
Expand Down
9 changes: 6 additions & 3 deletions tests/ahriman/application/handlers/test_handler_users.py
Expand Up @@ -54,16 +54,19 @@ def test_run(args: argparse.Namespace, configuration: Configuration, database: S

def test_run_empty_salt(args: argparse.Namespace, configuration: Configuration, mocker: MockerFixture) -> None:
"""
must raise exception if salt is required, but not set
must process users with empty password salt
"""
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)
create_user_mock = mocker.patch("ahriman.application.handlers.Users.user_create", return_value=user)
update_mock = mocker.patch("ahriman.core.database.SQLite.user_update")

with pytest.raises(configparser.NoOptionError):
Users.run(args, "x86_64", configuration, report=False)
Users.run(args, "x86_64", configuration, report=False)
create_user_mock.assert_called_once_with(args)
update_mock.assert_called_once_with(user)


def test_run_empty_salt_without_password(args: argparse.Namespace, configuration: Configuration, database: SQLite,
Expand Down

0 comments on commit 1f2d56e

Please sign in to comment.