From 47167aaa4c0516c8aa9168a6e0963f1ea979b4f2 Mon Sep 17 00:00:00 2001 From: "shuxu.li" Date: Thu, 29 Jan 2026 17:40:30 +0800 Subject: [PATCH 1/3] feat: Implement NoopAuthManager and integrate AuthManager into RestCatalog --- .../catalog/rest/auth/auth_managers.cc | 42 +++++- src/iceberg/catalog/rest/rest_catalog.cc | 107 ++++++++++---- src/iceberg/catalog/rest/rest_catalog.h | 11 +- src/iceberg/test/CMakeLists.txt | 1 + src/iceberg/test/auth_manager_test.cc | 137 ++++++++++++++++++ 5 files changed, 268 insertions(+), 30 deletions(-) create mode 100644 src/iceberg/test/auth_manager_test.cc diff --git a/src/iceberg/catalog/rest/auth/auth_managers.cc b/src/iceberg/catalog/rest/auth/auth_managers.cc index ab076e97f..8f0dd584b 100644 --- a/src/iceberg/catalog/rest/auth/auth_managers.cc +++ b/src/iceberg/catalog/rest/auth/auth_managers.cc @@ -21,8 +21,10 @@ #include #include +#include #include "iceberg/catalog/rest/auth/auth_properties.h" +#include "iceberg/catalog/rest/auth/auth_session.h" #include "iceberg/util/string_util.h" namespace iceberg::rest::auth { @@ -33,6 +35,17 @@ namespace { using AuthManagerRegistry = std::unordered_map; +/// \brief Known authentication types that are defined in the Iceberg spec. +const std::unordered_set& KnownAuthTypes() { + static const std::unordered_set types = { + AuthProperties::kAuthTypeNone, + AuthProperties::kAuthTypeBasic, + AuthProperties::kAuthTypeOAuth2, + AuthProperties::kAuthTypeSigV4, + }; + return types; +} + // Infer the authentication type from properties. std::string InferAuthType( const std::unordered_map& properties) { @@ -51,9 +64,30 @@ std::string InferAuthType( return AuthProperties::kAuthTypeNone; } +/// \brief Authentication manager that performs no authentication. +class NoopAuthManager : public AuthManager { + public: + Result> CatalogSession( + [[maybe_unused]] HttpClient& shared_client, + [[maybe_unused]] const std::unordered_map& properties) + override { + return AuthSession::MakeDefault({}); + } +}; + // Get the global registry of auth manager factories. AuthManagerRegistry& GetRegistry() { - static AuthManagerRegistry registry; + static AuthManagerRegistry registry = [] { + AuthManagerRegistry r; + // Register built-in "none" auth manager + r[AuthProperties::kAuthTypeNone] = + []([[maybe_unused]] std::string_view name, + [[maybe_unused]] const std::unordered_map& props) + -> Result> { + return std::make_unique(); + }; + return r; + }(); return registry; } @@ -71,8 +105,10 @@ Result> AuthManagers::Load( auto& registry = GetRegistry(); auto it = registry.find(auth_type); if (it == registry.end()) { - // TODO(Li Shuxu): Fallback to default auth manager implementations - return NotImplemented("Authentication type '{}' is not supported", auth_type); + if (KnownAuthTypes().contains(auth_type)) { + return NotImplemented("Authentication type '{}' is not yet supported", auth_type); + } + return InvalidArgument("Unknown authentication type: '{}'", auth_type); } return it->second(name, properties); diff --git a/src/iceberg/catalog/rest/rest_catalog.cc b/src/iceberg/catalog/rest/rest_catalog.cc index cc052e241..d645a74b6 100644 --- a/src/iceberg/catalog/rest/rest_catalog.cc +++ b/src/iceberg/catalog/rest/rest_catalog.cc @@ -26,6 +26,9 @@ #include +#include "iceberg/catalog/rest/auth/auth_manager.h" +#include "iceberg/catalog/rest/auth/auth_managers.h" +#include "iceberg/catalog/rest/auth/auth_session.h" #include "iceberg/catalog/rest/catalog_properties.h" #include "iceberg/catalog/rest/constant.h" #include "iceberg/catalog/rest/endpoint.h" @@ -66,12 +69,18 @@ std::unordered_set GetDefaultEndpoints() { } /// \brief Fetch server config and merge it with client config -Result FetchServerConfig(const ResourcePaths& paths, - const RestCatalogProperties& current_config) { +Result FetchServerConfig( + const ResourcePaths& paths, const RestCatalogProperties& current_config, + const std::shared_ptr& session) { ICEBERG_ASSIGN_OR_RAISE(auto config_path, paths.Config()); HttpClient client(current_config.ExtractHeaders()); + + // Get authentication headers + std::unordered_map auth_headers; + ICEBERG_RETURN_UNEXPECTED(session->Authenticate(auth_headers)); + ICEBERG_ASSIGN_OR_RAISE(const auto response, - client.Get(config_path, /*params=*/{}, /*headers=*/{}, + client.Get(config_path, /*params=*/{}, auth_headers, *DefaultErrorHandler::Instance())); ICEBERG_ASSIGN_OR_RAISE(auto json, FromJsonString(response.body())); return CatalogConfigFromJson(json); @@ -114,11 +123,23 @@ Result> RestCatalog::Make( if (!file_io) { return InvalidArgument("FileIO is required to create RestCatalog"); } + + std::string catalog_name = config.Get(RestCatalogProperties::kName); + ICEBERG_ASSIGN_OR_RAISE(auto auth_manager, + auth::AuthManagers::Load(catalog_name, config.configs())); + ICEBERG_ASSIGN_OR_RAISE( auto paths, ResourcePaths::Make(std::string(TrimTrailingSlash(uri)), config.Get(RestCatalogProperties::kPrefix))); - ICEBERG_ASSIGN_OR_RAISE(auto server_config, FetchServerConfig(*paths, config)); + // Create init session for fetching server configuration + HttpClient init_client(config.ExtractHeaders()); + ICEBERG_ASSIGN_OR_RAISE(auto init_session, + auth_manager->InitSession(init_client, config.configs())); + ICEBERG_ASSIGN_OR_RAISE(auto server_config, + FetchServerConfig(*paths, config, init_session)); + + // Merge client config with server defaults and overrides std::unique_ptr final_config = RestCatalogProperties::FromMap( MergeConfigs(server_config.defaults, config.configs(), server_config.overrides)); @@ -139,27 +160,43 @@ Result> RestCatalog::Make( paths, ResourcePaths::Make(std::string(TrimTrailingSlash(final_uri)), final_config->Get(RestCatalogProperties::kPrefix))); - return std::shared_ptr( - new RestCatalog(std::move(final_config), std::move(file_io), std::move(paths), - std::move(endpoints))); + auto client = std::make_unique(final_config->ExtractHeaders()); + ICEBERG_ASSIGN_OR_RAISE(auto catalog_session, + auth_manager->CatalogSession(*client, final_config->configs())); + return std::shared_ptr(new RestCatalog( + std::move(final_config), std::move(file_io), std::move(client), std::move(paths), + std::move(endpoints), std::move(auth_manager), std::move(catalog_session))); } RestCatalog::RestCatalog(std::unique_ptr config, std::shared_ptr file_io, + std::unique_ptr client, std::unique_ptr paths, - std::unordered_set endpoints) + std::unordered_set endpoints, + std::unique_ptr auth_manager, + std::shared_ptr catalog_session) : config_(std::move(config)), file_io_(std::move(file_io)), - client_(std::make_unique(config_->ExtractHeaders())), + client_(std::move(client)), paths_(std::move(paths)), name_(config_->Get(RestCatalogProperties::kName)), - supported_endpoints_(std::move(endpoints)) {} + supported_endpoints_(std::move(endpoints)), + auth_manager_(std::move(auth_manager)), + catalog_session_(std::move(catalog_session)) {} std::string_view RestCatalog::name() const { return name_; } +Result> RestCatalog::AuthHeaders() const { + std::unordered_map headers; + ICEBERG_RETURN_UNEXPECTED(catalog_session_->Authenticate(headers)); + return headers; +} + Result> RestCatalog::ListNamespaces(const Namespace& ns) const { ICEBERG_ENDPOINT_CHECK(supported_endpoints_, Endpoint::ListNamespaces()); ICEBERG_ASSIGN_OR_RAISE(auto path, paths_->Namespaces()); + ICEBERG_ASSIGN_OR_RAISE(auto auth_headers, AuthHeaders()); + std::vector result; std::string next_token; while (true) { @@ -172,7 +209,7 @@ Result> RestCatalog::ListNamespaces(const Namespace& ns) } ICEBERG_ASSIGN_OR_RAISE( const auto response, - client_->Get(path, params, /*headers=*/{}, *NamespaceErrorHandler::Instance())); + client_->Get(path, params, auth_headers, *NamespaceErrorHandler::Instance())); ICEBERG_ASSIGN_OR_RAISE(auto json, FromJsonString(response.body())); ICEBERG_ASSIGN_OR_RAISE(auto list_response, ListNamespacesResponseFromJson(json)); result.insert(result.end(), list_response.namespaces.begin(), @@ -189,10 +226,12 @@ Status RestCatalog::CreateNamespace( const Namespace& ns, const std::unordered_map& properties) { ICEBERG_ENDPOINT_CHECK(supported_endpoints_, Endpoint::CreateNamespace()); ICEBERG_ASSIGN_OR_RAISE(auto path, paths_->Namespaces()); + ICEBERG_ASSIGN_OR_RAISE(auto auth_headers, AuthHeaders()); + CreateNamespaceRequest request{.namespace_ = ns, .properties = properties}; ICEBERG_ASSIGN_OR_RAISE(auto json_request, ToJsonString(ToJson(request))); ICEBERG_ASSIGN_OR_RAISE(const auto response, - client_->Post(path, json_request, /*headers=*/{}, + client_->Post(path, json_request, auth_headers, *NamespaceErrorHandler::Instance())); ICEBERG_ASSIGN_OR_RAISE(auto json, FromJsonString(response.body())); ICEBERG_ASSIGN_OR_RAISE(auto create_response, CreateNamespaceResponseFromJson(json)); @@ -203,8 +242,10 @@ Result> RestCatalog::GetNamespacePr const Namespace& ns) const { ICEBERG_ENDPOINT_CHECK(supported_endpoints_, Endpoint::GetNamespaceProperties()); ICEBERG_ASSIGN_OR_RAISE(auto path, paths_->Namespace_(ns)); + ICEBERG_ASSIGN_OR_RAISE(auto auth_headers, AuthHeaders()); + ICEBERG_ASSIGN_OR_RAISE(const auto response, - client_->Get(path, /*params=*/{}, /*headers=*/{}, + client_->Get(path, /*params=*/{}, auth_headers, *NamespaceErrorHandler::Instance())); ICEBERG_ASSIGN_OR_RAISE(auto json, FromJsonString(response.body())); ICEBERG_ASSIGN_OR_RAISE(auto get_response, GetNamespaceResponseFromJson(json)); @@ -214,8 +255,10 @@ Result> RestCatalog::GetNamespacePr Status RestCatalog::DropNamespace(const Namespace& ns) { ICEBERG_ENDPOINT_CHECK(supported_endpoints_, Endpoint::DropNamespace()); ICEBERG_ASSIGN_OR_RAISE(auto path, paths_->Namespace_(ns)); + ICEBERG_ASSIGN_OR_RAISE(auto auth_headers, AuthHeaders()); + ICEBERG_ASSIGN_OR_RAISE(const auto response, - client_->Delete(path, /*params=*/{}, /*headers=*/{}, + client_->Delete(path, /*params=*/{}, auth_headers, *DropNamespaceErrorHandler::Instance())); return {}; } @@ -227,8 +270,10 @@ Result RestCatalog::NamespaceExists(const Namespace& ns) const { } ICEBERG_ASSIGN_OR_RAISE(auto path, paths_->Namespace_(ns)); + ICEBERG_ASSIGN_OR_RAISE(auto auth_headers, AuthHeaders()); + return CaptureNoSuchNamespace( - client_->Head(path, /*headers=*/{}, *NamespaceErrorHandler::Instance())); + client_->Head(path, auth_headers, *NamespaceErrorHandler::Instance())); } Status RestCatalog::UpdateNamespaceProperties( @@ -236,12 +281,14 @@ Status RestCatalog::UpdateNamespaceProperties( const std::unordered_set& removals) { ICEBERG_ENDPOINT_CHECK(supported_endpoints_, Endpoint::UpdateNamespace()); ICEBERG_ASSIGN_OR_RAISE(auto path, paths_->NamespaceProperties(ns)); + ICEBERG_ASSIGN_OR_RAISE(auto auth_headers, AuthHeaders()); + UpdateNamespacePropertiesRequest request{ .removals = std::vector(removals.begin(), removals.end()), .updates = updates}; ICEBERG_ASSIGN_OR_RAISE(auto json_request, ToJsonString(ToJson(request))); ICEBERG_ASSIGN_OR_RAISE(const auto response, - client_->Post(path, json_request, /*headers=*/{}, + client_->Post(path, json_request, auth_headers, *NamespaceErrorHandler::Instance())); ICEBERG_ASSIGN_OR_RAISE(auto json, FromJsonString(response.body())); ICEBERG_ASSIGN_OR_RAISE(auto update_response, @@ -251,8 +298,9 @@ Status RestCatalog::UpdateNamespaceProperties( Result> RestCatalog::ListTables(const Namespace& ns) const { ICEBERG_ENDPOINT_CHECK(supported_endpoints_, Endpoint::ListTables()); - ICEBERG_ASSIGN_OR_RAISE(auto path, paths_->Tables(ns)); + ICEBERG_ASSIGN_OR_RAISE(auto auth_headers, AuthHeaders()); + std::vector result; std::string next_token; while (true) { @@ -262,7 +310,7 @@ Result> RestCatalog::ListTables(const Namespace& ns } ICEBERG_ASSIGN_OR_RAISE( const auto response, - client_->Get(path, params, /*headers=*/{}, *TableErrorHandler::Instance())); + client_->Get(path, params, auth_headers, *TableErrorHandler::Instance())); ICEBERG_ASSIGN_OR_RAISE(auto json, FromJsonString(response.body())); ICEBERG_ASSIGN_OR_RAISE(auto list_response, ListTablesResponseFromJson(json)); result.insert(result.end(), list_response.identifiers.begin(), @@ -282,6 +330,7 @@ Result RestCatalog::CreateTableInternal( const std::unordered_map& properties, bool stage_create) { ICEBERG_ENDPOINT_CHECK(supported_endpoints_, Endpoint::CreateTable()); ICEBERG_ASSIGN_OR_RAISE(auto path, paths_->Tables(identifier.ns)); + ICEBERG_ASSIGN_OR_RAISE(auto auth_headers, AuthHeaders()); CreateTableRequest request{ .name = identifier.name, @@ -296,7 +345,7 @@ Result RestCatalog::CreateTableInternal( ICEBERG_ASSIGN_OR_RAISE(auto json_request, ToJsonString(ToJson(request))); ICEBERG_ASSIGN_OR_RAISE( const auto response, - client_->Post(path, json_request, /*headers=*/{}, *TableErrorHandler::Instance())); + client_->Post(path, json_request, auth_headers, *TableErrorHandler::Instance())); ICEBERG_ASSIGN_OR_RAISE(auto json, FromJsonString(response.body())); return LoadTableResultFromJson(json); @@ -320,6 +369,7 @@ Result> RestCatalog::UpdateTable( const std::vector>& updates) { ICEBERG_ENDPOINT_CHECK(supported_endpoints_, Endpoint::UpdateTable()); ICEBERG_ASSIGN_OR_RAISE(auto path, paths_->Table(identifier)); + ICEBERG_ASSIGN_OR_RAISE(auto auth_headers, AuthHeaders()); CommitTableRequest request{.identifier = identifier}; request.requirements.reserve(requirements.size()); @@ -334,7 +384,7 @@ Result> RestCatalog::UpdateTable( ICEBERG_ASSIGN_OR_RAISE(auto json_request, ToJsonString(ToJson(request))); ICEBERG_ASSIGN_OR_RAISE( const auto response, - client_->Post(path, json_request, /*headers=*/{}, *TableErrorHandler::Instance())); + client_->Post(path, json_request, auth_headers, *TableErrorHandler::Instance())); ICEBERG_ASSIGN_OR_RAISE(auto json, FromJsonString(response.body())); ICEBERG_ASSIGN_OR_RAISE(auto commit_response, CommitTableResponseFromJson(json)); @@ -363,6 +413,7 @@ Result> RestCatalog::StageCreateTable( Status RestCatalog::DropTable(const TableIdentifier& identifier, bool purge) { ICEBERG_ENDPOINT_CHECK(supported_endpoints_, Endpoint::DeleteTable()); ICEBERG_ASSIGN_OR_RAISE(auto path, paths_->Table(identifier)); + ICEBERG_ASSIGN_OR_RAISE(auto auth_headers, AuthHeaders()); std::unordered_map params; if (purge) { @@ -370,7 +421,7 @@ Status RestCatalog::DropTable(const TableIdentifier& identifier, bool purge) { } ICEBERG_ASSIGN_OR_RAISE( const auto response, - client_->Delete(path, params, /*headers=*/{}, *TableErrorHandler::Instance())); + client_->Delete(path, params, auth_headers, *TableErrorHandler::Instance())); return {}; } @@ -381,19 +432,22 @@ Result RestCatalog::TableExists(const TableIdentifier& identifier) const { } ICEBERG_ASSIGN_OR_RAISE(auto path, paths_->Table(identifier)); + ICEBERG_ASSIGN_OR_RAISE(auto auth_headers, AuthHeaders()); + return CaptureNoSuchTable( - client_->Head(path, /*headers=*/{}, *TableErrorHandler::Instance())); + client_->Head(path, auth_headers, *TableErrorHandler::Instance())); } Status RestCatalog::RenameTable(const TableIdentifier& from, const TableIdentifier& to) { ICEBERG_ENDPOINT_CHECK(supported_endpoints_, Endpoint::RenameTable()); ICEBERG_ASSIGN_OR_RAISE(auto path, paths_->Rename()); + ICEBERG_ASSIGN_OR_RAISE(auto auth_headers, AuthHeaders()); RenameTableRequest request{.source = from, .destination = to}; ICEBERG_ASSIGN_OR_RAISE(auto json_request, ToJsonString(ToJson(request))); ICEBERG_ASSIGN_OR_RAISE( const auto response, - client_->Post(path, json_request, /*headers=*/{}, *TableErrorHandler::Instance())); + client_->Post(path, json_request, auth_headers, *TableErrorHandler::Instance())); return {}; } @@ -402,9 +456,11 @@ Result RestCatalog::LoadTableInternal( const TableIdentifier& identifier) const { ICEBERG_ENDPOINT_CHECK(supported_endpoints_, Endpoint::LoadTable()); ICEBERG_ASSIGN_OR_RAISE(auto path, paths_->Table(identifier)); + ICEBERG_ASSIGN_OR_RAISE(auto auth_headers, AuthHeaders()); + ICEBERG_ASSIGN_OR_RAISE( const auto response, - client_->Get(path, /*params=*/{}, /*headers=*/{}, *TableErrorHandler::Instance())); + client_->Get(path, /*params=*/{}, auth_headers, *TableErrorHandler::Instance())); return response.body(); } @@ -422,6 +478,7 @@ Result> RestCatalog::RegisterTable( const TableIdentifier& identifier, const std::string& metadata_file_location) { ICEBERG_ENDPOINT_CHECK(supported_endpoints_, Endpoint::RegisterTable()); ICEBERG_ASSIGN_OR_RAISE(auto path, paths_->Register(identifier.ns)); + ICEBERG_ASSIGN_OR_RAISE(auto auth_headers, AuthHeaders()); RegisterTableRequest request{ .name = identifier.name, @@ -431,7 +488,7 @@ Result> RestCatalog::RegisterTable( ICEBERG_ASSIGN_OR_RAISE(auto json_request, ToJsonString(ToJson(request))); ICEBERG_ASSIGN_OR_RAISE( const auto response, - client_->Post(path, json_request, /*headers=*/{}, *TableErrorHandler::Instance())); + client_->Post(path, json_request, auth_headers, *TableErrorHandler::Instance())); ICEBERG_ASSIGN_OR_RAISE(auto json, FromJsonString(response.body())); ICEBERG_ASSIGN_OR_RAISE(auto load_result, LoadTableResultFromJson(json)); diff --git a/src/iceberg/catalog/rest/rest_catalog.h b/src/iceberg/catalog/rest/rest_catalog.h index 721df29d8..85d7fa52f 100644 --- a/src/iceberg/catalog/rest/rest_catalog.h +++ b/src/iceberg/catalog/rest/rest_catalog.h @@ -105,8 +105,13 @@ class ICEBERG_REST_EXPORT RestCatalog : public Catalog, private: RestCatalog(std::unique_ptr config, - std::shared_ptr file_io, std::unique_ptr paths, - std::unordered_set endpoints); + std::shared_ptr file_io, std::unique_ptr client, + std::unique_ptr paths, + std::unordered_set endpoints, + std::unique_ptr auth_manager, + std::shared_ptr catalog_session); + + Result> AuthHeaders() const; Result LoadTableInternal(const TableIdentifier& identifier) const; @@ -122,6 +127,8 @@ class ICEBERG_REST_EXPORT RestCatalog : public Catalog, std::unique_ptr paths_; std::string name_; std::unordered_set supported_endpoints_; + std::unique_ptr auth_manager_; + std::shared_ptr catalog_session_; }; } // namespace iceberg::rest diff --git a/src/iceberg/test/CMakeLists.txt b/src/iceberg/test/CMakeLists.txt index ae61d819f..46795d473 100644 --- a/src/iceberg/test/CMakeLists.txt +++ b/src/iceberg/test/CMakeLists.txt @@ -215,6 +215,7 @@ if(ICEBERG_BUILD_REST) add_rest_iceberg_test(rest_catalog_test SOURCES + auth_manager_test.cc endpoint_test.cc rest_json_serde_test.cc rest_util_test.cc) diff --git a/src/iceberg/test/auth_manager_test.cc b/src/iceberg/test/auth_manager_test.cc new file mode 100644 index 000000000..9d1520436 --- /dev/null +++ b/src/iceberg/test/auth_manager_test.cc @@ -0,0 +1,137 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +#include "iceberg/catalog/rest/auth/auth_manager.h" + +#include +#include + +#include +#include + +#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/http_client.h" +#include "iceberg/test/matchers.h" + +namespace iceberg::rest::auth { + +class AuthManagerTest : public ::testing::Test { + protected: + HttpClient client_{{}}; +}; + +// Verifies loading NoopAuthManager with explicit "none" auth type +TEST_F(AuthManagerTest, LoadNoopAuthManagerExplicit) { + std::unordered_map properties = { + {AuthProperties::kAuthType, "none"}}; + + auto manager_result = AuthManagers::Load("test-catalog", properties); + ASSERT_THAT(manager_result, IsOk()); + + auto session_result = manager_result.value()->CatalogSession(client_, properties); + ASSERT_THAT(session_result, IsOk()); + + std::unordered_map headers; + EXPECT_THAT(session_result.value()->Authenticate(headers), IsOk()); + EXPECT_TRUE(headers.empty()); +} + +// Verifies that NoopAuthManager is inferred when no auth properties are set +TEST_F(AuthManagerTest, LoadNoopAuthManagerInferred) { + auto manager_result = AuthManagers::Load("test-catalog", {}); + ASSERT_THAT(manager_result, IsOk()); +} + +// Verifies that auth type is case-insensitive +TEST_F(AuthManagerTest, AuthTypeCaseInsensitive) { + for (const auto& auth_type : {"NONE", "None", "NoNe"}) { + std::unordered_map properties = { + {AuthProperties::kAuthType, auth_type}}; + EXPECT_THAT(AuthManagers::Load("test-catalog", properties), IsOk()) + << "Failed for auth type: " << auth_type; + } +} + +// Verifies that unknown auth type returns InvalidArgument +TEST_F(AuthManagerTest, UnknownAuthTypeReturnsInvalidArgument) { + std::unordered_map properties = { + {AuthProperties::kAuthType, "unknown-auth-type"}}; + + auto result = AuthManagers::Load("test-catalog", properties); + EXPECT_THAT(result, IsError(ErrorKind::kInvalidArgument)); + EXPECT_THAT(result, HasErrorMessage("Unknown authentication type")); +} + +// Verifies that known but unimplemented auth type returns NotImplemented +TEST_F(AuthManagerTest, KnownButUnimplementedAuthType) { + std::unordered_map properties = { + {AuthProperties::kAuthType, "basic"}}; + + auto result = AuthManagers::Load("test-catalog", properties); + EXPECT_THAT(result, IsError(ErrorKind::kNotImplemented)); + EXPECT_THAT(result, HasErrorMessage("not yet supported")); +} + +// Verifies that OAuth2 auth type is inferred from credential or token properties +TEST_F(AuthManagerTest, InferOAuth2FromProperties) { + // From credential + auto result1 = AuthManagers::Load( + "test", {{AuthProperties::kOAuth2Credential, "client_id:client_secret"}}); + EXPECT_THAT(result1, IsError(ErrorKind::kNotImplemented)); + + // From token + auto result2 = + AuthManagers::Load("test", {{AuthProperties::kOAuth2Token, "bearer-token"}}); + EXPECT_THAT(result2, IsError(ErrorKind::kNotImplemented)); +} + +// Verifies custom auth manager registration +TEST_F(AuthManagerTest, RegisterCustomAuthManager) { + AuthManagers::Register( + "custom", + []([[maybe_unused]] std::string_view name, + [[maybe_unused]] const std::unordered_map& props) + -> Result> { + class CustomAuthManager : public AuthManager { + public: + Result> CatalogSession( + HttpClient&, const std::unordered_map&) override { + return AuthSession::MakeDefault({{"X-Custom-Auth", "custom-value"}}); + } + }; + return std::make_unique(); + }); + + std::unordered_map properties = { + {AuthProperties::kAuthType, "custom"}}; + + auto manager_result = AuthManagers::Load("test-catalog", properties); + ASSERT_THAT(manager_result, IsOk()); + + auto session_result = manager_result.value()->CatalogSession(client_, properties); + ASSERT_THAT(session_result, IsOk()); + + std::unordered_map headers; + EXPECT_THAT(session_result.value()->Authenticate(headers), IsOk()); + EXPECT_EQ(headers["X-Custom-Auth"], "custom-value"); +} + +} // namespace iceberg::rest::auth From 0895bc2071203061b428d59b98f2a015591bfdef Mon Sep 17 00:00:00 2001 From: "shuxu.li" Date: Thu, 29 Jan 2026 18:14:02 +0800 Subject: [PATCH 2/3] feat: Implement NoopAuthManager and integrate AuthManager into RestCatalog --- .../catalog/rest/auth/auth_managers.cc | 1 - src/iceberg/catalog/rest/rest_catalog.cc | 1 - src/iceberg/test/auth_manager_test.cc | 23 ------------------- 3 files changed, 25 deletions(-) diff --git a/src/iceberg/catalog/rest/auth/auth_managers.cc b/src/iceberg/catalog/rest/auth/auth_managers.cc index 8f0dd584b..0b7559a2b 100644 --- a/src/iceberg/catalog/rest/auth/auth_managers.cc +++ b/src/iceberg/catalog/rest/auth/auth_managers.cc @@ -79,7 +79,6 @@ class NoopAuthManager : public AuthManager { AuthManagerRegistry& GetRegistry() { static AuthManagerRegistry registry = [] { AuthManagerRegistry r; - // Register built-in "none" auth manager r[AuthProperties::kAuthTypeNone] = []([[maybe_unused]] std::string_view name, [[maybe_unused]] const std::unordered_map& props) diff --git a/src/iceberg/catalog/rest/rest_catalog.cc b/src/iceberg/catalog/rest/rest_catalog.cc index d645a74b6..d6bd760e1 100644 --- a/src/iceberg/catalog/rest/rest_catalog.cc +++ b/src/iceberg/catalog/rest/rest_catalog.cc @@ -139,7 +139,6 @@ Result> RestCatalog::Make( ICEBERG_ASSIGN_OR_RAISE(auto server_config, FetchServerConfig(*paths, config, init_session)); - // Merge client config with server defaults and overrides std::unique_ptr final_config = RestCatalogProperties::FromMap( MergeConfigs(server_config.defaults, config.configs(), server_config.overrides)); diff --git a/src/iceberg/test/auth_manager_test.cc b/src/iceberg/test/auth_manager_test.cc index 9d1520436..c6e9f1231 100644 --- a/src/iceberg/test/auth_manager_test.cc +++ b/src/iceberg/test/auth_manager_test.cc @@ -80,29 +80,6 @@ TEST_F(AuthManagerTest, UnknownAuthTypeReturnsInvalidArgument) { EXPECT_THAT(result, HasErrorMessage("Unknown authentication type")); } -// Verifies that known but unimplemented auth type returns NotImplemented -TEST_F(AuthManagerTest, KnownButUnimplementedAuthType) { - std::unordered_map properties = { - {AuthProperties::kAuthType, "basic"}}; - - auto result = AuthManagers::Load("test-catalog", properties); - EXPECT_THAT(result, IsError(ErrorKind::kNotImplemented)); - EXPECT_THAT(result, HasErrorMessage("not yet supported")); -} - -// Verifies that OAuth2 auth type is inferred from credential or token properties -TEST_F(AuthManagerTest, InferOAuth2FromProperties) { - // From credential - auto result1 = AuthManagers::Load( - "test", {{AuthProperties::kOAuth2Credential, "client_id:client_secret"}}); - EXPECT_THAT(result1, IsError(ErrorKind::kNotImplemented)); - - // From token - auto result2 = - AuthManagers::Load("test", {{AuthProperties::kOAuth2Token, "bearer-token"}}); - EXPECT_THAT(result2, IsError(ErrorKind::kNotImplemented)); -} - // Verifies custom auth manager registration TEST_F(AuthManagerTest, RegisterCustomAuthManager) { AuthManagers::Register( From 9da9f7603969a95738fe3e14820c06cf7eb7f02c Mon Sep 17 00:00:00 2001 From: "shuxu.li" Date: Thu, 29 Jan 2026 18:23:55 +0800 Subject: [PATCH 3/3] feat: Implement NoopAuthManager and integrate AuthManager into RestCatalog --- src/iceberg/catalog/rest/rest_catalog.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/iceberg/catalog/rest/rest_catalog.cc b/src/iceberg/catalog/rest/rest_catalog.cc index d6bd760e1..16d063e85 100644 --- a/src/iceberg/catalog/rest/rest_catalog.cc +++ b/src/iceberg/catalog/rest/rest_catalog.cc @@ -68,7 +68,7 @@ std::unordered_set GetDefaultEndpoints() { }; } -/// \brief Fetch server config and merge it with client config +/// \brief Fetch server configuration from the REST catalog server. Result FetchServerConfig( const ResourcePaths& paths, const RestCatalogProperties& current_config, const std::shared_ptr& session) {