Default account backend: write accounts to XDG_DATA_HOME, with fallback

If the user has ~/.mission-control/accounts/accounts.cfg we migrate
from there to XDG_DATA_HOME/telepathy/mission-control/accounts.cfg,
and if successful, delete the old name.

If the user has XDG_DATA_DIRS/telepathy/mission-control/accounts.cfg
(in a lower-priority path element than XDG_DATA_HOME), we use it,
with copy-on-write into XDG_DATA_HOME.

(Limitation: the account-store executable used in some tests only reads
from XDG_DATA_HOME, and doesn't understand the XDG_DATA_DIRS and
MC_ACCOUNT_DIR fallback.)

Bug: https://bugs.freedesktop.org/show_bug.cgi?id=35896
Signed-off-by: Simon McVittie <simon.mcvittie@collabora.co.uk>

4 files changed, 120 insertions, 37 deletions

diff --git a/src/mcd-account-manager-default.c b/src/mcd-account-manager-default.c
index dcf8c959..ff48ac73 100644
--- a/src/mcd-account-manager-default.c
+++ b/src/mcd-account-manager-default.c
@@ -20,7 +20,12 @@
  */
 
 #include "config.h"
+
+#include <errno.h>
 #include <string.h>
+
+#include <glib/gstdio.h>
+
 #include "mcd-account-manager-default.h"
 #include "mcd-debug.h"
 #include "mcd-misc.h"
@@ -343,7 +348,7 @@ G_DEFINE_TYPE_WITH_CODE (McdAccountManagerDefault, mcd_account_manager_default,
         account_storage_iface_init));
 
 static gchar *
-get_account_conf_filename (void)
+get_old_filename (void)
 {
   const gchar *base;
 
@@ -361,11 +366,18 @@ get_account_conf_filename (void)
     return g_build_filename (base, "accounts.cfg", NULL);
 }
 
+static gchar *
+account_filename_in (const gchar *dir)
+{
+  return g_build_filename (dir, "telepathy", "mission-control", "accounts.cfg",
+      NULL);
+}
+
 static void
 mcd_account_manager_default_init (McdAccountManagerDefault *self)
 {
   DEBUG ("mcd_account_manager_default_init");
-  self->filename = get_account_conf_filename ();
+  self->filename = account_filename_in (g_get_user_data_dir ());
   self->keyfile = g_key_file_new ();
   self->secrets = g_key_file_new ();
   self->removed = g_key_file_new ();
@@ -677,11 +689,57 @@ _list (const McpAccountStorage *self,
 
   if (!amd->loaded)
     {
+      const gchar * const *iter;
+
+      for (iter = g_get_system_data_dirs ();
+          iter != NULL && *iter != NULL;
+          iter++)
+        {
+          gchar *filename = account_filename_in (*iter);
+
+          if (g_file_test (filename, G_FILE_TEST_EXISTS))
+            {
+              am_default_load_keyfile (amd, filename);
+              amd->loaded = TRUE;
+              /* Do not set amd->save: we don't need to write it to a
+               * higher-priority directory until it actually changes. */
+            }
+
+          g_free (filename);
+
+          if (amd->loaded)
+            break;
+        }
+    }
+
+  if (!amd->loaded)
+    {
+      gchar *old_filename = get_old_filename ();
+
+      if (g_file_test (old_filename, G_FILE_TEST_EXISTS))
+        {
+          am_default_load_keyfile (amd, old_filename);
+          amd->loaded = TRUE;
+          amd->save = TRUE;
+
+          if (_commit (self, am, NULL))
+            {
+              DEBUG ("Migrated %s to new location: deleting old copy");
+              if (g_unlink (old_filename) != 0)
+                g_warning ("Unable to delete %s: %s", old_filename,
+                    g_strerror (errno));
+            }
+        }
+
+      g_free (old_filename);
+    }
+
+  if (!amd->loaded)
+    {
       DEBUG ("Creating initial account data");
       g_key_file_load_from_data (amd->keyfile, INITIAL_CONFIG, -1,
           G_KEY_FILE_KEEP_COMMENTS, NULL);
       amd->loaded = TRUE;
-      /* create the placeholder file */
       amd->save = TRUE;
       _commit (self, am, NULL);
     }
diff --git a/tests/account-store-default.c b/tests/account-store-default.c
index 944bf2f7..513799a5 100644
--- a/tests/account-store-default.c
+++ b/tests/account-store-default.c
@@ -146,26 +146,8 @@ _keyring_remove_account (const gchar *acct)
 
 static const gchar *default_config (void)
 {
-  const gchar *base;
-  static const gchar *path = NULL;
-
-  if (path != NULL)
-    return path;
-
-  base = g_getenv ("MC_ACCOUNT_DIR");
-
-  if (!base)
-    base = ACCOUNTS_DIR;
-
-  if (!base)
-    return NULL;
-
-  if (base[0] == '~')
-    path = g_build_filename (g_get_home_dir(), base + 1, "accounts.cfg", NULL);
-  else
-    path = g_build_filename (base, "accounts.cfg", NULL);
-
-  return path;
+  return g_build_filename (g_get_user_data_dir (), "telepathy",
+      "mission-control", "accounts.cfg", NULL);
 }
 
 static GKeyFile * default_keyfile (void)
diff --git a/tests/twisted/account-storage/default-keyring-storage.py b/tests/twisted/account-storage/default-keyring-storage.py
index c1230788..93feca4f 100644
--- a/tests/twisted/account-storage/default-keyring-storage.py
+++ b/tests/twisted/account-storage/default-keyring-storage.py
@@ -124,7 +124,9 @@ def stop_gnome_keyring_daemon():
 
 def test(q, bus, mc):
     ctl_dir = os.environ['MC_ACCOUNT_DIR']
-    key_file_name = os.path.join(ctl_dir, 'accounts.cfg')
+    old_key_file_name = os.path.join(ctl_dir, 'accounts.cfg')
+    new_key_file_name = os.path.join(os.environ['XDG_DATA_HOME'],
+            'telepathy', 'mission-control', 'accounts.cfg')
     group = 'fakecm/fakeprotocol/dontdivert_40example_2ecom0'
 
     account_manager, properties, interfaces = connect_to_mc(q, bus, mc)
@@ -161,7 +163,8 @@ def test(q, bus, mc):
     tell_mc_to_die(q, bus)
 
     # .. let's check the keyfile
-    kf = keyfile_read(key_file_name)
+    assert not os.path.exists(old_key_file_name)
+    kf = keyfile_read(new_key_file_name)
     assert group in kf, kf
     assert kf[group]['manager'] == 'fakecm'
     assert kf[group]['protocol'] == 'fakeprotocol'
@@ -204,7 +207,8 @@ def test(q, bus, mc):
         )
 
     # Check the account is correctly deleted
-    kf = keyfile_read(key_file_name)
+    assert not os.path.exists(old_key_file_name)
+    kf = keyfile_read(new_key_file_name)
     assert group not in kf, kf
 
     if use_keyring:
@@ -215,11 +219,22 @@ def test(q, bus, mc):
     # Tell MC to die, again
     tell_mc_to_die(q, bus)
 
-    # Write out account configurations in which the password is in
+    # Write out an account configuration in which the password is in
     # both the keyfile and the keyring
 
-    account_store('set', 'default', 'param-password', 'password_in_keyring')
-    open(ctl_dir + '/accounts.cfg', 'w').write(
+
+    if use_keyring:
+        account_store('set', 'default', 'param-password',
+                'password_in_keyring')
+
+    low_prio_key_file_name = os.path.join(
+            os.environ['XDG_DATA_DIRS'].split(':')[0],
+            'telepathy', 'mission-control', 'accounts.cfg')
+    os.makedirs(os.path.dirname(low_prio_key_file_name), 0700)
+
+    # This is deliberately a lower-priority location
+    os.remove(new_key_file_name)
+    open(low_prio_key_file_name, 'w').write(
 r"""# Telepathy accounts
 [%s]
 manager=fakecm
@@ -233,12 +248,14 @@ DisplayName=New and improved account
     account = get_fakecm_account(bus, mc, account_path)
     account_iface = dbus.Interface(account, cs.ACCOUNT)
 
-    pwd = account_store('get', 'default', 'param-password')
+    # Files in lower-priority XDG locations aren't copied until something
+    # actually changes, and they aren't deleted.
+    assert not os.path.exists(new_key_file_name)
+    assert os.path.exists(low_prio_key_file_name)
+
     if use_keyring:
+        pwd = account_store('get', 'default', 'param-password')
         assertEquals('password_in_keyring', pwd)
-    else:
-        # it was overwritten when we edited the keyfile
-        assertEquals('password_in_keyfile', pwd)
 
     # Delete the password (only), like Empathy 3.0-3.4 do when migrating
     account_iface.UpdateParameters({}, ['password'])
@@ -253,16 +270,41 @@ DisplayName=New and improved account
     # Tell MC to die yet again
     tell_mc_to_die(q, bus)
 
-    # Check the password is correctly deleted
-    kf = keyfile_read(key_file_name)
+    # Check the account has copied (not moved! XDG_DATA_DIRS are,
+    # conceptually, read-only) from the old to the new name
+    assert not os.path.exists(old_key_file_name)
+    assert os.path.exists(low_prio_key_file_name)
+    kf = keyfile_read(new_key_file_name)
     assert 'param-password' not in kf[group]
     pwd = account_store('get', 'default', 'param-password')
     assertEquals(None, pwd)
     pwd = account_store('count-passwords', 'default')
     assertEquals('0', pwd)
 
-    # Put it back, just so deleting all accounts won't raise errors
+    # Write out an account configuration in the old keyfile, to test
+    # migration
+    os.remove(new_key_file_name)
+    os.remove(low_prio_key_file_name)
+    open(old_key_file_name, 'w').write(
+r"""# Telepathy accounts
+[%s]
+manager=fakecm
+protocol=fakeprotocol
+param-account=dontdivert@example.com
+DisplayName=Ye olde account
+""" % group)
+
     account_manager, properties, interfaces = resuscitate_mc(q, bus, mc)
+    account = get_fakecm_account(bus, mc, account_path)
+    account_iface = dbus.Interface(account, cs.ACCOUNT)
+
+    # This time it *does* get moved (really copied+deleted) automatically
+    # during MC startup
+    assert not os.path.exists(old_key_file_name)
+    assert not os.path.exists(low_prio_key_file_name)
+    kf = keyfile_read(new_key_file_name)
+    assert 'param-password' not in kf[group]
+    assertEquals('Ye olde account', kf[group]['DisplayName'])
 
 if __name__ == '__main__':
     ctl_dir = os.environ['MC_ACCOUNT_DIR']
diff --git a/tests/twisted/account-storage/diverted-storage.py b/tests/twisted/account-storage/diverted-storage.py
index 608ad4eb..258b0e2b 100644
--- a/tests/twisted/account-storage/diverted-storage.py
+++ b/tests/twisted/account-storage/diverted-storage.py
@@ -39,7 +39,8 @@ def test(q, bus, mc):
     except OSError:
         pass
 
-    empty_key_file_name = os.path.join(accounts_dir, 'accounts.cfg')
+    empty_key_file_name = os.path.join(os.environ['XDG_DATA_HOME'],
+            'telepathy', 'mission-control', 'accounts.cfg')
 
     group = 'fakecm/fakeprotocol/someguy_40example_2ecom0'