conference: fix current protocol issues

This patch fix 3 little issues:
+ "device" was always empty. Uses the transport to fill the device
field. This will be used in the next version of the protocol.
+ raiseHand was bugguy in two ways:
    + It was only usable by moderators
    + Somebody was able to change the raiseHand for other peers

Change-Id: I32bf37c5063973a5a5962dd8984c87fb05d33a3d

src/conference.cpp

@@ -27,6 +27,7 @@
 #include "audio/audiolayer.h"
 #include "jamidht/jamiaccount.h"
 #include "string_utils.h"
+#include "sip/siptransport.h"
 
 #ifdef ENABLE_VIDEO
 #include "call.h"
@@ -96,12 +97,16 @@ Conference::Conference(const std::shared_ptr<Account>& account)
             auto shared = w.lock();
             if (!shared)
                 return;
+            auto acc = std::dynamic_pointer_cast<JamiAccount>(shared->account_.lock());
+            if (!acc)
+                return;
             ConfInfo newInfo;
             auto hostAdded = false;
             // Handle participants showing their video
             std::unique_lock<std::mutex> lk(shared->videoToCallMtx_);
             for (const auto& info : infos) {
                 std::string uri {};
+                std::string deviceId {};
                 auto it = shared->videoToCall_.find(info.source);
                 if (it == shared->videoToCall_.end())
                     it = shared->videoToCall_.emplace_hint(it, info.source, std::string());
@@ -113,9 +118,11 @@ Conference::Conference(const std::shared_ptr<Account>& account)
                     // a master of a conference and there is only one remote
                     // In the future, we should retrieve confInfo from the call
                     // To merge layouts informations
-                    if (auto call = getCall(it->second)) {
+                    if (auto call = std::dynamic_pointer_cast<SIPCall>(getCall(it->second))) {
                         uri = call->getPeerNumber();
                         isLocalMuted = call->isPeerMuted();
+                        if (auto* transport = call->getTransport())
+                            deviceId = transport->deviceId();
                     }
                 }
                 auto active = false;
@@ -126,13 +133,14 @@ Conference::Conference(const std::shared_ptr<Account>& account)
                 if (uri.empty()) {
                     hostAdded = true;
                     peerId = "host"sv;
+                    deviceId = acc->currentDeviceId();
                     isLocalMuted = shared->isMediaSourceMuted(MediaType::MEDIA_AUDIO);
                 }
                 auto isHandRaised = shared->isHandRaised(peerId);
                 auto isModeratorMuted = shared->isMuted(peerId);
                 auto sinkId = shared->getConfId() + peerId;
                 newInfo.emplace_back(ParticipantInfo {std::move(uri),
-                                                      {},
+                                                      deviceId,
                                                       std::move(sinkId),
                                                       active,
                                                       info.x,
@@ -1035,12 +1043,6 @@ Conference::onConfOrder(const std::string& callId, const std::string& confOrder)
     // Check if the peer is a master
     if (auto call = Manager::instance().getCallFromCallID(callId)) {
         auto peerID = string_remove_suffix(call->getPeerNumber(), '@');
-        if (!isModerator(peerID)) {
-            JAMI_WARN("Received conference order from a non master (%.*s)",
-                      (int) peerID.size(),
-                      peerID.data());
-            return;
-        }
 
         std::string err;
         Json::Value root;
@@ -1052,6 +1054,24 @@ Conference::onConfOrder(const std::string& callId, const std::string& confOrder)
                       peerID.data());
             return;
         }
+
+        if (root.isMember("handRaised")) {
+            auto state = root["handState"].asString() == "true";
+            if (peerID == root["handRaised"].asString()) {
+                // In this case, the user want to change their state
+                setHandRaised(root["handRaised"].asString(), state);
+            } else if (!state && isModerator(peerID)) {
+                // In this case a moderator can lower the hand
+                setHandRaised(root["handRaised"].asString(), state);
+            }
+        }
+
+        if (!isModerator(peerID)) {
+            JAMI_WARN("Received conference order from a non master (%.*s)",
+                      (int) peerID.size(),
+                      peerID.data());
+            return;
+        }
         if (isVideoEnabled() and root.isMember("layout")) {
             setLayout(root["layout"].asUInt());
         }
@@ -1065,9 +1085,6 @@ Conference::onConfOrder(const std::string& callId, const std::string& confOrder)
         if (root.isMember("hangupParticipant")) {
             hangupParticipant(root["hangupParticipant"].asString());
         }
-        if (root.isMember("handRaised")) {
-            setHandRaised(root["handRaised"].asString(), root["handState"].asString() == "true");
-        }
     }
 }
 

test/unitTest/call/conference.cpp

@@ -42,6 +42,7 @@ struct CallData
 {
     std::string callId {};
     std::string state {};
+    std::string device {};
     std::string hostState {};
     std::atomic_bool moderatorMuted {false};
     std::atomic_bool raisedHand {false};
@@ -50,6 +51,7 @@ struct CallData
     {
         callId = "";
         state = "";
+        device = "";
         hostState = "";
         moderatorMuted = false;
         raisedHand = false;
@@ -80,6 +82,7 @@ private:
     void testHandsUp();
     void testPeerLeaveConference();
     void testJoinCallFromOtherAccount();
+    void testDevices();
 
     CPPUNIT_TEST_SUITE(ConferenceTest);
     CPPUNIT_TEST(testGetConference);
@@ -90,6 +93,7 @@ private:
     CPPUNIT_TEST(testHandsUp);
     CPPUNIT_TEST(testPeerLeaveConference);
     CPPUNIT_TEST(testJoinCallFromOtherAccount);
+    CPPUNIT_TEST(testDevices);
     CPPUNIT_TEST_SUITE_END();
 
     // Common parts
@@ -210,12 +214,15 @@ ConferenceTest::registerSignalHandlers()
                 if (infos.at("uri").find(bobUri) != std::string::npos) {
                     bobCall.moderatorMuted = infos.at("audioModeratorMuted") == "true";
                     bobCall.raisedHand = infos.at("handRaised") == "true";
+                    bobCall.device = infos.at("device");
                 } else if (infos.at("uri").find(carlaUri) != std::string::npos) {
                     carlaCall.moderatorMuted = infos.at("audioModeratorMuted") == "true";
                     carlaCall.raisedHand = infos.at("handRaised") == "true";
+                    carlaCall.device = infos.at("device");
                 } else if (infos.at("uri").find(daviUri) != std::string::npos) {
                     daviCall.moderatorMuted = infos.at("audioModeratorMuted") == "true";
                     daviCall.raisedHand = infos.at("handRaised") == "true";
+                    daviCall.device = infos.at("device");
                 }
             }
             cv.notify_one();
@@ -452,17 +459,19 @@ ConferenceTest::testHandsUp()
     auto aliceAccount = Manager::instance().getAccount<JamiAccount>(aliceId);
     auto bobAccount = Manager::instance().getAccount<JamiAccount>(bobId);
     auto bobUri = bobAccount->getUsername();
+    auto carlaAccount = Manager::instance().getAccount<JamiAccount>(carlaId);
+    auto carlaUri = carlaAccount->getUsername();
     auto daviAccount = Manager::instance().getAccount<JamiAccount>(daviId);
     auto daviUri = daviAccount->getUsername();
 
     startConference();
 
     JAMI_INFO("Play with raise hand");
-    DRing::raiseParticipantHand(aliceId, confId, bobUri, true);
+    DRing::raiseParticipantHand(bobId, bobCall.callId, bobUri, true);
     CPPUNIT_ASSERT(
         cv.wait_for(lk, std::chrono::seconds(5), [&] { return bobCall.raisedHand.load(); }));
 
-    DRing::raiseParticipantHand(aliceId, confId, bobUri, false);
+    DRing::raiseParticipantHand(bobId, bobCall.callId, bobUri, false);
     CPPUNIT_ASSERT(
         cv.wait_for(lk, std::chrono::seconds(5), [&] { return !bobCall.raisedHand.load(); }));
 
@@ -475,10 +484,32 @@ ConferenceTest::testHandsUp()
         cv.wait_for(lk, std::chrono::seconds(20), [&] { return daviCall.hostState == "CURRENT"; }));
     Manager::instance().addParticipant(aliceId, call1, aliceId, confId);
 
-    DRing::raiseParticipantHand(aliceId, confId, daviUri, true);
+    // Remove davi from moderators
+    DRing::setModerator(aliceId, confId, daviUri, false);
+
+    // Test to raise hand
+    DRing::raiseParticipantHand(daviId, daviCall.callId, daviUri, true);
     CPPUNIT_ASSERT(
         cv.wait_for(lk, std::chrono::seconds(5), [&] { return daviCall.raisedHand.load(); }));
 
+    // Test to raise hand for another one (should fail)
+    DRing::raiseParticipantHand(bobId, bobCall.callId, carlaUri, true);
+    CPPUNIT_ASSERT(
+        !cv.wait_for(lk, std::chrono::seconds(5), [&] { return carlaCall.raisedHand.load(); }));
+
+    // However, a moderator should be able to lower the hand (but not a non moderator)
+    DRing::raiseParticipantHand(carlaId, carlaCall.callId, carlaUri, true);
+    CPPUNIT_ASSERT(
+        cv.wait_for(lk, std::chrono::seconds(5), [&] { return carlaCall.raisedHand.load(); }));
+
+    DRing::raiseParticipantHand(daviId, carlaCall.callId, carlaUri, false);
+    CPPUNIT_ASSERT(
+        !cv.wait_for(lk, std::chrono::seconds(5), [&] { return !carlaCall.raisedHand.load(); }));
+
+    DRing::raiseParticipantHand(bobId, bobCall.callId, carlaUri, false);
+    CPPUNIT_ASSERT(
+        cv.wait_for(lk, std::chrono::seconds(5), [&] { return !carlaCall.raisedHand.load(); }));
+
     Manager::instance().hangupCall(daviId, daviCall.callId);
     CPPUNIT_ASSERT(
         cv.wait_for(lk, std::chrono::seconds(20), [&] { return daviCall.state == "OVER"; }));
@@ -556,6 +587,27 @@ ConferenceTest::testJoinCallFromOtherAccount()
     DRing::unregisterSignalHandlers();
 }
 
+void
+ConferenceTest::testDevices()
+{
+    registerSignalHandlers();
+
+    auto bobAccount = Manager::instance().getAccount<JamiAccount>(bobId);
+    auto carlaAccount = Manager::instance().getAccount<JamiAccount>(carlaId);
+    auto bobDevice = std::string(bobAccount->currentDeviceId());
+    auto carlaDevice = std::string(carlaAccount->currentDeviceId());
+
+    startConference();
+
+    CPPUNIT_ASSERT(cv.wait_for(lk, std::chrono::seconds(5), [&] {
+        return bobCall.device == bobDevice && carlaDevice == carlaCall.device;
+    }));
+
+    hangupConference();
+
+    DRing::unregisterSignalHandlers();
+}
+
 } // namespace test
 } // namespace jami