From 3f124851222ce9c8395982493ae88ff235669abe Mon Sep 17 00:00:00 2001 From: Xavier Claessens 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 --- 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