feat(rest): support OAuth2 token exchange sessions - #867
Conversation
| HttpClient& client, | ||
| const std::unordered_map<std::string, std::string>& properties) override { | ||
| ICEBERG_ASSIGN_OR_RAISE(auto config, AuthProperties::FromProperties(properties)); | ||
| shared_client_ = &client; |
There was a problem hiding this comment.
shared_client_ is a borrowed HttpClient*, but child sessions use it after this call. Please pass a std::shared_ptr<HttpClient> through the manager/session API so the client lifetime is explicit.
| // Determine expiration time | ||
| expires_at_ = std::chrono::steady_clock::time_point{}; | ||
| if (token_response.expires_in_secs.has_value()) { | ||
| expires_at_ = std::chrono::steady_clock::now() + |
There was a problem hiding this comment.
expires_in is measured from the token request, but the session starts the clock later. A slow config request can consume part of the token lifetime; please carry the init fetch start time into the session.
| if (endpoint.starts_with('/')) { | ||
| return base_uri + endpoint; | ||
| } | ||
| return base_uri + "/" + std::string(TrimTrailingSlash(endpoint)); |
There was a problem hiding this comment.
Java preserves the relative endpoint suffix. Trimming oauth/token/ here changes the resolved URI; please keep the trailing slash and update the test.
| Result<std::string> ResolveOAuth2ServerUri( | ||
| const std::unordered_map<std::string, std::string>& properties) { | ||
| auto endpoint_it = properties.find(AuthProperties::kOAuth2ServerUri.key()); | ||
| std::string endpoint = endpoint_it == properties.end() || endpoint_it->second.empty() |
There was a problem hiding this comment.
Java defaults only when oauth2-server-uri is absent. This also defaults an explicitly empty value; please preserve that distinction or reject empty explicitly.
| std::unordered_map<std::string, std::string> properties = { | ||
| {AuthProperties::kAuthType, "oauth2"}, | ||
| {AuthProperties::kToken.key(), "my-static-token"}, | ||
| {AuthProperties::kCredential.key(), "client-id:client-secret"}, |
There was a problem hiding this comment.
Please add a test with a short expires_in and delayed config fetch to verify elapsed init time is deducted from the session lifetime.
| EXPECT_THAT(root->WithContext(SessionContext{}), IsError(ErrorKind::kInvalidArgument)); | ||
| } | ||
|
|
||
| TEST_F(RestCatalogIntegrationTest, OAuthContextCredentialEndToEnd) { |
There was a problem hiding this comment.
These tests call AuthManager directly, so they do not cover RestCatalog context/table wiring. Please add one real catalog path or move them to the unit suite.
Add RFC 8693 token exchange support, including token type helpers, request form construction, OAuth endpoint normalization, and response handling. Preserve OAuth metadata in auth sessions and create contextual and table-scoped child sessions from direct tokens, credentials, or typed tokens. Disable child refresh until session lifecycle management is available.
Add RFC 8693 token exchange support, including token type helpers,
request form construction, OAuth endpoint normalization, and response
handling.
Preserve OAuth metadata in auth sessions and create contextual and
table-scoped child sessions from direct tokens, credentials, or typed
tokens. Disable child refresh until session lifecycle management is
available.
|
As discussed offline, I'll directly address these minor issues to move forward. |
5ec2b7f to
b5e2ea0
Compare
|
I've made some changes to this PR. The intention is to be more Java-like so future feature catch-up and review will be much easier.
|
- propagate shared HttpClient ownership through REST auth managers - preserve OAuth2 session state and request-start expiry semantics - expose OAuth2 metadata through a synchronized OAuth2Info snapshot - encapsulate OAuth2 utilities and normalize token endpoints - update REST auth and integration tests
b5e2ea0 to
198ef6d
Compare
wgtmac
changed the title
2 participants
Add RFC 8693 token exchange support, including OAuth2 endpoint handling and response parsing. Create contextual and table child sessions from direct tokens, credentials, or typed tokens.
Keep OAuth2 metadata in one synchronized session snapshot. Use shared
HttpClientownership so refresh tasks can safely keep the client alive. Align token expiry with Java by preferring JWTexpand otherwise using the request start time plusexpires_in.Move OAuth2 helpers into
OAuth2Utiland update unit and integration tests. Leave catalog token refresh, token exchange during refresh, and child session caching as follow-up work.