Skip to content

feat(auth): implement AuthSessionCache for OAuth2 child sessions - #985

Open
lishuxu wants to merge 3 commits into
apache:mainfrom
lishuxu:feature/oauth-2
Open

lishuxu wants to merge 3 commits into
apache:mainfrom
lishuxu:feature/oauth-2

Conversation

@lishuxu

@lishuxu lishuxu commented Oct 5, 2026

Copy link
Copy Markdown
Contributor

No description provided.

@lishuxu
lishuxu force-pushed the feature/oauth-2 branch 4 times, most recently from 672cb4d to 6f4d8c9 Compare October 9, 2026 12:22
Comment thread src/iceberg/catalog/rest/auth/auth_manager.cc Outdated
ICEBERG_ASSIGN_OR_RAISE(auto config,
ChildConfig(*parent_info, parent_info->credential));
ChildConfig(parent_info, parent_info.credential));
return MakeSession(response, config, /*keep_refreshed=*/false);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Token-exchange children are pinned to keep_refreshed false, so an exchanged token just expires. Java builds this one through fromTokenExchange with refreshExecutor at OAuth2Manager.java:271, the same as the credential path, so it should probably follow keep_refreshed_ too.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Agreed, but enabling refresh now would switch the child to the catalog identity, because refresh only uses the parent's credential. I'll enable it once refresh via token exchange lands (TODO in DoRefreshAttempt).

Comment thread src/iceberg/catalog/rest/auth/auth_session_cache.cc Outdated
shuxu.li added 2 commits October 24, 2026 16:51
Cache contextual and table-scoped OAuth2 child sessions so that context
credentials do not trigger a token request on every catalog call (apache#962).

AuthSessionCache:
- Expire entries after auth.session-timeout-ms without access (default
  1 hour), and when the token of an OAuth2 session expires.
- Load each key once under concurrent Get() calls, with the loader
  running outside the lock. Failed loads are not cached.
- Hand every removed session to the removal listener exactly once. A
  session loaded after Close() is removed immediately.
- Sweep expired entries periodically on the auth refresh executor, every
  session timeout clamped to [1s, 60s]. Without an executor, entries are
  reclaimed when accessed again or on Close().

OAuth2Manager:
- Cache contextual sessions by session_id and table sessions by token.
- Authenticate child token requests with the parent session's headers.
- Credential child sessions follow token-refresh-enabled and, with
  refresh enabled, require the auth executor. Token and exchange child
  sessions do not refresh until refresh via token exchange is supported.
- Token child sessions no longer carry the parent's credential.

OAuth2Session:
- Add StopRefreshing(), a non-blocking stop used on cache removal.
- Add ExpiresAt() and IsRefreshable(), and include the session headers
  in OAuth2Info().
- Cache table sessions that use typed tokens. The child credential is now
  selected once by SelectChildCredential, and the table cache key is the
  value of the token actually used. Previously only the plain token
  property produced a key, so typed tokens (jwt, id_token, ...) skipped
  the cache and ran a token exchange on every table load.
- Evict only non-refreshable sessions when their token expires. Evicting
  a refreshable session could land before an in-flight refresh completed
  and stop refreshing for callers still holding it, such as loaded tables.
  Refreshable sessions now expire only after the idle timeout.
Since apache#1033, refreshing a session requires an auth executor. Credential
child sessions followed token-refresh-enabled unconditionally, so a
static-token catalog without an executor failed to create them. They now
refresh only when an executor is provided and otherwise keep their initial
token, as before, with a one-time warning.

This branch has not been deployed

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants