This is an automated email from the ASF dual-hosted git repository.
alexey pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git
The following commit(s) were added to refs/heads/master by this push:
new e0ef4a861 [client] KUDU-3472 C++ client API to import JWT
e0ef4a861 is described below
commit e0ef4a861bc5597b71a1a0ebdb88bde76d260a66
Author: Alexey Serbin <ale...@apache.org>
AuthorDate: Tue Apr 25 23:38:28 2023 -0700
[client] KUDU-3472 C++ client API to import JWT
This patch addresses add C++ client API to specify a JWT for a newly
built C++ client instance. It's an improvement in the usability of the
Kudu client API since with this new extra method the users don't need
to mess with the serialization of the authn credentials.
The new functionality is covered by already existing tests that have
been updated to start using the new API. More tests might be added
to cover other aspects of the JWT-related functionality, but these
are not directly related to this API change, so we can take care of them
in a separate patch.
Change-Id: I8333f1d0c3b4e92ef8430c7ad91bb7da963aceb9
Reviewed-on: http://gerrit.cloudera.org:8080/19808
Reviewed-by: Abhishek Chennaka <achenn...@cloudera.com>
Tested-by: Alexey Serbin <ale...@apache.org>
---
src/kudu/client/client.cc | 50 +++++++++++++++--
src/kudu/client/client.h | 16 ++++++
src/kudu/client/client_builder-internal.h | 1 +
src/kudu/integration-tests/security-itest.cc | 80 +++++++++-------------------
4 files changed, 87 insertions(+), 60 deletions(-)
diff --git a/src/kudu/client/client.cc b/src/kudu/client/client.cc
index 0a128f604..a5ec83d0a 100644
--- a/src/kudu/client/client.cc
+++ b/src/kudu/client/client.cc
@@ -128,6 +128,7 @@ using kudu::rpc::Messenger;
using kudu::rpc::MessengerBuilder;
using kudu::rpc::RpcController;
using kudu::rpc::UserCredentials;
+using kudu::security::JwtRawPB;
using kudu::tserver::ScanResponsePB;
using std::map;
using std::make_optional;
@@ -308,6 +309,11 @@ KuduClientBuilder&
KuduClientBuilder::connection_negotiation_timeout(
return *this;
}
+KuduClientBuilder& KuduClientBuilder::jwt(const string& jwt) {
+ data_->jwt_ = jwt;
+ return *this;
+}
+
KuduClientBuilder& KuduClientBuilder::import_authentication_credentials(string
authn_creds) {
data_->authn_creds_ = std::move(authn_creds);
return *this;
@@ -395,9 +401,44 @@ Status KuduClientBuilder::Build(shared_ptr<KuduClient>*
client) {
RETURN_NOT_OK(builder.Build(&messenger));
UserCredentials user_credentials;
- // Parse and import the provided authn data, if any.
- if (!data_->authn_creds_.empty()) {
- RETURN_NOT_OK(ImportAuthnCreds(data_->authn_creds_, messenger.get(),
&user_credentials));
+ // Parse and import the provided authn data, if any. Override the JWT portion
+ // of the imported authn credentials if the JWT was specified as well.
+ // If no serialized authn data was provided, use JWT if it's available.
+ if (!data_->authn_creds_.empty() || !data_->jwt_.empty()) {
+ string authn_creds;
+ if (!data_->authn_creds_.empty() && !data_->jwt_.empty()) {
+ // Merge the imported authn creds and the JWT: use data_->authn_creds_,
+ // but override its JWT portion with data_->jwt_.
+ AuthenticationCredentialsPB pb;
+ if (!pb.ParseFromString(data_->authn_creds_)) {
+ return Status::InvalidArgument("invalid authentication data");
+ }
+ if (pb.has_jwt()) {
+ JwtRawPB jwt_pb;
+ // Copying, not moving: Build() is supposed to be re-enterable.
+ *jwt_pb.mutable_jwt_data() = data_->jwt_;
+ *pb.mutable_jwt() = std::move(jwt_pb);
+ }
+ string creds;
+ if (!pb.SerializeToString(&creds)) {
+ return Status::RuntimeError("could not serialize authentication data");
+ }
+ authn_creds = std::move(creds);
+ } else if (!data_->jwt_.empty()) {
+ // Build authn creds with just the provided JWT.
+ JwtRawPB jwt_pb;
+ // Copying, not moving: Build() is supposed to be re-enterable.
+ *jwt_pb.mutable_jwt_data() = data_->jwt_;
+ AuthenticationCredentialsPB pb;
+ *pb.mutable_jwt() = std::move(jwt_pb);
+ if (!pb.SerializeToString(&authn_creds)) {
+ return Status::RuntimeError("could not serialize authentication data");
+ }
+ }
+ RETURN_NOT_OK(ImportAuthnCreds(
+ !authn_creds.empty() ? authn_creds : data_->authn_creds_,
+ messenger.get(),
+ &user_credentials));
}
if (!user_credentials.has_real_user()) {
// If there are no authentication credentials, then set the real user to
the
@@ -777,8 +818,7 @@ Status KuduClient::ExportAuthenticationCredentials(string*
authn_creds) const {
if (auto tok = data_->messenger_->authn_token(); tok) {
pb.mutable_authn_token()->CopyFrom(*tok);
}
- auto jwt = data_->messenger_->jwt();
- if (jwt) {
+ if (auto jwt = data_->messenger_->jwt(); jwt) {
pb.mutable_jwt()->CopyFrom(*jwt);
}
pb.set_real_user(data_->user_credentials_.real_user());
diff --git a/src/kudu/client/client.h b/src/kudu/client/client.h
index a46179814..2640675a0 100644
--- a/src/kudu/client/client.h
+++ b/src/kudu/client/client.h
@@ -304,9 +304,25 @@ class KUDU_EXPORT KuduClientBuilder {
/// @return Reference to the updated object.
KuduClientBuilder& connection_negotiation_timeout(const MonoDelta& timeout);
+ /// Set JWT (JSON Web Token) to authenticate the client to a server.
+ ///
+ /// @note If both @c import_authentication_credentials and
+ /// this method are called on the object, the JWT provided with this call
+ /// overrides the corresponding JWT (if present) that comes as part of the
+ /// imported authentication credentials.
+ ///
+ /// @param [in] jwt
+ /// The JSON web token to set.
+ /// @return Reference to the updated object.
+ KuduClientBuilder& jwt(const std::string& jwt);
/// Import serialized authentication credentials from another client.
///
+ /// @note If both @c import_authentication_credentials and
+ /// this method are called on the object, the JWT provided with this call
+ /// overrides the corresponding JWT (if present) that comes as part of the
+ /// imported authentication credentials.
+ ///
/// @param [in] authn_creds
/// The serialized authentication credentials, provided by a call to
/// @c KuduClient.ExportAuthenticationCredentials in the C++ client or
diff --git a/src/kudu/client/client_builder-internal.h
b/src/kudu/client/client_builder-internal.h
index 9e7c8cbe3..2817124e1 100644
--- a/src/kudu/client/client_builder-internal.h
+++ b/src/kudu/client/client_builder-internal.h
@@ -39,6 +39,7 @@ class KuduClientBuilder::Data {
MonoDelta default_rpc_timeout_;
MonoDelta connection_negotiation_timeout_;
std::string authn_creds_;
+ std::string jwt_;
internal::ReplicaController::Visibility replica_visibility_;
std::optional<int> num_reactors_;
std::string sasl_protocol_name_;
diff --git a/src/kudu/integration-tests/security-itest.cc
b/src/kudu/integration-tests/security-itest.cc
index 045309e88..71b5727e9 100644
--- a/src/kudu/integration-tests/security-itest.cc
+++ b/src/kudu/integration-tests/security-itest.cc
@@ -550,20 +550,12 @@ TEST_F(SecurityITest, TestJwtMiniCluster) {
const auto* const kSubject = "kudu-user";
const auto configure_builder_for =
[&] (const string& account_id, KuduClientBuilder* b, const uint64_t
delay_ms) {
- client::AuthenticationCredentialsPB pb;
- security::JwtRawPB jwt = security::JwtRawPB();
- *jwt.mutable_jwt_data() = cluster_->oidc()->CreateJwt(account_id,
kSubject, true);
- *pb.mutable_jwt() = std::move(jwt);
- string creds;
- CHECK(pb.SerializeToString(&creds));
-
- SleepFor(MonoDelta::FromMilliseconds(delay_ms));
-
for (auto i = 0; i < cluster_->num_masters(); ++i) {
b->add_master_server_addr(cluster_->master(i)->bound_rpc_addr().ToString());
}
- b->import_authentication_credentials(creds);
+ b->jwt(cluster_->oidc()->CreateJwt(account_id, kSubject, true));
b->require_authentication(true);
+ SleepFor(MonoDelta::FromMilliseconds(delay_ms));
};
{
@@ -579,7 +571,7 @@ TEST_F(SecurityITest, TestJwtMiniCluster) {
shared_ptr<KuduClient> client;
configure_builder_for(kInvalidAccount, &invalid_builder, 0);
Status s = invalid_builder.Build(&client);
- ASSERT_TRUE(s.IsRuntimeError());
+ ASSERT_TRUE(s.IsRuntimeError()) << s.ToString();
ASSERT_STR_CONTAINS(s.ToString(), "FATAL_INVALID_JWT");
}
{
@@ -587,7 +579,7 @@ TEST_F(SecurityITest, TestJwtMiniCluster) {
shared_ptr<KuduClient> client;
configure_builder_for(kValidAccount, &timeout_builder, 3 * kLifetimeMs);
Status s = timeout_builder.Build(&client);
- ASSERT_TRUE(s.IsRuntimeError());
+ ASSERT_TRUE(s.IsRuntimeError()) << s.ToString();
ASSERT_STR_CONTAINS(s.ToString(), "token expired");
}
{
@@ -598,7 +590,7 @@ TEST_F(SecurityITest, TestJwtMiniCluster) {
}
no_jwt_builder.require_authentication(true);
Status s = no_jwt_builder.Build(&client);
- ASSERT_TRUE(s. IsNotAuthorized());
+ ASSERT_TRUE(s. IsNotAuthorized()) << s.ToString();
ASSERT_STR_CONTAINS(s.ToString(), "Not authorized");
}
}
@@ -636,28 +628,17 @@ TEST_F(SecurityITest, TestJwtMiniClusterWithInvalidCert) {
cluster_opts_.mini_oidc_options = std::move(oidc_opts);
ASSERT_OK(StartCluster());
- {
- KuduClientBuilder client_builder;
- client::AuthenticationCredentialsPB pb;
- security::JwtRawPB jwt = security::JwtRawPB();
- *jwt.mutable_jwt_data() = cluster_->oidc()->CreateJwt(kValidAccount,
kSubject, true);
- *pb.mutable_jwt() = std::move(jwt);
- string creds;
- CHECK(pb.SerializeToString(&creds));
-
- for (auto i = 0; i < cluster_->num_masters(); ++i) {
-
client_builder.add_master_server_addr(cluster_->master(i)->bound_rpc_addr().ToString());
- }
- client_builder.import_authentication_credentials(creds);
- client_builder.require_authentication(true);
-
- shared_ptr<KuduClient> client;
-
- Status s = client_builder.Build(&client);
- ASSERT_FALSE(s.ok());
- ASSERT_TRUE(s.IsRuntimeError());
- ASSERT_STR_CONTAINS(s.ToString(), "Error initializing JWT helper: Failed
to load JWKS");
+ KuduClientBuilder client_builder;
+ for (auto i = 0; i < cluster_->num_masters(); ++i) {
+
client_builder.add_master_server_addr(cluster_->master(i)->bound_rpc_addr().ToString());
}
+ client_builder.jwt(cluster_->oidc()->CreateJwt(kValidAccount, kSubject,
true));
+ client_builder.require_authentication(true);
+
+ shared_ptr<KuduClient> client;
+ auto s = client_builder.Build(&client);
+ ASSERT_TRUE(s.IsRuntimeError()) << s.ToString();
+ ASSERT_STR_CONTAINS(s.ToString(), "Error initializing JWT helper: Failed to
load JWKS");
}
TEST_F(SecurityITest, TestJwtMiniClusterWithUntrustedCert) {
@@ -689,28 +670,17 @@ TEST_F(SecurityITest,
TestJwtMiniClusterWithUntrustedCert) {
cluster_opts_.mini_oidc_options = std::move(oidc_opts);
ASSERT_OK(StartCluster());
- {
- KuduClientBuilder client_builder;
- client::AuthenticationCredentialsPB pb;
- security::JwtRawPB jwt = security::JwtRawPB();
- *jwt.mutable_jwt_data() = cluster_->oidc()->CreateJwt(kValidAccount,
kSubject, true);
- *pb.mutable_jwt() = std::move(jwt);
- string creds;
- CHECK(pb.SerializeToString(&creds));
-
- for (auto i = 0; i < cluster_->num_masters(); ++i) {
-
client_builder.add_master_server_addr(cluster_->master(i)->bound_rpc_addr().ToString());
- }
- client_builder.import_authentication_credentials(creds);
- client_builder.require_authentication(true);
-
- shared_ptr<KuduClient> client;
-
- Status s = client_builder.Build(&client);
- ASSERT_FALSE(s.ok());
- ASSERT_TRUE(s.IsRuntimeError());
- ASSERT_STR_CONTAINS(s.ToString(), "Error initializing JWT helper: Failed
to load JWKS");
+ KuduClientBuilder client_builder;
+ for (auto i = 0; i < cluster_->num_masters(); ++i) {
+
client_builder.add_master_server_addr(cluster_->master(i)->bound_rpc_addr().ToString());
}
+ client_builder.jwt(cluster_->oidc()->CreateJwt(kValidAccount, kSubject,
true));
+ client_builder.require_authentication(true);
+
+ shared_ptr<KuduClient> client;
+ auto s = client_builder.Build(&client);
+ ASSERT_TRUE(s.IsRuntimeError()) << s.ToString();
+ ASSERT_STR_CONTAINS(s.ToString(), "Error initializing JWT helper: Failed to
load JWKS");
}
TEST_F(SecurityITest, TestWorldReadableKeytab) {
