Feat/oauth2 interactive auth - #1
Merged
Merged
Conversation
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Intercept a 401 in Client::send, parse the WWW-Authenticate challenge, and run the OAuth2 flow under a single-flight lock (acquire_oauth2_token) so concurrent requests share one browser prompt and one token refresh. The original request is retried exactly once with the fresh token; a second 401 surfaces as HttpNotOk instead of looping. This sits below the backon retry layer, so need_retry_fetch/need_retry_submit are unaffected. Removes the temporary #[allow(dead_code)] on run_flow and OAuth2State::acquire now that both are exercised, and adds tests/oauth2.rs covering the happy path + token caching, single-flight browser dedup under concurrent 401s, and a challenge-less 401 failing fast without invoking the redirect handler.
…ization header Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Committed under integration_tests/test_setup/oauth/ (not run in CI — the interactive flow needs a human browser login). oauth2_real_login now supports TRINO_OAUTH2_PORT and TRINO_OAUTH2_NO_VERIFY for the self-signed local stack. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
A folded-string entrypoint was mangled by Compose word-splitting (exit 127). Switch to exec-form list (verified: keytool now generates the keystore), and force bash for the Keycloak /dev/tcp healthcheck. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…or starts Trino refuses to start any authentication type without a shared secret. Verified live: coordinator now starts healthy and serves the 401 Bearer challenge over TLS. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Trino denies a query whose session user differs from the authenticated OAuth2 principal (impersonation). Match them by default; override via TRINO_OAUTH2_USER. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
A coordinator with several authentication types configured sends one WWW-Authenticate header per type, in `http-server.authentication.type` order. `HeaderMap::get` returns only the first value, so with the common `PASSWORD,OAUTH2` setup the client saw `Basic realm="Trino"`, failed to parse a Bearer challenge, and surfaced a bare `HttpNotOk(401)` — the OAuth2 flow never started. Verified against Trino 478: the challenges arrive as two separate headers with `Basic` first. Iterate `get_all` and take the first value that parses as a Bearer challenge. The local e2e stack now enables `PASSWORD,OAUTH2` (file authenticator, alice/alice) so the manual run stays on that ordering, and a wiremock test covers Basic-before-Bearer. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Both sites still claimed acquisition/polling would land in a later task, which stopped being true once send() grew the 401-challenge flow. Describe what each path actually does instead: - auth_req: lazy acquisition, driven by send() on the 401 challenge. - RowStream::drop: cached token only, on purpose — a Drop must not block on an interactive login, so an expired or absent token means the cancellation is lost. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
Description
Adds Trino's interactive (browser-based) OAuth2 authentication alongside the
existing Basic and JWT support.
Trino's OAuth2 makes the coordinator the OAuth client — the client never talks
to the IdP directly (no
client_id/PKCE/code exchange, so nooauth2/openidconnectcrate needed). The client reacts to a
401carryingWWW-Authenticate: Bearer x_redirect_server=..., x_token_server=...by presenting thelogin URL to the user, polling the token server until it gets a bearer token, then
retrying the request. From there it's identical to the existing JWT bearer path.
Usage
Bring your own presentation strategy with Auth::new_oauth2_with_handler(...) (a
RedirectHandler trait — headless, custom callback, etc.), and tune the token poll
loop with .with_poll(max_attempts, timeout).
How it fits
the single Client::send choke point — below the backon retry layer, so the
retry predicates are untouched.
redirect handler; concurrent 401s are de-duplicated by a single-flight lock so the
browser opens once.
for the life of the Client (no persistence, no machine-to-machine flow in this PR).
Breaking change
Auth is now #[non_exhaustive] and has a new OAuth2 variant — exhaustive match
on Auth must add a wildcard arm. See MIGRATION.md.
Tests
non-ASCII safety), token poll loop (nextUri follow, error, timeout bound).
round-trip, token caching, single-flight (browser once under concurrency), 401
without a challenge stays HttpNotOk, and re-auth sends exactly one
Authorization header.
stack under integration_tests/test_setup/oauth/. Verified end-to-end against a real
browser login. Not wired into CI (the interactive flow needs a human).
Both default and spooling feature sets build/test green; clippy -D warnings clean.