Skip to content

Commit ca126df

Browse files
committed
Support non-Azure OpenID providers
ISAR validates access tokens against Azure Entra ID, with the discovery document URL derived from AZURE_TENANT_ID. Add ISAR_OPENID_CONFIG_URL to read it from anywhere else instead, so ISAR can be pointed at a Keycloak realm for local development, for the armada integration tests, or for a deployment outside Azure. SingleTenantAzureAuthorizationCodeBearer does not accept an openid_config_url argument, so the base class is used directly when the override is set. Issuer validation stays enabled either way; the expected issuer is taken from the discovery document. The scheme is also extracted into a factory, since a module-level constant built at import time cannot be tested without reloading the module. ISAR_OPENID_SCOPE names the scope Swagger requests. Entra derives a token's audience from the requested scope, which is why the scope was built as "api://<client id>/user_impersonation". Other providers decouple the two: a Keycloak scope named "isar-api" yields the audience "isar-test". Validation is unaffected -- the expected audience is still AZURE_CLIENT_ID. ISAR_OPENAPI_AUTHORIZATION_URL and ISAR_OPENAPI_TOKEN_URL likewise keep Swagger's Authorize button off login.microsoftonline.com. Also pin the audience shapes ISAR accepts. fastapi-azure-auth declares "aud" as a plain string, so the array form permitted by RFC 7519 is rejected with an opaque 401 after signature validation succeeds, and there is no injection point for a different user model. Entra never emits the array form; other providers can. The constraint is handled where tokens are minted, and asserted here so that a dependency upgrade lifting it is noticed rather than silently relied upon. Document all of this in the README, and correct its claim that authentication is disabled by default. It has defaulted to enabled since the setting was introduced.
1 parent d24ead6 commit ca126df

4 files changed

Lines changed: 246 additions & 14 deletions

File tree

README.md

Lines changed: 36 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -270,22 +270,51 @@ option in the dictionary.
270270

271271
## API authentication
272272

273-
The API has an option to include user authentication. This can be enabled by setting the environment variable
273+
The API validates OAuth2 access tokens. Authentication is controlled by
274274

275275
```
276276
ISAR_AUTHENTICATION_ENABLED = true
277277
```
278278

279-
By default, the `local` storage module is used and API authentication is disabled. If using Azure Blob Storage a set of
280-
environment variables must be available which gives access to an app registration that may use the storage account.
281-
Enabling API authentication also requires the same environment variables. The required variables are
279+
which is **enabled by default**; set it to `false` to turn authentication off.
280+
281+
A token is accepted when its issuer matches the configured OpenID provider, its audience equals
282+
`ISAR_AZURE_CLIENT_ID`, its signature and lifetime are valid, and it carries the role named by
283+
`ISAR_REQUIRED_ROLE` (default `Mission.Control`) in a top-level `roles` claim.
284+
285+
### Azure Entra ID (default)
286+
287+
Requires an app registration, configured through
282288

283289
```
284-
AZURE_CLIENT_ID
285-
AZURE_TENANT_ID
286-
AZURE_CLIENT_SECRET
290+
ISAR_AZURE_CLIENT_ID
291+
ISAR_AZURE_TENANT_ID
287292
```
288293

294+
The same app registration is used for Azure Blob Storage, which additionally needs the bare
295+
`AZURE_CLIENT_ID`, `AZURE_TENANT_ID` and `AZURE_CLIENT_SECRET` variables that
296+
`EnvironmentCredential` reads.
297+
298+
### Any other OpenID Connect provider
299+
300+
Setting `ISAR_OPENID_CONFIG_URL` points ISAR at a different provider — for example a Keycloak
301+
realm, whether for local development or for a deployment outside Azure:
302+
303+
```
304+
ISAR_OPENID_CONFIG_URL = http://localhost:8080/realms/robotics/.well-known/openid-configuration
305+
ISAR_AZURE_CLIENT_ID = isar-test # the expected audience
306+
ISAR_OPENID_SCOPE = isar-api # the scope Swagger requests
307+
ISAR_OPENAPI_AUTHORIZATION_URL = http://localhost:8080/realms/robotics/protocol/openid-connect/auth
308+
ISAR_OPENAPI_TOKEN_URL = http://localhost:8080/realms/robotics/protocol/openid-connect/token
309+
```
310+
311+
Issuer validation stays enabled; the expected issuer is read from the discovery document. The
312+
last three variables only affect Swagger's "Authorize" button.
313+
314+
The provider must emit `nbf` and a `ver` claim of `"1.0"` or `"2.0"`, place roles in a flat
315+
top-level `roles` array, and issue `aud` as a **single string** rather than the array form that
316+
RFC 7519 also permits.
317+
289318
## MQTT communication
290319

291320
ISAR is able to publish parts of its internal state to topics on an MQTT broker whenever they change.

src/isar/apis/security/authentication.py

Lines changed: 50 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
from fastapi import Depends
55
from fastapi.security.base import SecurityBase
66
from fastapi_azure_auth import SingleTenantAzureAuthorizationCodeBearer
7+
from fastapi_azure_auth.auth import AzureAuthorizationCodeBearerBase
78
from fastapi_azure_auth.exceptions import InvalidAuthHttp
89
from fastapi_azure_auth.user import User
910
from pydantic import BaseModel
@@ -21,13 +22,55 @@ def __init__(self) -> None:
2122
self.scheme_name = "No Security"
2223

2324

24-
azure_scheme = SingleTenantAzureAuthorizationCodeBearer(
25-
app_client_id=settings.AZURE_CLIENT_ID,
26-
tenant_id=settings.AZURE_TENANT_ID,
27-
scopes={
28-
f"api://{settings.AZURE_CLIENT_ID}/user_impersonation": "user_impersonation",
29-
},
30-
)
25+
def build_azure_scheme() -> AzureAuthorizationCodeBearerBase:
26+
"""
27+
Build the security scheme used to validate access tokens.
28+
29+
By default this is a single tenant Azure Entra ID scheme. If
30+
``settings.OPENID_CONFIG_URL`` is set, the OpenID Connect discovery document is
31+
read from that URL instead, which allows ISAR to be pointed at a different
32+
OpenID provider, such as a Keycloak realm used for local development and by the
33+
integration tests.
34+
35+
``SingleTenantAzureAuthorizationCodeBearer`` does not accept an
36+
``openid_config_url`` argument, so the base class is used directly in that case.
37+
Issuer validation remains enabled either way; the expected issuer is taken from
38+
the discovery document.
39+
40+
The expected audience is ``settings.AZURE_CLIENT_ID`` regardless of provider. Note
41+
that the *scope* a caller requests need not resemble the audience it yields: Entra
42+
derives the audience from the scope, whereas an OpenID provider such as Keycloak
43+
maps a freely named scope onto an audience. ``settings.OPENID_SCOPE`` therefore
44+
overrides the scope advertised to Swagger without affecting validation.
45+
46+
Returns
47+
-------
48+
AzureAuthorizationCodeBearerBase
49+
The configured security scheme.
50+
"""
51+
scope_name: str = (
52+
settings.OPENID_SCOPE or f"api://{settings.AZURE_CLIENT_ID}/user_impersonation"
53+
)
54+
scopes: dict[str, str] = {scope_name: scope_name.rsplit("/", maxsplit=1)[-1]}
55+
56+
if settings.OPENID_CONFIG_URL:
57+
return AzureAuthorizationCodeBearerBase(
58+
app_client_id=settings.AZURE_CLIENT_ID,
59+
tenant_id=settings.AZURE_TENANT_ID,
60+
scopes=scopes,
61+
openid_config_url=settings.OPENID_CONFIG_URL,
62+
openapi_authorization_url=settings.OPENAPI_AUTHORIZATION_URL,
63+
openapi_token_url=settings.OPENAPI_TOKEN_URL,
64+
)
65+
66+
return SingleTenantAzureAuthorizationCodeBearer(
67+
app_client_id=settings.AZURE_CLIENT_ID,
68+
tenant_id=settings.AZURE_TENANT_ID,
69+
scopes=scopes,
70+
)
71+
72+
73+
azure_scheme: AzureAuthorizationCodeBearerBase = build_azure_scheme()
3174

3275

3376
async def validate_has_role(user: User = Depends(azure_scheme)) -> None:

src/isar/config/settings.py

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,28 @@ class Settings(BaseSettings):
122122
# ChainedTokenCredential (e.g. "WorkloadIdentity,ClientSecret").
123123
ALLOWED_AUTH_METHODS: str = Field(default="ClientSecret")
124124

125+
# Override the OpenID Connect discovery document URL used to validate access
126+
# tokens. When unset (the default), the URL is derived from AZURE_TENANT_ID and
127+
# points at Azure Entra ID. Set this to point ISAR at a different OpenID
128+
# provider, such as the Keycloak realm used for local development and by the
129+
# integration tests.
130+
OPENID_CONFIG_URL: str | None = Field(default=None)
131+
132+
# Override the authorization and token URLs advertised in the generated OpenAPI
133+
# document, so that Swagger's "Authorize" button does not point at
134+
# login.microsoftonline.com when a non-Azure issuer is configured. Only affect
135+
# the API documentation; token validation is governed by OPENID_CONFIG_URL.
136+
OPENAPI_AUTHORIZATION_URL: str | None = Field(default=None)
137+
OPENAPI_TOKEN_URL: str | None = Field(default=None)
138+
139+
# Override the scope Swagger requests when authorizing. Entra derives a token's
140+
# audience from the requested scope, so the default is the Entra-shaped
141+
# "api://<client id>/user_impersonation". Other OpenID providers decouple the
142+
# two: a Keycloak scope named "isar-api" may yield the audience "isar-test".
143+
# Only affects the API documentation; the expected audience is always
144+
# AZURE_CLIENT_ID.
145+
OPENID_SCOPE: str | None = Field(default=None)
146+
125147
# MQTT username
126148
# The username and password is set by the MQTT broker and must be known in advance
127149
# The password should be set as an environment variable "MQTT_PASSWORD"

tests/isar/apis/security/test_authentication.py

Lines changed: 138 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,19 @@
33
import jwt
44
import pytest
55
from fastapi.testclient import TestClient
6+
from fastapi_azure_auth import SingleTenantAzureAuthorizationCodeBearer
7+
from fastapi_azure_auth.auth import AzureAuthorizationCodeBearerBase
8+
from fastapi_azure_auth.user import User
9+
from pydantic import ValidationError
10+
from pytest import MonkeyPatch
11+
12+
from isar.apis.security.authentication import build_azure_scheme
13+
from isar.config.settings import settings
14+
15+
16+
def advertised_scopes(scheme: AzureAuthorizationCodeBearerBase) -> dict[str, str]:
17+
"""Scopes offered by Swagger's Authorize button, for the given scheme."""
18+
return scheme.oauth.model.flows.authorizationCode.scopes
619

720

821
def stub_access_token() -> str:
@@ -30,3 +43,128 @@ def test_authentication(
3043
)
3144

3245
assert response.status_code == expected_status_code
46+
47+
48+
class TestBuildAzureScheme:
49+
def test_defaults_to_single_tenant_azure_scheme(
50+
self, monkeypatch: MonkeyPatch
51+
) -> None:
52+
monkeypatch.setattr(settings, "OPENID_CONFIG_URL", None)
53+
54+
scheme = build_azure_scheme()
55+
56+
assert isinstance(scheme, SingleTenantAzureAuthorizationCodeBearer)
57+
# No override, so the discovery document URL is derived from the tenant ID
58+
# and points at Azure Entra ID.
59+
assert scheme.openid_config.config_url is None
60+
assert scheme.openid_config.tenant_id == settings.AZURE_TENANT_ID
61+
assert scheme.app_client_id == settings.AZURE_CLIENT_ID
62+
63+
def test_openid_config_url_is_honoured(self, monkeypatch: MonkeyPatch) -> None:
64+
config_url = (
65+
"http://keycloak:8080/realms/robotics/.well-known/openid-configuration"
66+
)
67+
authorization_url = (
68+
"http://keycloak:8080/realms/robotics/protocol/openid-connect/auth"
69+
)
70+
token_url = "http://keycloak:8080/realms/robotics/protocol/openid-connect/token"
71+
72+
monkeypatch.setattr(settings, "OPENID_CONFIG_URL", config_url)
73+
monkeypatch.setattr(settings, "OPENAPI_AUTHORIZATION_URL", authorization_url)
74+
monkeypatch.setattr(settings, "OPENAPI_TOKEN_URL", token_url)
75+
76+
scheme = build_azure_scheme()
77+
78+
assert not isinstance(scheme, SingleTenantAzureAuthorizationCodeBearer)
79+
assert isinstance(scheme, AzureAuthorizationCodeBearerBase)
80+
assert scheme.openid_config.config_url == config_url
81+
assert scheme.authorization_url == authorization_url
82+
assert scheme.token_url == token_url
83+
# The audience is still ISAR's own client ID, and issuer validation stays on.
84+
assert scheme.app_client_id == settings.AZURE_CLIENT_ID
85+
assert scheme.validate_iss is True
86+
87+
def test_openapi_urls_fall_back_to_azure_when_unset(
88+
self, monkeypatch: MonkeyPatch
89+
) -> None:
90+
monkeypatch.setattr(
91+
settings,
92+
"OPENID_CONFIG_URL",
93+
"http://keycloak:8080/realms/robotics/.well-known/openid-configuration",
94+
)
95+
monkeypatch.setattr(settings, "OPENAPI_AUTHORIZATION_URL", None)
96+
monkeypatch.setattr(settings, "OPENAPI_TOKEN_URL", None)
97+
98+
scheme = build_azure_scheme()
99+
100+
assert scheme.authorization_url is not None
101+
assert settings.AZURE_TENANT_ID in scheme.authorization_url
102+
103+
def test_scope_defaults_to_the_entra_shaped_scope(
104+
self, monkeypatch: MonkeyPatch
105+
) -> None:
106+
monkeypatch.setattr(settings, "OPENID_SCOPE", None)
107+
108+
scheme = build_azure_scheme()
109+
110+
expected = f"api://{settings.AZURE_CLIENT_ID}/user_impersonation"
111+
assert advertised_scopes(scheme) == {expected: "user_impersonation"}
112+
113+
def test_openid_scope_is_honoured(self, monkeypatch: MonkeyPatch) -> None:
114+
# Entra derives a token's audience from the requested scope; Keycloak and
115+
# other OpenID providers do not, so the scope must be nameable independently
116+
# of AZURE_CLIENT_ID.
117+
monkeypatch.setattr(settings, "OPENID_SCOPE", "isar-api")
118+
119+
scheme = build_azure_scheme()
120+
121+
assert advertised_scopes(scheme) == {"isar-api": "isar-api"}
122+
# Validation is unaffected: the expected audience is still the client ID.
123+
assert scheme.app_client_id == settings.AZURE_CLIENT_ID
124+
125+
126+
class TestAudienceClaimShape:
127+
"""Pin the audience shapes ISAR accepts.
128+
129+
``fastapi_azure_auth.user.Claims`` declares ``aud`` as a plain ``str``, so a
130+
token carrying the array form permitted by RFC 7519 section 4.1.3 is rejected —
131+
after signature validation succeeds — with an opaque 401. There is no injection
132+
point for a different user model: ``AzureAuthorizationCodeBearerBase.__call__``
133+
constructs ``User`` directly.
134+
135+
Entra never emits the array form, so this has never mattered. Other providers
136+
can: Keycloak does so as soon as two audience mappers apply to one token. The
137+
constraint is therefore handled where tokens are minted -- exactly one audience
138+
mapper per client scope, and never two API scopes in a single token -- and
139+
asserted here so that a ``fastapi-azure-auth`` upgrade which lifts the
140+
restriction is noticed rather than silently relied upon.
141+
"""
142+
143+
def test_string_audience_is_accepted(self) -> None:
144+
user = User(
145+
aud=settings.AZURE_CLIENT_ID,
146+
claims={},
147+
access_token="",
148+
iss="",
149+
sub="",
150+
exp=0,
151+
iat=0,
152+
nbf=0,
153+
ver="2.0",
154+
)
155+
156+
assert user.aud == settings.AZURE_CLIENT_ID
157+
158+
def test_array_audience_is_rejected(self) -> None:
159+
with pytest.raises(ValidationError):
160+
User(
161+
aud=[settings.AZURE_CLIENT_ID, "another-audience"],
162+
claims={},
163+
access_token="",
164+
iss="",
165+
sub="",
166+
exp=0,
167+
iat=0,
168+
nbf=0,
169+
ver="2.0",
170+
)

0 commit comments

Comments
 (0)