Fix workload identity authentication for WebSockets#3485
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9e6f44d6d8
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| return f"Bearer {self._workload_identity_auth.get_token()}", True | ||
|
|
||
| def _websocket_auth_headers(self) -> dict[str, str]: | ||
| authorization, _ = self._resolve_auth_header(self.auth_headers.get("Authorization")) |
There was a problem hiding this comment.
Respect WebSocket Authorization overrides before token exchange
With a workload-identity client this resolves a workload token unconditionally, while the WebSocket managers only merge extra_headers afterward. If a caller supplies extra_headers={"Authorization": ...} or omits that header to override auth for a specific socket, that override would win later, but the workload token exchange still runs first and can fail or block the connection despite the supplied header; HTTP requests avoid this by resolving only when the merged Authorization is the workload placeholder. Consider deferring workload-token resolution until after WebSocket extra headers are considered.
Useful? React with 👍 / 👎.
Summary
Testing
pytest -n 0 tests/test_client.py tests/test_websocket_connection_options.py(187 passed)The full generated API-resource suite requires the external mock server on port 4010, which was not running locally.