diff --git a/docs/reference/openapi.yaml b/docs/reference/openapi.yaml index d14664f57..94a1f1540 100644 --- a/docs/reference/openapi.yaml +++ b/docs/reference/openapi.yaml @@ -92,17 +92,25 @@ components: description: Client ID title: Client Id type: string + issuer: + anyOf: + - type: string + - type: 'null' + description: URL of OIDC provider + title: Issuer logout_redirect_endpoint: default: '' description: The oidc endpoint required to logout title: Logout Redirect Endpoint type: string well_known_url: + anyOf: + - type: string + - type: 'null' + deprecated: true description: URL to fetch OIDC config from the provider title: Well Known Url - type: string required: - - well_known_url - client_id title: OIDCConfig type: object @@ -433,7 +441,7 @@ info: name: Apache 2.0 url: https://www.apache.org/licenses/LICENSE-2.0.html title: BlueAPI Control - version: 1.3.0 + version: 1.4.0 openapi: 3.1.0 paths: /api/v1/devices: diff --git a/helm/blueapi/config_schema.json b/helm/blueapi/config_schema.json index b5d0c9bf3..b6c112a87 100644 --- a/helm/blueapi/config_schema.json +++ b/helm/blueapi/config_schema.json @@ -300,9 +300,31 @@ "additionalProperties": false, "properties": { "well_known_url": { + "anyOf": [ + { + "type": "string" + }, + { + "type": "null" + } + ], + "default": null, + "deprecated": true, "description": "URL to fetch OIDC config from the provider", - "title": "Well Known Url", - "type": "string" + "title": "Well Known Url" + }, + "issuer": { + "anyOf": [ + { + "type": "string" + }, + { + "type": "null" + } + ], + "default": null, + "description": "URL of OIDC provider", + "title": "Issuer" }, "client_id": { "description": "Client ID", @@ -323,7 +345,6 @@ } }, "required": [ - "well_known_url", "client_id" ], "title": "OIDCConfig", diff --git a/helm/blueapi/values.schema.json b/helm/blueapi/values.schema.json index 639209dd9..602e7ebef 100644 --- a/helm/blueapi/values.schema.json +++ b/helm/blueapi/values.schema.json @@ -736,7 +736,6 @@ "title": "OIDCConfig", "type": "object", "required": [ - "well_known_url", "client_id" ], "properties": { @@ -751,6 +750,18 @@ "description": "Client ID", "type": "string" }, + "issuer": { + "title": "Issuer", + "description": "URL of OIDC provider", + "anyOf": [ + { + "type": "string" + }, + { + "type": "null" + } + ] + }, "logout_redirect_endpoint": { "title": "Logout Redirect Endpoint", "description": "The oidc endpoint required to logout", @@ -760,7 +771,14 @@ "well_known_url": { "title": "Well Known Url", "description": "URL to fetch OIDC config from the provider", - "type": "string" + "anyOf": [ + { + "type": "string" + }, + { + "type": "null" + } + ] } }, "additionalProperties": false diff --git a/src/blueapi/config.py b/src/blueapi/config.py index 83d6d7021..1441e9d3b 100644 --- a/src/blueapi/config.py +++ b/src/blueapi/config.py @@ -1,3 +1,4 @@ +import logging import os import re import textwrap @@ -6,7 +7,7 @@ from functools import cached_property from pathlib import Path from string import Template -from typing import Annotated, Any, ClassVar, Generic, Literal, TypeVar, cast +from typing import Annotated, Any, ClassVar, Generic, Literal, Self, TypeVar, cast import requests import yaml @@ -22,11 +23,14 @@ UrlConstraints, ValidationError, field_validator, + model_validator, ) from pydantic.json_schema import SkipJsonSchema from blueapi.utils import BlueapiBaseModel, InvalidConfigError +LOGGER = logging.getLogger(__name__) + LogLevel = Literal["NOTSET", "DEBUG", "INFO", "WARNING", "ERROR", "CRITICAL"] FORBIDDEN_OWN_REMOTE_URL = "https://github.com/DiamondLightSource/blueapi.git" @@ -234,18 +238,47 @@ class ScratchConfig(BlueapiBaseModel): class OIDCConfig(BlueapiBaseModel): - well_known_url: str = Field( - description="URL to fetch OIDC config from the provider" + well_known_url: str | None = Field( + description="URL to fetch OIDC config from the provider", + deprecated=True, + default=None, ) + issuer: str | None = Field(description="URL of OIDC provider", default=None) client_id: str = Field(description="Client ID") client_audience: str = Field(description="Client Audience(s)", default="blueapi") logout_redirect_endpoint: str = Field( description="The oidc endpoint required to logout", default="" ) + @model_validator(mode="after") + def check_well_know_urls(self) -> Self: + if self.issuer is None and self.well_known_url is None: + raise ValueError("Please provide 'OIDCConfig.issuer'") + if self.well_known_url: + LOGGER.warning( + DeprecationWarning( + "OIDCConfig.well_known_url is deprecated, " + "Please use OIDCConfig.issuer" + ), + ) + return self + + @cached_property + def _well_known_url(self) -> str: + if self.issuer: + if self.well_known_url: + LOGGER.warning( + DeprecationWarning( + "well_known_url and issuer both are set. " + "Defaulting to issuer URL" + ), + ) + return self.issuer + "/.well-known/openid-configuration" + return cast(str, self.well_known_url) + @cached_property def _config_from_oidc_url(self) -> dict[str, Any]: - response: requests.Response = requests.get(self.well_known_url) + response = requests.get(self._well_known_url) response.raise_for_status() return response.json() @@ -259,10 +292,6 @@ def device_authorization_endpoint(self) -> str: def token_endpoint(self) -> str: return cast(str, self._config_from_oidc_url.get("token_endpoint")) - @cached_property - def issuer(self) -> str: - return cast(str, self._config_from_oidc_url.get("issuer")) - @cached_property def authorization_endpoint(self) -> str: return cast(str, self._config_from_oidc_url.get("authorization_endpoint")) @@ -303,7 +332,7 @@ class ApplicationConfig(BlueapiBaseModel): """ #: API version to publish in OpenAPI schema - REST_API_VERSION: ClassVar[str] = "1.3.0" + REST_API_VERSION: ClassVar[str] = "1.4.0" LICENSE_INFO: ClassVar[dict[str, str]] = { "name": "Apache 2.0", diff --git a/tests/conftest.py b/tests/conftest.py index 0203a0725..78af27578 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -54,18 +54,12 @@ def exporter() -> JsonObjectSpanExporter: return exporter -@pytest.fixture -def oidc_url() -> str: - return ( - "https://auth.example.com/realms/master/oidc/.well-known/openid-configuration" - ) +ISSUER = "https://auth.example.com/realms/master" @pytest.fixture -def oidc_config(oidc_url: str) -> OIDCConfig: - return OIDCConfig( - well_known_url=oidc_url, client_id="blueapi-cli", client_audience="blueapi" - ) +def oidc_config() -> OIDCConfig: + return OIDCConfig(issuer=ISSUER, client_id="blueapi-cli", client_audience="blueapi") CACHE_FILE = "blueapi_cache" @@ -86,7 +80,7 @@ def oidc_well_known() -> dict[str, Any]: "device_authorization_endpoint": "https://example.com/device_authorization", "authorization_endpoint": "https://example.com/authorization", "token_endpoint": "https://example.com/token", - "issuer": "https://example.com", + "issuer": ISSUER, "jwks_uri": "https://example.com/realms/master/protocol/openid-connect/certs", "end_session_endpoint": "https://example.com/end_session", "id_token_signing_alg_values_supported": ["RS256"], @@ -110,6 +104,7 @@ def _make_token( rsa_private_key: str, jwt_access_token: bool = False, valid_audience: bool = True, + issuer: str = ISSUER, ) -> dict[str, str]: now = time.time() @@ -117,7 +112,7 @@ def _make_token( "aud": "blueapi" if valid_audience else "invalid_audience", "exp": now + expires_in, "iat": now + issued_in, - "iss": "https://example.com", + "iss": issuer, "sub": "jd1", "name": "Jane Doe", "fedid": "jd1", @@ -242,7 +237,6 @@ def device_code() -> str: @pytest.fixture def mock_authn_server( - oidc_url: str, oidc_well_known: dict[str, Any], oidc_config: OIDCConfig, valid_token: dict[str, Any], @@ -256,7 +250,9 @@ def mock_authn_server( json=oidc_config.model_dump(), ) # Fetch well-known OIDC flow URLs from server - requests_mock.get(oidc_url, json=oidc_well_known) + requests_mock.get( + ISSUER + "/.well-known/openid-configuration", json=oidc_well_known + ) # When device flow begins, return a device_code requests_mock.post( oidc_well_known["device_authorization_endpoint"], diff --git a/tests/system_tests/config.yaml b/tests/system_tests/config.yaml index 714933ccd..9a35e517c 100644 --- a/tests/system_tests/config.yaml +++ b/tests/system_tests/config.yaml @@ -22,6 +22,6 @@ tiled: client_id: "tiled-writer" client_secret: "secret" oidc: - well_known_url: "http://localhost:8081/realms/master/.well-known/openid-configuration" + issuer: "http://localhost:8081/realms/master" client_id: "ixx-cli-blueapi" client_audience: "ixx-blueapi" diff --git a/tests/system_tests/test_blueapi_system.py b/tests/system_tests/test_blueapi_system.py index 4763cebfa..0d1e0f171 100644 --- a/tests/system_tests/test_blueapi_system.py +++ b/tests/system_tests/test_blueapi_system.py @@ -222,8 +222,8 @@ def test_cannot_access_endpoints( def test_can_get_oidc_config_without_auth(client_without_auth: BlueapiClient): - assert client_without_auth.oidc_config == OIDCConfig( - well_known_url="http://localhost:8081/realms/master/.well-known/openid-configuration", + assert client_without_auth.get_oidc_config() == OIDCConfig( + issuer="http://localhost:8081/realms/master", client_id="ixx-cli-blueapi", client_audience="ixx-blueapi", ) diff --git a/tests/unit_tests/cli/test_cli.py b/tests/unit_tests/cli/test_cli.py index 687b3a03f..e8ddd911e 100644 --- a/tests/unit_tests/cli/test_cli.py +++ b/tests/unit_tests/cli/test_cli.py @@ -1098,7 +1098,7 @@ def test_login_success( result = runner.invoke(main, ["-c", config_with_auth, "login"]) assert ( "Logging in\n" - "Please login from this URL:- https://example.com/verify\n" + "Please login from this URL:- https://auth.example.com/realms/master/verify\n" "Logged in and cached new token\n" == result.output ) assert result.exit_code == 0 @@ -1137,7 +1137,7 @@ def test_login_when_cached_token_decode_fails( result = runner.invoke(main, ["-c", config_with_auth, "login"]) assert ( "Logging in\n" - "Please login from this URL:- https://example.com/verify\n" + "Please login from this URL:- https://auth.example.com/realms/master/verify\n" "Logged in and cached new token\n" in result.output ) assert result.exit_code == 0 diff --git a/tests/unit_tests/test_config.py b/tests/unit_tests/test_config.py index 30fe551c4..49a8103eb 100644 --- a/tests/unit_tests/test_config.py +++ b/tests/unit_tests/test_config.py @@ -322,7 +322,8 @@ def test_config_yaml_parsed(temp_yaml_config_file): }, "numtracker": None, "oidc": { - "well_known_url": "https://auth.example.com/realms/sample/.well-known/openid-configuration", + "well_known_url": None, + "issuer": "https://auth.example.com/realms/sample", "client_id": "blueapi-client", "client_audience": "aud", "logout_redirect_endpoint": "", @@ -377,7 +378,8 @@ def test_config_yaml_parsed(temp_yaml_config_file): }, "numtracker": None, "oidc": { - "well_known_url": "https://auth.example.com/realms/sample/.well-known/openid-configuration", + "well_known_url": None, + "issuer": "https://auth.example.com/realms/sample", "client_id": "blueapi-client", "client_audience": "aud", "logout_redirect_endpoint": "", @@ -446,7 +448,7 @@ def test_config_yaml_parsed_complete(temp_yaml_config_file: dict): "api": {"host": "0.0.0.0", "port": 8001, "protocol": "http"}, "numtracker": None, "oidc": { - "well_known_url": "https://auth.example.com/realms/sample/.well-known/openid-configuration", + "issuer": "https://auth.example.com/realms/sample", "client_id": "blueapi-client", "client_audience": "aud", }, @@ -498,7 +500,6 @@ def test_oauth_config_model_post_init( oidc_config.authorization_endpoint == oidc_well_known["authorization_endpoint"] ) assert oidc_config.token_endpoint == oidc_well_known["token_endpoint"] - assert oidc_config.issuer == oidc_well_known["issuer"] assert oidc_config.jwks_uri == oidc_well_known["jwks_uri"] assert oidc_config.end_session_endpoint == oidc_well_known["end_session_endpoint"] @@ -539,3 +540,19 @@ def test_config_schema_updated() -> None: f"ApplicationConfig model is out of date with schema at \ {CONFIG_SCHEMA_LOCATION}. You may need to run `blueapi config-schema -u`" ) + + +def test_oidc_config_validation_error(): + with pytest.raises(ValueError, match="Please provide 'OIDCConfig.issuer'"): + ApplicationConfig( + oidc=OIDCConfig(well_known_url=None, issuer=None, client_id="blueapi") + ) + + +def test_oidc_config_urls(): + # Test well_known_url is still supported + oidc = OIDCConfig(well_known_url="url", issuer=None, client_id="blueapi") + assert oidc._well_known_url == "url" + # Test if both well_known_url and issuer are set it defaults to issuer + oidc = OIDCConfig(well_known_url="url", issuer="issuer_url", client_id="blueapi") + assert oidc.issuer == "issuer_url" diff --git a/tests/unit_tests/test_helm_chart.py b/tests/unit_tests/test_helm_chart.py index 43b8b5769..79ef617c7 100644 --- a/tests/unit_tests/test_helm_chart.py +++ b/tests/unit_tests/test_helm_chart.py @@ -69,7 +69,7 @@ ), logging=LoggingConfig(level="CRITICAL"), oidc=OIDCConfig( - well_known_url="foo.bar", + issuer="foo.bar", client_id="blueapi2", client_audience="blueapi++", ),