Repository navigation
Conversation
lishuxu
force-pushed
the
feature/oauth-2
branch
4 times, most recently
from
October 9, 2026 12:22
672cb4d to
6f4d8c9
Compare
shangxinli
reviewed
Oct 17, 2026
shangxinli
reviewed
Oct 17, 2026
| 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); |
Contributor
There was a problem hiding this comment.
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.
Contributor
Author
There was a problem hiding this comment.
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).
shangxinli
reviewed
Oct 17, 2026
added 2 commits
October 20, 2026 17:42
This commit implements Phase 2 of the OAuth2 enhancement (apache#962), adding session caching and lifecycle management for contextual and table-scoped child sessions. **Core Changes:** 1. **AuthSessionCache implementation** - Guava-like cache with expireAfterAccess semantics - Loading deduplication: concurrent Get() calls for the same key load once - Dual expiration: idle timeout + OAuth2 token expiration - Thread-safe with fine-grained locking (loader runs outside lock) - Periodic sweep for expired session cleanup - Removal listener integration for graceful session shutdown 2. **OAuth2Session enhancements** - Add `StopRefreshing()`: non-blocking refresh cancellation for cache eviction - Add `ExpiresAt()`: expose token expiration time for cache expiry checks - Add `IsRefreshable()`: indicates whether auto-refresh is enabled - Extend `OAuth2Info()` to include `headers` (fixes child auth Issue 2) - Improve `CloseImpl()`: always wait for active refreshes, defensive cancel - Add `StoreScheduledTask()`: handle Close/StopRefreshing race conditions 3. **AuthManager child session handling** - Cache contextual sessions by `session_id` (key: "ctx:<session_id>") - Cache table sessions by `token` value (key: "tbl:<token>") - Fix Issue 2: child sessions now use parent's headers for authentication - Credential-based child sessions inherit catalog-level `keep_refreshed` - Token/exchange child sessions remain non-refreshed (Phase 2 scope) 4. **Configuration** - Add `auth.session-timeout-ms` (default: 1 hour) - Document `token-exchange-enabled` as not yet implemented - Sweep interval: clamped between 1s and 60s based on timeout 5. **SessionContext extension** - Add `session_id` field for stable caller context identification - Add `identity` field for descriptive caller identity - Add `credentials` map for context-specific auth material **Testing:** - 14 new cache tests covering concurrency, expiration, and lifecycle - Extended auth_manager tests for child session creation **Notes:** - Token exchange refresh is not yet implemented (see TODO in DoRefreshAttempt) - Failed loads are not cached (retry on next Get) - Orphan sessions (loaded after Close) are stopped immediately
- 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.
lishuxu
force-pushed
the
feature/oauth-2
branch
from
October 20, 2026 09:48
6b19c11 to
870be7d
Compare
This branch has not been deployed
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.
No description provided.