lishuxu commented on code in PR #867:
URL: https://github.com/apache/iceberg-cpp/pull/867#discussion_r3834862337
##########
src/iceberg/catalog/rest/auth/auth_manager.cc:
##########
@@ -148,15 +158,109 @@ class OAuth2Manager : public AuthManager {
config.optional_oauth_params(), client);
}
- return AuthSession::MakeDefault({});
+ return MakeSession(AccessTokenResponse(""), config,
/*keep_refreshed=*/false);
+ }
+
+ Result<std::shared_ptr<AuthSession>> ContextualSession(
+ const SessionContext& context, std::shared_ptr<AuthSession> parent)
override {
+ return MaybeCreateChildSession(context.credentials,
/*allow_credential=*/true,
+ std::move(parent));
}
- // TODO(lishuxu): Override TableSession() for token exchange (RFC 8693).
- // TODO(lishuxu): Override ContextualSession() for per-context exchange.
+ Result<std::shared_ptr<AuthSession>> TableSession(
+ [[maybe_unused]] const TableIdentifier& table,
+ const std::unordered_map<std::string, std::string>& properties,
+ std::shared_ptr<AuthSession> parent) override {
+ return MaybeCreateChildSession(FilterTableSessionProperties(properties),
+ /*allow_credential=*/false,
std::move(parent));
+ }
private:
+ static OAuthTokenResponse AccessTokenResponse(std::string token) {
+ return {
+ .access_token = std::move(token),
+ .token_type = "bearer",
+ .issued_token_type = AuthProperties::kAccessTokenType,
+ };
+ }
+
+ static Result<AuthProperties> ChildConfig(const OAuth2SessionInfo&
parent_info,
+ const std::string& credential) {
+ auto properties = parent_info.optional_oauth_params;
+ properties[AuthProperties::kCredential.key()] = credential;
+ properties[AuthProperties::kScope.key()] = parent_info.scope;
+ properties[AuthProperties::kOAuth2ServerUri.key()] =
parent_info.oauth2_server_uri;
+ return AuthProperties::FromProperties(properties);
+ }
+
+ Result<std::shared_ptr<AuthSession>> MakeSession(
+ const OAuthTokenResponse& token_response, const AuthProperties& config,
+ bool keep_refreshed) const {
+ ICEBERG_PRECHECK(shared_client_ != nullptr,
+ "OAuth2 catalog session must be initialized before child
sessions");
+ return AuthSession::MakeOAuth2(token_response, config.oauth2_server_uri(),
+ config.client_id(), config.client_secret(),
+ config.scope(), keep_refreshed,
+ config.optional_oauth_params(),
*shared_client_);
+ }
+
+ Result<std::shared_ptr<AuthSession>> MaybeCreateChildSession(
+ const std::unordered_map<std::string, std::string>& credentials,
+ bool allow_credential, std::shared_ptr<AuthSession> parent) const {
+ auto token_it = credentials.find(AuthProperties::kToken.key());
+ auto credential_it = credentials.find(AuthProperties::kCredential.key());
+ auto typed_token = FindPreferredTypedToken(credentials);
+ if (token_it == credentials.end() &&
+ (!allow_credential || credential_it == credentials.end()) &&
+ !typed_token.has_value()) {
+ return parent;
+ }
+
+ ICEBERG_PRECHECK(shared_client_ != nullptr,
+ "OAuth2 catalog session must be initialized before child
sessions");
+ auto parent_info = parent->OAuth2Info();
+ ICEBERG_PRECHECK(parent_info.has_value(),
+ "OAuth2 child session requires OAuth2 parent metadata");
+
+ if (token_it != credentials.end()) {
+ ICEBERG_ASSIGN_OR_RAISE(auto config,
+ ChildConfig(*parent_info,
parent_info->credential));
+ return MakeSession(AccessTokenResponse(token_it->second), config,
Review Comment:
This patch intentionally supports initial token exchange only.
Child-session caching and refresh are deferred to a follow-up change,
because enabling refresh without cache/session ownership would create unmanaged
refresh tasks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
