[Chromoting] Implement the host domain policy.

remoting/host/plugin/host_script_object.cc

 // Copyright (c) 2012 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/plugin/host_script_object.h"
 
 #include "base/bind.h"
 #include "base/json/json_reader.h"
 #include "base/json/json_writer.h"
 #include "base/message_loop.h"
@@ skipping 60 matching lines @@
 const char* kAttrNameDisconnecting = "DISCONNECTING";
 const char* kAttrNameError = "ERROR";
 
 const int kMaxLoginAttempts = 5;
 
 // We may need to have more than one task running at the same time
 // (e.g. key generation and status update), yet unlikely to ever need
 // more than 2 threads.
 const int kMaxWorkerPoolThreads = 2;
 
+// Whether a given string ends with a given suffix.
+bool EndsWith(std::string s, std::string suffix) {

[Sergey Ulanov, 2012/07/31 20:59:53]

reuse base::EndsWith()?

[simonmorris, 2012/07/31 23:16:10]

Done.

+  if (s.length() < suffix.length()) {
+    return false;
+  }
+  return s.compare(s.length() - suffix.length(), suffix.length(), suffix) == 0;
+}
+
 }  // namespace
 
 HostNPScriptObject::HostNPScriptObject(
     NPP plugin,
     NPObject* parent,
     PluginThreadTaskRunner::Delegate* plugin_thread_delegate)
     : plugin_(plugin),
       parent_(parent),
       am_currently_logging_(false),
       state_(kDisconnected),
@@ skipping 444 matching lines @@
         &HostNPScriptObject::FinishConnectNetworkThread, base::Unretained(this),
         uid, auth_token, auth_service));
     return;
   }
 
   if (state_ != kStarting) {
     // Host has been stopped while we were fetching policy.
     return;
   }
 
+  // Check the host domain policy.
+  if (!host_domain_.empty() &&
+      !EndsWith(uid, std::string("@") + host_domain_)) {
+    SetState(kError);
+    return;
+  }
+
   // Verify that DesktopEnvironment has been created.
   if (desktop_environment_.get() == NULL) {
     SetState(kError);
     return;
   }
 
   // Generate a key pair for the Host to use.
   // TODO(wez): Move this to the worker thread.
   host_key_pair_.Generate();
 
@@ skipping 314 matching lines @@
     case kStarting:
       SetState(kDisconnecting);
       SetState(kDisconnected);
       disconnected_event_.Signal();
       return;
 
     case kDisconnecting:
       return;
 
     default:
-      DCHECK(host_);
       SetState(kDisconnecting);
 
+      if (!host_) {
+        OnShutdownFinished();
+        return;
+      }
       // ChromotingHost::Shutdown() may destroy SignalStrategy
-      // synchronously, bug SignalStrategy::Listener handlers are not
+      // synchronously, but SignalStrategy::Listener handlers are not
       // allowed to destroy SignalStrategy, so post task to call
       // Shutdown() later.
       host_context_->network_task_runner()->PostTask(
           FROM_HERE, base::Bind(
               &ChromotingHost::Shutdown, host_,
               base::Bind(&HostNPScriptObject::OnShutdownFinished,
                          base::Unretained(this))));
+      return;
   }
 }
 
 void HostNPScriptObject::OnShutdownFinished() {
   DCHECK(host_context_->network_task_runner()->BelongsToCurrentThread());
 
   disconnected_event_.Signal();
 }
 
 void HostNPScriptObject::OnPolicyUpdate(
     scoped_ptr<base::DictionaryValue> policies) {
   if (!host_context_->network_task_runner()->BelongsToCurrentThread()) {
     host_context_->network_task_runner()->PostTask(
         FROM_HERE,
         base::Bind(&HostNPScriptObject::OnPolicyUpdate,
                    base::Unretained(this), base::Passed(&policies)));
     return;
   }
 
   bool bool_value;
+  std::string string_value;

[Sergey Ulanov, 2012/07/31 20:59:53]

move this below to where it's used. Also use a better name for it, e.g.
domain_policy?

[simonmorris, 2012/07/31 23:16:10]

Done.

   if (policies->GetBoolean(policy_hack::PolicyWatcher::kNatPolicyName,
                            &bool_value)) {
-    OnNatPolicyUpdate(bool_value);
+    UpdateNatPolicy(bool_value);
+  }
+  if (policies->GetString(policy_hack::PolicyWatcher::kHostDomainPolicyName,

[Sergey Ulanov, 2012/07/31 20:59:53]

We always expect this value to be present - DCHECK if it's not there?
Same with kNatPolicyName

[simonmorris, 2012/07/31 23:16:10]

PolicyWatcher only emits changed policy values, so that its clients don't have
to compare new values to old values. So this value is only guaranteed to exist
the first time OnPolicyUpdate is called.

+                          &string_value)) {
+    UpdateHostDomainPolicy(string_value);
+  }
+
+  if (!pending_connect_.is_null()) {
+    pending_connect_.Run();
+    pending_connect_.Reset();
   }
 }
 
-void HostNPScriptObject::OnNatPolicyUpdate(bool nat_traversal_enabled) {
-  if (!host_context_->network_task_runner()->BelongsToCurrentThread()) {
-    host_context_->network_task_runner()->PostTask(
-        FROM_HERE,
-        base::Bind(&HostNPScriptObject::OnNatPolicyUpdate,
-                   base::Unretained(this), nat_traversal_enabled));
-    return;
-  }
+void HostNPScriptObject::UpdateNatPolicy(bool nat_traversal_enabled) {
+  DCHECK(host_context_->network_task_runner()->BelongsToCurrentThread());
 
-  VLOG(2) << "OnNatPolicyUpdate: " << nat_traversal_enabled;
+  VLOG(2) << "UpdateNatPolicy: " << nat_traversal_enabled;
 
   // When transitioning from enabled to disabled, force disconnect any
   // existing session.
   if (nat_traversal_enabled_ && !nat_traversal_enabled) {
     DisconnectInternal();
   }
 
   {
     base::AutoLock lock(nat_policy_lock_);
     policy_received_ = true;

[Sergey Ulanov, 2012/07/31 20:59:53]

This logically belongs to OnPolicyUpdate(). Then you need to acquire
nat_policy_lock_ only when setting policy_receveid_. 

I think ideally we shouldn't need nat_policy_lock_ at all.
HostNPScriptObject::SetProperty() could jump to the network thread if necessary,
but obviously it's not something that needs to be in this CL.

[simonmorris, 2012/07/31 23:16:10]

I've moved that line to OnPolicyUpdate(). But I think I still need the lock
here, because the nat_traversal_enabled_ may be read on a different thread.

     nat_traversal_enabled_ = nat_traversal_enabled;
   }
 
   UpdateWebappNatPolicy(nat_traversal_enabled_);
+}
 
-  if (!pending_connect_.is_null()) {
-    pending_connect_.Run();
-    pending_connect_.Reset();
+void HostNPScriptObject::UpdateHostDomainPolicy(
+    const std::string& host_domain) {
+  DCHECK(host_context_->network_task_runner()->BelongsToCurrentThread());
+
+  VLOG(2) << "UpdateHostDomainPolicy: " << host_domain;
+
+  // When setting a host domain policy, force disconnect any existing session.
+  if (!host_domain.empty() && state_ != kStarting) {

[Sergey Ulanov, 2012/07/31 20:59:53]

hm. This will disconnect all connection even when domain hasn't changed. add
host_domain_ != host_domain condition?

[simonmorris, 2012/07/31 23:16:10]

If the domain hasn't changed, the PolicyWatcher won't pass it to
OnPolicyUpdate().

+    DisconnectInternal();
   }
+
+  host_domain_ = host_domain;
 }
 
 void HostNPScriptObject::OnReceivedSupportID(
     bool success,
     const std::string& support_id,
     const base::TimeDelta& lifetime) {
   DCHECK(host_context_->network_task_runner()->BelongsToCurrentThread());
 
   if (!success) {
     SetState(kError);
@@ skipping 306 matching lines @@
   return is_good;
 }
 
 void HostNPScriptObject::SetException(const std::string& exception_string) {
   DCHECK(plugin_task_runner_->BelongsToCurrentThread());
   g_npnetscape_funcs->setexception(parent_, exception_string.c_str());
   LOG(INFO) << exception_string;
 }
 
 }  // namespace remoting