Revert "Remove all notion of secret parameter"

This reverts commit ae64063c953840f99b1204a222fabf5aa7a37b69.

10 files changed, 189 insertions, 8 deletions

diff --git a/mission-control-plugins/account-storage.c b/mission-control-plugins/account-storage.c
index 0d7c9323..7b813c4c 100644
--- a/mission-control-plugins/account-storage.c
+++ b/mission-control-plugins/account-storage.c
@@ -444,7 +444,8 @@ mcp_account_storage_priority (const McpAccountStorage *storage)
  * Before emitting this signal, the plugin must call
  * either mcp_account_manager_set_attribute(),
  * mcp_account_manager_set_parameter(),
- * or mcp_account_manager_set_value()
+ * or mcp_account_manager_set_value() and (if appropriate)
+ * mcp_account_manager_parameter_make_secret()
  * before returning from this method call.
  *
  * Note that mcp_account_manager_set_parameter() does not use the
diff --git a/mission-control-plugins/account.c b/mission-control-plugins/account.c
index 493a34c0..1744ef6a 100644
--- a/mission-control-plugins/account.c
+++ b/mission-control-plugins/account.c
@@ -216,6 +216,65 @@ mcp_account_manager_get_value (const McpAccountManager *mcpa,
 }
 
 /**
+ * mcp_account_manager_parameter_is_secret:
+ * @mcpa: an #McpAccountManager instance
+ * @account: the unique name of an account
+ * @key: the constant string "param-", plus a parameter name like
+ *  "account" or "password"
+ *
+ * Determine whether a given account parameter is secret.
+ * Generally this is determined by MC and passed down to plugins,
+ * but any #McpAccountStorage plugin may decide a parameter is
+ * secret, in which case the return value for this call will
+ * indicate that fact too.
+ *
+ * For historical reasons, this function only operates on parameters,
+ * but requires its argument to be prefixed with "param-".
+ *
+ * Returns: %TRUE for secret settings, %FALSE otherwise
+ */
+gboolean
+mcp_account_manager_parameter_is_secret (const McpAccountManager *mcpa,
+    const gchar *account,
+    const gchar *key)
+{
+  McpAccountManagerIface *iface = MCP_ACCOUNT_MANAGER_GET_IFACE (mcpa);
+
+  g_return_val_if_fail (iface != NULL, FALSE);
+  g_return_val_if_fail (iface->is_secret != NULL, FALSE);
+
+  return iface->is_secret (mcpa, account, key);
+}
+
+/**
+ * mcp_account_manager_parameter_make_secret:
+ * @mcpa: an #McpAccountManager instance
+ * @account: the unique name of an account
+ * @key: the constant string "param-", plus a parameter name like
+ *  "account" or "password"
+ *
+ * Flag an account setting as secret for the lifetime of this
+ * #McpAccountManager. For instance, this should be called if
+ * @key has been retrieved from gnome-keyring.
+ *
+ * For historical reasons, this function only operates on parameters,
+ * but requires its argument to be prefixed with "param-".
+ */
+void
+mcp_account_manager_parameter_make_secret (const McpAccountManager *mcpa,
+    const gchar *account,
+    const gchar *key)
+{
+  McpAccountManagerIface *iface = MCP_ACCOUNT_MANAGER_GET_IFACE (mcpa);
+
+  g_return_if_fail (iface != NULL);
+  g_return_if_fail (iface->make_secret != NULL);
+
+  g_debug ("%s.%s should be secret", account, key);
+  iface->make_secret (mcpa, account, key);
+}
+
+/**
  * mcp_account_manager_get_unique_name:
  * @mcpa: an #McpAccountManager instance
  * @manager: the name of the manager
diff --git a/mission-control-plugins/account.h b/mission-control-plugins/account.h
index c283ef99..4015457b 100644
--- a/mission-control-plugins/account.h
+++ b/mission-control-plugins/account.h
@@ -66,6 +66,14 @@ gchar * mcp_account_manager_get_value (const McpAccountManager *mcpa,
     const gchar *account,
     const gchar *key);
 
+gboolean mcp_account_manager_parameter_is_secret (const McpAccountManager *mcpa,
+    const gchar *account,
+    const gchar *key);
+
+void mcp_account_manager_parameter_make_secret (const McpAccountManager *mcpa,
+    const gchar *account,
+    const gchar *key);
+
 gchar * mcp_account_manager_get_unique_name (McpAccountManager *mcpa,
     const gchar *manager,
     const gchar *protocol,
diff --git a/mission-control-plugins/implementation.h b/mission-control-plugins/implementation.h
index 9cc04b4e..2ad28938 100644
--- a/mission-control-plugins/implementation.h
+++ b/mission-control-plugins/implementation.h
@@ -86,6 +86,14 @@ struct _McpAccountManagerIface {
       const gchar *acct,
       const gchar *key);
 
+  gboolean (*is_secret) (const McpAccountManager *ma,
+      const gchar *acct,
+      const gchar *key);
+
+  void (* make_secret) (const McpAccountManager *ma,
+      const gchar *acct,
+      const gchar *key);
+
   gchar * (* unique_name) (const McpAccountManager *ma,
       const gchar *manager,
       const gchar *protocol,
diff --git a/mission-control-plugins/mission-control-plugins.h b/mission-control-plugins/mission-control-plugins.h
index 806f472a..13d87e6f 100644
--- a/mission-control-plugins/mission-control-plugins.h
+++ b/mission-control-plugins/mission-control-plugins.h
@@ -27,6 +27,7 @@
 
 typedef enum {
     MCP_PARAMETER_FLAG_NONE = 0,
+    MCP_PARAMETER_FLAG_SECRET = TP_CONN_MGR_PARAM_FLAG_SECRET
 } McpParameterFlags;
 
 typedef enum {
diff --git a/src/mcd-account.c b/src/mcd-account.c
index 752e9266..7b51afb8 100644
--- a/src/mcd-account.c
+++ b/src/mcd-account.c
@@ -378,8 +378,9 @@ _mcd_account_set_parameter (McdAccount *account, const gchar *name,
     McdAccountPrivate *priv = account->priv;
     McdStorage *storage = priv->storage;
     const gchar *account_name = mcd_account_get_unique_name (account);
+    gboolean secret = mcd_account_parameter_is_secret (account, name);
 
-    mcd_storage_set_parameter (storage, account_name, name, value);
+    mcd_storage_set_parameter (storage, account_name, name, value, secret);
 }
 
 static GType mc_param_type (const TpConnectionManagerParam *param);
@@ -5155,6 +5156,19 @@ _mcd_account_needs_dispatch (McdAccount *self)
     return self->priv->always_dispatch;
 }
 
+gboolean
+mcd_account_parameter_is_secret (McdAccount *self, const gchar *name)
+{
+    McdAccountPrivate *priv = self->priv;
+    const TpConnectionManagerParam *param;
+
+    param = mcd_manager_get_protocol_param (priv->manager,
+                                            priv->protocol_name, name);
+
+    return (param != NULL &&
+        tp_connection_manager_param_is_secret (param));
+}
+
 void
 _mcd_account_set_changing_presence (McdAccount *self, gboolean value)
 {
diff --git a/src/mcd-account.h b/src/mcd-account.h
index 3aad723f..59d8d9e3 100644
--- a/src/mcd-account.h
+++ b/src/mcd-account.h
@@ -129,6 +129,9 @@ McdConnection *mcd_account_get_connection (McdAccount *account);
 gboolean mcd_account_check_request (McdAccount *account, GHashTable *request,
                                     GError **error);
 
+gboolean mcd_account_parameter_is_secret (McdAccount *self,
+                                              const gchar *name);
+
 void mcd_account_altered_by_plugin (McdAccount *account, const gchar *name);
 
 gchar * mcd_account_dup_display_name (McdAccount *self);
diff --git a/src/mcd-storage.c b/src/mcd-storage.c
index c31dc2e6..df79d89e 100644
--- a/src/mcd-storage.c
+++ b/src/mcd-storage.c
@@ -72,6 +72,9 @@ typedef struct {
      * e.g. { 'account': 'fred@example.com', 'password': 'foo' }
      * keys of @parameters and @escaped_parameters are disjoint */
     GHashTable *escaped_parameters;
+    /* set of owned strings
+     * e.g. { 'password': 'password' } */
+    GHashTable *secrets;
 
     /* owned storage plugin owning this account */
     McpAccountStorage *storage;
@@ -89,6 +92,8 @@ mcd_storage_account_new (McpAccountStorage *storage)
       g_free, (GDestroyNotify) g_variant_unref);
   sa->escaped_parameters = g_hash_table_new_full (g_str_hash, g_str_equal,
       g_free, g_free);
+  sa->secrets = g_hash_table_new_full (g_str_hash, g_str_equal,
+      g_free, NULL);
   sa->storage = g_object_ref (storage);
 
   return sa;
@@ -102,6 +107,7 @@ mcd_storage_account_free (gpointer p)
   g_hash_table_unref (sa->attributes);
   g_hash_table_unref (sa->parameters);
   g_hash_table_unref (sa->escaped_parameters);
+  g_hash_table_unref (sa->secrets);
   g_object_unref (sa->storage);
   g_slice_free (McdStorageAccount, sa);
 }
@@ -428,6 +434,12 @@ mcpa_set_parameter (const McpAccountManager *ma,
   if (value != NULL)
     g_hash_table_insert (sa->parameters, g_strdup (parameter),
         g_variant_ref_sink (value));
+
+  if (flags & MCP_PARAMETER_FLAG_SECRET)
+    {
+      DEBUG ("flagging %s parameter %s as secret", account, parameter);
+      g_hash_table_add (sa->secrets, g_strdup (parameter));
+    }
 }
 
 static void
@@ -513,6 +525,49 @@ list_keys (const McpAccountManager *ma,
   return (GStrv) g_ptr_array_free (ret, FALSE);
 }
 
+static gboolean
+is_secret (const McpAccountManager *ma,
+    const gchar *account,
+    const gchar *key)
+{
+  McdStorage *self = MCD_STORAGE (ma);
+  McdStorageAccount *sa = lookup_account (self, account);
+
+  if (sa == NULL || !g_str_has_prefix (key, "param-"))
+    return FALSE;
+
+  return g_hash_table_contains (sa->secrets, key + 6);
+}
+
+static void
+mcd_storage_make_secret (McdStorage *self,
+    const gchar *account,
+    const gchar *key)
+{
+  McdStorageAccount *sa;
+
+  g_return_if_fail (MCD_IS_STORAGE (self));
+  g_return_if_fail (account != NULL);
+  g_return_if_fail (key != NULL);
+
+  if (!g_str_has_prefix (key, "param-"))
+    return;
+
+  sa = lookup_account (self, account);
+  g_return_if_fail (sa != NULL);
+
+  DEBUG ("flagging %s parameter %s as secret", account, key + 6);
+  g_hash_table_add (sa->secrets, g_strdup (key + 6));
+}
+
+static void
+make_secret (const McpAccountManager *ma,
+    const gchar *account,
+    const gchar *key)
+{
+  mcd_storage_make_secret (MCD_STORAGE (ma), account, key);
+}
+
 static gchar *
 unique_name (const McpAccountManager *ma,
     const gchar *manager,
@@ -1493,13 +1548,17 @@ update_storage (McdStorage *self,
     const gchar *account,
     const gchar *key,
     GVariant *variant,
-    const gchar *escaped)
+    const gchar *escaped,
+    gboolean secret)
 {
   McpAccountManager *ma = MCP_ACCOUNT_MANAGER (self);
   gboolean parameter = g_str_has_prefix (key, "param-");
   McdStorageAccount *sa;
   const gchar *pn;
 
+  if (secret)
+    mcd_storage_make_secret (self, account, key);
+
   sa = lookup_account (self, account);
   g_return_if_fail (sa != NULL);
 
@@ -1517,7 +1576,8 @@ update_storage (McdStorage *self,
     }
   else if (variant != NULL && parameter &&
       mcp_account_storage_set_parameter (sa->storage, ma, account, key + 6,
-          variant, MCP_PARAMETER_FLAG_NONE))
+          variant,
+          secret ? MCP_PARAMETER_FLAG_SECRET : MCP_PARAMETER_FLAG_NONE))
     {
       DEBUG ("MCP:%s -> store parameter %s.%s", pn, account, key);
     }
@@ -1634,7 +1694,7 @@ mcd_storage_set_attribute (McdStorage *self,
       if (value != NULL)
         escaped = mcd_keyfile_escape_value (value);
 
-      update_storage (self, account, attribute, new_v, escaped);
+      update_storage (self, account, attribute, new_v, escaped, FALSE);
       g_free (escaped);
       updated = TRUE;
     }
@@ -1649,6 +1709,8 @@ mcd_storage_set_attribute (McdStorage *self,
  * @account: the unique name of an account
  * @parameter: the name of the parameter, e.g. "account"
  * @value: the value to be stored (or %NULL to erase it)
+ * @secret: whether the value is confidential (might get stored in the
+ * keyring, for example)
  *
  * Copies and stores the supplied @value (or removes it if %NULL) in the
  * internal cache.
@@ -1663,7 +1725,8 @@ gboolean
 mcd_storage_set_parameter (McdStorage *self,
     const gchar *account,
     const gchar *parameter,
-    const GValue *value)
+    const GValue *value,
+    gboolean secret)
 {
   GVariant *old_v;
   GVariant *new_v = NULL;
@@ -1707,7 +1770,7 @@ mcd_storage_set_parameter (McdStorage *self,
             g_variant_ref (new_v));
 
       g_snprintf (key, sizeof (key), "param-%s", parameter);
-      update_storage (self, account, key, new_v, new_escaped);
+      update_storage (self, account, key, new_v, new_escaped, secret);
       return TRUE;
     }
 
@@ -2184,6 +2247,8 @@ plugin_iface_init (McpAccountManagerIface *iface,
   iface->set_value = set_value;
   iface->set_attribute = mcpa_set_attribute;
   iface->set_parameter = mcpa_set_parameter;
+  iface->is_secret = is_secret;
+  iface->make_secret = make_secret;
   iface->unique_name = unique_name;
   iface->identify_account_async = identify_account_async;
   iface->identify_account_finish = identify_account_finish;
diff --git a/src/mcd-storage.h b/src/mcd-storage.h
index eb2ce36c..dc2435ff 100644
--- a/src/mcd-storage.h
+++ b/src/mcd-storage.h
@@ -88,7 +88,8 @@ gboolean mcd_storage_set_attribute (McdStorage *storage,
 gboolean mcd_storage_set_parameter (McdStorage *storage,
     const gchar *account,
     const gchar *parameter,
-    const GValue *value);
+    const GValue *value,
+    gboolean secret);
 
 gchar *mcd_storage_create_account (McdStorage *storage,
     const gchar *provider,
diff --git a/tests/twisted/dbus-account-plugin.c b/tests/twisted/dbus-account-plugin.c
index 9d79dc0a..d1c0bf14 100644
--- a/tests/twisted/dbus-account-plugin.c
+++ b/tests/twisted/dbus-account-plugin.c
@@ -1006,10 +1006,18 @@ test_dbus_account_plugin_get (const McpAccountStorage *storage,
       while (g_hash_table_iter_next (&iter, &k, &v))
         {
           gchar *param_foo;
+          McpParameterFlags flags;
 
           param_foo = g_strdup_printf ("param-%s", (const gchar *) k);
           mcp_account_manager_set_value (am, account_name, param_foo, v);
 
+          flags = GPOINTER_TO_UINT (g_hash_table_lookup (
+                account->parameter_flags, k));
+
+          if (flags & MCP_PARAMETER_FLAG_SECRET)
+            mcp_account_manager_parameter_make_secret (am, account_name,
+                param_foo);
+
           g_free (param_foo);
         }
 
@@ -1018,6 +1026,7 @@ test_dbus_account_plugin_get (const McpAccountStorage *storage,
       while (g_hash_table_iter_next (&iter, &k, &v))
         {
           gchar *param_foo;
+          guint32 flags;
           gchar *escaped = mcp_account_manager_escape_variant_for_keyfile (am,
               v);
 
@@ -1025,6 +1034,13 @@ test_dbus_account_plugin_get (const McpAccountStorage *storage,
           mcp_account_manager_set_value (am, account_name, param_foo, escaped);
           g_free (escaped);
 
+          flags = GPOINTER_TO_UINT (g_hash_table_lookup (account->parameter_flags,
+                k));
+
+          if (flags & MCP_PARAMETER_FLAG_SECRET)
+            mcp_account_manager_parameter_make_secret (am, account_name,
+                param_foo);
+
           g_free (param_foo);
         }
 
@@ -1037,12 +1053,17 @@ test_dbus_account_plugin_get (const McpAccountStorage *storage,
     {
       GVariant *v = g_hash_table_lookup (account->parameters, key + 6);
       const gchar *s = g_hash_table_lookup (account->untyped_parameters, key + 6);
+      guint32 flags = GPOINTER_TO_UINT (
+          g_hash_table_lookup (account->parameter_flags, key + 6));
 
       g_dbus_connection_emit_signal (self->bus, NULL,
           TEST_DBUS_ACCOUNT_PLUGIN_PATH, TEST_DBUS_ACCOUNT_PLUGIN_IFACE,
           "GetParameter",
           g_variant_new_parsed ("(%o, %s)", account->path, key + 6), NULL);
 
+      if (flags & MCP_PARAMETER_FLAG_SECRET)
+        mcp_account_manager_parameter_make_secret (am, account_name, key);
+
       if (v != NULL)
         {
           gchar *escaped = mcp_account_manager_escape_variant_for_keyfile (am,