Closed Bug 1238631 Opened 9 years ago Closed 3 years ago

Google talk cannot login when enable 2-factor auth

Categories

(Chat Core :: XMPP, defect)

defect
Not set
normal

Tracking

(thunderbird_esr91+ affected)

RESOLVED FIXED
101 Branch
Tracking Status
thunderbird_esr91 + affected

People

(Reporter: thomas, Assigned: clokep)

References

Details

Attachments

(1 file, 1 obsolete file)

There is always password fail when enabling 2-factor auth. An workaround right now is using "Sign in using App Passwords".
I think it need to use Oauth as ./chat/protocols/twitter/twitter.js .
Similar to bug 984027, yes it'd be nice to support oauth. I think this was never done because Google doesn't return a good error message (e.g. "You have 2 factor enabled!") it just returns the "invalid password" error if you try to use a password. We should just bite the bullet and use 2FA. We'd want to use https://dxr.mozilla.org/comm-central/source/mailnews/base/util/OAuth2.jsm or https://dxr.mozilla.org/comm-central/source/mail/base/modules/oauth.jsm
Depends on: 1645217
Attached patch gtalk-oauth.diff (obsolete) — Splinter Review

This adds support for using OAuth to log into GTalk, which is necessary for newer accounts / accounts that have 2FA / probably other situations. (I've had a GTalk account configured forever which works fine with an app password, I'm able to use that app password on another profile, but my other accounts need to use OAuth it seems.)

I'm not 100% sure all the changes to OAuth2Providers.jsm are wanted (do we really want to ask for GTalk when setting up a mail account? I just matched what we did for CardDAV/CalDav). Not sure if that file needs a separate review or not. GTalk does handle using a bare name (e.g. clokep or clokep@gmail.com) while I think the other protocols require the full email? So it is possible that the password manager would end up with separate OAuth tokens anyway, I don't think this is a huge deal.

I also documented the way the XMPP authentication mechanisms work since I hate reverse engineering that each time.

One oddity with this is that if you take too long to finish the OAuth flow then it will cause the account to disconnect, but will attempt to reconnect in the background. So once you finish it will eventually connect properly, but it might take a few tries. (Note that you won't get multiple OAuth windows though.) Any thought on what to do here?

Assignee: nobody → clokep
Status: NEW → ASSIGNED
Attachment #9242847 - Flags: review?(martin)
Comment on attachment 9242847 [details] [diff] [review] gtalk-oauth.diff Review of attachment 9242847 [details] [diff] [review]: ----------------------------------------------------------------- > Not sure if that file needs a separate review or not. I don't feel like I have the authority to approve the scope request change for new gmail accounts. > GTalk does handle using a bare name I think trying to normalize the username would make sense to me. Though discovering the existing entry doesn't seem to work at all for me, unless I get prompted again for a scope extension (which I shouldn't right now, from how I understand the code), which I think would honestly be the best way to handle it. But not sure how easy that is to implement in our OAuth infrastructure. > So it is possible that the password manager would end up with separate OAuth tokens anyway, I don't think this is a huge deal. Not sure how google handles multiple authorizations for the same OAuth app, but I'd assume it'd at least be confusing to the user if they are trying to manage their authorized applications. > One oddity with this is that if you take too long to finish the OAuth flow then it will cause the account to disconnect, but will attempt to reconnect in the background. So once you finish it will eventually connect properly, but it might take a few tries. (Note that you won't get multiple OAuth windows though.) Any thought on what to do here? That sounds like we're not responding to pings on the socket or similar during auth? ::: chat/protocols/gtalk/gtalk.jsm @@ +52,5 @@ > } > } > > +// OAuth2 is an authentication mechanism that allows the client to never receive > +// the password from the user. The logs into the website directly and the client > The logs into This logs into? The user logs into? @@ +58,5 @@ > +// > +// This is required for use with accounts which have two-factor authentication > +// configured (without app passwords) and maybe other configurations. > +// > +// See https://developers.google.com/talk/jep_extensions/oauth This is a 404 for me, but so are other talk docs. Did they finally get nuked? @@ +94,5 @@ > + // Create a new promise which fires when the OAuth callback finishes. > + let stanza = yield new Promise((resolve, reject) => { > + oauth.connect(resolve, reject, true); > + }).then(() => { > + // Login was successful, store the information for subsequence logins. *subsequent @@ +96,5 @@ > + oauth.connect(resolve, reject, true); > + }).then(() => { > + // Login was successful, store the information for subsequence logins. > + if (isNew) { > + console.log(`Saving refresh token for ${aUsername}`); I guess we have no reference to the account here to log to it? @@ +117,5 @@ > + } > + } > + > + // Generate the stanza for login. > + let key = btoa("\0" + aUsername + "\0" + oauth.accessToken); Do we have to handle the access token expiring? @@ +154,5 @@ > this._jid.domain = "gmail.com"; > this.authMechanisms = { PLAIN: PlainFullBindAuth }; > } > > + // If the account has no password configured, use use OAuth to authenticate. use use ::: chat/protocols/xmpp/xmpp-authmechs.jsm @@ +12,5 @@ > +// * send: The next XML stanza to send. > +// * log: The plaintext content to log (instead of the stanza, which likely > +// contains sensitive information). > +// > +// Each time the generator is yielded it is sent the response stanza from the Each time the generator yields it gets resumed with the response stanza...?
Attachment #9242847 - Flags: review?(martin) → review-

Thank you for the review, sorry for the slow response. Anything I just deleted from your previous comment was simply taken care of and will be included in the next version of the patch. Please see below for some comments / things I'm unsure about!

(In reply to Martin Giger [:freaktechnik] from comment #4)

I don't feel like I have the authority to approve the scope request change
for new gmail accounts.

Once we're happy with the chat pieces I'll ask Geoff or Magnus.

GTalk does handle using a bare name

I think trying to normalize the username would make sense to me. Though
discovering the existing entry doesn't seem to work at all for me, unless I
get prompted again for a scope extension (which I shouldn't right now, from
how I understand the code), which I think would honestly be the best way to
handle it. But not sure how easy that is to implement in our OAuth
infrastructure.

I'm not sure I understand -- are you suggesting normalizing to clokep@gmail.com if we're given just the bare clokep? It is a little bit weird that you can end up typing jibberish into the account manager and then logging into an account (if you provide a valid account during OAuth). I'm not sure if there's anything we can do to improve that though (or what Thunderbird does for email in that case).

So it is possible that the password manager would end up with separate OAuth tokens anyway, I don't think this is a huge deal.

Not sure how google handles multiple authorizations for the same OAuth app,
but I'd assume it'd at least be confusing to the user if they are trying to
manage their authorized applications.

Correct, but this can already happen with Thunderbird if you setup a calendar first, then an email account (or have legacy stuff setup for some reason).

One oddity with this is that if you take too long to finish the OAuth flow then it will cause the account to disconnect, but will attempt to reconnect in the background. So once you finish it will eventually connect properly, but it might take a few tries. (Note that you won't get multiple OAuth windows though.) Any thought on what to do here?

That sounds like we're not responding to pings on the socket or similar
during auth?

We do not receive pings during it (and it seems that sending them is invalid). Looking through various XEPs and RFCs it seems you're not allowed to send whitespace during SASL negotiation (see https://datatracker.ietf.org/doc/html/rfc6120#section-4.6.1), so I see two options:

  1. Try to store some data somewhere such that the automatic retry doesn't happen after a timeout; then manually call connect() when the OAuth negotiation is complete (checking if the account has disconnected).
  2. Always disconnect the account when bringing up the OAuth window, then reconnect (which will push us into the "we have an access token already" flow).

Both of these are complicated by not really having access to the account object, but we can overcome that (I think).

@@ +58,5 @@

+// See https://developers.google.com/talk/jep_extensions/oauth

This is a 404 for me, but so are other talk docs. Did they finally get nuked?

Yes, I've been using the archive.org stored version: https://web.archive.org/web/20201222012950/https://developers.google.com/talk/jep_extensions/oauth
Would you prefer I link to this version instead?

@@ +96,5 @@

console.log(Saving refresh token for ${aUsername});

I guess we have no reference to the account here to log to it?

Right, this should be returned with the stanza. I'll make that change.

@@ +117,5 @@

  • // Generate the stanza for login.
  • let key = btoa("\0" + aUsername + "\0" + oauth.accessToken);

Do we have to handle the access token expiring?

I believe this is handled for us by setting oauth.refreshToken higher up. And then during "connect()" it exchanges that for a new access token? I was able to login using this with an account I haven't touched since putting up this patch.

(In reply to Patrick Cloke [:clokep] from comment #5)

I'm not sure I understand -- are you suggesting normalizing to clokep@gmail.com if we're given just the bare clokep? It is a little bit weird that you can end up typing jibberish into the account manager and then logging into an account (if you provide a valid account during OAuth). I'm not sure if there's anything we can do to improve that though (or what Thunderbird does for email in that case).

I think the "proper" solution will be to restructure the way account setup works to build the account name from the result of OAuth. This also applies for example to Matrix.

Yes, I've been using the archive.org stored version: https://web.archive.org/web/20201222012950/https://developers.google.com/talk/jep_extensions/oauth
Would you prefer I link to this version instead?

I think it would be enough to have something in the comment indicate that you should look for an archived version of that.

I believe this is handled for us by setting oauth.refreshToken higher up. And then during "connect()" it exchanges that for a new access token? I was able to login using this with an account I haven't touched since putting up this patch.

Okay, so it's doing the whole "kinda" expiring thing, where as long as you keep your socket alive, you're good. I guess worst case if it forces a disconnect when the token expires auto reconnect could just handle it.

I wonder if we should always default to OAuth2, per bug 1757713.

Attachment #9242847 - Attachment is obsolete: true

Pushed by alessandro@thunderbird.net:
https://hg.mozilla.org/comm-central/rev/9867f448e4fa
Support logging into GTalk with OAuth. r=freaktechnik

Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → 101 Branch
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: