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 658961 - Be able to ask for IM account password
Be able to ask for IM account password
Status: RESOLVED WONTFIX
Product: gnome-shell
Classification: Core
Component: telepathy
unspecified
Other Linux
: Normal normal
: ---
Assigned To: GNOME Shell Telepathy maintainer(s)
gnome-shell-maint
Depends on:
Blocks:
 
 
Reported: 2011-09-13 16:03 UTC by William Jon McCann
Modified: 2017-04-06 22:16 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
screenshot (499.35 KB, image/png)
2011-09-13 16:03 UTC, William Jon McCann
  Details
screenshot (190.83 KB, image/jpeg)
2011-09-20 13:53 UTC, Guillaume Desmottes
  Details
Be able to ask for IM account password (7.57 KB, patch)
2011-09-30 10:17 UTC, Guillaume Desmottes
reviewed Details | Review
Be able to ask for IM account password (7.85 KB, patch)
2011-10-08 15:53 UTC, Guillaume Desmottes
none Details | Review
Be able to ask for IM account password (7.95 KB, patch)
2011-10-08 19:42 UTC, Guillaume Desmottes
none Details | Review
Be able to ask for IM account password (7.94 KB, patch)
2011-10-25 08:50 UTC, Guillaume Desmottes
reviewed Details | Review

Description William Jon McCann 2011-09-13 16:03:39 UTC
Created attachment 196405 [details]
screenshot

After installing updates after an install and rebooting I got the prompt in the attached shot. This is really weird. We don't want to be randomly prompting the user out of context for his/her password.

Entering the password for the gnome online accounts should be done from the gnome online accounts tool only. If that's what this is.
Comment 1 Guillaume Desmottes 2011-09-14 10:15:35 UTC
That's because of bug #658785 (the account shouldn't connect automatically) and bug #652546 (we don't, and can't, implement SSO for Google tak atm).

*** This bug has been marked as a duplicate of bug 658785 ***
Comment 2 Guillaume Desmottes 2011-09-20 10:18:30 UTC
Actually there is a 3rd bug: the Shell should ask itself for the password.

- Be sure to *not* have Emapthy running
- Try to connect a newly created GOA account (or another account not having its passwords saved)
- empathy-auth-client popups a dialog asking for the password

Expected behaviour: the Shell asks for this password in a less intrusive way. I guess we could use a tray notification allowing user to enter the password directly.
Comment 3 Guillaume Desmottes 2011-09-20 13:53:17 UTC
Created attachment 197058 [details]
screenshot

Here is how it looks like atm. Two problems so far:

- bug #659575 makes it look weird
- User needs to be able to ask for the password to be saved (if not he will have to enter it each time the account connect). How would you do that from an UI pov?
Comment 4 Cosimo Cecchi 2011-09-20 16:22:02 UTC
(In reply to comment #3)

> Here is how it looks like atm. Two problems so far:
> 
> - bug #659575 makes it look weird
> - User needs to be able to ask for the password to be saved (if not he will
> have to enter it each time the account connect). How would you do that from an
> UI pov?

My two cents:
- in the ideal use case, you should never ask passwords for GOA accounts, as they already have been granted an authorization from the online accounts panel. GOA should already cover the case when an account needs to renew its authorization credentials by itself (as far as I understand, this is still not possible due to bug 652546 at the moment, right?)
- for accounts which don't have the password saved (i.e. non-GOA accounts created manually from Empathy itself), I believe we could either have a switch in the notification itself, or just delegate that to empathy-accounts (e.g. always save the password typed there, and trigger the notification only in case no password has been entered)...I think I prefer the latter.
Comment 5 Guillaume Desmottes 2011-09-21 07:53:09 UTC
(In reply to comment #4)
> - in the ideal use case, you should never ask passwords for GOA accounts, as
> they already have been granted an authorization from the online accounts panel.
> GOA should already cover the case when an account needs to renew its
> authorization credentials by itself (as far as I understand, this is still not
> possible due to bug 652546 at the moment, right?)

Yes; that's not possible atm.

> - for accounts which don't have the password saved (i.e. non-GOA accounts
> created manually from Empathy itself), I believe we could either have a switch
> in the notification itself, or just delegate that to empathy-accounts (e.g.
> always save the password typed there, and trigger the notification only in case
> no password has been entered)...I think I prefer the latter.

I'm not sure to understand what you mean. Atm if empathy is running, it will
generate a notification having a 'Provide' button. Hitting this button will
open empathy-auth-client's password (cf first screenshot). I guess we could do
that in the Shell as well so that will work even if empathy isn't running but
that seems a bit weird to me tbh. Makes much sense to allow user to enter his
password directly into the notification.
Comment 6 Cosimo Cecchi 2011-09-21 14:12:41 UTC
(In reply to comment #5)

> > - for accounts which don't have the password saved (i.e. non-GOA accounts
> > created manually from Empathy itself), I believe we could either have a switch
> > in the notification itself, or just delegate that to empathy-accounts (e.g.
> > always save the password typed there, and trigger the notification only in case
> > no password has been entered)...I think I prefer the latter.
> 
> I'm not sure to understand what you mean. Atm if empathy is running, it will
> generate a notification having a 'Provide' button. Hitting this button will
> open empathy-auth-client's password (cf first screenshot). I guess we could do
> that in the Shell as well so that will work even if empathy isn't running but
> that seems a bit weird to me tbh. Makes much sense to allow user to enter his
> password directly into the notification.

I was referring only to the question "how do I toggle remember password if we use a notification?". What I mean is
- having the "remember password" option for GOA accounts is not interesting, as we want to fix bug 652546 and never prompt in that case. (I believe as a workaround in the meantime the password could be always saved for GOA accounts so we spawn the entry only once)
- for other custom accounts I was just proposing not to worry about this in the notification, as empathy-accounts' UI is the place where you create the account and decide whether to save a password or not.
Comment 7 Guillaume Desmottes 2011-09-30 10:15:41 UTC
Agreed. So we should only consider adding automatic saving of GOA accounts if we didn't fix bug #652546 for 3.4.

So that means my branch can be reviewed as it for now. :)
Comment 8 Guillaume Desmottes 2011-09-30 10:17:35 UTC
Created attachment 197865 [details] [review]
Be able to ask for IM account password
Comment 9 Cosimo Cecchi 2011-10-03 16:06:14 UTC
Review of attachment 197865 [details] [review]:

Just some drive-by comments

::: js/ui/telepathyClient.js
@@ +486,3 @@
+    _approveServerAuth: function(account, conn, channel, dispatchOp, context) {
+        let source = new ApproverSource(dispatchOp, _("Password needed"),
+                                        Shell.util_icon_from_string('gtk-dialog-authentication'));

Isn't that the GTK stock icon name? I think you should use 'dialog-password' here instead.

@@ +1586,3 @@
+        // Claim the channel as we are going to provide the password
+        this._dispatchOp.claim_with_async(this._tpClient,
+                                        Lang.bind(this, function(dispatchOp, result) {

I think it's clearer to indent it like this:

this._dispatchOp.claim_with_async(this._tpClient, Lang.bind(this,
    function(dispatchOp, result) {

    }));

::: src/shell-tp-client.c
@@ +507,3 @@
+    }
+  else if (status == TP_SASL_STATUS_SUCCEEDED ||
+      status == TP_SASL_STATUS_SERVER_FAILED ||

AFAICS it's wrong for this to be in an "else if()" block, as the channel won't be unreffed when the status is STATUS_SUCCEEDED.
Comment 10 Xavier Claessens 2011-10-03 18:13:56 UTC
Please handle only channels with X-TELEPATHY-PASSWORD auth mechanism, soon gtalk will use GOA's access-token and xmpp-messenger does not support password at all.

See related bug:
https://bugs.freedesktop.org/show_bug.cgi?id=41388

And similar fix in empathy:
http://git.gnome.org/browse/empathy/commit/?id=c7fe998d565bd37ed066e1fee0e7281c7f6117ce

Also in the case keyring has the password, won't this show the notification area and close it quickly?
Comment 11 Guillaume Desmottes 2011-10-08 15:53:19 UTC
(In reply to comment #9)
> ::: js/ui/telepathyClient.js
> @@ +486,3 @@
> +    _approveServerAuth: function(account, conn, channel, dispatchOp, context)
> {
> +        let source = new ApproverSource(dispatchOp, _("Password needed"),
> +                                       
> Shell.util_icon_from_string('gtk-dialog-authentication'));
> 
> Isn't that the GTK stock icon name? I think you should use 'dialog-password'
> here instead.

changed.

> @@ +1586,3 @@
> +        // Claim the channel as we are going to provide the password
> +        this._dispatchOp.claim_with_async(this._tpClient,
> +                                        Lang.bind(this, function(dispatchOp,
> result) {
> 
> I think it's clearer to indent it like this:
> 
> this._dispatchOp.claim_with_async(this._tpClient, Lang.bind(this,
>     function(dispatchOp, result) {
> 
>     }));

changed.

> ::: src/shell-tp-client.c
> @@ +507,3 @@
> +    }
> +  else if (status == TP_SASL_STATUS_SUCCEEDED ||
> +      status == TP_SASL_STATUS_SERVER_FAILED ||
> 
> AFAICS it's wrong for this to be in an "else if()" block, as the channel won't
> be unreffed when the status is STATUS_SUCCEEDED.

You're right; good catch. fixed.

(In reply to comment #10)
> Please handle only channels with X-TELEPATHY-PASSWORD auth mechanism, soon
> gtalk will use GOA's access-token and xmpp-messenger does not support password
> at all.

done.

> Also in the case keyring has the password, won't this show the notification
> area and close it quickly?

Nope, empathy-auth-client acts as a pre-approver (Observer).
Comment 12 Guillaume Desmottes 2011-10-08 15:53:56 UTC
Created attachment 198606 [details] [review]
Be able to ask for IM account password
Comment 13 Guillaume Desmottes 2011-10-08 19:42:57 UTC
Created attachment 198624 [details] [review]
Be able to ask for IM account password
Comment 14 Frederic Crozat 2011-10-24 13:42:30 UTC
Review of attachment 198624 [details] [review]:

::: js/ui/telepathyClient.js
@@ +495,3 @@
+
+        let source = new ApproverSource(dispatchOp, _("Password needed"),
+                                        Shell.util_icon_from_string('dialog-password'));

Shell.util_icon_from_string has removed, Gio.icon_new_for_string should be used instead.
Comment 15 Guillaume Desmottes 2011-10-25 08:50:40 UTC
Created attachment 199906 [details] [review]
Be able to ask for IM account password
Comment 16 André Klapper 2012-01-18 20:12:44 UTC
(In reply to comment #15)
> Be able to ask for IM account password

Could the last patch please get a review by the maintainers?
Comment 17 Jasper St. Pierre (not reading bugmail) 2012-01-18 20:28:46 UTC
Review of attachment 199906 [details] [review]:

Mostly fine, minor nits.

::: js/ui/telepathyClient.js
@@ +490,3 @@
+        // Only prompt user if channel supports password
+        if (available_mechanisms.indexOf('X-TELEPATHY-PASSWORD') == -1) {
+          Shell.decline_dispatch_op(context, 'Does not support X-TELEPATHY-PASSWORD');

Four space indents.

@@ +1590,3 @@
+
+        this._entry = new St.Entry({ can_focus: true });
+        this._entry.clutter_text.set_password_char('\u25cf'); // ● U+25CF BLACK CIRCLE

In order to embed Unicode chars inline, we'd need to put an "encoding" property in the modeline for tools to detect. I'd just remove the comment.

@@ +1606,3 @@
+                    Shell.provide_password (this._channel, text);
+                } catch (err) {
+                    throw new Error('Failed to Claim channel: ' + err);

Put the closing brace to the try on its own line.
Comment 18 André Klapper 2012-03-07 09:43:06 UTC
[Bumping GNOME Target field as this has not happened for 3.2]
Comment 19 Guillaume Desmottes 2012-03-19 14:41:09 UTC
Actually I'm not 100% happy of this approach. empathy-auth-client gained more logic in 3.4 to deal better with wrong password being entered, etc. Duplicating this logic into the Shell would be a bit sad from a code pov. Maybe we should have something more complex like gnome-keyring's unlock UIs which can be provided by different components?

I'd be in favor of not merging this patch for 3.4; empathy-auth-client still does the job even if it's not perfect from an UI pov.
Comment 20 André Klapper 2012-08-20 11:12:12 UTC
(In reply to comment #19 by Guillaume Desmottes)
> I'd be in favor of not merging this patch for 3.4; empathy-auth-client still
> does the job even if it's not perfect from an UI pov.

Has anything changed with regard to 3.6, or is this postponed to a "mysterious future"?
Comment 21 Guillaume Desmottes 2012-09-03 13:05:16 UTC
Nothing changed and the current situation is not that bad tbh so it's mostly a postpone to some future.
Comment 22 Xavier Claessens 2012-09-03 13:17:25 UTC
Note that this is fixed in Ubuntu, the password is asked by their control center. When GOA will have done the same work, it will be responsible of asking passwords.
Comment 23 Kjartan Maraas 2017-04-06 21:44:11 UTC
Removing GNOME 3.4 target
Comment 24 Florian Müllner 2017-04-06 22:16:46 UTC
Chat integration has been reduced to showing notifications for active one-on-one channels. That is, gnome-shell never touches the telepathy presence, either automatically or in response to user action. So if there's a presence change, it has been initiated in an application, and I don't think it makes sense to delegate any authentication handling to the shell.