Bug 34416

http://git.collabora.co.uk/?p=user/danni/telepathy-mission-control.git;a=shortlog;h=refs/heads/external-password-storage-34416

Here is an incomplete branch to implement this (it doesn't implement the PasswordSaved property). Incomplete because I've run into spec issues I've raised in #33485.

This branch also depends on the following tp-glib trivia: http://git.collabora.co.uk/?p=user/danni/telepathy-glib.git;a=commitdiff;h=aae116f232fa266eb194b8021eaca0e47004085b

(In reply to comment #1)
> This branch also depends on the following tp-glib trivia:
> http://git.collabora.co.uk/?p=user/danni/telepathy-glib.git;a=commitdiff;h=aae116f232fa266eb194b8021eaca0e47004085b

r+, please merge!

Ok. This is now ready for review.

The only thing I don't like about this approach is how often it calls IdentifyAccount(), but it seems like it's not correct to cache that value?

+
+        /* determine if we support Acct.I.ExternalPasswordStorage */
+        if (tp_proxy_has_interface_by_id (mcd_manager_get_tp_proxy (manager),
+                MC_IFACE_QUARK_CONNECTION_MANAGER_INTERFACE_ACCOUNT_STORAGE))
+        {
+            DEBUG ("CM %s has CM.I.AccountStorage iface",
+                   mcd_manager_get_name (manager));
+            mcd_dbus_activate_optional_interface (
+                TP_SVC_DBUS_PROPERTIES (account),
+                MC_TYPE_SVC_ACCOUNT_INTERFACE_EXTERNAL_PASSWORD_STORAGE);
+        }

Does this work if the TpConnectionManager's been constructed from the .manager file, without actually service-activating the CM? I suspect that it doesn't, unfortunately … I will check telepathy-glib.

+        tp_cli_protocol_call_identify_account (protocol, -1, params,
+            account_delete_identify_account_cb,
+            NULL, NULL, G_OBJECT (account));

I wonder if there's a chance that the account could be unreferenced, stopping the callback from being called, and hence stopping the password from being purged? I think taking a reference explicitly would be safest.

(In reply to comment #3)
> The only thing I don't like about this approach is how often it calls
> IdentifyAccount(), but it seems like it's not correct to cache that value?

On the contrary. MC should not only cache the value, but it should name accounts using it. Of course, because we have to support accounts whose name was chosen by MC forever, we'll need to have a separate field in the account to cache this value.

I think we could file a separate bug for that: it might be hairy, and what you have here works.

(In reply to comment #4)
> +
> +        /* determine if we support Acct.I.ExternalPasswordStorage */
> +        if (tp_proxy_has_interface_by_id (mcd_manager_get_tp_proxy (manager),
> +                MC_IFACE_QUARK_CONNECTION_MANAGER_INTERFACE_ACCOUNT_STORAGE))
> +        {
> +            DEBUG ("CM %s has CM.I.AccountStorage iface",
> +                   mcd_manager_get_name (manager));
> +            mcd_dbus_activate_optional_interface (
> +                TP_SVC_DBUS_PROPERTIES (account),
> +                MC_TYPE_SVC_ACCOUNT_INTERFACE_EXTERNAL_PASSWORD_STORAGE);
> +        }
> 
> Does this work if the TpConnectionManager's been constructed from the .manager
> file, without actually service-activating the CM? I suspect that it doesn't,
> unfortunately … I will check telepathy-glib.

It does! Hooray.

> +        tp_cli_protocol_call_identify_account (protocol, -1, params,
> +            account_delete_identify_account_cb,
> +            NULL, NULL, G_OBJECT (account));
> 
> I wonder if there's a chance that the account could be unreferenced, stopping
> the callback from being called, and hence stopping the password from being
> purged? I think taking a reference explicitly would be safest.

http://git.collabora.co.uk/?p=user/wjt/telepathy-mission-control-wjt.git;a=commitdiff;h=44cf2efe29e430c5794216c3e440a4d265dcec61

> (In reply to comment #3)
> > The only thing I don't like about this approach is how often it calls
> > IdentifyAccount(), but it seems like it's not correct to cache that value?
> 
> On the contrary. MC should not only cache the value, but it should name
> accounts using it. Of course, because we have to support accounts whose name
> was chosen by MC forever, we'll need to have a separate field in the account to
> cache this value.
> 
> I think we could file a separate bug for that: it might be hairy, and what you
> have here works.

This is bug 24640.

(In reply to comment #5)
> http://git.collabora.co.uk/?p=user/wjt/telepathy-mission-control-wjt.git;a=commitdiff;h=44cf2efe29e430c5794216c3e440a4d265dcec61

There are now a couple of patches at <http://git.collabora.co.uk/?p=user/wjt/telepathy-mission-control-wjt.git;a=shortlog;h=refs/heads/external-password-storage-34416>: one fixing this nit, and one to depend on telepathy-glib 0.13.14.

> This is bug 24640.

Actually, it's bug 34640. Whoops.

Woop woop. Will be in 5.7.4.