Skip to content

fix(connect): catch authlib OAuthError in background token refresh thread - #2111

Open
Solaris-star wants to merge 1 commit into
weaviate:mainfrom
Solaris-star:fix/2110-catch-authlib-oauth-error
Open

fix(connect): catch authlib OAuthError in background token refresh thread#2111
Solaris-star wants to merge 1 commit into
weaviate:mainfrom
Solaris-star:fix/2110-catch-authlib-oauth-error

Conversation

@Solaris-star

Copy link
Copy Markdown

Summary

Fixes #2110

The periodic_refresh_token daemon thread only caught httpx.HTTPError. When the identity provider rejects a refresh at the OAuth2 protocol level (e.g. Keycloak returns 400 invalid_grant due to token rotation, reuse, revocation, or session expiry), authlib raises OAuthError which inherits from AuthlibBaseErrorNOT from httpx.HTTPError. The uncaught exception killed the daemon thread silently, permanently breaking the connection's ability to refresh tokens.

Fix

Add AuthlibBaseError to the except clause so both transport-level (HTTPError) and protocol-level (OAuthError) failures are caught, warned, and retried after 1 second:

# Before
except HTTPError as exc:
# After
except (HTTPError, AuthlibBaseError) as exc:

Tests

Added test/test_token_refresh.py with 4 tests:

  • test_authlib_base_error_is_not_http_error — confirms AuthlibBaseError is NOT a subclass of HTTPError (proves the bug exists)
  • test_authlib_base_error_caught_by_fixed_except_clause — the fixed clause catches it
  • test_http_error_still_caught_by_fixed_except_clause — no regression on existing behavior
  • test_oauth_error_subclass_caught — concrete OAuthError subclass is also caught
$ pytest test/test_token_refresh.py -v
4 passed

…read

The periodic_refresh_token daemon thread only caught httpx.HTTPError.
When the identity provider rejects a refresh at the OAuth2 protocol
level (e.g. Keycloak returns 400 invalid_grant due to token rotation,
reuse, revocation, or session expiry), authlib raises OAuthError which
inherits from AuthlibBaseError — NOT from httpx.HTTPError. The uncaught
exception killed the daemon thread silently, permanently breaking the
connection's ability to refresh tokens (weaviate#2110).

Add AuthlibBaseError to the except clause so both transport-level
(HTTPError) and protocol-level (OAuthError) failures are caught,
warned, and retried after 1 second.

Adds test/test_token_refresh.py with 4 tests verifying:
- AuthlibBaseError is NOT a subclass of HTTPError (confirms the bug)
- The fixed except clause catches AuthlibBaseError
- HTTPError is still caught (no regression)
- Concrete OAuthError subclass is also caught

Fixes weaviate#2110

@orca-security-eu orca-security-eu Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Orca Security Scan Summary

Status Check Issues by priority
Passed Passed Infrastructure as Code high 0   medium 0   low 0   info 0 View in Orca
Passed Passed SAST high 0   medium 0   low 0   info 0 View in Orca
Passed Passed Secrets high 0   medium 0   low 0   info 0 View in Orca
Passed Passed Vulnerabilities high 0   medium 0   low 0   info 0 View in Orca

@weaviate-git-bot

Copy link
Copy Markdown

To avoid any confusion in the future about your contribution to Weaviate, we work with a Contributor License Agreement. If you agree, you can simply add a comment to this PR that you agree with the CLA so that we can merge.

beep boop - the Weaviate bot 👋🤖

PS:
Are you already a member of the Weaviate Forum?

@Solaris-star

Copy link
Copy Markdown
Author

I have read and agree to the Contributor License Agreement.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Background OIDC token-refresh thread dies silently on authlib.OAuthError, permanently breaking the connection

2 participants