From eaa433ffddbafe67f5eeb649265fba59cb25048c Mon Sep 17 00:00:00 2001 From: Li Jiajia Date: Sat, 11 Apr 2026 16:37:44 +0800 Subject: [PATCH 01/10] feat(auth): implement SigV4 authentication for REST catalog --- CMakeLists.txt | 1 + .../IcebergThirdpartyToolchain.cmake | 18 + src/iceberg/catalog/rest/CMakeLists.txt | 19 + .../catalog/rest/auth/auth_manager_internal.h | 7 + .../catalog/rest/auth/auth_managers.cc | 33 +- .../catalog/rest/auth/auth_properties.h | 14 + src/iceberg/catalog/rest/auth/auth_session.cc | 4 +- src/iceberg/catalog/rest/auth/auth_session.h | 11 +- .../catalog/rest/auth/sigv4_auth_manager.cc | 309 +++++++++++ .../catalog/rest/auth/sigv4_auth_manager.h | 129 +++++ src/iceberg/catalog/rest/http_client.cc | 58 ++- src/iceberg/test/CMakeLists.txt | 5 + src/iceberg/test/auth_manager_test.cc | 18 +- src/iceberg/test/sigv4_auth_test.cc | 487 ++++++++++++++++++ 14 files changed, 1087 insertions(+), 26 deletions(-) create mode 100644 src/iceberg/catalog/rest/auth/sigv4_auth_manager.cc create mode 100644 src/iceberg/catalog/rest/auth/sigv4_auth_manager.h create mode 100644 src/iceberg/test/sigv4_auth_test.cc diff --git a/CMakeLists.txt b/CMakeLists.txt index e7281fb11..0813463a5 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -45,6 +45,7 @@ option(ICEBERG_BUILD_TESTS "Build tests" ON) option(ICEBERG_BUILD_BUNDLE "Build the battery included library" ON) option(ICEBERG_BUILD_REST "Build rest catalog client" ON) option(ICEBERG_BUILD_REST_INTEGRATION_TESTS "Build rest catalog integration tests" OFF) +option(ICEBERG_BUILD_SIGV4 "Build SigV4 authentication support (requires AWS SDK)" OFF) option(ICEBERG_ENABLE_ASAN "Enable Address Sanitizer" OFF) option(ICEBERG_ENABLE_UBSAN "Enable Undefined Behavior Sanitizer" OFF) diff --git a/cmake_modules/IcebergThirdpartyToolchain.cmake b/cmake_modules/IcebergThirdpartyToolchain.cmake index 8b32eb749..839be47e3 100644 --- a/cmake_modules/IcebergThirdpartyToolchain.cmake +++ b/cmake_modules/IcebergThirdpartyToolchain.cmake @@ -531,3 +531,21 @@ endif() if(ICEBERG_BUILD_REST) resolve_cpr_dependency() endif() + +# ---------------------------------------------------------------------- +# AWS SDK for C++ + +function(resolve_aws_sdk_dependency) + find_package(AWSSDK REQUIRED COMPONENTS core) + list(APPEND ICEBERG_SYSTEM_DEPENDENCIES AWSSDK) + set(ICEBERG_SYSTEM_DEPENDENCIES + ${ICEBERG_SYSTEM_DEPENDENCIES} + PARENT_SCOPE) +endfunction() + +if(ICEBERG_BUILD_SIGV4) + if(NOT ICEBERG_BUILD_REST) + message(FATAL_ERROR "ICEBERG_BUILD_SIGV4 requires ICEBERG_BUILD_REST to be ON") + endif() + resolve_aws_sdk_dependency() +endif() diff --git a/src/iceberg/catalog/rest/CMakeLists.txt b/src/iceberg/catalog/rest/CMakeLists.txt index e91b12962..028dcc4f3 100644 --- a/src/iceberg/catalog/rest/CMakeLists.txt +++ b/src/iceberg/catalog/rest/CMakeLists.txt @@ -33,6 +33,10 @@ set(ICEBERG_REST_SOURCES rest_util.cc types.cc) +if(ICEBERG_BUILD_SIGV4) + list(APPEND ICEBERG_REST_SOURCES auth/sigv4_auth_manager.cc) +endif() + set(ICEBERG_REST_STATIC_BUILD_INTERFACE_LIBS) set(ICEBERG_REST_SHARED_BUILD_INTERFACE_LIBS) set(ICEBERG_REST_STATIC_INSTALL_INTERFACE_LIBS) @@ -51,6 +55,13 @@ list(APPEND "$,iceberg::iceberg_shared,iceberg::iceberg_static>" "$,iceberg::cpr,cpr::cpr>") +if(ICEBERG_BUILD_SIGV4) + list(APPEND ICEBERG_REST_STATIC_BUILD_INTERFACE_LIBS aws-cpp-sdk-core) + list(APPEND ICEBERG_REST_SHARED_BUILD_INTERFACE_LIBS aws-cpp-sdk-core) + list(APPEND ICEBERG_REST_STATIC_INSTALL_INTERFACE_LIBS aws-cpp-sdk-core) + list(APPEND ICEBERG_REST_SHARED_INSTALL_INTERFACE_LIBS aws-cpp-sdk-core) +endif() + add_iceberg_lib(iceberg_rest SOURCES ${ICEBERG_REST_SOURCES} @@ -63,4 +74,12 @@ add_iceberg_lib(iceberg_rest SHARED_INSTALL_INTERFACE_LIBS ${ICEBERG_REST_SHARED_INSTALL_INTERFACE_LIBS}) +if(ICEBERG_BUILD_SIGV4) + foreach(LIB iceberg_rest_static iceberg_rest_shared) + if(TARGET ${LIB}) + target_compile_definitions(${LIB} PUBLIC ICEBERG_BUILD_SIGV4) + endif() + endforeach() +endif() + iceberg_install_all_headers(iceberg/catalog/rest) diff --git a/src/iceberg/catalog/rest/auth/auth_manager_internal.h b/src/iceberg/catalog/rest/auth/auth_manager_internal.h index 051d05505..783fb2e70 100644 --- a/src/iceberg/catalog/rest/auth/auth_manager_internal.h +++ b/src/iceberg/catalog/rest/auth/auth_manager_internal.h @@ -47,4 +47,11 @@ Result> MakeOAuth2Manager( std::string_view name, const std::unordered_map& properties); +#ifdef ICEBERG_BUILD_SIGV4 +/// \brief Create a SigV4 authentication manager with a delegate. +Result> MakeSigV4AuthManager( + std::string_view name, + const std::unordered_map& properties); +#endif + } // namespace iceberg::rest::auth diff --git a/src/iceberg/catalog/rest/auth/auth_managers.cc b/src/iceberg/catalog/rest/auth/auth_managers.cc index f55885d75..0ff3a5623 100644 --- a/src/iceberg/catalog/rest/auth/auth_managers.cc +++ b/src/iceberg/catalog/rest/auth/auth_managers.cc @@ -22,6 +22,9 @@ #include #include "iceberg/catalog/rest/auth/auth_manager_internal.h" +#ifdef ICEBERG_BUILD_SIGV4 +# include "iceberg/catalog/rest/auth/sigv4_auth_manager.h" +#endif #include "iceberg/catalog/rest/auth/auth_properties.h" #include "iceberg/util/string_util.h" @@ -62,11 +65,15 @@ std::string InferAuthType( } AuthManagerRegistry CreateDefaultRegistry() { - return { + AuthManagerRegistry registry = { {AuthProperties::kAuthTypeNone, MakeNoopAuthManager}, {AuthProperties::kAuthTypeBasic, MakeBasicAuthManager}, {AuthProperties::kAuthTypeOAuth2, MakeOAuth2Manager}, }; +#ifdef ICEBERG_BUILD_SIGV4 + registry[AuthProperties::kAuthTypeSigV4] = MakeSigV4AuthManager; +#endif + return registry; } // Get the global registry of auth manager factories. @@ -98,4 +105,28 @@ Result> AuthManagers::Load( return it->second(name, properties); } +#ifdef ICEBERG_BUILD_SIGV4 +Result> MakeSigV4AuthManager( + std::string_view name, + const std::unordered_map& properties) { + // Determine the delegate auth type. Default to OAuth2 if not specified. + std::string delegate_type = AuthProperties::kAuthTypeOAuth2; + auto it = properties.find(AuthProperties::kSigV4DelegateAuthType); + if (it != properties.end() && !it->second.empty()) { + delegate_type = StringUtils::ToLower(it->second); + } + + // Prevent circular delegation (sigv4 -> sigv4 -> ...). + ICEBERG_PRECHECK(delegate_type != AuthProperties::kAuthTypeSigV4, + "Cannot delegate a SigV4 auth manager to another SigV4 auth manager"); + + // Load the delegate auth manager. + auto delegate_props = properties; + delegate_props[AuthProperties::kAuthType] = delegate_type; + + ICEBERG_ASSIGN_OR_RAISE(auto delegate, AuthManagers::Load(name, delegate_props)); + return std::make_unique(std::move(delegate)); +} +#endif + } // namespace iceberg::rest::auth diff --git a/src/iceberg/catalog/rest/auth/auth_properties.h b/src/iceberg/catalog/rest/auth/auth_properties.h index 05a7ea2c6..f5de44ea8 100644 --- a/src/iceberg/catalog/rest/auth/auth_properties.h +++ b/src/iceberg/catalog/rest/auth/auth_properties.h @@ -59,6 +59,20 @@ class ICEBERG_REST_EXPORT AuthProperties : public ConfigBase { inline static const std::string kSigV4DelegateAuthType = "rest.auth.sigv4.delegate-auth-type"; + // ---- SigV4 AWS credential entries ---- + + /// AWS region for SigV4 signing. + inline static const std::string kSigV4SigningRegion = "rest.signing-region"; + /// AWS service name for SigV4 signing. + inline static const std::string kSigV4SigningName = "rest.signing-name"; + inline static const std::string kSigV4SigningNameDefault = "execute-api"; + /// Static access key ID for SigV4 signing. + inline static const std::string kSigV4AccessKeyId = "rest.access-key-id"; + /// Static secret access key for SigV4 signing. + inline static const std::string kSigV4SecretAccessKey = "rest.secret-access-key"; + /// Optional session token for SigV4 signing. + inline static const std::string kSigV4SessionToken = "rest.session-token"; + // ---- OAuth2 entries ---- inline static Entry kToken{"token", ""}; diff --git a/src/iceberg/catalog/rest/auth/auth_session.cc b/src/iceberg/catalog/rest/auth/auth_session.cc index 7251dc4a9..a9956f5da 100644 --- a/src/iceberg/catalog/rest/auth/auth_session.cc +++ b/src/iceberg/catalog/rest/auth/auth_session.cc @@ -33,7 +33,9 @@ class DefaultAuthSession : public AuthSession { explicit DefaultAuthSession(std::unordered_map headers) : headers_(std::move(headers)) {} - Status Authenticate(std::unordered_map& headers) override { + Status Authenticate( + std::unordered_map& headers, + [[maybe_unused]] const HTTPRequestContext& request_context) override { for (const auto& [key, value] : headers_) { headers.try_emplace(key, value); } diff --git a/src/iceberg/catalog/rest/auth/auth_session.h b/src/iceberg/catalog/rest/auth/auth_session.h index 26b93877b..c6598ed42 100644 --- a/src/iceberg/catalog/rest/auth/auth_session.h +++ b/src/iceberg/catalog/rest/auth/auth_session.h @@ -23,6 +23,7 @@ #include #include +#include "iceberg/catalog/rest/endpoint.h" #include "iceberg/catalog/rest/iceberg_rest_export.h" #include "iceberg/catalog/rest/type_fwd.h" #include "iceberg/result.h" @@ -32,6 +33,13 @@ namespace iceberg::rest::auth { +/// \brief Context about the HTTP request being authenticated. +struct ICEBERG_REST_EXPORT HTTPRequestContext { + HttpMethod method = HttpMethod::kGet; + std::string url; + std::string body; +}; + /// \brief An authentication session that can authenticate outgoing HTTP requests. class ICEBERG_REST_EXPORT AuthSession { public: @@ -50,7 +58,8 @@ class ICEBERG_REST_EXPORT AuthSession { /// - NotAuthorized: Not authenticated (401) /// - IOError: Network or connection errors when reaching auth server /// - RestError: HTTP errors from authentication service - virtual Status Authenticate(std::unordered_map& headers) = 0; + virtual Status Authenticate(std::unordered_map& headers, + const HTTPRequestContext& request_context) = 0; /// \brief Close the session and release any resources. /// diff --git a/src/iceberg/catalog/rest/auth/sigv4_auth_manager.cc b/src/iceberg/catalog/rest/auth/sigv4_auth_manager.cc new file mode 100644 index 000000000..96603dbdb --- /dev/null +++ b/src/iceberg/catalog/rest/auth/sigv4_auth_manager.cc @@ -0,0 +1,309 @@ +/* + * 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/sigv4_auth_manager.h" + +#include +#include + +#include +#include +#include +#include +#include +#include + +#include "iceberg/catalog/rest/auth/auth_properties.h" +#include "iceberg/catalog/rest/endpoint.h" +#include "iceberg/util/macros.h" +#include "iceberg/util/string_util.h" + +namespace iceberg::rest::auth { + +namespace { + +/// \brief Ensures AWS SDK is initialized exactly once per process. +/// ShutdownAPI is intentionally never called (leak-by-design) to avoid +/// static destruction order issues with objects that may outlive shutdown. +class AwsSdkGuard { + public: + static void EnsureInitialized() { + static AwsSdkGuard instance; + (void)instance; + } + + private: + AwsSdkGuard() { + Aws::SDKOptions options; + Aws::InitAPI(options); + } +}; + +Aws::Http::HttpMethod ToAwsMethod(HttpMethod method) { + switch (method) { + case HttpMethod::kGet: + return Aws::Http::HttpMethod::HTTP_GET; + case HttpMethod::kPost: + return Aws::Http::HttpMethod::HTTP_POST; + case HttpMethod::kPut: + return Aws::Http::HttpMethod::HTTP_PUT; + case HttpMethod::kDelete: + return Aws::Http::HttpMethod::HTTP_DELETE; + case HttpMethod::kHead: + return Aws::Http::HttpMethod::HTTP_HEAD; + } + return Aws::Http::HttpMethod::HTTP_GET; +} + +std::unordered_map MergeProperties( + const std::unordered_map& base, + const std::unordered_map& overrides) { + auto merged = base; + for (const auto& [key, value] : overrides) { + merged.insert_or_assign(key, value); + } + return merged; +} + +} // namespace + +// ---- SigV4AuthSession ---- + +SigV4AuthSession::SigV4AuthSession( + std::shared_ptr delegate, std::string signing_region, + std::string signing_name, + std::shared_ptr credentials_provider) + : delegate_(std::move(delegate)), + signing_region_(std::move(signing_region)), + signing_name_(std::move(signing_name)), + credentials_provider_(std::move(credentials_provider)), + signer_(std::make_unique( + credentials_provider_, signing_name_.c_str(), signing_region_.c_str(), + Aws::Client::AWSAuthV4Signer::PayloadSigningPolicy::Always, + /*urlEscapePath=*/false)) {} + +SigV4AuthSession::~SigV4AuthSession() = default; + +Status SigV4AuthSession::Authenticate( + std::unordered_map& headers, + const HTTPRequestContext& request_context) { + // 1. Delegate authenticates first (e.g., adds OAuth2 Bearer token). + ICEBERG_RETURN_UNEXPECTED(delegate_->Authenticate(headers, request_context)); + + auto original_headers = headers; + + // 2. Relocate Authorization header (case-insensitive) so SigV4 takes precedence. + std::unordered_map signing_headers; + for (const auto& [name, value] : headers) { + if (StringUtils::EqualsIgnoreCase(name, "Authorization")) { + signing_headers[std::string(kRelocatedHeaderPrefix) + name] = value; + } else { + signing_headers[name] = value; + } + } + + // 3. Build AWS SDK request. + Aws::Http::URI aws_uri(request_context.url.c_str()); + auto aws_request = std::make_shared( + aws_uri, ToAwsMethod(request_context.method)); + + for (const auto& [name, value] : signing_headers) { + aws_request->SetHeaderValue(Aws::String(name.c_str()), Aws::String(value.c_str())); + } + + // 4. Set body content hash (matching Java's RESTSigV4AuthSession). + // Empty body: set EMPTY_BODY_SHA256 explicitly (Java line 118-121 workaround). + // Non-empty body: set body stream; the signer (PayloadSigningPolicy::Always) + // computes the real hex hash. Step 7 converts hex to Base64 after signing. + if (request_context.body.empty()) { + aws_request->SetHeaderValue("x-amz-content-sha256", Aws::String(kEmptyBodySha256)); + } else { + auto body_stream = + Aws::MakeShared("SigV4Body", request_context.body); + aws_request->AddContentBody(body_stream); + } + + // 5. Sign. + if (!signer_->SignRequest(*aws_request)) { + return std::unexpected( + Error{ErrorKind::kAuthenticationFailed, "SigV4 signing failed"}); + } + + // 6. Extract signed headers, relocating conflicts with originals. + headers.clear(); + auto signed_headers = aws_request->GetHeaders(); + for (auto it = signed_headers.begin(); it != signed_headers.end(); ++it) { + std::string name_str(it->first.c_str(), it->first.size()); + std::string value_str(it->second.c_str(), it->second.size()); + + for (const auto& [orig_name, orig_value] : original_headers) { + if (StringUtils::EqualsIgnoreCase(orig_name, name_str) && orig_value != value_str) { + headers[std::string(kRelocatedHeaderPrefix) + orig_name] = orig_value; + break; + } + } + + headers[name_str] = value_str; + } + + // 7. Convert body hash from hex to Base64 (matching Java's SignerChecksumParams + // output). Only convert if the value is a valid hex SHA256 (64 hex chars). + if (!request_context.body.empty()) { + auto it = headers.find("x-amz-content-sha256"); + if (it != headers.end() && it->second.size() == 64 && + it->second != std::string(kEmptyBodySha256)) { + auto decoded = Aws::Utils::HashingUtils::HexDecode(Aws::String(it->second.c_str())); + it->second = std::string(Aws::Utils::HashingUtils::Base64Encode(decoded).c_str()); + } + } + + return {}; +} + +Status SigV4AuthSession::Close() { return delegate_->Close(); } + +// ---- SigV4AuthManager ---- + +SigV4AuthManager::SigV4AuthManager(std::unique_ptr delegate) + : delegate_(std::move(delegate)) {} + +SigV4AuthManager::~SigV4AuthManager() = default; + +Result> SigV4AuthManager::InitSession( + HttpClient& init_client, + const std::unordered_map& properties) { + AwsSdkGuard::EnsureInitialized(); + ICEBERG_ASSIGN_OR_RAISE(auto delegate_session, + delegate_->InitSession(init_client, properties)); + return WrapSession(std::move(delegate_session), properties); +} + +Result> SigV4AuthManager::CatalogSession( + HttpClient& shared_client, + const std::unordered_map& properties) { + AwsSdkGuard::EnsureInitialized(); + catalog_properties_ = properties; + ICEBERG_ASSIGN_OR_RAISE(auto delegate_session, + delegate_->CatalogSession(shared_client, properties)); + return WrapSession(std::move(delegate_session), properties); +} + +Result> SigV4AuthManager::ContextualSession( + const std::unordered_map& context, + std::shared_ptr parent) { + auto* sigv4_parent = dynamic_cast(parent.get()); + ICEBERG_PRECHECK(sigv4_parent != nullptr, "Parent session is not SigV4"); + + ICEBERG_ASSIGN_OR_RAISE(auto delegate_session, delegate_->ContextualSession( + context, sigv4_parent->delegate())); + + auto merged = MergeProperties(catalog_properties_, context); + return WrapSession(std::move(delegate_session), merged); +} + +Result> SigV4AuthManager::TableSession( + const TableIdentifier& table, + const std::unordered_map& properties, + std::shared_ptr parent) { + auto* sigv4_parent = dynamic_cast(parent.get()); + ICEBERG_PRECHECK(sigv4_parent != nullptr, "Parent session is not SigV4"); + + ICEBERG_ASSIGN_OR_RAISE( + auto delegate_session, + delegate_->TableSession(table, properties, sigv4_parent->delegate())); + + auto merged = MergeProperties(catalog_properties_, properties); + return WrapSession(std::move(delegate_session), merged); +} + +Status SigV4AuthManager::Close() { return delegate_->Close(); } + +Result> +SigV4AuthManager::MakeCredentialsProvider( + const std::unordered_map& properties) { + auto access_key_it = properties.find(AuthProperties::kSigV4AccessKeyId); + auto secret_key_it = properties.find(AuthProperties::kSigV4SecretAccessKey); + bool has_ak = access_key_it != properties.end() && !access_key_it->second.empty(); + bool has_sk = secret_key_it != properties.end() && !secret_key_it->second.empty(); + + // Reject partial credentials — providing only one of AK/SK is a misconfiguration. + ICEBERG_PRECHECK( + has_ak == has_sk, "Both '{}' and '{}' must be set together, or neither", + AuthProperties::kSigV4AccessKeyId, AuthProperties::kSigV4SecretAccessKey); + + if (has_ak) { + auto session_token_it = properties.find(AuthProperties::kSigV4SessionToken); + if (session_token_it != properties.end() && !session_token_it->second.empty()) { + Aws::Auth::AWSCredentials credentials(access_key_it->second.c_str(), + secret_key_it->second.c_str(), + session_token_it->second.c_str()); + return std::make_shared(credentials); + } + Aws::Auth::AWSCredentials credentials(access_key_it->second.c_str(), + secret_key_it->second.c_str()); + return std::make_shared(credentials); + } + + return std::make_shared(); +} + +std::string SigV4AuthManager::ResolveSigningRegion( + const std::unordered_map& properties) { + auto it = properties.find(AuthProperties::kSigV4SigningRegion); + if (it != properties.end() && !it->second.empty()) { + return it->second; + } + auto legacy_it = properties.find(AuthProperties::kSigV4Region); + if (legacy_it != properties.end() && !legacy_it->second.empty()) { + return legacy_it->second; + } + if (const char* env = std::getenv("AWS_REGION")) { + return std::string(env); + } + if (const char* env = std::getenv("AWS_DEFAULT_REGION")) { + return std::string(env); + } + return "us-east-1"; +} + +std::string SigV4AuthManager::ResolveSigningName( + const std::unordered_map& properties) { + auto it = properties.find(AuthProperties::kSigV4SigningName); + if (it != properties.end() && !it->second.empty()) { + return it->second; + } + auto legacy_it = properties.find(AuthProperties::kSigV4Service); + if (legacy_it != properties.end() && !legacy_it->second.empty()) { + return legacy_it->second; + } + return AuthProperties::kSigV4SigningNameDefault; +} + +Result> SigV4AuthManager::WrapSession( + std::shared_ptr delegate_session, + const std::unordered_map& properties) { + auto region = ResolveSigningRegion(properties); + auto service = ResolveSigningName(properties); + ICEBERG_ASSIGN_OR_RAISE(auto credentials, MakeCredentialsProvider(properties)); + return std::make_shared(std::move(delegate_session), + std::move(region), std::move(service), + std::move(credentials)); +} + +} // namespace iceberg::rest::auth diff --git a/src/iceberg/catalog/rest/auth/sigv4_auth_manager.h b/src/iceberg/catalog/rest/auth/sigv4_auth_manager.h new file mode 100644 index 000000000..7ee9aa7bc --- /dev/null +++ b/src/iceberg/catalog/rest/auth/sigv4_auth_manager.h @@ -0,0 +1,129 @@ +/* + * 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. + */ + +#pragma once + +#include +#include +#include + +#include "iceberg/catalog/rest/auth/auth_manager.h" +#include "iceberg/catalog/rest/auth/auth_session.h" +#include "iceberg/catalog/rest/iceberg_rest_export.h" +#include "iceberg/result.h" + +namespace Aws::Auth { +class AWSCredentialsProvider; +} // namespace Aws::Auth + +namespace Aws::Client { +class AWSAuthV4Signer; +} // namespace Aws::Client + +namespace iceberg::rest::auth { + +/// \brief An AuthSession that signs requests with AWS SigV4. +/// +/// The request is first authenticated by the delegate AuthSession (e.g., OAuth2), +/// then signed with SigV4. In case of conflicting headers, the Authorization header +/// set by the delegate is relocated with an "Original-" prefix, then included in +/// the canonical headers to sign. +/// +/// See https://docs.aws.amazon.com/IAM/latest/UserGuide/reference_sigv.html +/// +/// Thread safety: Authenticate() is NOT thread-safe. Each session should be used +/// from a single thread, or callers must synchronize externally. +class ICEBERG_REST_EXPORT SigV4AuthSession : public AuthSession { + public: + /// SHA-256 hash of empty string, used for requests with no body. + static constexpr std::string_view kEmptyBodySha256 = + "e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855"; + + /// Prefix prepended to relocated headers that conflict with SigV4-signed headers. + static constexpr std::string_view kRelocatedHeaderPrefix = "Original-"; + + SigV4AuthSession( + std::shared_ptr delegate, std::string signing_region, + std::string signing_name, + std::shared_ptr credentials_provider); + + ~SigV4AuthSession() override; + + Status Authenticate(std::unordered_map& headers, + const HTTPRequestContext& request_context) override; + + Status Close() override; + + const std::shared_ptr& delegate() const { return delegate_; } + + private: + std::shared_ptr delegate_; + std::string signing_region_; + std::string signing_name_; + std::shared_ptr credentials_provider_; + /// Shared signer instance, matching Java's single Aws4Signer per manager. + std::unique_ptr signer_; +}; + +/// \brief An AuthManager that produces SigV4AuthSession instances. +/// +/// Wraps a delegate AuthManager to handle double authentication (e.g., OAuth2 + SigV4). +/// +/// Thread safety: CatalogSession() must be called before ContextualSession() or +/// TableSession(). Concurrent calls are NOT safe — callers must synchronize externally. +class ICEBERG_REST_EXPORT SigV4AuthManager : public AuthManager { + public: + explicit SigV4AuthManager(std::unique_ptr delegate); + ~SigV4AuthManager() override; + + Result> InitSession( + HttpClient& init_client, + const std::unordered_map& properties) override; + + Result> CatalogSession( + HttpClient& shared_client, + const std::unordered_map& properties) override; + + Result> ContextualSession( + const std::unordered_map& context, + std::shared_ptr parent) override; + + Result> TableSession( + const TableIdentifier& table, + const std::unordered_map& properties, + std::shared_ptr parent) override; + + Status Close() override; + + private: + static Result> + MakeCredentialsProvider(const std::unordered_map& properties); + static std::string ResolveSigningRegion( + const std::unordered_map& properties); + static std::string ResolveSigningName( + const std::unordered_map& properties); + Result> WrapSession( + std::shared_ptr delegate_session, + const std::unordered_map& properties); + + std::unique_ptr delegate_; + std::unordered_map catalog_properties_; +}; + +} // namespace iceberg::rest::auth diff --git a/src/iceberg/catalog/rest/http_client.cc b/src/iceberg/catalog/rest/http_client.cc index 2e383b0ae..f7d7c80b0 100644 --- a/src/iceberg/catalog/rest/http_client.cc +++ b/src/iceberg/catalog/rest/http_client.cc @@ -19,6 +19,8 @@ #include "iceberg/catalog/rest/http_client.h" +#include + #include #include @@ -72,12 +74,12 @@ constexpr std::string_view kRestExceptionType = "RESTException"; Result BuildHeaders( const std::unordered_map& request_headers, const std::unordered_map& default_headers, - auth::AuthSession& session) { + auth::AuthSession& session, const auth::HTTPRequestContext& request_context) { std::unordered_map headers(default_headers); for (const auto& [key, val] : request_headers) { headers.insert_or_assign(key, val); } - ICEBERG_RETURN_UNEXPECTED(session.Authenticate(headers)); + ICEBERG_RETURN_UNEXPECTED(session.Authenticate(headers, request_context)); return cpr::Header(headers.begin(), headers.end()); } @@ -91,6 +93,24 @@ cpr::Parameters GetParameters( return cpr_params; } +/// \brief Append URL-encoded query parameters to a URL, sorted by key. +std::string AppendQueryString( + const std::string& base_url, + const std::unordered_map& params) { + if (params.empty()) return base_url; + std::map sorted(params.begin(), params.end()); + std::string url = base_url + "?"; + bool first = true; + for (const auto& [k, v] : sorted) { + if (!first) url += "&"; + auto ek = EncodeString(k); + auto ev = EncodeString(v); + url += (ek ? *ek : k) + "=" + (ev ? *ev : v); + first = false; + } + return url; +} + /// \brief Checks if the HTTP status code indicates a successful response. bool IsSuccessful(int32_t status_code) { return status_code == 200 // OK @@ -149,8 +169,10 @@ Result HttpClient::Get( const std::string& path, const std::unordered_map& params, const std::unordered_map& headers, const ErrorHandler& error_handler, auth::AuthSession& session) { - ICEBERG_ASSIGN_OR_RAISE(auto all_headers, - BuildHeaders(headers, default_headers_, session)); + ICEBERG_ASSIGN_OR_RAISE( + auto all_headers, + BuildHeaders(headers, default_headers_, session, + {HttpMethod::kGet, AppendQueryString(path, params), ""})); cpr::Response response = cpr::Get(cpr::Url{path}, GetParameters(params), all_headers, *connection_pool_); @@ -164,8 +186,9 @@ Result HttpClient::Post( const std::string& path, const std::string& body, const std::unordered_map& headers, const ErrorHandler& error_handler, auth::AuthSession& session) { - ICEBERG_ASSIGN_OR_RAISE(auto all_headers, - BuildHeaders(headers, default_headers_, session)); + ICEBERG_ASSIGN_OR_RAISE( + auto all_headers, + BuildHeaders(headers, default_headers_, session, {HttpMethod::kPost, path, body})); cpr::Response response = cpr::Post(cpr::Url{path}, cpr::Body{body}, all_headers, *connection_pool_); @@ -182,16 +205,20 @@ Result HttpClient::PostForm( const ErrorHandler& error_handler, auth::AuthSession& session) { std::unordered_map form_headers(headers); form_headers.insert_or_assign(kHeaderContentType, kMimeTypeFormUrlEncoded); - ICEBERG_ASSIGN_OR_RAISE(auto all_headers, - BuildHeaders(form_headers, default_headers_, session)); std::vector pair_list; pair_list.reserve(form_data.size()); for (const auto& [key, val] : form_data) { pair_list.emplace_back(key, val); } + // Use cpr's own encoding as the signing body to ensure consistency with the + // actual payload sent over the wire. + cpr::Payload payload(pair_list.begin(), pair_list.end()); + std::string encoded_body = payload.GetContent(); + ICEBERG_ASSIGN_OR_RAISE(auto all_headers, + BuildHeaders(form_headers, default_headers_, session, + {HttpMethod::kPost, path, encoded_body})); cpr::Response response = - cpr::Post(cpr::Url{path}, cpr::Payload(pair_list.begin(), pair_list.end()), - all_headers, *connection_pool_); + cpr::Post(cpr::Url{path}, std::move(payload), all_headers, *connection_pool_); ICEBERG_RETURN_UNEXPECTED(HandleFailureResponse(response, error_handler)); HttpResponse http_response; @@ -202,8 +229,9 @@ Result HttpClient::PostForm( Result HttpClient::Head( const std::string& path, const std::unordered_map& headers, const ErrorHandler& error_handler, auth::AuthSession& session) { - ICEBERG_ASSIGN_OR_RAISE(auto all_headers, - BuildHeaders(headers, default_headers_, session)); + ICEBERG_ASSIGN_OR_RAISE( + auto all_headers, + BuildHeaders(headers, default_headers_, session, {HttpMethod::kHead, path, ""})); cpr::Response response = cpr::Head(cpr::Url{path}, all_headers, *connection_pool_); ICEBERG_RETURN_UNEXPECTED(HandleFailureResponse(response, error_handler)); @@ -216,8 +244,10 @@ Result HttpClient::Delete( const std::string& path, const std::unordered_map& params, const std::unordered_map& headers, const ErrorHandler& error_handler, auth::AuthSession& session) { - ICEBERG_ASSIGN_OR_RAISE(auto all_headers, - BuildHeaders(headers, default_headers_, session)); + ICEBERG_ASSIGN_OR_RAISE( + auto all_headers, + BuildHeaders(headers, default_headers_, session, + {HttpMethod::kDelete, AppendQueryString(path, params), ""})); cpr::Response response = cpr::Delete(cpr::Url{path}, GetParameters(params), all_headers, *connection_pool_); diff --git a/src/iceberg/test/CMakeLists.txt b/src/iceberg/test/CMakeLists.txt index 2dc90da64..c29a8b20e 100644 --- a/src/iceberg/test/CMakeLists.txt +++ b/src/iceberg/test/CMakeLists.txt @@ -230,6 +230,11 @@ if(ICEBERG_BUILD_REST) rest_json_serde_test.cc rest_util_test.cc) + if(ICEBERG_BUILD_SIGV4) + add_rest_iceberg_test(sigv4_auth_test SOURCES sigv4_auth_test.cc) + target_link_libraries(sigv4_auth_test PRIVATE aws-cpp-sdk-core) + endif() + if(ICEBERG_BUILD_REST_INTEGRATION_TESTS) add_rest_iceberg_test(rest_catalog_integration_test SOURCES diff --git a/src/iceberg/test/auth_manager_test.cc b/src/iceberg/test/auth_manager_test.cc index bd06fee3f..6db8bc391 100644 --- a/src/iceberg/test/auth_manager_test.cc +++ b/src/iceberg/test/auth_manager_test.cc @@ -64,7 +64,7 @@ TEST_F(AuthManagerTest, LoadNoopAuthManagerExplicit) { ASSERT_THAT(session_result, IsOk()); std::unordered_map headers; - EXPECT_THAT(session_result.value()->Authenticate(headers), IsOk()); + EXPECT_THAT(session_result.value()->Authenticate(headers, {}), IsOk()); EXPECT_TRUE(headers.empty()); } @@ -108,7 +108,7 @@ TEST_F(AuthManagerTest, LoadBasicAuthManager) { ASSERT_THAT(session_result, IsOk()); std::unordered_map headers; - EXPECT_THAT(session_result.value()->Authenticate(headers), IsOk()); + EXPECT_THAT(session_result.value()->Authenticate(headers, {}), IsOk()); // base64("admin:secret") == "YWRtaW46c2VjcmV0" EXPECT_EQ(headers["Authorization"], "Basic YWRtaW46c2VjcmV0"); } @@ -127,7 +127,7 @@ TEST_F(AuthManagerTest, BasicAuthTypeCaseInsensitive) { ASSERT_THAT(session_result, IsOk()) << "Failed for auth type: " << auth_type; std::unordered_map headers; - EXPECT_THAT(session_result.value()->Authenticate(headers), IsOk()); + EXPECT_THAT(session_result.value()->Authenticate(headers, {}), IsOk()); // base64("user:pass") == "dXNlcjpwYXNz" EXPECT_EQ(headers["Authorization"], "Basic dXNlcjpwYXNz"); } @@ -173,7 +173,7 @@ TEST_F(AuthManagerTest, BasicAuthSpecialCharacters) { ASSERT_THAT(session_result, IsOk()); std::unordered_map headers; - EXPECT_THAT(session_result.value()->Authenticate(headers), IsOk()); + EXPECT_THAT(session_result.value()->Authenticate(headers, {}), IsOk()); // base64("user@domain.com:p@ss:w0rd!") == "dXNlckBkb21haW4uY29tOnBAc3M6dzByZCE=" EXPECT_EQ(headers["Authorization"], "Basic dXNlckBkb21haW4uY29tOnBAc3M6dzByZCE="); } @@ -205,7 +205,7 @@ TEST_F(AuthManagerTest, RegisterCustomAuthManager) { ASSERT_THAT(session_result, IsOk()); std::unordered_map headers; - EXPECT_THAT(session_result.value()->Authenticate(headers), IsOk()); + EXPECT_THAT(session_result.value()->Authenticate(headers, {}), IsOk()); EXPECT_EQ(headers["X-Custom-Auth"], "custom-value"); } @@ -223,7 +223,7 @@ TEST_F(AuthManagerTest, OAuth2StaticToken) { ASSERT_THAT(session_result, IsOk()); std::unordered_map headers; - EXPECT_THAT(session_result.value()->Authenticate(headers), IsOk()); + EXPECT_THAT(session_result.value()->Authenticate(headers, {}), IsOk()); EXPECT_EQ(headers["Authorization"], "Bearer my-static-token"); } @@ -240,7 +240,7 @@ TEST_F(AuthManagerTest, OAuth2InferredFromToken) { ASSERT_THAT(session_result, IsOk()); std::unordered_map headers; - EXPECT_THAT(session_result.value()->Authenticate(headers), IsOk()); + EXPECT_THAT(session_result.value()->Authenticate(headers, {}), IsOk()); EXPECT_EQ(headers["Authorization"], "Bearer inferred-token"); } @@ -259,7 +259,7 @@ TEST_F(AuthManagerTest, OAuth2MissingCredentials) { // Session should have no auth headers std::unordered_map headers; - ASSERT_TRUE(session_result.value()->Authenticate(headers).has_value()); + ASSERT_TRUE(session_result.value()->Authenticate(headers, {}).has_value()); EXPECT_EQ(headers.find("Authorization"), headers.end()); } @@ -280,7 +280,7 @@ TEST_F(AuthManagerTest, OAuth2TokenTakesPriorityOverCredential) { ASSERT_THAT(session_result, IsOk()); std::unordered_map headers; - ASSERT_THAT(session_result.value()->Authenticate(headers), IsOk()); + ASSERT_THAT(session_result.value()->Authenticate(headers, {}), IsOk()); EXPECT_EQ(headers["Authorization"], "Bearer my-static-token"); } diff --git a/src/iceberg/test/sigv4_auth_test.cc b/src/iceberg/test/sigv4_auth_test.cc new file mode 100644 index 000000000..159339d03 --- /dev/null +++ b/src/iceberg/test/sigv4_auth_test.cc @@ -0,0 +1,487 @@ +/* + * 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 +#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/auth/sigv4_auth_manager.h" +#include "iceberg/catalog/rest/http_client.h" +#include "iceberg/table_identifier.h" +#include "iceberg/test/matchers.h" + +namespace iceberg::rest::auth { + +class SigV4AuthTest : public ::testing::Test { + protected: + static void SetUpTestSuite() { + static bool initialized = false; + if (!initialized) { + Aws::SDKOptions options; + Aws::InitAPI(options); + initialized = true; + } + } + + HttpClient client_{{}}; + + std::unordered_map MakeSigV4Properties() { + return { + {AuthProperties::kAuthType, "sigv4"}, + {AuthProperties::kSigV4SigningRegion, "us-east-1"}, + {AuthProperties::kSigV4SigningName, "execute-api"}, + {AuthProperties::kSigV4AccessKeyId, "AKIAIOSFODNN7EXAMPLE"}, + {AuthProperties::kSigV4SecretAccessKey, + "wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY"}, + }; + } +}; + +TEST_F(SigV4AuthTest, LoadSigV4AuthManager) { + auto properties = MakeSigV4Properties(); + auto manager_result = AuthManagers::Load("test-catalog", properties); + ASSERT_THAT(manager_result, IsOk()); +} + +TEST_F(SigV4AuthTest, CatalogSessionProducesSession) { + auto properties = MakeSigV4Properties(); + 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()); +} + +TEST_F(SigV4AuthTest, AuthenticateAddsAuthorizationHeader) { + auto properties = MakeSigV4Properties(); + 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; + HTTPRequestContext ctx{HttpMethod::kGet, "https://example.com/v1/config", ""}; + ASSERT_THAT(session_result.value()->Authenticate(headers, ctx), IsOk()); + + EXPECT_NE(headers.find("authorization"), headers.end()); + EXPECT_TRUE(headers["authorization"].starts_with("AWS4-HMAC-SHA256")); + EXPECT_NE(headers.find("x-amz-date"), headers.end()); +} + +TEST_F(SigV4AuthTest, AuthenticateWithPostBody) { + auto properties = MakeSigV4Properties(); + 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; + headers["Content-Type"] = "application/json"; + HTTPRequestContext ctx{HttpMethod::kPost, "https://example.com/v1/namespaces", + R"({"namespace":["ns1"]})"}; + ASSERT_THAT(session_result.value()->Authenticate(headers, ctx), IsOk()); + + EXPECT_NE(headers.find("authorization"), headers.end()); + EXPECT_TRUE(headers["authorization"].starts_with("AWS4-HMAC-SHA256")); +} + +TEST_F(SigV4AuthTest, DelegateAuthorizationHeaderRelocated) { + auto properties = MakeSigV4Properties(); + properties[AuthProperties::kToken.key()] = "my-oauth-token"; + properties[AuthProperties::kSigV4DelegateAuthType] = "oauth2"; + + 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; + HTTPRequestContext ctx{HttpMethod::kGet, "https://example.com/v1/config", ""}; + ASSERT_THAT(session_result.value()->Authenticate(headers, ctx), IsOk()); + + EXPECT_NE(headers.find("authorization"), headers.end()); + EXPECT_TRUE(headers["authorization"].starts_with("AWS4-HMAC-SHA256")); + EXPECT_NE(headers.find("original-authorization"), headers.end()); + EXPECT_EQ(headers["original-authorization"], "Bearer my-oauth-token"); +} + +TEST_F(SigV4AuthTest, AuthenticateWithSessionToken) { + auto properties = MakeSigV4Properties(); + properties[AuthProperties::kSigV4SessionToken] = "FwoGZXIvYXdzEBYaDHqa0"; + + 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; + HTTPRequestContext ctx{HttpMethod::kGet, "https://example.com/v1/config", ""}; + ASSERT_THAT(session_result.value()->Authenticate(headers, ctx), IsOk()); + + EXPECT_NE(headers.find("authorization"), headers.end()); + EXPECT_NE(headers.find("x-amz-security-token"), headers.end()); + EXPECT_EQ(headers["x-amz-security-token"], "FwoGZXIvYXdzEBYaDHqa0"); +} + +TEST_F(SigV4AuthTest, CustomSigningNameAndRegion) { + auto properties = MakeSigV4Properties(); + properties[AuthProperties::kSigV4SigningRegion] = "eu-west-1"; + properties[AuthProperties::kSigV4SigningName] = "custom-service"; + + 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; + HTTPRequestContext ctx{HttpMethod::kGet, "https://example.com/v1/config", ""}; + ASSERT_THAT(session_result.value()->Authenticate(headers, ctx), IsOk()); + + auto auth_it = headers.find("authorization"); + ASSERT_NE(auth_it, headers.end()); + EXPECT_TRUE(auth_it->second.find("eu-west-1") != std::string::npos); + EXPECT_TRUE(auth_it->second.find("custom-service") != std::string::npos); +} + +TEST_F(SigV4AuthTest, AuthTypeCaseInsensitive) { + for (const auto& auth_type : {"SIGV4", "SigV4", "sigV4"}) { + auto properties = MakeSigV4Properties(); + properties[AuthProperties::kAuthType] = auth_type; + EXPECT_THAT(AuthManagers::Load("test-catalog", properties), IsOk()) + << "Failed for auth type: " << auth_type; + } +} + +TEST_F(SigV4AuthTest, DelegateDefaultsToOAuth2NoAuth) { + auto properties = MakeSigV4Properties(); + 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; + HTTPRequestContext ctx{HttpMethod::kGet, "https://example.com/v1/config", ""}; + ASSERT_THAT(session_result.value()->Authenticate(headers, ctx), IsOk()); + + EXPECT_EQ(headers.find("original-authorization"), headers.end()); +} + +TEST_F(SigV4AuthTest, TableSessionInheritsProperties) { + auto properties = MakeSigV4Properties(); + auto manager_result = AuthManagers::Load("test-catalog", properties); + ASSERT_THAT(manager_result, IsOk()); + + auto catalog_session = manager_result.value()->CatalogSession(client_, properties); + ASSERT_THAT(catalog_session, IsOk()); + + iceberg::TableIdentifier table_id{iceberg::Namespace{{"ns1"}}, "table1"}; + std::unordered_map table_props; + auto table_session = manager_result.value()->TableSession(table_id, table_props, + catalog_session.value()); + ASSERT_THAT(table_session, IsOk()); + + std::unordered_map headers; + HTTPRequestContext ctx{HttpMethod::kGet, "https://example.com/v1/ns1/tables/table1", + ""}; + ASSERT_THAT(table_session.value()->Authenticate(headers, ctx), IsOk()); + EXPECT_NE(headers.find("authorization"), headers.end()); +} + +// ---------- Tests ported from Java TestRESTSigV4AuthSession ---------- + +// Java: authenticateWithoutBody +TEST_F(SigV4AuthTest, AuthenticateWithoutBodyDetailedHeaders) { + auto properties = MakeSigV4Properties(); + 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; + headers["Content-Type"] = "application/json"; + HTTPRequestContext ctx{HttpMethod::kGet, "http://localhost:8080/path", ""}; + ASSERT_THAT(session_result.value()->Authenticate(headers, ctx), IsOk()); + + // Original header preserved + EXPECT_EQ(headers["content-type"], "application/json"); + + // Host header generated by the signer + EXPECT_NE(headers.find("host"), headers.end()); + + // SigV4 headers + auto auth_it = headers.find("authorization"); + ASSERT_NE(auth_it, headers.end()); + EXPECT_TRUE(auth_it->second.starts_with("AWS4-HMAC-SHA256 Credential=")); + + EXPECT_TRUE(auth_it->second.find("content-type") != std::string::npos); + EXPECT_TRUE(auth_it->second.find("host") != std::string::npos); + EXPECT_TRUE(auth_it->second.find("x-amz-content-sha256") != std::string::npos); + EXPECT_TRUE(auth_it->second.find("x-amz-date") != std::string::npos); + + // Empty body SHA256 hash + EXPECT_EQ(headers["x-amz-content-sha256"], SigV4AuthSession::kEmptyBodySha256); + + // X-Amz-Date present + EXPECT_NE(headers.find("x-amz-date"), headers.end()); +} + +// Java: authenticateWithBody +TEST_F(SigV4AuthTest, AuthenticateWithBodyDetailedHeaders) { + auto properties = MakeSigV4Properties(); + 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; + headers["Content-Type"] = "application/json"; + std::string body = R"({"namespace":["ns1"]})"; + HTTPRequestContext ctx{HttpMethod::kPost, "http://localhost:8080/path", body}; + ASSERT_THAT(session_result.value()->Authenticate(headers, ctx), IsOk()); + + // SigV4 Authorization header + auto auth_it = headers.find("authorization"); + ASSERT_NE(auth_it, headers.end()); + EXPECT_TRUE(auth_it->second.starts_with("AWS4-HMAC-SHA256 Credential=")); + + // x-amz-content-sha256 should be Base64-encoded body SHA256 (matching Java) + auto sha_it = headers.find("x-amz-content-sha256"); + ASSERT_NE(sha_it, headers.end()); + EXPECT_NE(sha_it->second, SigV4AuthSession::kEmptyBodySha256); + + EXPECT_EQ(sha_it->second.size(), 44) + << "Expected Base64 SHA256, got: " << sha_it->second; +} + +// Java: authenticateConflictingAuthorizationHeader +TEST_F(SigV4AuthTest, ConflictingAuthorizationHeaderIncludedInSignedHeaders) { + auto properties = MakeSigV4Properties(); + properties[AuthProperties::kToken.key()] = "my-oauth-token"; + properties[AuthProperties::kSigV4DelegateAuthType] = "oauth2"; + + 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; + headers["Content-Type"] = "application/json"; + HTTPRequestContext ctx{HttpMethod::kGet, "http://localhost:8080/path", ""}; + ASSERT_THAT(session_result.value()->Authenticate(headers, ctx), IsOk()); + + // SigV4 Authorization header + auto auth_it = headers.find("authorization"); + ASSERT_NE(auth_it, headers.end()); + EXPECT_TRUE(auth_it->second.starts_with("AWS4-HMAC-SHA256 Credential=")); + + // Relocated delegate header should be in SignedHeaders + EXPECT_TRUE(auth_it->second.find("original-authorization") != std::string::npos) + << "SignedHeaders should include 'original-authorization', got: " + << auth_it->second; + + // Relocated Authorization present + auto orig_it = headers.find("original-authorization"); + ASSERT_NE(orig_it, headers.end()); + EXPECT_EQ(orig_it->second, "Bearer my-oauth-token"); +} + +// Java: authenticateConflictingSigv4Headers +TEST_F(SigV4AuthTest, ConflictingSigV4HeadersRelocated) { + auto delegate = AuthSession::MakeDefault({ + {"x-amz-content-sha256", "fake-sha256"}, + {"X-Amz-Date", "fake-date"}, + {"Content-Type", "application/json"}, + }); + auto credentials = + std::make_shared(Aws::Auth::AWSCredentials( + "AKIAIOSFODNN7EXAMPLE", "wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY")); + auto session = std::make_shared(delegate, "us-east-1", "execute-api", + credentials); + + std::unordered_map headers; + HTTPRequestContext ctx{HttpMethod::kGet, "http://localhost:8080/path", ""}; + ASSERT_THAT(session->Authenticate(headers, ctx), IsOk()); + + // The real x-amz-content-sha256 should be the empty body hash (signer overwrites fake) + EXPECT_EQ(headers["x-amz-content-sha256"], SigV4AuthSession::kEmptyBodySha256); + + // The fake values should be relocated since the signer produced different values + auto orig_sha_it = headers.find("Original-x-amz-content-sha256"); + ASSERT_NE(orig_sha_it, headers.end()); + EXPECT_EQ(orig_sha_it->second, "fake-sha256"); + + auto orig_date_it = headers.find("Original-X-Amz-Date"); + ASSERT_NE(orig_date_it, headers.end()); + EXPECT_EQ(orig_date_it->second, "fake-date"); + + // SigV4 Authorization present + EXPECT_NE(headers.find("authorization"), headers.end()); +} + +// Java: close (TestRESTSigV4AuthSession) +TEST_F(SigV4AuthTest, SessionCloseDelegatesToInner) { + auto delegate = AuthSession::MakeDefault({}); + auto credentials = std::make_shared( + Aws::Auth::AWSCredentials("id", "secret")); + auto session = std::make_shared(delegate, "us-east-1", "execute-api", + credentials); + + // Close should succeed without error + EXPECT_THAT(session->Close(), IsOk()); +} + +// ---------- Tests ported from Java TestRESTSigV4AuthManager ---------- + +// Java: createCustomDelegate +TEST_F(SigV4AuthTest, CreateCustomDelegateNone) { + std::unordered_map properties = { + {AuthProperties::kAuthType, "sigv4"}, + {AuthProperties::kSigV4DelegateAuthType, "none"}, + {AuthProperties::kSigV4SigningRegion, "us-west-2"}, + {AuthProperties::kSigV4AccessKeyId, "id"}, + {AuthProperties::kSigV4SecretAccessKey, "secret"}, + }; + + 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()); + + // Authenticate should work with noop delegate + std::unordered_map headers; + HTTPRequestContext ctx{HttpMethod::kGet, "https://example.com/v1/config", ""}; + ASSERT_THAT(session_result.value()->Authenticate(headers, ctx), IsOk()); + + EXPECT_NE(headers.find("authorization"), headers.end()); + + EXPECT_EQ(headers.find("original-authorization"), headers.end()); + EXPECT_EQ(headers.find("original-authorization"), headers.end()); +} + +// Java: createInvalidCustomDelegate +TEST_F(SigV4AuthTest, CreateInvalidCustomDelegateSigV4Circular) { + std::unordered_map properties = { + {AuthProperties::kAuthType, "sigv4"}, + {AuthProperties::kSigV4DelegateAuthType, "sigv4"}, + {AuthProperties::kSigV4SigningRegion, "us-east-1"}, + {AuthProperties::kSigV4AccessKeyId, "id"}, + {AuthProperties::kSigV4SecretAccessKey, "secret"}, + }; + + auto result = AuthManagers::Load("test-catalog", properties); + EXPECT_THAT(result, IsError(ErrorKind::kInvalidArgument)); + EXPECT_THAT(result, + HasErrorMessage("Cannot delegate a SigV4 auth manager to another SigV4")); +} + +// Java: contextualSession +TEST_F(SigV4AuthTest, ContextualSessionOverridesProperties) { + auto properties = MakeSigV4Properties(); + properties[AuthProperties::kSigV4SigningRegion] = "us-west-2"; + + auto manager_result = AuthManagers::Load("test-catalog", properties); + ASSERT_THAT(manager_result, IsOk()); + + auto catalog_session = manager_result.value()->CatalogSession(client_, properties); + ASSERT_THAT(catalog_session, IsOk()); + + // Context overrides region and credentials + std::unordered_map context = { + {AuthProperties::kSigV4AccessKeyId, "id2"}, + {AuthProperties::kSigV4SecretAccessKey, "secret2"}, + {AuthProperties::kSigV4SigningRegion, "eu-west-1"}, + }; + + auto ctx_session = + manager_result.value()->ContextualSession(context, catalog_session.value()); + ASSERT_THAT(ctx_session, IsOk()); + + std::unordered_map headers; + HTTPRequestContext req_ctx{HttpMethod::kGet, "https://example.com/v1/config", ""}; + ASSERT_THAT(ctx_session.value()->Authenticate(headers, req_ctx), IsOk()); + + auto auth_it = headers.find("authorization"); + ASSERT_NE(auth_it, headers.end()); + + EXPECT_TRUE(auth_it->second.find("eu-west-1") != std::string::npos) + << "Expected eu-west-1 in Authorization, got: " << auth_it->second; +} + +// Java: tableSession (with property override) +TEST_F(SigV4AuthTest, TableSessionOverridesProperties) { + auto properties = MakeSigV4Properties(); + properties[AuthProperties::kSigV4SigningRegion] = "us-west-2"; + + auto manager_result = AuthManagers::Load("test-catalog", properties); + ASSERT_THAT(manager_result, IsOk()); + + auto catalog_session = manager_result.value()->CatalogSession(client_, properties); + ASSERT_THAT(catalog_session, IsOk()); + + // Table properties override region and credentials + std::unordered_map table_props = { + {AuthProperties::kSigV4AccessKeyId, "table-key-id"}, + {AuthProperties::kSigV4SecretAccessKey, "table-secret"}, + {AuthProperties::kSigV4SigningRegion, "ap-southeast-1"}, + }; + + iceberg::TableIdentifier table_id{iceberg::Namespace{{"db1"}}, "table1"}; + auto table_session = manager_result.value()->TableSession(table_id, table_props, + catalog_session.value()); + ASSERT_THAT(table_session, IsOk()); + + std::unordered_map headers; + HTTPRequestContext req_ctx{HttpMethod::kGet, "https://example.com/v1/db1/tables/table1", + ""}; + ASSERT_THAT(table_session.value()->Authenticate(headers, req_ctx), IsOk()); + + auto auth_it = headers.find("authorization"); + ASSERT_NE(auth_it, headers.end()); + + EXPECT_TRUE(auth_it->second.find("ap-southeast-1") != std::string::npos) + << "Expected ap-southeast-1 in Authorization, got: " << auth_it->second; +} + +// Java: close (TestRESTSigV4AuthManager) +TEST_F(SigV4AuthTest, ManagerCloseDelegatesToInner) { + auto properties = MakeSigV4Properties(); + auto manager_result = AuthManagers::Load("test-catalog", properties); + ASSERT_THAT(manager_result, IsOk()); + + // Close should succeed without error + EXPECT_THAT(manager_result.value()->Close(), IsOk()); +} + +} // namespace iceberg::rest::auth From 43262822ec41ecf42d613bb9cdd14cae8b7401b6 Mon Sep 17 00:00:00 2001 From: Li Jiajia Date: Sat, 11 Apr 2026 19:01:45 +0800 Subject: [PATCH 02/10] fix(ci): enable SigV4 build in cpp-linter workflow --- .github/workflows/cpp-linter.yml | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/.github/workflows/cpp-linter.yml b/.github/workflows/cpp-linter.yml index 26324479c..1a257ee9d 100644 --- a/.github/workflows/cpp-linter.yml +++ b/.github/workflows/cpp-linter.yml @@ -37,14 +37,26 @@ jobs: uses: actions/checkout@v6 - name: Install dependencies shell: bash - run: sudo apt-get update && sudo apt-get install -y libcurl4-openssl-dev + run: sudo apt-get update && sudo apt-get install -y libcurl4-openssl-dev ninja-build + - name: Cache vcpkg packages + uses: actions/cache@v4 + id: vcpkg-cache + with: + path: /usr/local/share/vcpkg/installed + key: vcpkg-x64-linux-aws-sdk-cpp-core-${{ hashFiles('.github/workflows/cpp-linter.yml') }} + - name: Install AWS SDK via vcpkg + if: steps.vcpkg-cache.outputs.cache-hit != 'true' + shell: bash + run: vcpkg install aws-sdk-cpp[core]:x64-linux - name: Run build env: CC: gcc-14 CXX: g++-14 run: | mkdir build && cd build - cmake .. -G Ninja -DCMAKE_EXPORT_COMPILE_COMMANDS=ON + cmake .. -G Ninja -DCMAKE_EXPORT_COMPILE_COMMANDS=ON \ + -DICEBERG_BUILD_SIGV4=ON \ + -DCMAKE_TOOLCHAIN_FILE=/usr/local/share/vcpkg/scripts/buildsystems/vcpkg.cmake cmake --build . - uses: cpp-linter/cpp-linter-action@0f6d1b8d7e38b584cbee606eb23d850c217d54f8 id: linter From 6e12fbd111d297ad104898192176a7de1987ef64 Mon Sep 17 00:00:00 2001 From: Li Jiajia Date: Tue, 14 Apr 2026 11:13:58 +0800 Subject: [PATCH 03/10] address review feedback --- src/iceberg/catalog/rest/auth/auth_managers.cc | 4 +++- .../catalog/rest/auth/sigv4_auth_manager.cc | 16 ++++++---------- .../catalog/rest/auth/sigv4_auth_manager.h | 1 - 3 files changed, 9 insertions(+), 12 deletions(-) diff --git a/src/iceberg/catalog/rest/auth/auth_managers.cc b/src/iceberg/catalog/rest/auth/auth_managers.cc index 0ff3a5623..67d6f9634 100644 --- a/src/iceberg/catalog/rest/auth/auth_managers.cc +++ b/src/iceberg/catalog/rest/auth/auth_managers.cc @@ -118,7 +118,9 @@ Result> MakeSigV4AuthManager( // Prevent circular delegation (sigv4 -> sigv4 -> ...). ICEBERG_PRECHECK(delegate_type != AuthProperties::kAuthTypeSigV4, - "Cannot delegate a SigV4 auth manager to another SigV4 auth manager"); + "Cannot delegate a SigV4 auth manager to another SigV4 auth " + "manager (delegate_type='{}')", + delegate_type); // Load the delegate auth manager. auto delegate_props = properties; diff --git a/src/iceberg/catalog/rest/auth/sigv4_auth_manager.cc b/src/iceberg/catalog/rest/auth/sigv4_auth_manager.cc index 96603dbdb..1e81fcb0f 100644 --- a/src/iceberg/catalog/rest/auth/sigv4_auth_manager.cc +++ b/src/iceberg/catalog/rest/auth/sigv4_auth_manager.cc @@ -31,6 +31,7 @@ #include "iceberg/catalog/rest/auth/auth_properties.h" #include "iceberg/catalog/rest/endpoint.h" +#include "iceberg/util/checked_cast.h" #include "iceberg/util/macros.h" #include "iceberg/util/string_util.h" @@ -207,8 +208,7 @@ Result> SigV4AuthManager::CatalogSession( Result> SigV4AuthManager::ContextualSession( const std::unordered_map& context, std::shared_ptr parent) { - auto* sigv4_parent = dynamic_cast(parent.get()); - ICEBERG_PRECHECK(sigv4_parent != nullptr, "Parent session is not SigV4"); + auto sigv4_parent = internal::checked_pointer_cast(std::move(parent)); ICEBERG_ASSIGN_OR_RAISE(auto delegate_session, delegate_->ContextualSession( context, sigv4_parent->delegate())); @@ -221,8 +221,7 @@ Result> SigV4AuthManager::TableSession( const TableIdentifier& table, const std::unordered_map& properties, std::shared_ptr parent) { - auto* sigv4_parent = dynamic_cast(parent.get()); - ICEBERG_PRECHECK(sigv4_parent != nullptr, "Parent session is not SigV4"); + auto sigv4_parent = internal::checked_pointer_cast(std::move(parent)); ICEBERG_ASSIGN_OR_RAISE( auto delegate_session, @@ -248,15 +247,12 @@ SigV4AuthManager::MakeCredentialsProvider( AuthProperties::kSigV4AccessKeyId, AuthProperties::kSigV4SecretAccessKey); if (has_ak) { + Aws::Auth::AWSCredentials credentials(access_key_it->second.c_str(), + secret_key_it->second.c_str()); auto session_token_it = properties.find(AuthProperties::kSigV4SessionToken); if (session_token_it != properties.end() && !session_token_it->second.empty()) { - Aws::Auth::AWSCredentials credentials(access_key_it->second.c_str(), - secret_key_it->second.c_str(), - session_token_it->second.c_str()); - return std::make_shared(credentials); + credentials.SetSessionToken(session_token_it->second.c_str()); } - Aws::Auth::AWSCredentials credentials(access_key_it->second.c_str(), - secret_key_it->second.c_str()); return std::make_shared(credentials); } diff --git a/src/iceberg/catalog/rest/auth/sigv4_auth_manager.h b/src/iceberg/catalog/rest/auth/sigv4_auth_manager.h index 7ee9aa7bc..8b546e4ee 100644 --- a/src/iceberg/catalog/rest/auth/sigv4_auth_manager.h +++ b/src/iceberg/catalog/rest/auth/sigv4_auth_manager.h @@ -77,7 +77,6 @@ class ICEBERG_REST_EXPORT SigV4AuthSession : public AuthSession { std::string signing_region_; std::string signing_name_; std::shared_ptr credentials_provider_; - /// Shared signer instance, matching Java's single Aws4Signer per manager. std::unique_ptr signer_; }; From eb74231e8bd992dbd1c6205148ce195c2a8c1f8c Mon Sep 17 00:00:00 2001 From: Li Jiajia Date: Tue, 14 Apr 2026 11:31:40 +0800 Subject: [PATCH 04/10] add single-arg Authenticate() overload --- src/iceberg/catalog/rest/auth/auth_session.h | 5 +++++ src/iceberg/test/auth_manager_test.cc | 18 +++++++++--------- 2 files changed, 14 insertions(+), 9 deletions(-) diff --git a/src/iceberg/catalog/rest/auth/auth_session.h b/src/iceberg/catalog/rest/auth/auth_session.h index c6598ed42..9ea0efe42 100644 --- a/src/iceberg/catalog/rest/auth/auth_session.h +++ b/src/iceberg/catalog/rest/auth/auth_session.h @@ -61,6 +61,11 @@ class ICEBERG_REST_EXPORT AuthSession { virtual Status Authenticate(std::unordered_map& headers, const HTTPRequestContext& request_context) = 0; + /// \brief Convenience overload for callers that don't need a request context. + Status Authenticate(std::unordered_map& headers) { + return Authenticate(headers, HTTPRequestContext{}); + } + /// \brief Close the session and release any resources. /// /// This method is called when the session is no longer needed. For stateful diff --git a/src/iceberg/test/auth_manager_test.cc b/src/iceberg/test/auth_manager_test.cc index 6db8bc391..bd06fee3f 100644 --- a/src/iceberg/test/auth_manager_test.cc +++ b/src/iceberg/test/auth_manager_test.cc @@ -64,7 +64,7 @@ TEST_F(AuthManagerTest, LoadNoopAuthManagerExplicit) { ASSERT_THAT(session_result, IsOk()); std::unordered_map headers; - EXPECT_THAT(session_result.value()->Authenticate(headers, {}), IsOk()); + EXPECT_THAT(session_result.value()->Authenticate(headers), IsOk()); EXPECT_TRUE(headers.empty()); } @@ -108,7 +108,7 @@ TEST_F(AuthManagerTest, LoadBasicAuthManager) { ASSERT_THAT(session_result, IsOk()); std::unordered_map headers; - EXPECT_THAT(session_result.value()->Authenticate(headers, {}), IsOk()); + EXPECT_THAT(session_result.value()->Authenticate(headers), IsOk()); // base64("admin:secret") == "YWRtaW46c2VjcmV0" EXPECT_EQ(headers["Authorization"], "Basic YWRtaW46c2VjcmV0"); } @@ -127,7 +127,7 @@ TEST_F(AuthManagerTest, BasicAuthTypeCaseInsensitive) { ASSERT_THAT(session_result, IsOk()) << "Failed for auth type: " << auth_type; std::unordered_map headers; - EXPECT_THAT(session_result.value()->Authenticate(headers, {}), IsOk()); + EXPECT_THAT(session_result.value()->Authenticate(headers), IsOk()); // base64("user:pass") == "dXNlcjpwYXNz" EXPECT_EQ(headers["Authorization"], "Basic dXNlcjpwYXNz"); } @@ -173,7 +173,7 @@ TEST_F(AuthManagerTest, BasicAuthSpecialCharacters) { ASSERT_THAT(session_result, IsOk()); std::unordered_map headers; - EXPECT_THAT(session_result.value()->Authenticate(headers, {}), IsOk()); + EXPECT_THAT(session_result.value()->Authenticate(headers), IsOk()); // base64("user@domain.com:p@ss:w0rd!") == "dXNlckBkb21haW4uY29tOnBAc3M6dzByZCE=" EXPECT_EQ(headers["Authorization"], "Basic dXNlckBkb21haW4uY29tOnBAc3M6dzByZCE="); } @@ -205,7 +205,7 @@ TEST_F(AuthManagerTest, RegisterCustomAuthManager) { ASSERT_THAT(session_result, IsOk()); std::unordered_map headers; - EXPECT_THAT(session_result.value()->Authenticate(headers, {}), IsOk()); + EXPECT_THAT(session_result.value()->Authenticate(headers), IsOk()); EXPECT_EQ(headers["X-Custom-Auth"], "custom-value"); } @@ -223,7 +223,7 @@ TEST_F(AuthManagerTest, OAuth2StaticToken) { ASSERT_THAT(session_result, IsOk()); std::unordered_map headers; - EXPECT_THAT(session_result.value()->Authenticate(headers, {}), IsOk()); + EXPECT_THAT(session_result.value()->Authenticate(headers), IsOk()); EXPECT_EQ(headers["Authorization"], "Bearer my-static-token"); } @@ -240,7 +240,7 @@ TEST_F(AuthManagerTest, OAuth2InferredFromToken) { ASSERT_THAT(session_result, IsOk()); std::unordered_map headers; - EXPECT_THAT(session_result.value()->Authenticate(headers, {}), IsOk()); + EXPECT_THAT(session_result.value()->Authenticate(headers), IsOk()); EXPECT_EQ(headers["Authorization"], "Bearer inferred-token"); } @@ -259,7 +259,7 @@ TEST_F(AuthManagerTest, OAuth2MissingCredentials) { // Session should have no auth headers std::unordered_map headers; - ASSERT_TRUE(session_result.value()->Authenticate(headers, {}).has_value()); + ASSERT_TRUE(session_result.value()->Authenticate(headers).has_value()); EXPECT_EQ(headers.find("Authorization"), headers.end()); } @@ -280,7 +280,7 @@ TEST_F(AuthManagerTest, OAuth2TokenTakesPriorityOverCredential) { ASSERT_THAT(session_result, IsOk()); std::unordered_map headers; - ASSERT_THAT(session_result.value()->Authenticate(headers, {}), IsOk()); + ASSERT_THAT(session_result.value()->Authenticate(headers), IsOk()); EXPECT_EQ(headers["Authorization"], "Bearer my-static-token"); } From 6a9cd32f2d9fef41a1f9dac579813a412d4447f0 Mon Sep 17 00:00:00 2001 From: Li Jiajia Date: Tue, 14 Apr 2026 13:34:33 +0800 Subject: [PATCH 05/10] sigv4 x-amz-content-sha256 must be Base64 in canonical headers --- .../catalog/rest/auth/sigv4_auth_manager.cc | 70 ++++++++++--------- 1 file changed, 36 insertions(+), 34 deletions(-) diff --git a/src/iceberg/catalog/rest/auth/sigv4_auth_manager.cc b/src/iceberg/catalog/rest/auth/sigv4_auth_manager.cc index 1e81fcb0f..0d52534c0 100644 --- a/src/iceberg/catalog/rest/auth/sigv4_auth_manager.cc +++ b/src/iceberg/catalog/rest/auth/sigv4_auth_manager.cc @@ -82,6 +82,25 @@ std::unordered_map MergeProperties( return merged; } +/// SigV4 signer reproducing Java RESTSigV4AuthSession's +/// SignerChecksumParams(SHA256, X_AMZ_CONTENT_SHA256) output: canonical +/// headers carry Base64(SHA256(body)), canonical request trailer uses hex. +class RestSigV4Signer : public Aws::Client::AWSAuthV4Signer { + public: + RestSigV4Signer(const std::shared_ptr& creds, + const char* service_name, const Aws::String& region) + : Aws::Client::AWSAuthV4Signer(creds, service_name, region, + PayloadSigningPolicy::Always, + /*urlEscapePath=*/false) { + // AWSAuthV4Signer normally overwrites x-amz-content-sha256 with the hex + // body hash right before canonicalization, which would clobber the Base64 + // value the caller pre-sets. Clearing this flag skips that overwrite so + // canonical headers see the caller's Base64, while ComputePayloadHash + // still feeds hex into the canonical request trailer. + m_includeSha256HashHeader = false; + } +}; + } // namespace // ---- SigV4AuthSession ---- @@ -94,22 +113,18 @@ SigV4AuthSession::SigV4AuthSession( signing_region_(std::move(signing_region)), signing_name_(std::move(signing_name)), credentials_provider_(std::move(credentials_provider)), - signer_(std::make_unique( - credentials_provider_, signing_name_.c_str(), signing_region_.c_str(), - Aws::Client::AWSAuthV4Signer::PayloadSigningPolicy::Always, - /*urlEscapePath=*/false)) {} + signer_(std::make_unique( + credentials_provider_, signing_name_.c_str(), signing_region_.c_str())) {} SigV4AuthSession::~SigV4AuthSession() = default; Status SigV4AuthSession::Authenticate( std::unordered_map& headers, const HTTPRequestContext& request_context) { - // 1. Delegate authenticates first (e.g., adds OAuth2 Bearer token). ICEBERG_RETURN_UNEXPECTED(delegate_->Authenticate(headers, request_context)); - auto original_headers = headers; - // 2. Relocate Authorization header (case-insensitive) so SigV4 takes precedence. + // Relocate any delegate-set Authorization so SigV4 takes precedence. std::unordered_map signing_headers; for (const auto& [name, value] : headers) { if (StringUtils::EqualsIgnoreCase(name, "Authorization")) { @@ -119,59 +134,46 @@ Status SigV4AuthSession::Authenticate( } } - // 3. Build AWS SDK request. Aws::Http::URI aws_uri(request_context.url.c_str()); auto aws_request = std::make_shared( aws_uri, ToAwsMethod(request_context.method)); - for (const auto& [name, value] : signing_headers) { aws_request->SetHeaderValue(Aws::String(name.c_str()), Aws::String(value.c_str())); } - // 4. Set body content hash (matching Java's RESTSigV4AuthSession). - // Empty body: set EMPTY_BODY_SHA256 explicitly (Java line 118-121 workaround). - // Non-empty body: set body stream; the signer (PayloadSigningPolicy::Always) - // computes the real hex hash. Step 7 converts hex to Base64 after signing. + // Empty body uses hex EMPTY_BODY_SHA256 (Java workaround for the signer + // producing an invalid checksum for empty bodies); non-empty body uses + // Base64(SHA256(body)). See RestSigV4Signer doc for why this value survives + // signing to land in the canonical headers unchanged. if (request_context.body.empty()) { aws_request->SetHeaderValue("x-amz-content-sha256", Aws::String(kEmptyBodySha256)); } else { auto body_stream = Aws::MakeShared("SigV4Body", request_context.body); aws_request->AddContentBody(body_stream); + auto sha256 = Aws::Utils::HashingUtils::CalculateSHA256( + Aws::String(request_context.body.data(), request_context.body.size())); + aws_request->SetHeaderValue("x-amz-content-sha256", + Aws::Utils::HashingUtils::Base64Encode(sha256)); } - // 5. Sign. if (!signer_->SignRequest(*aws_request)) { return std::unexpected( Error{ErrorKind::kAuthenticationFailed, "SigV4 signing failed"}); } - // 6. Extract signed headers, relocating conflicts with originals. + // Merge signed headers back; relocate any original value that conflicts. headers.clear(); - auto signed_headers = aws_request->GetHeaders(); - for (auto it = signed_headers.begin(); it != signed_headers.end(); ++it) { - std::string name_str(it->first.c_str(), it->first.size()); - std::string value_str(it->second.c_str(), it->second.size()); - + for (const auto& [aws_name, aws_value] : aws_request->GetHeaders()) { + std::string name(aws_name.c_str(), aws_name.size()); + std::string value(aws_value.c_str(), aws_value.size()); for (const auto& [orig_name, orig_value] : original_headers) { - if (StringUtils::EqualsIgnoreCase(orig_name, name_str) && orig_value != value_str) { + if (StringUtils::EqualsIgnoreCase(orig_name, name) && orig_value != value) { headers[std::string(kRelocatedHeaderPrefix) + orig_name] = orig_value; break; } } - - headers[name_str] = value_str; - } - - // 7. Convert body hash from hex to Base64 (matching Java's SignerChecksumParams - // output). Only convert if the value is a valid hex SHA256 (64 hex chars). - if (!request_context.body.empty()) { - auto it = headers.find("x-amz-content-sha256"); - if (it != headers.end() && it->second.size() == 64 && - it->second != std::string(kEmptyBodySha256)) { - auto decoded = Aws::Utils::HashingUtils::HexDecode(Aws::String(it->second.c_str())); - it->second = std::string(Aws::Utils::HashingUtils::Base64Encode(decoded).c_str()); - } + headers[std::move(name)] = std::move(value); } return {}; From abb2cf656b5245a7f266b24f6da4421d26ed6e9c Mon Sep 17 00:00:00 2001 From: Li Jiajia Date: Tue, 14 Apr 2026 22:51:27 +0800 Subject: [PATCH 06/10] adopt request-in/request-out Authenticate interface --- .../catalog/rest/auth/auth_properties.h | 10 -- src/iceberg/catalog/rest/auth/auth_session.cc | 9 +- src/iceberg/catalog/rest/auth/auth_session.h | 27 ++-- .../catalog/rest/auth/sigv4_auth_manager.cc | 68 ++++----- .../catalog/rest/auth/sigv4_auth_manager.h | 3 +- src/iceberg/catalog/rest/http_client.cc | 57 +++++--- src/iceberg/test/auth_manager_test.cc | 56 +++---- src/iceberg/test/sigv4_auth_test.cc | 138 ++++++++++-------- 8 files changed, 192 insertions(+), 176 deletions(-) diff --git a/src/iceberg/catalog/rest/auth/auth_properties.h b/src/iceberg/catalog/rest/auth/auth_properties.h index f5de44ea8..f6dfc4ae8 100644 --- a/src/iceberg/catalog/rest/auth/auth_properties.h +++ b/src/iceberg/catalog/rest/auth/auth_properties.h @@ -54,23 +54,13 @@ class ICEBERG_REST_EXPORT AuthProperties : public ConfigBase { // ---- SigV4 entries ---- - inline static const std::string kSigV4Region = "rest.auth.sigv4.region"; - inline static const std::string kSigV4Service = "rest.auth.sigv4.service"; inline static const std::string kSigV4DelegateAuthType = "rest.auth.sigv4.delegate-auth-type"; - - // ---- SigV4 AWS credential entries ---- - - /// AWS region for SigV4 signing. inline static const std::string kSigV4SigningRegion = "rest.signing-region"; - /// AWS service name for SigV4 signing. inline static const std::string kSigV4SigningName = "rest.signing-name"; inline static const std::string kSigV4SigningNameDefault = "execute-api"; - /// Static access key ID for SigV4 signing. inline static const std::string kSigV4AccessKeyId = "rest.access-key-id"; - /// Static secret access key for SigV4 signing. inline static const std::string kSigV4SecretAccessKey = "rest.secret-access-key"; - /// Optional session token for SigV4 signing. inline static const std::string kSigV4SessionToken = "rest.session-token"; // ---- OAuth2 entries ---- diff --git a/src/iceberg/catalog/rest/auth/auth_session.cc b/src/iceberg/catalog/rest/auth/auth_session.cc index a9956f5da..3d6788479 100644 --- a/src/iceberg/catalog/rest/auth/auth_session.cc +++ b/src/iceberg/catalog/rest/auth/auth_session.cc @@ -33,13 +33,12 @@ class DefaultAuthSession : public AuthSession { explicit DefaultAuthSession(std::unordered_map headers) : headers_(std::move(headers)) {} - Status Authenticate( - std::unordered_map& headers, - [[maybe_unused]] const HTTPRequestContext& request_context) override { + Result Authenticate(const HTTPRequest& request) override { + HTTPRequest authenticated = request; for (const auto& [key, value] : headers_) { - headers.try_emplace(key, value); + authenticated.headers.try_emplace(key, value); } - return {}; + return authenticated; } private: diff --git a/src/iceberg/catalog/rest/auth/auth_session.h b/src/iceberg/catalog/rest/auth/auth_session.h index 9ea0efe42..396d529e4 100644 --- a/src/iceberg/catalog/rest/auth/auth_session.h +++ b/src/iceberg/catalog/rest/auth/auth_session.h @@ -33,10 +33,14 @@ namespace iceberg::rest::auth { -/// \brief Context about the HTTP request being authenticated. -struct ICEBERG_REST_EXPORT HTTPRequestContext { +/// \brief An outgoing HTTP request passed through an AuthSession. Mirrors the +/// HTTPRequest type used by the Java reference implementation so signing +/// implementations like SigV4 can operate on method, url, headers, and body +/// as a single value. +struct ICEBERG_REST_EXPORT HTTPRequest { HttpMethod method = HttpMethod::kGet; std::string url; + std::unordered_map headers; std::string body; }; @@ -45,26 +49,21 @@ class ICEBERG_REST_EXPORT AuthSession { public: virtual ~AuthSession() = default; - /// \brief Authenticate the given request headers. + /// \brief Authenticate an outgoing HTTP request. /// - /// This method adds authentication information (e.g., Authorization header) - /// to the provided headers map. The implementation should be idempotent. + /// Returns a new request with authentication information (e.g., an + /// Authorization header) added. Implementations must be idempotent and must + /// not mutate the input request. /// - /// \param[in,out] headers The headers map to add authentication information to. - /// \return Status indicating success or one of the following errors: + /// \param request The request to authenticate. + /// \return The authenticated request on success, or one of: /// - AuthenticationFailed: General authentication failure (invalid credentials, /// etc.) /// - TokenExpired: Authentication token has expired and needs refresh /// - NotAuthorized: Not authenticated (401) /// - IOError: Network or connection errors when reaching auth server /// - RestError: HTTP errors from authentication service - virtual Status Authenticate(std::unordered_map& headers, - const HTTPRequestContext& request_context) = 0; - - /// \brief Convenience overload for callers that don't need a request context. - Status Authenticate(std::unordered_map& headers) { - return Authenticate(headers, HTTPRequestContext{}); - } + virtual Result Authenticate(const HTTPRequest& request) = 0; /// \brief Close the session and release any resources. /// diff --git a/src/iceberg/catalog/rest/auth/sigv4_auth_manager.cc b/src/iceberg/catalog/rest/auth/sigv4_auth_manager.cc index 0d52534c0..bca6c8f55 100644 --- a/src/iceberg/catalog/rest/auth/sigv4_auth_manager.cc +++ b/src/iceberg/catalog/rest/auth/sigv4_auth_manager.cc @@ -82,9 +82,8 @@ std::unordered_map MergeProperties( return merged; } -/// SigV4 signer reproducing Java RESTSigV4AuthSession's -/// SignerChecksumParams(SHA256, X_AMZ_CONTENT_SHA256) output: canonical -/// headers carry Base64(SHA256(body)), canonical request trailer uses hex. +/// Matches Java RESTSigV4AuthSession: canonical headers carry +/// Base64(SHA256(body)), canonical request trailer uses hex. class RestSigV4Signer : public Aws::Client::AWSAuthV4Signer { public: RestSigV4Signer(const std::shared_ptr& creds, @@ -92,11 +91,9 @@ class RestSigV4Signer : public Aws::Client::AWSAuthV4Signer { : Aws::Client::AWSAuthV4Signer(creds, service_name, region, PayloadSigningPolicy::Always, /*urlEscapePath=*/false) { - // AWSAuthV4Signer normally overwrites x-amz-content-sha256 with the hex - // body hash right before canonicalization, which would clobber the Base64 - // value the caller pre-sets. Clearing this flag skips that overwrite so - // canonical headers see the caller's Base64, while ComputePayloadHash - // still feeds hex into the canonical request trailer. + // Skip the signer's hex overwrite of x-amz-content-sha256 so canonical + // headers see the caller's Base64; ComputePayloadHash still feeds hex + // into the canonical request trailer. m_includeSha256HashHeader = false; } }; @@ -118,15 +115,13 @@ SigV4AuthSession::SigV4AuthSession( SigV4AuthSession::~SigV4AuthSession() = default; -Status SigV4AuthSession::Authenticate( - std::unordered_map& headers, - const HTTPRequestContext& request_context) { - ICEBERG_RETURN_UNEXPECTED(delegate_->Authenticate(headers, request_context)); - auto original_headers = headers; +Result SigV4AuthSession::Authenticate(const HTTPRequest& request) { + ICEBERG_ASSIGN_OR_RAISE(auto delegate_request, delegate_->Authenticate(request)); + const auto& original_headers = delegate_request.headers; // Relocate any delegate-set Authorization so SigV4 takes precedence. std::unordered_map signing_headers; - for (const auto& [name, value] : headers) { + for (const auto& [name, value] : original_headers) { if (StringUtils::EqualsIgnoreCase(name, "Authorization")) { signing_headers[std::string(kRelocatedHeaderPrefix) + name] = value; } else { @@ -134,25 +129,23 @@ Status SigV4AuthSession::Authenticate( } } - Aws::Http::URI aws_uri(request_context.url.c_str()); + Aws::Http::URI aws_uri(delegate_request.url.c_str()); auto aws_request = std::make_shared( - aws_uri, ToAwsMethod(request_context.method)); + aws_uri, ToAwsMethod(delegate_request.method)); for (const auto& [name, value] : signing_headers) { aws_request->SetHeaderValue(Aws::String(name.c_str()), Aws::String(value.c_str())); } // Empty body uses hex EMPTY_BODY_SHA256 (Java workaround for the signer - // producing an invalid checksum for empty bodies); non-empty body uses - // Base64(SHA256(body)). See RestSigV4Signer doc for why this value survives - // signing to land in the canonical headers unchanged. - if (request_context.body.empty()) { + // producing an invalid checksum on empty bodies); non-empty uses Base64. + if (delegate_request.body.empty()) { aws_request->SetHeaderValue("x-amz-content-sha256", Aws::String(kEmptyBodySha256)); } else { auto body_stream = - Aws::MakeShared("SigV4Body", request_context.body); + Aws::MakeShared("SigV4Body", delegate_request.body); aws_request->AddContentBody(body_stream); auto sha256 = Aws::Utils::HashingUtils::CalculateSHA256( - Aws::String(request_context.body.data(), request_context.body.size())); + Aws::String(delegate_request.body.data(), delegate_request.body.size())); aws_request->SetHeaderValue("x-amz-content-sha256", Aws::Utils::HashingUtils::Base64Encode(sha256)); } @@ -162,21 +155,25 @@ Status SigV4AuthSession::Authenticate( Error{ErrorKind::kAuthenticationFailed, "SigV4 signing failed"}); } - // Merge signed headers back; relocate any original value that conflicts. - headers.clear(); + // Fill headers with the signed set, relocating any conflicting originals. + HTTPRequest signed_request{.method = delegate_request.method, + .url = std::move(delegate_request.url), + .headers = {}, + .body = std::move(delegate_request.body)}; for (const auto& [aws_name, aws_value] : aws_request->GetHeaders()) { std::string name(aws_name.c_str(), aws_name.size()); std::string value(aws_value.c_str(), aws_value.size()); for (const auto& [orig_name, orig_value] : original_headers) { if (StringUtils::EqualsIgnoreCase(orig_name, name) && orig_value != value) { - headers[std::string(kRelocatedHeaderPrefix) + orig_name] = orig_value; + signed_request.headers[std::string(kRelocatedHeaderPrefix) + orig_name] = + orig_value; break; } } - headers[std::move(name)] = std::move(value); + signed_request.headers[std::move(name)] = std::move(value); } - return {}; + return signed_request; } Status SigV4AuthSession::Close() { return delegate_->Close(); } @@ -243,7 +240,6 @@ SigV4AuthManager::MakeCredentialsProvider( bool has_ak = access_key_it != properties.end() && !access_key_it->second.empty(); bool has_sk = secret_key_it != properties.end() && !secret_key_it->second.empty(); - // Reject partial credentials — providing only one of AK/SK is a misconfiguration. ICEBERG_PRECHECK( has_ak == has_sk, "Both '{}' and '{}' must be set together, or neither", AuthProperties::kSigV4AccessKeyId, AuthProperties::kSigV4SecretAccessKey); @@ -263,14 +259,10 @@ SigV4AuthManager::MakeCredentialsProvider( std::string SigV4AuthManager::ResolveSigningRegion( const std::unordered_map& properties) { - auto it = properties.find(AuthProperties::kSigV4SigningRegion); - if (it != properties.end() && !it->second.empty()) { + if (auto it = properties.find(AuthProperties::kSigV4SigningRegion); + it != properties.end() && !it->second.empty()) { return it->second; } - auto legacy_it = properties.find(AuthProperties::kSigV4Region); - if (legacy_it != properties.end() && !legacy_it->second.empty()) { - return legacy_it->second; - } if (const char* env = std::getenv("AWS_REGION")) { return std::string(env); } @@ -282,14 +274,10 @@ std::string SigV4AuthManager::ResolveSigningRegion( std::string SigV4AuthManager::ResolveSigningName( const std::unordered_map& properties) { - auto it = properties.find(AuthProperties::kSigV4SigningName); - if (it != properties.end() && !it->second.empty()) { + if (auto it = properties.find(AuthProperties::kSigV4SigningName); + it != properties.end() && !it->second.empty()) { return it->second; } - auto legacy_it = properties.find(AuthProperties::kSigV4Service); - if (legacy_it != properties.end() && !legacy_it->second.empty()) { - return legacy_it->second; - } return AuthProperties::kSigV4SigningNameDefault; } diff --git a/src/iceberg/catalog/rest/auth/sigv4_auth_manager.h b/src/iceberg/catalog/rest/auth/sigv4_auth_manager.h index 8b546e4ee..48cc0eb2e 100644 --- a/src/iceberg/catalog/rest/auth/sigv4_auth_manager.h +++ b/src/iceberg/catalog/rest/auth/sigv4_auth_manager.h @@ -65,8 +65,7 @@ class ICEBERG_REST_EXPORT SigV4AuthSession : public AuthSession { ~SigV4AuthSession() override; - Status Authenticate(std::unordered_map& headers, - const HTTPRequestContext& request_context) override; + Result Authenticate(const HTTPRequest& request) override; Status Close() override; diff --git a/src/iceberg/catalog/rest/http_client.cc b/src/iceberg/catalog/rest/http_client.cc index f7d7c80b0..8dec6f239 100644 --- a/src/iceberg/catalog/rest/http_client.cc +++ b/src/iceberg/catalog/rest/http_client.cc @@ -70,17 +70,22 @@ namespace { /// \brief Default error type for unparseable REST responses. constexpr std::string_view kRestExceptionType = "RESTException"; -/// \brief Prepare headers for an HTTP request. -Result BuildHeaders( - const std::unordered_map& request_headers, +/// \brief Merge default headers with per-request headers (per-request wins). +std::unordered_map MergeHeaders( const std::unordered_map& default_headers, - auth::AuthSession& session, const auth::HTTPRequestContext& request_context) { - std::unordered_map headers(default_headers); + const std::unordered_map& request_headers) { + std::unordered_map merged(default_headers); for (const auto& [key, val] : request_headers) { - headers.insert_or_assign(key, val); + merged.insert_or_assign(key, val); } - ICEBERG_RETURN_UNEXPECTED(session.Authenticate(headers, request_context)); - return cpr::Header(headers.begin(), headers.end()); + return merged; +} + +/// \brief Authenticate the request and return the final cpr::Header. +Result AuthenticateRequest(const auth::HTTPRequest& request, + auth::AuthSession& session) { + ICEBERG_ASSIGN_OR_RAISE(auto authenticated, session.Authenticate(request)); + return cpr::Header(authenticated.headers.begin(), authenticated.headers.end()); } /// \brief Converts a map of string key-value pairs to cpr::Parameters. @@ -171,8 +176,11 @@ Result HttpClient::Get( const ErrorHandler& error_handler, auth::AuthSession& session) { ICEBERG_ASSIGN_OR_RAISE( auto all_headers, - BuildHeaders(headers, default_headers_, session, - {HttpMethod::kGet, AppendQueryString(path, params), ""})); + AuthenticateRequest({.method = HttpMethod::kGet, + .url = AppendQueryString(path, params), + .headers = MergeHeaders(default_headers_, headers), + .body = ""}, + session)); cpr::Response response = cpr::Get(cpr::Url{path}, GetParameters(params), all_headers, *connection_pool_); @@ -188,7 +196,11 @@ Result HttpClient::Post( const ErrorHandler& error_handler, auth::AuthSession& session) { ICEBERG_ASSIGN_OR_RAISE( auto all_headers, - BuildHeaders(headers, default_headers_, session, {HttpMethod::kPost, path, body})); + AuthenticateRequest({.method = HttpMethod::kPost, + .url = path, + .headers = MergeHeaders(default_headers_, headers), + .body = body}, + session)); cpr::Response response = cpr::Post(cpr::Url{path}, cpr::Body{body}, all_headers, *connection_pool_); @@ -214,9 +226,13 @@ Result HttpClient::PostForm( // actual payload sent over the wire. cpr::Payload payload(pair_list.begin(), pair_list.end()); std::string encoded_body = payload.GetContent(); - ICEBERG_ASSIGN_OR_RAISE(auto all_headers, - BuildHeaders(form_headers, default_headers_, session, - {HttpMethod::kPost, path, encoded_body})); + ICEBERG_ASSIGN_OR_RAISE( + auto all_headers, + AuthenticateRequest({.method = HttpMethod::kPost, + .url = path, + .headers = MergeHeaders(default_headers_, form_headers), + .body = encoded_body}, + session)); cpr::Response response = cpr::Post(cpr::Url{path}, std::move(payload), all_headers, *connection_pool_); @@ -231,7 +247,11 @@ Result HttpClient::Head( const ErrorHandler& error_handler, auth::AuthSession& session) { ICEBERG_ASSIGN_OR_RAISE( auto all_headers, - BuildHeaders(headers, default_headers_, session, {HttpMethod::kHead, path, ""})); + AuthenticateRequest({.method = HttpMethod::kHead, + .url = path, + .headers = MergeHeaders(default_headers_, headers), + .body = ""}, + session)); cpr::Response response = cpr::Head(cpr::Url{path}, all_headers, *connection_pool_); ICEBERG_RETURN_UNEXPECTED(HandleFailureResponse(response, error_handler)); @@ -246,8 +266,11 @@ Result HttpClient::Delete( const ErrorHandler& error_handler, auth::AuthSession& session) { ICEBERG_ASSIGN_OR_RAISE( auto all_headers, - BuildHeaders(headers, default_headers_, session, - {HttpMethod::kDelete, AppendQueryString(path, params), ""})); + AuthenticateRequest({.method = HttpMethod::kDelete, + .url = AppendQueryString(path, params), + .headers = MergeHeaders(default_headers_, headers), + .body = ""}, + session)); cpr::Response response = cpr::Delete(cpr::Url{path}, GetParameters(params), all_headers, *connection_pool_); diff --git a/src/iceberg/test/auth_manager_test.cc b/src/iceberg/test/auth_manager_test.cc index bd06fee3f..04ad2bd1d 100644 --- a/src/iceberg/test/auth_manager_test.cc +++ b/src/iceberg/test/auth_manager_test.cc @@ -63,9 +63,9 @@ TEST_F(AuthManagerTest, LoadNoopAuthManagerExplicit) { 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()); + auto auth_result = session_result.value()->Authenticate({}); + ASSERT_THAT(auth_result, IsOk()); + EXPECT_TRUE(auth_result.value().headers.empty()); } // Verifies that NoopAuthManager is inferred when no auth properties are set @@ -107,10 +107,10 @@ TEST_F(AuthManagerTest, LoadBasicAuthManager) { 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()); + auto auth_result = session_result.value()->Authenticate({}); + ASSERT_THAT(auth_result, IsOk()); // base64("admin:secret") == "YWRtaW46c2VjcmV0" - EXPECT_EQ(headers["Authorization"], "Basic YWRtaW46c2VjcmV0"); + EXPECT_EQ(auth_result.value().headers["Authorization"], "Basic YWRtaW46c2VjcmV0"); } // Verifies BasicAuthManager is case-insensitive for auth type @@ -126,10 +126,10 @@ TEST_F(AuthManagerTest, BasicAuthTypeCaseInsensitive) { auto session_result = manager_result.value()->CatalogSession(client_, properties); ASSERT_THAT(session_result, IsOk()) << "Failed for auth type: " << auth_type; - std::unordered_map headers; - EXPECT_THAT(session_result.value()->Authenticate(headers), IsOk()); + auto auth_result = session_result.value()->Authenticate({}); + ASSERT_THAT(auth_result, IsOk()) << "Failed for auth type: " << auth_type; // base64("user:pass") == "dXNlcjpwYXNz" - EXPECT_EQ(headers["Authorization"], "Basic dXNlcjpwYXNz"); + EXPECT_EQ(auth_result.value().headers["Authorization"], "Basic dXNlcjpwYXNz"); } } @@ -172,10 +172,11 @@ TEST_F(AuthManagerTest, BasicAuthSpecialCharacters) { 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()); + auto auth_result = session_result.value()->Authenticate({}); + ASSERT_THAT(auth_result, IsOk()); // base64("user@domain.com:p@ss:w0rd!") == "dXNlckBkb21haW4uY29tOnBAc3M6dzByZCE=" - EXPECT_EQ(headers["Authorization"], "Basic dXNlckBkb21haW4uY29tOnBAc3M6dzByZCE="); + EXPECT_EQ(auth_result.value().headers["Authorization"], + "Basic dXNlckBkb21haW4uY29tOnBAc3M6dzByZCE="); } // Verifies custom auth manager registration @@ -204,9 +205,9 @@ TEST_F(AuthManagerTest, RegisterCustomAuthManager) { 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"); + auto auth_result = session_result.value()->Authenticate({}); + ASSERT_THAT(auth_result, IsOk()); + EXPECT_EQ(auth_result.value().headers["X-Custom-Auth"], "custom-value"); } // Verifies OAuth2 with static token @@ -222,9 +223,9 @@ TEST_F(AuthManagerTest, OAuth2StaticToken) { 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["Authorization"], "Bearer my-static-token"); + auto auth_result = session_result.value()->Authenticate({}); + ASSERT_THAT(auth_result, IsOk()); + EXPECT_EQ(auth_result.value().headers["Authorization"], "Bearer my-static-token"); } // Verifies OAuth2 type is inferred from token property @@ -239,9 +240,9 @@ TEST_F(AuthManagerTest, OAuth2InferredFromToken) { 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["Authorization"], "Bearer inferred-token"); + auto auth_result = session_result.value()->Authenticate({}); + ASSERT_THAT(auth_result, IsOk()); + EXPECT_EQ(auth_result.value().headers["Authorization"], "Bearer inferred-token"); } // Verifies OAuth2 returns unauthenticated session when neither token nor credential is @@ -258,9 +259,10 @@ TEST_F(AuthManagerTest, OAuth2MissingCredentials) { ASSERT_THAT(session_result, IsOk()); // Session should have no auth headers - std::unordered_map headers; - ASSERT_TRUE(session_result.value()->Authenticate(headers).has_value()); - EXPECT_EQ(headers.find("Authorization"), headers.end()); + auto auth_result = session_result.value()->Authenticate({}); + ASSERT_TRUE(auth_result.has_value()); + EXPECT_EQ(auth_result.value().headers.find("Authorization"), + auth_result.value().headers.end()); } // Verifies that when both token and credential are provided, token takes priority @@ -279,9 +281,9 @@ TEST_F(AuthManagerTest, OAuth2TokenTakesPriorityOverCredential) { auto session_result = manager_result.value()->CatalogSession(client_, properties); ASSERT_THAT(session_result, IsOk()); - std::unordered_map headers; - ASSERT_THAT(session_result.value()->Authenticate(headers), IsOk()); - EXPECT_EQ(headers["Authorization"], "Bearer my-static-token"); + auto auth_result = session_result.value()->Authenticate({}); + ASSERT_THAT(auth_result, IsOk()); + EXPECT_EQ(auth_result.value().headers["Authorization"], "Bearer my-static-token"); } // Verifies OAuthTokenResponse JSON parsing diff --git a/src/iceberg/test/sigv4_auth_test.cc b/src/iceberg/test/sigv4_auth_test.cc index 159339d03..caa9b5603 100644 --- a/src/iceberg/test/sigv4_auth_test.cc +++ b/src/iceberg/test/sigv4_auth_test.cc @@ -82,12 +82,13 @@ TEST_F(SigV4AuthTest, AuthenticateAddsAuthorizationHeader) { auto session_result = manager_result.value()->CatalogSession(client_, properties); ASSERT_THAT(session_result, IsOk()); - std::unordered_map headers; - HTTPRequestContext ctx{HttpMethod::kGet, "https://example.com/v1/config", ""}; - ASSERT_THAT(session_result.value()->Authenticate(headers, ctx), IsOk()); + HTTPRequest request{.method = HttpMethod::kGet, .url = "https://example.com/v1/config"}; + auto auth_result = session_result.value()->Authenticate(request); + ASSERT_THAT(auth_result, IsOk()); + const auto& headers = auth_result.value().headers; EXPECT_NE(headers.find("authorization"), headers.end()); - EXPECT_TRUE(headers["authorization"].starts_with("AWS4-HMAC-SHA256")); + EXPECT_TRUE(headers.at("authorization").starts_with("AWS4-HMAC-SHA256")); EXPECT_NE(headers.find("x-amz-date"), headers.end()); } @@ -99,14 +100,16 @@ TEST_F(SigV4AuthTest, AuthenticateWithPostBody) { auto session_result = manager_result.value()->CatalogSession(client_, properties); ASSERT_THAT(session_result, IsOk()); - std::unordered_map headers; - headers["Content-Type"] = "application/json"; - HTTPRequestContext ctx{HttpMethod::kPost, "https://example.com/v1/namespaces", - R"({"namespace":["ns1"]})"}; - ASSERT_THAT(session_result.value()->Authenticate(headers, ctx), IsOk()); + HTTPRequest request{.method = HttpMethod::kPost, + .url = "https://example.com/v1/namespaces", + .headers = {{"Content-Type", "application/json"}}, + .body = R"({"namespace":["ns1"]})"}; + auto auth_result = session_result.value()->Authenticate(request); + ASSERT_THAT(auth_result, IsOk()); + const auto& headers = auth_result.value().headers; EXPECT_NE(headers.find("authorization"), headers.end()); - EXPECT_TRUE(headers["authorization"].starts_with("AWS4-HMAC-SHA256")); + EXPECT_TRUE(headers.at("authorization").starts_with("AWS4-HMAC-SHA256")); } TEST_F(SigV4AuthTest, DelegateAuthorizationHeaderRelocated) { @@ -120,14 +123,15 @@ TEST_F(SigV4AuthTest, DelegateAuthorizationHeaderRelocated) { auto session_result = manager_result.value()->CatalogSession(client_, properties); ASSERT_THAT(session_result, IsOk()); - std::unordered_map headers; - HTTPRequestContext ctx{HttpMethod::kGet, "https://example.com/v1/config", ""}; - ASSERT_THAT(session_result.value()->Authenticate(headers, ctx), IsOk()); + HTTPRequest request{.method = HttpMethod::kGet, .url = "https://example.com/v1/config"}; + auto auth_result = session_result.value()->Authenticate(request); + ASSERT_THAT(auth_result, IsOk()); + const auto& headers = auth_result.value().headers; EXPECT_NE(headers.find("authorization"), headers.end()); - EXPECT_TRUE(headers["authorization"].starts_with("AWS4-HMAC-SHA256")); + EXPECT_TRUE(headers.at("authorization").starts_with("AWS4-HMAC-SHA256")); EXPECT_NE(headers.find("original-authorization"), headers.end()); - EXPECT_EQ(headers["original-authorization"], "Bearer my-oauth-token"); + EXPECT_EQ(headers.at("original-authorization"), "Bearer my-oauth-token"); } TEST_F(SigV4AuthTest, AuthenticateWithSessionToken) { @@ -140,13 +144,14 @@ TEST_F(SigV4AuthTest, AuthenticateWithSessionToken) { auto session_result = manager_result.value()->CatalogSession(client_, properties); ASSERT_THAT(session_result, IsOk()); - std::unordered_map headers; - HTTPRequestContext ctx{HttpMethod::kGet, "https://example.com/v1/config", ""}; - ASSERT_THAT(session_result.value()->Authenticate(headers, ctx), IsOk()); + HTTPRequest request{.method = HttpMethod::kGet, .url = "https://example.com/v1/config"}; + auto auth_result = session_result.value()->Authenticate(request); + ASSERT_THAT(auth_result, IsOk()); + const auto& headers = auth_result.value().headers; EXPECT_NE(headers.find("authorization"), headers.end()); EXPECT_NE(headers.find("x-amz-security-token"), headers.end()); - EXPECT_EQ(headers["x-amz-security-token"], "FwoGZXIvYXdzEBYaDHqa0"); + EXPECT_EQ(headers.at("x-amz-security-token"), "FwoGZXIvYXdzEBYaDHqa0"); } TEST_F(SigV4AuthTest, CustomSigningNameAndRegion) { @@ -160,9 +165,10 @@ TEST_F(SigV4AuthTest, CustomSigningNameAndRegion) { auto session_result = manager_result.value()->CatalogSession(client_, properties); ASSERT_THAT(session_result, IsOk()); - std::unordered_map headers; - HTTPRequestContext ctx{HttpMethod::kGet, "https://example.com/v1/config", ""}; - ASSERT_THAT(session_result.value()->Authenticate(headers, ctx), IsOk()); + HTTPRequest request{.method = HttpMethod::kGet, .url = "https://example.com/v1/config"}; + auto auth_result = session_result.value()->Authenticate(request); + ASSERT_THAT(auth_result, IsOk()); + const auto& headers = auth_result.value().headers; auto auth_it = headers.find("authorization"); ASSERT_NE(auth_it, headers.end()); @@ -187,9 +193,10 @@ TEST_F(SigV4AuthTest, DelegateDefaultsToOAuth2NoAuth) { auto session_result = manager_result.value()->CatalogSession(client_, properties); ASSERT_THAT(session_result, IsOk()); - std::unordered_map headers; - HTTPRequestContext ctx{HttpMethod::kGet, "https://example.com/v1/config", ""}; - ASSERT_THAT(session_result.value()->Authenticate(headers, ctx), IsOk()); + HTTPRequest request{.method = HttpMethod::kGet, .url = "https://example.com/v1/config"}; + auto auth_result = session_result.value()->Authenticate(request); + ASSERT_THAT(auth_result, IsOk()); + const auto& headers = auth_result.value().headers; EXPECT_EQ(headers.find("original-authorization"), headers.end()); } @@ -208,11 +215,12 @@ TEST_F(SigV4AuthTest, TableSessionInheritsProperties) { catalog_session.value()); ASSERT_THAT(table_session, IsOk()); - std::unordered_map headers; - HTTPRequestContext ctx{HttpMethod::kGet, "https://example.com/v1/ns1/tables/table1", - ""}; - ASSERT_THAT(table_session.value()->Authenticate(headers, ctx), IsOk()); - EXPECT_NE(headers.find("authorization"), headers.end()); + HTTPRequest request{.method = HttpMethod::kGet, + .url = "https://example.com/v1/ns1/tables/table1"}; + auto auth_result = table_session.value()->Authenticate(request); + ASSERT_THAT(auth_result, IsOk()); + EXPECT_NE(auth_result.value().headers.find("authorization"), + auth_result.value().headers.end()); } // ---------- Tests ported from Java TestRESTSigV4AuthSession ---------- @@ -226,13 +234,15 @@ TEST_F(SigV4AuthTest, AuthenticateWithoutBodyDetailedHeaders) { auto session_result = manager_result.value()->CatalogSession(client_, properties); ASSERT_THAT(session_result, IsOk()); - std::unordered_map headers; - headers["Content-Type"] = "application/json"; - HTTPRequestContext ctx{HttpMethod::kGet, "http://localhost:8080/path", ""}; - ASSERT_THAT(session_result.value()->Authenticate(headers, ctx), IsOk()); + HTTPRequest request{.method = HttpMethod::kGet, + .url = "http://localhost:8080/path", + .headers = {{"Content-Type", "application/json"}}}; + auto auth_result = session_result.value()->Authenticate(request); + ASSERT_THAT(auth_result, IsOk()); + const auto& headers = auth_result.value().headers; // Original header preserved - EXPECT_EQ(headers["content-type"], "application/json"); + EXPECT_EQ(headers.at("content-type"), "application/json"); // Host header generated by the signer EXPECT_NE(headers.find("host"), headers.end()); @@ -248,7 +258,7 @@ TEST_F(SigV4AuthTest, AuthenticateWithoutBodyDetailedHeaders) { EXPECT_TRUE(auth_it->second.find("x-amz-date") != std::string::npos); // Empty body SHA256 hash - EXPECT_EQ(headers["x-amz-content-sha256"], SigV4AuthSession::kEmptyBodySha256); + EXPECT_EQ(headers.at("x-amz-content-sha256"), SigV4AuthSession::kEmptyBodySha256); // X-Amz-Date present EXPECT_NE(headers.find("x-amz-date"), headers.end()); @@ -263,11 +273,13 @@ TEST_F(SigV4AuthTest, AuthenticateWithBodyDetailedHeaders) { auto session_result = manager_result.value()->CatalogSession(client_, properties); ASSERT_THAT(session_result, IsOk()); - std::unordered_map headers; - headers["Content-Type"] = "application/json"; - std::string body = R"({"namespace":["ns1"]})"; - HTTPRequestContext ctx{HttpMethod::kPost, "http://localhost:8080/path", body}; - ASSERT_THAT(session_result.value()->Authenticate(headers, ctx), IsOk()); + HTTPRequest request{.method = HttpMethod::kPost, + .url = "http://localhost:8080/path", + .headers = {{"Content-Type", "application/json"}}, + .body = R"({"namespace":["ns1"]})"}; + auto auth_result = session_result.value()->Authenticate(request); + ASSERT_THAT(auth_result, IsOk()); + const auto& headers = auth_result.value().headers; // SigV4 Authorization header auto auth_it = headers.find("authorization"); @@ -295,10 +307,12 @@ TEST_F(SigV4AuthTest, ConflictingAuthorizationHeaderIncludedInSignedHeaders) { auto session_result = manager_result.value()->CatalogSession(client_, properties); ASSERT_THAT(session_result, IsOk()); - std::unordered_map headers; - headers["Content-Type"] = "application/json"; - HTTPRequestContext ctx{HttpMethod::kGet, "http://localhost:8080/path", ""}; - ASSERT_THAT(session_result.value()->Authenticate(headers, ctx), IsOk()); + HTTPRequest request{.method = HttpMethod::kGet, + .url = "http://localhost:8080/path", + .headers = {{"Content-Type", "application/json"}}}; + auto auth_result = session_result.value()->Authenticate(request); + ASSERT_THAT(auth_result, IsOk()); + const auto& headers = auth_result.value().headers; // SigV4 Authorization header auto auth_it = headers.find("authorization"); @@ -329,12 +343,13 @@ TEST_F(SigV4AuthTest, ConflictingSigV4HeadersRelocated) { auto session = std::make_shared(delegate, "us-east-1", "execute-api", credentials); - std::unordered_map headers; - HTTPRequestContext ctx{HttpMethod::kGet, "http://localhost:8080/path", ""}; - ASSERT_THAT(session->Authenticate(headers, ctx), IsOk()); + HTTPRequest request{.method = HttpMethod::kGet, .url = "http://localhost:8080/path"}; + auto auth_result = session->Authenticate(request); + ASSERT_THAT(auth_result, IsOk()); + const auto& headers = auth_result.value().headers; // The real x-amz-content-sha256 should be the empty body hash (signer overwrites fake) - EXPECT_EQ(headers["x-amz-content-sha256"], SigV4AuthSession::kEmptyBodySha256); + EXPECT_EQ(headers.at("x-amz-content-sha256"), SigV4AuthSession::kEmptyBodySha256); // The fake values should be relocated since the signer produced different values auto orig_sha_it = headers.find("Original-x-amz-content-sha256"); @@ -380,13 +395,12 @@ TEST_F(SigV4AuthTest, CreateCustomDelegateNone) { ASSERT_THAT(session_result, IsOk()); // Authenticate should work with noop delegate - std::unordered_map headers; - HTTPRequestContext ctx{HttpMethod::kGet, "https://example.com/v1/config", ""}; - ASSERT_THAT(session_result.value()->Authenticate(headers, ctx), IsOk()); + HTTPRequest request{.method = HttpMethod::kGet, .url = "https://example.com/v1/config"}; + auto auth_result = session_result.value()->Authenticate(request); + ASSERT_THAT(auth_result, IsOk()); + const auto& headers = auth_result.value().headers; EXPECT_NE(headers.find("authorization"), headers.end()); - - EXPECT_EQ(headers.find("original-authorization"), headers.end()); EXPECT_EQ(headers.find("original-authorization"), headers.end()); } @@ -428,9 +442,10 @@ TEST_F(SigV4AuthTest, ContextualSessionOverridesProperties) { manager_result.value()->ContextualSession(context, catalog_session.value()); ASSERT_THAT(ctx_session, IsOk()); - std::unordered_map headers; - HTTPRequestContext req_ctx{HttpMethod::kGet, "https://example.com/v1/config", ""}; - ASSERT_THAT(ctx_session.value()->Authenticate(headers, req_ctx), IsOk()); + HTTPRequest request{.method = HttpMethod::kGet, .url = "https://example.com/v1/config"}; + auto auth_result = ctx_session.value()->Authenticate(request); + ASSERT_THAT(auth_result, IsOk()); + const auto& headers = auth_result.value().headers; auto auth_it = headers.find("authorization"); ASSERT_NE(auth_it, headers.end()); @@ -462,10 +477,11 @@ TEST_F(SigV4AuthTest, TableSessionOverridesProperties) { catalog_session.value()); ASSERT_THAT(table_session, IsOk()); - std::unordered_map headers; - HTTPRequestContext req_ctx{HttpMethod::kGet, "https://example.com/v1/db1/tables/table1", - ""}; - ASSERT_THAT(table_session.value()->Authenticate(headers, req_ctx), IsOk()); + HTTPRequest request{.method = HttpMethod::kGet, + .url = "https://example.com/v1/db1/tables/table1"}; + auto auth_result = table_session.value()->Authenticate(request); + ASSERT_THAT(auth_result, IsOk()); + const auto& headers = auth_result.value().headers; auto auth_it = headers.find("authorization"); ASSERT_NE(auth_it, headers.end()); From 4197ceb71cc8522dafba9c39b4fb4de7f0168ff5 Mon Sep 17 00:00:00 2001 From: Li Jiajia Date: Tue, 14 Apr 2026 23:02:30 +0800 Subject: [PATCH 07/10] move MakeSigV4AuthManager to sigv4_auth_manager.cc --- .../catalog/rest/auth/auth_managers.cc | 29 ------------------- .../catalog/rest/auth/sigv4_auth_manager.cc | 24 +++++++++++++++ 2 files changed, 24 insertions(+), 29 deletions(-) diff --git a/src/iceberg/catalog/rest/auth/auth_managers.cc b/src/iceberg/catalog/rest/auth/auth_managers.cc index 67d6f9634..0a1d12788 100644 --- a/src/iceberg/catalog/rest/auth/auth_managers.cc +++ b/src/iceberg/catalog/rest/auth/auth_managers.cc @@ -22,9 +22,6 @@ #include #include "iceberg/catalog/rest/auth/auth_manager_internal.h" -#ifdef ICEBERG_BUILD_SIGV4 -# include "iceberg/catalog/rest/auth/sigv4_auth_manager.h" -#endif #include "iceberg/catalog/rest/auth/auth_properties.h" #include "iceberg/util/string_util.h" @@ -105,30 +102,4 @@ Result> AuthManagers::Load( return it->second(name, properties); } -#ifdef ICEBERG_BUILD_SIGV4 -Result> MakeSigV4AuthManager( - std::string_view name, - const std::unordered_map& properties) { - // Determine the delegate auth type. Default to OAuth2 if not specified. - std::string delegate_type = AuthProperties::kAuthTypeOAuth2; - auto it = properties.find(AuthProperties::kSigV4DelegateAuthType); - if (it != properties.end() && !it->second.empty()) { - delegate_type = StringUtils::ToLower(it->second); - } - - // Prevent circular delegation (sigv4 -> sigv4 -> ...). - ICEBERG_PRECHECK(delegate_type != AuthProperties::kAuthTypeSigV4, - "Cannot delegate a SigV4 auth manager to another SigV4 auth " - "manager (delegate_type='{}')", - delegate_type); - - // Load the delegate auth manager. - auto delegate_props = properties; - delegate_props[AuthProperties::kAuthType] = delegate_type; - - ICEBERG_ASSIGN_OR_RAISE(auto delegate, AuthManagers::Load(name, delegate_props)); - return std::make_unique(std::move(delegate)); -} -#endif - } // namespace iceberg::rest::auth diff --git a/src/iceberg/catalog/rest/auth/sigv4_auth_manager.cc b/src/iceberg/catalog/rest/auth/sigv4_auth_manager.cc index bca6c8f55..3fd918ceb 100644 --- a/src/iceberg/catalog/rest/auth/sigv4_auth_manager.cc +++ b/src/iceberg/catalog/rest/auth/sigv4_auth_manager.cc @@ -29,6 +29,8 @@ #include #include +#include "iceberg/catalog/rest/auth/auth_manager_internal.h" +#include "iceberg/catalog/rest/auth/auth_managers.h" #include "iceberg/catalog/rest/auth/auth_properties.h" #include "iceberg/catalog/rest/endpoint.h" #include "iceberg/util/checked_cast.h" @@ -292,4 +294,26 @@ Result> SigV4AuthManager::WrapSession( std::move(credentials)); } +Result> MakeSigV4AuthManager( + std::string_view name, + const std::unordered_map& properties) { + // Default to OAuth2 when delegate type is not specified. + std::string delegate_type = AuthProperties::kAuthTypeOAuth2; + if (auto it = properties.find(AuthProperties::kSigV4DelegateAuthType); + it != properties.end() && !it->second.empty()) { + delegate_type = StringUtils::ToLower(it->second); + } + + // Prevent circular delegation (sigv4 -> sigv4 -> ...). + ICEBERG_PRECHECK(delegate_type != AuthProperties::kAuthTypeSigV4, + "Cannot delegate a SigV4 auth manager to another SigV4 auth " + "manager (delegate_type='{}')", + delegate_type); + + auto delegate_props = properties; + delegate_props[AuthProperties::kAuthType] = delegate_type; + ICEBERG_ASSIGN_OR_RAISE(auto delegate, AuthManagers::Load(name, delegate_props)); + return std::make_unique(std::move(delegate)); +} + } // namespace iceberg::rest::auth From f26927e334823c25ad06f872eacbf77c662b899e Mon Sep 17 00:00:00 2001 From: Li Jiajia Date: Tue, 14 Apr 2026 23:42:50 +0800 Subject: [PATCH 08/10] Meson: wire SigV4 behind a feature option --- meson.options | 7 ++ .../catalog/rest/auth/sigv4_auth_manager.cc | 39 ++++---- .../catalog/rest/auth/sigv4_auth_manager.h | 24 +++-- src/iceberg/catalog/rest/http_client.cc | 96 ++++++++----------- src/iceberg/catalog/rest/meson.build | 34 +++++-- src/iceberg/test/meson.build | 8 ++ src/iceberg/test/sigv4_auth_test.cc | 41 +++++++- 7 files changed, 153 insertions(+), 96 deletions(-) diff --git a/meson.options b/meson.options index 9152af34d..c53574889 100644 --- a/meson.options +++ b/meson.options @@ -44,4 +44,11 @@ option( value: 'disabled', ) +option( + 'sigv4', + type: 'feature', + description: 'Build AWS SigV4 authentication support for rest catalog', + value: 'disabled', +) + option('tests', type: 'feature', description: 'Build tests', value: 'enabled') diff --git a/src/iceberg/catalog/rest/auth/sigv4_auth_manager.cc b/src/iceberg/catalog/rest/auth/sigv4_auth_manager.cc index 3fd918ceb..bf9fc5da4 100644 --- a/src/iceberg/catalog/rest/auth/sigv4_auth_manager.cc +++ b/src/iceberg/catalog/rest/auth/sigv4_auth_manager.cc @@ -20,6 +20,7 @@ #include "iceberg/catalog/rest/auth/sigv4_auth_manager.h" #include +#include #include #include @@ -107,13 +108,15 @@ class RestSigV4Signer : public Aws::Client::AWSAuthV4Signer { SigV4AuthSession::SigV4AuthSession( std::shared_ptr delegate, std::string signing_region, std::string signing_name, - std::shared_ptr credentials_provider) + std::shared_ptr credentials_provider, + std::unordered_map effective_properties) : delegate_(std::move(delegate)), signing_region_(std::move(signing_region)), signing_name_(std::move(signing_name)), credentials_provider_(std::move(credentials_provider)), signer_(std::make_unique( - credentials_provider_, signing_name_.c_str(), signing_region_.c_str())) {} + credentials_provider_, signing_name_.c_str(), signing_region_.c_str())), + effective_properties_(std::move(effective_properties)) {} SigV4AuthSession::~SigV4AuthSession() = default; @@ -121,7 +124,6 @@ Result SigV4AuthSession::Authenticate(const HTTPRequest& request) { ICEBERG_ASSIGN_OR_RAISE(auto delegate_request, delegate_->Authenticate(request)); const auto& original_headers = delegate_request.headers; - // Relocate any delegate-set Authorization so SigV4 takes precedence. std::unordered_map signing_headers; for (const auto& [name, value] : original_headers) { if (StringUtils::EqualsIgnoreCase(name, "Authorization")) { @@ -138,8 +140,8 @@ Result SigV4AuthSession::Authenticate(const HTTPRequest& request) { aws_request->SetHeaderValue(Aws::String(name.c_str()), Aws::String(value.c_str())); } - // Empty body uses hex EMPTY_BODY_SHA256 (Java workaround for the signer - // producing an invalid checksum on empty bodies); non-empty uses Base64. + // Empty body: hex EMPTY_BODY_SHA256 (Java parity workaround for the signer + // computing an invalid checksum on empty bodies). Non-empty: Base64. if (delegate_request.body.empty()) { aws_request->SetHeaderValue("x-amz-content-sha256", Aws::String(kEmptyBodySha256)); } else { @@ -152,12 +154,14 @@ Result SigV4AuthSession::Authenticate(const HTTPRequest& request) { Aws::Utils::HashingUtils::Base64Encode(sha256)); } - if (!signer_->SignRequest(*aws_request)) { - return std::unexpected( - Error{ErrorKind::kAuthenticationFailed, "SigV4 signing failed"}); + { + std::lock_guard lock(signing_mutex_); + if (!signer_->SignRequest(*aws_request)) { + return std::unexpected( + Error{ErrorKind::kAuthenticationFailed, "SigV4 signing failed"}); + } } - // Fill headers with the signed set, relocating any conflicting originals. HTTPRequest signed_request{.method = delegate_request.method, .url = std::move(delegate_request.url), .headers = {}, @@ -200,7 +204,6 @@ Result> SigV4AuthManager::CatalogSession( HttpClient& shared_client, const std::unordered_map& properties) { AwsSdkGuard::EnsureInitialized(); - catalog_properties_ = properties; ICEBERG_ASSIGN_OR_RAISE(auto delegate_session, delegate_->CatalogSession(shared_client, properties)); return WrapSession(std::move(delegate_session), properties); @@ -214,8 +217,8 @@ Result> SigV4AuthManager::ContextualSession( ICEBERG_ASSIGN_OR_RAISE(auto delegate_session, delegate_->ContextualSession( context, sigv4_parent->delegate())); - auto merged = MergeProperties(catalog_properties_, context); - return WrapSession(std::move(delegate_session), merged); + auto merged = MergeProperties(sigv4_parent->effective_properties(), context); + return WrapSession(std::move(delegate_session), std::move(merged)); } Result> SigV4AuthManager::TableSession( @@ -228,8 +231,8 @@ Result> SigV4AuthManager::TableSession( auto delegate_session, delegate_->TableSession(table, properties, sigv4_parent->delegate())); - auto merged = MergeProperties(catalog_properties_, properties); - return WrapSession(std::move(delegate_session), merged); + auto merged = MergeProperties(sigv4_parent->effective_properties(), properties); + return WrapSession(std::move(delegate_session), std::move(merged)); } Status SigV4AuthManager::Close() { return delegate_->Close(); } @@ -285,13 +288,13 @@ std::string SigV4AuthManager::ResolveSigningName( Result> SigV4AuthManager::WrapSession( std::shared_ptr delegate_session, - const std::unordered_map& properties) { + std::unordered_map properties) { auto region = ResolveSigningRegion(properties); auto service = ResolveSigningName(properties); ICEBERG_ASSIGN_OR_RAISE(auto credentials, MakeCredentialsProvider(properties)); - return std::make_shared(std::move(delegate_session), - std::move(region), std::move(service), - std::move(credentials)); + return std::make_shared( + std::move(delegate_session), std::move(region), std::move(service), + std::move(credentials), std::move(properties)); } Result> MakeSigV4AuthManager( diff --git a/src/iceberg/catalog/rest/auth/sigv4_auth_manager.h b/src/iceberg/catalog/rest/auth/sigv4_auth_manager.h index 48cc0eb2e..88dcc789d 100644 --- a/src/iceberg/catalog/rest/auth/sigv4_auth_manager.h +++ b/src/iceberg/catalog/rest/auth/sigv4_auth_manager.h @@ -20,6 +20,7 @@ #pragma once #include +#include #include #include @@ -47,8 +48,8 @@ namespace iceberg::rest::auth { /// /// See https://docs.aws.amazon.com/IAM/latest/UserGuide/reference_sigv.html /// -/// Thread safety: Authenticate() is NOT thread-safe. Each session should be used -/// from a single thread, or callers must synchronize externally. +/// Thread safety: Authenticate() is thread-safe; concurrent signing calls are +/// serialized by an internal mutex. class ICEBERG_REST_EXPORT SigV4AuthSession : public AuthSession { public: /// SHA-256 hash of empty string, used for requests with no body. @@ -61,7 +62,8 @@ class ICEBERG_REST_EXPORT SigV4AuthSession : public AuthSession { SigV4AuthSession( std::shared_ptr delegate, std::string signing_region, std::string signing_name, - std::shared_ptr credentials_provider); + std::shared_ptr credentials_provider, + std::unordered_map effective_properties); ~SigV4AuthSession() override; @@ -71,20 +73,27 @@ class ICEBERG_REST_EXPORT SigV4AuthSession : public AuthSession { const std::shared_ptr& delegate() const { return delegate_; } + /// Merged properties this session was built from. Child sessions inherit + /// from this (not the catalog's) so contextual overrides propagate into + /// table sessions. + const std::unordered_map& effective_properties() const { + return effective_properties_; + } + private: std::shared_ptr delegate_; std::string signing_region_; std::string signing_name_; std::shared_ptr credentials_provider_; std::unique_ptr signer_; + std::unordered_map effective_properties_; + // AWSAuthV4Signer::SignRequest mutates shared signer state. + mutable std::mutex signing_mutex_; }; /// \brief An AuthManager that produces SigV4AuthSession instances. /// /// Wraps a delegate AuthManager to handle double authentication (e.g., OAuth2 + SigV4). -/// -/// Thread safety: CatalogSession() must be called before ContextualSession() or -/// TableSession(). Concurrent calls are NOT safe — callers must synchronize externally. class ICEBERG_REST_EXPORT SigV4AuthManager : public AuthManager { public: explicit SigV4AuthManager(std::unique_ptr delegate); @@ -118,10 +127,9 @@ class ICEBERG_REST_EXPORT SigV4AuthManager : public AuthManager { const std::unordered_map& properties); Result> WrapSession( std::shared_ptr delegate_session, - const std::unordered_map& properties); + std::unordered_map properties); std::unique_ptr delegate_; - std::unordered_map catalog_properties_; }; } // namespace iceberg::rest::auth diff --git a/src/iceberg/catalog/rest/http_client.cc b/src/iceberg/catalog/rest/http_client.cc index 8dec6f239..b37995833 100644 --- a/src/iceberg/catalog/rest/http_client.cc +++ b/src/iceberg/catalog/rest/http_client.cc @@ -81,21 +81,8 @@ std::unordered_map MergeHeaders( return merged; } -/// \brief Authenticate the request and return the final cpr::Header. -Result AuthenticateRequest(const auth::HTTPRequest& request, - auth::AuthSession& session) { - ICEBERG_ASSIGN_OR_RAISE(auto authenticated, session.Authenticate(request)); - return cpr::Header(authenticated.headers.begin(), authenticated.headers.end()); -} - -/// \brief Converts a map of string key-value pairs to cpr::Parameters. -cpr::Parameters GetParameters( - const std::unordered_map& params) { - cpr::Parameters cpr_params; - for (const auto& [key, val] : params) { - cpr_params.Add({key, val}); - } - return cpr_params; +cpr::Header ToCprHeader(const auth::HTTPRequest& request) { + return cpr::Header(request.headers.begin(), request.headers.end()); } /// \brief Append URL-encoded query parameters to a URL, sorted by key. @@ -175,14 +162,13 @@ Result HttpClient::Get( const std::unordered_map& headers, const ErrorHandler& error_handler, auth::AuthSession& session) { ICEBERG_ASSIGN_OR_RAISE( - auto all_headers, - AuthenticateRequest({.method = HttpMethod::kGet, - .url = AppendQueryString(path, params), - .headers = MergeHeaders(default_headers_, headers), - .body = ""}, - session)); - cpr::Response response = - cpr::Get(cpr::Url{path}, GetParameters(params), all_headers, *connection_pool_); + auto authenticated, + session.Authenticate({.method = HttpMethod::kGet, + .url = AppendQueryString(path, params), + .headers = MergeHeaders(default_headers_, headers), + .body = ""})); + cpr::Response response = cpr::Get(cpr::Url{authenticated.url}, + ToCprHeader(authenticated), *connection_pool_); ICEBERG_RETURN_UNEXPECTED(HandleFailureResponse(response, error_handler)); HttpResponse http_response; @@ -195,14 +181,14 @@ Result HttpClient::Post( const std::unordered_map& headers, const ErrorHandler& error_handler, auth::AuthSession& session) { ICEBERG_ASSIGN_OR_RAISE( - auto all_headers, - AuthenticateRequest({.method = HttpMethod::kPost, - .url = path, - .headers = MergeHeaders(default_headers_, headers), - .body = body}, - session)); + auto authenticated, + session.Authenticate({.method = HttpMethod::kPost, + .url = path, + .headers = MergeHeaders(default_headers_, headers), + .body = body})); cpr::Response response = - cpr::Post(cpr::Url{path}, cpr::Body{body}, all_headers, *connection_pool_); + cpr::Post(cpr::Url{authenticated.url}, cpr::Body{authenticated.body}, + ToCprHeader(authenticated), *connection_pool_); ICEBERG_RETURN_UNEXPECTED(HandleFailureResponse(response, error_handler)); HttpResponse http_response; @@ -222,19 +208,18 @@ Result HttpClient::PostForm( for (const auto& [key, val] : form_data) { pair_list.emplace_back(key, val); } - // Use cpr's own encoding as the signing body to ensure consistency with the - // actual payload sent over the wire. - cpr::Payload payload(pair_list.begin(), pair_list.end()); - std::string encoded_body = payload.GetContent(); + // Sign the exact bytes cpr will put on the wire. + std::string encoded_body = + cpr::Payload(pair_list.begin(), pair_list.end()).GetContent(); ICEBERG_ASSIGN_OR_RAISE( - auto all_headers, - AuthenticateRequest({.method = HttpMethod::kPost, - .url = path, - .headers = MergeHeaders(default_headers_, form_headers), - .body = encoded_body}, - session)); + auto authenticated, + session.Authenticate({.method = HttpMethod::kPost, + .url = path, + .headers = MergeHeaders(default_headers_, form_headers), + .body = std::move(encoded_body)})); cpr::Response response = - cpr::Post(cpr::Url{path}, std::move(payload), all_headers, *connection_pool_); + cpr::Post(cpr::Url{authenticated.url}, cpr::Body{authenticated.body}, + ToCprHeader(authenticated), *connection_pool_); ICEBERG_RETURN_UNEXPECTED(HandleFailureResponse(response, error_handler)); HttpResponse http_response; @@ -246,13 +231,13 @@ Result HttpClient::Head( const std::string& path, const std::unordered_map& headers, const ErrorHandler& error_handler, auth::AuthSession& session) { ICEBERG_ASSIGN_OR_RAISE( - auto all_headers, - AuthenticateRequest({.method = HttpMethod::kHead, - .url = path, - .headers = MergeHeaders(default_headers_, headers), - .body = ""}, - session)); - cpr::Response response = cpr::Head(cpr::Url{path}, all_headers, *connection_pool_); + auto authenticated, + session.Authenticate({.method = HttpMethod::kHead, + .url = path, + .headers = MergeHeaders(default_headers_, headers), + .body = ""})); + cpr::Response response = cpr::Head(cpr::Url{authenticated.url}, + ToCprHeader(authenticated), *connection_pool_); ICEBERG_RETURN_UNEXPECTED(HandleFailureResponse(response, error_handler)); HttpResponse http_response; @@ -265,14 +250,13 @@ Result HttpClient::Delete( const std::unordered_map& headers, const ErrorHandler& error_handler, auth::AuthSession& session) { ICEBERG_ASSIGN_OR_RAISE( - auto all_headers, - AuthenticateRequest({.method = HttpMethod::kDelete, - .url = AppendQueryString(path, params), - .headers = MergeHeaders(default_headers_, headers), - .body = ""}, - session)); - cpr::Response response = - cpr::Delete(cpr::Url{path}, GetParameters(params), all_headers, *connection_pool_); + auto authenticated, + session.Authenticate({.method = HttpMethod::kDelete, + .url = AppendQueryString(path, params), + .headers = MergeHeaders(default_headers_, headers), + .body = ""})); + cpr::Response response = cpr::Delete(cpr::Url{authenticated.url}, + ToCprHeader(authenticated), *connection_pool_); ICEBERG_RETURN_UNEXPECTED(HandleFailureResponse(response, error_handler)); HttpResponse http_response; diff --git a/src/iceberg/catalog/rest/meson.build b/src/iceberg/catalog/rest/meson.build index ef2500456..6ab8bc42d 100644 --- a/src/iceberg/catalog/rest/meson.build +++ b/src/iceberg/catalog/rest/meson.build @@ -40,16 +40,26 @@ cpr_needs_static = ( cpr_dep = dependency('cpr', static: cpr_needs_static) iceberg_rest_build_deps = [iceberg_dep, cpr_dep] +iceberg_rest_compile_defs = [] + +sigv4_opt = get_option('sigv4') +aws_sdk_core_dep = dependency('aws-cpp-sdk-core', required: sigv4_opt) +if aws_sdk_core_dep.found() + iceberg_rest_sources += files('auth/sigv4_auth_manager.cc') + iceberg_rest_build_deps += aws_sdk_core_dep + iceberg_rest_compile_defs += '-DICEBERG_BUILD_SIGV4' +endif + iceberg_rest_lib = library( 'iceberg_rest', sources: iceberg_rest_sources, dependencies: iceberg_rest_build_deps, gnu_symbol_visibility: 'hidden', - cpp_shared_args: ['-DICEBERG_REST_EXPORTING'], - cpp_static_args: ['-DICEBERG_REST_STATIC'], + cpp_shared_args: ['-DICEBERG_REST_EXPORTING'] + iceberg_rest_compile_defs, + cpp_static_args: ['-DICEBERG_REST_STATIC'] + iceberg_rest_compile_defs, ) -iceberg_rest_compile_args = [] +iceberg_rest_compile_args = iceberg_rest_compile_defs if get_option('default_library') == 'static' iceberg_rest_compile_args += ['-DICEBERG_REST_STATIC'] endif @@ -78,13 +88,17 @@ install_headers( subdir: 'iceberg/catalog/rest', ) +iceberg_rest_auth_headers = [ + 'auth/auth_manager.h', + 'auth/auth_managers.h', + 'auth/auth_properties.h', + 'auth/auth_session.h', + 'auth/oauth2_util.h', +] +if aws_sdk_core_dep.found() + iceberg_rest_auth_headers += ['auth/sigv4_auth_manager.h'] +endif install_headers( - [ - 'auth/auth_manager.h', - 'auth/auth_managers.h', - 'auth/auth_properties.h', - 'auth/auth_session.h', - 'auth/oauth2_util.h', - ], + iceberg_rest_auth_headers, subdir: 'iceberg/catalog/rest/auth', ) diff --git a/src/iceberg/test/meson.build b/src/iceberg/test/meson.build index 9a8da9dd5..ac93ff138 100644 --- a/src/iceberg/test/meson.build +++ b/src/iceberg/test/meson.build @@ -116,6 +116,14 @@ if get_option('rest').enabled() 'dependencies': [iceberg_rest_dep], }, } + if aws_sdk_core_dep.found() + iceberg_tests += { + 'sigv4_auth_test': { + 'sources': files('sigv4_auth_test.cc'), + 'dependencies': [iceberg_rest_dep, aws_sdk_core_dep], + }, + } + endif if get_option('rest_integration_test').enabled() if host_machine.system() == 'windows' warning('Cannot build rest integration test on Windows, skipping.') diff --git a/src/iceberg/test/sigv4_auth_test.cc b/src/iceberg/test/sigv4_auth_test.cc index caa9b5603..fafadb7a4 100644 --- a/src/iceberg/test/sigv4_auth_test.cc +++ b/src/iceberg/test/sigv4_auth_test.cc @@ -340,8 +340,9 @@ TEST_F(SigV4AuthTest, ConflictingSigV4HeadersRelocated) { auto credentials = std::make_shared(Aws::Auth::AWSCredentials( "AKIAIOSFODNN7EXAMPLE", "wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY")); - auto session = std::make_shared(delegate, "us-east-1", "execute-api", - credentials); + auto session = std::make_shared( + delegate, "us-east-1", "execute-api", credentials, + std::unordered_map{}); HTTPRequest request{.method = HttpMethod::kGet, .url = "http://localhost:8080/path"}; auto auth_result = session->Authenticate(request); @@ -369,8 +370,9 @@ TEST_F(SigV4AuthTest, SessionCloseDelegatesToInner) { auto delegate = AuthSession::MakeDefault({}); auto credentials = std::make_shared( Aws::Auth::AWSCredentials("id", "secret")); - auto session = std::make_shared(delegate, "us-east-1", "execute-api", - credentials); + auto session = std::make_shared( + delegate, "us-east-1", "execute-api", credentials, + std::unordered_map{}); // Close should succeed without error EXPECT_THAT(session->Close(), IsOk()); @@ -490,6 +492,37 @@ TEST_F(SigV4AuthTest, TableSessionOverridesProperties) { << "Expected ap-southeast-1 in Authorization, got: " << auth_it->second; } +TEST_F(SigV4AuthTest, TableSessionInheritsContextualOverrides) { + auto properties = MakeSigV4Properties(); + properties[AuthProperties::kSigV4SigningRegion] = "us-west-2"; + + auto manager_result = AuthManagers::Load("test-catalog", properties); + ASSERT_THAT(manager_result, IsOk()); + + auto catalog_session = manager_result.value()->CatalogSession(client_, properties); + ASSERT_THAT(catalog_session, IsOk()); + + auto ctx_session = manager_result.value()->ContextualSession( + {{AuthProperties::kSigV4SigningRegion, "eu-west-1"}}, catalog_session.value()); + ASSERT_THAT(ctx_session, IsOk()); + + iceberg::TableIdentifier table_id{iceberg::Namespace{{"db1"}}, "table1"}; + auto table_session = manager_result.value()->TableSession(table_id, /*properties=*/{}, + ctx_session.value()); + ASSERT_THAT(table_session, IsOk()); + + HTTPRequest request{.method = HttpMethod::kGet, + .url = "https://example.com/v1/db1/tables/table1"}; + auto auth_result = table_session.value()->Authenticate(request); + ASSERT_THAT(auth_result, IsOk()); + + auto auth_it = auth_result.value().headers.find("authorization"); + ASSERT_NE(auth_it, auth_result.value().headers.end()); + EXPECT_TRUE(auth_it->second.find("eu-west-1") != std::string::npos) + << "Table session should inherit eu-west-1 from contextual parent, got: " + << auth_it->second; +} + // Java: close (TestRESTSigV4AuthManager) TEST_F(SigV4AuthTest, ManagerCloseDelegatesToInner) { auto properties = MakeSigV4Properties(); From 655be23b14b2baab47b1473098a8b0b8dc3a0ab3 Mon Sep 17 00:00:00 2001 From: Li Jiajia Date: Wed, 15 Apr 2026 12:23:09 +0800 Subject: [PATCH 09/10] drop unnecessary signing mutex --- .../catalog/rest/auth/sigv4_auth_manager.cc | 14 +++++--------- src/iceberg/catalog/rest/auth/sigv4_auth_manager.h | 7 ++----- src/iceberg/catalog/rest/http_client.cc | 2 +- src/iceberg/catalog/rest/meson.build | 5 +---- src/iceberg/test/sigv4_auth_test.cc | 6 +++--- 5 files changed, 12 insertions(+), 22 deletions(-) diff --git a/src/iceberg/catalog/rest/auth/sigv4_auth_manager.cc b/src/iceberg/catalog/rest/auth/sigv4_auth_manager.cc index bf9fc5da4..e334f08f2 100644 --- a/src/iceberg/catalog/rest/auth/sigv4_auth_manager.cc +++ b/src/iceberg/catalog/rest/auth/sigv4_auth_manager.cc @@ -20,7 +20,6 @@ #include "iceberg/catalog/rest/auth/sigv4_auth_manager.h" #include -#include #include #include @@ -154,12 +153,9 @@ Result SigV4AuthSession::Authenticate(const HTTPRequest& request) { Aws::Utils::HashingUtils::Base64Encode(sha256)); } - { - std::lock_guard lock(signing_mutex_); - if (!signer_->SignRequest(*aws_request)) { - return std::unexpected( - Error{ErrorKind::kAuthenticationFailed, "SigV4 signing failed"}); - } + if (!signer_->SignRequest(*aws_request)) { + return std::unexpected(Error{.kind = ErrorKind::kAuthenticationFailed, + .message = "SigV4 signing failed"}); } HTTPRequest signed_request{.method = delegate_request.method, @@ -269,10 +265,10 @@ std::string SigV4AuthManager::ResolveSigningRegion( return it->second; } if (const char* env = std::getenv("AWS_REGION")) { - return std::string(env); + return {env}; } if (const char* env = std::getenv("AWS_DEFAULT_REGION")) { - return std::string(env); + return {env}; } return "us-east-1"; } diff --git a/src/iceberg/catalog/rest/auth/sigv4_auth_manager.h b/src/iceberg/catalog/rest/auth/sigv4_auth_manager.h index 88dcc789d..4e173a284 100644 --- a/src/iceberg/catalog/rest/auth/sigv4_auth_manager.h +++ b/src/iceberg/catalog/rest/auth/sigv4_auth_manager.h @@ -20,7 +20,6 @@ #pragma once #include -#include #include #include @@ -48,8 +47,8 @@ namespace iceberg::rest::auth { /// /// See https://docs.aws.amazon.com/IAM/latest/UserGuide/reference_sigv.html /// -/// Thread safety: Authenticate() is thread-safe; concurrent signing calls are -/// serialized by an internal mutex. +/// Thread safety: Authenticate() is thread-safe as long as the delegate +/// session is. class ICEBERG_REST_EXPORT SigV4AuthSession : public AuthSession { public: /// SHA-256 hash of empty string, used for requests with no body. @@ -87,8 +86,6 @@ class ICEBERG_REST_EXPORT SigV4AuthSession : public AuthSession { std::shared_ptr credentials_provider_; std::unique_ptr signer_; std::unordered_map effective_properties_; - // AWSAuthV4Signer::SignRequest mutates shared signer state. - mutable std::mutex signing_mutex_; }; /// \brief An AuthManager that produces SigV4AuthSession instances. diff --git a/src/iceberg/catalog/rest/http_client.cc b/src/iceberg/catalog/rest/http_client.cc index b37995833..cc5e132eb 100644 --- a/src/iceberg/catalog/rest/http_client.cc +++ b/src/iceberg/catalog/rest/http_client.cc @@ -82,7 +82,7 @@ std::unordered_map MergeHeaders( } cpr::Header ToCprHeader(const auth::HTTPRequest& request) { - return cpr::Header(request.headers.begin(), request.headers.end()); + return {request.headers.begin(), request.headers.end()}; } /// \brief Append URL-encoded query parameters to a URL, sorted by key. diff --git a/src/iceberg/catalog/rest/meson.build b/src/iceberg/catalog/rest/meson.build index 6ab8bc42d..dd518b2f6 100644 --- a/src/iceberg/catalog/rest/meson.build +++ b/src/iceberg/catalog/rest/meson.build @@ -98,7 +98,4 @@ iceberg_rest_auth_headers = [ if aws_sdk_core_dep.found() iceberg_rest_auth_headers += ['auth/sigv4_auth_manager.h'] endif -install_headers( - iceberg_rest_auth_headers, - subdir: 'iceberg/catalog/rest/auth', -) +install_headers(iceberg_rest_auth_headers, subdir: 'iceberg/catalog/rest/auth') diff --git a/src/iceberg/test/sigv4_auth_test.cc b/src/iceberg/test/sigv4_auth_test.cc index fafadb7a4..fd52e8dad 100644 --- a/src/iceberg/test/sigv4_auth_test.cc +++ b/src/iceberg/test/sigv4_auth_test.cc @@ -209,7 +209,7 @@ TEST_F(SigV4AuthTest, TableSessionInheritsProperties) { auto catalog_session = manager_result.value()->CatalogSession(client_, properties); ASSERT_THAT(catalog_session, IsOk()); - iceberg::TableIdentifier table_id{iceberg::Namespace{{"ns1"}}, "table1"}; + iceberg::TableIdentifier table_id{.ns = iceberg::Namespace{{"ns1"}}, .name = "table1"}; std::unordered_map table_props; auto table_session = manager_result.value()->TableSession(table_id, table_props, catalog_session.value()); @@ -474,7 +474,7 @@ TEST_F(SigV4AuthTest, TableSessionOverridesProperties) { {AuthProperties::kSigV4SigningRegion, "ap-southeast-1"}, }; - iceberg::TableIdentifier table_id{iceberg::Namespace{{"db1"}}, "table1"}; + iceberg::TableIdentifier table_id{.ns = iceberg::Namespace{{"db1"}}, .name = "table1"}; auto table_session = manager_result.value()->TableSession(table_id, table_props, catalog_session.value()); ASSERT_THAT(table_session, IsOk()); @@ -506,7 +506,7 @@ TEST_F(SigV4AuthTest, TableSessionInheritsContextualOverrides) { {{AuthProperties::kSigV4SigningRegion, "eu-west-1"}}, catalog_session.value()); ASSERT_THAT(ctx_session, IsOk()); - iceberg::TableIdentifier table_id{iceberg::Namespace{{"db1"}}, "table1"}; + iceberg::TableIdentifier table_id{.ns = iceberg::Namespace{{"db1"}}, .name = "table1"}; auto table_session = manager_result.value()->TableSession(table_id, /*properties=*/{}, ctx_session.value()); ASSERT_THAT(table_session, IsOk()); From 39f4164f5d528cadef84d51d042306111a1c1489 Mon Sep 17 00:00:00 2001 From: Li Jiajia Date: Wed, 15 Apr 2026 15:25:40 +0800 Subject: [PATCH 10/10] address review feedback --- .../catalog/rest/auth/sigv4_auth_manager.cc | 24 ++++++++++--------- src/iceberg/catalog/rest/http_client.cc | 14 ++++++----- 2 files changed, 21 insertions(+), 17 deletions(-) diff --git a/src/iceberg/catalog/rest/auth/sigv4_auth_manager.cc b/src/iceberg/catalog/rest/auth/sigv4_auth_manager.cc index e334f08f2..2e7992772 100644 --- a/src/iceberg/catalog/rest/auth/sigv4_auth_manager.cc +++ b/src/iceberg/catalog/rest/auth/sigv4_auth_manager.cc @@ -19,13 +19,13 @@ #include "iceberg/catalog/rest/auth/sigv4_auth_manager.h" -#include #include #include #include #include #include +#include #include #include @@ -33,7 +33,6 @@ #include "iceberg/catalog/rest/auth/auth_managers.h" #include "iceberg/catalog/rest/auth/auth_properties.h" #include "iceberg/catalog/rest/endpoint.h" -#include "iceberg/util/checked_cast.h" #include "iceberg/util/macros.h" #include "iceberg/util/string_util.h" @@ -208,7 +207,9 @@ Result> SigV4AuthManager::CatalogSession( Result> SigV4AuthManager::ContextualSession( const std::unordered_map& context, std::shared_ptr parent) { - auto sigv4_parent = internal::checked_pointer_cast(std::move(parent)); + auto sigv4_parent = std::dynamic_pointer_cast(std::move(parent)); + ICEBERG_PRECHECK(sigv4_parent != nullptr, + "SigV4AuthManager parent must be a SigV4AuthSession"); ICEBERG_ASSIGN_OR_RAISE(auto delegate_session, delegate_->ContextualSession( context, sigv4_parent->delegate())); @@ -221,7 +222,9 @@ Result> SigV4AuthManager::TableSession( const TableIdentifier& table, const std::unordered_map& properties, std::shared_ptr parent) { - auto sigv4_parent = internal::checked_pointer_cast(std::move(parent)); + auto sigv4_parent = std::dynamic_pointer_cast(std::move(parent)); + ICEBERG_PRECHECK(sigv4_parent != nullptr, + "SigV4AuthManager parent must be a SigV4AuthSession"); ICEBERG_ASSIGN_OR_RAISE( auto delegate_session, @@ -233,6 +236,8 @@ Result> SigV4AuthManager::TableSession( Status SigV4AuthManager::Close() { return delegate_->Close(); } +// TODO(sigv4): support loading a custom AWSCredentialsProvider via a class +// name property, matching Java's AwsProperties.restCredentialsProvider(). Result> SigV4AuthManager::MakeCredentialsProvider( const std::unordered_map& properties) { @@ -264,13 +269,10 @@ std::string SigV4AuthManager::ResolveSigningRegion( it != properties.end() && !it->second.empty()) { return it->second; } - if (const char* env = std::getenv("AWS_REGION")) { - return {env}; - } - if (const char* env = std::getenv("AWS_DEFAULT_REGION")) { - return {env}; - } - return "us-east-1"; + // Delegates the full resolution chain (AWS_DEFAULT_REGION / AWS_REGION env, + // ~/.aws/config profile, EC2/ECS IMDS, fallback us-east-1) to the AWS SDK. + // Set AWS_EC2_METADATA_DISABLED=true to skip IMDS on non-EC2 hosts. + return {Aws::Client::ClientConfiguration().region.c_str()}; } std::string SigV4AuthManager::ResolveSigningName( diff --git a/src/iceberg/catalog/rest/http_client.cc b/src/iceberg/catalog/rest/http_client.cc index cc5e132eb..2872f9069 100644 --- a/src/iceberg/catalog/rest/http_client.cc +++ b/src/iceberg/catalog/rest/http_client.cc @@ -86,7 +86,7 @@ cpr::Header ToCprHeader(const auth::HTTPRequest& request) { } /// \brief Append URL-encoded query parameters to a URL, sorted by key. -std::string AppendQueryString( +Result AppendQueryString( const std::string& base_url, const std::unordered_map& params) { if (params.empty()) return base_url; @@ -95,9 +95,9 @@ std::string AppendQueryString( bool first = true; for (const auto& [k, v] : sorted) { if (!first) url += "&"; - auto ek = EncodeString(k); - auto ev = EncodeString(v); - url += (ek ? *ek : k) + "=" + (ev ? *ev : v); + ICEBERG_ASSIGN_OR_RAISE(auto ek, EncodeString(k)); + ICEBERG_ASSIGN_OR_RAISE(auto ev, EncodeString(v)); + url += ek + "=" + ev; first = false; } return url; @@ -161,10 +161,11 @@ Result HttpClient::Get( const std::string& path, const std::unordered_map& params, const std::unordered_map& headers, const ErrorHandler& error_handler, auth::AuthSession& session) { + ICEBERG_ASSIGN_OR_RAISE(auto url, AppendQueryString(path, params)); ICEBERG_ASSIGN_OR_RAISE( auto authenticated, session.Authenticate({.method = HttpMethod::kGet, - .url = AppendQueryString(path, params), + .url = std::move(url), .headers = MergeHeaders(default_headers_, headers), .body = ""})); cpr::Response response = cpr::Get(cpr::Url{authenticated.url}, @@ -249,10 +250,11 @@ Result HttpClient::Delete( const std::string& path, const std::unordered_map& params, const std::unordered_map& headers, const ErrorHandler& error_handler, auth::AuthSession& session) { + ICEBERG_ASSIGN_OR_RAISE(auto url, AppendQueryString(path, params)); ICEBERG_ASSIGN_OR_RAISE( auto authenticated, session.Authenticate({.method = HttpMethod::kDelete, - .url = AppendQueryString(path, params), + .url = std::move(url), .headers = MergeHeaders(default_headers_, headers), .body = ""})); cpr::Response response = cpr::Delete(cpr::Url{authenticated.url},