From 004a3a029e30f64ac63770150942d439627f5d68 Mon Sep 17 00:00:00 2001 From: oysand Date: Wed, 5 Aug 2026 15:27:11 +0200 Subject: [PATCH] Support non-Azure OpenID providers --- README.md | 39 +++++- src/isar/apis/security/authentication.py | 45 ++++++- src/isar/config/settings.py | 12 ++ .../isar/apis/security/test_authentication.py | 122 ++++++++++++++++++ 4 files changed, 204 insertions(+), 14 deletions(-) diff --git a/README.md b/README.md index 0017d8160..5e91bebc0 100644 --- a/README.md +++ b/README.md @@ -270,22 +270,47 @@ option in the dictionary. ## API authentication -The API has an option to include user authentication. This can be enabled by setting the environment variable +The API validates OAuth2 access tokens. Authentication is controlled by ``` ISAR_AUTHENTICATION_ENABLED = true ``` -By default, the `local` storage module is used and API authentication is disabled. If using Azure Blob Storage a set of -environment variables must be available which gives access to an app registration that may use the storage account. -Enabling API authentication also requires the same environment variables. The required variables are +which is **enabled by default**; set it to `false` to turn authentication off. + +A token is accepted when its issuer matches the configured OpenID provider, its audience equals +`ISAR_AZURE_CLIENT_ID`, its signature and lifetime are valid, and it carries the role named by +`ISAR_REQUIRED_ROLE` (default `Mission.Control`) in a top-level `roles` claim. + +### Azure Entra ID (default) + +Requires an app registration, configured through + +``` +ISAR_AZURE_CLIENT_ID +ISAR_AZURE_TENANT_ID +``` + +The same app registration is used for Azure Blob Storage, which additionally needs the bare +`AZURE_CLIENT_ID`, `AZURE_TENANT_ID` and `AZURE_CLIENT_SECRET` variables that +`EnvironmentCredential` reads. + +### Any other OpenID Connect provider + +`ISAR_OPENID_CONFIG_URL` points ISAR at a different provider, such as a Keycloak realm: ``` -AZURE_CLIENT_ID -AZURE_TENANT_ID -AZURE_CLIENT_SECRET +ISAR_OPENID_CONFIG_URL = http://localhost:8080/realms/robotics/.well-known/openid-configuration +ISAR_AZURE_CLIENT_ID = isar-test # the expected audience +ISAR_OPENID_SCOPE = isar-api # the scope Swagger requests +ISAR_OPENAPI_AUTHORIZATION_URL = http://localhost:8080/realms/robotics/protocol/openid-connect/auth +ISAR_OPENAPI_TOKEN_URL = http://localhost:8080/realms/robotics/protocol/openid-connect/token ``` +The last three variables only affect Swagger's "Authorize" button. The provider must emit `nbf` +and a `ver` claim of `"1.0"` or `"2.0"`, place roles in a flat top-level `roles` array, and +issue `aud` as a single string rather than the array form RFC 7519 also permits. + ## MQTT communication ISAR is able to publish parts of its internal state to topics on an MQTT broker whenever they change. diff --git a/src/isar/apis/security/authentication.py b/src/isar/apis/security/authentication.py index 673e9ef59..e13ba50c9 100644 --- a/src/isar/apis/security/authentication.py +++ b/src/isar/apis/security/authentication.py @@ -5,6 +5,7 @@ from fastapi import Depends from fastapi.security.base import SecurityBase from fastapi_azure_auth import SingleTenantAzureAuthorizationCodeBearer +from fastapi_azure_auth.auth import AzureAuthorizationCodeBearerBase from fastapi_azure_auth.exceptions import InvalidAuthHttp from fastapi_azure_auth.user import User from pydantic import BaseModel @@ -22,13 +23,43 @@ def __init__(self) -> None: self.scheme_name = "No Security" -azure_scheme = SingleTenantAzureAuthorizationCodeBearer( - app_client_id=settings.AZURE_CLIENT_ID, - tenant_id=settings.AZURE_TENANT_ID, - scopes={ - f"api://{settings.AZURE_CLIENT_ID}/user_impersonation": "user_impersonation", - }, -) +def build_azure_scheme() -> AzureAuthorizationCodeBearerBase: + """ + Build the security scheme used to validate access tokens. + + Azure Entra ID by default, or the provider given by + ``settings.OPENID_CONFIG_URL``. The base class is used for the latter because + ``SingleTenantAzureAuthorizationCodeBearer`` does not accept an + ``openid_config_url``. Issuer validation stays enabled either way. + + Returns + ------- + AzureAuthorizationCodeBearerBase + The configured security scheme. + """ + scope_name: str = ( + settings.OPENID_SCOPE or f"api://{settings.AZURE_CLIENT_ID}/user_impersonation" + ) + scopes: dict[str, str] = {scope_name: scope_name.rsplit("/", maxsplit=1)[-1]} + + if settings.OPENID_CONFIG_URL: + return AzureAuthorizationCodeBearerBase( + app_client_id=settings.AZURE_CLIENT_ID, + tenant_id=settings.AZURE_TENANT_ID, + scopes=scopes, + openid_config_url=settings.OPENID_CONFIG_URL, + openapi_authorization_url=settings.OPENAPI_AUTHORIZATION_URL, + openapi_token_url=settings.OPENAPI_TOKEN_URL, + ) + + return SingleTenantAzureAuthorizationCodeBearer( + app_client_id=settings.AZURE_CLIENT_ID, + tenant_id=settings.AZURE_TENANT_ID, + scopes=scopes, + ) + + +azure_scheme: AzureAuthorizationCodeBearerBase = build_azure_scheme() async def validate_has_role(user: User = Depends(azure_scheme)) -> None: diff --git a/src/isar/config/settings.py b/src/isar/config/settings.py index 094c1aa8e..dd9d11210 100644 --- a/src/isar/config/settings.py +++ b/src/isar/config/settings.py @@ -122,6 +122,18 @@ class Settings(BaseSettings): # ChainedTokenCredential (e.g. "WorkloadIdentity,ClientSecret"). ALLOWED_AUTH_METHODS: str = Field(default="ClientSecret") + # OpenID Connect discovery document URL. Unset means Azure Entra ID, derived + # from AZURE_TENANT_ID. Set it to use another provider, such as Keycloak. + OPENID_CONFIG_URL: str | None = Field(default=None) + + # Swagger's "Authorize" button only. Validation is unaffected. + OPENAPI_AUTHORIZATION_URL: str | None = Field(default=None) + OPENAPI_TOKEN_URL: str | None = Field(default=None) + + # The scope Swagger requests. Entra derives the audience from the scope, hence + # the "api:///user_impersonation" default; other providers do not. + OPENID_SCOPE: str | None = Field(default=None) + # MQTT username # The username and password is set by the MQTT broker and must be known in advance # The password should be set as an environment variable "MQTT_PASSWORD" diff --git a/tests/isar/apis/security/test_authentication.py b/tests/isar/apis/security/test_authentication.py index f6cebd90e..36959bce0 100644 --- a/tests/isar/apis/security/test_authentication.py +++ b/tests/isar/apis/security/test_authentication.py @@ -3,6 +3,19 @@ import jwt import pytest from fastapi.testclient import TestClient +from fastapi_azure_auth import SingleTenantAzureAuthorizationCodeBearer +from fastapi_azure_auth.auth import AzureAuthorizationCodeBearerBase +from fastapi_azure_auth.user import User +from pydantic import ValidationError +from pytest import MonkeyPatch + +from isar.apis.security.authentication import build_azure_scheme +from isar.config.settings import settings + + +def advertised_scopes(scheme: AzureAuthorizationCodeBearerBase) -> dict[str, str]: + """Scopes offered by Swagger's Authorize button, for the given scheme.""" + return scheme.oauth.model.flows.authorizationCode.scopes def stub_access_token() -> str: @@ -30,3 +43,112 @@ def test_authentication( ) assert response.status_code == expected_status_code + + +class TestBuildAzureScheme: + def test_defaults_to_single_tenant_azure_scheme( + self, monkeypatch: MonkeyPatch + ) -> None: + monkeypatch.setattr(settings, "OPENID_CONFIG_URL", None) + + scheme = build_azure_scheme() + + assert isinstance(scheme, SingleTenantAzureAuthorizationCodeBearer) + assert scheme.openid_config.config_url is None + assert scheme.openid_config.tenant_id == settings.AZURE_TENANT_ID + assert scheme.app_client_id == settings.AZURE_CLIENT_ID + + def test_openid_config_url_is_honoured(self, monkeypatch: MonkeyPatch) -> None: + config_url = ( + "http://keycloak:8080/realms/robotics/.well-known/openid-configuration" + ) + authorization_url = ( + "http://keycloak:8080/realms/robotics/protocol/openid-connect/auth" + ) + token_url = "http://keycloak:8080/realms/robotics/protocol/openid-connect/token" + + monkeypatch.setattr(settings, "OPENID_CONFIG_URL", config_url) + monkeypatch.setattr(settings, "OPENAPI_AUTHORIZATION_URL", authorization_url) + monkeypatch.setattr(settings, "OPENAPI_TOKEN_URL", token_url) + + scheme = build_azure_scheme() + + assert not isinstance(scheme, SingleTenantAzureAuthorizationCodeBearer) + assert isinstance(scheme, AzureAuthorizationCodeBearerBase) + assert scheme.openid_config.config_url == config_url + assert scheme.authorization_url == authorization_url + assert scheme.token_url == token_url + assert scheme.app_client_id == settings.AZURE_CLIENT_ID + assert scheme.validate_iss is True + + def test_openapi_urls_fall_back_to_azure_when_unset( + self, monkeypatch: MonkeyPatch + ) -> None: + monkeypatch.setattr( + settings, + "OPENID_CONFIG_URL", + "http://keycloak:8080/realms/robotics/.well-known/openid-configuration", + ) + monkeypatch.setattr(settings, "OPENAPI_AUTHORIZATION_URL", None) + monkeypatch.setattr(settings, "OPENAPI_TOKEN_URL", None) + + scheme = build_azure_scheme() + + assert scheme.authorization_url is not None + assert settings.AZURE_TENANT_ID in scheme.authorization_url + + def test_scope_defaults_to_the_entra_shaped_scope( + self, monkeypatch: MonkeyPatch + ) -> None: + monkeypatch.setattr(settings, "OPENID_SCOPE", None) + + scheme = build_azure_scheme() + + expected = f"api://{settings.AZURE_CLIENT_ID}/user_impersonation" + assert advertised_scopes(scheme) == {expected: "user_impersonation"} + + def test_openid_scope_is_honoured(self, monkeypatch: MonkeyPatch) -> None: + monkeypatch.setattr(settings, "OPENID_SCOPE", "isar-api") + + scheme = build_azure_scheme() + + assert advertised_scopes(scheme) == {"isar-api": "isar-api"} + assert scheme.app_client_id == settings.AZURE_CLIENT_ID + + +class TestAudienceClaimShape: + """Pin the audience shapes ISAR accepts. + + ``fastapi_azure_auth`` declares ``aud`` as a plain ``str``, so the array form + RFC 7519 also permits is rejected with an opaque 401. Asserted here so that a + dependency upgrade lifting the restriction is noticed. + """ + + def test_string_audience_is_accepted(self) -> None: + user = User( + aud=settings.AZURE_CLIENT_ID, + claims={}, + access_token="", + iss="", + sub="", + exp=0, + iat=0, + nbf=0, + ver="2.0", + ) + + assert user.aud == settings.AZURE_CLIENT_ID + + def test_array_audience_is_rejected(self) -> None: + with pytest.raises(ValidationError): + User( + aud=[settings.AZURE_CLIENT_ID, "another-audience"], + claims={}, + access_token="", + iss="", + sub="", + exp=0, + iat=0, + nbf=0, + ver="2.0", + )