-
Notifications
You must be signed in to change notification settings - Fork 14
feat: Token exchange for tiled insertion #1342
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 12 commits
c1b373e
82337c5
e4ac3a6
92157dc
11f8d8c
b12204c
8780c9d
c8f22ff
dd175d6
9cd9ffe
41b614d
5479b4a
cabb737
20d46df
827cf75
a0cf64c
7d834e1
f206777
efb823a
fdf60ad
181c503
c399996
f87bedd
fa0155b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| tests/system_tests/services/blueapi-oauth2-proxy/oauth2-proxy.cfg:generic-api-key:16 | ||
| tests/system_tests/services/tiled-oauth2-proxy/oauth2-proxy.cfg:generic-api-key:14 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -112,7 +112,13 @@ class TiledConfig(BlueapiBaseModel): | |
| default=False, | ||
| ) | ||
| url: HttpUrl = HttpUrl("http://localhost:8407") | ||
| api_key: str | None = os.environ.get("TILED_SINGLE_USER_API_KEY", None) | ||
| token_exchange_secret: str = Field( | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are these three fields always going to be all present or all absent? Could we make it into a nested
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There are 2 fields client id and client secret .The token_url is set from the config.oidc Can this remain in the same level ? |
||
| description="Token exchange client secret", default="" | ||
| ) | ||
| token_url: str = Field(default="") | ||
| token_exchange_client_id: str = Field( | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is the client_id of the tiled service? The keycloak docs use the terms
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In this case the |
||
| description="Token exchange Client ID", default="" | ||
| ) | ||
|
|
||
|
|
||
| class WorkerEventConfig(BlueapiBaseModel): | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -181,6 +181,17 @@ def _update_scan_num(md: dict[str, Any]) -> int: | |
| "Tiled has been configured but `instrument` metadata is not set - " | ||
| "this field is required to make authorization decisions." | ||
| ) | ||
| if configuration.oidc is None: | ||
| raise InvalidConfigError( | ||
| "Tiled has been configured but oidc configuration is missing " | ||
| "this field is required to make authorization decisions." | ||
| ) | ||
| if tiled_conf.token_exchange_secret == "": | ||
| raise InvalidConfigError( | ||
| "Tiled has been enabled but Token exchange secret has not been set " | ||
| "this field is required to enable tiled insertion." | ||
| ) | ||
| tiled_conf.token_url = configuration.oidc.token_endpoint | ||
|
Comment on lines
+184
to
+194
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does this mean we can't run blueapi against unauthenticated local tiled instances for testing?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. According to my understanding of Tiled, The only way tiled supports unauthenticated is allow_anonymous_access which still also requires a API key to write to tiled. Do you want me to support 2 ways to insert data into Tiled If you go to http://localhost:4181/ you will get the authenticated tiled user interface behind oauth2. |
||
| self.tiled_conf = tiled_conf | ||
|
|
||
| def find_device(self, addr: str | list[str]) -> Device | None: | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| CONTEXT_HEADER = "traceparent" | ||
| VENDOR_CONTEXT_HEADER = "tracestate" | ||
| AUTHORIZAITON_HEADER = "authorization" | ||
|
ZohebShaikh marked this conversation as resolved.
Outdated
|
||
| PROPAGATED_HEADERS = {CONTEXT_HEADER, VENDOR_CONTEXT_HEADER, AUTHORIZAITON_HEADER} | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -12,6 +12,8 @@ | |||||||||||||||||||||||
| from blueapi.core.context import BlueskyContext | ||||||||||||||||||||||||
| from blueapi.core.event import EventStream | ||||||||||||||||||||||||
| from blueapi.log import set_up_logging | ||||||||||||||||||||||||
| from blueapi.service.authentication import TiledAuth | ||||||||||||||||||||||||
| from blueapi.service.constants import AUTHORIZAITON_HEADER | ||||||||||||||||||||||||
| from blueapi.service.model import ( | ||||||||||||||||||||||||
| DeviceModel, | ||||||||||||||||||||||||
| PlanModel, | ||||||||||||||||||||||||
|
|
@@ -184,10 +186,27 @@ def begin_task( | |||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| if tiled_config := active_context.tiled_conf: | ||||||||||||||||||||||||
| # Tiled queries the root node, so must create an authorized client | ||||||||||||||||||||||||
| blueapi_jwt_token = "" | ||||||||||||||||||||||||
| if pass_through_headers is None: | ||||||||||||||||||||||||
| raise ValueError( | ||||||||||||||||||||||||
| "Tiled config is enabled but no " | ||||||||||||||||||||||||
| f"{AUTHORIZAITON_HEADER} header in request" | ||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||
| authorization_header_value = pass_through_headers.get(AUTHORIZAITON_HEADER) | ||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This doesn't check that the pass through headers contains the auth header. Might as well try and get it and fail if it's not there. Might need to default pass_through_headers if it's None (or make it required?) - numtracker is already needing to do it above. It would also be good if we could still run blueapi in testing using an unauthenticated instance of tiled. Could we make auth optional if it's not configured then make the error useful if we get a 401 back from tiled?
Suggested change
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Below this line I check if there is a token in the Headers |
||||||||||||||||||||||||
| from fastapi.security.utils import get_authorization_scheme_param | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| _, blueapi_jwt_token = get_authorization_scheme_param( | ||||||||||||||||||||||||
| authorization_header_value | ||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| if blueapi_jwt_token == "": | ||||||||||||||||||||||||
| raise KeyError("Tiled config is enabled but no Bearer Token in request") | ||||||||||||||||||||||||
| tiled_client = from_uri( | ||||||||||||||||||||||||
| str(tiled_config.url), | ||||||||||||||||||||||||
| api_key=tiled_config.api_key, | ||||||||||||||||||||||||
| headers=pass_through_headers, | ||||||||||||||||||||||||
| auth=TiledAuth( | ||||||||||||||||||||||||
| tiled_config, | ||||||||||||||||||||||||
| blueapi_jwt_token=blueapi_jwt_token, | ||||||||||||||||||||||||
| ), | ||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||
| tiled_writer_token = active_context.run_engine.subscribe( | ||||||||||||||||||||||||
| TiledWriter(tiled_client, batch_size=1) | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
Uh oh!
There was an error while loading. Please reload this page.