kudu-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From t...@apache.org
Subject [2/2] kudu git commit: rpc: add a new RemoteUser class for the server side user info
Date Thu, 23 Feb 2017 02:04:15 GMT
rpc: add a new RemoteUser class for the server side user info

We were previously using UserCredentials both as a client-side and
server-side class to represent a user. However, we'd like to capture
more information on the server side (such as both the full principal as
well as the shortened "username", and maybe the authentication method,
etc).

This patch splits out a new RemoteUser class which is used for
server-side connections. I also simplified the existing UserCredentials
class a bit while I was at it.

Change-Id: I899dc3b63d1f68ebf4120f1f8f6ab4b6e21c1a18
Reviewed-on: http://gerrit.cloudera.org:8080/6105
Tested-by: Kudu Jenkins
Reviewed-by: Todd Lipcon <todd@apache.org>


Project: http://git-wip-us.apache.org/repos/asf/kudu/repo
Commit: http://git-wip-us.apache.org/repos/asf/kudu/commit/d6dac682
Tree: http://git-wip-us.apache.org/repos/asf/kudu/tree/d6dac682
Diff: http://git-wip-us.apache.org/repos/asf/kudu/diff/d6dac682

Branch: refs/heads/master
Commit: d6dac682b7cf515fb9fae78bf4daf12b9df164ce
Parents: ed9ce95
Author: Todd Lipcon <todd@apache.org>
Authored: Tue Feb 21 23:12:21 2017 -0800
Committer: Todd Lipcon <todd@apache.org>
Committed: Thu Feb 23 01:59:16 2017 +0000

----------------------------------------------------------------------
 src/kudu/master/master_service.cc  |  4 +--
 src/kudu/rpc/CMakeLists.txt        |  1 +
 src/kudu/rpc/connection.cc         | 12 ++++-----
 src/kudu/rpc/connection.h          | 31 +++++++++++++++++-----
 src/kudu/rpc/inbound_call.cc       |  4 +--
 src/kudu/rpc/inbound_call.h        |  4 +--
 src/kudu/rpc/negotiation.cc        |  4 +--
 src/kudu/rpc/outbound_call.cc      | 10 +++----
 src/kudu/rpc/outbound_call.h       |  4 +--
 src/kudu/rpc/reactor.cc            |  4 +--
 src/kudu/rpc/remote_user.cc        | 34 ++++++++++++++++++++++++
 src/kudu/rpc/remote_user.h         | 47 +++++++++++++++++++++++++++++++++
 src/kudu/rpc/rpc-test-base.h       |  8 +++---
 src/kudu/rpc/rpc_context.cc        |  7 ++---
 src/kudu/rpc/rpc_context.h         |  7 +++--
 src/kudu/rpc/server_negotiation.cc | 10 +++----
 src/kudu/rpc/server_negotiation.h  |  9 +++++--
 src/kudu/rpc/user_credentials.cc   |  6 -----
 src/kudu/rpc/user_credentials.h    | 20 ++++----------
 19 files changed, 154 insertions(+), 72 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/kudu/blob/d6dac682/src/kudu/master/master_service.cc
----------------------------------------------------------------------
diff --git a/src/kudu/master/master_service.cc b/src/kudu/master/master_service.cc
index 4352025..0f3cc19 100644
--- a/src/kudu/master/master_service.cc
+++ b/src/kudu/master/master_service.cc
@@ -28,8 +28,8 @@
 #include "kudu/master/master_cert_authority.h"
 #include "kudu/master/ts_descriptor.h"
 #include "kudu/master/ts_manager.h"
+#include "kudu/rpc/remote_user.h"
 #include "kudu/rpc/rpc_context.h"
-#include "kudu/rpc/user_credentials.h"
 #include "kudu/server/webserver.h"
 #include "kudu/security/token_signer.h"
 #include "kudu/security/token_verifier.h"
@@ -388,7 +388,7 @@ void MasterServiceImpl::ConnectToMaster(const ConnectToMasterRequestPB*
/*req*/,
     // we want.
     SignedTokenPB authn_token;
     Status s = server_->token_signer()->GenerateAuthnToken(
-        rpc->user_credentials().real_user(),
+        rpc->remote_user().username(),
         &authn_token);
     if (!s.ok()) {
       KLOG_EVERY_N_SECS(WARNING, 1)

http://git-wip-us.apache.org/repos/asf/kudu/blob/d6dac682/src/kudu/rpc/CMakeLists.txt
----------------------------------------------------------------------
diff --git a/src/kudu/rpc/CMakeLists.txt b/src/kudu/rpc/CMakeLists.txt
index 6cfb3b6..19a7610 100644
--- a/src/kudu/rpc/CMakeLists.txt
+++ b/src/kudu/rpc/CMakeLists.txt
@@ -53,6 +53,7 @@ set(KRPC_SRCS
     proxy.cc
     reactor.cc
     remote_method.cc
+    remote_user.cc
     request_tracker.cc
     result_tracker.cc
     rpc.cc

http://git-wip-us.apache.org/repos/asf/kudu/blob/d6dac682/src/kudu/rpc/connection.cc
----------------------------------------------------------------------
diff --git a/src/kudu/rpc/connection.cc b/src/kudu/rpc/connection.cc
index d8edd08..809abe5 100644
--- a/src/kudu/rpc/connection.cc
+++ b/src/kudu/rpc/connection.cc
@@ -435,10 +435,6 @@ void Connection::QueueResponseForCall(gscoped_ptr<InboundCall>
call) {
   reactor_thread_->reactor()->ScheduleReactorTask(task);
 }
 
-void Connection::set_user_credentials(const UserCredentials &user_credentials) {
-  user_credentials_.CopyFrom(user_credentials);
-}
-
 RpczStore* Connection::rpcz_store() {
   return reactor_thread_->reactor()->messenger()->rpcz_store();
 }
@@ -670,10 +666,7 @@ Status Connection::DumpPB(const DumpRunningRpcsRequestPB& req,
   resp->set_remote_ip(remote_.ToString());
   if (negotiation_complete_) {
     resp->set_state(RpcConnectionPB::OPEN);
-    resp->set_remote_user_credentials(user_credentials_.ToString());
   } else {
-    // It's racy to dump credentials while negotiating, since the Connection
-    // object is owned by the negotiation thread at that point.
     resp->set_state(RpcConnectionPB::NEGOTIATING);
   }
 
@@ -685,6 +678,11 @@ Status Connection::DumpPB(const DumpRunningRpcsRequestPB& req,
       }
     }
   } else if (direction_ == SERVER) {
+    if (negotiation_complete_) {
+      // It's racy to dump credentials while negotiating, since the Connection
+      // object is owned by the negotiation thread at that point.
+      resp->set_remote_user_credentials(remote_user_.ToString());
+    }
     for (const inbound_call_map_t::value_type& entry : calls_being_handled_) {
       InboundCall* c = entry.second;
       c->DumpPB(req, resp->add_calls_in_flight());

http://git-wip-us.apache.org/repos/asf/kudu/blob/d6dac682/src/kudu/rpc/connection.h
----------------------------------------------------------------------
diff --git a/src/kudu/rpc/connection.h b/src/kudu/rpc/connection.h
index ad5ffed..e475cc5 100644
--- a/src/kudu/rpc/connection.h
+++ b/src/kudu/rpc/connection.h
@@ -34,6 +34,7 @@
 #include "kudu/gutil/ref_counted.h"
 #include "kudu/rpc/inbound_call.h"
 #include "kudu/rpc/outbound_call.h"
+#include "kudu/rpc/remote_user.h"
 #include "kudu/rpc/transfer.h"
 #include "kudu/util/monotime.h"
 #include "kudu/util/net/sockaddr.h"
@@ -121,14 +122,17 @@ class Connection : public RefCountedThreadSafe<Connection> {
   // The address of the remote end of the connection.
   const Sockaddr &remote() const { return remote_; }
 
-  // Set the user credentials which should be used to log in.
-  void set_user_credentials(const UserCredentials &user_credentials);
-
-  // Modify the user credentials which will be used to log in.
-  UserCredentials* mutable_user_credentials() { return &user_credentials_; }
+  // Set the user credentials for an outbound connection.
+  void set_local_user_credentials(UserCredentials creds) {
+    DCHECK_EQ(direction_, CLIENT);
+    local_user_credentials_ = std::move(creds);
+  }
 
   // Get the user credentials which will be used to log in.
-  const UserCredentials &user_credentials() const { return user_credentials_; }
+  const UserCredentials& local_user_credentials() const {
+    DCHECK_EQ(direction_, CLIENT);
+    return local_user_credentials_;
+  }
 
   RpczStore* rpcz_store();
 
@@ -168,6 +172,16 @@ class Connection : public RefCountedThreadSafe<Connection> {
     remote_features_ = std::move(remote_features);
   }
 
+  void set_remote_user(RemoteUser user) {
+    DCHECK_EQ(direction_, SERVER);
+    remote_user_ = std::move(user);
+  }
+
+  const RemoteUser& remote_user() const {
+    DCHECK_EQ(direction_, SERVER);
+    return remote_user_;
+  }
+
  private:
   friend struct CallAwaitingResponse;
   friend class QueueTransferTask;
@@ -234,7 +248,10 @@ class Connection : public RefCountedThreadSafe<Connection> {
   std::unique_ptr<Socket> socket_;
 
   // The credentials of the user operating on this connection (if a client user).
-  UserCredentials user_credentials_;
+  UserCredentials local_user_credentials_;
+
+  // The authenticated remote user (if this is an inbound connection on the server).
+  RemoteUser remote_user_;
 
   // whether we are client or server
   Direction direction_;

http://git-wip-us.apache.org/repos/asf/kudu/blob/d6dac682/src/kudu/rpc/inbound_call.cc
----------------------------------------------------------------------
diff --git a/src/kudu/rpc/inbound_call.cc b/src/kudu/rpc/inbound_call.cc
index d9c5e8f..448fd70 100644
--- a/src/kudu/rpc/inbound_call.cc
+++ b/src/kudu/rpc/inbound_call.cc
@@ -213,8 +213,8 @@ void InboundCall::DumpPB(const DumpRunningRpcsRequestPB& req,
                            .ToMicroseconds());
 }
 
-const UserCredentials& InboundCall::user_credentials() const {
-  return conn_->user_credentials();
+const RemoteUser& InboundCall::remote_user() const {
+  return conn_->remote_user();
 }
 
 const Sockaddr& InboundCall::remote_address() const {

http://git-wip-us.apache.org/repos/asf/kudu/blob/d6dac682/src/kudu/rpc/inbound_call.h
----------------------------------------------------------------------
diff --git a/src/kudu/rpc/inbound_call.h b/src/kudu/rpc/inbound_call.h
index 9f25ae5..4f99dee 100644
--- a/src/kudu/rpc/inbound_call.h
+++ b/src/kudu/rpc/inbound_call.h
@@ -49,10 +49,10 @@ namespace rpc {
 
 class Connection;
 class DumpRunningRpcsRequestPB;
+class RemoteUser;
 class RpcCallInProgressPB;
 struct RpcMethodInfo;
 class RpcSidecar;
-class UserCredentials;
 
 struct InboundCallTiming {
   MonoTime time_received;   // Time the call was first accepted.
@@ -130,7 +130,7 @@ class InboundCall {
 
   void DumpPB(const DumpRunningRpcsRequestPB& req, RpcCallInProgressPB* resp);
 
-  const UserCredentials& user_credentials() const;
+  const RemoteUser& remote_user() const;
 
   const Sockaddr& remote_address() const;
 

http://git-wip-us.apache.org/repos/asf/kudu/blob/d6dac682/src/kudu/rpc/negotiation.cc
----------------------------------------------------------------------
diff --git a/src/kudu/rpc/negotiation.cc b/src/kudu/rpc/negotiation.cc
index 817be72..8b5b3d4 100644
--- a/src/kudu/rpc/negotiation.cc
+++ b/src/kudu/rpc/negotiation.cc
@@ -179,7 +179,7 @@ static Status DoClientNegotiation(Connection* conn, MonoTime deadline)
{
     }
   }
 
-  RETURN_NOT_OK(client_negotiation.EnablePlain(conn->user_credentials().real_user(), ""));
+  RETURN_NOT_OK(client_negotiation.EnablePlain(conn->local_user_credentials().real_user(),
""));
   client_negotiation.set_deadline(deadline);
 
   RETURN_NOT_OK(WaitForClientConnect(client_negotiation.socket(), deadline));
@@ -226,7 +226,7 @@ static Status DoServerNegotiation(Connection* conn, const MonoTime&
deadline) {
   // Transfer the negotiated socket and state back to the connection.
   conn->adopt_socket(server_negotiation.release_socket());
   conn->set_remote_features(server_negotiation.take_client_features());
-  conn->mutable_user_credentials()->set_real_user(server_negotiation.authenticated_user());
+  conn->set_remote_user(server_negotiation.take_authenticated_user());
 
   return Status::OK();
 }

http://git-wip-us.apache.org/repos/asf/kudu/blob/d6dac682/src/kudu/rpc/outbound_call.cc
----------------------------------------------------------------------
diff --git a/src/kudu/rpc/outbound_call.cc b/src/kudu/rpc/outbound_call.cc
index dafcf2f..19ec0ec 100644
--- a/src/kudu/rpc/outbound_call.cc
+++ b/src/kudu/rpc/outbound_call.cc
@@ -363,17 +363,17 @@ ConnectionId::ConnectionId(const ConnectionId& other) {
   DoCopyFrom(other);
 }
 
-ConnectionId::ConnectionId(const Sockaddr& remote, const UserCredentials& user_credentials)
{
+ConnectionId::ConnectionId(const Sockaddr& remote, UserCredentials user_credentials)
{
   remote_ = remote;
-  user_credentials_.CopyFrom(user_credentials);
+  user_credentials_ = std::move(user_credentials);
 }
 
 void ConnectionId::set_remote(const Sockaddr& remote) {
   remote_ = remote;
 }
 
-void ConnectionId::set_user_credentials(const UserCredentials& user_credentials) {
-  user_credentials_.CopyFrom(user_credentials);
+void ConnectionId::set_user_credentials(UserCredentials user_credentials) {
+  user_credentials_ = std::move(user_credentials);
 }
 
 void ConnectionId::CopyFrom(const ConnectionId& other) {
@@ -389,7 +389,7 @@ string ConnectionId::ToString() const {
 
 void ConnectionId::DoCopyFrom(const ConnectionId& other) {
   remote_ = other.remote_;
-  user_credentials_.CopyFrom(other.user_credentials_);
+  user_credentials_ = other.user_credentials_;
 }
 
 size_t ConnectionId::HashCode() const {

http://git-wip-us.apache.org/repos/asf/kudu/blob/d6dac682/src/kudu/rpc/outbound_call.h
----------------------------------------------------------------------
diff --git a/src/kudu/rpc/outbound_call.h b/src/kudu/rpc/outbound_call.h
index fbaaa31..fa599fd 100644
--- a/src/kudu/rpc/outbound_call.h
+++ b/src/kudu/rpc/outbound_call.h
@@ -65,14 +65,14 @@ class ConnectionId {
   ConnectionId(const ConnectionId& other);
 
   // Convenience constructor.
-  ConnectionId(const Sockaddr& remote, const UserCredentials& user_credentials);
+  ConnectionId(const Sockaddr& remote, UserCredentials user_credentials);
 
   // The remote address.
   void set_remote(const Sockaddr& remote);
   const Sockaddr& remote() const { return remote_; }
 
   // The credentials of the user associated with this connection, if any.
-  void set_user_credentials(const UserCredentials& user_credentials);
+  void set_user_credentials(UserCredentials user_credentials);
   const UserCredentials& user_credentials() const { return user_credentials_; }
   UserCredentials* mutable_user_credentials() { return &user_credentials_; }
 

http://git-wip-us.apache.org/repos/asf/kudu/blob/d6dac682/src/kudu/rpc/reactor.cc
----------------------------------------------------------------------
diff --git a/src/kudu/rpc/reactor.cc b/src/kudu/rpc/reactor.cc
index da185b0..85f2776 100644
--- a/src/kudu/rpc/reactor.cc
+++ b/src/kudu/rpc/reactor.cc
@@ -344,7 +344,7 @@ Status ReactorThread::FindOrStartConnection(const ConnectionId& conn_id,
 
   // Register the new connection in our map.
   *conn = new Connection(this, conn_id.remote(), std::move(new_socket), Connection::CLIENT);
-  (*conn)->set_user_credentials(conn_id.user_credentials());
+  (*conn)->set_local_user_credentials(conn_id.user_credentials());
 
   // Kick off blocking client connection negotiation.
   Status s = StartConnectionNegotiation(*conn);
@@ -435,7 +435,7 @@ void ReactorThread::DestroyConnection(Connection *conn,
 
   // Unlink connection from lists.
   if (conn->direction() == Connection::CLIENT) {
-    ConnectionId conn_id(conn->remote(), conn->user_credentials());
+    ConnectionId conn_id(conn->remote(), conn->local_user_credentials());
     auto it = client_conns_.find(conn_id);
     CHECK(it != client_conns_.end()) << "Couldn't find connection " << conn->ToString();
     client_conns_.erase(it);

http://git-wip-us.apache.org/repos/asf/kudu/blob/d6dac682/src/kudu/rpc/remote_user.cc
----------------------------------------------------------------------
diff --git a/src/kudu/rpc/remote_user.cc b/src/kudu/rpc/remote_user.cc
new file mode 100644
index 0000000..ad9b4d1
--- /dev/null
+++ b/src/kudu/rpc/remote_user.cc
@@ -0,0 +1,34 @@
+// 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 "kudu/rpc/remote_user.h"
+
+#include <string>
+
+#include "kudu/gutil/strings/substitute.h"
+
+using std::string;
+
+namespace kudu {
+namespace rpc {
+
+string RemoteUser::ToString() const {
+  return strings::Substitute("{username='$0'}", username_);
+}
+
+} // namespace rpc
+} // namespace kudu

http://git-wip-us.apache.org/repos/asf/kudu/blob/d6dac682/src/kudu/rpc/remote_user.h
----------------------------------------------------------------------
diff --git a/src/kudu/rpc/remote_user.h b/src/kudu/rpc/remote_user.h
new file mode 100644
index 0000000..e085343
--- /dev/null
+++ b/src/kudu/rpc/remote_user.h
@@ -0,0 +1,47 @@
+// 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>
+
+namespace kudu {
+namespace rpc {
+
+// Server-side view of the remote authenticated user.
+//
+// This class may be read by multiple threads concurrently after
+// its initialization during RPC negotiation.
+class RemoteUser {
+ public:
+  const std::string& username() const { return username_; }
+
+  void set_username(std::string username) {
+    username_ = std::move(username);
+  }
+
+  // Returns a string representation of the object.
+  std::string ToString() const;
+
+ private:
+  // The real username of the remote user. In the case of a Kerberos
+  // principal, this has already been mapped to a local username.
+  // TODO(todd): actually do the above mapping.
+  std::string username_;
+};
+
+} // namespace rpc
+} // namespace kudu

http://git-wip-us.apache.org/repos/asf/kudu/blob/d6dac682/src/kudu/rpc/rpc-test-base.h
----------------------------------------------------------------------
diff --git a/src/kudu/rpc/rpc-test-base.h b/src/kudu/rpc/rpc-test-base.h
index dcdd5c3..35a19f2 100644
--- a/src/kudu/rpc/rpc-test-base.h
+++ b/src/kudu/rpc/rpc-test-base.h
@@ -227,8 +227,8 @@ class CalculatorService : public CalculatorServiceIf {
   void WhoAmI(const WhoAmIRequestPB* /*req*/,
               WhoAmIResponsePB* resp,
               RpcContext* context) override {
-    const UserCredentials& creds = context->user_credentials();
-    resp->mutable_credentials()->set_real_user(creds.real_user());
+    const RemoteUser& user = context->remote_user();
+    resp->mutable_credentials()->set_real_user(user.username());
     resp->set_address(context->remote_address().ToString());
     context->RespondSuccess();
   }
@@ -287,7 +287,7 @@ class CalculatorService : public CalculatorServiceIf {
   bool AuthorizeDisallowAlice(const google::protobuf::Message* /*req*/,
                               google::protobuf::Message* /*resp*/,
                               RpcContext* context) override {
-    if (context->user_credentials().real_user() == "alice") {
+    if (context->remote_user().username() == "alice") {
       context->RespondFailure(Status::NotAuthorized("alice is not allowed to call this
method"));
       return false;
     }
@@ -297,7 +297,7 @@ class CalculatorService : public CalculatorServiceIf {
   bool AuthorizeDisallowBob(const google::protobuf::Message* /*req*/,
                             google::protobuf::Message* /*resp*/,
                             RpcContext* context) override {
-    if (context->user_credentials().real_user() == "bob") {
+    if (context->remote_user().username() == "bob") {
       context->RespondFailure(Status::NotAuthorized("bob is not allowed to call this method"));
       return false;
     }

http://git-wip-us.apache.org/repos/asf/kudu/blob/d6dac682/src/kudu/rpc/rpc_context.cc
----------------------------------------------------------------------
diff --git a/src/kudu/rpc/rpc_context.cc b/src/kudu/rpc/rpc_context.cc
index a953b6d..2e45085 100644
--- a/src/kudu/rpc/rpc_context.cc
+++ b/src/kudu/rpc/rpc_context.cc
@@ -22,6 +22,7 @@
 
 #include "kudu/rpc/outbound_call.h"
 #include "kudu/rpc/inbound_call.h"
+#include "kudu/rpc/remote_user.h"
 #include "kudu/rpc/result_tracker.h"
 #include "kudu/rpc/rpc_sidecar.h"
 #include "kudu/rpc/service_if.h"
@@ -144,8 +145,8 @@ Status RpcContext::AddRpcSidecar(gscoped_ptr<RpcSidecar> car, int*
idx) {
   return call_->AddRpcSidecar(std::move(car), idx);
 }
 
-const UserCredentials& RpcContext::user_credentials() const {
-  return call_->user_credentials();
+const RemoteUser& RpcContext::remote_user() const {
+  return call_->remote_user();
 }
 
 const Sockaddr& RpcContext::remote_address() const {
@@ -153,7 +154,7 @@ const Sockaddr& RpcContext::remote_address() const {
 }
 
 std::string RpcContext::requestor_string() const {
-  return call_->user_credentials().ToString() + " at " +
+  return call_->remote_user().ToString() + " at " +
     call_->remote_address().ToString();
 }
 

http://git-wip-us.apache.org/repos/asf/kudu/blob/d6dac682/src/kudu/rpc/rpc_context.h
----------------------------------------------------------------------
diff --git a/src/kudu/rpc/rpc_context.h b/src/kudu/rpc/rpc_context.h
index 2ee58b7..f0c7e04 100644
--- a/src/kudu/rpc/rpc_context.h
+++ b/src/kudu/rpc/rpc_context.h
@@ -38,10 +38,9 @@ class Trace;
 namespace rpc {
 
 class InboundCall;
+class RemoteUser;
 class ResultTracker;
 class RpcSidecar;
-class UserCredentials;
-
 
 #define PANIC_RPC(rpc_context, message) \
   do { \
@@ -156,8 +155,8 @@ class RpcContext {
   // by the RPC response.
   Status AddRpcSidecar(gscoped_ptr<RpcSidecar> car, int* idx);
 
-  // Return the credentials of the remote user who made this call.
-  const UserCredentials& user_credentials() const;
+  // Return the identity of remote user who made this call.
+  const RemoteUser& remote_user() const;
 
   // Return the remote IP address and port which sent the current RPC call.
   const Sockaddr& remote_address() const;

http://git-wip-us.apache.org/repos/asf/kudu/blob/d6dac682/src/kudu/rpc/server_negotiation.cc
----------------------------------------------------------------------
diff --git a/src/kudu/rpc/server_negotiation.cc b/src/kudu/rpc/server_negotiation.cc
index bbd2c58..5fac8dc 100644
--- a/src/kudu/rpc/server_negotiation.cc
+++ b/src/kudu/rpc/server_negotiation.cc
@@ -103,10 +103,6 @@ SaslMechanism::Type ServerNegotiation::negotiated_mechanism() const {
   return negotiated_mech_;
 }
 
-const string& ServerNegotiation::authenticated_user() const {
-  return authenticated_user_;
-}
-
 void ServerNegotiation::set_server_fqdn(const string& domain_name) {
   helper_.set_server_fqdn(domain_name);
 }
@@ -511,7 +507,7 @@ Status ServerNegotiation::AuthenticateBySasl(faststring* recv_buf) {
                         reinterpret_cast<const void**>(&username));
   // We expect that SASL_USERNAME will always get set.
   CHECK(rc == SASL_OK && username != nullptr) << "No username on authenticated
connection";
-  authenticated_user_ = username;
+  authenticated_user_.set_username(username);
 
   return Status::OK();
 }
@@ -560,7 +556,7 @@ Status ServerNegotiation::AuthenticateByToken(faststring* recv_buf) {
     // get a signed authn token without a subject.
     return Status::RuntimeError("authentication token has no username");
   }
-  authenticated_user_ = token.authn().username();
+  authenticated_user_.set_username(token.authn().username());
 
   // Respond with success message.
   pb.Clear();
@@ -576,7 +572,7 @@ Status ServerNegotiation::AuthenticateByCertificate() {
   // Grab the subject from the client's cert.
   security::Cert cert;
   RETURN_NOT_OK(tls_handshake_.GetRemoteCert(&cert));
-  authenticated_user_ = cert.SubjectName();
+  authenticated_user_.set_username(cert.SubjectName());
 
   return Status::OK();
 }

http://git-wip-us.apache.org/repos/asf/kudu/blob/d6dac682/src/kudu/rpc/server_negotiation.h
----------------------------------------------------------------------
diff --git a/src/kudu/rpc/server_negotiation.h b/src/kudu/rpc/server_negotiation.h
index f615d3b..38202c3 100644
--- a/src/kudu/rpc/server_negotiation.h
+++ b/src/kudu/rpc/server_negotiation.h
@@ -26,6 +26,7 @@
 
 #include "kudu/gutil/gscoped_ptr.h"
 #include "kudu/rpc/negotiation.h"
+#include "kudu/rpc/remote_user.h"
 #include "kudu/rpc/rpc_header.pb.h"
 #include "kudu/rpc/sasl_common.h"
 #include "kudu/rpc/sasl_helper.h"
@@ -101,7 +102,11 @@ class ServerNegotiation {
 
   // Name of the user that was authenticated.
   // Must be called after a successful Negotiate().
-  const std::string& authenticated_user() const;
+  //
+  // Subsequent calls will return bogus data.
+  RemoteUser take_authenticated_user() {
+    return std::move(authenticated_user_);
+  }
 
   // Specify the fully-qualified domain name of the remote server.
   // Must be called before Negotiate(). Required for some mechanisms.
@@ -224,7 +229,7 @@ class ServerNegotiation {
 
   // The successfully-authenticated user, if applicable. Filled in during
   // negotiation.
-  std::string authenticated_user_;
+  RemoteUser authenticated_user_;
 
   // The authentication type. Filled in during negotiation.
   AuthenticationType negotiated_authn_;

http://git-wip-us.apache.org/repos/asf/kudu/blob/d6dac682/src/kudu/rpc/user_credentials.cc
----------------------------------------------------------------------
diff --git a/src/kudu/rpc/user_credentials.cc b/src/kudu/rpc/user_credentials.cc
index 5065271..fdc3ac2 100644
--- a/src/kudu/rpc/user_credentials.cc
+++ b/src/kudu/rpc/user_credentials.cc
@@ -28,8 +28,6 @@ using std::string;
 namespace kudu {
 namespace rpc {
 
-UserCredentials::UserCredentials() {}
-
 bool UserCredentials::has_real_user() const {
   return !real_user_.empty();
 }
@@ -38,10 +36,6 @@ void UserCredentials::set_real_user(const string& real_user) {
   real_user_ = real_user;
 }
 
-void UserCredentials::CopyFrom(const UserCredentials& other) {
-  real_user_ = other.real_user_;
-}
-
 string UserCredentials::ToString() const {
   // Does not print the password.
   return strings::Substitute("{real_user=$0}", real_user_);

http://git-wip-us.apache.org/repos/asf/kudu/blob/d6dac682/src/kudu/rpc/user_credentials.h
----------------------------------------------------------------------
diff --git a/src/kudu/rpc/user_credentials.h b/src/kudu/rpc/user_credentials.h
index ebeccb2..56af70a 100644
--- a/src/kudu/rpc/user_credentials.h
+++ b/src/kudu/rpc/user_credentials.h
@@ -18,29 +18,21 @@
 
 #include <string>
 
-#include "kudu/gutil/macros.h"
-
 namespace kudu {
 namespace rpc {
 
-// Client-side user credentials, such as a user's username & password.
-// In the future, we will add Kerberos credentials.
-//
-// TODO(mpercy): this is actually used server side too -- should
-// we instead introduce a RemoteUser class or something?
+// Client-side user credentials. Currently this is more-or-less a simple wrapper
+// around a username string. However, we anticipate moving more credentials such as
+// tokens into a per-Proxy structure rather than Messenger-wide, and this will
+// be the place to store them.
 class UserCredentials {
  public:
-   UserCredentials();
-
   // Real user.
   bool has_real_user() const;
   void set_real_user(const std::string& real_user);
   const std::string& real_user() const { return real_user_; }
 
-  // Copy state from another object to this one.
-  void CopyFrom(const UserCredentials& other);
-
-  // Returns a string representation of the object, not including the password field.
+  // Returns a string representation of the object.
   std::string ToString() const;
 
   std::size_t HashCode() const;
@@ -49,8 +41,6 @@ class UserCredentials {
  private:
   // Remember to update HashCode() and Equals() when new fields are added.
   std::string real_user_;
-
-  DISALLOW_COPY_AND_ASSIGN(UserCredentials);
 };
 
 } // namespace rpc


Mime
View raw message