Skip to content

feat: Implement BasicAuthManager to support basic authentication #564

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
wgtmac merged 2 commits into from
Feb 26, 2026
Merged

feat: Implement BasicAuthManager to support basic authentication #564

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ba9354e
feat: Implement BasicAuthManager to suppoert basic authentication
Feb 13, 2026
005e4c0
feat: Implement BasicAuthManager and refactor auth manager structure
Feb 26, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions src/iceberg/catalog/rest/auth/auth_managers.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
#include "iceberg/catalog/rest/auth/auth_properties.h"
#include "iceberg/catalog/rest/auth/auth_session.h"
#include "iceberg/util/string_util.h"
#include "iceberg/util/transform_util.h"

namespace iceberg::rest::auth {

Expand Down Expand Up @@ -78,6 +79,34 @@ class NoopAuthManager : public AuthManager {
}
};

/// \brief Authentication manager that performs basic authentication.
class BasicAuthManager : public AuthManager {
Copy link
Copy Markdown
Member

@wgtmac wgtmac Feb 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks a little bit weird that BasicAuthManager and other AuthManager implementations are declared in auth_managers.h/cc not auth_manager.h/cc. It would be better to move them to the latter and make auth_managers.h/cc simply a factory.

Copy link
Copy Markdown
Contributor Author

@lishuxu lishuxu Feb 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point!
Moved NoopAuthManager and BasicAuthManager implementations to auth_manager.cc, and added auth_manager_internal.h to expose their factory functions (MakeNoopAuthManager/MakeBasicAuthManager) only to auth_managers.cc.

public:
static Result<std::unique_ptr<AuthManager>> Make(
[[maybe_unused]] std::string_view name,
[[maybe_unused]] const std::unordered_map<std::string, std::string>& properties) {
return std::make_unique<BasicAuthManager>();
}

Result<std::shared_ptr<AuthSession>> CatalogSession(
[[maybe_unused]] HttpClient& client,
const std::unordered_map<std::string, std::string>& properties) override {
auto username_it = properties.find(AuthProperties::kBasicUsername);
if (username_it == properties.end()) {
return InvalidArgument("Invalid username: missing required property '{}'",
AuthProperties::kBasicUsername);
}
auto password_it = properties.find(AuthProperties::kBasicPassword);
if (password_it == properties.end()) {
return InvalidArgument("Invalid password: missing required property '{}'",
AuthProperties::kBasicPassword);
}
Comment thread
lishuxu marked this conversation as resolved.
Outdated
std::string credential = username_it->second + ":" + password_it->second;
return AuthSession::MakeDefault(
{{"Authorization", "Basic " + TransformUtil::Base64Encode(credential)}});
}
};

template <typename T>
AuthManagerFactory MakeAuthFactory() {
return
Expand All @@ -88,6 +117,7 @@ AuthManagerFactory MakeAuthFactory() {
AuthManagerRegistry CreateDefaultRegistry() {
return {
{AuthProperties::kAuthTypeNone, MakeAuthFactory<NoopAuthManager>()},
{AuthProperties::kAuthTypeBasic, MakeAuthFactory<BasicAuthManager>()},
};
}

Expand Down
84 changes: 84 additions & 0 deletions src/iceberg/test/auth_manager_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,90 @@ TEST_F(AuthManagerTest, UnknownAuthTypeReturnsInvalidArgument) {
EXPECT_THAT(result, HasErrorMessage("Unknown authentication type"));
}

// Verifies loading BasicAuthManager with valid credentials
TEST_F(AuthManagerTest, LoadBasicAuthManager) {
std::unordered_map<std::string, std::string> properties = {
{AuthProperties::kAuthType, "basic"},
{AuthProperties::kBasicUsername, "admin"},
{AuthProperties::kBasicPassword, "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());

std::unordered_map<std::string, std::string> headers;
EXPECT_THAT(session_result.value()->Authenticate(headers), IsOk());
// base64("admin:secret") == "YWRtaW46c2VjcmV0"
EXPECT_EQ(headers["Authorization"], "Basic YWRtaW46c2VjcmV0");
}

// Verifies BasicAuthManager is case-insensitive for auth type
TEST_F(AuthManagerTest, BasicAuthTypeCaseInsensitive) {
for (const auto& auth_type : {"BASIC", "Basic", "bAsIc"}) {
std::unordered_map<std::string, std::string> properties = {
{AuthProperties::kAuthType, auth_type},
{AuthProperties::kBasicUsername, "user"},
{AuthProperties::kBasicPassword, "pass"}};
auto manager_result = AuthManagers::Load("test-catalog", properties);
ASSERT_THAT(manager_result, IsOk()) << "Failed for auth type: " << auth_type;

auto session_result = manager_result.value()->CatalogSession(client_, properties);
ASSERT_THAT(session_result, IsOk()) << "Failed for auth type: " << auth_type;

std::unordered_map<std::string, std::string> headers;
EXPECT_THAT(session_result.value()->Authenticate(headers), IsOk());
// base64("user:pass") == "dXNlcjpwYXNz"
EXPECT_EQ(headers["Authorization"], "Basic dXNlcjpwYXNz");
}
}

// Verifies BasicAuthManager fails when username is missing
TEST_F(AuthManagerTest, BasicAuthMissingUsername) {
std::unordered_map<std::string, std::string> properties = {
{AuthProperties::kAuthType, "basic"}, {AuthProperties::kBasicPassword, "secret"}};

auto manager_result = AuthManagers::Load("test-catalog", properties);
ASSERT_THAT(manager_result, IsOk());

auto session_result = manager_result.value()->CatalogSession(client_, properties);
EXPECT_THAT(session_result, IsError(ErrorKind::kInvalidArgument));
EXPECT_THAT(session_result, HasErrorMessage("Invalid username"));
}

// Verifies BasicAuthManager fails when password is missing
TEST_F(AuthManagerTest, BasicAuthMissingPassword) {
std::unordered_map<std::string, std::string> properties = {
{AuthProperties::kAuthType, "basic"}, {AuthProperties::kBasicUsername, "admin"}};

auto manager_result = AuthManagers::Load("test-catalog", properties);
ASSERT_THAT(manager_result, IsOk());

auto session_result = manager_result.value()->CatalogSession(client_, properties);
EXPECT_THAT(session_result, IsError(ErrorKind::kInvalidArgument));
EXPECT_THAT(session_result, HasErrorMessage("Invalid password"));
}

// Verifies BasicAuthManager handles special characters in credentials
TEST_F(AuthManagerTest, BasicAuthSpecialCharacters) {
std::unordered_map<std::string, std::string> properties = {
{AuthProperties::kAuthType, "basic"},
{AuthProperties::kBasicUsername, "user@domain.com"},
{AuthProperties::kBasicPassword, "p@ss:w0rd!"}};

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<std::string, std::string> headers;
EXPECT_THAT(session_result.value()->Authenticate(headers), IsOk());
// base64("user@domain.com:p@ss:w0rd!") == "dXNlckBkb21haW4uY29tOnBAc3M6dzByZCE="
EXPECT_EQ(headers["Authorization"], "Basic dXNlckBkb21haW4uY29tOnBAc3M6dzByZCE=");
}

// Verifies custom auth manager registration
TEST_F(AuthManagerTest, RegisterCustomAuthManager) {
AuthManagers::Register(
Expand Down
Loading