From e698de3e4e3ce3533b77dc108d55c99a3f61be97 Mon Sep 17 00:00:00 2001 From: Stefaan Lippens Date: Tue, 20 Apr 2021 16:43:42 +0200 Subject: [PATCH] Add support for default OIDC provider (based on order advertised by backend) see Open-EO/openeo-api#373 --- CHANGELOG.md | 2 +- openeo/rest/connection.py | 16 +++++---- tests/rest/test_connection.py | 62 +++++++++++++++++++++++++++-------- 3 files changed, 58 insertions(+), 22 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2762a8900..781eb29f5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,7 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Add dependency on `xarray` package ([#159](https://github.com/Open-EO/openeo-python-client/issues/159), [#190](https://github.com/Open-EO/openeo-python-client/pull/190), EP-3578) - Add support for default OIDC clients advertised by backend ([#192](https://github.com/Open-EO/openeo-python-client/issues/192), [Open-EO/openeo-api#366](https://github.com/Open-EO/openeo-api/pull/366)) - +- Add support for default OIDC provider (based on provider order advertised by backend) ([Open-EO/openeo-api#373](https://github.com/Open-EO/openeo-api/pull/373)) ### Changed diff --git a/openeo/rest/connection.py b/openeo/rest/connection.py index ba13aacab..ac0bb6111 100644 --- a/openeo/rest/connection.py +++ b/openeo/rest/connection.py @@ -5,6 +5,7 @@ import logging import sys import warnings +from collections import OrderedDict from pathlib import Path from typing import Dict, List, Tuple, Union, Callable, Optional, Any, Iterator from urllib.parse import urljoin @@ -293,7 +294,7 @@ def _get_oidc_provider(self, provider_id: Union[str, None] = None) -> Tuple[str, """ if self._api_version.at_least("1.0.0"): oidc_info = self.get("/credentials/oidc", expected_status=200).json() - providers = {p["id"]: p for p in oidc_info["providers"]} + providers = OrderedDict((p["id"], p) for p in oidc_info["providers"]) _log.info("Found OIDC providers: {p}".format(p=list(providers.keys()))) if provider_id: if provider_id not in providers: @@ -305,7 +306,7 @@ def _get_oidc_provider(self, provider_id: Union[str, None] = None) -> Tuple[str, provider = providers[provider_id] elif len(providers) == 1: provider_id, provider = providers.popitem() - _log.info("No OIDC provider given, but only one available: {p!r}. Use that one.".format( + _log.info("No OIDC provider given, but only one available: {p!r}. Using that one.".format( p=provider_id )) else: @@ -317,13 +318,14 @@ def _get_oidc_provider(self, provider_id: Union[str, None] = None) -> Tuple[str, provider_id = intersection.pop() provider = providers[provider_id] _log.info( - "No OIDC provider id given, but only one in config (backend {b!r}): {p!r}." - " Use that one.".format(b=backend, p=provider_id) + "No OIDC provider given, but only one in config (for backend {b!r}): {p!r}." + " Using that one.".format(b=backend, p=provider_id) ) else: - raise OpenEoClientException("No OIDC provider id given. Pick one from: {p!r}.".format( - p=list(providers.keys())) - ) + provider_id, provider = providers.popitem(last=False) + _log.info("No OIDC provider given. Using first provider {p!r} as advertised by backend.".format( + p=provider_id + )) provider = OidcProviderInfo.from_dict(provider) else: # Per spec: '/credentials/oidc' will redirect to OpenID Connect discovery document diff --git a/tests/rest/test_connection.py b/tests/rest/test_connection.py index 74fe84179..8cfc92db2 100644 --- a/tests/rest/test_connection.py +++ b/tests/rest/test_connection.py @@ -1,3 +1,4 @@ +import logging import re import typing import unittest.mock as mock @@ -457,7 +458,7 @@ def test_authenticate_oidc_authorization_code_040(requests_mock): @pytest.mark.slow -def test_authenticate_oidc_authorization_code_100_single_implicit(requests_mock): +def test_authenticate_oidc_authorization_code_100_single_implicit(requests_mock, caplog): requests_mock.get(API_URL, json={"api_version": "1.0.0"}) client_id = "myclient" requests_mock.get(API_URL + 'credentials/oidc', json={ @@ -473,11 +474,13 @@ def test_authenticate_oidc_authorization_code_100_single_implicit(requests_mock) ) # With all this set up, kick off the openid connect flow + caplog.set_level(logging.INFO) conn = Connection(API_URL) assert isinstance(conn.auth, NullAuth) conn.authenticate_oidc_authorization_code(client_id=client_id, webbrowser_open=oidc_mock.webbrowser_open) assert isinstance(conn.auth, BearerAuth) assert conn.auth.bearer == 'oidc/fauth/' + oidc_mock.state["access_token"] + assert "No OIDC provider given, but only one available: 'fauth'. Using that one." in caplog.text def test_authenticate_oidc_authorization_code_100_single_wrong_id(requests_mock): @@ -496,7 +499,7 @@ def test_authenticate_oidc_authorization_code_100_single_wrong_id(requests_mock) ) -def test_authenticate_oidc_authorization_code_100_multiple_no_id(requests_mock): +def test_authenticate_oidc_authorization_code_100_multiple_no_given_id(requests_mock, caplog): requests_mock.get(API_URL, json={"api_version": "1.0.0"}) client_id = "myclient" requests_mock.get(API_URL + 'credentials/oidc', json={ @@ -505,13 +508,23 @@ def test_authenticate_oidc_authorization_code_100_multiple_no_id(requests_mock): {"id": "bauth", "issuer": "https://bauth.test", "title": "Bar Auth", "scopes": ["openid", "w"]}, ] }) + oidc_mock = OidcMock( + requests_mock=requests_mock, + expected_grant_type="authorization_code", + expected_client_id=client_id, + expected_fields={"scope": "openid w"}, + oidc_discovery_url="https://fauth.test/.well-known/openid-configuration", + scopes_supported=["openid", "w"], + ) # With all this set up, kick off the openid connect flow + caplog.set_level(logging.INFO) conn = Connection(API_URL) assert isinstance(conn.auth, NullAuth) - match = r"No OIDC provider id given. Pick one from: \[('fauth', 'bauth'|'bauth', 'fauth')\]\." - with pytest.raises(OpenEoClientException, match=match): - conn.authenticate_oidc_authorization_code(client_id=client_id, webbrowser_open=pytest.fail) + conn.authenticate_oidc_authorization_code(client_id=client_id, webbrowser_open=oidc_mock.webbrowser_open) + assert isinstance(conn.auth, BearerAuth) + assert conn.auth.bearer == 'oidc/fauth/' + oidc_mock.state["access_token"] + assert "No OIDC provider given. Using first provider 'fauth' as advertised by backend." in caplog.text def test_authenticate_oidc_authorization_code_100_multiple_wrong_id(requests_mock): @@ -829,7 +842,7 @@ def test_authenticate_oidc_device_flow(requests_mock, store_refresh_token, scope @pytest.mark.slow -def test_authenticate_oidc_device_flow_client_from_config(requests_mock, auth_config): +def test_authenticate_oidc_device_flow_client_from_config(requests_mock, auth_config, caplog): requests_mock.get(API_URL, json={"api_version": "1.0.0"}) client_id = "myclient" client_secret = "$3cr3t" @@ -853,6 +866,7 @@ def test_authenticate_oidc_device_flow_client_from_config(requests_mock, auth_co ) # With all this set up, kick off the openid connect flow + caplog.set_level(logging.INFO) refresh_token_store = mock.Mock() conn = Connection(API_URL, refresh_token_store=refresh_token_store) assert isinstance(conn.auth, NullAuth) @@ -861,6 +875,8 @@ def test_authenticate_oidc_device_flow_client_from_config(requests_mock, auth_co assert isinstance(conn.auth, BearerAuth) assert conn.auth.bearer == 'oidc/oi/' + oidc_mock.state["access_token"] assert refresh_token_store.mock_calls == [] + assert "No OIDC provider given, but only one available: 'oi'. Using that one." in caplog.text + assert "Using client_id 'myclient' from config (provider 'oi')" in caplog.text @pytest.mark.slow @@ -893,9 +909,9 @@ def test_authenticate_oidc_device_flow_no_support(requests_mock, auth_config): with pytest.raises(OidcException, match="No support for device code flow"): conn.authenticate_oidc_device() - -def test_authenticate_oidc_device_flow_multiple_providers_no_given(requests_mock, auth_config): - """OIDC device flow with multiple OIDC providers and none specified to use.""" +@pytest.mark.slow +def test_authenticate_oidc_device_flow_multiple_providers_no_given(requests_mock, auth_config, caplog): + """OIDC device flow + PKCE with multiple OIDC providers and none specified to use.""" requests_mock.get(API_URL, json={"api_version": "1.0.0"}) client_id = "myclient" requests_mock.get(API_URL + 'credentials/oidc', json={ @@ -904,18 +920,33 @@ def test_authenticate_oidc_device_flow_multiple_providers_no_given(requests_mock {"id": "bauth", "issuer": "https://bauth.test", "title": "Bar Auth", "scopes": ["openid", "w"]}, ] }) + oidc_mock = OidcMock( + requests_mock=requests_mock, + expected_grant_type="urn:ietf:params:oauth:grant-type:device_code", + expected_client_id=client_id, + expected_fields={ + "scope": "openid w", "code_verifier": True, "code_challenge": True + }, + scopes_supported=["openid", "w"], + oidc_discovery_url="https://fauth.test/.well-known/openid-configuration", + ) assert auth_config.load() == {} # With all this set up, kick off the openid connect flow - conn = Connection(API_URL) + caplog.set_level(logging.INFO) + refresh_token_store = mock.Mock() + conn = Connection(API_URL, refresh_token_store=refresh_token_store) assert isinstance(conn.auth, NullAuth) - match = r"No OIDC provider id given. Pick one from: \[('fauth', 'bauth'|'bauth', 'fauth')\]\." - with pytest.raises(OpenEoClientException, match=match): - conn.authenticate_oidc_device(client_id=client_id) + oidc_mock.state["device_code_callback_timeline"] = ["great success"] + conn.authenticate_oidc_device(client_id=client_id) + assert isinstance(conn.auth, BearerAuth) + assert conn.auth.bearer == 'oidc/fauth/' + oidc_mock.state["access_token"] + assert refresh_token_store.mock_calls == [] + assert "No OIDC provider given. Using first provider 'fauth' as advertised by backend." in caplog.text @pytest.mark.slow -def test_authenticate_oidc_device_flow_multiple_provider_one_config_no_given(requests_mock, auth_config): +def test_authenticate_oidc_device_flow_multiple_provider_one_config_no_given(requests_mock, auth_config, caplog): """OIDC device flow + PKCE with multiple OIDC providers, one in config and none specified to use.""" requests_mock.get(API_URL, json={"api_version": "1.0.0"}) client_id = "myclient" @@ -939,6 +970,7 @@ def test_authenticate_oidc_device_flow_multiple_provider_one_config_no_given(req auth_config.set_oidc_client_config(backend=API_URL, provider_id="fauth", client_id=client_id) # With all this set up, kick off the openid connect flow + caplog.set_level(logging.INFO) refresh_token_store = mock.Mock() conn = Connection(API_URL, refresh_token_store=refresh_token_store) assert isinstance(conn.auth, NullAuth) @@ -947,6 +979,8 @@ def test_authenticate_oidc_device_flow_multiple_provider_one_config_no_given(req assert isinstance(conn.auth, BearerAuth) assert conn.auth.bearer == 'oidc/fauth/' + oidc_mock.state["access_token"] assert refresh_token_store.mock_calls == [] + assert "No OIDC provider given, but only one in config (for backend 'https://oeo.test/'): 'fauth'. Using that one." in caplog.text + assert "Using client_id 'myclient' from config (provider 'fauth')" in caplog.text @pytest.mark.slow