Antalya 26.6: Forward auth tokens to data lake catalogs - #2329
Open
zvonand wants to merge 6 commits into
Open
Conversation
zvonand
added
antalya-26.6
port-antalya
zvonand
force-pushed
the
feature/antalya-26.6/aouth-forward-to-datalake
branch
from
September 9, 2026 06:48
5d6ced1 to
33ba90f
Compare
… actor token `RestCatalog::getForwardedToken` re-reads the server-level `enable_token_forwarding` setting on every forwarded request instead of trusting the decision `Session::authenticate` made once. The setting is hot-reloadable, so without this an operator turning it off kept forwarding the token captured by every already-authenticated session until the server restarted. Exchanged session tokens are dropped at the same time, so they cannot outlive the policy they were minted under. `oauth_forward_actor_token` mints the service principal's token through the new `RestCatalog::getServicePrincipalToken` instead of sending an `actor_token` only when one happened to be cached already, which meant delegation silently degraded to plain impersonation. A failure to mint it propagates rather than downgrading the exchange. `IDatabase::checkDatabase` takes a `ContextPtr`, so `CHECK DATABASE` against a `DataLakeCatalog` database forwards the querying user's token instead of sending none. `ContextData`'s copy constructor copies `forwarded_auth_token`: `Context::createCopy` builds the query context from the session context, so leaving it out lost the token before the query could forward it. Tests: - `tests/integration/test_datalake_token_forwarding` covers the write path (`INSERT`, `DROP TABLE`) and `CHECK DATABASE`, both forwarding and failing closed, and the listing assertions now require a known table to come back rather than any number, which held just as well when the catalog refused the request. - `gtest_rest_catalog_token_forwarding` turns the server switch on through a fixture and adds a case for turning it off at runtime; the actor token case now asserts the minted service principal token is what is sent. - `05028_datalake_token_forwarding_fail_closed` pins down which `CATALOG_USER_TOKEN_NOT_AVAILABLE` branch fires and looks for a JWT-shaped canary credential in `system.query_log` and `system.text_log`. - `test_datalake_sso_lakekeeper`: the Keycloak realm no longer declares top-level `clientScopes`, since supplying that array drops Keycloak's built-ins and leaves tokens without `sub` or `preferred_username`; `LAKEKEEPER__OPENID_SCOPE` is a single scope, which is how Lakekeeper compares it. Token users get a `token_users` role through `common_roles` so that every denial asserted comes from the catalog rather than from ClickHouse access control.
zvonand
force-pushed
the
feature/antalya-26.6/aouth-forward-to-datalake
branch
from
September 9, 2026 20:59
4acc349 to
b5f1b30
Compare
`DatabaseDataLake::getTablesIterator` and `getLightWeightTablesIterator`
rethrow catalog errors when `show_data_lake_catalogs_in_system_tables` is on,
instead of swallowing every failure into an empty listing. Two tests were
written against the previous behaviour:
- `05028_datalake_token_forwarding_fail_closed`: `InterpreterShowTablesQuery`
turns that setting on for an explicit `SHOW TABLES` against a
`DataLakeCatalog` database, so the refusal now reaches the client. The test
still asserts that no table name is disclosed, and additionally that
`CATALOG_USER_TOKEN_NOT_AVAILABLE` is reported. Plain `system.tables`, where
the setting is off, keeps listing nothing without an exception.
- `test_datalake_sso_lakekeeper::test_users_see_different_tables`: the listing
query sets the setting explicitly, and Lakekeeper answers a listing the
principal has no grant for with `NoSuchWarehouseException` ("Warehouse not
found or access denied"), so the query fails rather than returning an empty
result. This is the expectation `test_alice_cannot_read_bobs_table` already
had for the same denial.
`RestCatalog::commitSettingsChanges` published the new state and the newly minted `access_token`, but left both per-user caches populated. Both hold artifacts derived from the credentials that were just replaced: session tokens `exchangeUserToken` obtained with the old `client_id`/`client_secret`, and the credentials the catalog vended to the resulting identity. `ALTER DATABASE ... MODIFY SETTING catalog_credential = ...` is how an operator rotates a leaked client secret, so leaving the caches warm kept the rotated secret working for the rest of the cache TTL -- up to `oauth_user_token_cache_ttl` and `vended_credentials_cache_ttl`, both 300 seconds by default. The 401 retry path does not help: a session token already issued has its own expiry, independent of the client secret that minted it, so the catalog keeps honouring it. Both caches are now cleared, after the new state is published so that a request racing with the ALTER re-populates from the new credentials and not the old ones.
|
cool |
Clearing the caches in `commitSettingsChanges` does not cover a request that authenticated before the ALTER and finishes after it. The window is a whole catalog round trip, not a few instructions: a query takes a `CatalogState` snapshot, exchanges a token or waits for `loadTable`, and only then writes its result back -- after the clear. The pre-rotation session token and the credentials the catalog vended to that identity are put straight back, for a full cache TTL. The same race hits `access_token`, and there it is worse. `getAuthHeaders` mints from the caller's snapshot and publishes unconditionally, so a grant that started before the ALTER overwrites the token the ALTER eagerly published. That is the plain non-forwarding `client_credentials` path, not just forwarding, and nothing bounds it: the rotated-away credential stays in force for the whole lifetime of that token, or until a 401 forces `update_token`. `commitSettingsChanges` now bumps an `auth_generation` counter, and a request tags whatever it derives from a snapshot with the generation it read. Generation-scoped cache keys make a late write unreachable rather than merely late, and `publishServiceToken` publishes only when the credentials it minted with are still in force. The ordering is what makes it hold. A reader takes the generation before the state, so a stale pair is discarded and the reverse pairing only wastes a cache fill. `commitSettingsChanges` publishes the state, bumps, then clears, so a write that slips past the generation check necessarily started before the clear and is wiped by it, while a write landing after the clear is already keyed to a dead generation. The bump and the publish it guards share `auth_publish_mutex`, which is never held across a network request. The counter is deliberately not a field of `CatalogState`: that is republished for unrelated reasons, and a counter that only moves forward must not travel back with it. Both regression tests park a catalog route so the ALTER lands while a request is genuinely in flight; both fail without this change.
`loadConfigIfNeeded` is a read-modify-write on the catalog state with a `/v1/config` request in the middle: it snapshots the state, loads, then republishes that snapshot with the config attached. `config_mutex` keeps two config loads apart, but `commitSettingsChanges` publishes without taking it, so an `ALTER DATABASE ... MODIFY SETTING catalog_credential` landing in that window is silently undone -- the credentials it published are replaced by the ones the load started with. Unlike the cache races this is bounded by nothing: the database keeps running on the rotated-away secret until the next ALTER. Reachable only with `oauth_forward_user_token = 1`, where there may be no service credential at construction time and `/v1/config` is therefore deferred to the first user query instead of being fetched in the constructor. The publish now checks the auth generation under `auth_publish_mutex`, the same lock `commitSettingsChanges` publishes under, and drops the result if the credentials moved on. Dropped rather than merged into the current state: the config was read with credentials no longer in force, and they may resolve the warehouse to a different prefix or base location. `config_loaded` stays false, so the next request loads it again against the credentials that are. Two concurrent ALTERs have the same read-modify-write shape between `prepareSettingsChanges` and `commitSettingsChanges`; that is left alone here.
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
2 participants
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.
Changelog category (leave one):
Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):
Forward auth tokens to data lake catalogs
CI/CD Options
Exclude tests:
Regression jobs to run: