Fix DaemonControllerLinux::SetConfigAndStart() to reload config automatically.

Also fixed several issues with managing chromoting from the webapp. Now it is
possible to start the app and change PIN.

BUG=120950

Review URL: https://chromiumcodereview.appspot.com/10824286

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@151969 0039d316-1c4b-4281-b951-d872f2087c98

remoting/host/host_config.h

@@ -9,6 +9,10 @@
 
 #include "base/basictypes.h"
 
+namespace base {
+class DictionaryValue;
+}  // namespace base
+
 namespace remoting {
 
 // Following constants define names for configuration parameters.
@@ -60,6 +64,11 @@ class MutableHostConfig : public HostConfig {
                          const std::string& in_value) = 0;
   virtual void SetBoolean(const std::string& path, bool in_value) = 0;
 
+  // Copy configuration from specified |dictionary|. Returns false if the
+  // |dictionary| contains some values that cannot be saved in the config. In
+  // that case, all other values are still copied.
+  virtual bool CopyFrom(const base::DictionaryValue* dictionary) = 0;
+
   // Saves changes.
   virtual bool Save() = 0;
 

remoting/host/in_memory_host_config.cc

@@ -44,4 +44,23 @@ void InMemoryHostConfig::SetBoolean(const std::string& path, bool in_value) {
   values_->SetBoolean(path, in_value);
 }
 
+bool InMemoryHostConfig::CopyFrom(const base::DictionaryValue* dictionary) {
+  bool result = true;
+
+  for (DictionaryValue::key_iterator key(dictionary->begin_keys());
+       key != dictionary->end_keys(); ++key) {
+    std::string str_value;
+    bool bool_value;
+    if (dictionary->GetString(*key, &str_value)) {
+      SetString(*key, str_value);
+    } else if (dictionary->GetBoolean(*key, &bool_value)) {
+      SetBoolean(*key, bool_value);
+    } else {
+      result = false;
+    }
+  }
+
+  return result;
+}
+
 }  // namespace remoting

remoting/host/in_memory_host_config.h

@@ -34,6 +34,8 @@ class InMemoryHostConfig : public MutableHostConfig,
                          const std::string& in_value) OVERRIDE;
   virtual void SetBoolean(const std::string& path, bool in_value) OVERRIDE;
 
+  virtual bool CopyFrom(const base::DictionaryValue* dictionary) OVERRIDE;
+
   virtual bool Save() OVERRIDE;
 
  protected:

remoting/host/plugin/daemon_controller_linux.cc

@@ -43,15 +43,6 @@ std::string GetMd5(const std::string& value) {
   return StringToLowerASCII(base::HexEncode(digest.a, sizeof(digest.a)));
 }
 
-// TODO(sergeyu): This is a very hacky implementation of
-// DaemonController interface for linux. Current version works, but
-// there are sevaral problems with it:
-//   * All calls are executed synchronously, even though this API is
-//     supposed to be asynchronous.
-//   * The host is configured by passing configuration data as CL
-//     argument - this is obviously not secure.
-// Rewrite this code to solve these two problems.
-// http://crbug.com/120950 .
 class DaemonControllerLinux : public remoting::DaemonController {
  public:
   DaemonControllerLinux();
@@ -235,19 +226,9 @@ void DaemonControllerLinux::DoSetConfigAndStart(
     scoped_ptr<base::DictionaryValue> config,
     const CompletionCallback& done_callback) {
   JsonHostConfig config_file(GetConfigPath());
-  for (DictionaryValue::key_iterator key(config->begin_keys());
-       key != config->end_keys(); ++key) {
-    std::string value;
-    if (!config->GetString(*key, &value)) {
-      LOG(ERROR) << *key << " is not a string.";
-      done_callback.Run(RESULT_FAILED);
-      return;
-    }
-    config_file.SetString(*key, value);
-  }
-
-  bool success = config_file.Save();
-  if (!success) {
+  if (!config_file.CopyFrom(config.get()) ||
+      !config_file.Save()) {
+    LOG(ERROR) << "Failed to update config file.";
     done_callback.Run(RESULT_FAILED);
     return;
   }
@@ -268,23 +249,25 @@ void DaemonControllerLinux::DoUpdateConfig(
     scoped_ptr<base::DictionaryValue> config,
     const CompletionCallback& done_callback) {
   JsonHostConfig config_file(GetConfigPath());
-  if (!config_file.Read()) {
+  if (!config_file.Read() ||
+      !config_file.CopyFrom(config.get()) ||
+      !config_file.Save()) {
+    LOG(ERROR) << "Failed to update config file.";
     done_callback.Run(RESULT_FAILED);
+    return;
   }
 
-  for (DictionaryValue::key_iterator key(config->begin_keys());
-       key != config->end_keys(); ++key) {
-    std::string value;
-    if (!config->GetString(*key, &value)) {
-      LOG(ERROR) << *key << " is not a string.";
-      done_callback.Run(RESULT_FAILED);
-      return;
-    }
-    config_file.SetString(*key, value);
+  std::vector<std::string> args;
+  args.push_back("--reload");
+  AsyncResult result;
+  int exit_code;
+  if (RunScript(args, &exit_code)) {
+    result = (exit_code == 0) ? RESULT_OK : RESULT_FAILED;
+  } else {
+    result = RESULT_FAILED;
   }
-  bool success = config_file.Save();
-  done_callback.Run(success ? RESULT_OK : RESULT_FAILED);
-  // TODO(sergeyu): Send signal to the daemon to restart the host.
+
+  done_callback.Run(result);
 }
 
 void DaemonControllerLinux::DoStop(const CompletionCallback& done_callback) {

remoting/host/plugin/daemon_controller_mac.cc

@@ -228,15 +228,10 @@ void DaemonControllerMac::DoUpdateConfig(
     done_callback.Run(RESULT_FAILED);
     return;
   }
-  for (DictionaryValue::key_iterator key(config->begin_keys());
-       key != config->end_keys(); ++key) {
-    std::string value;
-    if (!config->GetString(*key, &value)) {
-      LOG(ERROR) << *key << " is not a string.";
-      done_callback.Run(RESULT_FAILED);
-      return;
-    }
-    config_file.SetString(*key, value);
+  if (!config_file.CopyFrom(config.get())) {
+    LOG(ERROR) << "Failed to update configuration.";
+    done_callback.Run(RESULT_FAILED);
+    return;
   }
 
   std::string config_data = config_file.GetSerializedData();

remoting/tools/me2me_virtual_host.py

@@ -118,6 +118,7 @@ class Config:
     del self.data["xmpp_login"]
     del self.data["chromoting_auth_token"]
     del self.data["xmpp_auth_token"]
+    del self.data["oauth_refresh_token"]
 
   def clear_host_info(self):
     del self.data["host_id"]
@@ -129,7 +130,10 @@ class Authentication:
   """Manage authentication tokens for Chromoting/xmpp"""
 
   def __init__(self):
-    pass
+    self.login = None
+    self.chromoting_auth_token = None
+    self.xmpp_auth_token = None
+    self.oauth_refresh_token = None
 
   def generate_tokens(self):
     """Prompt for username/password and use them to generate new authentication
@@ -150,19 +154,41 @@ class Authentication:
     self.xmpp_auth_token = xmpp_authenticator.authenticate(self.login,
                                                            password)
 
+  def has_chromoting_credentials(self):
+    """Returns True if we have credentials for the directory"""
+    return self.chromoting_auth_token != None
+
+  def has_xmpp_credentials(self):
+    """Returns True if we have credentials to authenticate XMPP connection"""
+    # XMPP connection can be authenticated using either OAuth or XMPP token.
+    return self.oauth_refresh_token != None or self.xmpp_auth_token != None
+
   def load_config(self, config):
+    """Loads the config and returns false if the config is invalid. After
+    config is loaded caller must use has_xmpp_credentials() and
+    has_chromoting_credentials() to check that the credentials it needs are
+    present in the config."""
+
+    # Host can use different types of auth tokens depending on how the config
+    # was generated. E.g. if the config was created by the webapp it will have
+    # only oauth token. We still consider config to be valid in this case.
+    self.chromoting_auth_token = config.get("chromoting_auth_token")
+    self.oauth_refresh_token = config.get("oauth_refresh_token")
+    self.xmpp_auth_token = config.get("xmpp_auth_token")
+
     try:
       self.login = config["xmpp_login"]
-      self.chromoting_auth_token = config["chromoting_auth_token"]
-      self.xmpp_auth_token = config["xmpp_auth_token"]
     except KeyError:
       return False
     return True
 
   def save_config(self, config):
     config["xmpp_login"] = self.login
-    config["chromoting_auth_token"] = self.chromoting_auth_token
-    config["xmpp_auth_token"] = self.xmpp_auth_token
+    if self.chromoting_auth_token:
+      config["chromoting_auth_token"] = self.chromoting_auth_token
+    if self.xmpp_auth_token:
+      config["xmpp_auth_token"] = self.xmpp_auth_token
+
 
 class Host:
   """This manages the configuration for a host.
@@ -661,6 +687,9 @@ def main():
   parser.add_option("", "--silent", dest="silent", default=False,
                     action="store_true",
                     help="Start the host without trying to configure it.")
+  parser.add_option("", "--reload", dest="reload", default=False,
+                    action="store_true",
+                    help="Signal currently running host to reload the config.")
   (options, args) = parser.parse_args()
 
   host_hash = hashlib.md5(socket.gethostname()).hexdigest()
@@ -679,6 +708,13 @@ def main():
       os.kill(pid, signal.SIGTERM)
     return 0
 
+  if options.reload:
+    running, pid = PidFile(pid_filename).check()
+    if not running:
+      return 1
+    os.kill(pid, signal.SIGUSR1)
+    return 0
+
   if not options.size:
     options.size = [DEFAULT_SIZE]
 
@@ -743,11 +779,16 @@ def main():
   host_config_loaded = host.load_config(host_config)
 
   if options.silent:
+    # Just validate the config when run with --silent.
     if not host_config_loaded or not auth_config_loaded:
       logging.error("Failed to load host configuration.")
       return 1
+    if not auth.has_xmpp_credentials():
+      logging.error("Auth tokens are not configured.")
+      return 1
   else:
-    need_auth_tokens = not auth_config_loaded
+    need_auth_tokens = (
+        not auth_config_loaded or not auth.has_chromoting_credentials())
     need_register_host = not host_config_loaded
     # Outside the loop so user doesn't get asked twice.
     if need_register_host: