diff --git a/python/packages/jumpstarter-cli-common/jumpstarter_cli_common/oauth2_client_test.py b/python/packages/jumpstarter-cli-common/jumpstarter_cli_common/oauth2_client_test.py new file mode 100644 index 000000000..758b304d2 --- /dev/null +++ b/python/packages/jumpstarter-cli-common/jumpstarter_cli_common/oauth2_client_test.py @@ -0,0 +1,408 @@ +import warnings +from unittest.mock import AsyncMock, MagicMock, patch +from urllib.parse import parse_qs, urlparse + +import pytest +import requests + +from jumpstarter_cli_common.oidc import ( + Config, + OAuthError, + OAuthStateMismatchError, + _OAuth2Client, +) + + +@pytest.fixture +def anyio_backend(): + return "asyncio" + + +class TestOAuth2ClientInit: + def test_init_sets_attributes(self): + client = _OAuth2Client(client_id="my-client", scope=["openid", "profile"]) + assert client.client_id == "my-client" + assert client.scope == ["openid", "profile"] + assert client.redirect_uri is None + assert client._expected_state is None + assert isinstance(client._session, requests.Session) + + def test_init_with_redirect_uri(self): + client = _OAuth2Client( + client_id="my-client", + scope=["openid"], + redirect_uri="http://localhost:8080/callback", + ) + assert client.redirect_uri == "http://localhost:8080/callback" + + +class TestOAuth2ClientVerifyProperty: + def test_verify_getter_returns_session_verify(self): + client = _OAuth2Client(client_id="c", scope=[]) + assert client.verify is True + + def test_verify_setter_updates_session_verify(self): + client = _OAuth2Client(client_id="c", scope=[]) + client.verify = False + assert client.verify is False + assert client._session.verify is False + + def test_verify_setter_with_cert_path(self): + client = _OAuth2Client(client_id="c", scope=[]) + client.verify = "/path/to/ca-bundle.crt" + assert client.verify == "/path/to/ca-bundle.crt" + assert client._session.verify == "/path/to/ca-bundle.crt" + + +class TestOAuth2ClientClose: + def test_close_calls_session_close(self): + client = _OAuth2Client(client_id="c", scope=[]) + with patch.object(client._session, "close") as mock_close: + client.close() + mock_close.assert_called_once() + + +class TestOAuth2ClientCreateAuthorizationUrl: + def test_basic_url_construction(self): + client = _OAuth2Client(client_id="test-client", scope=["openid", "profile"]) + url, state = client.create_authorization_url("https://auth.example.com/authorize") + + parsed = urlparse(url) + params = parse_qs(parsed.query) + + assert parsed.scheme == "https" + assert parsed.netloc == "auth.example.com" + assert parsed.path == "/authorize" + assert params["response_type"] == ["code"] + assert params["client_id"] == ["test-client"] + assert params["scope"] == ["openid profile"] + assert params["state"] == [state] + assert len(state) > 0 + + def test_stores_expected_state(self): + client = _OAuth2Client(client_id="test-client", scope=["openid"]) + _, state = client.create_authorization_url("https://auth.example.com/authorize") + assert client._expected_state == state + + def test_includes_redirect_uri_when_set(self): + client = _OAuth2Client( + client_id="test-client", + scope=["openid"], + redirect_uri="http://localhost:9999/callback", + ) + url, _state = client.create_authorization_url("https://auth.example.com/authorize") + params = parse_qs(urlparse(url).query) + assert params["redirect_uri"] == ["http://localhost:9999/callback"] + + def test_no_redirect_uri_when_not_set(self): + client = _OAuth2Client(client_id="test-client", scope=["openid"]) + url, _state = client.create_authorization_url("https://auth.example.com/authorize") + params = parse_qs(urlparse(url).query) + assert "redirect_uri" not in params + + def test_extra_kwargs_included(self): + client = _OAuth2Client(client_id="test-client", scope=["openid"]) + url, _state = client.create_authorization_url( + "https://auth.example.com/authorize", + prompt="consent", + nonce="abc123", + ) + params = parse_qs(urlparse(url).query) + assert params["prompt"] == ["consent"] + assert params["nonce"] == ["abc123"] + + def test_url_with_existing_query_params(self): + client = _OAuth2Client(client_id="test-client", scope=["openid"]) + url, _state = client.create_authorization_url("https://auth.example.com/authorize?foo=bar") + assert "authorize?foo=bar&" in url + + def test_state_is_unique(self): + client = _OAuth2Client(client_id="test-client", scope=["openid"]) + _, state1 = client.create_authorization_url("https://auth.example.com/authorize") + _, state2 = client.create_authorization_url("https://auth.example.com/authorize") + assert state1 != state2 + + +class TestOAuth2ClientFetchToken: + def _mock_response(self, json_data, status_code=200): + mock_resp = MagicMock() + mock_resp.json.return_value = json_data + mock_resp.raise_for_status.return_value = None + mock_resp.status_code = status_code + return mock_resp + + def test_fetch_token_with_grant_type(self): + token_data = {"access_token": "tok123", "token_type": "Bearer"} + client = _OAuth2Client(client_id="my-client", scope=["openid", "profile"]) + + with patch.object(client._session, "post", return_value=self._mock_response(token_data)) as mock_post: + result = client.fetch_token( + "https://auth.example.com/token", + grant_type="password", + username="user", + password="pass", + ) + + assert result == token_data + call_kwargs = mock_post.call_args + post_data = call_kwargs.kwargs["data"] + assert post_data["client_id"] == "my-client" + assert post_data["grant_type"] == "password" + assert post_data["username"] == "user" + assert post_data["password"] == "pass" + assert post_data["scope"] == "openid profile" + + def test_fetch_token_sends_correct_headers(self): + token_data = {"access_token": "tok123"} + client = _OAuth2Client(client_id="my-client", scope=["openid"]) + + with patch.object(client._session, "post", return_value=self._mock_response(token_data)) as mock_post: + client.fetch_token("https://auth.example.com/token", grant_type="password") + + headers = mock_post.call_args.kwargs["headers"] + assert headers["Accept"] == "application/json" + assert headers["Content-Type"] == "application/x-www-form-urlencoded" + + def test_fetch_token_with_authorization_response(self): + token_data = {"access_token": "tok456", "token_type": "Bearer"} + client = _OAuth2Client( + client_id="my-client", + scope=["openid"], + redirect_uri="http://localhost:8080/callback", + ) + client._expected_state = "xyz" + + callback_url = "http://localhost:8080/callback?code=authcode123&state=xyz" + + with patch.object(client._session, "post", return_value=self._mock_response(token_data)) as mock_post: + result = client.fetch_token( + "https://auth.example.com/token", + authorization_response=callback_url, + ) + + assert result == token_data + post_data = mock_post.call_args.kwargs["data"] + assert post_data["code"] == "authcode123" + assert post_data["redirect_uri"] == "http://localhost:8080/callback" + assert post_data["grant_type"] == "authorization_code" + + def test_fetch_token_validates_state(self): + client = _OAuth2Client(client_id="my-client", scope=["openid"]) + client._expected_state = "expected-state" + + callback_url = "http://localhost:8080/callback?code=abc&state=wrong-state" + + with pytest.raises(OAuthStateMismatchError, match="state mismatch"): + client.fetch_token( + "https://auth.example.com/token", + authorization_response=callback_url, + ) + + def test_fetch_token_validates_state_missing_in_response(self): + client = _OAuth2Client(client_id="my-client", scope=["openid"]) + client._expected_state = "expected-state" + + callback_url = "http://localhost:8080/callback?code=abc" + + with pytest.raises(OAuthStateMismatchError, match="state mismatch"): + client.fetch_token( + "https://auth.example.com/token", + authorization_response=callback_url, + ) + + def test_fetch_token_skips_state_validation_when_no_expected_state(self): + token_data = {"access_token": "tok789"} + client = _OAuth2Client(client_id="my-client", scope=["openid"]) + # _expected_state is None (no create_authorization_url was called) + + callback_url = "http://localhost:8080/callback?code=abc&state=any" + + with patch.object(client._session, "post", return_value=self._mock_response(token_data)): + result = client.fetch_token( + "https://auth.example.com/token", + authorization_response=callback_url, + ) + assert result == token_data + + def test_fetch_token_raises_on_oauth_error_response(self): + client = _OAuth2Client(client_id="my-client", scope=["openid"]) + + callback_url = "http://localhost:8080/callback?error=access_denied&error_description=User+denied+access" + + with pytest.raises(OAuthError, match="access_denied.*User denied access"): + client.fetch_token( + "https://auth.example.com/token", + authorization_response=callback_url, + ) + + def test_fetch_token_raises_on_oauth_error_without_description(self): + client = _OAuth2Client(client_id="my-client", scope=["openid"]) + + callback_url = "http://localhost:8080/callback?error=server_error" + + with pytest.raises(OAuthError, match="server_error"): + client.fetch_token( + "https://auth.example.com/token", + authorization_response=callback_url, + ) + + def test_fetch_token_authorization_response_without_redirect_uri(self): + token_data = {"access_token": "tok000"} + client = _OAuth2Client(client_id="my-client", scope=["openid"]) + + callback_url = "http://localhost:8080/callback?code=abc" + + with patch.object(client._session, "post", return_value=self._mock_response(token_data)) as mock_post: + client.fetch_token( + "https://auth.example.com/token", + authorization_response=callback_url, + ) + + post_data = mock_post.call_args.kwargs["data"] + assert "redirect_uri" not in post_data + + def test_fetch_token_scope_provided_in_kwargs(self): + token_data = {"access_token": "tok999"} + client = _OAuth2Client(client_id="my-client", scope=["openid", "profile"]) + + with patch.object(client._session, "post", return_value=self._mock_response(token_data)) as mock_post: + client.fetch_token( + "https://auth.example.com/token", + grant_type="client_credentials", + scope="custom_scope", + ) + + post_data = mock_post.call_args.kwargs["data"] + assert post_data["scope"] == "custom_scope" + + def test_fetch_token_raises_on_http_error(self): + client = _OAuth2Client(client_id="my-client", scope=["openid"]) + mock_resp = MagicMock() + mock_resp.raise_for_status.side_effect = requests.HTTPError("401 Unauthorized") + + with patch.object(client._session, "post", return_value=mock_resp): + with pytest.raises(requests.HTTPError): + client.fetch_token("https://auth.example.com/token", grant_type="password") + + def test_fetch_token_raises_on_non_json_response(self): + client = _OAuth2Client(client_id="my-client", scope=["openid"]) + mock_resp = MagicMock() + mock_resp.raise_for_status.return_value = None + mock_resp.status_code = 200 + mock_resp.json.side_effect = ValueError("No JSON") + + with patch.object(client._session, "post", return_value=mock_resp): + with pytest.raises(OAuthError, match="non-JSON response"): + client.fetch_token("https://auth.example.com/token", grant_type="password") + + +class TestConfigClient: + def test_client_returns_oauth2_client(self): + config = Config(issuer="https://issuer.example.com", client_id="test-client") + session = config.client() + assert isinstance(session, _OAuth2Client) + assert session.client_id == "test-client" + assert session.scope == ["openid", "profile"] + + def test_client_passes_kwargs(self): + config = Config(issuer="https://issuer.example.com", client_id="test-client") + session = config.client(redirect_uri="http://localhost:9999/callback") + assert session.redirect_uri == "http://localhost:9999/callback" + + +class TestConfigGrantMethods: + """Tests for Config grant methods that use _OAuth2Client.fetch_token.""" + + @pytest.fixture + def config(self): + return Config(issuer="https://issuer.example.com", client_id="test-client") + + @pytest.fixture + def mock_configuration(self): + return { + "token_endpoint": "https://issuer.example.com/token", + "authorization_endpoint": "https://issuer.example.com/authorize", + } + + @pytest.fixture + def token_response(self): + return {"access_token": "access-tok", "token_type": "Bearer", "expires_in": 3600} + + @pytest.mark.anyio + async def test_token_exchange_grant(self, config, mock_configuration, token_response): + with ( + patch.object(config, "configuration", new_callable=AsyncMock, return_value=mock_configuration), + patch.object(_OAuth2Client, "fetch_token", return_value=token_response) as mock_fetch, + patch.object(_OAuth2Client, "close") as mock_close, + ): + result = await config.token_exchange_grant(token="id-token-value", connector_id="ldap") + + assert result == token_response + call_kwargs = mock_fetch.call_args + assert call_kwargs.args[0] == "https://issuer.example.com/token" + assert call_kwargs.kwargs["grant_type"] == "urn:ietf:params:oauth:grant-type:token-exchange" + assert call_kwargs.kwargs["subject_token"] == "id-token-value" + assert call_kwargs.kwargs["connector_id"] == "ldap" + mock_close.assert_called_once() + + @pytest.mark.anyio + async def test_refresh_token_grant(self, config, mock_configuration, token_response): + with ( + patch.object(config, "configuration", new_callable=AsyncMock, return_value=mock_configuration), + patch.object(_OAuth2Client, "fetch_token", return_value=token_response) as mock_fetch, + patch.object(_OAuth2Client, "close") as mock_close, + ): + result = await config.refresh_token_grant(refresh_token="refresh-tok") + + assert result == token_response + call_kwargs = mock_fetch.call_args + assert call_kwargs.args[0] == "https://issuer.example.com/token" + assert call_kwargs.kwargs["grant_type"] == "refresh_token" + assert call_kwargs.kwargs["refresh_token"] == "refresh-tok" + mock_close.assert_called_once() + + @pytest.mark.anyio + async def test_password_grant(self, config, mock_configuration, token_response): + with ( + patch.object(config, "configuration", new_callable=AsyncMock, return_value=mock_configuration), + patch.object(_OAuth2Client, "fetch_token", return_value=token_response) as mock_fetch, + patch.object(_OAuth2Client, "close") as mock_close, + ): + result = await config.password_grant(username="testuser", password="testpass") + + assert result == token_response + call_kwargs = mock_fetch.call_args + assert call_kwargs.args[0] == "https://issuer.example.com/token" + assert call_kwargs.kwargs["grant_type"] == "password" + assert call_kwargs.kwargs["username"] == "testuser" + assert call_kwargs.kwargs["password"] == "testpass" + mock_close.assert_called_once() + + @pytest.mark.anyio + async def test_grant_closes_client_on_error(self, config, mock_configuration): + with ( + patch.object(config, "configuration", new_callable=AsyncMock, return_value=mock_configuration), + patch.object(_OAuth2Client, "fetch_token", side_effect=requests.HTTPError("500")), + patch.object(_OAuth2Client, "close") as mock_close, + ): + with pytest.raises(requests.HTTPError): + await config.password_grant(username="u", password="p") + + mock_close.assert_called_once() + + +class TestNoAuthlibDeprecationWarning: + def test_import_oidc_module_does_not_raise_deprecation_warning(self): + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("always") + import importlib + + import jumpstarter_cli_common.oidc + + importlib.reload(jumpstarter_cli_common.oidc) + deprecation_warnings = [ + w for w in caught if issubclass(w.category, DeprecationWarning) + ] + assert deprecation_warnings == [], ( + f"DeprecationWarning raised on import: {deprecation_warnings}" + ) diff --git a/python/packages/jumpstarter-cli-common/jumpstarter_cli_common/oidc.py b/python/packages/jumpstarter-cli-common/jumpstarter_cli_common/oidc.py index 08661c853..9223b7614 100644 --- a/python/packages/jumpstarter-cli-common/jumpstarter_cli_common/oidc.py +++ b/python/packages/jumpstarter-cli-common/jumpstarter_cli_common/oidc.py @@ -1,24 +1,126 @@ import json import os +import secrets import ssl import time from dataclasses import dataclass from functools import wraps -from typing import ClassVar +from typing import Any, ClassVar +from urllib.parse import parse_qs, urlencode, urlparse import aiohttp import certifi import click +import requests from aiohttp import web from anyio import create_memory_object_stream from anyio.to_thread import run_sync -from authlib.integrations.requests_client import OAuth2Session from joserfc.jws import extract_compact from yarl import URL from jumpstarter.config.env import JMP_OIDC_CALLBACK_PORT +class OAuthError(Exception): + """Raised when an OAuth2 error response is received.""" + + +class OAuthStateMismatchError(OAuthError): + """Raised when the OAuth2 state parameter does not match the expected value.""" + + +class _OAuth2Client: + """Minimal OAuth2 client wrapping requests.Session for authorization code, + password, token-exchange, and refresh-token grants.""" + + def __init__(self, client_id: str, scope: list[str], redirect_uri: str | None = None): + self.client_id = client_id + self.scope = scope + self.redirect_uri = redirect_uri + self._session = requests.Session() + self._expected_state: str | None = None + + @property + def verify(self) -> bool | str: + return self._session.verify + + @verify.setter + def verify(self, value: bool | str) -> None: + self._session.verify = value + + def close(self) -> None: + """Close the underlying requests session.""" + self._session.close() + + def create_authorization_url(self, url: str, **kwargs) -> tuple[str, str]: + """Build an authorization URL with a generated state parameter.""" + state = secrets.token_urlsafe(32) + self._expected_state = state + params = { + "response_type": "code", + "client_id": self.client_id, + "scope": " ".join(self.scope), + "state": state, + } + if self.redirect_uri: + params["redirect_uri"] = self.redirect_uri + params.update(kwargs) + separator = "&" if "?" in url else "?" + return f"{url}{separator}{urlencode(params)}", state + + def fetch_token(self, url: str, **kwargs) -> dict[str, Any]: + """Exchange credentials for an OAuth2 token via HTTP POST.""" + data: dict[str, Any] = {"client_id": self.client_id} + + authorization_response = kwargs.pop("authorization_response", None) + if authorization_response: + parsed = urlparse(authorization_response) + qs = parse_qs(parsed.query) + + error = qs.get("error", [None])[0] + if error: + description = qs.get("error_description", [""])[0] + raise OAuthError( + f"OAuth2 authorization error: {error}" + + (f" - {description}" if description else "") + ) + + if self._expected_state is not None: + returned_state = qs.get("state", [None])[0] + if returned_state != self._expected_state: + raise OAuthStateMismatchError( + "OAuth2 state mismatch: possible CSRF attack" + ) + + code = qs.get("code", [None])[0] + if code: + data["code"] = code + if self.redirect_uri: + data["redirect_uri"] = self.redirect_uri + data.setdefault("grant_type", "authorization_code") + + data.update(kwargs) + + if "scope" not in data: + data["scope"] = " ".join(self.scope) + + response = self._session.post( + url, + data=data, + headers={ + "Accept": "application/json", + "Content-Type": "application/x-www-form-urlencoded", + }, + ) + response.raise_for_status() + try: + return response.json() + except ValueError as exc: + raise OAuthError( + f"Token endpoint returned non-JSON response (HTTP {response.status_code})" + ) from exc + + def _get_ssl_context() -> ssl.SSLContext: """Create an SSL context that respects SSL_CERT_FILE environment variable.""" ssl_ctx = ssl.create_default_context() @@ -77,49 +179,52 @@ def _scopes(self) -> list[str]: return list(self.scope) def client(self, **kwargs): - session = OAuth2Session(client_id=self.client_id, scope=self._scopes(), **kwargs) + session = _OAuth2Client(client_id=self.client_id, scope=self._scopes(), **kwargs) session.verify = False if self.insecure_tls else (os.environ.get("SSL_CERT_FILE") or certifi.where()) return session async def token_exchange_grant(self, token: str, **kwargs): config = await self.configuration() - client = self.client() - - return await run_sync( - lambda: client.fetch_token( - config["token_endpoint"], - grant_type="urn:ietf:params:oauth:grant-type:token-exchange", - requested_token_type="urn:ietf:params:oauth:token-type:access_token", - subject_token_type="urn:ietf:params:oauth:token-type:id_token", - subject_token=token, - **kwargs, + try: + return await run_sync( + lambda: client.fetch_token( + config["token_endpoint"], + grant_type="urn:ietf:params:oauth:grant-type:token-exchange", + requested_token_type="urn:ietf:params:oauth:token-type:access_token", + subject_token_type="urn:ietf:params:oauth:token-type:id_token", + subject_token=token, + **kwargs, + ) ) - ) + finally: + client.close() async def refresh_token_grant(self, refresh_token: str): config = await self.configuration() - client = self.client() - - return await run_sync( - lambda: client.fetch_token( - config["token_endpoint"], - grant_type="refresh_token", - refresh_token=refresh_token, + try: + return await run_sync( + lambda: client.fetch_token( + config["token_endpoint"], + grant_type="refresh_token", + refresh_token=refresh_token, + ) ) - ) + finally: + client.close() async def password_grant(self, username: str, password: str): config = await self.configuration() - client = self.client() - - return await run_sync( - lambda: client.fetch_token( - config["token_endpoint"], grant_type="password", username=username, password=password + try: + return await run_sync( + lambda: client.fetch_token( + config["token_endpoint"], grant_type="password", username=username, password=password + ) ) - ) + finally: + client.close() async def authorization_code_grant(self, callback_port: int | None = None, prompt: str | None = None): config = await self.configuration() @@ -173,9 +278,12 @@ async def callback(request): await site.stop() await runner.cleanup() - return await run_sync( - lambda: client.fetch_token(config["token_endpoint"], authorization_response=authorization_response) - ) + try: + return await run_sync( + lambda: client.fetch_token(config["token_endpoint"], authorization_response=authorization_response) + ) + finally: + client.close() def decode_jwt(token: str): diff --git a/python/packages/jumpstarter-cli-common/pyproject.toml b/python/packages/jumpstarter-cli-common/pyproject.toml index e892545fa..701a52858 100644 --- a/python/packages/jumpstarter-cli-common/pyproject.toml +++ b/python/packages/jumpstarter-cli-common/pyproject.toml @@ -10,7 +10,7 @@ dependencies = [ "jumpstarter", "pydantic>=2.8.2", "click>=8.1.7.2", - "authlib>=1.4.1", + "requests>=2.32.0", "truststore>=0.10.1", "joserfc>=1.0.3", "yarl>=1.18.3", diff --git a/python/uv.lock b/python/uv.lock index 23f95c4eb..5dd853b9e 100644 --- a/python/uv.lock +++ b/python/uv.lock @@ -364,19 +364,6 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/77/06/bb80f5f86020c4551da315d78b3ab75e8228f89f0162f2c3a819e407941a/attrs-25.3.0-py3-none-any.whl", hash = "sha256:427318ce031701fea540783410126f03899a97ffc6f61596ad581ac2e40e3bc3", size = 63815, upload-time = "2025-03-13T11:10:21.14Z" }, ] -[[package]] -name = "authlib" -version = "1.6.0" -source = { registry = "https://pypi.org/simple" } -dependencies = [ - { name = "cryptography", version = "44.0.3", source = { registry = "https://pypi.org/simple" }, marker = "python_full_version < '3.12'" }, - { name = "cryptography", version = "45.0.4", source = { registry = "https://pypi.org/simple" }, marker = "python_full_version >= '3.12'" }, -] -sdist = { url = "https://files.pythonhosted.org/packages/a2/9d/b1e08d36899c12c8b894a44a5583ee157789f26fc4b176f8e4b6217b56e1/authlib-1.6.0.tar.gz", hash = "sha256:4367d32031b7af175ad3a323d571dc7257b7099d55978087ceae4a0d88cd3210", size = 158371, upload-time = "2025-05-23T00:21:45.011Z" } -wheels = [ - { url = "https://files.pythonhosted.org/packages/84/29/587c189bbab1ccc8c86a03a5d0e13873df916380ef1be461ebe6acebf48d/authlib-1.6.0-py2.py3-none-any.whl", hash = "sha256:91685589498f79e8655e8a8947431ad6288831d643f11c55c2143ffcc738048d", size = 239981, upload-time = "2025-05-23T00:21:43.075Z" }, -] - [[package]] name = "babel" version = "2.17.0" @@ -2154,11 +2141,11 @@ dev = [ name = "jumpstarter-cli-common" source = { editable = "packages/jumpstarter-cli-common" } dependencies = [ - { name = "authlib" }, { name = "click" }, { name = "joserfc" }, { name = "jumpstarter" }, { name = "pydantic" }, + { name = "requests" }, { name = "rich" }, { name = "truststore" }, { name = "yarl" }, @@ -2174,11 +2161,11 @@ dev = [ [package.metadata] requires-dist = [ - { name = "authlib", specifier = ">=1.4.1" }, { name = "click", specifier = ">=8.1.7.2" }, { name = "joserfc", specifier = ">=1.0.3" }, { name = "jumpstarter", editable = "packages/jumpstarter" }, { name = "pydantic", specifier = ">=2.8.2" }, + { name = "requests", specifier = ">=2.32.0" }, { name = "rich", specifier = ">=14.0.0" }, { name = "truststore", specifier = ">=0.10.1" }, { name = "yarl", specifier = ">=1.18.3" },