[remoting][FTL] Ignore broadcasted message

This CL makes FtlMessagingClient ignore broadcasted message that doesn't
have a sender registration ID. This can happen when the user registers
a new device, which will broadcast a USERDATA message to all of the
user's devices. If this happens with a CHROMOTING_MESSAGE then it means
the sender has bug.

Currently we still ack messages with no sender registration ID, since
normally we don't expect Chromoting hosts/clients to broadcast. And if
someone (might be FTL backend) does broadcast, we want to make sure the
message won't just sit in the inbox forever.

Change-Id: Ib99796675918aae8ddcaf572c032d3fb8b929771
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1626653
Commit-Queue: Joe Downing <joedow@chromium.org>
Reviewed-by: Joe Downing <joedow@chromium.org>
Cr-Commit-Position: refs/heads/master@{#662488}

remoting/signaling/ftl_messaging_client.cc

@@ -216,6 +216,11 @@ void FtlMessagingClient::RunMessageCallbacks(const ftl::InboxMessage& message) {
   }
   message_tracker_.TrackId(message.message_id());
 
+  if (message.sender_registration_id().empty()) {
+    LOG(WARNING) << "Ignored incoming message with no sender registration ID.";
+    return;
+  }
+
   if (message.message_type() !=
       ftl::InboxMessage_MessageType_CHROMOTING_MESSAGE) {
     LOG(WARNING) << "Received message with unknown type: "

remoting/signaling/ftl_messaging_client_unittest.cc

@@ -264,6 +264,31 @@ TEST_F(FtlMessagingClientTest, TestPullMessages_Unauthenticated) {
   run_loop.Run();
 }
 
+TEST_F(FtlMessagingClientTest, TestPullMessages_IgnoresMessageWithoutRegId) {
+  base::RunLoop run_loop;
+
+  auto subscription = messaging_client_->RegisterMessageCallback(
+      CreateNotReachedMessageCallback());
+
+  messaging_client_->PullMessages(test::CheckStatusThenQuitRunLoopCallback(
+      FROM_HERE, grpc::StatusCode::OK, &run_loop));
+
+  ftl::PullMessagesResponse response;
+  ftl::InboxMessage* message = response.add_messages();
+  *message = CreateInboxMessage(kMessage1Id, kMessage1Text);
+  message->clear_sender_registration_id();
+  ServerWaitAndRespondToPullMessagesRequest(response, grpc::Status::OK);
+  ServerWaitAndRespondToAckMessagesRequest(
+      base::BindLambdaForTesting([&](const ftl::AckMessagesRequest& request) {
+        EXPECT_EQ(1, request.messages_size());
+        EXPECT_EQ(kFakeReceiverId, request.messages(0).receiver_id().id());
+        EXPECT_EQ(kMessage1Id, request.messages(0).message_id());
+        return grpc::Status::OK;
+      }),
+      base::DoNothing());
+  run_loop.Run();
+}
+
 TEST_F(FtlMessagingClientTest, TestPullMessages_IgnoresUnknownMessageType) {
   base::RunLoop run_loop;
 
@@ -275,9 +300,7 @@ TEST_F(FtlMessagingClientTest, TestPullMessages_IgnoresUnknownMessageType) {
 
   ftl::PullMessagesResponse response;
   ftl::InboxMessage* message = response.add_messages();
-  message->set_message_id(kMessage1Id);
-  message->mutable_sender_id()->set_id(kFakeSenderId);
-  message->mutable_receiver_id()->set_id(kFakeReceiverId);
+  *message = CreateInboxMessage(kMessage1Id, kMessage1Text);
   message->set_message_type(ftl::InboxMessage_MessageType_UNKNOWN);
   ServerWaitAndRespondToPullMessagesRequest(response, grpc::Status::OK);
   ServerWaitAndRespondToAckMessagesRequest(