Repository navigation
fix(connect): send OIDC requests through our own httpx client (#2220) - #2222
Open
dudanogueira wants to merge 1 commit into
Open
dudanogueira wants to merge 1 commit into
dudanogueira wants to merge 1 commit into
Conversation
…te#2220) With OIDC auth the authlib session was the HTTP client for every request. Authlib >= 1.8 builds its sessions on httpx2 whenever httpx2 is installed (e.g. pulled in by openai/langchain), so the httpx Timeout we pass on each request was read as a single number and every call failed: async with "unsupported operand type(s) for +: 'float' and 'Timeout'", sync with "'Timeout' object cannot be interpreted as an integer". The session now only holds and refreshes the token. Requests go through the same httpx client API-key and anonymous connections use; we already set the bearer header per request and refresh in the background, and authlib's inline refresh lives in request()/stream(), which we never called. close() closes the session too, and token-fetch failures from an httpx2-based session raise AuthenticationFailedError like httpx ones. As a side effect OIDC requests now honour the configured proxies, pool limits and retries, which the session never received.
There was a problem hiding this comment.
Orca Security Scan Summary
| Status | Check | Issues by priority | |
|---|---|---|---|
| Infrastructure as Code | View in Orca | ||
| SAST | View in Orca | ||
| Secrets | View in Orca | ||
| Vulnerabilities | View in Orca |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
Fixes #2220. With OIDC auth (bearer token, client credentials, username/password), every request fails as soon as
httpx2is installed next to the client. Installingopenai/langchainis enough to pull it in:TypeError: unsupported operand type(s) for +: 'float' and 'Timeout'TypeError: 'Timeout' object cannot be interpreted as an integer, which then gets masked by anAssertionErrorinclose()The authlib session was the HTTP client for every request. Since Authlib 1.8,
authlib.integrations.httpx_clientbuilds its sessions onhttpx2.AsyncClient/httpx2.Clientwheneverhttpx2is importable. We then hand that client anhttpx.Timeouton each request.httpx2doesn't recognise the foreign object and puts the whole thing into each slot ({'connect': Timeout(...), ...}), so the socket layer receives aTimeoutwhere it expects a number. API-key and anonymous connections aren't affected because they use our own client.CI didn't catch this because
requirements-devel.txtpinsauthlib==1.6.12, whilesetup.cfgallowsauthlib>=1.7.1,<2.0.0.Approach
The authlib session now only holds and refreshes the OIDC token. Every request goes through the same
httpxclient that API-key and anonymous connections use. Nothing that relied on the session's HTTP behaviour is lost:Authorization: Bearer <token>ourselves on every request (__get_latest_headers→get_current_bearer_token).request()/stream(). We only ever calledbuild_request()+send(), so it never ran. Our background refresher was already the only refresh mechanism, and it now operates onself._oauth_session.This works with both
httpx- andhttpx2-based authlib, so it needs no dependency pins or upgrades. It also doesn't conflict with a later move tohttpx2(#2155).Rejected alternatives:
authlib<1.8: this only postpones the problem, and it blocks users who need newer authlib.isinstancechecks): this touches many call sites and stays fragile. Any futurehttpxobject we pass to the session would bring the bug back.Key areas for review
v4.pyOIDC branch of__process_oidc_response: the session goes into_oauth_session, then__make_clientsbuilds the real client.v4.pyclose(): both the client and the session are closed._OAUTH_HTTP_ERRORS: token-fetch failures from anhttpx2-based session still becomeAuthenticationFailedError.Risks and mitigations
_make_mounts, so the configuredProxies, pool limits and retries apply. Before, the session never received them. Users who relied on OIDC requests bypassing their configured proxy would see a difference. Token requests still don't use these settings; that's addressed in a follow-up PR.Testing
test_oidc_requests_use_own_http_client(sync and async, three credential types each) checks that requests go through a plainhttpxclient and thatclose()releases the session. It fails onmainand passes here, whichever authlib version is installed, so CI's pinned 1.6.12 still catches a regression.mock_tests/test_auth.py,test_timeouts.pyandtest_collection.py(62 tests) pass with authlib 1.6.12, and with authlib 1.9.1 +httpx22.16.0.httpx2and with authlib 1.7.2: connect, an authenticated call, a read timeout mapped toWeaviateTimeoutError, andclose()all work for all three credential types in sync and async. Onmain, all six fail with authlib 1.9.1 +httpx2. The background refresh keeps fetching tokens for both the refresh-token and client-credentials paths.httpx2installed: the 4 errors onmainin_send(thehttpx/httpx2type mix) are gone.