Repository navigation
Add workload identity federation via RFC 8693 token exchange - #1245
jjcollinge wants to merge 1 commit into
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1245 +/- ##
==========================================
+ Coverage 83.89% 84.34% +0.44%
==========================================
Files 123 131 +8
Lines 10265 10636 +371
==========================================
+ Hits 8612 8971 +359
- Misses 1653 1665 +12 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
CasperGN
left a comment
There was a problem hiding this comment.
Request changes
The design is sound: bearer tokens go through per-call metadata, mTLS goes through a channel swap with a drain period, and the providers are well separated. Two things need fixing before this leaves draft.
Blocking: the HTTP path sends bearer tokens over plain http://. The PR says a credential manager "requires a TLS endpoint". gRPC enforces that indirectly, because explicit credentials force secure_channel. DaprHttpClient checks nothing. I pointed DaprHttpClient(credential_manager=<RFC8693 manager>) at a local http://127.0.0.1 server and it received dapr-api-token: SECRET-ACCESS-TOKEN in cleartext. With the env-var opt-in and the default DAPR_HTTP_ENDPOINT, every workload-identity user ends up here. Please reject a bearer manager on a non-TLS endpoint, in both the HTTP and gRPC constructors. gRPC currently upgrades a plaintext endpoint to TLS without saying so, and the failure only shows up as a handshake error on the first call.
python-sdk/dapr/clients/http/client.py
Lines 95 to 100 in 9e0c7eb
python-sdk/dapr/clients/grpc/_channel.py
Lines 106 to 107 in 9e0c7eb
Blocking: DaprHealth.wait_for_sidecar() ignores the credential manager. Every client constructor calls it first. It sends only DAPR_API_TOKEN and never presents a client certificate. Against an endpoint that requires client certs, with DAPR_CLIENT_CERT_CHAIN_PATH/_PRIVATE_KEY_PATH set, the check fails with TLSV13_ALERT_CERTIFICATE_REQUIRED until it times out, so the client can never be constructed. It also still sends the static token that the PR says the credential manager replaces. The same applies to dapr/aio/clients/health.py. Every end-to-end test patches wait_for_sidecar to a no-op, so none of them catch this.
python-sdk/dapr/clients/health.py
Lines 37 to 48 in 9e0c7eb
Non-blocking: the workflow extension doesn't use any of this. DaprWorkflowClient and WorkflowRuntime still read DAPR_API_TOKEN directly. Anyone who sets the env vars and uses workflows gets the old auth with no warning. Either wire it in or document the scope clearly.
python-sdk/dapr/ext/workflow/dapr_workflow_client.py
Lines 81 to 82 in 9e0c7eb
Non-blocking: the async client now fails outside a running loop when the env vars are set. dapr.aio.clients.DaprClient() built at module level raises a bare RuntimeError: no running event loop once DAPR_CLIENT_* is set. The same code works without them. A clear error message would help, or defer ensure_current_loop and the start() task to first use.
python-sdk/dapr/aio/clients/grpc/client.py
Lines 166 to 173 in 9e0c7eb
Non-blocking: secrets show up in repr. WorkloadCredential is a plain dataclass, so repr() prints the private key PEM and the bearer token. Any logger.debug("%r", cred) or traceback local-variable dump leaks them. field(repr=False) on token and private_key is cheap.
python-sdk/dapr/credentials/base.py
Lines 42 to 47 in 9e0c7eb
Non-blocking: test gaps found by mutation. Three mutations survived: dropping suppress_api_token_interceptor from the aio channel (only the sync client has a "replaces DAPR_API_TOKEN" test), ignoring refresh_margin, and removing the per-loop lock rebuild in AsyncCredentialManager._current_lock. The last one passes test_get_is_reusable_across_independent_event_loops anyway, so either the rebuild is dead code or the test doesn't exercise it.
Docs: this adds new env vars, extras and a constructor parameter, so it needs a dapr/docs PR. The example README alone isn't enough.
What I checked: all three commits at 9e0c7eb. tests/credentials: 119 passed. Full unit suite: 1819 passed. ruff check, ruff format --check and mypy are clean. I ran 12 mutations against tests/credentials + tests/clients: 9 killed, including the send_bytes mutable-default fix, drain grace, listener notify, retry-after-failure and kind validation; 3 survived, listed above. I also wrote local repros for the plaintext-HTTP token leak, the health check failing under client-cert mTLS, and the aio client failing outside a loop.
Signed-off-by: Joni Collinge <jonathancollinge@live.com>
9e0c7eb to
43b1c46
Compare
Description
Lets the Dapr clients authenticate with a short-lived token obtained through workload identity federation,
instead of a static
DAPR_API_TOKEN. The workload's platform, such as Kubernetes or a cloud provider,issues it an identity token. The client exchanges that token at an RFC 8693 token exchange endpoint and
sends the result as
dapr-api-token. No credential has to be provisioned up front, and the token isexchanged again ahead of its expiry.
New
dapr.credentialspackageCredentialProvider/WorkloadCredential: a provider returns a token with an optional expiry.CredentialManager/AsyncCredentialManager: cache the token and refresh it in the background aheadof expiry. A failed refresh keeps the last good token and retries.
get()never blocks on an in-flightrefresh.
oauth2.RFC8693TokenExchangeProvider: exchanges a subject token for an access token, with optionalaudienceandscope. RFC 6749 errors are surfaced, and a malformedexpires_inis rejected.FileAttestationSource: re-reads the file on every exchange, so tokens rotated on disk are picked up.kubernetes_service_account_source(): a file source for a projected service account token.CallableAttestationSource: for metadata endpoints or CI OIDC endpoints.Client wiring
DaprClient,DaprGrpcClient,DaprGrpcClientAsync,DaprHttpClientandDaprInvocationHttpClientaccept an optional
credential_manager.channel.
DAPR_API_TOKENand requires a TLS endpoint.Configuration
DAPR_WORKLOAD_IDENTITY_TOKEN_URLandDAPR_WORKLOAD_IDENTITY_TOKEN_FILE_PATH, optionally withDAPR_WORKLOAD_IDENTITY_AUDIENCEandDAPR_WORKLOAD_IDENTITY_SCOPE.DAPR_API_TOKENis also set, a warning is logged and the workload identity token is used.Packaging
dapr[workload-identity](httpx), included indapr[all].import dapr.clientsworks without the extra.Fixes to existing code
DaprHttpClient.send_byteswrote into its mutable defaultheaders={}. Auth headers from one call thenleaked into later calls made without explicit headers, including the actor HTTP client's. It now copies
the headers.
create_aio_channelcreated a plaintext channel, dropping explicit credentials, unless the endpoint had?tls=true. Explicit credentials now always produce a secure channel.Issue reference
Please reference the issue this PR will close: #[issue number]
Checklist