Repository navigation
feat(temporal): add workload identity authentication #561
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 all commits
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,111 @@ | ||
| """Renewable Azure Workload Identity authentication for Temporal clients.""" | ||
|
|
||
| from __future__ import annotations | ||
|
|
||
| import os | ||
| import time | ||
| import asyncio | ||
| import logging | ||
| from typing import Any, Protocol | ||
| from dataclasses import field, dataclass | ||
| from collections.abc import Callable, Awaitable | ||
|
|
||
| from temporalio.client import Client | ||
| from azure.identity.aio import WorkloadIdentityCredential | ||
| from azure.core.credentials import AccessToken | ||
|
|
||
| logger = logging.getLogger(__name__) | ||
|
|
||
| _AUTH_SCOPE_ENV = "TEMPORAL_AUTH_SCOPE" | ||
| _REFRESH_MARGIN_SECONDS = 300 | ||
| _MIN_REFRESH_DELAY_SECONDS = 30 | ||
| _RETRY_DELAY_SECONDS = 30 | ||
|
|
||
|
|
||
| class AsyncTokenCredential(Protocol): | ||
| async def get_token(self, *scopes: str) -> AccessToken: ... | ||
|
|
||
| async def close(self) -> None: ... | ||
|
|
||
|
|
||
| @dataclass(eq=False) | ||
| class TemporalTokenSession: | ||
| """Keep a Temporal client's bearer token current for the process lifetime.""" | ||
|
|
||
| client: Client | ||
| credential: AsyncTokenCredential | ||
| scope: str | ||
| token: AccessToken | ||
| now: Callable[[], float] = time.time | ||
| sleep: Callable[[float], Awaitable[None]] = asyncio.sleep | ||
| _task: asyncio.Task[None] | None = field(default=None, init=False) | ||
|
|
||
| def start(self) -> None: | ||
| self._task = asyncio.create_task(self._run()) | ||
| _ACTIVE_SESSIONS.add(self) | ||
| self._task.add_done_callback(lambda _: _ACTIVE_SESSIONS.discard(self)) | ||
|
|
||
| async def refresh_once(self) -> bool: | ||
| """Replace the token only after a valid refresh succeeds.""" | ||
| try: | ||
| refreshed = await self.credential.get_token(self.scope) | ||
| if refreshed.expires_on <= self.now(): | ||
| raise ValueError("Azure returned an expired Temporal access token") | ||
| except Exception: | ||
| log = logger.error if self.token.expires_on <= self.now() else logger.warning | ||
| log("Temporal access token refresh failed; retaining the current token", exc_info=True) | ||
| return False | ||
|
|
||
| self.client.api_key = refreshed.token | ||
| self.token = refreshed | ||
| return True | ||
|
|
||
| async def _run(self) -> None: | ||
| try: | ||
| delay = max( | ||
| _MIN_REFRESH_DELAY_SECONDS, | ||
| self.token.expires_on - self.now() - _REFRESH_MARGIN_SECONDS, | ||
| ) | ||
| while True: | ||
| await self.sleep(delay) | ||
| delay = ( | ||
| max( | ||
| _MIN_REFRESH_DELAY_SECONDS, | ||
| self.token.expires_on - self.now() - _REFRESH_MARGIN_SECONDS, | ||
| ) | ||
| if await self.refresh_once() | ||
| else _RETRY_DELAY_SECONDS | ||
| ) | ||
| finally: | ||
| await self.credential.close() | ||
|
|
||
|
|
||
| _ACTIVE_SESSIONS: set[TemporalTokenSession] = set() | ||
|
|
||
|
|
||
| async def connect_temporal_client(connect_options: dict[str, Any]) -> Client: | ||
| """Connect anonymously by default, or with renewable Workload Identity.""" | ||
| scope = os.environ.get(_AUTH_SCOPE_ENV) | ||
| if not scope: | ||
| return await Client.connect(**connect_options) | ||
|
|
||
| credential = WorkloadIdentityCredential() | ||
| try: | ||
| token = await credential.get_token(scope) | ||
| if token.expires_on <= time.time(): | ||
| raise ValueError("Azure returned an expired Temporal access token") | ||
| authenticated_options = dict(connect_options) | ||
| if authenticated_options.get("tls") is None: | ||
| authenticated_options["tls"] = False | ||
| client = await Client.connect(api_key=token.token, **authenticated_options) | ||
| except Exception: | ||
| await credential.close() | ||
| raise | ||
|
Comment on lines
+101
to
+103
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.
Cancelling startup during Prompt To Fix With AIThis is a comment left during a code review.
Path: src/agentex/lib/core/clients/temporal/auth.py
Line: 101-103
Comment:
**Cancelled startup skips cleanup**
Cancelling startup during `credential.get_token` or `Client.connect` skips this cleanup because `asyncio.CancelledError` is not an `Exception`. No refresh session exists yet to close the credential, so startup timeouts can leave its HTTP resources open. Close the credential on cancellation as well, then re-raise it.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly. |
||
|
|
||
| TemporalTokenSession( | ||
| client=client, | ||
| credential=credential, | ||
| scope=scope, | ||
| token=token, | ||
| ).start() | ||
| return client | ||
|
Comment on lines
+105
to
+111
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. Each authenticated connection starts a task that holds its client and credential forever. Callers receive only the client and have no supported way to stop that task. Creating and discarding clients in the same event loop leaves every old session refreshing tokens and holding resources. Expose an async close operation and call it when the worker or application shuts down. Prompt To Fix With AIThis is a comment left during a code review.
Path: src/agentex/lib/core/clients/temporal/auth.py
Line: 105-111
Comment:
**Old clients keep refreshing**
Each authenticated connection starts a task that holds its client and credential forever. Callers receive only the client and have no supported way to stop that task. Creating and discarding clients in the same event loop leaves every old session refreshing tokens and holding resources. Expose an async close operation and call it when the worker or application shuts down.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time! |
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When
TEMPORAL_AUTH_SCOPEis set, both client factories omittls, so this branch sends the Azure bearer token withtls=False. If the network does not provide separate encryption, someone watching the traffic can copy and reuse the token until it expires. Default authenticated connections to TLS, and require an explicit choice for plaintext connections.How this was verified: Both factories pass connection options without
tls, andconnect_temporal_clientaddstls=Falsealongside the token inapi_key.Prompt To Fix With AI