Blob Blame History Raw
From 1d87daa346e054a13e8e6d78986981b4532867f1 Mon Sep 17 00:00:00 2001
From: Debarshi Ray <debarshir@gnome.org>
Date: Fri, 22 Jan 2016 17:27:58 +0100
Subject: [PATCH 01/11] Use g_clear_pointer wherever applicable

---
 src/daemon/goadaemon.c                  | 7 ++-----
 src/daemon/main.c                       | 6 ++----
 src/goabackend/goaexchangeprovider.c    | 6 ++----
 src/goabackend/goaimapsmtpprovider.c    | 9 +++------
 src/goabackend/goakerberosprovider.c    | 8 ++------
 src/goabackend/goamediaserverprovider.c | 6 ++----
 src/goabackend/goaoauth2provider.c      | 3 +--
 src/goabackend/goaoauthprovider.c       | 9 +++------
 src/goabackend/goaowncloudprovider.c    | 6 ++----
 9 files changed, 19 insertions(+), 41 deletions(-)

diff --git a/src/daemon/goadaemon.c b/src/daemon/goadaemon.c
index 1a7b1fff65b5..12dadbbf54f2 100644
--- a/src/daemon/goadaemon.c
+++ b/src/daemon/goadaemon.c
@@ -934,9 +934,7 @@ get_all_providers_cb (GObject      *source,
   g_free (group);
   g_free (id);
   g_free (path);
-  if (key_file != NULL)
-    g_key_file_free (key_file);
-
+  g_clear_pointer (&key_file, (GDestroyNotify) g_key_file_free);
   g_object_unref (data->daemon);
   g_object_unref (data->manager);
   g_object_unref (data->invocation);
@@ -1087,8 +1085,7 @@ on_account_handle_remove (GoaAccount            *account,
  out:
   g_clear_object (&provider);
   g_free (data);
-  if (key_file != NULL)
-    g_key_file_free (key_file);
+  g_clear_pointer (&key_file, (GDestroyNotify) g_key_file_free);
   g_free (group);
   g_free (path);
   return TRUE; /* invocation was handled */
diff --git a/src/daemon/main.c b/src/daemon/main.c
index fd3242e94380..a04cc3992a83 100644
--- a/src/daemon/main.c
+++ b/src/daemon/main.c
@@ -135,10 +135,8 @@ main (int    argc,
     g_object_unref (tp_linker);
   if (name_owner_id != 0)
     g_bus_unown_name (name_owner_id);
-  if (loop != NULL)
-    g_main_loop_unref (loop);
-  if (opt_context != NULL)
-    g_option_context_free (opt_context);
+  g_clear_pointer (&loop, (GDestroyNotify) g_main_loop_unref);
+  g_clear_pointer (&opt_context, (GDestroyNotify) g_option_context_free);
 
   g_message ("goa-daemon version %s exiting", PACKAGE_VERSION);
 
diff --git a/src/goabackend/goaexchangeprovider.c b/src/goabackend/goaexchangeprovider.c
index 78191e3c5678..923fb6563f5d 100644
--- a/src/goabackend/goaexchangeprovider.c
+++ b/src/goabackend/goaexchangeprovider.c
@@ -754,8 +754,7 @@ add_account (GoaProvider    *provider,
     g_assert (ret != NULL);
 
   g_free (data.account_object_path);
-  if (data.loop != NULL)
-    g_main_loop_unref (data.loop);
+  g_clear_pointer (&data.loop, (GDestroyNotify) g_main_loop_unref);
   g_clear_object (&data.cancellable);
   if (ews_client != NULL)
     g_object_unref (ews_client);
@@ -909,8 +908,7 @@ refresh_account (GoaProvider    *provider,
     g_propagate_error (error, data.error);
 
   gtk_widget_destroy (dialog);
-  if (data.loop != NULL)
-    g_main_loop_unref (data.loop);
+  g_clear_pointer (&data.loop, (GDestroyNotify) g_main_loop_unref);
   g_clear_object (&data.cancellable);
   if (ews_client != NULL)
     g_object_unref (ews_client);
diff --git a/src/goabackend/goaimapsmtpprovider.c b/src/goabackend/goaimapsmtpprovider.c
index 9c609cbe440f..4e41a6449c43 100644
--- a/src/goabackend/goaimapsmtpprovider.c
+++ b/src/goabackend/goaimapsmtpprovider.c
@@ -489,8 +489,7 @@ ensure_credentials_sync (GoaProvider         *provider,
   g_free (smtp_password);
   g_free (smtp_server);
   g_free (smtp_username);
-  if (credentials != NULL)
-    g_variant_unref (credentials);
+  g_clear_pointer (&credentials, (GDestroyNotify) g_variant_unref);
   return ret;
 }
 
@@ -1203,8 +1202,7 @@ add_account (GoaProvider    *provider,
 
   g_free (domain);
   g_free (data.account_object_path);
-  if (data.loop != NULL)
-    g_main_loop_unref (data.loop);
+  g_clear_pointer (&data.loop, (GDestroyNotify) g_main_loop_unref);
   g_clear_object (&data.cancellable);
   g_clear_object (&imap_auth);
   g_clear_object (&smtp_auth);
@@ -1488,8 +1486,7 @@ refresh_account (GoaProvider    *provider,
   g_free (imap_username);
   g_free (smtp_server);
   g_free (smtp_username);
-  if (data.loop != NULL)
-    g_main_loop_unref (data.loop);
+  g_clear_pointer (&data.loop, (GDestroyNotify) g_main_loop_unref);
   g_clear_object (&data.cancellable);
   g_clear_object (&imap_auth);
   g_clear_object (&smtp_auth);
diff --git a/src/goabackend/goakerberosprovider.c b/src/goabackend/goakerberosprovider.c
index 69a1c9142466..a3a9bd9f5369 100644
--- a/src/goabackend/goakerberosprovider.c
+++ b/src/goabackend/goakerberosprovider.c
@@ -358,10 +358,7 @@ get_ticket_sync (GoaKerberosProvider *self,
 out:
   g_clear_object (&ticketing);
   g_free (object_path);
-
-  if (credentials != NULL)
-    g_variant_unref (credentials);
-
+  g_clear_pointer (&credentials, (GDestroyNotify) g_variant_unref);
   return ret;
 }
 
@@ -1315,8 +1312,7 @@ start_over:
   g_free (request.account_object_path);
   g_free (principal);
   g_free (principal_for_display);
-  if (request.loop != NULL)
-    g_main_loop_unref (request.loop);
+  g_clear_pointer (&request.loop, (GDestroyNotify) g_main_loop_unref);
   return object;
 }
 
diff --git a/src/goabackend/goamediaserverprovider.c b/src/goabackend/goamediaserverprovider.c
index eded24c256bc..860fe1588043 100644
--- a/src/goabackend/goamediaserverprovider.c
+++ b/src/goabackend/goamediaserverprovider.c
@@ -216,8 +216,7 @@ ensure_credentials_sync (GoaProvider         *provider,
   ret = TRUE;
 
  out:
-  if (credentials != NULL)
-    g_variant_unref (credentials);
+  g_clear_pointer (&credentials, (GDestroyNotify) g_variant_unref);
   return ret;
 }
 
@@ -529,8 +528,7 @@ add_account (GoaProvider  *provider,
   g_free (data.account_object_path);
   g_free (data.friendly_name);
   g_free (data.udn);
-  if (data.loop != NULL)
-    g_main_loop_unref (data.loop);
+  g_clear_pointer (&data.loop, (GDestroyNotify) g_main_loop_unref);
   return ret;
 }
 
diff --git a/src/goabackend/goaoauth2provider.c b/src/goabackend/goaoauth2provider.c
index 2980c470384d..4e6f185c5c32 100644
--- a/src/goabackend/goaoauth2provider.c
+++ b/src/goabackend/goaoauth2provider.c
@@ -1592,8 +1592,7 @@ goa_oauth2_provider_get_access_token_sync (GoaOAuth2Provider  *provider,
   g_free (refresh_token);
   g_free (old_refresh_token);
   g_free (password);
-  if (credentials != NULL)
-    g_variant_unref (credentials);
+  g_clear_pointer (&credentials, (GDestroyNotify) g_variant_unref);
 
   g_mutex_unlock (lock);
 
diff --git a/src/goabackend/goaoauthprovider.c b/src/goabackend/goaoauthprovider.c
index 6225affbf754..2cd9b472e067 100644
--- a/src/goabackend/goaoauthprovider.c
+++ b/src/goabackend/goaoauthprovider.c
@@ -991,8 +991,7 @@ get_tokens_and_identity (GoaOAuthProvider *provider,
   g_free (url);
 
   g_free (data.oauth_verifier);
-  if (data.loop != NULL)
-    g_main_loop_unref (data.loop);
+  g_clear_pointer (&data.loop, (GDestroyNotify) g_main_loop_unref);
   g_free (data.access_token);
   g_free (data.access_token_secret);
   g_free (escaped_request_token);
@@ -1170,8 +1169,7 @@ goa_oauth_provider_add_account (GoaProvider *_provider,
   g_free (access_token_secret);
   g_free (session_handle);
   g_free (data.account_object_path);
-  if (data.loop != NULL)
-    g_main_loop_unref (data.loop);
+  g_clear_pointer (&data.loop, (GDestroyNotify) g_main_loop_unref);
   return ret;
 }
 
@@ -1544,8 +1542,7 @@ goa_oauth_provider_get_access_token_sync (GoaOAuthProvider   *provider,
   g_free (access_token_secret_for_refresh);
   g_free (session_handle_for_refresh);
   g_free (password);
-  if (credentials != NULL)
-    g_variant_unref (credentials);
+  g_clear_pointer (&credentials, (GDestroyNotify) g_variant_unref);
 
   g_mutex_unlock (lock);
 
diff --git a/src/goabackend/goaowncloudprovider.c b/src/goabackend/goaowncloudprovider.c
index 5b408adeb397..e202e05770b9 100644
--- a/src/goabackend/goaowncloudprovider.c
+++ b/src/goabackend/goaowncloudprovider.c
@@ -919,8 +919,7 @@ add_account (GoaProvider    *provider,
   g_free (server);
   g_free (uri);
   g_free (data.account_object_path);
-  if (data.loop != NULL)
-    g_main_loop_unref (data.loop);
+  g_clear_pointer (&data.loop, (GDestroyNotify) g_main_loop_unref);
   g_clear_object (&data.cancellable);
   g_clear_object (&http_client);
   return ret;
@@ -1076,8 +1075,7 @@ refresh_account (GoaProvider    *provider,
   gtk_widget_destroy (dialog);
   g_free (uri);
   g_free (uri_webdav);
-  if (data.loop != NULL)
-    g_main_loop_unref (data.loop);
+  g_clear_pointer (&data.loop, (GDestroyNotify) g_main_loop_unref);
   g_clear_object (&data.cancellable);
   g_clear_object (&http_client);
   return ret;
-- 
2.5.5


From 2fa0c6a91224c68411c67a69dfaa15a5e50a4848 Mon Sep 17 00:00:00 2001
From: Debarshi Ray <debarshir@gnome.org>
Date: Wed, 4 May 2016 16:59:28 +0200
Subject: [PATCH 02/11] daemon, utils: Simplify saving a GKeyFile to a path

We can now use g_key_file_save_to_file which wraps g_key_file_to_data
and g_file_set_contents.

At the moment, g_key_file_to_data is documented as never throwing an
error. So we don't lose anything in terms of error reporting. Even if
this changes in the future, I think we can live without a separate
error message for g_key_file_to_data.

Bump minimum GLib version to 2.40.

https://bugzilla.gnome.org/show_bug.cgi?id=688041
---
 configure.ac              |  2 +-
 src/daemon/goadaemon.c    | 37 ++-----------------------------------
 src/goabackend/goautils.c | 24 +++---------------------
 3 files changed, 6 insertions(+), 57 deletions(-)

diff --git a/configure.ac b/configure.ac
index a76ce9770d96..e6c78588394e 100644
--- a/configure.ac
+++ b/configure.ac
@@ -82,7 +82,7 @@ GTK_DOC_CHECK([1.3])
 # Libraries
 #
 
-PKG_CHECK_MODULES(GLIB, [glib-2.0 gio-2.0 gio-unix-2.0 >= 2.35])
+PKG_CHECK_MODULES(GLIB, [glib-2.0 gio-2.0 gio-unix-2.0 >= 2.40])
 AC_SUBST(GLIB_CFLAGS)
 AC_SUBST(GLIB_LIBS)
 
diff --git a/src/daemon/goadaemon.c b/src/daemon/goadaemon.c
index 12dadbbf54f2..67f43151ea7c 100644
--- a/src/daemon/goadaemon.c
+++ b/src/daemon/goadaemon.c
@@ -890,23 +890,8 @@ get_all_providers_cb (GObject      *source,
       g_key_file_set_string (key_file, group, key, value);
     }
 
-  g_free (key_file_data);
-  error = NULL;
-  key_file_data = g_key_file_to_data (key_file,
-                                      &length,
-                                      &error);
-  if (key_file_data == NULL)
-    {
-      g_prefix_error (&error, "Error generating key-value-file: ");
-      g_dbus_method_invocation_take_error (data->invocation, error);
-      goto out;
-    }
-
   error = NULL;
-  if (!g_file_set_contents (path,
-                            key_file_data,
-                            length,
-                            &error))
+  if (!g_key_file_save_to_file (key_file, path, &error))
     {
       g_prefix_error (&error, "Error writing key-value-file %s: ", path);
       g_dbus_method_invocation_take_error (data->invocation, error);
@@ -987,8 +972,6 @@ on_account_handle_remove (GoaAccount            *account,
   const gchar *provider_type;
   gchar *path;
   gchar *group;
-  gchar *data;
-  gsize length;
   GError *error;
 
   provider = NULL;
@@ -996,7 +979,6 @@ on_account_handle_remove (GoaAccount            *account,
   path = NULL;
   group = NULL;
   key_file = NULL;
-  data = NULL;
 
   /* update key-file - right now we only support removing the account
    * if the entry is in ~/.config/goa-1.0/accounts.conf
@@ -1025,21 +1007,7 @@ on_account_handle_remove (GoaAccount            *account,
     }
 
   error = NULL;
-  data = g_key_file_to_data (key_file,
-                             &length,
-                             &error);
-  if (data == NULL)
-    {
-      g_prefix_error (&error, "Error generating key-value-file: ");
-      g_dbus_method_invocation_take_error (invocation, error);
-      goto out;
-    }
-
-  error = NULL;
-  if (!g_file_set_contents (path,
-                            data,
-                            length,
-                            &error))
+  if (!g_key_file_save_to_file (key_file, path, &error))
     {
       g_prefix_error (&error, "Error writing key-value-file %s: ", path);
       g_dbus_method_invocation_take_error (invocation, error);
@@ -1084,7 +1052,6 @@ on_account_handle_remove (GoaAccount            *account,
 
  out:
   g_clear_object (&provider);
-  g_free (data);
   g_clear_pointer (&key_file, (GDestroyNotify) g_key_file_free);
   g_free (group);
   g_free (path);
diff --git a/src/goabackend/goautils.c b/src/goabackend/goautils.c
index deb8236d9e35..6dbd53a06469 100644
--- a/src/goabackend/goautils.c
+++ b/src/goabackend/goautils.c
@@ -380,12 +380,8 @@ goa_utils_keyfile_remove_key (GoaAccount *account, const gchar *key)
 {
   GError *error;
   GKeyFile *key_file;
-  gchar *contents;
   gchar *group;
   gchar *path;
-  gsize length;
-
-  contents = NULL;
 
   path = g_strdup_printf ("%s/goa-1.0/accounts.conf", g_get_user_config_dir ());
   group = g_strdup_printf ("Account %s", goa_account_get_id (account));
@@ -407,10 +403,9 @@ goa_utils_keyfile_remove_key (GoaAccount *account, const gchar *key)
     }
 
   g_key_file_remove_key (key_file, group, key, NULL);
-  contents = g_key_file_to_data (key_file, &length, NULL);
 
   error = NULL;
-  if (!g_file_set_contents (path, contents, length, &error))
+  if (!g_key_file_save_to_file (key_file, path, &error))
     {
       g_prefix_error (&error, "Error writing key-value-file %s: ", path);
       g_warning ("%s (%s, %d)", error->message, g_quark_to_string (error->domain), error->code);
@@ -419,7 +414,6 @@ goa_utils_keyfile_remove_key (GoaAccount *account, const gchar *key)
     }
 
  out:
-  g_free (contents);
   g_key_file_free (key_file);
   g_free (group);
   g_free (path);
@@ -430,12 +424,8 @@ goa_utils_keyfile_set_boolean (GoaAccount *account, const gchar *key, gboolean v
 {
   GError *error;
   GKeyFile *key_file;
-  gchar *contents;
   gchar *group;
   gchar *path;
-  gsize length;
-
-  contents = NULL;
 
   path = g_strdup_printf ("%s/goa-1.0/accounts.conf", g_get_user_config_dir ());
   group = g_strdup_printf ("Account %s", goa_account_get_id (account));
@@ -457,10 +447,9 @@ goa_utils_keyfile_set_boolean (GoaAccount *account, const gchar *key, gboolean v
     }
 
   g_key_file_set_boolean (key_file, group, key, value);
-  contents = g_key_file_to_data (key_file, &length, NULL);
 
   error = NULL;
-  if (!g_file_set_contents (path, contents, length, &error))
+  if (!g_key_file_save_to_file (key_file, path, &error))
     {
       g_prefix_error (&error, "Error writing key-value-file %s: ", path);
       g_warning ("%s (%s, %d)", error->message, g_quark_to_string (error->domain), error->code);
@@ -469,7 +458,6 @@ goa_utils_keyfile_set_boolean (GoaAccount *account, const gchar *key, gboolean v
     }
 
  out:
-  g_free (contents);
   g_key_file_free (key_file);
   g_free (group);
   g_free (path);
@@ -480,12 +468,8 @@ goa_utils_keyfile_set_string (GoaAccount *account, const gchar *key, const gchar
 {
   GError *error;
   GKeyFile *key_file;
-  gchar *contents;
   gchar *group;
   gchar *path;
-  gsize length;
-
-  contents = NULL;
 
   path = g_strdup_printf ("%s/goa-1.0/accounts.conf", g_get_user_config_dir ());
   group = g_strdup_printf ("Account %s", goa_account_get_id (account));
@@ -507,10 +491,9 @@ goa_utils_keyfile_set_string (GoaAccount *account, const gchar *key, const gchar
     }
 
   g_key_file_set_string (key_file, group, key, value);
-  contents = g_key_file_to_data (key_file, &length, NULL);
 
   error = NULL;
-  if (!g_file_set_contents (path, contents, length, &error))
+  if (!g_key_file_save_to_file (key_file, path, &error))
     {
       g_prefix_error (&error, "Error writing key-value-file %s: ", path);
       g_warning ("%s (%s, %d)", error->message, g_quark_to_string (error->domain), error->code);
@@ -519,7 +502,6 @@ goa_utils_keyfile_set_string (GoaAccount *account, const gchar *key, const gchar
     }
 
  out:
-  g_free (contents);
   g_key_file_free (key_file);
   g_free (group);
   g_free (path);
-- 
2.5.5


From ca9a49620b98455bdbef5b813cbf607bfec1759b Mon Sep 17 00:00:00 2001
From: Debarshi Ray <debarshir@gnome.org>
Date: Wed, 4 May 2016 18:47:43 +0200
Subject: [PATCH 03/11] media-server: Don't touch the keyring

Media servers (eg., DLNA) don't need any credentials, therefore don't
bother storing and looking up an empty entry in the keyring.

https://bugzilla.gnome.org/show_bug.cgi?id=688041
---
 src/goabackend/goamediaserverprovider.c | 44 ++-------------------------------
 1 file changed, 2 insertions(+), 42 deletions(-)

diff --git a/src/goabackend/goamediaserverprovider.c b/src/goabackend/goamediaserverprovider.c
index 860fe1588043..c3b90b46b498 100644
--- a/src/goabackend/goamediaserverprovider.c
+++ b/src/goabackend/goamediaserverprovider.c
@@ -189,35 +189,10 @@ ensure_credentials_sync (GoaProvider         *provider,
                          GCancellable        *cancellable,
                          GError             **error)
 {
-  GVariant *credentials;
-  gboolean ret;
-
-  credentials = NULL;
-  ret = FALSE;
-
-  credentials = goa_utils_lookup_credentials_sync (provider,
-                                                   object,
-                                                   cancellable,
-                                                   error);
-
-  if (credentials == NULL)
-    {
-      if (error != NULL)
-        {
-          (*error)->domain = GOA_ERROR;
-          (*error)->code = GOA_ERROR_NOT_AUTHORIZED;
-          goto out;
-        }
-    }
-
   if (out_expires_in != NULL)
     *out_expires_in = 0;
 
-  ret = TRUE;
-
- out:
-  g_clear_pointer (&credentials, (GDestroyNotify) g_variant_unref);
-  return ret;
+  return TRUE;
 }
 
 /* ---------------------------------------------------------------------------------------------------- */
@@ -542,28 +517,13 @@ refresh_account (GoaProvider  *provider,
                  GError      **error)
 {
   GoaAccount *account;
-  GVariantBuilder credentials;
-  gboolean ret;
-
-  ret = FALSE;
-
-  g_variant_builder_init (&credentials, G_VARIANT_TYPE_VARDICT);
-
-  if (!goa_utils_store_credentials_for_object_sync (provider,
-                                                    object,
-                                                    g_variant_builder_end (&credentials),
-                                                    NULL, /* GCancellable */
-                                                    error))
-    goto out;
 
   account = goa_object_peek_account (object);
   goa_account_call_ensure_credentials (account,
                                        NULL, /* GCancellable */
                                        NULL, NULL); /* callback, user_data */
-  ret = TRUE;
 
- out:
-  return ret;
+  return TRUE;
 }
 
 /* ---------------------------------------------------------------------------------------------------- */
-- 
2.5.5


From ae365bdc72ad0d95d6c9db019e6608e9cae46ba0 Mon Sep 17 00:00:00 2001
From: Debarshi Ray <debarshir@gnome.org>
Date: Fri, 6 May 2016 18:50:41 +0200
Subject: [PATCH 04/11] daemon: Move object_path_to_group to its own area

In a subsequent commit, we are going to add one more similar utility
function that will be used in multiple places. Therefore, let's club
them together and give them a separate home.

https://bugzilla.gnome.org/show_bug.cgi?id=688041
---
 src/daemon/goadaemon.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/src/daemon/goadaemon.c b/src/daemon/goadaemon.c
index 67f43151ea7c..d799a09db669 100644
--- a/src/daemon/goadaemon.c
+++ b/src/daemon/goadaemon.c
@@ -350,6 +350,15 @@ diff_sorted_lists (GList *list1,
 
 /* ---------------------------------------------------------------------------------------------------- */
 
+static gchar *
+object_path_to_group (const gchar *object_path)
+{
+  g_return_val_if_fail (g_str_has_prefix (object_path, "/org/gnome/OnlineAccounts/Accounts/"), NULL);
+  return g_strdup_printf ("Account %s", object_path + sizeof "/org/gnome/OnlineAccounts/Accounts/" - 1);
+}
+
+/* ---------------------------------------------------------------------------------------------------- */
+
 typedef struct
 {
   GKeyFile *key_file;
@@ -558,13 +567,6 @@ update_account_object (GoaDaemon           *self,
   return ret;
 }
 
-static gchar *
-object_path_to_group (const gchar *object_path)
-{
-  g_return_val_if_fail (g_str_has_prefix (object_path, "/org/gnome/OnlineAccounts/Accounts/"), NULL);
-  return g_strdup_printf ("Account %s", object_path + sizeof "/org/gnome/OnlineAccounts/Accounts/" - 1);
-}
-
 static void
 process_config_entries (GoaDaemon  *self,
                         GHashTable *group_name_to_key_file_data)
-- 
2.5.5


From 345e5c4e62632560bc767aad3d3904ad8379ef1f Mon Sep 17 00:00:00 2001
From: Debarshi Ray <debarshir@gnome.org>
Date: Fri, 6 May 2016 18:53:11 +0200
Subject: [PATCH 05/11] daemon: Rename the variable 'id' to 'group'

It refers to the name of a group in the key file, not an ID.

https://bugzilla.gnome.org/show_bug.cgi?id=688041
---
 src/daemon/goadaemon.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/src/daemon/goadaemon.c b/src/daemon/goadaemon.c
index d799a09db669..450bda7406e6 100644
--- a/src/daemon/goadaemon.c
+++ b/src/daemon/goadaemon.c
@@ -572,7 +572,7 @@ process_config_entries (GoaDaemon  *self,
                         GHashTable *group_name_to_key_file_data)
 {
   GHashTableIter iter;
-  const gchar *id;
+  const gchar *group;
   KeyFileData *key_file_data;
   GList *existing_object_paths;
   GList *config_object_paths;
@@ -598,15 +598,15 @@ process_config_entries (GoaDaemon  *self,
 
   config_object_paths = NULL;
   g_hash_table_iter_init (&iter, group_name_to_key_file_data);
-  while (g_hash_table_iter_next (&iter, (gpointer*) &id, (gpointer*) &key_file_data))
+  while (g_hash_table_iter_next (&iter, (gpointer*) &group, (gpointer*) &key_file_data))
     {
       gchar *object_path;
 
       /* create and validate object path */
-      object_path = g_strdup_printf ("/org/gnome/OnlineAccounts/Accounts/%s", id + sizeof "Account " - 1);
-      if (strstr (id + sizeof "Account " - 1, "/") != NULL || !g_variant_is_object_path (object_path))
+      object_path = g_strdup_printf ("/org/gnome/OnlineAccounts/Accounts/%s", group + sizeof "Account " - 1);
+      if (strstr (group + sizeof "Account " - 1, "/") != NULL || !g_variant_is_object_path (object_path))
         {
-          g_warning ("`%s' is not a valid account identifier", id);
+          g_warning ("`%s' is not a valid account identifier", group);
           g_free (object_path);
           continue;
         }
-- 
2.5.5


From 63d0c7f7e64ef7d68df30a7f63c397651df346e9 Mon Sep 17 00:00:00 2001
From: Debarshi Ray <debarshir@gnome.org>
Date: Fri, 6 May 2016 19:00:32 +0200
Subject: [PATCH 06/11] daemon: Refactor code to get ID from group name into a
 function

https://bugzilla.gnome.org/show_bug.cgi?id=688041
---
 src/daemon/goadaemon.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/src/daemon/goadaemon.c b/src/daemon/goadaemon.c
index 450bda7406e6..6335c6708718 100644
--- a/src/daemon/goadaemon.c
+++ b/src/daemon/goadaemon.c
@@ -350,6 +350,13 @@ diff_sorted_lists (GList *list1,
 
 /* ---------------------------------------------------------------------------------------------------- */
 
+static const gchar *
+group_to_id (const gchar *group)
+{
+  g_return_val_if_fail (g_str_has_prefix (group, "Account "), NULL);
+  return group + sizeof "Account " - 1;
+}
+
 static gchar *
 object_path_to_group (const gchar *object_path)
 {
@@ -600,11 +607,14 @@ process_config_entries (GoaDaemon  *self,
   g_hash_table_iter_init (&iter, group_name_to_key_file_data);
   while (g_hash_table_iter_next (&iter, (gpointer*) &group, (gpointer*) &key_file_data))
     {
+      const gchar *id;
       gchar *object_path;
 
+      id = group_to_id (group);
+
       /* create and validate object path */
-      object_path = g_strdup_printf ("/org/gnome/OnlineAccounts/Accounts/%s", group + sizeof "Account " - 1);
-      if (strstr (group + sizeof "Account " - 1, "/") != NULL || !g_variant_is_object_path (object_path))
+      object_path = g_strdup_printf ("/org/gnome/OnlineAccounts/Accounts/%s", id);
+      if (strstr (id, "/") != NULL || !g_variant_is_object_path (object_path))
         {
           g_warning ("`%s' is not a valid account identifier", group);
           g_free (object_path);
-- 
2.5.5


From e7a111758703a55c10193db962b7fea0467c10d1 Mon Sep 17 00:00:00 2001
From: Debarshi Ray <debarshir@gnome.org>
Date: Fri, 6 May 2016 19:04:35 +0200
Subject: [PATCH 07/11] daemon, utils: Rename goa_utils_delete_credentials_sync

... because we are going to add a for_id variant in a subsequent
commit.

https://bugzilla.gnome.org/show_bug.cgi?id=688041
---
 src/daemon/goadaemon.c    | 2 +-
 src/goabackend/goautils.c | 8 ++++----
 src/goabackend/goautils.h | 8 ++++----
 3 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/src/daemon/goadaemon.c b/src/daemon/goadaemon.c
index 6335c6708718..1d04e02fe3a2 100644
--- a/src/daemon/goadaemon.c
+++ b/src/daemon/goadaemon.c
@@ -1052,7 +1052,7 @@ on_account_handle_remove (GoaAccount            *account,
     }
 
   error = NULL;
-  if (!goa_utils_delete_credentials_sync (provider, account, NULL, &error))
+  if (!goa_utils_delete_credentials_for_account_sync (provider, account, NULL, &error))
     {
       g_dbus_method_invocation_take_error (invocation, error);
       goto out;
diff --git a/src/goabackend/goautils.c b/src/goabackend/goautils.c
index 6dbd53a06469..c58ccb82c5d3 100644
--- a/src/goabackend/goautils.c
+++ b/src/goabackend/goautils.c
@@ -169,10 +169,10 @@ goa_utils_set_dialog_title (GoaProvider *provider, GtkDialog *dialog, gboolean a
 }
 
 gboolean
-goa_utils_delete_credentials_sync (GoaProvider   *provider,
-                                   GoaAccount    *object,
-                                   GCancellable  *cancellable,
-                                   GError       **error)
+goa_utils_delete_credentials_for_account_sync (GoaProvider   *provider,
+                                               GoaAccount    *object,
+                                               GCancellable  *cancellable,
+                                               GError       **error)
 {
   gboolean ret;
   gchar *password_key;
diff --git a/src/goabackend/goautils.h b/src/goabackend/goautils.h
index ca3671bcbc9a..baf475548d8c 100644
--- a/src/goabackend/goautils.h
+++ b/src/goabackend/goautils.h
@@ -54,10 +54,10 @@ gchar           *goa_utils_data_input_stream_read_line (GDataInputStream  *strea
 
 void             goa_utils_set_dialog_title (GoaProvider *provider, GtkDialog *dialog, gboolean add_account);
 
-gboolean         goa_utils_delete_credentials_sync (GoaProvider    *provider,
-                                                    GoaAccount     *account,
-                                                    GCancellable   *cancellable,
-                                                    GError        **error);
+gboolean         goa_utils_delete_credentials_for_account_sync (GoaProvider    *provider,
+                                                                GoaAccount     *account,
+                                                                GCancellable   *cancellable,
+                                                                GError        **error);
 
 GVariant        *goa_utils_lookup_credentials_sync (GoaProvider    *provider,
                                                     GoaObject      *object,
-- 
2.5.5


From 9bbf94ba603903565d09e3c2a832d349d412d05b Mon Sep 17 00:00:00 2001
From: Debarshi Ray <debarshir@gnome.org>
Date: Fri, 6 May 2016 19:11:02 +0200
Subject: [PATCH 08/11] utils: Add goa_utils_delete_credentials_for_id_sync
 helper

https://bugzilla.gnome.org/show_bug.cgi?id=688041
---
 src/goabackend/goautils.c | 22 ++++++++++++++++++----
 src/goabackend/goautils.h |  5 +++++
 2 files changed, 23 insertions(+), 4 deletions(-)

diff --git a/src/goabackend/goautils.c b/src/goabackend/goautils.c
index c58ccb82c5d3..553adf36df21 100644
--- a/src/goabackend/goautils.c
+++ b/src/goabackend/goautils.c
@@ -174,13 +174,29 @@ goa_utils_delete_credentials_for_account_sync (GoaProvider   *provider,
                                                GCancellable  *cancellable,
                                                GError       **error)
 {
+  const gchar *id;
+
+  g_return_val_if_fail (GOA_IS_PROVIDER (provider), FALSE);
+  g_return_val_if_fail (GOA_IS_ACCOUNT (object), FALSE);
+  g_return_val_if_fail (cancellable == NULL || G_IS_CANCELLABLE (cancellable), FALSE);
+  g_return_val_if_fail (error == NULL || *error == NULL, FALSE);
+
+  id = goa_account_get_id (object);
+  return goa_utils_delete_credentials_for_id_sync (provider, id, cancellable, error);
+}
+
+gboolean
+goa_utils_delete_credentials_for_id_sync (GoaProvider   *provider,
+                                          const gchar   *id,
+                                          GCancellable  *cancellable,
+                                          GError       **error)
+{
   gboolean ret;
   gchar *password_key;
-  const gchar *id;
   GError *sec_error = NULL;
 
   g_return_val_if_fail (GOA_IS_PROVIDER (provider), FALSE);
-  g_return_val_if_fail (GOA_IS_ACCOUNT (object), FALSE);
+  g_return_val_if_fail (id != NULL && id[0] != '\0', FALSE);
   g_return_val_if_fail (cancellable == NULL || G_IS_CANCELLABLE (cancellable), FALSE);
   g_return_val_if_fail (error == NULL || *error == NULL, FALSE);
 
@@ -188,8 +204,6 @@ goa_utils_delete_credentials_for_account_sync (GoaProvider   *provider,
 
   password_key = NULL;
 
-  id = goa_account_get_id (object);
-
   password_key = g_strdup_printf ("%s:gen%d:%s",
                                   goa_provider_get_provider_type (GOA_PROVIDER (provider)),
                                   goa_provider_get_credentials_generation (GOA_PROVIDER (provider)),
diff --git a/src/goabackend/goautils.h b/src/goabackend/goautils.h
index baf475548d8c..07a3c70353f9 100644
--- a/src/goabackend/goautils.h
+++ b/src/goabackend/goautils.h
@@ -59,6 +59,11 @@ gboolean         goa_utils_delete_credentials_for_account_sync (GoaProvider    *
                                                                 GCancellable   *cancellable,
                                                                 GError        **error);
 
+gboolean         goa_utils_delete_credentials_for_id_sync (GoaProvider    *provider,
+                                                           const gchar    *id,
+                                                           GCancellable   *cancellable,
+                                                           GError        **error);
+
 GVariant        *goa_utils_lookup_credentials_sync (GoaProvider    *provider,
                                                     GoaObject      *object,
                                                     GCancellable   *cancellable,
-- 
2.5.5


From f022f0d737ee9d9ad6fd345fc458c3d49667f44e Mon Sep 17 00:00:00 2001
From: Debarshi Ray <debarshir@gnome.org>
Date: Thu, 5 May 2016 17:37:17 +0200
Subject: [PATCH 09/11] daemon: Clean up temporary accounts (eg., Kerberos)

... by removing them from both accounts.conf and the keyring.

https://bugzilla.gnome.org/show_bug.cgi?id=688041
---
 src/daemon/goadaemon.c | 49 ++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 48 insertions(+), 1 deletion(-)

diff --git a/src/daemon/goadaemon.c b/src/daemon/goadaemon.c
index 1d04e02fe3a2..3f3fd47e9b62 100644
--- a/src/daemon/goadaemon.c
+++ b/src/daemon/goadaemon.c
@@ -421,6 +421,7 @@ add_config_file (GoaDaemon     *self,
     }
   else
     {
+      gboolean needs_update = FALSE;
       gchar **groups;
       const char *guid;
       gsize num_groups;
@@ -451,9 +452,44 @@ add_config_file (GoaDaemon     *self,
                   if (session_id != NULL &&
                       g_strcmp0 (session_id, guid) != 0)
                     {
+                      GoaProvider *provider = NULL;
+                      const gchar *id;
+                      gchar *provider_type = NULL;
+
                       g_debug ("ignoring account \"%s\" in file %s because it's stale",
                                groups[n], path);
+
+                      needs_update = g_key_file_remove_group (key_file, groups[n], NULL);
+
+                      id = group_to_id (groups[n]);
+                      if (id == NULL)
+                        {
+                          g_warning ("Unable to get account ID from group: %s", groups[n]);
+                          goto cleanup_and_continue;
+                        }
+
+                      provider_type = g_key_file_get_string (key_file, groups[n], "Provider", NULL);
+                      if (provider_type != NULL)
+                        provider = goa_provider_get_for_provider_type (provider_type);
+
+                      if (provider == NULL)
+                        {
+                          g_warning ("Unsupported account type %s for ID %s (no provider)", provider_type, id);
+                          goto cleanup_and_continue;
+                        }
+
+                      error = NULL;
+                      if (!goa_utils_delete_credentials_for_id_sync (provider, id, NULL, &error))
+                        {
+                          g_warning ("Unable to clean-up stale keyring entries: %s", error->message);
+                          g_error_free (error);
+                          goto cleanup_and_continue;
+                        }
+
+                    cleanup_and_continue:
+                      g_clear_object (&provider);
                       g_free (groups[n]);
+                      g_free (provider_type);
                       g_free (session_id);
                       continue;
                     }
@@ -461,7 +497,7 @@ add_config_file (GoaDaemon     *self,
                 }
               else
                 {
-                  g_key_file_remove_key (key_file, groups[n], "SessionId", NULL);
+                  needs_update = g_key_file_remove_key (key_file, groups[n], "SessionId", NULL);
                 }
 
               g_hash_table_insert (group_name_to_key_file_data,
@@ -476,6 +512,17 @@ add_config_file (GoaDaemon     *self,
         }
       g_free (groups);
 
+      if (needs_update)
+        {
+          error = NULL;
+          if (!g_key_file_save_to_file (key_file, path, &error))
+            {
+              g_prefix_error (&error, "Error writing key-value-file %s: ", path);
+              g_warning ("%s (%s, %d)", error->message, g_quark_to_string (error->domain), error->code);
+              g_error_free (error);
+            }
+        }
+
       *key_files_to_free = g_list_prepend (*key_files_to_free, key_file);
     }
 }
-- 
2.5.5


From 306d78cf1014797967c20c3959d4c8b122cb5574 Mon Sep 17 00:00:00 2001
From: Debarshi Ray <debarshir@gnome.org>
Date: Thu, 5 May 2016 17:43:35 +0200
Subject: [PATCH 10/11] daemon: Don't store empty entries in the keyring

If the account is being created without any credentials, then we assume
that the backend either (a) doesn't need any credentials (eg., media
servers), or (b) has other means of differentiating between accounts
that need credentials and those that do not (eg., Kerberos).

https://bugzilla.gnome.org/show_bug.cgi?id=688041
---
 src/daemon/goadaemon.c | 21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/src/daemon/goadaemon.c b/src/daemon/goadaemon.c
index 3f3fd47e9b62..213f154c12aa 100644
--- a/src/daemon/goadaemon.c
+++ b/src/daemon/goadaemon.c
@@ -844,6 +844,7 @@ get_all_providers_cb (GObject      *source,
   gchar *group;
   gchar *key_file_data;
   gsize length;
+  gsize n_credentials;
   gchar *object_path;
   GVariantIter iter;
   const gchar *key;
@@ -957,14 +958,18 @@ get_all_providers_cb (GObject      *source,
       goto out;
     }
 
-  /* We don't want to fail AddAccount if we could not store the
-   * credentials in the keyring.
-   */
-  goa_utils_store_credentials_for_id_sync (provider,
-                                           id,
-                                           data->credentials,
-                                           NULL, /* GCancellable */
-                                           NULL);
+  n_credentials = g_variant_n_children (data->credentials);
+  if (n_credentials > 0)
+    {
+      /* We don't want to fail AddAccount if we could not store the
+       * credentials in the keyring.
+       */
+      goa_utils_store_credentials_for_id_sync (provider,
+                                               id,
+                                               data->credentials,
+                                               NULL, /* GCancellable */
+                                               NULL);
+    }
 
   goa_daemon_reload_configuration (data->daemon);
 
-- 
2.5.5


From 2c93ab96ad922d380f0c2e4808e21d2864e97129 Mon Sep 17 00:00:00 2001
From: Debarshi Ray <debarshir@gnome.org>
Date: Mon, 18 Jul 2016 15:01:06 +0200
Subject: [PATCH 11/11] daemon: Ensure temporary accounts are really removed
 from the keyring

We should remove the GKeyFile group only after we have read everything
that we need from it. Otherwise, it will lead to:
  goa-daemon-WARNING **: Unsupported account type (null) for ID
    account_1466692958_0 (no provider)

Fall out from 56c056df2b5e3bc7f44dbc0915fa431db7628d91

https://bugzilla.gnome.org/show_bug.cgi?id=688041
---
 src/daemon/goadaemon.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/daemon/goadaemon.c b/src/daemon/goadaemon.c
index 213f154c12aa..04cb2403432a 100644
--- a/src/daemon/goadaemon.c
+++ b/src/daemon/goadaemon.c
@@ -459,8 +459,6 @@ add_config_file (GoaDaemon     *self,
                       g_debug ("ignoring account \"%s\" in file %s because it's stale",
                                groups[n], path);
 
-                      needs_update = g_key_file_remove_group (key_file, groups[n], NULL);
-
                       id = group_to_id (groups[n]);
                       if (id == NULL)
                         {
@@ -478,6 +476,8 @@ add_config_file (GoaDaemon     *self,
                           goto cleanup_and_continue;
                         }
 
+                      needs_update = g_key_file_remove_group (key_file, groups[n], NULL);
+
                       error = NULL;
                       if (!goa_utils_delete_credentials_for_id_sync (provider, id, NULL, &error))
                         {
-- 
2.5.5