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

remoting/host/chromoting_host.cc

Index: remoting/host/chromoting_host.cc
diff --git a/remoting/host/chromoting_host.cc b/remoting/host/chromoting_host.cc
index 7f3cadd24de9411605b8446be262038672ef696e..7ffb04b7488e1abde48ff19f1caa3611f533e13a 100644
--- a/remoting/host/chromoting_host.cc
+++ b/remoting/host/chromoting_host.cc
@@ -183,8 +183,8 @@ void ChromotingHost::AddStatusObserver(
 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,
@@ -280,6 +280,14 @@ void ChromotingHost::OnNewClientSession(
     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())) {
@@ -367,10 +375,22 @@ void ChromotingHost::OnClientDisconnected(ConnectionToClient* connection) {
   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();
   }
@@ -401,18 +421,20 @@ std::string ChromotingHost::GenerateHostAuthToken(
   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);
@@ -466,6 +488,7 @@ void ChromotingHost::LocalLoginSucceeded(
   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('/');
@@ -473,6 +496,12 @@ void ChromotingHost::LocalLoginSucceeded(
       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(