Core: Refresh exchanged sessions by token exchange - #17601
Open
bharos wants to merge 1 commit into
Open
Conversation
A session created by exchanging a subject token inherits both credential and exchangeEnabled from its parent. When token-exchange-enabled is false, refreshToken takes the client_credentials branch and mints a token for the catalog client, so the session silently stops representing the exchanged subject. Pin exchangeEnabled to true for sessions built by fromTokenExchange, so that how a session was obtained decides how it is renewed. The flag added in apache#13809 continues to control credential derived sessions. Fixes apache#17600 Generated-by: GitHub Copilot (Claude Opus 5)
bharos
force-pushed
the
exchanged-session-refresh
branch
from
August 10, 2026 23:51
d84d617 to
26f2a84
Compare
bharos
marked this pull request as ready for review
August 11, 2026 00:03
Contributor
Author
|
cc @adutra @singhpk234 WDYT about this fix, please take a look |
uros-b
reviewed
Aug 11, 2026
uros-b
left a comment
Member
There was a problem hiding this comment.
cc @waterWang and @bharos regarding #17602 (review).
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.
Fixes #17600.
A session created by exchanging a subject token is built through
fromTokenResponse, which copies the parent configuration withAuthConfig.builder().from(parent.config()). That inherits bothcredentialandexchangeEnabledfrom the catalog session.OAuth2Util.refreshTokenbranches only onexchangeEnabled. So whentoken-exchange-enabledisfalse, an exchanged session refreshes itself with the parent's client credential and receives a token for the catalog client. The session keeps working, but it no longer represents the exchanged subject.This pins
exchangeEnabledtotruefor sessions built byfromTokenExchange, so how a session was obtained decides how it is renewed. #13809 added the flag to control how credential derived sessions refresh, and that behaviour is unchanged.Two points worth calling out for review:
This makes
exchangeEnabledon a child session record how the session was obtained, rather than only reflecting what was configured. An alternative is a separate field onAuthConfigfor provenance, at the cost of a wider change. I went with the smaller one, happy to switch.The pinned value propagates one level further, because
RESTSessionCatalogpasses the contextual session as the parent of table sessions andfromAccessTokenalso copies the parent config. A vended credential table session under an exchanged session therefore re-exchanges its own token instead of falling back to the client credential. That looks like the correct behaviour for the same reason, but it is a behaviour change beyond the contextual session itself.Not addressed here:
fromAccessTokeninherits the parent credential in the same way, so the bearer token path carries a similar hazard. Left for a separate change to keep this one reviewable.Testing: added
exchangedSessionRefreshesByExchangeWhenExchangeIsDisabledtoTestOAuth2Util. It fails without the production change. No existing test reaches this path, since the two tests inTestRESTCatalogthat disable token exchange never create a session throughfromTokenExchange.AI Disclosure