Fix issues with cluster+security

- was using "none" not empty string for no ID.
- was multicasting secure id for update and shadow connections.


git-svn-id: https://svn.apache.org/repos/asf/qpid/trunk/qpid@948967 13f79535-47bb-0310-9956-ffa450edef68

3 files changed, 24 insertions, 30 deletions

diff --git a/cpp/src/qpid/broker/ConnectionHandler.cpp b/cpp/src/qpid/broker/ConnectionHandler.cpp
index b2d4210473..bf1af2fe88 100644
--- a/cpp/src/qpid/broker/ConnectionHandler.cpp
+++ b/cpp/src/qpid/broker/ConnectionHandler.cpp
@@ -181,14 +181,6 @@ void ConnectionHandler::Handler::tuneOk(uint16_t /*channelmax*/,
     connection.setHeartbeatInterval(heartbeat);
 }
 
-void ConnectionHandler::Handler::callUserIdCallbacks ( ) {
-    string s;
-    if ( false == authenticator->getUsername(s) )
-        s = "none";
-    if ( userIdCallback )
-      userIdCallback ( s );
-}
-
 void ConnectionHandler::Handler::open(const string& /*virtualHost*/,
                                       const framing::Array& /*capabilities*/, bool /*insist*/)
 {
@@ -204,7 +196,14 @@ void ConnectionHandler::Handler::open(const string& /*virtualHost*/,
         if (sl.get()) secured->activateSecurityLayer(sl);
     }
 
-    callUserIdCallbacks ( );
+    if ( userIdCallback ) {
+        string s;
+        // Not checking the return value of getUsername, if there is
+        // no username then we want to call the userIdCallback anyway
+        // with an empty string.
+        authenticator->getUsername(s);
+        userIdCallback(s);
+    }
 }
 
 
diff --git a/cpp/src/qpid/broker/ConnectionHandler.h b/cpp/src/qpid/broker/ConnectionHandler.h
index 0372942188..ecc8868e87 100644
--- a/cpp/src/qpid/broker/ConnectionHandler.h
+++ b/cpp/src/qpid/broker/ConnectionHandler.h
@@ -68,12 +68,7 @@ class ConnectionHandler : public framing::FrameHandler
         void closeOk();
 
         UserIdCallback userIdCallback;
-        void setUserIdCallback ( UserIdCallback fn ) {
-                 userIdCallback = fn;
-             };
-
-
-        void callUserIdCallbacks ( );
+        void setUserIdCallback ( UserIdCallback fn ) { userIdCallback = fn; };
 
 
         void start(const qpid::framing::FieldTable& serverProperties,
diff --git a/cpp/src/qpid/cluster/Connection.cpp b/cpp/src/qpid/cluster/Connection.cpp
index d7e5ee5cd9..18d0e0e599 100644
--- a/cpp/src/qpid/cluster/Connection.cpp
+++ b/cpp/src/qpid/cluster/Connection.cpp
@@ -620,26 +620,26 @@ void Connection::managementAgents(const std::string& data) {
 }
 
 
-// Only the direct, non-shadow gets this call.
 void Connection::mcastUserId ( std::string & id ) {
-    cluster.getMulticast().mcastControl( ClusterConnectionSecureUserIdBody(ProtocolVersion(), string(id)), getId() );
-
-  {
-      sys::Mutex::ScopedLock l(connectionNegotiationMonitor);
-      inConnectionNegotiation = false;
-      mcastSentButNotReceived = false;
-      connectionNegotiationMonitor.notify();
-  }
+    // Only the directly connected broker will mcast the secure user id, and only
+    // for client connections (not update connections)
+    if (isLocalClient())
+        cluster.getMulticast().mcastControl(
+            ClusterConnectionSecureUserIdBody(ProtocolVersion(), string(id)), getId() );
+    {
+        // This call signals the end of the connection negotiation phase.
+        sys::Mutex::ScopedLock l(connectionNegotiationMonitor);
+        inConnectionNegotiation = false;
+        mcastSentButNotReceived = false;
+        connectionNegotiationMonitor.notify();
+    }
 }
 
 // All connections, shadow or not, get this call.
 void Connection::secureUserId(const std::string& id) {
-    if ( isShadow() ) {
-        // If the user ID is "none", it is not legitimate.  Take no action.
-        if ( strcmp ( id.c_str(), "none" ) ) {
-            connection->setUserId ( id );
-        }
-    }
+    // Only set the user ID on shadow connections, and only if id is not the empty string.
+    if ( isShadow() && !id.empty() )
+        connection->setUserId ( id );
 }