Don't use auth.json in me2me_virtual_host.

remoting/tools/me2me_virtual_host.py

 #!/usr/bin/env python
 # 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.
 
 # Virtual Me2Me implementation.  This script runs and manages the processes
 # required for a Virtual Me2Me desktop, which are: X server, X desktop
 # session, and Host process.
 # This script is intended to run continuously as a background daemon
 # process, running under an ordinary (non-root) user account.
@@ skipping 56 matching lines @@
 FIRST_X_DISPLAY_NUMBER = 20
 
 X_AUTH_FILE = os.path.expanduser("~/.Xauthority")
 os.environ["XAUTHORITY"] = X_AUTH_FILE
 
 
 # Globals needed by the atexit cleanup() handler.
 g_desktops = []
 g_pidfile = None
 
+class Config:
+  def __init__(self, path):
+    self.path = path
+    self.data = {}
+    self.changed = False
+
+  def load(self):
+    try:
+      settings_file = open(self.path, 'r')
+      self.data = json.load(settings_file)
+      self.changed = False
+      settings_file.close()
+    except:
+      return False
+    return True
+
+  def save(self):
+    if not self.changed:
+      return True
+    try:
+      old_umask = os.umask(0066)
+      settings_file = open(self.path, 'w')
+      settings_file.write(json.dumps(self.data, indent=2))
+      settings_file.close()
+      os.umask(old_umask)
+    except:
+      return False
+    self.changed = False
+    return True
+
+  def copy_from(self, config):
+    """ Copy all parameters from |config|.
+    Args:
+      config: Config object to copy from
+    """
+    self.data.update(config.data)
+    self.changed = True
+
+  def get(self, key):
+    return self.data.get(key)
+
+  def __getitem__(self, key):
+    return self.data[key]
+
+  def __setitem__(self, key, value):
+    self.data[key] = value
+    self.changed = True
+
+  def __delitem__(self, key):
+    del self.data[key]
+    self.changed = True
 
 class Authentication:
   """Manage authentication tokens for Chromoting/xmpp"""
 
-  def __init__(self, config_file):
-    self.config_file = config_file
+  def __init__(self, config):
+    self.config = config
 
   def generate_tokens(self):
     """Prompt for username/password and use them to generate new authentication
     tokens.
 
     Raises:
       Exception: Failed to get new authentication tokens.
     """
     print "Email:",
     self.login = raw_input()
     password = getpass.getpass("App-specific password: ")
 
     chromoting_auth = gaia_auth.GaiaAuthenticator('chromoting')
     self.chromoting_auth_token = chromoting_auth.authenticate(self.login,
                                                               password)
 
     xmpp_authenticator = gaia_auth.GaiaAuthenticator('chromiumsync')
     self.xmpp_auth_token = xmpp_authenticator.authenticate(self.login,
                                                            password)
 
   def load_config(self):
     try:
-      settings_file = open(self.config_file, 'r')
-      data = json.load(settings_file)
-      settings_file.close()
-      self.login = data["xmpp_login"]
-      self.chromoting_auth_token = data["chromoting_auth_token"]
-      self.xmpp_auth_token = data["xmpp_auth_token"]
-    except:
+      self.login = self.config["xmpp_login"]
+      self.chromoting_auth_token = self.config["chromoting_auth_token"]
+      self.xmpp_auth_token = self.config["xmpp_auth_token"]
+    except KeyError:
       return False
     return True
 
   def save_config(self):
-    data = {
-        "xmpp_login": self.login,
-        "chromoting_auth_token": self.chromoting_auth_token,
-        "xmpp_auth_token": self.xmpp_auth_token,
-    }
-    # File will contain private keys, so deny read/write access to others.
-    old_umask = os.umask(0066)
-    settings_file = open(self.config_file, 'w')
-    settings_file.write(json.dumps(data, indent=2))
-    settings_file.close()
-    os.umask(old_umask)
+    self.config["xmpp_login"] = self.login
+    self.config["chromoting_auth_token"] = self.chromoting_auth_token
+    self.config["xmpp_auth_token"] = self.xmpp_auth_token
 
+  def clear_config(self):
+    del self.config["xmpp_login"]
+    del self.config["chromoting_auth_token"]
+    del self.config["xmpp_auth_token"]
 
 class Host:
   """This manages the configuration for a host.
 
   Callers should instantiate a Host object (passing in a filename where the
   config will be kept), then should call either of the methods:
 
   * register(auth): Create a new Host configuration and register it
   with the Directory Service (the "auth" parameter is used to
   authenticate with the Service).
   * load_config(): Load a config from disk, with details of an existing Host
   registration.
 
   After calling register() (or making any config changes) the method
   save_config() should be called to save the details to disk.
   """
 
   server = 'www.googleapis.com'
   url = 'https://' + server + '/chromoting/v1/@me/hosts'
 
-  def __init__(self, config_file, auth):
+  def __init__(self, config, auth):
     """
     Args:
-      config_file: Host configuration file path
+      config: Host configuration object
       auth: Authentication object with credentials for authenticating with the
         Directory service.
     """
-    self.config_file = config_file
+    self.config = config
     self.auth = auth
     self.host_id = str(uuid.uuid1())
     self.host_name = socket.gethostname()
     self.host_secret_hash = None
     self.private_key = None
 
   def register(self):
     """Generates a private key for the stored |host_id|, and registers it with
     the Directory service.
 
@@ skipping 49 matching lines @@
   def set_pin(self, pin):
     if pin == "":
       self.host_secret_hash = "plain:"
     else:
       self.host_secret_hash = "hmac:" + base64.b64encode(
           hmac.new(str(self.host_id), pin, hashlib.sha256).digest())
 
   def is_pin_set(self):
     return self.host_secret_hash
 
   def load_config(self):

[Lambros, 2012/08/08 22:11:29]

Now that load_config no longer pulls data from a file, the semantics don't make
much sense.  It now looks as if this object has two internal copies of all its
state, and this method just copies one part to another part.  It would be better
if this method accepted a Config object as a parameter (instead of as a member
variable), and then the method "loads" data from the supplied Config object. 
Alternatively, have Config as a member variable (self.config), and don't copy
state - just have the fields "host_id", "host_name",... as getters (and setters)
that operate on self.config, or say that callers are allowed to operate on
self.config directly.

[Sergey Ulanov, 2012/08/08 22:41:03]

You can look at it as if self.config is just a reference to the config object
and not internal state of this object (and it's shared between auth and host
objects). But yes, I agree it's better to just pass the config to this method.
Done.

     try:
-      settings_file = open(self.config_file, 'r')
-      data = json.load(settings_file)
-      settings_file.close()
-    except:
-      logging.info("Failed to load: " + self.config_file)
+      self.host_id = self.config["host_id"]
+      self.host_name = self.config["host_name"]
+      self.host_secret_hash = self.config.get("host_secret_hash")
+      self.private_key = self.config["private_key"]
+    except KeyError:
       return False
-    self.host_id = data["host_id"]
-    self.host_name = data["host_name"]
-    self.host_secret_hash = data.get("host_secret_hash")
-    self.private_key = data["private_key"]
     return True
 
   def save_config(self):

[Lambros, 2012/08/08 22:11:29]

Same applies as for load_config.

[Sergey Ulanov, 2012/08/08 22:41:03]

Done.

-    data = {
-        "host_id": self.host_id,
-        "host_name": self.host_name,
-        "host_secret_hash": self.host_secret_hash,
-        "private_key": self.private_key,
-    }
-    if self.host_secret_hash:
-      data["host_secret_hash"] = self.host_secret_hash
+    self.config["host_id"] = self.host_id
+    self.config["host_name"] = self.host_name
+    self.config["host_secret_hash"] = self.host_secret_hash
+    self.config["private_key"] = self.private_key
 
-    old_umask = os.umask(0066)
-    settings_file = open(self.config_file, 'w')
-    settings_file.write(json.dumps(data, indent=2))
-    settings_file.close()
-    os.umask(old_umask)
-
+  def clear_config(self):

[Lambros, 2012/08/08 22:11:29]

Similar thing here.  clear_config operates on self.config, but not on
self.host_id etc - so you have the paradox where self appears to have valid
config data, but self.config is cleared out.

[Sergey Ulanov, 2012/08/08 22:41:03]

Moved this function to Config class.

+    del self.config["host_id"]
+    del self.config["host_name"]
+    del self.config["host_secret_hash"]
+    del self.config["private_key"]
 
 class Desktop:
   """Manage a single virtual desktop"""
 
   def __init__(self, sizes):
     self.x_proc = None
     self.session_proc = None
     self.host_proc = None
     self.sizes = sizes
     g_desktops.append(self)
@@ skipping 111 matching lines @@
     self.session_proc = subprocess.Popen(XSESSION_COMMAND,
                                          stdin=open(os.devnull, "r"),
                                          cwd=HOME_DIR,
                                          env=self.child_env)
     if not self.session_proc.pid:
       raise Exception("Could not start X session")
 
   def launch_host(self, host):
     # Start remoting host
     args = [locate_executable(REMOTING_COMMAND),
-            "--host-config=%s" % (host.config_file)]
-    if host.auth.config_file != host.config_file:
-      args.append("--auth-config=%s" % (host.auth.config_file))
+            "--host-config=%s" % (host.config.path)]
     self.host_proc = subprocess.Popen(args, env=self.child_env)
     logging.info(args)
     if not self.host_proc.pid:
       raise Exception("Could not start remoting host")
 
 
 class PidFile:
   """Class to allow creating and deleting a file which holds the PID of the
   running process.  This is used to detect if a process is already running, and
   inform the user of the PID.  On process termination, the PID file is
@@ skipping 296 matching lines @@
 
   atexit.register(cleanup)
 
   for s in [signal.SIGHUP, signal.SIGINT, signal.SIGTERM, signal.SIGUSR1]:
     signal.signal(s, signal_handler)
 
   # Ensure full path to config directory exists.
   if not os.path.exists(CONFIG_DIR):
     os.makedirs(CONFIG_DIR, mode=0700)
 
-  host_config_file = os.path.join(CONFIG_DIR, "host#%s.json" % host_hash)
+  host_config = Config(os.path.join(CONFIG_DIR, "host#%s.json" % host_hash))
+  host_config.load()
 
-  # --silent option is specified when we are started from WebApp UI. Don't use
-  # separate auth file in that case.
-  # TODO(sergeyu): Always use host config for auth parameters.
-  if options.silent:
-    auth_config_file = host_config_file
-  else:
-    auth_config_file = os.path.join(CONFIG_DIR, "auth.json")
+  auth = Authentication(host_config)
+  auth_config_loaded = auth.load_config()
+  if not auth_config_loaded and not options.silent:
+    # If we failed to load authentication parameters from the host config
+    # then try loading them from the legacy auth.json file.
+    auth_config = Config(os.path.join(CONFIG_DIR, "auth.json"))
+    if auth_config.load():
+      host_config.copy_from(auth_config)
+      auth_config_loaded = auth.load_config()
 
-  auth = Authentication(auth_config_file)
-  auth_config_loaded = auth.load_config()
-
-  host = Host(host_config_file, auth)
+  host = Host(host_config, auth)
   host_config_loaded = host.load_config()
 
   if options.silent:
     if not host_config_loaded or not auth_config_loaded:
       logging.error("Failed to load host configuration.")
       return 1
   else:
     need_auth_tokens = not auth_config_loaded
     need_register_host = not host_config_loaded
     # Outside the loop so user doesn't get asked twice.
     if need_register_host:
       host.ask_pin()
     elif options.new_pin or not host.is_pin_set():
       host.ask_pin()
       host.save_config()
       running, pid = PidFile(pid_filename).check()
       if running and pid != 0:
         os.kill(pid, signal.SIGUSR1)
         print "The running instance has been updated with the new PIN."
         return 0
 
     # The loop is to deal with the case of registering a new Host with
     # previously-saved auth tokens (from a previous run of this script), which
     # may require re-prompting for username & password.
     while True:
-      try:
-        if need_auth_tokens:
+      if need_auth_tokens:
+        try:
           auth.generate_tokens()
           auth.save_config()

[Lambros, 2012/08/08 22:11:29]

Since save_config() no longer writes to disk, this is a change of behavior if
you get to "return 1" below "need_register_host:" - the auth data is no longer
saved in this case.  It's not a big deal, but it would be nice if we could save
the auth data if this happens, so the user doesn't have to re-enter their
details.

[Sergey Ulanov, 2012/08/08 22:41:03]

Done.

           need_auth_tokens = False
-      except Exception:
-        logging.error("Authentication failed")
-        return 1
+        except Exception:
+          logging.error("Authentication failed")
+          return 1
 
-      try:
-        if need_register_host:
+      if need_register_host:
+        try:
           host.register()
           host.save_config()
-      except urllib2.HTTPError, err:
-        if err.getcode() == 401:
-          # Authentication failed - re-prompt for username & password.
-          need_auth_tokens = True
-          continue
-        else:
-          # Not an authentication error.
-          logging.error("Directory returned error: " + str(err))
-          logging.error(err.read())
-          return 1
+        except urllib2.HTTPError, err:
+          if err.getcode() == 401:
+            # Authentication failed - re-prompt for username & password.
+            need_auth_tokens = True
+            continue
+          else:
+            # Not an authentication error.
+            logging.error("Directory returned error: " + str(err))
+            logging.error(err.read())
+            return 1
 
       # |auth| and |host| are both set up, so break out of the loop.
       break
 
+    if not host_config.save():
+      logging.error("Faled to save host configuration.")
+      return 1
+
   global g_pidfile
   g_pidfile = PidFile(pid_filename)
   running, pid = g_pidfile.check()
 
   if running:
     print "An instance of this script is already running."
     print "Use the -k flag to terminate the running instance."
     print "If this isn't the case, delete '%s' and try again." % pid_filename
     return 1
 
@@ skipping 78 matching lines @@
       logging.info("Host process terminated")
       desktop.host_proc = None
 
       # These exit-codes must match the ones used by the host.
       # See remoting/host/constants.h.
       # Delete the host or auth configuration depending on the returned error
       # code, so the next time this script is run, a new configuration
       # will be created and registered.
       if os.WEXITSTATUS(status) == 2:
         logging.info("Host configuration is invalid - exiting.")
-        try:
-          os.remove(host.config_file)
-          os.remove(auth.config_file)
-        except:
-          pass
+        auth.clear_config()
+        host.clear_config()
+        host_config.save()
         return 0
       elif os.WEXITSTATUS(status) == 3:
         logging.info("Host ID has been deleted - exiting.")
-        try:
-          os.remove(host.config_file)
-        except:
-          pass
+        host.clear_config()
+        host_config.save()
         return 0
       elif os.WEXITSTATUS(status) == 4:
         logging.info("OAuth credentials are invalid - exiting.")
-        try:
-          os.remove(auth.config_file)
-        except:
-          pass
+        auth.clear_config()
+        host_config.save()
         return 0
       elif os.WEXITSTATUS(status) == 5:
         logging.info("Host domain is blocked by policy - exiting.")
         os.remove(host.config_file)
         return 0
 
 if __name__ == "__main__":
   logging.basicConfig(level=logging.DEBUG)
   sys.exit(main())