Notify calling web-app when Host plugin becomes connected to a client.

remoting/host/chromoting_host.cc

 // Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "remoting/host/chromoting_host.h"
 
 #include "build/build_config.h"
 #include "remoting/base/constants.h"
 #include "remoting/base/encoder.h"
 #include "remoting/base/encoder_row_based.h"
@@ skipping 165 matching lines @@
     const scoped_refptr<HostStatusObserver>& observer) {
   DCHECK_EQ(state_, kInitial);
   status_observers_.push_back(observer);
 }
 
 ////////////////////////////////////////////////////////////////////////////
 // protocol::ConnectionToClient::EventHandler implementations
 void ChromotingHost::OnConnectionOpened(ConnectionToClient* connection) {
   DCHECK_EQ(context_->network_message_loop(), MessageLoop::current());
   VLOG(1) << "Connection to client established.";
+  // TODO(wez): ChromotingHost shouldn't need to know about Me2Mom.
   if (is_me2mom_) {
-    // TODO(wez): Improve our authentication framework.
     context_->main_message_loop()->PostTask(
         FROM_HERE,
         NewRunnableMethod(this, &ChromotingHost::ProcessPreAuthentication,
                           make_scoped_refptr(connection)));
   }
 }
 
 void ChromotingHost::OnConnectionClosed(ConnectionToClient* connection) {
   DCHECK_EQ(context_->network_message_loop(), MessageLoop::current());
 
@@ skipping 75 matching lines @@
 
 void ChromotingHost::OnNewClientSession(
     protocol::Session* session,
     protocol::SessionManager::IncomingSessionResponse* response) {
   base::AutoLock auto_lock(lock_);
   if (state_ != kStarted) {
     *response = protocol::SessionManager::DECLINE;
     return;
   }
 
+  // If we are running Me2Mom and already have an authenticated client then
+  // reject the connection immediately.

[Jamie, 2011/06/10 18:00:38]

What's the concern here? If we're worried that a bad guy might have obtained the
access code and try to connect, then I'd be tempted to go further and disconnect
everyone. After all, who's to say that the bad guy didn't get there first?

Also, is this really core to getting the correct state into the web-app? It
feels orthogonal to it, and would probably be better as a separate CL.

[Wez, 2011/06/13 20:30:35]

The Access Code is one-shot by design, so from a security perspective,
regardless of whether you can think of a good exploit for it, it should be
implemented as one-shot to avoid nasty surprises due to the more lax implemented
vs designed behaviour.

In practice, the issue is that the time between the Access Code being generated
and being used is likely to be an order of magnitude smaller than the duration
of a session, so leaving the Access Code open for the duration of the session
increases the attack window by that order of magnitude, too.

I agree that this is really a separate change, though; I'll pull it out (you
win!), but you have to review it, please (you lose!).

+  // TODO(wez): ChromotingHost shouldn't need to know about Me2Mom.
+  if (is_me2mom_ && AuthenticatedClientsCount() > 0) {
+    *response = protocol::SessionManager::DECLINE;
+    return;
+  }
+
   // Check that the client has access to the host.
   if (!access_verifier_->VerifyPermissions(session->jid(),
                                            session->initiator_token())) {
     *response = protocol::SessionManager::DECLINE;
     return;
   }
 
   // TODO(simonmorris): The resolution is set in the video stream now,
   // so it doesn't need to be set here.
   *protocol_config_->mutable_initial_resolution() =
@@ skipping 67 matching lines @@
     if (client->get()->authenticated()) {
       recorder_->Stop(NULL);
       recorder_ = NULL;
     }
   }
 
   // Close the connection to connection just to be safe.
   connection->Disconnect();
 
   // Also remove reference to ConnectionToClient from this object.
+  int old_authenticated_clients = AuthenticatedClientsCount();
   clients_.erase(client);
 
-  if (!HasAuthenticatedClients()) {
+  // Notify the observers of the change, if any.
+  int authenticated_clients = AuthenticatedClientsCount();
+  if (old_authenticated_clients != authenticated_clients) {
+    for (StatusObserverList::iterator it = status_observers_.begin();
+         it != status_observers_.end(); ++it) {
+      (*it)->OnAuthenticatedClientsChanged(authenticated_clients);
+    }
+  }
+
+  // Enable the "curtain", if at least one client is actually authenticated.
+  if (AuthenticatedClientsCount() > 0) {
     EnableCurtainMode(false);
+    // TODO(wez): ChromotingHost shouldn't need to know about Me2Mom.
     if (is_me2mom_)
       desktop_environment_->disconnect_window()->Hide();
   }
 }
 
 // TODO(sergeyu): Move this to SessionManager?
 Encoder* ChromotingHost::CreateEncoder(const protocol::SessionConfig* config) {
   const protocol::ChannelConfig& video_config = config->video_config();
 
   if (video_config.codec == protocol::ChannelConfig::CODEC_VERBATIM) {
@@ skipping 10 matching lines @@
 
   return NULL;
 }
 
 std::string ChromotingHost::GenerateHostAuthToken(
     const std::string& encoded_client_token) {
   // TODO(ajwong): Return the signature of this instead.
   return encoded_client_token;
 }
 
-bool ChromotingHost::HasAuthenticatedClients() const {
+int ChromotingHost::AuthenticatedClientsCount() const {
+  int authenticated_clients = 0;
   for (ClientList::const_iterator it = clients_.begin(); it != clients_.end();
        ++it) {
     if (it->get()->authenticated())
-      return true;
+      ++authenticated_clients;
   }
-  return false;
+  return authenticated_clients;
 }
 
 void ChromotingHost::EnableCurtainMode(bool enable) {
   // TODO(jamiewalch): This will need to be more sophisticated when we think
   // about proper crash recovery and daemon mode.
+  // TODO(wez): CurtainMode shouldn't be driven directly by ChromotingHost.

[Jamie, 2011/06/10 18:00:38]

+1

[Wez, 2011/06/13 20:30:35]

As requested, I've removed the dups of this comment...

   if (is_me2mom_ || enable == is_curtained_)
     return;
   desktop_environment_->curtain()->EnableCurtainMode(enable);
   is_curtained_ = enable;
 }
 
 void ChromotingHost::LocalLoginSucceeded(
     scoped_refptr<ConnectionToClient> connection) {
   if (MessageLoop::current() != context_->main_message_loop()) {
     context_->main_message_loop()->PostTask(
@@ skipping 33 matching lines @@
                                    context_->encode_message_loop(),
                                    context_->network_message_loop(),
                                    desktop_environment_->capturer(),
                                    encoder);
   }
 
   // Immediately add the connection and start the session.
   recorder_->AddConnection(connection);
   recorder_->Start();
   EnableCurtainMode(true);
+  // TODO(wez): ChromotingHost shouldn't need to know about Me2Mom.
   if (is_me2mom_) {
     std::string username = connection->session()->jid();
     size_t pos = username.find('/');
     if (pos != std::string::npos)
       username.replace(pos, std::string::npos, "");
     desktop_environment_->disconnect_window()->Show(this, username);
   }
+
+  // Notify observers that there is at least one authenticated client.
+  for (StatusObserverList::iterator it = status_observers_.begin();
+       it != status_observers_.end(); ++it) {
+    (*it)->OnAuthenticatedClientsChanged(AuthenticatedClientsCount());
+  }
 }
 
 void ChromotingHost::LocalLoginFailed(
     scoped_refptr<ConnectionToClient> connection) {
   if (MessageLoop::current() != context_->main_message_loop()) {
     context_->main_message_loop()->PostTask(
         FROM_HERE,
         NewRunnableMethod(this, &ChromotingHost::LocalLoginFailed, connection));
     return;
   }
@@ skipping 11 matching lines @@
   ClientList::iterator client;
   for (client = clients_.begin(); client != clients_.end(); ++client) {
     if (client->get()->connection() == connection)
       break;
   }
   CHECK(client != clients_.end());
   client->get()->OnAuthorizationComplete(true);
 }
 
 }  // namespace remoting