wgtmac commented on code in PR #479:
URL: https://github.com/apache/iceberg-cpp/pull/479#discussion_r2698357208
##########
src/iceberg/catalog/rest/CMakeLists.txt:
##########
@@ -24,7 +26,10 @@ set(ICEBERG_REST_SOURCES
resource_paths.cc
rest_catalog.cc
rest_util.cc
- types.cc)
+ types.cc
+ auth/auth_manager.cc
Review Comment:
Sort them alphabetically
##########
src/iceberg/catalog/rest/auth/auth_manager.h:
##########
@@ -0,0 +1,95 @@
+/*
+ * 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 <memory>
+#include <string>
+#include <unordered_map>
+
+#include "iceberg/catalog/rest/iceberg_rest_export.h"
+#include "iceberg/catalog/rest/type_fwd.h"
+#include "iceberg/result.h"
+#include "iceberg/table_identifier.h"
Review Comment:
```suggestion
```
We can remove this
##########
src/iceberg/catalog/rest/auth/auth_properties.h:
##########
@@ -0,0 +1,69 @@
+/*
+ * 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 <string_view>
+
+/// \file iceberg/catalog/rest/auth/auth_properties.h
+/// \brief Property keys and constants for REST catalog authentication.
+
+namespace iceberg::rest::auth {
+
+/// \brief Property keys and constants for authentication configuration.
+///
+/// This struct defines all the property keys used to configure authentication
+/// for the REST catalog. It follows the same naming conventions as Java
Iceberg.
+struct AuthProperties {
+ /// \brief Property key for specifying the authentication type.
+ static constexpr std::string_view kAuthType = "rest.auth.type";
+ /// \brief Authentication type: no authentication.
+ static constexpr std::string_view kAuthTypeNone = "none";
+ /// \brief Authentication type: HTTP Basic authentication.
+ static constexpr std::string_view kAuthTypeBasic = "basic";
+ /// \brief Authentication type: OAuth2 authentication.
+ static constexpr std::string_view kAuthTypeOAuth2 = "oauth2";
+ /// \brief Authentication type: AWS SigV4 authentication.
+ static constexpr std::string_view kAuthTypeSigV4 = "sigv4";
+ /// \brief Property key for Basic auth username.
Review Comment:
```suggestion
/// \brief Property key for Basic auth username.
```
##########
src/iceberg/catalog/rest/auth/auth_manager.h:
##########
@@ -0,0 +1,95 @@
+/*
+ * 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 <memory>
+#include <string>
+#include <unordered_map>
+
+#include "iceberg/catalog/rest/iceberg_rest_export.h"
+#include "iceberg/catalog/rest/type_fwd.h"
+#include "iceberg/result.h"
+#include "iceberg/table_identifier.h"
+
+/// \file iceberg/catalog/rest/auth/auth_manager.h
+/// \brief Authentication manager interface for REST catalog.
+
+namespace iceberg::rest::auth {
+
+/// \brief Produces authentication sessions for catalog and table requests.
+///
+/// AuthManager is responsible for creating authentication sessions at
different scopes:
+/// - InitSession: Short-lived session for catalog initialization (optional)
+/// - CatalogSession: Long-lived session for catalog-level operations
(required)
+/// - TableSession: Optional table-specific session or reuse of catalog session
+///
+/// Implementations are registered via AuthManagers::Register() and loaded by
auth type.
Review Comment:
```suggestion
```
We don't need such verbose comments.
##########
src/iceberg/catalog/rest/auth/auth_session.h:
##########
@@ -0,0 +1,94 @@
+/*
+ * 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 <memory>
+#include <string>
+#include <unordered_map>
+
+#include "iceberg/catalog/rest/iceberg_rest_export.h"
+#include "iceberg/result.h"
+
+/// \file iceberg/catalog/rest/auth/auth_session.h
+/// \brief Authentication session interface for REST catalog.
+
+namespace iceberg::rest::auth {
+
+/// \brief An authentication session that can authenticate outgoing HTTP
requests.
+///
+/// Authentication sessions are typically immutable, but may hold resources
that need
+/// to be released when the session is no longer needed (e.g., token refresh
threads).
+/// Implementations should override Close() to release any such resources.
+///
+class ICEBERG_REST_EXPORT AuthSession {
+ public:
+ virtual ~AuthSession() = default;
+
+ /// \brief Authenticate the given request headers.
+ ///
+ /// This method adds authentication information (e.g., Authorization header)
+ /// to the provided headers map. The implementation should be idempotent.
+ ///
+ /// \param[in,out] headers The headers map to add authentication information
to.
+ /// \return Status indicating success or failure of authentication.
+ /// - Success: Returns Status::OK
+ /// - Failure: Returns one of the following errors:
+ /// - 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<std::string, std::string>&
headers) = 0;
+
+ /// \brief Close the session and release any resources.
+ ///
+ /// This method is called when the session is no longer needed. For stateful
+ /// sessions (e.g., OAuth2 with token refresh), this should stop any
background
+ /// threads and release resources.
+ ///
+ /// \return Status indicating success or failure of closing the session.
+ virtual Status Close() { return {}; }
+};
+
+/// \brief A default authentication session that adds static headers to
requests.
+///
+/// This implementation authenticates requests by adding a fixed set of
headers.
+/// It is suitable for authentication methods that use static credentials,
+/// such as Basic auth or static bearer tokens.
+class ICEBERG_REST_EXPORT DefaultAuthSession : public AuthSession {
Review Comment:
Is it better to just define it in the auth_session.cc and use a factory to
create it?
##########
src/iceberg/catalog/rest/auth/auth_properties.h:
##########
@@ -0,0 +1,69 @@
+/*
+ * 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 <string_view>
+
+/// \file iceberg/catalog/rest/auth/auth_properties.h
+/// \brief Property keys and constants for REST catalog authentication.
+
+namespace iceberg::rest::auth {
+
+/// \brief Property keys and constants for authentication configuration.
+///
+/// This struct defines all the property keys used to configure authentication
+/// for the REST catalog. It follows the same naming conventions as Java
Iceberg.
+struct AuthProperties {
+ /// \brief Property key for specifying the authentication type.
+ static constexpr std::string_view kAuthType = "rest.auth.type";
+ /// \brief Authentication type: no authentication.
+ static constexpr std::string_view kAuthTypeNone = "none";
+ /// \brief Authentication type: HTTP Basic authentication.
+ static constexpr std::string_view kAuthTypeBasic = "basic";
+ /// \brief Authentication type: OAuth2 authentication.
+ static constexpr std::string_view kAuthTypeOAuth2 = "oauth2";
+ /// \brief Authentication type: AWS SigV4 authentication.
+ static constexpr std::string_view kAuthTypeSigV4 = "sigv4";
+ /// \brief Property key for Basic auth username.
+ static constexpr std::string_view kBasicUsername =
"rest.auth.basic.username";
+ /// \brief Property key for Basic auth password.
+ static constexpr std::string_view kBasicPassword =
"rest.auth.basic.password";
+ /// \brief Property key for OAuth2 token (bearer token).
+ static constexpr std::string_view kOAuth2Token = "token";
Review Comment:
```suggestion
static constexpr std::string_view kOAuth2Token = "token";
```
##########
src/iceberg/catalog/rest/auth/auth_managers.h:
##########
@@ -0,0 +1,70 @@
+/*
+ * 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 <functional>
+#include <memory>
+#include <string>
+#include <string_view>
+#include <unordered_map>
+
+#include "iceberg/catalog/rest/auth/auth_manager.h"
+#include "iceberg/catalog/rest/iceberg_rest_export.h"
+#include "iceberg/result.h"
+#include "iceberg/util/string_util.h"
+
+/// \file iceberg/catalog/rest/auth/auth_managers.h
+/// \brief Factory for creating authentication managers.
+
+namespace iceberg::rest::auth {
+
+/// \brief Function that builds an AuthManager for a given catalog.
+///
+/// \param name Catalog name passed to the manager.
+/// \param properties Consolidated catalog configuration.
+/// \return Newly created manager instance.
+using AuthManagerFactory = std::function<std::unique_ptr<AuthManager>(
+ std::string_view name,
+ const std::unordered_map<std::string, std::string>& properties)>;
+
+/// \brief Registry type for AuthManager factories with heterogeneous lookup
support.
+using AuthManagerRegistry =
Review Comment:
We can move this line to the cc file because users do not need to know this.
##########
src/iceberg/catalog/rest/auth/auth_managers.h:
##########
@@ -0,0 +1,70 @@
+/*
+ * 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 <functional>
+#include <memory>
+#include <string>
+#include <string_view>
+#include <unordered_map>
+
+#include "iceberg/catalog/rest/auth/auth_manager.h"
+#include "iceberg/catalog/rest/iceberg_rest_export.h"
+#include "iceberg/result.h"
+#include "iceberg/util/string_util.h"
+
+/// \file iceberg/catalog/rest/auth/auth_managers.h
+/// \brief Factory for creating authentication managers.
+
+namespace iceberg::rest::auth {
+
+/// \brief Function that builds an AuthManager for a given catalog.
+///
+/// \param name Catalog name passed to the manager.
Review Comment:
```suggestion
/// \param name Name of the auth manager.
```
##########
src/iceberg/catalog/rest/auth/auth_managers.h:
##########
@@ -0,0 +1,70 @@
+/*
+ * 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 <functional>
+#include <memory>
+#include <string>
+#include <string_view>
+#include <unordered_map>
+
+#include "iceberg/catalog/rest/auth/auth_manager.h"
+#include "iceberg/catalog/rest/iceberg_rest_export.h"
+#include "iceberg/result.h"
+#include "iceberg/util/string_util.h"
+
+/// \file iceberg/catalog/rest/auth/auth_managers.h
+/// \brief Factory for creating authentication managers.
+
+namespace iceberg::rest::auth {
+
+/// \brief Function that builds an AuthManager for a given catalog.
+///
+/// \param name Catalog name passed to the manager.
+/// \param properties Consolidated catalog configuration.
+/// \return Newly created manager instance.
+using AuthManagerFactory = std::function<std::unique_ptr<AuthManager>(
Review Comment:
Should the factory return `Result<std::unique_ptr<AuthManager>>` just in
case of any error or implementation is unavailable?
##########
src/iceberg/catalog/rest/auth/auth_managers.cc:
##########
@@ -0,0 +1,85 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include "iceberg/catalog/rest/auth/auth_managers.h"
+
+#include <algorithm>
+#include <cctype>
+
+#include "iceberg/catalog/rest/auth/auth_properties.h"
+#include "iceberg/util/string_util.h"
+
+namespace iceberg::rest::auth {
+
+namespace {
+
+/// \brief Infer the authentication type from properties.
+///
+/// If no explicit auth type is set, this function tries to infer it from
+/// other properties. If "credential" or "token" is present, it implies
+/// OAuth2 authentication. Otherwise, defaults to no authentication.
+///
+/// This behavior is consistent with Java Iceberg's AuthManagers.
+std::string InferAuthType(
+ const std::unordered_map<std::string, std::string>& properties) {
+ // Check for explicit auth type first
+ auto it = properties.find(std::string(AuthProperties::kAuthType));
+ if (it != properties.end() && !it->second.empty()) {
+ return StringUtils::ToLower(it->second);
+ }
+
+ // Infer from OAuth2 properties (credential or token)
+ bool has_credential =
+ properties.contains(std::string(AuthProperties::kOAuth2Credential));
+ bool has_token =
properties.contains(std::string(AuthProperties::kOAuth2Token));
+ if (has_credential || has_token) {
+ return std::string(AuthProperties::kAuthTypeOAuth2);
+ }
+
+ // Default to no authentication
+ return std::string(AuthProperties::kAuthTypeNone);
+}
+
+/// \brief Get the global registry of auth manager factories.
+AuthManagerRegistry& GetRegistry() {
+ static AuthManagerRegistry registry;
+ return registry;
+}
+
+} // namespace
+
+void AuthManagers::Register(std::string_view auth_type, AuthManagerFactory
factory) {
+ GetRegistry()[StringUtils::ToLower(std::string(auth_type))] =
std::move(factory);
Review Comment:
It seems that we have unnecessary string allocations here.
##########
src/iceberg/catalog/rest/auth/auth_managers.cc:
##########
@@ -0,0 +1,85 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include "iceberg/catalog/rest/auth/auth_managers.h"
+
+#include <algorithm>
+#include <cctype>
+
+#include "iceberg/catalog/rest/auth/auth_properties.h"
+#include "iceberg/util/string_util.h"
+
+namespace iceberg::rest::auth {
+
+namespace {
+
+/// \brief Infer the authentication type from properties.
+///
+/// If no explicit auth type is set, this function tries to infer it from
+/// other properties. If "credential" or "token" is present, it implies
+/// OAuth2 authentication. Otherwise, defaults to no authentication.
+///
+/// This behavior is consistent with Java Iceberg's AuthManagers.
+std::string InferAuthType(
+ const std::unordered_map<std::string, std::string>& properties) {
+ // Check for explicit auth type first
+ auto it = properties.find(std::string(AuthProperties::kAuthType));
+ if (it != properties.end() && !it->second.empty()) {
+ return StringUtils::ToLower(it->second);
+ }
+
+ // Infer from OAuth2 properties (credential or token)
+ bool has_credential =
+ properties.contains(std::string(AuthProperties::kOAuth2Credential));
+ bool has_token =
properties.contains(std::string(AuthProperties::kOAuth2Token));
+ if (has_credential || has_token) {
+ return std::string(AuthProperties::kAuthTypeOAuth2);
+ }
+
+ // Default to no authentication
+ return std::string(AuthProperties::kAuthTypeNone);
+}
+
+/// \brief Get the global registry of auth manager factories.
+AuthManagerRegistry& GetRegistry() {
+ static AuthManagerRegistry registry;
+ return registry;
+}
+
+} // namespace
+
+void AuthManagers::Register(std::string_view auth_type, AuthManagerFactory
factory) {
+ GetRegistry()[StringUtils::ToLower(std::string(auth_type))] =
std::move(factory);
+}
+
+Result<std::unique_ptr<AuthManager>> AuthManagers::Load(
+ std::string_view name,
+ const std::unordered_map<std::string, std::string>& properties) {
+ std::string auth_type = InferAuthType(properties);
+
+ auto& registry = GetRegistry();
+ auto it = registry.find(auth_type);
+ if (it == registry.end()) {
+ return NotImplemented("Authentication type '{}' is not supported",
auth_type);
Review Comment:
Please add a TODO to say we need to fallback to our default auth manager
implementations.
##########
src/iceberg/catalog/rest/auth/auth_session.h:
##########
@@ -0,0 +1,94 @@
+/*
+ * 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 <memory>
+#include <string>
+#include <unordered_map>
+
+#include "iceberg/catalog/rest/iceberg_rest_export.h"
+#include "iceberg/result.h"
+
+/// \file iceberg/catalog/rest/auth/auth_session.h
+/// \brief Authentication session interface for REST catalog.
+
+namespace iceberg::rest::auth {
+
+/// \brief An authentication session that can authenticate outgoing HTTP
requests.
+///
+/// Authentication sessions are typically immutable, but may hold resources
that need
+/// to be released when the session is no longer needed (e.g., token refresh
threads).
+/// Implementations should override Close() to release any such resources.
+///
+class ICEBERG_REST_EXPORT AuthSession {
+ public:
+ virtual ~AuthSession() = default;
+
+ /// \brief Authenticate the given request headers.
+ ///
+ /// This method adds authentication information (e.g., Authorization header)
+ /// to the provided headers map. The implementation should be idempotent.
+ ///
+ /// \param[in,out] headers The headers map to add authentication information
to.
+ /// \return Status indicating success or failure of authentication.
+ /// - Success: Returns Status::OK
+ /// - Failure: Returns one of the following errors:
Review Comment:
```suggestion
/// \return Status indicating success or one of the following errors:
```
##########
src/iceberg/catalog/rest/type_fwd.h:
##########
@@ -34,3 +34,11 @@ class RestCatalog;
class RestCatalogProperties;
} // namespace iceberg::rest
+
+namespace iceberg::rest::auth {
+
+class AuthManager;
+class AuthSession;
+class DefaultAuthSession;
Review Comment:
Why do we need to forward declare `DefaultAuthSession`? Isn't it used only
by the implementation?
##########
src/iceberg/catalog/rest/auth/auth_managers.h:
##########
@@ -0,0 +1,70 @@
+/*
+ * 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 <functional>
+#include <memory>
+#include <string>
+#include <string_view>
+#include <unordered_map>
+
+#include "iceberg/catalog/rest/auth/auth_manager.h"
+#include "iceberg/catalog/rest/iceberg_rest_export.h"
+#include "iceberg/result.h"
+#include "iceberg/util/string_util.h"
+
+/// \file iceberg/catalog/rest/auth/auth_managers.h
+/// \brief Factory for creating authentication managers.
+
+namespace iceberg::rest::auth {
+
+/// \brief Function that builds an AuthManager for a given catalog.
Review Comment:
```suggestion
/// \brief Function that creates an AuthManager from its name.
```
##########
src/iceberg/catalog/rest/auth/auth_properties.h:
##########
@@ -0,0 +1,69 @@
+/*
+ * 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 <string_view>
+
+/// \file iceberg/catalog/rest/auth/auth_properties.h
+/// \brief Property keys and constants for REST catalog authentication.
+
+namespace iceberg::rest::auth {
+
+/// \brief Property keys and constants for authentication configuration.
+///
+/// This struct defines all the property keys used to configure authentication
+/// for the REST catalog. It follows the same naming conventions as Java
Iceberg.
+struct AuthProperties {
+ /// \brief Property key for specifying the authentication type.
+ static constexpr std::string_view kAuthType = "rest.auth.type";
+ /// \brief Authentication type: no authentication.
+ static constexpr std::string_view kAuthTypeNone = "none";
+ /// \brief Authentication type: HTTP Basic authentication.
+ static constexpr std::string_view kAuthTypeBasic = "basic";
+ /// \brief Authentication type: OAuth2 authentication.
+ static constexpr std::string_view kAuthTypeOAuth2 = "oauth2";
+ /// \brief Authentication type: AWS SigV4 authentication.
+ static constexpr std::string_view kAuthTypeSigV4 = "sigv4";
+ /// \brief Property key for Basic auth username.
+ static constexpr std::string_view kBasicUsername =
"rest.auth.basic.username";
+ /// \brief Property key for Basic auth password.
+ static constexpr std::string_view kBasicPassword =
"rest.auth.basic.password";
+ /// \brief Property key for OAuth2 token (bearer token).
+ static constexpr std::string_view kOAuth2Token = "token";
+ /// \brief Property key for OAuth2 credential (client_id:client_secret).
+ static constexpr std::string_view kOAuth2Credential = "credential";
+ /// \brief Property key for OAuth2 scope.
+ static constexpr std::string_view kOAuth2Scope = "scope";
+ /// \brief Property key for OAuth2 server URI.
+ static constexpr std::string_view kOAuth2ServerUri = "oauth2-server-uri";
+ /// \brief Property key for enabling token refresh.
+ static constexpr std::string_view kOAuth2TokenRefreshEnabled =
"token-refresh-enabled";
+ /// \brief Default OAuth2 scope for catalog operations.
+ static constexpr std::string_view kOAuth2DefaultScope = "catalog";
+ /// \brief Property key for SigV4 region.
Review Comment:
```suggestion
/// \brief Property key for SigV4 region.
```
##########
src/iceberg/catalog/rest/auth/auth_session.h:
##########
@@ -0,0 +1,94 @@
+/*
+ * 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 <memory>
+#include <string>
+#include <unordered_map>
+
+#include "iceberg/catalog/rest/iceberg_rest_export.h"
+#include "iceberg/result.h"
+
+/// \file iceberg/catalog/rest/auth/auth_session.h
+/// \brief Authentication session interface for REST catalog.
+
+namespace iceberg::rest::auth {
+
+/// \brief An authentication session that can authenticate outgoing HTTP
requests.
+///
+/// Authentication sessions are typically immutable, but may hold resources
that need
+/// to be released when the session is no longer needed (e.g., token refresh
threads).
+/// Implementations should override Close() to release any such resources.
+///
Review Comment:
```suggestion
```
They are duplicate in the comment for Close.
##########
src/iceberg/catalog/rest/auth/auth_managers.h:
##########
@@ -0,0 +1,70 @@
+/*
+ * 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 <functional>
+#include <memory>
+#include <string>
+#include <string_view>
+#include <unordered_map>
+
+#include "iceberg/catalog/rest/auth/auth_manager.h"
+#include "iceberg/catalog/rest/iceberg_rest_export.h"
+#include "iceberg/result.h"
+#include "iceberg/util/string_util.h"
+
+/// \file iceberg/catalog/rest/auth/auth_managers.h
+/// \brief Factory for creating authentication managers.
+
+namespace iceberg::rest::auth {
+
+/// \brief Function that builds an AuthManager for a given catalog.
+///
+/// \param name Catalog name passed to the manager.
+/// \param properties Consolidated catalog configuration.
Review Comment:
```suggestion
/// \param properties Properties required by the auth manager.
```
##########
src/iceberg/catalog/rest/auth/auth_managers.cc:
##########
@@ -0,0 +1,85 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+#include "iceberg/catalog/rest/auth/auth_managers.h"
+
+#include <algorithm>
+#include <cctype>
+
+#include "iceberg/catalog/rest/auth/auth_properties.h"
+#include "iceberg/util/string_util.h"
+
+namespace iceberg::rest::auth {
+
+namespace {
+
+/// \brief Infer the authentication type from properties.
+///
+/// If no explicit auth type is set, this function tries to infer it from
+/// other properties. If "credential" or "token" is present, it implies
+/// OAuth2 authentication. Otherwise, defaults to no authentication.
+///
+/// This behavior is consistent with Java Iceberg's AuthManagers.
+std::string InferAuthType(
+ const std::unordered_map<std::string, std::string>& properties) {
+ // Check for explicit auth type first
+ auto it = properties.find(std::string(AuthProperties::kAuthType));
+ if (it != properties.end() && !it->second.empty()) {
+ return StringUtils::ToLower(it->second);
+ }
+
+ // Infer from OAuth2 properties (credential or token)
+ bool has_credential =
+ properties.contains(std::string(AuthProperties::kOAuth2Credential));
Review Comment:
Do we need to define them as const std::string? It creates a string every
time it is used.
##########
src/iceberg/catalog/rest/auth/auth_managers.h:
##########
@@ -0,0 +1,70 @@
+/*
+ * 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 <functional>
+#include <memory>
+#include <string>
+#include <string_view>
+#include <unordered_map>
+
+#include "iceberg/catalog/rest/auth/auth_manager.h"
+#include "iceberg/catalog/rest/iceberg_rest_export.h"
+#include "iceberg/result.h"
+#include "iceberg/util/string_util.h"
+
+/// \file iceberg/catalog/rest/auth/auth_managers.h
+/// \brief Factory for creating authentication managers.
+
+namespace iceberg::rest::auth {
+
+/// \brief Function that builds an AuthManager for a given catalog.
+///
+/// \param name Catalog name passed to the manager.
+/// \param properties Consolidated catalog configuration.
+/// \return Newly created manager instance.
+using AuthManagerFactory = std::function<std::unique_ptr<AuthManager>(
+ std::string_view name,
+ const std::unordered_map<std::string, std::string>& properties)>;
+
+/// \brief Registry type for AuthManager factories with heterogeneous lookup
support.
+using AuthManagerRegistry =
+ std::unordered_map<std::string, AuthManagerFactory, StringHash,
StringEqual>;
+
+/// \brief Registry-backed factory for AuthManager implementations.
+class ICEBERG_REST_EXPORT AuthManagers {
+ public:
+ /// \brief Load a manager by consulting the "rest.auth.type" configuration.
+ ///
+ /// \param name Catalog name passed to the manager.
Review Comment:
Is this correct? Why it is catalog name?
##########
src/iceberg/catalog/rest/auth/auth_manager.h:
##########
@@ -0,0 +1,95 @@
+/*
+ * 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 <memory>
+#include <string>
+#include <unordered_map>
+
+#include "iceberg/catalog/rest/iceberg_rest_export.h"
+#include "iceberg/catalog/rest/type_fwd.h"
+#include "iceberg/result.h"
+#include "iceberg/table_identifier.h"
+
+/// \file iceberg/catalog/rest/auth/auth_manager.h
+/// \brief Authentication manager interface for REST catalog.
+
+namespace iceberg::rest::auth {
+
+/// \brief Produces authentication sessions for catalog and table requests.
+///
+/// AuthManager is responsible for creating authentication sessions at
different scopes:
+/// - InitSession: Short-lived session for catalog initialization (optional)
+/// - CatalogSession: Long-lived session for catalog-level operations
(required)
+/// - TableSession: Optional table-specific session or reuse of catalog session
+///
+/// Implementations are registered via AuthManagers::Register() and loaded by
auth type.
+class ICEBERG_REST_EXPORT AuthManager {
+ public:
+ virtual ~AuthManager() = default;
+
+ /// \brief Create a short-lived session used to contact the configuration
endpoint.
+ ///
+ /// This session is used only during catalog initialization to fetch server
+ /// configuration and perform initial authentication. It is typically
discarded after
+ /// initialization.
+ ///
+ /// \param init_client HTTP client used for initialization requests.
+ /// \param properties Client configuration supplied by the catalog.
+ /// \return Session for initialization or an error if credentials cannot be
acquired.
+ virtual Result<std::unique_ptr<AuthSession>> InitSession(
+ HttpClient& init_client,
+ const std::unordered_map<std::string, std::string>& properties);
+
+ /// \brief Create the long-lived catalog session that acts as the parent
session.
+ ///
+ /// This session is used for all catalog-level operations (list namespaces,
list tables,
+ /// etc.) and serves as the parent session for table-specific operations. It
is owned
+ /// by the catalog and reused throughout the catalog's lifetime.
+ ///
+ /// \param shared_client HTTP client owned by the catalog and reused for
auth calls.
+ /// \param properties Catalog properties (client config + server defaults).
+ /// \return Session for catalog operations or an error if authentication
cannot be set
+ /// up.
+ virtual Result<std::unique_ptr<AuthSession>> CatalogSession(
+ HttpClient& shared_client,
+ const std::unordered_map<std::string, std::string>& properties) = 0;
+
+ /// \brief Create or reuse a session scoped to a single table/view.
+ ///
+ /// This method can return a new table-specific session or indicate that the
parent
+ /// catalog session should be reused by returning nullptr.
+ ///
+ /// \param table Target table identifier.
+ /// \param properties Table-specific auth properties returned by the server.
+ /// \param parent Catalog session to inherit from or extract information
from.
+ /// \return A new session for the table, nullptr to reuse parent session, or
an error.
Review Comment:
It looks weird to return nullptr to reuse parent session. I'm not sure if it
is a good idea to use `std::unique_ptr<AuthSession>&& parent` to make the
ownership explicit: either directly return `parent` or the created session owns
its parent. I didn't any case to share the session, am I missing something?
##########
src/iceberg/catalog/rest/auth/auth_properties.h:
##########
@@ -0,0 +1,69 @@
+/*
+ * 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 <string_view>
+
+/// \file iceberg/catalog/rest/auth/auth_properties.h
+/// \brief Property keys and constants for REST catalog authentication.
+
+namespace iceberg::rest::auth {
+
+/// \brief Property keys and constants for authentication configuration.
+///
+/// This struct defines all the property keys used to configure authentication
+/// for the REST catalog. It follows the same naming conventions as Java
Iceberg.
+struct AuthProperties {
+ /// \brief Property key for specifying the authentication type.
+ static constexpr std::string_view kAuthType = "rest.auth.type";
+ /// \brief Authentication type: no authentication.
+ static constexpr std::string_view kAuthTypeNone = "none";
+ /// \brief Authentication type: HTTP Basic authentication.
+ static constexpr std::string_view kAuthTypeBasic = "basic";
+ /// \brief Authentication type: OAuth2 authentication.
+ static constexpr std::string_view kAuthTypeOAuth2 = "oauth2";
+ /// \brief Authentication type: AWS SigV4 authentication.
+ static constexpr std::string_view kAuthTypeSigV4 = "sigv4";
+ /// \brief Property key for Basic auth username.
Review Comment:
Let's add blank line to separate different categories.
##########
src/iceberg/catalog/rest/auth/auth_manager.h:
##########
@@ -0,0 +1,95 @@
+/*
+ * 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 <memory>
+#include <string>
+#include <unordered_map>
+
+#include "iceberg/catalog/rest/iceberg_rest_export.h"
+#include "iceberg/catalog/rest/type_fwd.h"
+#include "iceberg/result.h"
+#include "iceberg/table_identifier.h"
+
+/// \file iceberg/catalog/rest/auth/auth_manager.h
+/// \brief Authentication manager interface for REST catalog.
+
+namespace iceberg::rest::auth {
+
+/// \brief Produces authentication sessions for catalog and table requests.
+///
+/// AuthManager is responsible for creating authentication sessions at
different scopes:
+/// - InitSession: Short-lived session for catalog initialization (optional)
+/// - CatalogSession: Long-lived session for catalog-level operations
(required)
+/// - TableSession: Optional table-specific session or reuse of catalog session
+///
+/// Implementations are registered via AuthManagers::Register() and loaded by
auth type.
+class ICEBERG_REST_EXPORT AuthManager {
+ public:
+ virtual ~AuthManager() = default;
+
+ /// \brief Create a short-lived session used to contact the configuration
endpoint.
+ ///
+ /// This session is used only during catalog initialization to fetch server
+ /// configuration and perform initial authentication. It is typically
discarded after
+ /// initialization.
+ ///
+ /// \param init_client HTTP client used for initialization requests.
+ /// \param properties Client configuration supplied by the catalog.
+ /// \return Session for initialization or an error if credentials cannot be
acquired.
+ virtual Result<std::unique_ptr<AuthSession>> InitSession(
+ HttpClient& init_client,
+ const std::unordered_map<std::string, std::string>& properties);
+
+ /// \brief Create the long-lived catalog session that acts as the parent
session.
+ ///
+ /// This session is used for all catalog-level operations (list namespaces,
list tables,
+ /// etc.) and serves as the parent session for table-specific operations. It
is owned
+ /// by the catalog and reused throughout the catalog's lifetime.
+ ///
+ /// \param shared_client HTTP client owned by the catalog and reused for
auth calls.
+ /// \param properties Catalog properties (client config + server defaults).
+ /// \return Session for catalog operations or an error if authentication
cannot be set
+ /// up.
+ virtual Result<std::unique_ptr<AuthSession>> CatalogSession(
Review Comment:
Why we don't have contextual session as the Java impl? I think this is used
by each rest endpoint before sending the http request.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
