This is an automated email from the ASF dual-hosted git repository.

wgtmac pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/iceberg-cpp.git


The following commit(s) were added to refs/heads/main by this push:
     new 0471f8fb feat(rest): add an option to ignore server OAuth2 endpoints 
(#1041)
0471f8fb is described below

commit 0471f8fba6063bac5c75ed904e74bd5b65be52e1
Author: lishuxu <[email protected]>
AuthorDate: Wed Oct 28 15:02:09 2026 +0800

    feat(rest): add an option to ignore server OAuth2 endpoints (#1041)
    
    Introduce `rest.auth.oauth2.trust-server-uri` (default: `true`) to control 
whether server configuration can select the OAuth2 token endpoint.
    
    When set to `false`:
    - The client must explicitly configure an `oauth2-server-uri` starting with 
`http://` or `https://`.
    - The client endpoint is preserved after merging server defaults and 
overrides, even when the server changes `uri`.
    
    Also align OAuth2 endpoint path handling with Java and reuse the resolved 
endpoint for refresh and child sessions. Add focused tests for these behaviors.
    
    The default config merge precedence remains unchanged.
---
 src/iceberg/catalog/rest/auth/auth_properties.cc |  31 -------
 src/iceberg/catalog/rest/catalog_properties.h    |   6 ++
 src/iceberg/catalog/rest/http_client.cc          |  47 +++++++---
 src/iceberg/catalog/rest/http_client.h           |   7 +-
 src/iceberg/catalog/rest/rest_catalog.cc         |  22 ++++-
 src/iceberg/catalog/rest/rest_catalog.h          |   3 +-
 src/iceberg/test/auth_manager_test.cc            |  78 +---------------
 src/iceberg/test/http_client_test.cc             |  28 ++++++
 src/iceberg/test/rest_catalog_unit_test.cc       | 113 +++++++++++++++++++++++
 9 files changed, 216 insertions(+), 119 deletions(-)

diff --git a/src/iceberg/catalog/rest/auth/auth_properties.cc 
b/src/iceberg/catalog/rest/auth/auth_properties.cc
index f373df61..2f6d1d9c 100644
--- a/src/iceberg/catalog/rest/auth/auth_properties.cc
+++ b/src/iceberg/catalog/rest/auth/auth_properties.cc
@@ -21,9 +21,6 @@
 
 #include <utility>
 
-#include "iceberg/catalog/rest/catalog_properties.h"
-#include "iceberg/catalog/rest/rest_util.h"
-
 namespace iceberg::rest::auth {
 
 namespace {
@@ -36,31 +33,6 @@ std::pair<std::string, std::string> ParseCredential(const 
std::string& credentia
   return {credential.substr(0, colon_pos), credential.substr(colon_pos + 1)};
 }
 
-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()
-                             ? AuthProperties::kOAuth2ServerUri.value()
-                             : endpoint_it->second;
-
-  if (endpoint.starts_with("http://";) || endpoint.starts_with("https://";)) {
-    return endpoint;
-  }
-  if (endpoint.empty()) {
-    return endpoint;
-  }
-  auto uri_it = properties.find(RestCatalogProperties::kUri.key());
-  if (uri_it == properties.end() || uri_it->second.empty()) {
-    return endpoint;
-  }
-
-  auto base_uri = std::string(TrimTrailingSlash(uri_it->second));
-  if (endpoint.starts_with('/')) {
-    return base_uri + endpoint;
-  }
-  return base_uri + "/" + endpoint;
-}
-
 }  // namespace
 
 std::unordered_map<std::string, std::string> 
AuthProperties::optional_oauth_params()
@@ -87,9 +59,6 @@ Result<AuthProperties> AuthProperties::FromProperties(
     config.client_secret_ = std::move(secret);
   }
 
-  ICEBERG_ASSIGN_OR_RAISE(auto oauth2_server_uri, 
ResolveOAuth2ServerUri(properties));
-  config.Set(kOAuth2ServerUri, std::move(oauth2_server_uri));
-
   // TODO(lishuxu): Parse JWT exp claim from token to set expires_at_millis_.
 
   return config;
diff --git a/src/iceberg/catalog/rest/catalog_properties.h 
b/src/iceberg/catalog/rest/catalog_properties.h
index a1f205a6..0b24af58 100644
--- a/src/iceberg/catalog/rest/catalog_properties.h
+++ b/src/iceberg/catalog/rest/catalog_properties.h
@@ -78,6 +78,12 @@ class ICEBERG_REST_EXPORT RestCatalogProperties
   /// \brief HTTP socket/read timeout in milliseconds, matching Java REST 
clients.
   inline static Entry<int64_t> 
kClientSocketTimeoutMs{"rest.client.socket-timeout-ms", 0};
 
+  /// \brief Whether server configuration may select the OAuth2 token endpoint.
+  /// When false, the client must explicitly configure an absolute http:// or 
https://
+  /// oauth2-server-uri. This setting is read from client configuration only.
+  inline static Entry<bool> 
kTrustServerOAuth2Uri{"rest.auth.oauth2.trust-server-uri",
+                                                  true};
+
   /// \brief Create a default RestCatalogProperties instance.
   static RestCatalogProperties default_properties();
 
diff --git a/src/iceberg/catalog/rest/http_client.cc 
b/src/iceberg/catalog/rest/http_client.cc
index 718a5b73..019f7820 100644
--- a/src/iceberg/catalog/rest/http_client.cc
+++ b/src/iceberg/catalog/rest/http_client.cc
@@ -192,25 +192,45 @@ Status HandleFailureResponse(const cpr::Response& 
response,
 
 HttpClient::HttpClient(std::unordered_map<std::string, std::string> 
default_headers,
                        std::chrono::milliseconds connection_timeout,
-                       std::chrono::milliseconds socket_timeout)
-    : default_headers_{std::move(default_headers)},
+                       std::chrono::milliseconds socket_timeout,
+                       std::optional<std::string> base_uri)
+    : base_uri_{std::move(base_uri)},
+      default_headers_{std::move(default_headers)},
       connection_timeout_{connection_timeout},
       socket_timeout_{socket_timeout},
       connection_pool_{std::make_unique<cpr::ConnectionPool>()} {
   // Set default Content-Type for all requests (including GET/HEAD/DELETE).
-  // Many systems require that content type is set regardless and will fail,
-  // even on an empty bodied request.
+  // Many systems require that content type is set regardless, even on an 
empty body.
   default_headers_[kHeaderContentType] = kMimeTypeApplicationJson;
   default_headers_[kHeaderUserAgent] = kUserAgent;
 }
 
 HttpClient::~HttpClient() = default;
 
+Result<std::string> HttpClient::RequestUri(const std::string& path) const {
+  // Match Java HTTPRequest.check, then requestUri. URI syntax validation 
remains
+  // the transport's responsibility rather than emulating java.net.URI.
+  ICEBERG_PRECHECK(!path.starts_with('/'), "Request paths must not start with 
'/'");
+  const bool has_absolute_path =
+      path.starts_with("https://";) || path.starts_with("http://";);
+  ICEBERG_PRECHECK(base_uri_.has_value() || has_absolute_path,
+                   "Relative request path requires a base URI");
+  std::string full_path;
+  if (has_absolute_path) {
+    full_path = path;
+  } else {
+    auto base_uri = TrimTrailingSlash(*base_uri_);
+    full_path = std::string(TrimTrailingSlash(std::string(base_uri) + "/" + 
path));
+  }
+  return full_path;
+}
+
 Result<HttpResponse> HttpClient::Get(
     const std::string& path, const std::unordered_map<std::string, 
std::string>& params,
     const std::unordered_map<std::string, std::string>& headers,
     const ErrorHandler& error_handler, auth::AuthSession& session) {
-  ICEBERG_ASSIGN_OR_RAISE(auto url, AppendQueryString(path, params));
+  ICEBERG_ASSIGN_OR_RAISE(auto request_uri, RequestUri(path));
+  ICEBERG_ASSIGN_OR_RAISE(auto url, AppendQueryString(request_uri, params));
   ICEBERG_ASSIGN_OR_RAISE(auto authenticated,
                           AuthenticateRequest(session, HttpMethod::kGet, 
std::move(url),
                                               MergeHeaders(default_headers_, 
headers)));
@@ -229,9 +249,10 @@ Result<HttpResponse> HttpClient::Post(
     const std::string& path, const std::string& body,
     const std::unordered_map<std::string, std::string>& headers,
     const ErrorHandler& error_handler, auth::AuthSession& session) {
+  ICEBERG_ASSIGN_OR_RAISE(auto request_uri, RequestUri(path));
   ICEBERG_ASSIGN_OR_RAISE(
       auto authenticated,
-      AuthenticateRequest(session, HttpMethod::kPost, path,
+      AuthenticateRequest(session, HttpMethod::kPost, std::move(request_uri),
                           MergeHeaders(default_headers_, headers), body));
   cpr::Response response =
       cpr::Post(cpr::Url{authenticated.url}, cpr::Body{authenticated.body},
@@ -249,6 +270,7 @@ Result<HttpResponse> HttpClient::PostForm(
     const std::unordered_map<std::string, std::string>& form_data,
     const std::unordered_map<std::string, std::string>& headers,
     const ErrorHandler& error_handler, auth::AuthSession& session) {
+  ICEBERG_ASSIGN_OR_RAISE(auto request_uri, RequestUri(path));
   std::unordered_map<std::string, std::string> form_headers(headers);
   form_headers.insert_or_assign(kHeaderContentType, kMimeTypeFormUrlEncoded);
   std::vector<cpr::Pair> pair_list;
@@ -261,7 +283,7 @@ Result<HttpResponse> HttpClient::PostForm(
       cpr::Payload(pair_list.begin(), pair_list.end()).GetContent();
   ICEBERG_ASSIGN_OR_RAISE(
       auto authenticated,
-      AuthenticateRequest(session, HttpMethod::kPost, path,
+      AuthenticateRequest(session, HttpMethod::kPost, std::move(request_uri),
                           MergeHeaders(default_headers_, form_headers),
                           std::move(encoded_body)));
   cpr::Response response =
@@ -278,9 +300,11 @@ Result<HttpResponse> HttpClient::PostForm(
 Result<HttpResponse> HttpClient::Head(
     const std::string& path, const std::unordered_map<std::string, 
std::string>& headers,
     const ErrorHandler& error_handler, auth::AuthSession& session) {
-  ICEBERG_ASSIGN_OR_RAISE(auto authenticated,
-                          AuthenticateRequest(session, HttpMethod::kHead, path,
-                                              MergeHeaders(default_headers_, 
headers)));
+  ICEBERG_ASSIGN_OR_RAISE(auto request_uri, RequestUri(path));
+  ICEBERG_ASSIGN_OR_RAISE(
+      auto authenticated,
+      AuthenticateRequest(session, HttpMethod::kHead, std::move(request_uri),
+                          MergeHeaders(default_headers_, headers)));
   cpr::Response response =
       cpr::Head(cpr::Url{authenticated.url}, ToCprHeader(authenticated),
                 kNoFollowRedirects, *connection_pool_,
@@ -296,7 +320,8 @@ Result<HttpResponse> HttpClient::Delete(
     const std::string& path, const std::unordered_map<std::string, 
std::string>& params,
     const std::unordered_map<std::string, std::string>& headers,
     const ErrorHandler& error_handler, auth::AuthSession& session) {
-  ICEBERG_ASSIGN_OR_RAISE(auto url, AppendQueryString(path, params));
+  ICEBERG_ASSIGN_OR_RAISE(auto request_uri, RequestUri(path));
+  ICEBERG_ASSIGN_OR_RAISE(auto url, AppendQueryString(request_uri, params));
   ICEBERG_ASSIGN_OR_RAISE(
       auto authenticated,
       AuthenticateRequest(session, HttpMethod::kDelete, std::move(url),
diff --git a/src/iceberg/catalog/rest/http_client.h 
b/src/iceberg/catalog/rest/http_client.h
index a1850640..9a245938 100644
--- a/src/iceberg/catalog/rest/http_client.h
+++ b/src/iceberg/catalog/rest/http_client.h
@@ -75,7 +75,8 @@ class ICEBERG_REST_EXPORT HttpClient {
   /// \param connection_timeout,socket_timeout Zero leaves requests unbounded.
   explicit HttpClient(std::unordered_map<std::string, std::string> 
default_headers = {},
                       std::chrono::milliseconds connection_timeout = {},
-                      std::chrono::milliseconds socket_timeout = {});
+                      std::chrono::milliseconds socket_timeout = {},
+                      std::optional<std::string> base_uri = std::nullopt);
   virtual ~HttpClient();
 
   HttpClient(const HttpClient&) = delete;
@@ -115,6 +116,10 @@ class ICEBERG_REST_EXPORT HttpClient {
       const ErrorHandler& error_handler, auth::AuthSession& session);
 
  private:
+  /// \brief Build a request URI from this client's base URI and a path.
+  Result<std::string> RequestUri(const std::string& path) const;
+
+  std::optional<std::string> base_uri_;
   std::unordered_map<std::string, std::string> default_headers_;
   std::chrono::milliseconds connection_timeout_;
   std::chrono::milliseconds socket_timeout_;
diff --git a/src/iceberg/catalog/rest/rest_catalog.cc 
b/src/iceberg/catalog/rest/rest_catalog.cc
index c8d88d06..a25d28eb 100644
--- a/src/iceberg/catalog/rest/rest_catalog.cc
+++ b/src/iceberg/catalog/rest/rest_catalog.cc
@@ -35,6 +35,7 @@
 #include <nlohmann/json.hpp>
 
 #include "iceberg/catalog/rest/auth/auth_managers.h"
+#include "iceberg/catalog/rest/auth/auth_properties.h"
 #include "iceberg/catalog/rest/auth/auth_session.h"
 #include "iceberg/catalog/rest/catalog_properties.h"
 #include "iceberg/catalog/rest/constant.h"
@@ -456,14 +457,27 @@ Result<std::shared_ptr<RestCatalog>> RestCatalog::Make(
       const auto timeout_of = [&properties](const auto& entry) {
         return std::chrono::milliseconds(std::max<int64_t>(0, 
properties.Get(entry)));
       };
+      ICEBERG_ASSIGN_OR_RAISE(auto uri, properties.Uri());
       return std::make_shared<HttpClient>(
           properties.ExtractHeaders(),
           timeout_of(RestCatalogProperties::kClientConnectionTimeoutMs),
-          timeout_of(RestCatalogProperties::kClientSocketTimeoutMs));
+          timeout_of(RestCatalogProperties::kClientSocketTimeoutMs), 
std::string(uri));
     };
   }
   ICEBERG_ASSIGN_OR_RAISE(auto uri, config.Uri());
 
+  const bool trust_server_oauth2_uri =
+      config.Get(RestCatalogProperties::kTrustServerOAuth2Uri);
+  const auto endpoint =
+      config.configs().find(auth::AuthProperties::kOAuth2ServerUri.key());
+  if (!trust_server_oauth2_uri) {
+    ICEBERG_PRECHECK(
+        endpoint != config.configs().end() && 
(endpoint->second.starts_with("https://";) ||
+                                               
endpoint->second.starts_with("http://";)),
+        "An explicit absolute http:// or https:// oauth2-server-uri is 
required when "
+        "rest.auth.oauth2.trust-server-uri is false");
+  }
+
   std::string catalog_name = config.Get(RestCatalogProperties::kName);
   ICEBERG_ASSIGN_OR_RAISE(auto auth_manager,
                           auth::AuthManagers::Load(catalog_name, 
config.configs()));
@@ -484,6 +498,12 @@ Result<std::shared_ptr<RestCatalog>> RestCatalog::Make(
 
   RestCatalogProperties final_config = RestCatalogProperties::FromMap(
       MergeConfigs(server_config.defaults, config.configs(), 
server_config.overrides));
+  // The server cannot change the client's endpoint trust policy or, when 
disabled,
+  // move the OAuth endpoint directly or through an override of the catalog 
URI.
+  final_config.Set(RestCatalogProperties::kTrustServerOAuth2Uri, 
trust_server_oauth2_uri);
+  if (!trust_server_oauth2_uri) {
+    final_config.mutable_configs().insert_or_assign(endpoint->first, 
endpoint->second);
+  }
 
   std::unordered_set<Endpoint> endpoints;
   if (!server_config.endpoints.empty()) {
diff --git a/src/iceberg/catalog/rest/rest_catalog.h 
b/src/iceberg/catalog/rest/rest_catalog.h
index d648fa76..561ff347 100644
--- a/src/iceberg/catalog/rest/rest_catalog.h
+++ b/src/iceberg/catalog/rest/rest_catalog.h
@@ -59,7 +59,8 @@ class ICEBERG_REST_EXPORT RestCatalog final
 
   /// \brief Create a RestCatalog instance.
   /// \param client_factory Optional HTTP client factory, called with the 
initial config
-  /// and then the merged server config. When empty, uses the default HTTP 
client.
+  /// and then the merged server config. It should configure the HTTP client's 
base URI
+  /// from the supplied properties. When empty, uses the default HTTP client.
   /// \param auth_executor Borrowed OAuth2 refresh executor. With OAuth2 
credentials,
   /// token-refresh-enabled=true (default) requires it; null returns 
InvalidArgument.
   /// Must enqueue asynchronously and outlive the catalog, its auth sessions 
and their
diff --git a/src/iceberg/test/auth_manager_test.cc 
b/src/iceberg/test/auth_manager_test.cc
index f6f1400b..0bae4306 100644
--- a/src/iceberg/test/auth_manager_test.cc
+++ b/src/iceberg/test/auth_manager_test.cc
@@ -75,80 +75,6 @@ class AuthManagerTest : public ::testing::Test {
   std::shared_ptr<HttpClient> client_ = std::make_shared<HttpClient>();
 };
 
-TEST(AuthPropertiesTest, ResolvesDefaultOAuth2ServerUri) {
-  ICEBERG_UNWRAP_OR_FAIL(
-      auto config,
-      AuthProperties::FromProperties({
-          {RestCatalogProperties::kUri.key(), 
"https://catalog.example.com/api/"},
-          {RestCatalogProperties::kPrefix.key(), "warehouse"},
-      }));
-
-  EXPECT_EQ(config.oauth2_server_uri(),
-            "https://catalog.example.com/api/v1/oauth/tokens";);
-}
-
-TEST(AuthPropertiesTest, PreservesEmptyOAuth2ServerUri) {
-  ICEBERG_UNWRAP_OR_FAIL(
-      auto config, AuthProperties::FromProperties({
-                       {RestCatalogProperties::kUri.key(), 
"https://catalog.example.com"},
-                       {AuthProperties::kOAuth2ServerUri.key(), ""},
-                   }));
-
-  EXPECT_TRUE(config.oauth2_server_uri().empty());
-}
-
-TEST(AuthPropertiesTest, ResolvesExplicitRelativeOAuth2ServerUri) {
-  ICEBERG_UNWRAP_OR_FAIL(
-      auto config,
-      AuthProperties::FromProperties({
-          {RestCatalogProperties::kUri.key(), 
"https://catalog.example.com/api/"},
-          {AuthProperties::kOAuth2ServerUri.key(), "oauth/token/"},
-      }));
-
-  EXPECT_EQ(config.oauth2_server_uri(), 
"https://catalog.example.com/api/oauth/token/";);
-}
-
-TEST(AuthPropertiesTest, PreservesExplicitAbsoluteOAuth2ServerUri) {
-  ICEBERG_UNWRAP_OR_FAIL(
-      auto config,
-      AuthProperties::FromProperties({
-          {RestCatalogProperties::kUri.key(), "https://catalog.example.com"},
-          {AuthProperties::kOAuth2ServerUri.key(), 
"https://auth.example.com/token/"},
-      }));
-
-  EXPECT_EQ(config.oauth2_server_uri(), "https://auth.example.com/token/";);
-}
-
-TEST(AuthPropertiesTest, PreservesRelativeOAuth2ServerUriWithoutCatalogUri) {
-  ICEBERG_UNWRAP_OR_FAIL(auto config,
-                         AuthProperties::FromProperties({
-                             {AuthProperties::kOAuth2ServerUri.key(), 
"oauth/token"},
-                         }));
-
-  EXPECT_EQ(config.oauth2_server_uri(), "oauth/token");
-}
-
-TEST(AuthPropertiesTest, PreservesAbsolutePathWithoutCatalogUri) {
-  ICEBERG_UNWRAP_OR_FAIL(auto config,
-                         AuthProperties::FromProperties({
-                             {AuthProperties::kOAuth2ServerUri.key(), 
"/oauth/token"},
-                         }));
-
-  EXPECT_EQ(config.oauth2_server_uri(), "/oauth/token");
-}
-
-TEST(AuthPropertiesTest, ResolvesOAuth2ServerUriWithLeadingSlash) {
-  ICEBERG_UNWRAP_OR_FAIL(
-      auto config,
-      AuthProperties::FromProperties({
-          {RestCatalogProperties::kUri.key(), 
"https://catalog.example.com/api/"},
-          {AuthProperties::kOAuth2ServerUri.key(), "/v1/oauth/tokens"},
-      }));
-
-  EXPECT_EQ(config.oauth2_server_uri(),
-            "https://catalog.example.com/api/v1/oauth/tokens";);
-}
-
 // Verifies loading NoopAuthManager with explicit "none" auth type
 TEST_F(AuthManagerTest, LoadNoopAuthManagerExplicit) {
   std::unordered_map<std::string, std::string> properties = {
@@ -437,6 +363,7 @@ TEST_F(AuthManagerTest, OAuth2StaticToken) {
 TEST_F(AuthManagerTest, OAuth2InferredFromToken) {
   std::unordered_map<std::string, std::string> properties = {
       {AuthProperties::kToken.key(), "inferred-token"},
+      {RestCatalogProperties::kUri.key(), "https://catalog.example.com"},
   };
 
   auto manager_result = AuthManagers::Load("test-catalog", properties);
@@ -455,6 +382,7 @@ TEST_F(AuthManagerTest, OAuth2InferredFromToken) {
 TEST_F(AuthManagerTest, OAuth2MissingCredentials) {
   std::unordered_map<std::string, std::string> properties = {
       {AuthProperties::kAuthType, "oauth2"},
+      {RestCatalogProperties::kUri.key(), "https://catalog.example.com"},
   };
 
   auto manager_result = AuthManagers::Load("test-catalog", properties);
@@ -514,6 +442,7 @@ TEST_F(AuthManagerTest, 
OAuth2ContextTokenCreatesChildAndHasPriority) {
 TEST_F(AuthManagerTest, OAuth2ContextTypedTokenOnlyUsesCredentials) {
   std::unordered_map<std::string, std::string> properties = {
       {AuthProperties::kAuthType, "oauth2"},
+      {RestCatalogProperties::kUri.key(), "https://catalog.example.com"},
   };
   ICEBERG_UNWRAP_OR_FAIL(auto manager, AuthManagers::Load("test-catalog", 
properties));
   ICEBERG_UNWRAP_OR_FAIL(auto parent, manager->CatalogSession(client_, 
properties));
@@ -557,6 +486,7 @@ TEST_F(AuthManagerTest, OAuth2TableTokenCreatesChild) {
 TEST_F(AuthManagerTest, OAuth2TableIgnoresCredential) {
   std::unordered_map<std::string, std::string> properties = {
       {AuthProperties::kAuthType, "oauth2"},
+      {RestCatalogProperties::kUri.key(), "https://catalog.example.com"},
   };
   ICEBERG_UNWRAP_OR_FAIL(auto manager, AuthManagers::Load("test-catalog", 
properties));
   ICEBERG_UNWRAP_OR_FAIL(auto parent, manager->CatalogSession(client_, 
properties));
diff --git a/src/iceberg/test/http_client_test.cc 
b/src/iceberg/test/http_client_test.cc
index 9e503729..6cc65313 100644
--- a/src/iceberg/test/http_client_test.cc
+++ b/src/iceberg/test/http_client_test.cc
@@ -314,6 +314,34 @@ TEST(HttpClientTest, 
PostReportsTransferFailureAfterSuccessfulStatus) {
 
 #endif
 
+class CapturingAuthSession final : public auth::AuthSession {
+ public:
+  Result<HttpRequest> Authenticate(HttpRequest request) override {
+    request_ = std::move(request);
+    return InvalidArgument("captured");
+  }
+
+  const HttpRequest& request() const { return request_; }
+
+ private:
+  HttpRequest request_;
+};
+
+TEST(HttpClientTest, BuildsRequestUriLikeJavaHttpRequest) {
+  HttpClient client({}, {}, {}, std::string("https://catalog.example/api/";));
+  CapturingAuthSession session;
+
+  auto relative = client.PostForm("v1/oauth/tokens///", {}, {},
+                                  *DefaultErrorHandler::Instance(), session);
+  EXPECT_THAT(relative, IsError(ErrorKind::kInvalidArgument));
+  EXPECT_EQ(session.request().url, 
"https://catalog.example/api/v1/oauth/tokens";);
+
+  auto absolute = client.PostForm("https://auth.example/token/";, {}, {},
+                                  *DefaultErrorHandler::Instance(), session);
+  EXPECT_THAT(absolute, IsError(ErrorKind::kInvalidArgument));
+  EXPECT_EQ(session.request().url, "https://auth.example/token/";);
+}
+
 // Redirect responses must be reported without replaying the request at the 
target.
 TEST(HttpClientTest, DoesNotFollowRedirects) {
 #ifdef _WIN32
diff --git a/src/iceberg/test/rest_catalog_unit_test.cc 
b/src/iceberg/test/rest_catalog_unit_test.cc
index 75bea714..1b162763 100644
--- a/src/iceberg/test/rest_catalog_unit_test.cc
+++ b/src/iceberg/test/rest_catalog_unit_test.cc
@@ -17,6 +17,8 @@
  * under the License.
  */
 
+#include <chrono>
+#include <future>
 #include <memory>
 #include <optional>
 #include <string>
@@ -29,6 +31,7 @@
 #include <nlohmann/json.hpp>
 
 #include "iceberg/catalog.h"
+#include "iceberg/catalog/rest/auth/auth_properties.h"
 #include "iceberg/catalog/rest/auth/auth_session.h"
 #include "iceberg/catalog/rest/catalog_properties.h"
 #include "iceberg/catalog/rest/endpoint.h"
@@ -39,6 +42,7 @@
 #include "iceberg/catalog/rest/rest_table.h"
 #include "iceberg/catalog/rest/rest_table_scan.h"
 #include "iceberg/catalog/rest/types.h"
+#include "iceberg/catalog/session_context.h"
 #include "iceberg/file_io.h"
 #include "iceberg/file_io_registry.h"
 #include "iceberg/labels.h"
@@ -614,6 +618,115 @@ TEST_P(RestCatalogScanLifetimeTest, 
ScanKeepsCredentialRefreshAlive) {
 
 INSTANTIATE_TEST_SUITE_P(ScanModes, RestCatalogScanLifetimeTest, 
::testing::Bool());
 
+class RestCatalogOAuth2UriTest : public RestCatalogLoadTableTest {
+ protected:
+  void SetUp() override {
+    RestCatalogLoadTableTest::SetUp();
+    init_client_ = std::make_shared<::testing::StrictMock<MockHttpClient>>();
+    mock_client_ = std::make_shared<::testing::StrictMock<MockHttpClient>>();
+    properties_ = {{"uri", "https://catalog.example/api"},
+                   {"credential", "client:secret"},
+                   {"oauth2-server-uri", "https://auth.example/token/"},
+                   {"token-refresh-enabled", "false"},
+                   {"io-impl", "rest-catalog-unit-test-noop"}};
+  }
+
+  void ExpectInit(const CatalogConfig& server_config,
+                  const std::string& token_response =
+                      R"({"access_token":"init","token_type":"bearer"})") {
+    EXPECT_CALL(*init_client_, PostForm("https://auth.example/token/";, _, _, 
_, _))
+        .WillOnce(Return(HttpResponse(200, token_response)));
+    EXPECT_CALL(*init_client_, Get("https://catalog.example/api/v1/config";, _, 
_, _, _))
+        .WillOnce(Return(HttpResponse(200, ToJson(server_config).dump())));
+  }
+
+  Result<std::shared_ptr<RestCatalog>> MakeOAuth2Catalog(Executor* executor = 
nullptr) {
+    return RestCatalog::Make(
+        RestCatalogProperties::FromMap(properties_),
+        [this](const RestCatalogProperties&) -> 
Result<std::shared_ptr<HttpClient>> {
+          return ++factory_calls_ == 1 ? init_client_ : mock_client_;
+        },
+        executor);
+  }
+
+  void CheckContextualRequest(const std::shared_ptr<RestCatalog>& root,
+                              const std::string& endpoint, const std::string& 
catalog_uri,
+                              const std::string& credential_type = 
"credential") {
+    EXPECT_CALL(*mock_client_, PostForm(endpoint, _, _, _, _))
+        .WillOnce(Return(
+            HttpResponse(200, 
R"({"access_token":"context","token_type":"bearer"})")));
+    EXPECT_CALL(*mock_client_, Get(catalog_uri + "/v1/namespaces", _, _, _, _))
+        .WillOnce(Return(HttpResponse(200, R"({"namespaces":[]})")));
+    ICEBERG_UNWRAP_OR_FAIL(
+        auto scoped,
+        root->WithContext({.session_id = "context",
+                           .credentials = {{credential_type, 
"user:context-secret"}}}));
+    EXPECT_THAT(scoped->ListNamespaces({}), IsOk());
+  }
+
+  std::shared_ptr<MockHttpClient> init_client_;
+  std::unordered_map<std::string, std::string> properties_;
+  int factory_calls_ = 0;
+};
+
+TEST_F(RestCatalogOAuth2UriTest, TrustsServerOverridesByDefault) {
+  ExpectInit({.overrides = {{"uri", "HTTPS://other-catalog.example/base/"},
+                            {"oauth2-server-uri", "oauth/token///"},
+                            {"rest.auth.oauth2.trust-server-uri", "false"}}});
+  ICEBERG_UNWRAP_OR_FAIL(auto root, MakeOAuth2Catalog());
+  CheckContextualRequest(root, "oauth/token///", 
"HTTPS://other-catalog.example/base",
+                         auth::AuthProperties::kIdTokenType);
+}
+
+TEST_F(RestCatalogOAuth2UriTest, 
RequiresAbsoluteClientEndpointBeforeAnyIoWhenUntrusted) {
+  properties_["rest.auth.oauth2.trust-server-uri"] = "false";
+  properties_.erase("oauth2-server-uri");
+  EXPECT_THAT(MakeOAuth2Catalog(), IsError(ErrorKind::kInvalidArgument));
+  for (const auto* endpoint :
+       {"", "token", "/token", "//auth.example/token", 
"ftp://auth.example/token";,
+        "HTTPS://auth.example/token"}) {
+    SCOPED_TRACE(endpoint);
+    properties_["oauth2-server-uri"] = endpoint;
+    EXPECT_THAT(MakeOAuth2Catalog(), IsError(ErrorKind::kInvalidArgument));
+  }
+  EXPECT_EQ(factory_calls_, 0);
+}
+
+TEST_F(RestCatalogOAuth2UriTest, RefreshUsesTheClientEndpointWhenUntrusted) {
+  test::MockExecutor executor;
+  std::promise<ExecutorTask> queued;
+  auto ready = queued.get_future();
+  EXPECT_CALL(executor, Submit).WillOnce([&](ExecutorTask task) -> Status {
+    queued.set_value(std::move(task));
+    return {};
+  });
+  properties_["rest.auth.oauth2.trust-server-uri"] = "false";
+  properties_["token-refresh-enabled"] = "true";
+  ExpectInit({.overrides = {{"uri", "https://other-catalog.example"},
+                            {"oauth2-server-uri", 
"https://server-auth.example/token"},
+                            {"rest.auth.oauth2.trust-server-uri", "true"}}},
+             
R"({"access_token":"expired","token_type":"bearer","expires_in":0})");
+  EXPECT_CALL(*mock_client_, PostForm("https://auth.example/token/";, _, _, _, 
_))
+      .WillOnce(Return(
+          HttpResponse(200, 
R"({"access_token":"refreshed","token_type":"bearer"})")));
+  ICEBERG_UNWRAP_OR_FAIL(auto root, MakeOAuth2Catalog(&executor));
+  ASSERT_EQ(ready.wait_for(std::chrono::seconds(5)), 
std::future_status::ready);
+  std::move(ready.get())();
+  EXPECT_CALL(*mock_client_,
+              Get("https://other-catalog.example/v1/namespaces";, _, _, _, _))
+      .WillOnce(
+          [](const std::string&, const std::unordered_map<std::string, 
std::string>&,
+             const std::unordered_map<std::string, std::string>&, const 
ErrorHandler&,
+             auth::AuthSession& session) -> Result<HttpResponse> {
+            EXPECT_EQ(session.OAuth2Info()->token, "refreshed");
+            return HttpResponse(200, R"({"namespaces":[]})");
+          });
+  ICEBERG_UNWRAP_OR_FAIL(auto catalog, root->AsCatalog());
+  EXPECT_THAT(catalog->ListNamespaces({}), IsOk());
+  CheckContextualRequest(root, "https://auth.example/token/";,
+                         "https://other-catalog.example";);
+}
+
 }  // namespace
 
 }  // namespace iceberg::rest

Reply via email to