Default account backend: when deleting, always delete from both places

Our tracking of whether something is "secret" (in the gnome-keyring) is
pretty shaky, and in particular, we can forget that things are meant
to be "secret" sometimes (we lose that information when deleting
parameters).

Happily, when we're deleting things, it doesn't actually matter: the
right thing to do is clearly to delete from both locations, regardless
of where we think it ought to be.

Similarly, when we're setting a property to a new value, it's appropriate
to delete it from both locations, then put it in the right location
(only).

Signed-off-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
Reviewed-by: Jonny Lamb <jonny.lamb@collabora.co.uk>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=42088

1 files changed, 7 insertions, 8 deletions

diff --git a/src/mcd-account-manager-default.c b/src/mcd-account-manager-default.c
index 4c474c50..1d901424 100644
--- a/src/mcd-account-manager-default.c
+++ b/src/mcd-account-manager-default.c
@@ -411,6 +411,10 @@ _set (const McpAccountStorage *self,
   /* if we have a keyring, secrets are segregated */
   secret = mcp_account_manager_parameter_is_secret (am, account, key);
 
+  /* remove it from both sets, then re-add it to the right one if non-null */
+  g_key_file_remove_key (amd->secrets, account, key, NULL);
+  g_key_file_remove_key (amd->keyfile, account, key, NULL);
+
   if (val != NULL)
     {
       if (secret)
@@ -418,13 +422,6 @@ _set (const McpAccountStorage *self,
       else
         g_key_file_set_value (amd->keyfile, account, key, val);
     }
-  else
-    {
-      if (secret)
-        g_key_file_remove_key (amd->secrets, account, key, NULL);
-      else
-        g_key_file_remove_key (amd->keyfile, account, key, NULL);
-    }
 
   /* if we removed the account before, it now exists again, so... */
   g_hash_table_remove (amd->removed_accounts, account);
@@ -580,8 +577,10 @@ _delete (const McpAccountStorage *self,
           g_key_file_remove_group (amd->keyfile, account, NULL);
           _delete_from_keyring (self, am, account, NULL);
         }
-      else if (mcp_account_manager_parameter_is_secret (am, account, key))
+      else
         {
+          /* always delete from keyring, even if we didn't previously
+           * think it was secret - we might have been wrong */
           _delete_from_keyring (self, am, account, key);
         }