[bharos]

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.

[bharos]

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

[uros-b]

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

[adutra]

@bharos thanks for flagging me here. TBH, I am not very supportive of this change 😅

First off, the code paths you are modifying are only activated when a vended token exchange happens, that is, a session context or table context contains a subject token and a subject token type, and the auth manager then exchanges this token against another one, using the parent session's token as the actor token.

To my best knowledge, only Trino is leveraging this today:

https://github.com/trinodb/trino/blob/38406672349c33d4902bca7a5ebd380b6b382802/plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/catalog/rest/TrinoRestCatalog.java#L484-L510

And even Trino is struggling with it. They may remove this code soon.

Next, I believe you are conflating an initial token request using token exchange (which is a standard OAuth2 grant) with a token refresh request using token exchange (which is an Iceberg idiosyncrasy). It's not because the initial token was obtained via token exchange that it should be refreshed via token exchange.

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.

Yes that is true. And I agree it's odd. But if we were to fix this properly, we would need to replay the initial token exchange request, not send a refresh request using Iceberg's non-standard token exchange flow (which IDPs cannot understand).

(Small digression: the question of how to properly refresh an exchanged token is very complex: the exchanged token is bound to the subject token's own lifespan. In general, IDPs do not issue refresh tokens for exchanged tokens, which makes exchanged tokens kind of un-refreshable in practice.)

Therefore, unless you have a concrete use case that requires this change, I think it just doesn't make sense anymore to change this part of the codebase.

Generally speaking, I think we should stop updating OAuthUtil – unless it's a critical bugfix – and move our efforts to the AuthManager v2. Please help moving it forward, the first PR is here: #15703.

[github-actions]

This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@iceberg.apache.org list. Thank you for your contributions.

[github-actions]

This pull request has been closed due to lack of activity. This is not a judgement on the merit of the PR in any way. It is just a way of keeping the PR queue manageable. If you think that is incorrect, or the pull request requires review, you can revive the PR at any time.