From b18bdd67daa5fe7d9c83ead84807b4edc5c40d2c Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Fri, 8 Nov 2013 16:56:21 +0000 Subject: [PATCH 3/3] Allow backends, but not D-Bus clients, to delete restricted accounts I assumed that if TP_STORAGE_RESTRICTION_FLAG_CANNOT_SET_ENABLED or TP_STORAGE_RESTRICTION_FLAG_CANNOT_SET_PRESENCE, then a hypothetical CANNOT_DELETE flag would also have been present. --- src/mcd-account-manager.c | 10 +++++--- src/mcd-account.c | 29 ++++++++++++++++++---- src/mcd-account.h | 5 +++- .../twisted/account-manager/restricted-storage.py | 22 +++++++++++++++- 4 files changed, 56 insertions(+), 10 deletions(-) diff --git a/src/mcd-account-manager.c b/src/mcd-account-manager.c index c8af416..806f38f 100644 --- a/src/mcd-account-manager.c +++ b/src/mcd-account-manager.c @@ -395,7 +395,9 @@ deleted_cb (GObject *plugin, const gchar *name, gpointer data) /* this unhooks the account's signal handlers */ g_hash_table_remove (manager->priv->accounts, name); tp_svc_account_manager_emit_account_removed (manager, object_path); - mcd_account_delete (account, _mcd_account_delete_cb, NULL); + mcd_account_delete (account, + MCD_DBUS_PROP_SET_FLAG_ALREADY_IN_STORAGE, + _mcd_account_delete_cb, NULL); } } @@ -706,7 +708,8 @@ complete_account_creation_finish (McdAccount *account, if (!cad->ok) { - mcd_account_delete (account, NULL, NULL); + mcd_account_delete (account, MCD_DBUS_PROP_SET_FLAG_NONE, + NULL, NULL); tp_clear_object (&account); } @@ -1159,7 +1162,8 @@ migrate_create_account_cb (McdAccountManager *account_manager, DEBUG ("Account %s migrated, removing it", mcd_account_get_unique_name (ctx->account)); - mcd_account_delete (ctx->account, migrate_delete_account_cb, ctx); + mcd_account_delete (ctx->account, MCD_DBUS_PROP_SET_FLAG_NONE, + migrate_delete_account_cb, ctx); } static void diff --git a/src/mcd-account.c b/src/mcd-account.c index de4abd1..7b51afb 100644 --- a/src/mcd-account.c +++ b/src/mcd-account.c @@ -671,8 +671,12 @@ account_delete_identify_account_cb (TpProxy *protocol, g_object_unref (account); } +static TpStorageRestrictionFlags mcd_account_get_storage_restrictions ( + McdAccount *account); + void mcd_account_delete (McdAccount *account, + McdDBusPropSetFlags flags, McdAccountDeleteCb callback, gpointer user_data) { @@ -682,6 +686,23 @@ mcd_account_delete (McdAccount *account, const gchar *name = mcd_account_get_unique_name (account); TpConnectionManager *cm = mcd_account_get_cm (account); + /* We don't really have a flag for "cannot delete accounts" yet, but + * it seems reasonable that if you can't disable it or put it + * offline, you shouldn't be able to delete it. Also, we're going + * to try to disable it in a moment anyway. */ + if ((flags & MCD_DBUS_PROP_SET_FLAG_ALREADY_IN_STORAGE) == 0 && + (mcd_account_get_storage_restrictions (account) & + (TP_STORAGE_RESTRICTION_FLAG_CANNOT_SET_ENABLED | + TP_STORAGE_RESTRICTION_FLAG_CANNOT_SET_PRESENCE)) != 0) + { + g_set_error (&error, TP_ERROR, TP_ERROR_PERMISSION_DENIED, + "Storage plugin for %s does not allow deleting it", + name); + callback (account, error, user_data); + g_error_free (error); + return; + } + /* if the CM implements CM.I.AccountStorage, we need to tell the CM * to forget any account credentials it knows */ if (tp_proxy_has_interface_by_id (cm, @@ -705,7 +726,7 @@ mcd_account_delete (McdAccount *account, /* got to turn the account off before removing it, otherwise we can * * end up with an orphaned CM holding the account online */ if (!_mcd_account_set_enabled (account, FALSE, FALSE, - MCD_DBUS_PROP_SET_FLAG_NONE, &error)) + flags, &error)) { g_warning ("could not disable account %s (%s)", name, error->message); callback (account, error, user_data); @@ -1133,9 +1154,6 @@ get_has_been_online (TpSvcDBusProperties *self, const gchar *name, g_value_set_boolean (value, priv->has_been_online); } -static TpStorageRestrictionFlags mcd_account_get_storage_restrictions ( - McdAccount *account); - /** * mcd_account_set_enabled: * @account: the #McdAccount @@ -2443,7 +2461,8 @@ account_remove (TpSvcAccount *svc, DBusGMethodInvocation *context) data->context = context; DEBUG ("called"); - mcd_account_delete (self, account_remove_delete_cb, data); + mcd_account_delete (self, MCD_DBUS_PROP_SET_FLAG_NONE, + account_remove_delete_cb, data); } /* diff --git a/src/mcd-account.h b/src/mcd-account.h index 7843c27..59d8d9e 100644 --- a/src/mcd-account.h +++ b/src/mcd-account.h @@ -40,6 +40,7 @@ typedef struct _McdAccountClass McdAccountClass; #include "mcd-connection.h" #include "mcd-account-manager.h" +#include "mcd-dbusprop.h" struct _McdAccount { @@ -84,7 +85,9 @@ McdAccount *mcd_account_new (McdAccountManager *account_manager, const gchar *name, McdConnectivityMonitor *minotaur); -void mcd_account_delete (McdAccount *account, McdAccountDeleteCb callback, +void mcd_account_delete (McdAccount *account, + McdDBusPropSetFlags flags, + McdAccountDeleteCb callback, gpointer user_data); const gchar *mcd_account_get_unique_name (McdAccount *account); diff --git a/tests/twisted/account-manager/restricted-storage.py b/tests/twisted/account-manager/restricted-storage.py index bf01a2c..6724863 100644 --- a/tests/twisted/account-manager/restricted-storage.py +++ b/tests/twisted/account-manager/restricted-storage.py @@ -142,6 +142,9 @@ def test(q, bus, mc, fake_accounts_service=None, **kwargs): 'ConnectAutomatically', not online) q.expect('dbus-error', method='Set') + call_async(q, account, 'Remove') + q.expect('dbus-error', method='Remove') + # ... but the backend can still change them if enabled and online: q.forbid_events(try_to_connect) @@ -159,7 +162,24 @@ def test(q, bus, mc, fake_accounts_service=None, **kwargs): 'Enabled': True, 'ConnectAutomatically': True, }) - q.expect_many(*try_to_connect) + + events = q.expect_many(*try_to_connect) + conn = SimulatedConnection(q, bus, 'fakecm', 'fakeprotocol', + account_tail.replace('/', '_'), 'ezio', + has_presence=True) + q.dbus_return(events[-1].message, conn.bus_name, + conn.object_path, signature='so') + + # we are connected: deleting the account should + # disconnect us + fake_accounts_service.delete_account(account_tail) + q.expect_many( + EventPattern('dbus-signal', signal='AccountRemoved', + args=[account_path]), + EventPattern('dbus-signal', signal='Removed', + path=account_path), + EventPattern('dbus-method-call', method='Disconnect'), + ) if __name__ == '__main__': exec_test(test, {}, pass_kwargs=True) -- 1.8.4.2