After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 680776 - Store passwords in libaccounts
Store passwords in libaccounts
Status: RESOLVED FIXED
Product: empathy
Classification: Core
Component: UOA
2.33.x
Other Linux
: Normal enhancement
: ---
Assigned To: empathy-maint
empathy-maint
Depends on:
Blocks:
 
 
Reported: 2012-07-29 10:08 UTC by Guillaume Desmottes
Modified: 2012-08-27 13:49 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Remember password at least until session restart (6.65 KB, patch)
2012-08-24 13:38 UTC, Xavier Claessens
accepted-commit_now Details | Review
UOA: Set password auth mechanism/method on generic IM services (10.24 KB, patch)
2012-08-24 13:38 UTC, Xavier Claessens
accepted-commit_now Details | Review
UOA: Use an AgManager singleton (6.57 KB, patch)
2012-08-24 13:38 UTC, Xavier Claessens
none Details | Review
UOA: Store password into signond instead of gnome-keyring (12.87 KB, patch)
2012-08-24 13:38 UTC, Xavier Claessens
none Details | Review
Move X-TELEPATHY-PASSWORD mechanism code into empathy-sasl-mechanisms.c (8.17 KB, patch)
2012-08-24 13:38 UTC, Xavier Claessens
accepted-commit_now Details | Review
UOA auth: rename QueryInforData to AuthContext (6.29 KB, patch)
2012-08-24 13:38 UTC, Xavier Claessens
accepted-commit_now Details | Review
UOA auth: factor out auth_context_done() (1.75 KB, patch)
2012-08-24 13:38 UTC, Xavier Claessens
accepted-commit_now Details | Review
UOA auth: Move more code into auth_context_new() (3.82 KB, patch)
2012-08-24 13:38 UTC, Xavier Claessens
accepted-commit_now Details | Review
UOA auth: Wait for callback when requesting password (1.78 KB, patch)
2012-08-24 13:38 UTC, Xavier Claessens
accepted-commit_now Details | Review
UOA auth: handle the password mechanism as well (1.96 KB, patch)
2012-08-24 13:38 UTC, Xavier Claessens
accepted-commit_now Details | Review
UOA mc plugin: Use create_account_async (4.32 KB, patch)
2012-08-24 13:38 UTC, Xavier Claessens
none Details | Review
UOA auth: Request password if no credentials have ever be stored (4.95 KB, patch)
2012-08-24 13:38 UTC, Xavier Claessens
accepted-commit_now Details | Review

Description Guillaume Desmottes 2012-07-29 10:08:20 UTC
This will make easier for passwords to be shared among services.
Comment 1 Xavier Claessens 2012-08-24 13:38:08 UTC
Created attachment 222324 [details] [review]
Remember password at least until session restart
Comment 2 Xavier Claessens 2012-08-24 13:38:11 UTC
Created attachment 222325 [details] [review]
UOA: Set password auth mechanism/method on generic IM services
Comment 3 Xavier Claessens 2012-08-24 13:38:14 UTC
Created attachment 222326 [details] [review]
UOA: Use an AgManager singleton

This is especially useful in empathy-keyring where it would reload
all accounts each time we set a password.
Comment 4 Xavier Claessens 2012-08-24 13:38:17 UTC
Created attachment 222327 [details] [review]
UOA: Store password into signond instead of gnome-keyring
Comment 5 Xavier Claessens 2012-08-24 13:38:20 UTC
Created attachment 222328 [details] [review]
Move X-TELEPATHY-PASSWORD mechanism code into empathy-sasl-mechanisms.c
Comment 6 Xavier Claessens 2012-08-24 13:38:23 UTC
Created attachment 222329 [details] [review]
UOA auth: rename QueryInforData to AuthContext

This data is not used only to for the query_info call anymore,
so better have a more generic name.
Comment 7 Xavier Claessens 2012-08-24 13:38:25 UTC
Created attachment 222330 [details] [review]
UOA auth: factor out auth_context_done()
Comment 8 Xavier Claessens 2012-08-24 13:38:28 UTC
Created attachment 222331 [details] [review]
UOA auth: Move more code into auth_context_new()
Comment 9 Xavier Claessens 2012-08-24 13:38:31 UTC
Created attachment 222332 [details] [review]
UOA auth: Wait for callback when requesting password

signon_auth_session_process() does not keep its own ref on the session
object, so freeing the AuthContext before callback cancels the call.
Comment 10 Xavier Claessens 2012-08-24 13:38:35 UTC
Created attachment 222333 [details] [review]
UOA auth: handle the password mechanism as well

Since password is stored in SSO now, we can use the same code path
than other accounts.
Comment 11 Xavier Claessens 2012-08-24 13:38:38 UTC
Created attachment 222334 [details] [review]
UOA mc plugin: Use create_account_async

The sync API is racy because the AgAccountId isn't known
until ag_account_store() is done. This means that the storage
identifier isn't valid until then.
Comment 12 Xavier Claessens 2012-08-24 13:38:41 UTC
Created attachment 222335 [details] [review]
UOA auth: Request password if no credentials have ever be stored
Comment 13 Xavier Claessens 2012-08-24 13:41:20 UTC
git branch: http://cgit.collabora.com/git/user/xclaesse/empathy.git/log/?h=uoa-pwd
Comment 14 Xavier Claessens 2012-08-24 14:07:26 UTC
(In reply to comment #11)
> Created an attachment (id=222334) [details] [review]
> UOA mc plugin: Use create_account_async
> 
> The sync API is racy because the AgAccountId isn't known
> until ag_account_store() is done. This means that the storage
> identifier isn't valid until then.

This one is unrelated. It is for bug #682608. I've removed it from this git branch.
Comment 15 Xavier Claessens 2012-08-24 14:12:27 UTC
Note that there is a known bug in signond: http://code.google.com/p/accounts-sso/issues/detail?id=108

That means that if you don't check "remember password" box, it won't work. But in theory that should then remember the password in RAM, so until you restart the session.
Comment 16 Guillaume Desmottes 2012-08-27 09:21:56 UTC
Review of attachment 222324 [details] [review]:

++ but please test extensively
Comment 17 Guillaume Desmottes 2012-08-27 09:22:35 UTC
Review of attachment 222325 [details] [review]:

++
Comment 18 Guillaume Desmottes 2012-08-27 09:25:43 UTC
Review of attachment 222326 [details] [review]:

++
Comment 19 Guillaume Desmottes 2012-08-27 09:45:03 UTC
Review of attachment 222327 [details] [review]:

::: libempathy/empathy-keyring.c
@@ +451,3 @@
+    g_simple_async_result_set_from_error (data->result, error);
+
+

I don't think you have to use _in_idle as we are already in a callback.

@@ +468,3 @@
+    {
+      g_simple_async_result_set_from_error (data->result, error);
+static void

I don't think you have to use _in_idle as we are already in a callback.

@@ +516,3 @@
+      /* This is the first time we store password for this account */
+      params = tp_account_get_parameters (tp_account);
+

Add a comment saying this is for IRC.

@@ +695,3 @@
+          /* I see no other way to forget the stored password than overwriting
+           * with an empty one. */
+

Shouldn't the 3rd arg be TRUE to actually erase it accross sessions?
Comment 20 Guillaume Desmottes 2012-08-27 09:55:46 UTC
Review of attachment 222328 [details] [review]:

++

::: libempathy/empathy-sasl-mechanisms.c
@@ +44,3 @@
   { EMPATHY_SASL_MECHANISM_WLM, MECH_WLM },
   { EMPATHY_SASL_MECHANISM_GOOGLE, MECH_GOOGLE },
+  { EMPATHY_SASL_MECHANISM_PASSWORD, MECH_PASSWORD },

Move it to the first line as it's the more generic one?
Comment 21 Guillaume Desmottes 2012-08-27 09:58:00 UTC
Review of attachment 222329 [details] [review]:

++
Comment 22 Guillaume Desmottes 2012-08-27 09:58:26 UTC
Review of attachment 222330 [details] [review]:

++
Comment 23 Guillaume Desmottes 2012-08-27 09:59:21 UTC
Review of attachment 222331 [details] [review]:

++
Comment 24 Guillaume Desmottes 2012-08-27 10:00:07 UTC
Review of attachment 222332 [details] [review]:

++
Comment 25 Guillaume Desmottes 2012-08-27 10:07:32 UTC
Review of attachment 222335 [details] [review]:

A bit tricky but ok.
Comment 26 Guillaume Desmottes 2012-08-27 10:10:03 UTC
Review of attachment 222333 [details] [review]:

++
Comment 27 Xavier Claessens 2012-08-27 13:39:11 UTC
(In reply to comment #19)
> @@ +695,3 @@
> +          /* I see no other way to forget the stored password than overwriting
> +           * with an empty one. */
> +
> 
> Shouldn't the 3rd arg be TRUE to actually erase it accross sessions?

According to Mardy it will erase the pwd even when FALSE, and there is no benefit in storing an empty password... :)
Comment 28 Xavier Claessens 2012-08-27 13:42:34 UTC
(In reply to comment #20)
> Review of attachment 222328 [details] [review]:
> 
> ++
> 
> ::: libempathy/empathy-sasl-mechanisms.c
> @@ +44,3 @@
>    { EMPATHY_SASL_MECHANISM_WLM, MECH_WLM },
>    { EMPATHY_SASL_MECHANISM_GOOGLE, MECH_GOOGLE },
> +  { EMPATHY_SASL_MECHANISM_PASSWORD, MECH_PASSWORD },
> 
> Move it to the first line as it's the more generic one?

Actually no, otherwise it will be prefered over other mechs. I've added a comment.
Comment 29 Xavier Claessens 2012-08-27 13:49:02 UTC
All fixed and merged.