From 9e782bb0b1619de8b0bb828bbefc0ffffca2ce1f Mon Sep 17 00:00:00 2001 From: Evgeniy Alekseev Date: Sun, 12 Sep 2021 19:59:06 +0300 Subject: [PATCH] full coverage --- docs/configuration.md | 3 +- package/etc/ahriman.ini | 2 + src/ahriman/core/auth/auth.py | 6 +- src/ahriman/core/auth/mapping.py | 4 +- src/ahriman/core/auth/oauth.py | 25 +++-- src/ahriman/web/views/user/login.py | 4 +- tests/ahriman/core/auth/conftest.py | 15 ++- tests/ahriman/core/auth/test_auth.py | 31 ++++-- tests/ahriman/core/auth/test_mapping.py | 55 ++++++----- tests/ahriman/core/auth/test_oauth.py | 98 +++++++++++++++++++ tests/ahriman/models/test_user.py | 21 ++++ .../views/service/test_views_service_add.py | 6 +- .../service/test_views_service_remove.py | 2 +- .../service/test_views_service_search.py | 6 +- .../views/status/test_views_status_ahriman.py | 4 +- .../views/status/test_views_status_package.py | 10 +- .../status/test_views_status_packages.py | 2 +- .../views/status/test_views_status_status.py | 2 +- tests/ahriman/web/views/test_views_index.py | 8 +- .../web/views/user/test_views_user_login.py | 72 +++++++++++++- .../web/views/user/test_views_user_logout.py | 4 +- tests/testresources/core/ahriman.ini | 4 + 22 files changed, 309 insertions(+), 75 deletions(-) create mode 100644 tests/ahriman/core/auth/test_oauth.py diff --git a/docs/configuration.md b/docs/configuration.md index c5a17e3b..86a08c7e 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -30,7 +30,6 @@ Base authorization settings. `OAuth2` provider requires `aioauth-client` library * `client_secret` - OAuth2 application client secret key, string, required in case if `oauth2` is used. * `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 `oauth2` is used. -* `oauth_redirect_uri` - full URI for OAuth2 redirect, must point to `/user-api/v1/login`, e.g. `https://example.com/user-api/v1/login`, string, required in case if `oauth2` 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 `oauth2` is used. * `salt` - password hash salt, string, required in case if authorization enabled (automatically generated by `create-user` subcommand). @@ -127,7 +126,7 @@ Group name must refer to architecture, e.g. it should be `s3:x86_64` for x86_64 Web server settings. If any of `host`/`port` is not set, web integration will be disabled. Group name must refer to architecture, e.g. it should be `web:x86_64` for x86_64 architecture. -* `address` - optional address in form `proto://host:port` (`port` can be omitted in case of default `proto` ports), will be used instead of `http://{host}:{port}` in case if set, string, optional. +* `address` - optional address in form `proto://host:port` (`port` can be omitted in case of default `proto` ports), will be used instead of `http://{host}:{port}` in case if set, string, optional. This option is required in case if `OAuth` provider is used. * `host` - host to bind, string, optional. * `password` - password to authorize in web service in order to update service status, string, required in case if authorization enabled. * `port` - port to bind, int, optional. diff --git a/package/etc/ahriman.ini b/package/etc/ahriman.ini index f4c22a7a..094b9b1f 100644 --- a/package/etc/ahriman.ini +++ b/package/etc/ahriman.ini @@ -12,6 +12,8 @@ root = / target = disabled allow_read_only = yes max_age = 604800 +oauth_provider = GoogleClient +oauth_scopes = https://www.googleapis.com/auth/userinfo.email [build] archbuild_flags = diff --git a/src/ahriman/core/auth/auth.py b/src/ahriman/core/auth/auth.py index 07c994ea..86b6aa0c 100644 --- a/src/ahriman/core/auth/auth.py +++ b/src/ahriman/core/auth/auth.py @@ -19,6 +19,8 @@ # from __future__ import annotations +import logging + from typing import Dict, Optional, Type from ahriman.core.configuration import Configuration @@ -47,6 +49,8 @@ class Auth: :param configuration: configuration instance :param provider: authorization type definition """ + self.logger = logging.getLogger("http") + self.allow_read_only = configuration.getboolean("auth", "allow_read_only") self.allowed_paths = set(configuration.getlist("auth", "allowed_paths")) self.allowed_paths.update(self.ALLOWED_PATHS) @@ -124,7 +128,7 @@ class Auth: return False # request without context is not allowed return uri in self.allowed_paths or any(uri.startswith(path) for path in self.allowed_paths_groups) - async def known_username(self, username: str) -> bool: # pylint: disable=no-self-use + async def known_username(self, username: Optional[str]) -> bool: # pylint: disable=no-self-use """ check if user is known :param username: username diff --git a/src/ahriman/core/auth/mapping.py b/src/ahriman/core/auth/mapping.py index 922461cf..778e7e6f 100644 --- a/src/ahriman/core/auth/mapping.py +++ b/src/ahriman/core/auth/mapping.py @@ -64,13 +64,13 @@ class Mapping(Auth): normalized_user = username.lower() return self._users.get(normalized_user) - async def known_username(self, username: str) -> bool: + async def known_username(self, username: Optional[str]) -> bool: """ check if user is known :param username: username :return: True in case if user is known and can be authorized and False otherwise """ - return self.get_user(username) is not None + return username is not None and self.get_user(username) is not None async def verify_access(self, username: str, required: UserAccess, context: Optional[str]) -> bool: """ diff --git a/src/ahriman/core/auth/oauth.py b/src/ahriman/core/auth/oauth.py index 059759e0..9784aa69 100644 --- a/src/ahriman/core/auth/oauth.py +++ b/src/ahriman/core/auth/oauth.py @@ -19,7 +19,7 @@ # import aioauth_client # type: ignore -from typing import Type +from typing import Optional, Type from ahriman.core.auth.mapping import Mapping from ahriman.core.configuration import Configuration @@ -47,7 +47,9 @@ class OAuth(Mapping): Mapping.__init__(self, configuration, provider) self.client_id = configuration.get("auth", "client_id") self.client_secret = configuration.get("auth", "client_secret") - self.redirect_uri = configuration.get("auth", "oauth_redirect_uri") + # in order to use OAuth feature the service must be publicity available + # thus we expect that address is set + self.redirect_uri = f"""{configuration.get("web", "address")}/user-api/v1/login""" self.provider = self.get_provider(configuration.get("auth", "oauth_provider")) # it is list but we will have to convert to string it anyway self.scopes = configuration.get("auth", "oauth_scopes") @@ -91,16 +93,21 @@ class OAuth(Mapping): uri: str = client.get_authorize_url(scope=self.scopes, redirect_uri=self.redirect_uri) return uri - async def get_oauth_username(self, code: str) -> str: + async def get_oauth_username(self, code: str) -> Optional[str]: """ extract OAuth username from remote :param code: authorization code provided by external service :return: username as is in OAuth provider """ - client = self.get_client() - access_token, _ = await client.get_access_token(code, redirect_uri=self.redirect_uri) - client.access_token = access_token + try: + client = self.get_client() + access_token, _ = await client.get_access_token(code, redirect_uri=self.redirect_uri) + client.access_token = access_token - user, _ = await client.user_info() - username: str = user.email - return username + print(f"HEEELOOOO {client}") + user, _ = await client.user_info() + username: str = user.email + return username + except Exception: + self.logger.exception("got exception while performing request") + return None diff --git a/src/ahriman/web/views/user/login.py b/src/ahriman/web/views/user/login.py index cb17c41f..18c05dd0 100644 --- a/src/ahriman/web/views/user/login.py +++ b/src/ahriman/web/views/user/login.py @@ -41,10 +41,10 @@ class LoginView(BaseView): code = self.request.query.getone("code", default=None) oauth_provider = self.validator - if not isinstance(oauth_provider, OAuth): + if not isinstance(oauth_provider, OAuth): # there is actually property, but mypy does not like it anyway raise HTTPMethodNotAllowed(self.request.method, ["POST"]) - if code is None: + if not code: return HTTPFound(oauth_provider.get_oauth_url()) response = HTTPFound("/") diff --git a/tests/ahriman/core/auth/conftest.py b/tests/ahriman/core/auth/conftest.py index 6d2e30be..a21974f0 100644 --- a/tests/ahriman/core/auth/conftest.py +++ b/tests/ahriman/core/auth/conftest.py @@ -1,13 +1,26 @@ import pytest from ahriman.core.auth.mapping import Mapping +from ahriman.core.auth.oauth import OAuth from ahriman.core.configuration import Configuration @pytest.fixture -def mapping_auth(configuration: Configuration) -> Mapping: +def mapping(configuration: Configuration) -> Mapping: """ auth provider fixture + :param configuration: configuration fixture :return: auth service instance """ return Mapping(configuration) + + +@pytest.fixture +def oauth(configuration: Configuration) -> OAuth: + """ + OAuth provider fixture + :param configuration: configuration fixture + :return: OAuth2 service instance + """ + configuration.set("web", "address", "https://example.com") + return OAuth(configuration) diff --git a/tests/ahriman/core/auth/test_auth.py b/tests/ahriman/core/auth/test_auth.py index 6c9a6469..0a359066 100644 --- a/tests/ahriman/core/auth/test_auth.py +++ b/tests/ahriman/core/auth/test_auth.py @@ -2,12 +2,21 @@ import pytest from ahriman.core.auth.auth import Auth from ahriman.core.auth.mapping import Mapping +from ahriman.core.auth.oauth import OAuth 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 +def test_auth_control(auth: Auth) -> None: + """ + must return a control for authorization + """ + assert auth.auth_control + assert "button" in auth.auth_control # I think it should be button + + def test_load_dummy(configuration: Configuration) -> None: """ must load dummy validator if authorization is not enabled @@ -34,7 +43,17 @@ def test_load_mapping(configuration: Configuration) -> None: assert isinstance(auth, Mapping) -def test_get_users(mapping_auth: Auth, configuration: Configuration) -> None: +def test_load_oauth(configuration: Configuration) -> None: + """ + must load OAuth2 validator if option set + """ + configuration.set_option("auth", "target", "oauth") + configuration.set_option("web", "address", "https://example.com") + auth = Auth.load(configuration) + assert isinstance(auth, OAuth) + + +def test_get_users(mapping: Auth, configuration: Configuration) -> None: """ must return valid user list """ @@ -48,12 +67,12 @@ def test_get_users(mapping_auth: Auth, configuration: Configuration) -> None: 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) + users = mapping.get_users(configuration) expected = {user_write.username: user_write, user_read.username: user_read} assert users == expected -def test_get_users_normalized(mapping_auth: Auth, configuration: Configuration) -> None: +def test_get_users_normalized(mapping: Auth, configuration: Configuration) -> None: """ must return user list with normalized usernames in keys """ @@ -61,13 +80,13 @@ def test_get_users_normalized(mapping_auth: Auth, configuration: Configuration) read_section = Configuration.section_name("auth", user.access.value) configuration.set_option(read_section, user.username, user.password) - users = mapping_auth.get_users(configuration) + users = mapping.get_users(configuration) expected = user.username.lower() assert expected in users assert users[expected].username == expected -def test_get_users_duplicate(mapping_auth: Auth, configuration: Configuration, user: User) -> None: +def test_get_users_duplicate(mapping: Auth, configuration: Configuration, user: User) -> None: """ must raise exception on duplicate username """ @@ -77,7 +96,7 @@ def test_get_users_duplicate(mapping_auth: Auth, configuration: Configuration, u configuration.set_option(read_section, user.username, user.password) with pytest.raises(DuplicateUser): - mapping_auth.get_users(configuration) + mapping.get_users(configuration) async def test_check_credentials(auth: Auth, user: User) -> None: diff --git a/tests/ahriman/core/auth/test_mapping.py b/tests/ahriman/core/auth/test_mapping.py index 9a758d33..216aab24 100644 --- a/tests/ahriman/core/auth/test_mapping.py +++ b/tests/ahriman/core/auth/test_mapping.py @@ -3,70 +3,71 @@ from ahriman.models.user import User from ahriman.models.user_access import UserAccess -async def test_check_credentials(mapping_auth: Mapping, user: User) -> None: +async def test_check_credentials(mapping: Mapping, user: User) -> None: """ must return true for valid credentials """ current_password = user.password - user.password = user.hash_password(mapping_auth.salt) - mapping_auth._users[user.username] = user - assert await mapping_auth.check_credentials(user.username, current_password) + user.password = user.hash_password(mapping.salt) + mapping._users[user.username] = user + assert await mapping.check_credentials(user.username, current_password) # here password is hashed so it is invalid - assert not await mapping_auth.check_credentials(user.username, user.password) + assert not await mapping.check_credentials(user.username, user.password) -async def test_check_credentials_empty(mapping_auth: Mapping) -> None: +async def test_check_credentials_empty(mapping: Mapping) -> None: """ must reject on empty credentials """ - assert not await mapping_auth.check_credentials(None, "") - assert not await mapping_auth.check_credentials("", None) - assert not await mapping_auth.check_credentials(None, None) + assert not await mapping.check_credentials(None, "") + assert not await mapping.check_credentials("", None) + assert not await mapping.check_credentials(None, None) -async def test_check_credentials_unknown(mapping_auth: Mapping, user: User) -> None: +async def test_check_credentials_unknown(mapping: Mapping, user: User) -> None: """ must reject on unknown user """ - assert not await mapping_auth.check_credentials(user.username, user.password) + assert not await mapping.check_credentials(user.username, user.password) -def test_get_user(mapping_auth: Mapping, user: User) -> None: +def test_get_user(mapping: Mapping, user: User) -> None: """ must return user from storage by username """ - mapping_auth._users[user.username] = user - assert mapping_auth.get_user(user.username) == user + mapping._users[user.username] = user + assert mapping.get_user(user.username) == user -def test_get_user_normalized(mapping_auth: Mapping, user: User) -> None: +def test_get_user_normalized(mapping: Mapping, 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 + mapping._users[user.username] = user + assert mapping.get_user(user.username.upper()) == user -def test_get_user_unknown(mapping_auth: Mapping, user: User) -> None: +def test_get_user_unknown(mapping: Mapping, user: User) -> None: """ must return None in case if no user found """ - assert mapping_auth.get_user(user.username) is None + assert mapping.get_user(user.username) is None -async def test_known_username(mapping_auth: Mapping, user: User) -> None: +async def test_known_username(mapping: Mapping, user: User) -> None: """ must allow only known users """ - mapping_auth._users[user.username] = user - assert await mapping_auth.known_username(user.username) - assert not await mapping_auth.known_username(user.password) + mapping._users[user.username] = user + assert await mapping.known_username(user.username) + assert not await mapping.known_username(None) + assert not await mapping.known_username(user.password) -async def test_verify_access(mapping_auth: Mapping, user: User) -> None: +async def test_verify_access(mapping: Mapping, user: User) -> None: """ must verify user access """ - mapping_auth._users[user.username] = user - assert await mapping_auth.verify_access(user.username, user.access, None) - assert not await mapping_auth.verify_access(user.username, UserAccess.Write, None) + mapping._users[user.username] = user + assert await mapping.verify_access(user.username, user.access, None) + assert not await mapping.verify_access(user.username, UserAccess.Write, None) diff --git a/tests/ahriman/core/auth/test_oauth.py b/tests/ahriman/core/auth/test_oauth.py new file mode 100644 index 00000000..73dc2ffc --- /dev/null +++ b/tests/ahriman/core/auth/test_oauth.py @@ -0,0 +1,98 @@ +import aioauth_client +import pytest + +from pytest_mock import MockerFixture + +from ahriman.core.auth.oauth import OAuth +from ahriman.core.exceptions import InvalidOption + + +def test_auth_control(oauth: OAuth) -> None: + """ + must return a control for authorization + """ + assert oauth.auth_control + assert " None: + """ + must return valid provider type + """ + assert OAuth.get_provider("OAuth2Client") == aioauth_client.OAuth2Client + assert OAuth.get_provider("GoogleClient") == aioauth_client.GoogleClient + assert OAuth.get_provider("GoogleClient") == aioauth_client.GoogleClient + + +def test_get_provider_not_a_type() -> None: + """ + must raise an exception if attribute is not a type + """ + with pytest.raises(InvalidOption): + OAuth.get_provider("__version__") + + +def test_get_provider_invalid_type() -> None: + """ + must raise an exception if attribute is not an OAuth2 client + """ + with pytest.raises(InvalidOption): + OAuth.get_provider("User") + with pytest.raises(InvalidOption): + OAuth.get_provider("OAuth1Client") + + +def test_get_client(oauth: OAuth) -> None: + """ + must return valid OAuth2 client + """ + client = oauth.get_client() + assert isinstance(client, aioauth_client.GoogleClient) + assert client.client_id == oauth.client_id + assert client.client_secret == oauth.client_secret + + +def test_get_oauth_url(oauth: OAuth, mocker: MockerFixture) -> None: + """ + must generate valid OAuth authorization URL + """ + authorize_url_mock = mocker.patch("aioauth_client.GoogleClient.get_authorize_url") + oauth.get_oauth_url() + authorize_url_mock.assert_called_with(scope=oauth.scopes, redirect_uri=oauth.redirect_uri) + + +async def test_get_oauth_username(oauth: OAuth, mocker: MockerFixture) -> None: + """ + must return authorized user ID + """ + access_token_mock = mocker.patch("aioauth_client.GoogleClient.get_access_token", return_value=("token", "")) + user_info_mock = mocker.patch("aioauth_client.GoogleClient.user_info", + return_value=(aioauth_client.User(email="email"), "")) + + email = await oauth.get_oauth_username("code") + access_token_mock.assert_called_with("code", redirect_uri=oauth.redirect_uri) + user_info_mock.assert_called_once() + assert email == "email" + + +async def test_get_oauth_username_exception_1(oauth: OAuth, mocker: MockerFixture) -> None: + """ + must return None in case of OAuth request error (get_access_token) + """ + mocker.patch("aioauth_client.GoogleClient.get_access_token", side_effect=Exception()) + user_info_mock = mocker.patch("aioauth_client.GoogleClient.user_info") + + email = await oauth.get_oauth_username("code") + assert email is None + user_info_mock.assert_not_called() + + +async def test_get_oauth_username_exception_2(oauth: OAuth, mocker: MockerFixture) -> None: + """ + must return None in case of OAuth request error (user_info) + """ + mocker.patch("aioauth_client.GoogleClient.get_access_token", return_value=("token", "")) + mocker.patch("aioauth_client.GoogleClient.user_info", side_effect=Exception()) + + email = await oauth.get_oauth_username("code") + assert email is None diff --git a/tests/ahriman/models/test_user.py b/tests/ahriman/models/test_user.py index 6acc2f3d..be3257fa 100644 --- a/tests/ahriman/models/test_user.py +++ b/tests/ahriman/models/test_user.py @@ -10,6 +10,7 @@ def test_from_option(user: User) -> None: # default is read access user.access = UserAccess.Write assert User.from_option(user.username, user.password) != user + assert User.from_option(user.username, user.password, user.access) == user def test_from_option_empty() -> None: @@ -32,6 +33,26 @@ def test_check_credentials_hash_password(user: User) -> None: assert not user.check_credentials(user.password, "salt") +def test_check_credentials_empty_hash(user: User) -> None: + """ + must reject any authorization if the hash is invalid + """ + current_password = user.password + assert not user.check_credentials(current_password, "salt") + user.password = "" + assert not user.check_credentials(current_password, "salt") + + +def test_hash_password_empty_hash(user: User) -> None: + """ + must return empty string after hash in case if password not set + """ + user.password = "" + assert user.hash_password("salt") == "" + user.password = None + assert user.hash_password("salt") == "" + + def test_generate_password() -> None: """ must generate password with specified length diff --git a/tests/ahriman/web/views/service/test_views_service_add.py b/tests/ahriman/web/views/service/test_views_service_add.py index 24c750c9..4442dcab 100644 --- a/tests/ahriman/web/views/service/test_views_service_add.py +++ b/tests/ahriman/web/views/service/test_views_service_add.py @@ -9,7 +9,7 @@ async def test_post(client: TestClient, mocker: MockerFixture) -> None: add_mock = mocker.patch("ahriman.core.spawn.Spawn.packages_add") response = await client.post("/service-api/v1/add", json={"packages": ["ahriman"]}) - assert response.status == 200 + assert response.ok add_mock.assert_called_with(["ahriman"], True) @@ -20,7 +20,7 @@ async def test_post_now(client: TestClient, mocker: MockerFixture) -> None: add_mock = mocker.patch("ahriman.core.spawn.Spawn.packages_add") response = await client.post("/service-api/v1/add", json={"packages": ["ahriman"], "build_now": False}) - assert response.status == 200 + assert response.ok add_mock.assert_called_with(["ahriman"], False) @@ -42,5 +42,5 @@ async def test_post_update(client: TestClient, mocker: MockerFixture) -> None: add_mock = mocker.patch("ahriman.core.spawn.Spawn.packages_add") response = await client.post("/service-api/v1/update", json={"packages": ["ahriman"]}) - assert response.status == 200 + assert response.ok add_mock.assert_called_with(["ahriman"], True) diff --git a/tests/ahriman/web/views/service/test_views_service_remove.py b/tests/ahriman/web/views/service/test_views_service_remove.py index d7c45d80..c801c3be 100644 --- a/tests/ahriman/web/views/service/test_views_service_remove.py +++ b/tests/ahriman/web/views/service/test_views_service_remove.py @@ -9,7 +9,7 @@ async def test_post(client: TestClient, mocker: MockerFixture) -> None: add_mock = mocker.patch("ahriman.core.spawn.Spawn.packages_remove") response = await client.post("/service-api/v1/remove", json={"packages": ["ahriman"]}) - assert response.status == 200 + assert response.ok add_mock.assert_called_with(["ahriman"]) diff --git a/tests/ahriman/web/views/service/test_views_service_search.py b/tests/ahriman/web/views/service/test_views_service_search.py index bfd3158d..4f248d72 100644 --- a/tests/ahriman/web/views/service/test_views_service_search.py +++ b/tests/ahriman/web/views/service/test_views_service_search.py @@ -11,7 +11,7 @@ async def test_get(client: TestClient, aur_package_ahriman: aur.Package, mocker: mocker.patch("aur.search", return_value=[aur_package_ahriman]) response = await client.get("/service-api/v1/search", params={"for": "ahriman"}) - assert response.status == 200 + assert response.ok assert await response.json() == ["ahriman"] @@ -33,7 +33,7 @@ async def test_get_join(client: TestClient, mocker: MockerFixture) -> None: search_mock = mocker.patch("aur.search") response = await client.get("/service-api/v1/search", params=[("for", "ahriman"), ("for", "maybe")]) - assert response.status == 200 + assert response.ok search_mock.assert_called_with("ahriman maybe") @@ -44,7 +44,7 @@ async def test_get_join_filter(client: TestClient, mocker: MockerFixture) -> Non search_mock = mocker.patch("aur.search") response = await client.get("/service-api/v1/search", params=[("for", "ah"), ("for", "maybe")]) - assert response.status == 200 + assert response.ok search_mock.assert_called_with("maybe") diff --git a/tests/ahriman/web/views/status/test_views_status_ahriman.py b/tests/ahriman/web/views/status/test_views_status_ahriman.py index 489da68c..7108701b 100644 --- a/tests/ahriman/web/views/status/test_views_status_ahriman.py +++ b/tests/ahriman/web/views/status/test_views_status_ahriman.py @@ -11,7 +11,7 @@ async def test_get(client: TestClient) -> None: response = await client.get("/status-api/v1/ahriman") status = BuildStatus.from_json(await response.json()) - assert response.status == 200 + assert response.ok assert status.status == BuildStatusEnum.Unknown @@ -26,7 +26,7 @@ async def test_post(client: TestClient) -> None: response = await client.get("/status-api/v1/ahriman") status = BuildStatus.from_json(await response.json()) - assert response.status == 200 + assert response.ok assert status.status == BuildStatusEnum.Success diff --git a/tests/ahriman/web/views/status/test_views_status_package.py b/tests/ahriman/web/views/status/test_views_status_package.py index bb760890..cdbe447e 100644 --- a/tests/ahriman/web/views/status/test_views_status_package.py +++ b/tests/ahriman/web/views/status/test_views_status_package.py @@ -14,7 +14,7 @@ async def test_get(client: TestClient, package_ahriman: Package, package_python_ json={"status": BuildStatusEnum.Success.value, "package": package_python_schedule.view()}) response = await client.get(f"/status-api/v1/packages/{package_ahriman.base}") - assert response.status == 200 + assert response.ok packages = [Package.from_json(item["package"]) for item in await response.json()] assert packages @@ -45,7 +45,7 @@ async def test_delete(client: TestClient, package_ahriman: Package, package_pyth assert response.status == 404 response = await client.get(f"/status-api/v1/packages/{package_python_schedule.base}") - assert response.status == 200 + assert response.ok async def test_delete_unknown(client: TestClient, package_ahriman: Package, package_python_schedule: Package) -> None: @@ -62,7 +62,7 @@ async def test_delete_unknown(client: TestClient, package_ahriman: Package, pack assert response.status == 404 response = await client.get(f"/status-api/v1/packages/{package_python_schedule.base}") - assert response.status == 200 + assert response.ok async def test_post(client: TestClient, package_ahriman: Package) -> None: @@ -75,7 +75,7 @@ async def test_post(client: TestClient, package_ahriman: Package) -> None: assert post_response.status == 204 response = await client.get(f"/status-api/v1/packages/{package_ahriman.base}") - assert response.status == 200 + assert response.ok async def test_post_exception(client: TestClient, package_ahriman: Package) -> None: @@ -100,7 +100,7 @@ async def test_post_light(client: TestClient, package_ahriman: Package) -> None: assert post_response.status == 204 response = await client.get(f"/status-api/v1/packages/{package_ahriman.base}") - assert response.status == 200 + assert response.ok statuses = { Package.from_json(item["package"]).base: BuildStatus.from_json(item["status"]) for item in await response.json() diff --git a/tests/ahriman/web/views/status/test_views_status_packages.py b/tests/ahriman/web/views/status/test_views_status_packages.py index c1d18b0e..30d4a962 100644 --- a/tests/ahriman/web/views/status/test_views_status_packages.py +++ b/tests/ahriman/web/views/status/test_views_status_packages.py @@ -15,7 +15,7 @@ async def test_get(client: TestClient, package_ahriman: Package, package_python_ json={"status": BuildStatusEnum.Success.value, "package": package_python_schedule.view()}) response = await client.get("/status-api/v1/packages") - assert response.status == 200 + assert response.ok packages = [Package.from_json(item["package"]) for item in await response.json()] assert packages diff --git a/tests/ahriman/web/views/status/test_views_status_status.py b/tests/ahriman/web/views/status/test_views_status_status.py index e7e7fd1c..776f3775 100644 --- a/tests/ahriman/web/views/status/test_views_status_status.py +++ b/tests/ahriman/web/views/status/test_views_status_status.py @@ -14,7 +14,7 @@ async def test_get(client: TestClient, package_ahriman: Package) -> None: json={"status": BuildStatusEnum.Success.value, "package": package_ahriman.view()}) response = await client.get("/status-api/v1/status") - assert response.status == 200 + assert response.ok json = await response.json() assert json["version"] == version.__version__ diff --git a/tests/ahriman/web/views/test_views_index.py b/tests/ahriman/web/views/test_views_index.py index 61342667..0c9ff926 100644 --- a/tests/ahriman/web/views/test_views_index.py +++ b/tests/ahriman/web/views/test_views_index.py @@ -6,7 +6,7 @@ async def test_get(client_with_auth: TestClient) -> None: must generate status page correctly (/) """ response = await client_with_auth.get("/") - assert response.status == 200 + assert response.ok assert await response.text() @@ -15,7 +15,7 @@ async def test_get_index(client_with_auth: TestClient) -> None: must generate status page correctly (/index.html) """ response = await client_with_auth.get("/index.html") - assert response.status == 200 + assert response.ok assert await response.text() @@ -24,7 +24,7 @@ async def test_get_without_auth(client: TestClient) -> None: must use dummy authorized_userid function in case if no security library installed """ response = await client.get("/") - assert response.status == 200 + assert response.ok assert await response.text() @@ -33,4 +33,4 @@ async def test_get_static(client: TestClient) -> None: must return static files """ response = await client.get("/static/favicon.ico") - assert response.status == 200 + assert response.ok diff --git a/tests/ahriman/web/views/user/test_views_user_login.py b/tests/ahriman/web/views/user/test_views_user_login.py index 8be422f1..622cf565 100644 --- a/tests/ahriman/web/views/user/test_views_user_login.py +++ b/tests/ahriman/web/views/user/test_views_user_login.py @@ -1,9 +1,75 @@ from aiohttp.test_utils import TestClient from pytest_mock import MockerFixture +from unittest.mock import MagicMock +from ahriman.core.auth.oauth import OAuth from ahriman.models.user import User +async def test_get_default_validator(client_with_auth: TestClient) -> None: + """ + must return 405 in case if no OAuth enabled + """ + get_response = await client_with_auth.get("/user-api/v1/login") + assert get_response.status == 405 + + +async def test_get_redirect_to_oauth(client_with_auth: TestClient) -> None: + """ + must redirect to OAuth service provider in case if no code is supplied + """ + oauth = client_with_auth.app["validator"] = MagicMock(spec=OAuth) + oauth.get_oauth_url.return_value = "https://example.com" + + get_response = await client_with_auth.get("/user-api/v1/login") + assert get_response.ok + oauth.get_oauth_url.assert_called_once() + + +async def test_get_redirect_to_oauth_empty_code(client_with_auth: TestClient) -> None: + """ + must redirect to OAuth service provider in case if empty code is supplied + """ + oauth = client_with_auth.app["validator"] = MagicMock(spec=OAuth) + oauth.get_oauth_url.return_value = "https://example.com" + + get_response = await client_with_auth.get("/user-api/v1/login", params={"code": ""}) + assert get_response.ok + oauth.get_oauth_url.assert_called_once() + + +async def test_get(client_with_auth: TestClient, mocker: MockerFixture) -> None: + """ + must login user correctly from OAuth + """ + oauth = client_with_auth.app["validator"] = MagicMock(spec=OAuth) + oauth.get_oauth_username.return_value = "user" + oauth.known_username.return_value = True + oauth.enabled = False # lol + remember_mock = mocker.patch("aiohttp_security.remember") + + get_response = await client_with_auth.get("/user-api/v1/login", params={"code": "code"}) + + assert get_response.ok + oauth.get_oauth_username.assert_called_with("code") + oauth.known_username.assert_called_with("user") + remember_mock.assert_called_once() + + +async def test_get_unauthorized(client_with_auth: TestClient, mocker: MockerFixture) -> None: + """ + must return unauthorized from OAuth + """ + oauth = client_with_auth.app["validator"] = MagicMock(spec=OAuth) + oauth.known_username.return_value = False + remember_mock = mocker.patch("aiohttp_security.remember") + + get_response = await client_with_auth.get("/user-api/v1/login", params={"code": "code"}) + + assert get_response.status == 401 + remember_mock.assert_not_called() + + async def test_post(client_with_auth: TestClient, user: User, mocker: MockerFixture) -> None: """ must login user correctly @@ -12,10 +78,10 @@ async def test_post(client_with_auth: TestClient, user: User, mocker: MockerFixt remember_mock = mocker.patch("aiohttp_security.remember") post_response = await client_with_auth.post("/user-api/v1/login", json=payload) - assert post_response.status == 200 + assert post_response.ok post_response = await client_with_auth.post("/user-api/v1/login", data=payload) - assert post_response.status == 200 + assert post_response.ok remember_mock.assert_called() @@ -26,7 +92,7 @@ async def test_post_skip(client: TestClient, user: User) -> None: """ payload = {"username": user.username, "password": user.password} post_response = await client.post("/user-api/v1/login", json=payload) - assert post_response.status == 200 + assert post_response.ok async def test_post_unauthorized(client_with_auth: TestClient, user: User, mocker: MockerFixture) -> None: diff --git a/tests/ahriman/web/views/user/test_views_user_logout.py b/tests/ahriman/web/views/user/test_views_user_logout.py index 3d287bb0..7e316204 100644 --- a/tests/ahriman/web/views/user/test_views_user_logout.py +++ b/tests/ahriman/web/views/user/test_views_user_logout.py @@ -11,7 +11,7 @@ async def test_post(client_with_auth: TestClient, mocker: MockerFixture) -> None forget_mock = mocker.patch("aiohttp_security.forget") post_response = await client_with_auth.post("/user-api/v1/logout") - assert post_response.status == 200 + assert post_response.ok forget_mock.assert_called_once() @@ -32,4 +32,4 @@ async def test_post_disabled(client: TestClient) -> None: must raise exception if auth is disabled """ post_response = await client.post("/user-api/v1/logout") - assert post_response.status == 200 + assert post_response.ok diff --git a/tests/testresources/core/ahriman.ini b/tests/testresources/core/ahriman.ini index 95343437..54ade25c 100644 --- a/tests/testresources/core/ahriman.ini +++ b/tests/testresources/core/ahriman.ini @@ -10,6 +10,10 @@ root = / [auth] allow_read_only = no +client_id = client_id +client_secret = client_secret +oauth_provider = GoogleClient +oauth_scopes = https://www.googleapis.com/auth/userinfo.email salt = salt [build]