Attachment 89208 Details for Bug 27727 – 01/26] Remove all notion of secret parameter

[patch] 01/26] Remove all notion of secret parameter

0001-Remove-all-notion-of-secret-parameter.patch (text/plain), 16.27 KB, created by Simon McVittie on 2013-11-14 18:11:35 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Simon McVittie

Created: 2013-11-14 18:11:35 UTC

Size: 16.27 KB

patch

obsolete

>From 3f124851222ce9c8395982493ae88ff235669abe Mon Sep 17 00:00:00 2001
>From: Xavier Claessens <xavier.claessens@collabora.co.uk>
>Date: Fri, 8 Nov 2013 13:17:10 -0500
>Subject: [PATCH 01/26] Remove all notion of secret parameter
>
>We now depend on SASLAuthentication for handling secret, and MC
>does not have gnome-keyring anymore.
>
>[Adjusted to apply before other storage API changes -smcv]
>Bug: https://bugs.freedesktop.org/show_bug.cgi?id=71384
>Signed-off-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
>---
> mission-control-plugins/account-storage.c         |  3 +-
> mission-control-plugins/account.c                 | 59 ------------------
> mission-control-plugins/account.h                 |  8 ---
> mission-control-plugins/implementation.h          |  8 ---
> mission-control-plugins/mission-control-plugins.h |  1 -
> src/mcd-account.c                                 | 16 +----
> src/mcd-account.h                                 |  3 -
> src/mcd-storage.c                                 | 73 ++---------------------
> src/mcd-storage.h                                 |  3 +-
> tests/twisted/dbus-account-plugin.c               | 21 -------
> 10 files changed, 8 insertions(+), 187 deletions(-)
>
>diff --git a/mission-control-plugins/account-storage.c b/mission-control-plugins/account-storage.c
>index b51c126..14f4f6d 100644
>--- a/mission-control-plugins/account-storage.c
>+++ b/mission-control-plugins/account-storage.c
>@@ -395,8 +395,7 @@ 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() and (if appropriate)
>- * mcp_account_manager_parameter_make_secret()
>+ * or mcp_account_manager_set_value()
>  * 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 1744ef6..493a34c 100644
>--- a/mission-control-plugins/account.c
>+++ b/mission-control-plugins/account.c
>@@ -216,65 +216,6 @@ 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 4015457..c283ef9 100644
>--- a/mission-control-plugins/account.h
>+++ b/mission-control-plugins/account.h
>@@ -66,14 +66,6 @@ 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 2ad2893..9cc04b4 100644
>--- a/mission-control-plugins/implementation.h
>+++ b/mission-control-plugins/implementation.h
>@@ -86,14 +86,6 @@ 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 13d87e6..806f472 100644
>--- a/mission-control-plugins/mission-control-plugins.h
>+++ b/mission-control-plugins/mission-control-plugins.h
>@@ -27,7 +27,6 @@
> 
> 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 7b51afb..752e926 100644
>--- a/src/mcd-account.c
>+++ b/src/mcd-account.c
>@@ -378,9 +378,8 @@ _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, secret);
>+    mcd_storage_set_parameter (storage, account_name, name, value);
> }
> 
> static GType mc_param_type (const TpConnectionManagerParam *param);
>@@ -5156,19 +5155,6 @@ _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 59d8d9e..3aad723 100644
>--- a/src/mcd-account.h
>+++ b/src/mcd-account.h
>@@ -129,9 +129,6 @@ 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 f82cb79..6cc603b 100644
>--- a/src/mcd-storage.c
>+++ b/src/mcd-storage.c
>@@ -72,9 +72,6 @@ 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;
> } McdStorageAccount;
> 
> static void
>@@ -85,7 +82,6 @@ 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_slice_free (McdStorageAccount, sa);
> }
> 
>@@ -222,8 +218,6 @@ ensure_account (McdStorage *self,
>           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);
>       g_hash_table_insert (self->accounts, g_strdup (account), sa);
>     }
> 
>@@ -430,12 +424,6 @@ 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
>@@ -519,47 +507,6 @@ 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;
>-
>-  DEBUG ("flagging %s parameter %s as secret", account, key + 6);
>-  sa = ensure_account (self, account);
>-  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,
>@@ -1547,17 +1494,13 @@ update_storage (McdStorage *self,
>     const gchar *account,
>     const gchar *key,
>     GVariant *variant,
>-    const gchar *escaped,
>-    gboolean secret)
>+    const gchar *escaped)
> {
>   GList *store;
>   gboolean done = FALSE;
>   gboolean parameter = g_str_has_prefix (key, "param-");
>   McpAccountManager *ma = MCP_ACCOUNT_MANAGER (self);
> 
>-  if (secret)
>-    mcd_storage_make_secret (self, account, key);
>-
>   /* we're deleting, which is unconditional, no need to check if anyone *
>    * claims this setting for themselves                                 */
>   if (escaped == NULL)
>@@ -1582,8 +1525,7 @@ update_storage (McdStorage *self,
>         }
>       else if (variant != NULL && parameter &&
>           mcp_account_storage_set_parameter (plugin, ma, account, key + 6,
>-            variant,
>-            secret ? MCP_PARAMETER_FLAG_SECRET : MCP_PARAMETER_FLAG_NONE))
>+            variant, MCP_PARAMETER_FLAG_NONE))
>         {
>           done = TRUE;
>           DEBUG ("MCP:%s -> store parameter %s.%s", pn, account, key);
>@@ -1700,7 +1642,7 @@ mcd_storage_set_attribute (McdStorage *self,
>       if (value != NULL)
>         escaped = mcd_keyfile_escape_value (value);
> 
>-      update_storage (self, account, attribute, new_v, escaped, FALSE);
>+      update_storage (self, account, attribute, new_v, escaped);
>       g_free (escaped);
>       updated = TRUE;
>     }
>@@ -1715,8 +1657,6 @@ 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.
>@@ -1731,8 +1671,7 @@ gboolean
> mcd_storage_set_parameter (McdStorage *self,
>     const gchar *account,
>     const gchar *parameter,
>-    const GValue *value,
>-    gboolean secret)
>+    const GValue *value)
> {
>   GVariant *old_v;
>   GVariant *new_v = NULL;
>@@ -1775,7 +1714,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, secret);
>+      update_storage (self, account, key, new_v, new_escaped);
>       return TRUE;
>     }
> 
>@@ -2279,8 +2218,6 @@ 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 e440845..930b04b 100644
>--- a/src/mcd-storage.h
>+++ b/src/mcd-storage.h
>@@ -88,8 +88,7 @@ gboolean mcd_storage_set_attribute (McdStorage *storage,
> gboolean mcd_storage_set_parameter (McdStorage *storage,
>     const gchar *account,
>     const gchar *parameter,
>-    const GValue *value,
>-    gboolean secret);
>+    const GValue *value);
> 
> 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 a8a2e4d..c81bce4 100644
>--- a/tests/twisted/dbus-account-plugin.c
>+++ b/tests/twisted/dbus-account-plugin.c
>@@ -1006,18 +1006,10 @@ 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);
>         }
> 
>@@ -1026,7 +1018,6 @@ 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);
> 
>@@ -1034,13 +1025,6 @@ 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);
>         }
> 
>@@ -1053,17 +1037,12 @@ 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,
>-- 
>1.8.4.3
>

Actions: View | Splinter Review

Attachments on bug 27727: 35148 | 89208 | 89209 | 89210 | 89211 | 89212 | 89213 | 89214 | 89215 | 89216 | 89217 | 89218 | 89219 | 89220 | 89221 | 89222 | 89223 | 89224 | 89225 | 89226 | 89227 | 89228 | 89229 | 89230 | 89231 | 89232 | 89233 | 89237

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.