diff --git a/docs/changelog/next_release/177.bugfix.rst b/docs/changelog/next_release/177.bugfix.rst new file mode 100644 index 00000000..a54e2993 --- /dev/null +++ b/docs/changelog/next_release/177.bugfix.rst @@ -0,0 +1 @@ +Replace sync methods of Keycloak client with async ones. Previously interaction with Keycloak could block asyncio event loop. diff --git a/poetry.lock b/poetry.lock index b0f7a7c3..88c6c201 100644 --- a/poetry.lock +++ b/poetry.lock @@ -682,7 +682,7 @@ version = "3.4.2" description = "The Real First Universal Charset Detector. Open, modern and actively maintained alternative to Chardet." optional = false python-versions = ">=3.7" -groups = ["main", "docs", "test"] +groups = ["main", "docs"] files = [ {file = "charset_normalizer-3.4.2-cp310-cp310-macosx_10_9_universal2.whl", hash = "sha256:7c48ed483eb946e6c04ccbe02c6b4d1d48e51944b6db70f697e089c193404941"}, {file = "charset_normalizer-3.4.2-cp310-cp310-manylinux_2_17_aarch64.manylinux2014_aarch64.whl", hash = "sha256:b2d318c11350e10662026ad0eb71bb51c7812fc8590825304ae0bdd4ac283acd"}, @@ -3136,7 +3136,7 @@ version = "6.0.3" description = "YAML parser and emitter for Python" optional = false python-versions = ">=3.8" -groups = ["main", "dev", "test"] +groups = ["main", "dev"] files = [ {file = "PyYAML-6.0.3-cp38-cp38-macosx_10_13_x86_64.whl", hash = "sha256:c2514fceb77bc5e7a2f7adfaa1feb2fb311607c9cb518dbc378688ec73d8292f"}, {file = "PyYAML-6.0.3-cp38-cp38-manylinux2014_aarch64.manylinux_2_17_aarch64.manylinux_2_28_aarch64.whl", hash = "sha256:9c57bb8c96f6d1808c030b1687b9b5fb476abaa47f0db9c0101f5e9f394e97f4"}, @@ -3220,7 +3220,7 @@ version = "2.32.4" description = "Python HTTP for Humans." optional = false python-versions = ">=3.8" -groups = ["main", "docs", "test"] +groups = ["main", "docs"] files = [ {file = "requests-2.32.4-py3-none-any.whl", hash = "sha256:27babd3cda2a6d50b30443204ee89830707d396671944c998b5975b031ac2b2c"}, {file = "requests-2.32.4.tar.gz", hash = "sha256:27d0316682c8a29834d3264820024b62a36942083d52caf2f14c0591336d3422"}, @@ -3272,24 +3272,19 @@ files = [ requests = ">=2.0.1,<3.0.0" [[package]] -name = "responses" -version = "0.25.8" -description = "A utility library for mocking out the `requests` Python library." +name = "respx" +version = "0.22.0" +description = "A utility for mocking out the Python HTTPX and HTTP Core libraries." optional = false python-versions = ">=3.8" groups = ["test"] files = [ - {file = "responses-0.25.8-py3-none-any.whl", hash = "sha256:0c710af92def29c8352ceadff0c3fe340ace27cf5af1bbe46fb71275bcd2831c"}, - {file = "responses-0.25.8.tar.gz", hash = "sha256:9374d047a575c8f781b94454db5cab590b6029505f488d12899ddb10a4af1cf4"}, + {file = "respx-0.22.0-py2.py3-none-any.whl", hash = "sha256:631128d4c9aba15e56903fb5f66fb1eff412ce28dd387ca3a81339e52dbd3ad0"}, + {file = "respx-0.22.0.tar.gz", hash = "sha256:3c8924caa2a50bd71aefc07aa812f2466ff489f1848c96e954a5362d17095d91"}, ] [package.dependencies] -pyyaml = "*" -requests = ">=2.30.0,<3.0" -urllib3 = ">=1.25.10,<3.0" - -[package.extras] -tests = ["coverage (>=6.0.0)", "flake8", "mypy", "pytest (>=7.0.0)", "pytest-asyncio", "pytest-cov", "pytest-httpserver", "tomli ; python_version < \"3.11\"", "tomli-w", "types-PyYAML", "types-requests"] +httpx = ">=0.25.0" [[package]] name = "roman-numerals-py" @@ -3998,7 +3993,7 @@ version = "2.5.0" description = "HTTP library with thread-safe connection pooling, file post, and more." optional = false python-versions = ">=3.9" -groups = ["main", "docs", "test"] +groups = ["main", "docs"] files = [ {file = "urllib3-2.5.0-py3-none-any.whl", hash = "sha256:e6b01673c0fa6a13e374b50871808eb3bf7046c4b125b216f6bf1cc604cff0dc"}, {file = "urllib3-2.5.0.tar.gz", hash = "sha256:3fc47733c7e419d4bc3f6b3dc2b4f890bb743906a30d56ba4a5bfa4bbff92760"}, @@ -4211,4 +4206,4 @@ worker = ["asgi-correlation-id", "celery", "coloredlogs", "horizon-hwm-store", " [metadata] lock-version = "2.1" python-versions = "^3.11" -content-hash = "713ab8e45e6206d9911aada531c0c7cf61a5d7575a8a524886a4fdbd428661c1" +content-hash = "0ed9df128c16aafa7fce4f297ad48717a2e218fb6c485c2e762f0f17f15418d5" diff --git a/pyproject.toml b/pyproject.toml index 280fbf84..545ac865 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -135,7 +135,7 @@ pytest-lazy-fixtures = "^1.1.1" faker = "^37.4.0" coverage = "^7.9.1" gevent = ">=24.11.1,<26.0.0" -responses = "^0.25.7" +respx = "^0.22.0" dirty-equals = "^0.9.0" [tool.poetry.group.dev.dependencies] diff --git a/syncmaster/server/providers/auth/base_provider.py b/syncmaster/server/providers/auth/base_provider.py index becfd586..104236be 100644 --- a/syncmaster/server/providers/auth/base_provider.py +++ b/syncmaster/server/providers/auth/base_provider.py @@ -52,7 +52,7 @@ def __init__( ... @abstractmethod - async def get_current_user(self, access_token: Any, request: Request) -> User: + async def get_current_user(self, access_token: str | None, request: Request) -> User: """ This method should return currently logged in user. diff --git a/syncmaster/server/providers/auth/dummy_provider.py b/syncmaster/server/providers/auth/dummy_provider.py index 044e1251..ae0ed25f 100644 --- a/syncmaster/server/providers/auth/dummy_provider.py +++ b/syncmaster/server/providers/auth/dummy_provider.py @@ -37,7 +37,7 @@ def setup(cls, app: FastAPI) -> FastAPI: app.dependency_overrides[DummyAuthProviderSettings] = lambda: settings return app - async def get_current_user(self, access_token: str, *args, **kwargs) -> User: + async def get_current_user(self, access_token: str | None, *args, **kwargs) -> User: if not access_token: raise AuthorizationError("Missing auth credentials") diff --git a/syncmaster/server/providers/auth/keycloak_provider.py b/syncmaster/server/providers/auth/keycloak_provider.py index 39281864..6e6db37b 100644 --- a/syncmaster/server/providers/auth/keycloak_provider.py +++ b/syncmaster/server/providers/auth/keycloak_provider.py @@ -1,7 +1,7 @@ # SPDX-FileCopyrightText: 2023-2024 MTS PJSC # SPDX-License-Identifier: Apache-2.0 import logging -from typing import Annotated, Any +from typing import Annotated, Any, NoReturn from fastapi import Depends, FastAPI, Request from jwcrypto.common import JWException @@ -62,25 +62,24 @@ async def get_token_authorization_code_grant( client_secret: str | None = None, ) -> dict[str, Any]: try: - token = self.keycloak_openid.token( + return await self.keycloak_openid.a_token( grant_type="authorization_code", code=code, redirect_uri=self.settings.keycloak.redirect_uri, ) - return token except KeycloakOperationError as e: raise AuthorizationError("Failed to get token") from e - async def get_current_user(self, access_token: str, request: Request) -> User: # noqa: WPS231 + async def get_current_user(self, access_token: str | None, request: Request) -> User: # noqa: WPS231, WPS217 if not access_token: - log.debug("No access token found in session.") - self.redirect_to_auth() + log.debug("No access token found in session") + await self.redirect_to_auth() refresh_token = request.session.get("refresh_token") try: # if user is disabled or blocked in Keycloak after the token is issued, he will # remain authorized until the token expires (not more than 15 minutes in MTS SSO) - token_info = self.keycloak_openid.decode_token(token=access_token) + token_info = await self.keycloak_openid.a_decode_token(access_token) except (KeycloakOperationError, JWException) as e: log.info("Access token is invalid or expired: %s", e) token_info = None @@ -89,20 +88,20 @@ async def get_current_user(self, access_token: str, request: Request) -> User: log.debug("Access token invalid. Attempting to refresh.") try: - new_tokens = self.keycloak_openid.refresh_token(refresh_token) + new_tokens = await self.keycloak_openid.a_refresh_token(refresh_token) new_access_token = new_tokens["access_token"] new_refresh_token = new_tokens["refresh_token"] request.session["access_token"] = new_access_token request.session["refresh_token"] = new_refresh_token - token_info = self.keycloak_openid.decode_token( + token_info = await self.keycloak_openid.a_decode_token( token=new_access_token, ) log.debug("Access token refreshed and decoded successfully.") except (KeycloakOperationError, JWException) as e: log.debug("Failed to refresh access token: %s", e) - self.redirect_to_auth() + await self.redirect_to_auth() if not token_info: raise AuthorizationError("Invalid token payload") @@ -129,8 +128,8 @@ async def get_current_user(self, access_token: str, request: Request) -> User: ) return user - def redirect_to_auth(self) -> None: - auth_url = self.keycloak_openid.auth_url( + async def redirect_to_auth(self) -> NoReturn: + auth_url = await self.keycloak_openid.a_auth_url( redirect_uri=self.settings.keycloak.redirect_uri, scope=self.settings.keycloak.scope, ) @@ -142,7 +141,7 @@ async def logout(self, user: User, refresh_token: str | None) -> None: return try: - self.keycloak_openid.logout(refresh_token) + await self.keycloak_openid.a_logout(refresh_token) except KeycloakOperationError as err: msg = f"Can't logout user: {user.username}" log.debug("%s. Error: %s", msg, err) diff --git a/tests/test_unit/test_auth/auth_fixtures/__init__.py b/tests/test_unit/test_auth/auth_fixtures/__init__.py index ca1d1a65..e1357aa6 100644 --- a/tests/test_unit/test_auth/auth_fixtures/__init__.py +++ b/tests/test_unit/test_auth/auth_fixtures/__init__.py @@ -1,5 +1,6 @@ from tests.test_unit.test_auth.auth_fixtures.keycloak_fixture import ( create_session_cookie, + mock_keycloak_api, mock_keycloak_logout, mock_keycloak_realm, mock_keycloak_token_refresh, @@ -9,6 +10,7 @@ __all__ = [ "create_session_cookie", + "mock_keycloak_api", "mock_keycloak_logout", "mock_keycloak_realm", "mock_keycloak_token_refresh", diff --git a/tests/test_unit/test_auth/auth_fixtures/keycloak_fixture.py b/tests/test_unit/test_auth/auth_fixtures/keycloak_fixture.py index 256de00f..50cacedc 100644 --- a/tests/test_unit/test_auth/auth_fixtures/keycloak_fixture.py +++ b/tests/test_unit/test_auth/auth_fixtures/keycloak_fixture.py @@ -4,7 +4,8 @@ import jwt import pytest -import responses +import pytest_asyncio +import respx from cryptography.hazmat.primitives.asymmetric import rsa from cryptography.hazmat.primitives.serialization import ( Encoding, @@ -83,8 +84,17 @@ def _create_session_cookie(user, expire_in_msec=60000) -> str: return _create_session_cookie +@pytest_asyncio.fixture +async def mock_keycloak_api(settings): # noqa: F811 + keycloak_settings = settings.auth.model_dump()["keycloak"] + server_url = keycloak_settings["server_url"] + + async with respx.mock(base_url=server_url, assert_all_called=False) as respx_mock: + yield respx_mock + + @pytest.fixture -def mock_keycloak_well_known(settings): +def mock_keycloak_well_known(settings, mock_keycloak_api): keycloak_settings = settings.auth.model_dump()["keycloak"] server_url = keycloak_settings["server_url"] realm_name = keycloak_settings["client_id"] @@ -92,9 +102,7 @@ def mock_keycloak_well_known(settings): well_known_url = f"{realm_url}/.well-known/openid-configuration" openid_url = f"{realm_url}/protocol/openid-connect" - responses.add( - responses.GET, - well_known_url, + mock_keycloak_api.get(well_known_url).respond( json={ "authorization_endpoint": f"{openid_url}/auth", "token_endpoint": f"{openid_url}/token", @@ -103,35 +111,33 @@ def mock_keycloak_well_known(settings): "jwks_uri": f"{openid_url}/certs", "issuer": realm_url, }, - status=200, + status_code=200, content_type="application/json", ) @pytest.fixture -def mock_keycloak_realm(settings, rsa_keys): +def mock_keycloak_realm(settings, rsa_keys, mock_keycloak_api): keycloak_settings = settings.auth.model_dump()["keycloak"] server_url = keycloak_settings["server_url"] realm_name = keycloak_settings["client_id"] realm_url = f"{server_url}/realms/{realm_name}" public_pem_str = get_public_key_pem(rsa_keys["public_key"]) - responses.add( - responses.GET, - realm_url, + mock_keycloak_api.get(realm_url).respond( json={ "realm": realm_name, "public_key": public_pem_str, "token-service": f"{realm_url}/protocol/openid-connect/token", "account-service": f"{realm_url}/account", }, - status=200, + status_code=200, content_type="application/json", ) @pytest.fixture -def mock_keycloak_token_refresh(settings, rsa_keys): +def mock_keycloak_token_refresh(settings, rsa_keys, mock_keycloak_api): keycloak_settings = settings.auth.model_dump()["keycloak"] server_url = keycloak_settings["server_url"] realm_name = keycloak_settings["client_id"] @@ -154,30 +160,24 @@ def mock_keycloak_token_refresh(settings, rsa_keys): new_access_token = jwt.encode(payload, private_pem, algorithm="RS256") new_refresh_token = "mock_new_refresh_token" - responses.add( - responses.POST, - token_url, + mock_keycloak_api.post(token_url).respond( json={ "access_token": new_access_token, "refresh_token": new_refresh_token, "token_type": "bearer", "expires_in": expires_in, }, - status=200, + status_code=200, content_type="application/json", ) @pytest.fixture -def mock_keycloak_logout(settings): +def mock_keycloak_logout(settings, mock_keycloak_api): keycloak_settings = settings.auth.model_dump()["keycloak"] server_url = keycloak_settings["server_url"] realm_name = keycloak_settings["client_id"] realm_url = f"{server_url}/realms/{realm_name}" logout_url = f"{realm_url}/protocol/openid-connect/logout" - responses.add( - responses.POST, - logout_url, - status=204, - ) + mock_keycloak_api.post(logout_url).respond(status_code=204) diff --git a/tests/test_unit/test_auth/test_auth_keycloak.py b/tests/test_unit/test_auth/test_auth_keycloak.py index 5acaecb4..ed7d89c3 100644 --- a/tests/test_unit/test_auth/test_auth_keycloak.py +++ b/tests/test_unit/test_auth/test_auth_keycloak.py @@ -1,7 +1,6 @@ import logging import pytest -import responses from dirty_equals import IsStr from httpx import AsyncClient @@ -12,7 +11,6 @@ pytestmark = [pytest.mark.asyncio, pytest.mark.server] -@responses.activate @pytest.mark.parametrize( "settings", [ @@ -44,7 +42,6 @@ async def test_keycloak_get_user_unauthorized( } -@responses.activate @pytest.mark.flaky @pytest.mark.parametrize( "settings", @@ -81,7 +78,6 @@ async def test_keycloak_get_user_authorized( } -@responses.activate @pytest.mark.parametrize( "settings", [ @@ -120,7 +116,6 @@ async def test_keycloak_get_user_expired_access_token( } -@responses.activate @pytest.mark.parametrize( "settings", [ @@ -153,7 +148,6 @@ async def test_keycloak_get_user_inactive( } -@responses.activate @pytest.mark.parametrize( "settings", [ @@ -184,7 +178,6 @@ async def test_keycloak_auth_callback( assert response.status_code == 204, response.text -@responses.activate @pytest.mark.parametrize( "settings", [