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 759828 - Don't offer chat feature when built with --disable-telepathy
Don't offer chat feature when built with --disable-telepathy
Status: RESOLVED FIXED
Product: gnome-online-accounts
Classification: Core
Component: general
3.19.x
Other All
: Normal minor
: ---
Assigned To: Michael Catanzaro
GNOME Online Accounts maintainer(s)
Depends on:
Blocks:
 
 
Reported: 2015-12-24 01:41 UTC by Michael Catanzaro
Modified: 2016-01-13 19:17 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
google: pretend not to support chat when telepathy provider is disabled (2.22 KB, patch)
2015-12-24 02:46 UTC, Michael Catanzaro
none Details | Review
google: pretend not to support chat when telepathy provider is disabled (2.86 KB, patch)
2016-01-11 20:49 UTC, Michael Catanzaro
committed Details | Review
google: Do not support chat when telepathy is disabled (2.24 KB, patch)
2016-01-13 13:07 UTC, Debarshi Ray
committed Details | Review

Description Michael Catanzaro 2015-12-24 01:41:51 UTC
goa_provider_show_account() should not create a Chat toggle for Google accounts if configured with --disable-telepathy.

Also, audit other account types for this issue.
Comment 1 Michael Catanzaro 2015-12-24 01:46:30 UTC
(In reply to Michael Catanzaro from comment #0)
> Also, audit other account types for this issue.

It's only an issue for Google.
Comment 2 Michael Catanzaro 2015-12-24 01:54:55 UTC
I guess the trick here is that technically, goagoogleprovider does not use telepathy, so from a code perspective, it's kind of correct to display the chat field... something COULD use it.

But in practice, Google Chat is only usable via Empathy, and if telepathy support is disabled we don't want to see it.
Comment 3 Michael Catanzaro 2015-12-24 02:14:08 UTC
Also, the Google chat contacts need to not show up in Empathy.
Comment 4 Michael Catanzaro 2015-12-24 02:46:01 UTC
Created attachment 317842 [details] [review]
google: pretend not to support chat when telepathy provider is disabled

Since only Empathy uses this feature, and if the telepathy provider is
disabled, that means we really don't want to support Empathy. This is
useful for downstreams that don't want to ship Empathy, and therefore
don't want any chat configuration in the Online Accounts panel, or for
Google contacts to show up when running Empathy.
Comment 5 Debarshi Ray 2016-01-11 17:20:10 UTC
Review of attachment 317842 [details] [review]:

Thanks for the patch, Michael.

We should also disable asking for the Google Talk scope in get_scope. Otherwise the user will see that we are asking for permissions to use Google Talk, without anything visible actually using it.

There is one issue, though. As far as I can tell, Google Talk only works with OAuth2 nowadays, which means that I can no longer add an account directly in empathy-accounts and make it work. In that scenario, it is no longer just a question of shipping empathy by default. We are practically saying that empathy is dead and should be retired completely from distributors' repositories.

::: src/goabackend/goagoogleprovider.c
@@ +445,3 @@
+   *
+   * Yes, it's slight abuse to look at whether the unrelated Telepathy provider
+   * is enabled to decide whether to enable chat. But in practice, only Empathy

Nitpick. It's actually telepathy-mission-control that uses it, not empathy, via a mission-control plugin. It's just that the plugin lives in empathy.git for some odd reason.
Comment 6 Debarshi Ray 2016-01-11 17:29:42 UTC
(In reply to Debarshi Ray from comment #5)
> There is one issue, though. As far as I can tell, Google Talk only works
> with OAuth2 nowadays, which means that I can no longer add an account
> directly in empathy-accounts and make it work. In that scenario, it is no
> longer just a question of shipping empathy by default. We are practically
> saying that empathy is dead and should be retired completely from
> distributors' repositories.

For your information, I filed bug 760483 against empathy to suggest dropping password-based Google accounts from the UI.
Comment 7 Michael Catanzaro 2016-01-11 17:59:49 UTC
(In reply to Debarshi Ray from comment #5)
> We should also disable asking for the Google Talk scope in get_scope.
> Otherwise the user will see that we are asking for permissions to use Google
> Talk, without anything visible actually using it.

OK.

> There is one issue, though. As far as I can tell, Google Talk only works
> with OAuth2 nowadays, which means that I can no longer add an account
> directly in empathy-accounts and make it work. In that scenario, it is no
> longer just a question of shipping empathy by default. We are practically
> saying that empathy is dead and should be retired completely from
> distributors' repositories.

I don't agree... Empathy handles way more than just Google. I use Empathy for IRC, XMPP, and SIP. We're simply saying that Empathy is now a standalone app, not a core component of our OS, so it needs to handle authentication on its own. Seems like there is already a serious Empathy bug independent of this issue if it offers to create Google accounts in its UI and that feature does not work.

> ::: src/goabackend/goagoogleprovider.c
> Nitpick. It's actually telepathy-mission-control that uses it, not empathy,
> via a mission-control plugin. It's just that the plugin lives in empathy.git
> for some odd reason.

OK.
Comment 8 Debarshi Ray 2016-01-11 18:09:53 UTC
(In reply to Michael Catanzaro from comment #7)
> > There is one issue, though. As far as I can tell, Google Talk only works
> > with OAuth2 nowadays, which means that I can no longer add an account
> > directly in empathy-accounts and make it work. In that scenario, it is no
> > longer just a question of shipping empathy by default. We are practically
> > saying that empathy is dead and should be retired completely from
> > distributors' repositories.
> 
> I don't agree... Empathy handles way more than just Google. I use Empathy
> for IRC, XMPP, and SIP. We're simply saying that Empathy is now a standalone
> app, not a core component of our OS, so it needs to handle authentication on
> its own. Seems like there is already a serious Empathy bug independent of
> this issue if it offers to create Google accounts in its UI and that feature
> does not work.

Ok, then.
Comment 9 Michael Catanzaro 2016-01-11 20:49:40 UTC
Created attachment 318824 [details] [review]
google: pretend not to support chat when telepathy provider is disabled

Since only Empathy uses this feature, and if the telepathy provider is
disabled, that means we really don't want to support Empathy. This is
useful for downstreams that don't want to ship Empathy, and therefore
don't want any chat configuration in the Online Accounts panel, or for
Google contacts to show up when running Empathy.

Debarshi notes that "[i]t's actually telepathy-mission-control that uses
it, not empathy, via a mission-control plugin. It's just that the plugin
lives in empathy.git for some odd reason." In theory, other apps besides
Empathy could make use of these accounts (but none do).
Comment 10 Debarshi Ray 2016-01-13 13:07:10 UTC
Created attachment 318948 [details] [review]
google: Do not support chat when telepathy is disabled

I pushed it after streamlining the comment and commit message a bit.
Comment 11 Debarshi Ray 2016-01-13 13:16:38 UTC
I am wondering what to do in the credentials_generation vfunc, though. The credentials generation lets us forcefully invalidate credentials. eg., if we added/removed a OAuth2 scope and don't want to use tokens generated against the older set of scopes, then we bump the generation by 1 and force existing account holders to "refresh" their account in the Online Accounts panel.

At the moment it is a very simplistic system, and has served us well to deal with "add new service Foo to provider Bar", but it is not designed for such compile time switches.

One option is to turn it into a floating point and then return xx.0, xx.1, etc. to represent each combination of compile-time options.
Comment 12 Michael Catanzaro 2016-01-13 19:17:48 UTC
This is not a problem for normal users:

rishi: mcatanzaro: The problem could be that a paranoid user notices that we are using GTalk permissions without doing anything obvious.
mcatanzaro: rishi: OK, but only for users who previously had a version of g-o-a with Google Chat support, and only until they next refresh their credentials.
rishi: mcatanzaro: Yes. The thing that got me thinking was that when we add a new service/scope, we force them to get a new token.
rishi: Maybe we should do the same when we are dropping some scopes.
rishi: Note that I am not saying this is a problem.
rishi: Just thinking out loud. :)
mcatanzaro:  rishi: Honestly it might be better to do nothing, so as not to force users to re-enter Google credentials unnecessarily.
mcatanzaro:  We have to do that when we add permissions, but we don't have to do that when removing permissions... might as well wait until the next time the user has to enter credentials for some other reason.
rishi:  mcatanzaro: Can you dump that into the bug and close it? :)
mcatanzaro:  OK

Could be for experienced users or developers, though:

rishi:  mcatanzaro: The switching builds/distro will lead to subtle weirdness.
rishi:  Because the key without GTalk will overwrite the key with GTalk.
rishi:  And then you might suddenly see empathy not working for no clear reason.
mcatanzaro: rishi: In that case the user was already not using Empathy via GOA on his/her old distro, though.