realworld fixes

* add method set_option to Configuration and also use it everywhere
* split CreateUser handler to additional read method
* check user duplicate on auth mapping read
* generate salt by using passlib instead of random.choice
* case-insensetive usernames
* update dependencies
* update configuration reference
* improve tests
This commit is contained in:
2021-09-02 00:53:33 +03:00
parent b755088024
commit 21d74c7a03
24 changed files with 293 additions and 154 deletions

View File

@ -28,67 +28,60 @@ def test_run(args: argparse.Namespace, configuration: Configuration, mocker: Moc
must run command
"""
args = _default_args(args)
get_auth_configuration_mock = mocker.patch("ahriman.application.handlers.CreateUser.get_auth_configuration")
create_configuration_mock = mocker.patch("ahriman.application.handlers.CreateUser.create_configuration")
create_user = mocker.patch("ahriman.application.handlers.CreateUser.create_user")
get_salt_mock = mocker.patch("ahriman.application.handlers.CreateUser.get_salt")
CreateUser.run(args, "x86_64", configuration)
get_auth_configuration_mock.assert_called_once()
create_configuration_mock.assert_called_once()
create_user.assert_called_once()
get_salt_mock.assert_called_once()
def test_create_configuration(user: User, mocker: MockerFixture) -> None:
def test_create_configuration(configuration: Configuration, user: User, mocker: MockerFixture) -> None:
"""
must correctly create configuration file
"""
section = Configuration.section_name("auth", user.access.value)
mocker.patch("pathlib.Path.open")
add_section_mock = mocker.patch("configparser.RawConfigParser.add_section")
set_mock = mocker.patch("configparser.RawConfigParser.set")
write_mock = mocker.patch("configparser.RawConfigParser.write")
set_mock = mocker.patch("ahriman.core.configuration.Configuration.set_option")
write_mock = mocker.patch("ahriman.core.configuration.Configuration.write")
CreateUser.create_configuration(user, "salt", Path("path"))
CreateUser.create_configuration(configuration, user, "salt")
write_mock.assert_called_once()
add_section_mock.assert_has_calls([mock.call("auth"), mock.call(section)])
set_mock.assert_has_calls([
mock.call("auth", "salt", pytest.helpers.anyvar(str)),
mock.call(section, user.username, user.password)
])
def test_create_configuration_not_loaded(configuration: Configuration, user: User, mocker: MockerFixture) -> None:
"""
must do nothing in case if configuration is not loaded
"""
configuration.path = None
mocker.patch("pathlib.Path.open")
write_mock = mocker.patch("ahriman.core.configuration.Configuration.write")
CreateUser.create_configuration(configuration, user, "salt")
write_mock.assert_not_called()
def test_create_configuration_user_exists(configuration: Configuration, user: User, mocker: MockerFixture) -> None:
"""
must correctly update configuration file if user already exists
"""
section = Configuration.section_name("auth", user.access.value)
configuration.add_section(section)
configuration.set(section, user.username, "")
configuration.set_option(section, user.username, "")
mocker.patch("pathlib.Path.open")
mocker.patch("configparser.ConfigParser", return_value=configuration)
mocker.patch("configparser.RawConfigParser.write")
add_section_mock = mocker.patch("configparser.RawConfigParser.add_section")
mocker.patch("ahriman.core.configuration.Configuration.write")
CreateUser.create_configuration(user, "salt", Path("path"))
add_section_mock.assert_not_called()
CreateUser.create_configuration(configuration, user, "salt")
assert configuration.get(section, user.username) == user.password
def test_create_configuration_file_exists(user: User, mocker: MockerFixture) -> None:
"""
must correctly update configuration file if file already exists
"""
mocker.patch("pathlib.Path.open")
mocker.patch("pathlib.Path.is_file", return_value=True)
mocker.patch("configparser.RawConfigParser.write")
read_mock = mocker.patch("configparser.RawConfigParser.read")
CreateUser.create_configuration(user, "salt", Path("path"))
read_mock.assert_called_once()
def test_create_user(args: argparse.Namespace, user: User) -> None:
"""
must create user
@ -130,3 +123,22 @@ def test_get_salt_generate(configuration: Configuration) -> None:
salt = CreateUser.get_salt(configuration, 16)
assert salt
assert len(salt) == 16
def test_get_auth_configuration() -> None:
"""
must load empty configuration
"""
assert CreateUser.get_auth_configuration(Path("path"))
def test_get_auth_configuration_exists(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")
CreateUser.get_auth_configuration(Path("path"))
read_mock.assert_called_once()

View File

@ -62,19 +62,12 @@ def test_create_ahriman_configuration(args: argparse.Namespace, configuration: C
"""
args = _default_args(args)
mocker.patch("pathlib.Path.open")
add_section_mock = mocker.patch("configparser.RawConfigParser.add_section")
set_mock = mocker.patch("configparser.RawConfigParser.set")
write_mock = mocker.patch("configparser.RawConfigParser.write")
set_option_mock = mocker.patch("ahriman.core.configuration.Configuration.set_option")
write_mock = mocker.patch("ahriman.core.configuration.Configuration.write")
command = Setup.build_command(args.build_command, "x86_64")
Setup.create_ahriman_configuration(args, "x86_64", args.repository, configuration.include)
add_section_mock.assert_has_calls([
mock.call(Configuration.section_name("build", "x86_64")),
mock.call("repository"),
mock.call(Configuration.section_name("sign", "x86_64")),
mock.call(Configuration.section_name("web", "x86_64")),
])
set_mock.assert_has_calls([
set_option_mock.assert_has_calls([
mock.call(Configuration.section_name("build", "x86_64"), "build_command", str(command)),
mock.call("repository", "name", args.repository),
mock.call(Configuration.section_name("sign", "x86_64"), "target",
@ -92,9 +85,9 @@ def test_create_devtools_configuration(args: argparse.Namespace, repository_path
"""
args = _default_args(args)
mocker.patch("pathlib.Path.open")
mocker.patch("configparser.RawConfigParser.set")
add_section_mock = mocker.patch("configparser.RawConfigParser.add_section")
write_mock = mocker.patch("configparser.RawConfigParser.write")
mocker.patch("ahriman.core.configuration.Configuration.set")
add_section_mock = mocker.patch("ahriman.core.configuration.Configuration.add_section")
write_mock = mocker.patch("ahriman.core.configuration.Configuration.write")
Setup.create_devtools_configuration(args.build_command, "x86_64", args.from_configuration,
args.no_multilib, args.repository, repository_paths)
@ -112,13 +105,11 @@ def test_create_devtools_configuration_no_multilib(args: argparse.Namespace, rep
"""
args = _default_args(args)
mocker.patch("pathlib.Path.open")
mocker.patch("configparser.RawConfigParser.set")
add_section_mock = mocker.patch("configparser.RawConfigParser.add_section")
write_mock = mocker.patch("configparser.RawConfigParser.write")
mocker.patch("ahriman.core.configuration.Configuration.set")
write_mock = mocker.patch("ahriman.core.configuration.Configuration.write")
Setup.create_devtools_configuration(args.build_command, "x86_64", args.from_configuration,
True, args.repository, repository_paths)
add_section_mock.assert_called_once()
write_mock.assert_called_once()

View File

@ -10,5 +10,5 @@ def mapping_auth(configuration: Configuration) -> MappingAuth:
auth provider fixture
:return: auth service instance
"""
configuration.set("web", "auth", "yes")
configuration.set_option("auth", "enabled", "yes")
return MappingAuth(configuration)

View File

@ -9,7 +9,7 @@ def test_load_dummy(configuration: Configuration) -> None:
"""
must load dummy validator if authorization is not enabled
"""
configuration.set("web", "auth", "no")
configuration.set_option("auth", "enabled", "no")
auth = Auth.load(configuration)
assert isinstance(auth, Auth)
@ -26,7 +26,7 @@ def test_load_mapping(configuration: Configuration) -> None:
"""
must load mapping validator if option set
"""
configuration.set("web", "auth", "yes")
configuration.set_option("auth", "enabled", "yes")
auth = Auth.load(configuration)
assert isinstance(auth, MappingAuth)

View File

@ -1,5 +1,8 @@
import pytest
from ahriman.core.auth.mapping_auth import MappingAuth
from ahriman.core.configuration import Configuration
from ahriman.core.exceptions import DuplicateUser
from ahriman.models.user import User
from ahriman.models.user_access import UserAccess
@ -10,25 +13,53 @@ def test_get_users(mapping_auth: MappingAuth, configuration: Configuration) -> N
"""
user_write = User("user_write", "pwd_write", UserAccess.Write)
write_section = Configuration.section_name("auth", user_write.access.value)
configuration.add_section(write_section)
configuration.set(write_section, user_write.username, user_write.password)
configuration.set_option(write_section, user_write.username, user_write.password)
user_read = User("user_read", "pwd_read", UserAccess.Read)
read_section = Configuration.section_name("auth", user_read.access.value)
configuration.add_section(read_section)
configuration.set(read_section, user_read.username, user_read.password)
configuration.set_option(read_section, user_read.username, user_read.password)
user_read = User("user_read", "pwd_read", UserAccess.Read)
read_section = Configuration.section_name("auth", user_read.access.value)
configuration.set_option(read_section, user_read.username, user_read.password)
users = mapping_auth.get_users(configuration)
expected = {user_write.username: user_write, user_read.username: user_read}
assert users == expected
def test_get_users_normalized(mapping_auth: MappingAuth, configuration: Configuration) -> None:
"""
must return user list with normalized usernames in keys
"""
user = User("UsEr", "pwd_read", UserAccess.Read)
read_section = Configuration.section_name("auth", user.access.value)
configuration.set_option(read_section, user.username, user.password)
users = mapping_auth.get_users(configuration)
expected = user.username.lower()
assert expected in users
assert users[expected].username == expected
def test_get_users_duplicate(mapping_auth: MappingAuth, configuration: Configuration, user: User) -> None:
"""
must raise exception on duplicate username
"""
write_section = Configuration.section_name("auth", UserAccess.Write.value)
configuration.set_option(write_section, user.username, user.password)
read_section = Configuration.section_name("auth", UserAccess.Read.value)
configuration.set_option(read_section, user.username, user.password)
with pytest.raises(DuplicateUser):
mapping_auth.get_users(configuration)
def test_check_credentials(mapping_auth: MappingAuth, user: User) -> None:
"""
must return true for valid credentials
"""
current_password = user.password
user.password = user.generate_password(user.password, mapping_auth.salt)
mapping_auth.users[user.username] = user
user.password = user.hash_password(user.password, mapping_auth.salt)
mapping_auth._users[user.username] = user
assert mapping_auth.check_credentials(user.username, current_password)
assert not mapping_auth.check_credentials(user.username, user.password) # here password is hashed so it is invalid
@ -49,11 +80,34 @@ def test_check_credentials_unknown(mapping_auth: MappingAuth, user: User) -> Non
assert not mapping_auth.check_credentials(user.username, user.password)
def test_get_user(mapping_auth: MappingAuth, user: User) -> None:
"""
must return user from storage by username
"""
mapping_auth._users[user.username] = user
assert mapping_auth.get_user(user.username) == user
def test_get_user_normalized(mapping_auth: MappingAuth, user: User) -> None:
"""
must return user from storage by username case-insensitive
"""
mapping_auth._users[user.username] = user
assert mapping_auth.get_user(user.username.upper()) == user
def test_get_user_unknown(mapping_auth: MappingAuth, user: User) -> None:
"""
must return None in case if no user found
"""
assert mapping_auth.get_user(user.username) is None
def test_known_username(mapping_auth: MappingAuth, user: User) -> None:
"""
must allow only known users
"""
mapping_auth.users[user.username] = user
mapping_auth._users[user.username] = user
assert mapping_auth.known_username(user.username)
assert not mapping_auth.known_username(user.password)
@ -62,6 +116,6 @@ def test_verify_access(mapping_auth: MappingAuth, user: User) -> None:
"""
must verify user access
"""
mapping_auth.users[user.username] = user
mapping_auth._users[user.username] = user
assert mapping_auth.verify_access(user.username, user.access)
assert not mapping_auth.verify_access(user.username, UserAccess.Write)

View File

@ -23,8 +23,8 @@ def test_send_auth(configuration: Configuration, mocker: MockerFixture) -> None:
"""
must send an email with attachment with auth
"""
configuration.set("email", "user", "username")
configuration.set("email", "password", "password")
configuration.set_option("email", "user", "username")
configuration.set_option("email", "password", "password")
smtp_mock = mocker.patch("smtplib.SMTP")
report = Email("x86_64", configuration)
@ -36,7 +36,7 @@ def test_send_auth_no_password(configuration: Configuration, mocker: MockerFixtu
"""
must send an email with attachment without auth if no password supplied
"""
configuration.set("email", "user", "username")
configuration.set_option("email", "user", "username")
smtp_mock = mocker.patch("smtplib.SMTP")
report = Email("x86_64", configuration)
@ -48,7 +48,7 @@ def test_send_auth_no_user(configuration: Configuration, mocker: MockerFixture)
"""
must send an email with attachment without auth if no user supplied
"""
configuration.set("email", "password", "password")
configuration.set_option("email", "password", "password")
smtp_mock = mocker.patch("smtplib.SMTP")
report = Email("x86_64", configuration)
@ -60,7 +60,7 @@ def test_send_ssl_tls(configuration: Configuration, mocker: MockerFixture) -> No
"""
must send an email with attachment with ssl/tls
"""
configuration.set("email", "ssl", "ssl")
configuration.set_option("email", "ssl", "ssl")
smtp_mock = mocker.patch("smtplib.SMTP_SSL")
report = Email("x86_64", configuration)
@ -75,7 +75,7 @@ def test_send_starttls(configuration: Configuration, mocker: MockerFixture) -> N
"""
must send an email with attachment with starttls
"""
configuration.set("email", "ssl", "starttls")
configuration.set_option("email", "ssl", "starttls")
smtp_mock = mocker.patch("smtplib.SMTP")
report = Email("x86_64", configuration)
@ -109,7 +109,7 @@ def test_generate_no_empty(configuration: Configuration, package_ahriman: Packag
"""
must not generate report with built packages if no_empty_report is set
"""
configuration.set("email", "no_empty_report", "yes")
configuration.set_option("email", "no_empty_report", "yes")
send_mock = mocker.patch("ahriman.core.report.email.Email._send")
report = Email("x86_64", configuration)
@ -122,7 +122,7 @@ def test_generate_no_empty_with_built(configuration: Configuration, package_ahri
"""
must generate report with built packages if no_empty_report is set
"""
configuration.set("email", "no_empty_report", "yes")
configuration.set_option("email", "no_empty_report", "yes")
send_mock = mocker.patch("ahriman.core.report.email.Email._send")
report = Email("x86_64", configuration)

View File

@ -19,8 +19,8 @@ def test_load_full_client(configuration: Configuration) -> None:
"""
must load full client if settings set
"""
configuration.set("web", "host", "localhost")
configuration.set("web", "port", "8080")
configuration.set_option("web", "host", "localhost")
configuration.set_option("web", "port", "8080")
assert isinstance(Client.load(configuration), WebClient)
@ -28,7 +28,7 @@ def test_load_full_client_from_address(configuration: Configuration) -> None:
"""
must load full client if settings set
"""
configuration.set("web", "address", "http://localhost:8080")
configuration.set_option("web", "address", "http://localhost:8080")
assert isinstance(Client.load(configuration), WebClient)

View File

@ -33,11 +33,11 @@ def test_parse_address(configuration: Configuration) -> None:
"""
must extract address correctly
"""
configuration.set("web", "host", "localhost")
configuration.set("web", "port", "8080")
configuration.set_option("web", "host", "localhost")
configuration.set_option("web", "port", "8080")
assert WebClient.parse_address(configuration) == "http://localhost:8080"
configuration.set("web", "address", "http://localhost:8081")
configuration.set_option("web", "address", "http://localhost:8081")
assert WebClient.parse_address(configuration) == "http://localhost:8081"

View File

@ -9,7 +9,7 @@ def test_from_path(mocker: MockerFixture) -> None:
"""
must load configuration
"""
read_mock = mocker.patch("configparser.RawConfigParser.read")
read_mock = mocker.patch("ahriman.core.configuration.Configuration.read")
load_includes_mock = mocker.patch("ahriman.core.configuration.Configuration.load_includes")
load_logging_mock = mocker.patch("ahriman.core.configuration.Configuration.load_logging")
path = Path("path")
@ -33,7 +33,7 @@ def test_absolute_path_for_absolute(configuration: Configuration) -> None:
must not change path for absolute path in settings
"""
path = Path("/a/b/c")
configuration.set("build", "path", str(path))
configuration.set_option("build", "path", str(path))
assert configuration.getpath("build", "path") == path
@ -42,7 +42,7 @@ def test_absolute_path_for_relative(configuration: Configuration) -> None:
must prepend root path to relative path
"""
path = Path("a")
configuration.set("build", "path", str(path))
configuration.set_option("build", "path", str(path))
result = configuration.getpath("build", "path")
assert result.is_absolute()
assert result.parent == configuration.path.parent
@ -61,8 +61,7 @@ def test_dump_architecture_specific(configuration: Configuration) -> None:
dump must contain architecture specific settings
"""
section = configuration.section_name("build", "x86_64")
configuration.add_section(section)
configuration.set(section, "archbuild_flags", "hello flag")
configuration.set_option(section, "archbuild_flags", "hello flag")
configuration.merge_sections("x86_64")
dump = configuration.dump()
@ -76,7 +75,7 @@ def test_getlist(configuration: Configuration) -> None:
"""
must return list of string correctly
"""
configuration.set("build", "test_list", "a b c")
configuration.set_option("build", "test_list", "a b c")
assert configuration.getlist("build", "test_list") == ["a", "b", "c"]
@ -85,7 +84,7 @@ def test_getlist_empty(configuration: Configuration) -> None:
must return list of string correctly for non-existing option
"""
assert configuration.getlist("build", "test_list") == []
configuration.set("build", "test_list", "")
configuration.set_option("build", "test_list", "")
assert configuration.getlist("build", "test_list") == []
@ -93,7 +92,7 @@ def test_getlist_single(configuration: Configuration) -> None:
"""
must return list of strings for single string
"""
configuration.set("build", "test_list", "a")
configuration.set_option("build", "test_list", "a")
assert configuration.getlist("build", "test_list") == ["a"]
@ -101,7 +100,7 @@ def test_load_includes_missing(configuration: Configuration) -> None:
"""
must not fail if not include directory found
"""
configuration.set("settings", "include", "path")
configuration.set_option("settings", "include", "path")
configuration.load_includes()
@ -144,8 +143,23 @@ def test_merge_sections_missing(configuration: Configuration) -> None:
"""
section = configuration.section_name("build", "x86_64")
configuration.remove_section("build")
configuration.add_section(section)
configuration.set(section, "key", "value")
configuration.set_option(section, "key", "value")
configuration.merge_sections("x86_64")
assert configuration.get("build", "key") == "value"
def test_set_option(configuration: Configuration) -> None:
"""
must set option correctly
"""
configuration.set_option("settings", "option", "value")
assert configuration.get("settings", "option") == "value"
def test_set_option_new_section(configuration: Configuration) -> None:
"""
must set option correctly even if no section found
"""
configuration.set_option("section", "option", "value")
assert configuration.get("section", "option") == "value"

View File

@ -21,17 +21,30 @@ def test_from_option_empty() -> None:
assert User.from_option(None, None) is None
def test_check_credentials_generate_password(user: User) -> None:
def test_check_credentials_hash_password(user: User) -> None:
"""
must generate and validate user password
"""
current_password = user.password
user.password = user.generate_password(current_password, "salt")
user.password = user.hash_password(current_password, "salt")
assert user.check_credentials(current_password, "salt")
assert not user.check_credentials(current_password, "salt1")
assert not user.check_credentials(user.password, "salt")
def test_generate_password() -> None:
"""
must generate password with specified length
"""
password = User.generate_password(16)
assert password
assert len(password) == 16
password = User.generate_password(42)
assert password
assert len(password) == 42
def test_verify_access_read(user: User) -> None:
"""
user with read access must be able to only request read

View File

@ -32,12 +32,12 @@ def application_with_auth(configuration: Configuration, user: User, mocker: Mock
:param mocker: mocker object
:return: application test instance
"""
configuration.set("web", "auth", "yes")
configuration.set_option("auth", "enabled", "yes")
mocker.patch.object(ahriman.core.auth.helpers, "_has_aiohttp_security", True)
mocker.patch("pathlib.Path.mkdir")
application = setup_service("x86_64", configuration)
generated = User(user.username, user.generate_password(user.password, application["validator"].salt), user.access)
application["validator"].users[generated.username] = generated
generated = User(user.username, user.hash_password(user.password, application["validator"].salt), user.access)
application["validator"]._users[generated.username] = generated
return application

View File

@ -25,8 +25,8 @@ def authorization_policy(configuration: Configuration, user: User) -> Authorizat
fixture for authorization policy
:return: authorization policy fixture
"""
configuration.set("web", "auth", "yes")
configuration.set_option("auth", "enabled", "yes")
validator = Auth.load(configuration)
policy = AuthorizationPolicy(validator)
policy.validator.users = {user.username: user}
policy.validator._users = {user.username: user}
return policy

View File

@ -35,7 +35,7 @@ def test_run(application: web.Application, mocker: MockerFixture) -> None:
must run application
"""
port = 8080
application["configuration"].set("web", "port", str(port))
application["configuration"].set_option("web", "port", str(port))
run_application_mock = mocker.patch("aiohttp.web.run_app")
run_server(application)
@ -48,7 +48,7 @@ def test_run_with_auth(application_with_auth: web.Application, mocker: MockerFix
must run application
"""
port = 8080
application_with_auth["configuration"].set("web", "port", str(port))
application_with_auth["configuration"].set_option("web", "port", str(port))
run_application_mock = mocker.patch("aiohttp.web.run_app")
run_server(application_with_auth)