Skip to content

Core: Refresh exchanged sessions by token exchange - #17601

Open
bharos wants to merge 1 commit into
apache:mainfrom
bharos:exchanged-session-refresh
Open

Core: Refresh exchanged sessions by token exchange#17601
bharos wants to merge 1 commit into
apache:mainfrom
bharos:exchanged-session-refresh

Conversation

@bharos

@bharos bharos commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Fixes #17600.

A session created by exchanging a subject token is built through fromTokenResponse, which copies the parent configuration with AuthConfig.builder().from(parent.config()). That inherits both credential and exchangeEnabled from the catalog session.

OAuth2Util.refreshToken branches only on exchangeEnabled. So when token-exchange-enabled is false, 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 exchangeEnabled to true for sessions built by fromTokenExchange, 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:

  1. This makes exchangeEnabled on a child session record how the session was obtained, rather than only reflecting what was configured. An alternative is a separate field on AuthConfig for provenance, at the cost of a wider change. I went with the smaller one, happy to switch.

  2. The pinned value propagates one level further, because RESTSessionCatalog passes the contextual session as the parent of table sessions and fromAccessToken also 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: fromAccessToken inherits 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 exchangedSessionRefreshesByExchangeWhenExchangeIsDisabled to TestOAuth2Util. It fails without the production change. No existing test reaches this path, since the two tests in TestRESTCatalog that disable token exchange never create a session through fromTokenExchange.


AI Disclosure

  • Model: Claude Opus 5
  • Platform/Tool: GitHub Copilot
  • Human Oversight: fully reviewed
  • Prompt Summary: Investigate why an exchanged REST catalog session loses its identity after refresh, confirm the behaviour on main, and produce a minimal fix with a regression test.

@github-actions github-actions Bot added the core label Aug 10, 2026
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
bharos force-pushed the exchanged-session-refresh branch from d84d617 to 26f2a84 Compare August 10, 2026 23:51
@bharos
bharos marked this pull request as ready for review August 11, 2026 00:03
@bharos

bharos commented Aug 11, 2026

Copy link
Copy Markdown
Contributor Author

cc @adutra @singhpk234 WDYT about this fix, please take a look

@uros-b uros-b left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

cc @waterWang and @bharos regarding #17602 (review).

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Core: Exchanged AuthSession refreshes with the parent's client credential, silently changing identity

2 participants