Skip to content

Commit dd3f40d

Browse files
authored
feat: Validate tiled ServiceAccount config at startup (#1548)
Ensure that the tiled auth configuration is valid and fail fast instead of when the first data is written.
1 parent b2d8ee9 commit dd3f40d

7 files changed

Lines changed: 137 additions & 4 deletions

File tree

helm/blueapi/config_schema.json

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -345,8 +345,15 @@
345345
"default": "account",
346346
"title": "Audience",
347347
"type": "string"
348+
},
349+
"tiled_service_account_check": {
350+
"title": "Tiled Service Account Check",
351+
"type": "string"
348352
}
349353
},
354+
"required": [
355+
"tiled_service_account_check"
356+
],
350357
"title": "OpaConfig",
351358
"type": "object",
352359
"$id": "OpaConfig"

helm/blueapi/values.schema.json

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -755,6 +755,9 @@
755755
"$id": "OpaConfig",
756756
"title": "OpaConfig",
757757
"type": "object",
758+
"required": [
759+
"tiled_service_account_check"
760+
],
758761
"properties": {
759762
"audience": {
760763
"title": "Audience",
@@ -768,6 +771,10 @@
768771
"format": "uri",
769772
"maxLength": 2083,
770773
"minLength": 1
774+
},
775+
"tiled_service_account_check": {
776+
"title": "Tiled Service Account Check",
777+
"type": "string"
771778
}
772779
},
773780
"additionalProperties": false

src/blueapi/config.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -299,6 +299,7 @@ class Tag(StrEnum):
299299
class OpaConfig(BlueapiBaseModel):
300300
root: HttpUrl = HttpUrl("http://localhost:8181")
301301
audience: str = "account"
302+
tiled_service_account_check: str
302303

303304

304305
class ApplicationConfig(BlueapiBaseModel):

src/blueapi/service/authorization.py

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,8 @@
55

66
from aiohttp import ClientSession
77

8-
from blueapi.config import OpaConfig
8+
from blueapi.config import OIDCConfig, OpaConfig, ServiceAccount
9+
from blueapi.service.authentication import TiledAuth
910

1011
LOGGER = logging.getLogger(__name__)
1112

@@ -45,3 +46,29 @@ def for_config(
4546
return aclosing(cls(instrument, config))
4647
LOGGER.info("No OPA config provided - not creating OpaClient")
4748
return nullcontext()
49+
50+
async def require_tiled_service_account(self, token: str):
51+
if not await self._call_opa(
52+
self._config.tiled_service_account_check,
53+
{"token": token, "beamline": self._instrument},
54+
):
55+
raise ValueError(
56+
f"Tiled service account is not valid for '{self._instrument}'"
57+
)
58+
59+
60+
async def validate_tiled_config(
61+
tiled: ServiceAccount | str | None, oidc: OIDCConfig | None, opa: OpaClient | None
62+
):
63+
if not isinstance(tiled, ServiceAccount):
64+
# can't validate an API key
65+
return
66+
67+
if not opa or not oidc:
68+
LOGGER.info("Missing OPA or OIDC configuration required to validate tiled auth")
69+
return
70+
71+
LOGGER.info("Validating tiled configuration")
72+
tiled.token_url = oidc.token_endpoint
73+
auth = TiledAuth(tiled)
74+
await opa.require_tiled_service_account(auth.get_access_token())

src/blueapi/service/main.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@
4040
from blueapi.worker import TrackableTask, WorkerState
4141
from blueapi.worker.event import TaskStatusEnum
4242

43-
from .authorization import OpaClient
43+
from .authorization import OpaClient, validate_tiled_config
4444
from .model import (
4545
DeviceModel,
4646
DeviceResponse,
@@ -98,6 +98,7 @@ async def inner(app: FastAPI):
9898
setup_runner(config)
9999
async with OpaClient.for_config(meta and meta.instrument, config.opa) as opa:
100100
app.state.authz = opa
101+
await validate_tiled_config(config.tiled.authentication, config.oidc, opa)
101102
yield
102103
teardown_runner()
103104

tests/unit_tests/service/test_authorization.py

Lines changed: 91 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,13 @@
1-
from unittest.mock import AsyncMock, MagicMock, patch
1+
from contextlib import AbstractContextManager, nullcontext
2+
from unittest.mock import AsyncMock, MagicMock, Mock, patch
23

34
import pytest
45
from pydantic import HttpUrl
56

6-
from blueapi.config import OpaConfig
7+
from blueapi.config import OIDCConfig, OpaConfig, ServiceAccount
78
from blueapi.service.authorization import (
89
OpaClient,
10+
validate_tiled_config,
911
)
1012

1113
# Reusable client patch decorator
@@ -20,9 +22,50 @@
2022
def opa_config() -> OpaConfig:
2123
return OpaConfig(
2224
root=HttpUrl("http://auth.example.com"),
25+
tiled_service_account_check="/auth/tiled",
2326
)
2427

2528

29+
@patch_client_session
30+
@pytest.mark.parametrize(
31+
"result,context",
32+
[
33+
(False, pytest.raises(ValueError, match="Tiled service account is not valid ")),
34+
(True, nullcontext()),
35+
],
36+
)
37+
async def test_tiled_service_account(
38+
session: MagicMock,
39+
opa_config: OpaConfig,
40+
result: bool,
41+
context: AbstractContextManager,
42+
):
43+
session.return_value.post = AsyncMock(
44+
return_value=MagicMock(json=AsyncMock(return_value={"result": result}))
45+
)
46+
47+
client = OpaClient(instrument="p99", config=opa_config)
48+
49+
session.assert_called_once_with(base_url="http://auth.example.com/")
50+
with context:
51+
await client.require_tiled_service_account(token="foo_bar")
52+
session().post.assert_called_once_with(
53+
"/auth/tiled",
54+
json={"input": {"token": "foo_bar", "beamline": "p99", "audience": "account"}},
55+
)
56+
57+
58+
@patch_client_session
59+
async def test_exception_raised_when_opa_fails(
60+
session: MagicMock, opa_config: OpaConfig
61+
):
62+
session.return_value.post = AsyncMock(side_effect=RuntimeError("Connection failed"))
63+
async with OpaClient.for_config("p45", opa_config) as client:
64+
assert client is not None
65+
with pytest.raises(RuntimeError, match="Connection failed"):
66+
await client.require_tiled_service_account(token="foo_bar")
67+
68+
2669
@patch_client_session
2770
async def test_session_closed(session: MagicMock, opa_config: OpaConfig):
2871
async with OpaClient.for_config("p45", opa_config):
@@ -60,3 +103,49 @@ async def test_opa_adds_input_fields(session: MagicMock, opa_config: OpaConfig):
60103
"foo/bar",
61104
json={"input": {"beamline": "p45", "audience": "account", "foo": "bar"}},
62105
)
106+
107+
108+
async def test_validate_tiled_config():
109+
opa = MagicMock(spec=OpaClient)
110+
tiled = ServiceAccount()
111+
oidc = Mock(spec=OIDCConfig)
112+
oidc.token_endpoint = "token-endpoint"
113+
with patch("blueapi.service.authorization.TiledAuth") as auth:
114+
auth.return_value.get_access_token.return_value = "tiled-token"
115+
await validate_tiled_config(tiled, oidc, opa)
116+
117+
auth.assert_called_once_with(tiled)
118+
opa.require_tiled_service_account.assert_called_once_with("tiled-token")
119+
120+
121+
@pytest.mark.parametrize(
122+
"tiled_auth,oidc,opa_client",
123+
[
124+
(None, None, MagicMock(spec=OpaClient)),
125+
(
126+
None,
127+
OIDCConfig(well_known_url="http://example.com", client_id="test-client"),
128+
MagicMock(spec=OpaClient),
129+
),
130+
("api_key", None, MagicMock(spec=OpaClient)),
131+
(
132+
"api_key",
133+
OIDCConfig(well_known_url="http://example.com", client_id="test-client"),
134+
MagicMock(spec=OpaClient),
135+
),
136+
(ServiceAccount(), None, MagicMock(spec=OpaClient)),
137+
(
138+
ServiceAccount(),
139+
OIDCConfig(well_known_url="http://example.com", client_id="test-client"),
140+
None,
141+
),
142+
],
143+
)
144+
async def test_validate_tiled_config_with_missing_config(
145+
tiled_auth: ServiceAccount | str | None,
146+
oidc: OIDCConfig | None,
147+
opa_client: MagicMock | None,
148+
):
149+
assert await validate_tiled_config(tiled_auth, oidc, opa_client) is None
150+
if opa_client is not None:
151+
opa_client.require_tiled_service_account.assert_not_called()

tests/unit_tests/test_config.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -340,6 +340,7 @@ def test_config_yaml_parsed(temp_yaml_config_file):
340340
"opa": {
341341
"root": "http://opa.example.com/",
342342
"audience": "account",
343+
"tiled_service_account_check": "v1/tiled_service_account",
343344
},
344345
},
345346
{

0 commit comments

Comments
 (0)