1059100 - [email] Implement XOAUTH2 authentication support for gmail

Status: VERIFIED FIXED

(Firefox OS Graveyard :: Gaia::E-Mail, defect)

Description

[Andrew Sutherland [:asuth] (he/him)]

We already wanted to implement XOAuth2 for gmail, but now we *really* have to since Google requires a specific setting to be enabled to allow mail clients to connect via standard username/password (if an application-specific password is not in use).  An authentication failure will occur when trying to create the account that generates an error message that we don't correctly detect.  (We may need to spin-off a related fix for that for branches where we can't take the XOAuth2 changes.)

Context:
- Google's original sorta-nebulous announcement about this:
http://googleonlinesecurity.blogspot.de/2014/04/new-security-measures-will-affect-older.html

- The help page they link to when this happens:
https://support.google.com/accounts/answer/6010255?hl=en

- A post that provides some more context:
http://www.ghacks.net/2014/07/21/gmail-starts-block-less-secure-apps-enable-access/

- :jcranmer's post to the imap-protocol list about federation/standards concerns (note: discussion crosses months):
http://mailman13.u.washington.edu/pipermail/imap-protocol/2014-April/002243.html

- The start of the Thunderbird tb-planning discussion on this (note: discussion also crosses months):
https://mail.mozilla.org/pipermail/tb-planning/2014-April/003236.html

Comment 1

[Andrew Sutherland [:asuth] (he/him)]

- Google's IMAP/OAuth pages are rooted here:
https://developers.google.com/gmail/oauth_overview

relatedly, hotmail.com/outlook.com apparently also support OAuth2 now, so we'll want to look into that too, although that also depends on us supporting their IMAP implementation.  (Bug 958614 which will require more than just the change to email.js because of apparent semantics differences with SINCE/BEFORE)

Comment 2

[Dylan Oliver [:doliver]]

[Blocking Requested - why for this release]: 

Per comment #1, without changing our authentication model for Gmail accounts, many users may be unable to figure out how to successfully connect to their email account. 

We can target 2.1 sprint 4 to deliver this. New strings would likely be included in this feature.

Comment 3

[Tony Chung [:tchung]]

QA did some testing in the current scenario against 2.1 builds.   here's the summary rundown:

WORKS: new gmail account, no 2-step auth   
FAILS: new gmail account, yes 2-step auth
FAILS: old gmail account, no 2-step auth
FAILS: old gmail account, yes 2-step auth

This was tested against IMAP.  not yet pop3 or smtp.

More testing notes at: https://etherpad.mozilla.org/gmail-auth.

Comment 4

[Andrew Sutherland [:asuth] (he/him)]

Attachment: proposed v2.0 specific mitigation

Comment 5

[Andrew Sutherland [:asuth] (he/him)]

Attachment: here's what that mitigation looks like

Example of the 2.0 mitigation flow *without any new strings* implemented by my patch.

Comment 6

[James Burke [:jrburke]]

Attachment: hack-flows.png

This is a hacked up flow I have in a branch that does an oauth jump for Gmail. At least for the positive/success path, I think we could get away without having new strings in the UI. What I am unsure about is:

* that "Firefox OS" string on the gmail page
* Any error cases that might need special strings

Comment 7

[Dylan Oliver [:doliver]]

(In reply to James Burke [:jrburke] from comment #6)
> * that "Firefox OS" string on the gmail page

That string is configured in the Google API console so we can change it out of band to say whatever we want (although it is not localizable in any way that I know of). In the other apps (Calendar & Contacts) we do set our product name to "Mozilla Firefox OS" for display there. (Ultimately OEMs will create their own oauth configs and they'll get to choose what goes there.)

Comment 8

[Candice Serran (:cserran)]

Since this is a new feature based on the regressed google behavior, updating the feature flag for late uplift and removing blocking flag. Team has mitigation and implementation plan in place. Targeting to land this week

Comment 9

[Harly Hsu[:harly]]

Attachment: [2.1 Email] Account Login_Draft.pdf

Attaching a draft UX spec to get us started.

Comment 11

[Andrew Sutherland [:asuth] (he/him)]

Attachment: GELAM pull request with mcav's oauth work and asuth's autoconfig and integration work

I integrated all the things.  Here's the GELAM one.  This combines:

- My changes to split out the autoconfig bits where we figure out what we want to try and do from the actual account creation process.  The ActiveSync bits turned out to be somewhat annoying, but as an upside, I think I discovered that our AutoDiscover was probably never ever working because of an XML namespace thing and have now fixed that.

  - The autoconfig stuff depends on a change to mail-fakeservers so that we can actually simulate AutoDiscover (which is how I discovered the sadness).  This was important to my confidence in this change.  The pull request for that is at https://github.com/mozilla-b2g/mail-fakeservers/pull/19 but you will find that I merged it already as a=test-only and released the 0.0.25 npm release.  The rationale for that is that no one but us uses mail-fakeservers and we're not going to upgrade the version we use except as part of a review.  This way the travis tests can run and pass and "make post-tests" won't screw-up either.

  - The activesync changes depend on a minor change to jsas.  The pull request is up at https://github.com/mozilla-b2g/jsas/pull/19.  I also pushed the branch into the mozilla-b2g/jsas repo (but did not merge it!) so that in my GELAM commit I could reference that hash.

- mcav's excellent OAuth2 work.  Major props on the thorough test coverage that's not skimping on making sure things work right.  I like the slog.intercept stuff a lot along with the use of mustLog; I feel sorta bad that my autoconfig updated tests aren't as fancy!  However, that's got a clear functional-style so maybe it's not needed.  I'm looking forward to the future when we can clean that all up a bit more and have some huge comment blocks with rationale and best practices ;)

- I cherry-picked mcav's oauth stuff on top of my autoconfig stuff then did all the integration work on top of that.  This mainly entailed plumbing through all the configuration information and state.  I de-hardcoded gmail and ended up just putting all the oauth2 config stuff inside credentials.oauth2.


note, r=asuth for the mcav-authored bits.  Still need mcav to review my bits back.

Comment 12

[Andrew Sutherland [:asuth] (he/him)]

Attachment: activesync jsas review

Comment 13

[Andrew Sutherland [:asuth] (he/him)]

Attachment: gaia review

Here's the gaia pull request.  Since I just built on top of :jrburke's branch, I pushed my integration work to that branch in his repo and issued the pull request from there.  I pushed one commit for the gaia work and one commit for the gelam install-into-gaia stuff so it can be ignored for review purposes, etc.

The notable aspects of my changes were:

- I removed some of the generic-ish support in fetch.js.  The GMail oauth2 stuff is pretty stock OAuth2, or at least the Microsoft OAuth2 stuff does nearly exactly the same thing, so the parametrization/indirection did not seem necessary.

- I've split out the constants/etc. as such:
  - Our local autoconfig files for gmail specify the (arbitrary) endpoint URLs and the scope.  It also provides an arbitrary secretGroup "name".
  - The secretGroup keys into the values in apps/email/build.json that can be overridden by distribution mechanisms.  The secrets are just the client_id and client_secret which are all that partners should change in their builds.

- I hooked up the oauth stuff to the reauthentication mechanism.  I tested this.

- I changed fetch.js sorta appropriately with that.  There's some obvious plumbing changes there, I tried to update comments and elaborate as needed.

- I made the "username and password" settings screen hide the password box and the "save" button based on the MailAccount.authMechanism.  Arguably the "username and password" string should maybe change or something, but I don't really care.

- I screwed up and left _hackLearnAboutAccount in setup_progress.js but don't don't want to do the rebase dance right now to fix that.

Comment 14

[Andrew Sutherland [:asuth] (he/him)]

Attachment: sideloadable email-oauth app zippy thing a la https://github.com/jrburke/gaia-dev-zip

Here's the zip if anyone wants to play without making their own build.  I haven't actually tried this specific side-loady thing, but presumably it should work.  Certainly it's more fun than doing a "make reset-gaia" to get the email app on there.  Because there is a manifest change, you need to either do a reset-gaia thing or a sideload thing or trigger an explicit reinstall/etc. to get the manifest happy.

What I've tested:
- creating an account using oauth using:
  - one of my gmail accounts (direct local autoconfig)
  - the doliver.net gmail account (autoconfig by way of mx lookup)
- explicitly revoking access to my gmail test account to test re-auth.

What I haven't tested but want to test:
- Create an ActiveSync account using AutoDiscover.  Unfortunately this may be harder to test because our QA accounts all use "onmicrosoft.com" which mx resolves to a root of outlook.com which will immediately bounce to our explicitly known m.hotmail.com ActiveSync endpoint, bypassing AutoDiscover.


Review-wise, I should note that I'm basically at r=asuth for James' gaia stuff, but I did skim some things, and in particular I'm concerned about us permanently leaving a progress card up in the card stack through our initial use of setup_progress.  Some strings/etc. may need some investigation too.


But in general I think we're pretty much there and hopefully we can get UX sign-off during their Tuesday morning.

Comment 15

[Jayme Mercado [:JMercado] (Inactive 3/4/2016)]

I have redone the tests performed by Oliver on the latest 2.1 build using the Email app in comment 14.  The user was able to log into all accounts with and without 2-step authorization.  Accounts used, 2 accounts made on March 29th or earlier and 2 accounts made today.

Environmental Variables:
Device: Flame 2.1
BuildID: 20140908022757
Gaia: e7ac3a51932f7f7a5b5a6935dcaad1343b7c5fa5
Gecko: d1b97cc46b5a
Version: 34.0a2 (2.1) 
Firmware Version: v123
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Comment 16

[Andrew Sutherland [:asuth] (he/him)]

Comment on attachment 8485673 [details] [review]
gaia review

r=asuth on the :jrburke-authored code. James, ideally you can reverse sign-off on my bits in here.  Note that if you want to r+ my GELAM parts you can do that too if you feel sufficiently comfortable with the code or the results ;)  Note that there may still be more changes to come and we'll sign off as appropriate.

Comment 17

[James Burke [:jrburke]]

Attachment: email-oauth.zip

We did some more work on some edge cases and tightening down the code. It should be good enough to get formal ui-review on it now.

This attachment is a zip file that can be sideloaded via:
https://github.com/jrburke/gaia-dev-zip#for-the-ux-person

Asking Harly for review since he did the ux spec. I am not including screenshots because with the input field focusing and the oauth window dialog display, it is best to do this final review by observing the behaviors in-app.

Suggested things to try:

* A gmail account, for the oauth experience
* A non-gmail account for the password experience
* The manual setup flow (you can go to settings for the non-gmail password case and use those settings if you need a set).

Also check under the account settings, after the account has been configured, the "Username" (for oauth accounts) and "Username and password" (for password accounts) sub-sections.

Comment 18

[Harly Hsu[:harly]]

Comment on attachment 8486217 [details]
email-oauth.zip

Juwei, could you do the review? Thanks.

Comment 19

[Juwei Huang[:juwei]]

Hi James,
It looks overall great! Thanks :)
Yet the patch seems have some issues:

1) Harly & I just found that sometimes when we login to Google account and accept Firefox OS, the screen jumps back to "New account" page. I have to tap on "Next" and "Accept" like 3 times to enter the next step. I believe there is a bug causes the issue.

2) When I try to login the email account I've used to login in my device before, the screen will jump directly to the authorization page without asking the password....
It may cause some privacy issues. Could you help to figure out why?

Thank you!

Comment 20

[James Burke [:jrburke]]

1) I will try to reproduce today. The only thing I can think of is if the tap happens just to the left of the Accept button on the google-hosted page, that the cancel or negative option button might be receiving the tap and that triggers the cancel patch for us? I will definitely try more today. Maybe we have an issue with the return response though. If I cannot reproduce, I will likely update the app to include a bit more logging then ask you to reproduce with that build.

2) If you are adding an account, then deleting it in the email account, then adding that same account again, then I can see that behavior occurring.

After the first login, the authentication cookies that google uses are stored in the cookie jar for the browser that is used for the email app. So further jumps to google for the same account will show this behavior.

However, if you add a second, different gmail account, it should prompt you for the password for that second account. Note that after successful login with that second account, depending on how Google manages its cookies, you will not get a prompt either for this second account when doing the add account, remove account, add account flow.

This does bring up a good point that if the user deleted their account, we should clear those cookies or ask google to always prompt for auth. If the user deletes their account and gives their phone to someone else, that person should not be able to create an account using their past credentials.

Dev notes: there is approval_prompt=force for the oauth API with google, but that seems to just indicate always showing the Allow prompt, not about asking the user to reauthenticate. We might want to pass that anyway as other gaia apps do, and may help guarantee a new refresh_token. Although those notes were in the "Web Server" oauth2 pathway. At the very least, we should just log at the end of the oauth jump if we got an access_token and a refresh_token, for logcat debugging purposes.

Comment 21

[Marcus Cavanaugh [:mcav]]

Comment on attachment 8485669 [details] [review]
GELAM pull request with mcav's oauth work and asuth's autoconfig and integration work

Looks good. The GELAM script-runner stuff should prove useful, nice touch!

Comment 22

[James Burke [:jrburke]]

Attachment: email-oauth-2.zip

This app zip fixes the two issues Juwei reported:

1) The Allow button going back to account setup screen: we had a race condition with some new code we added yesterday, fixed now.

2) Maintaining the user as logged in on google between account setups. We now ask google to not keep around the auth info. This results in the user needing to enter their password any time they jump over to the google site.

Comment 23

[James Burke [:jrburke]]

Comment on attachment 8485673 [details] [review]
gaia review

r+ for the non-me parts I did in Gaia. 

As a final test, I tested an email app update where before the update a gmail account was configured via ordinary password, and an activesync account was configured. Then I updated the app to this code change, and the accounts continued to work, with the gmail one staying in the password-type, as expected.

Comment 24

[Andrew Sutherland [:asuth] (he/him)]

(Note: :bajaj, :clee, and friends are aware of the l10n-ness and planned uplift of this.  We should have already had this keyword on, though.  Apologies.)

Comment 25

[Juwei Huang[:juwei]]

Comment on attachment 8486796 [details]
email-oauth-2.zip

Looks good! Thanks James!

Comment 26

[Andrew Sutherland [:asuth] (he/him)]

landed on gaia/master:
https://github.com/mozilla-b2g/gaia/pull/23800
https://github.com/mozilla-b2g/gaia/commit/f108c706fae43cd61628babdd9463e7695b2496e

landed on gaia-email-libs-and-more/master:
https://github.com/mozilla-b2g/gaia-email-libs-and-more/pull/336
https://github.com/mozilla-b2g/gaia-email-libs-and-more/commit/403e0cdd87cf4ec921f2811bb02242e7b1682bec

landed on jsas/worker-thread:
https://github.com/mozilla-b2g/jsas/pull/19
https://github.com/mozilla-b2g/jsas/commit/fbdc727926a05901dfea6583cd84a4c1d5f5c371

Note that I did include a minor high-probability (on Travis at lest) intermittent orange fix in GELAM since Travis was unhappy with us without it.  a=test-only on that bit.

Comment 27

[Tony Chung [:tchung]]

hi Jayme, please verifyme that this bug is fixed against 2.1 build tomorrow.  it should get uplifted soon.

Comment 28

[Andrew Sutherland [:asuth] (he/him)]

Comment on attachment 8485673 [details] [review]
gaia review

[Approval Request Comment]
[Bug caused by]: A change in Google behaviour necessitates aggressive adoption of XOAuth2 authentication to avoid requiring the user to enable "less secure apps" settings (new!) or use of an application-specific password (existing).  This is also generally a major security and ease-of-setup win for gmail users, but we're justifying the uplift more on the massive downsides to not uplift.  This has been discussed previously at fancy meetings.
[User impact] if declined: Non-2-factor-auth users with accounts at least one month old who had not been using IMAP with AUTH=PLAIN in the recent past when Google flipped the switch on this around July 15th, 2014 would see an authentication error that implies that their password is wrong.  Their password is not wrong, they have to turn on the less secure apps setting.
[Testing completed]: :jrburke, :doliver, :asuth, :juwei, and :harly have all done various testing of gmail accounts, etc.  We've added non-trivial back-end test coverage as well.
[Risk to taking this patch]: Given the recent branching of v2.1 and the testing we've done with this patch, we're not expecting much risk.  At a lower level, the email libs we use are already used for XOAUTH2 by whiteout.io too.
[String changes made]: 6 Strings Added.  All with Localization Notes!

Note: I verified the cherry-pick of this to 2.1 from 2.2/master happens cleanly and that I am able to use the resulting cherry-picked patch to add an account and sync the account, etc.

Comment 29

[bhavana bajaj [:bajaj]]

I think it will help QA verify as many testcases including error cases to ensure we have the necessary string in before the string freeze on (9/14).

Comment 30

[Tony Chung [:tchung]]

proper flag for warranting bug verification is to use whiteboard == qa-approval

Comment 31

[Ryan VanderMeulen [:RyanVM]]

v2.1: https://github.com/mozilla-b2g/gaia/commit/d61264cd0c1f797b6be11e33524d8d52983c87e4

Comment 32

[Tony Chung [:tchung]]

jayme, please verify this is fixed against 2.1.  thanks!

Comment 33

[Jayme Mercado [:JMercado] (Inactive 3/4/2016)]

This issue is fixed on 2.1 and 2.1 Flame.  The user is able to log in with all security setups on a google account and can still log in just fine with other email services (tested yahoo and hotmail).

Environmental Variables:
Device: Flame 2.1
BuildID: 20140910165554
Gaia: d61264cd0c1f797b6be11e33524d8d52983c87e4
Gecko: 1d44dfce2e5b
Version: 34.0a2 (2.1) 
Firmware Version: v123
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

Environmental Variables:
Device: Flame 2.2
BuildID: 20140911063332
Gaia: e3b9d0d6516177636965d97c63c60981a24a0662
Gecko: 98ea98c8191a
Version: 35.0a1 (2.2) 
Firmware Version: v123
User Agent: Mozilla/5.0 (Mobile; rv:35.0) Gecko/35.0 Firefox/35.0