1030650 - Implement x-auto-login

Status: RESOLVED INCOMPLETE

(Firefox for Android Graveyard :: General, enhancement)

Description

[Jozsef Fejes]

The stock browser supports the following: when you visit a page that needs Google authentication, it can log in automatically with the Google account that you are using for your phone (yellow bar on top of the screen). I would like to suggest for Firefox to support the same.

The user base is probably hundreds of millions, we're talking about Firefox for Android, and probably a majority of the phone owners are logged in with their Google account.

Not only Google products use Google authentication. Endomondo, Stack Exchange sites, etc. So there's a great user base and also a great number of sites where this feature could be used.

Typing in passwords on a phone is clumsy, but the accounts are already stored nicely and securely by the Android OS. So this would be a small but useful and convenient enhancement for many users.

This is especially important when you have 2-step verification enabled. It's not the simplest of things to log in when you are using the browser and Google authenticator on the same phone, you have to switch apps, remember the code, and a one time password is only valid for a few seconds.

Comment 1

[Gian-Carlo Pascutto [:gcp]]

Relevant link:
http://eatdev.tumblr.com/post/40358289314/google-autologin-in-an-android-webview

Comment 2

[Gian-Carlo Pascutto [:gcp]]

https://android.googlesource.com/platform/packages/apps/Browser/+/master/src/com/android/browser/DeviceAccountLogin.java

http://developer.android.com/reference/android/accounts/AccountManager.html#getAuthToken%28android.accounts.Account,%20java.lang.String,%20android.os.Bundle,%20android.app.Activity,%20android.accounts.AccountManagerCallback%3Candroid.os.Bundle%3E,%20android.os.Handler%29

Comment 3

[Richard Newman [:rnewman]]

This seems like a reasonable thing to do, assuming that (a) we can partition it correctly so we still work fine on AOSP devices, (b) it doesn't come with a permissions bump, (c) the UX has our usual finesse.

Comment 4

[Gian-Carlo Pascutto [:gcp]]

As for (b), it needs http://developer.android.com/reference/android/Manifest.permission.html#USE_CREDENTIALS.

Comment 5

[Nick Alexander :nalexander [he/him]]

Jozsef, this is neat functionality, thanks for suggesting it.

A quick read suggest that this is really non-standard, however.  I think the relevant line for the WebView -> content direction is line 90 of

https://android.googlesource.com/platform/packages/apps/Browser/+/012dfef735bde3adda165e5add78b61c1a9862c3/src/com/android/browser/DeviceAccountLogin.java

which will perform just the correct redirect (for Google accounts?).  We can certainly do something like that.  The tricky bit is the plumbing for content -> WebView, namely the "onReceivedLoginRequest" message, which looks like a semi-standard approach based on a header, X-Auto-Login, that Google must use as part of their redirect flow:

https://groups.google.com/a/chromium.org/forum/#!topic/chromium-reviews/TjFG2yO24ys

In any case, this is a neat idea.  I wonder what it would look like for Firefox Accounts...

Comment 6

[Nick Alexander :nalexander [he/him]]

(In reply to Gian-Carlo Pascutto [:gcp] from comment #4)
> As for (b), it needs
> http://developer.android.com/reference/android/Manifest.permission.
> html#USE_CREDENTIALS.

Which we already request (for Sync and other things).

Comment 8

[Axel Nennker]

Attachment: x-auto-login.patch

This patch adds an observer for the x-auto-login header and parses it's value.
Currently it fails to send the value of the header over to the Android side.
The java side is implemented too (but untested because the value never reaches it).
I tried to follow the messaging conventions by stealing code from the contacts handling...

Comment 9

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 8516678 [details] [diff] [review]
x-auto-login.patch

The "malloc_usable_size_const_ptr" part of the patch might be better as a separate patch/bug.

You might have forgotten to "hg add" DeviceLogin.jsm and DeviceLogin.java

Comment 10

[Axel Nennker]

Attachment: x-auto-login.patch

Removed the patch portion for https://bugzilla.mozilla.org/show_bug.cgi?id=1083116 which not related to this patch.
hg add-ed DeviceLogin.jsm and DeviceLogin.java

Comment 11

[Axel Nennker]

Things to do:
- add http listener/observer only if local account system like Android/AccountManager is available.
- UI 
  - ask for user consent and continue stopped channel if user denied or an error occured.
    This might lead to duplicate prompts if the account system and/or the identity provider prompt the user too.
  - select one of multiple accounts if more than one is available on the device (take first one for now)
  - implement "remember-my-decision" checkbox

- talk to Google's identity people about standardizing this

Comment 12

[Nick Alexander :nalexander [he/him]]

Comment on attachment 8516688 [details] [diff] [review]
x-auto-login.patch

Review of attachment 8516688 [details] [diff] [review]:
-----------------------------------------------------------------

I see some good things here but this needs:

1) comments about the role of the main objects (DeviceLogin in both JS and Java);
2) a thorough explanation of the expected messaging flow between JS and Java (in DeviceLogin.jsm);
3) unused cruft and debug code removed.

I've given some suggestions for how to structure the JS.  This is really quite interesting, and I want to see it implemented and how it works, so I'm going to apply locally and see if I can figure it out.  I'll update the ticket with any progress I make; in the meantime, try to follow my suggestions and feel free to ask questions.

Thanks for pushing on this and posting these impressive first steps!

::: mobile/android/base/BrowserApp.java
@@ +1579,4 @@
>                  bringToFrontIntent.setClassName(AppConstants.ANDROID_PACKAGE_NAME, AppConstants.BROWSER_INTENT_CLASS_NAME);
>                  bringToFrontIntent.setFlags(Intent.FLAG_ACTIVITY_REORDER_TO_FRONT);
>                  startActivity(bringToFrontIntent);
> +            } else if (event.equals("Android:DeviceLogin:GetWebToken")) {

What is the difference between these messages?  This just gets dropped.

::: mobile/android/base/GeckoApp.java
@@ +208,4 @@
>      private static final String RESTARTER_ACTION = "org.mozilla.gecko.restart";
>      private static final String RESTARTER_CLASS = "org.mozilla.gecko.Restarter";
>  
> +    private static DeviceLogin mDeviceLogin = null;

Why?  This appears unused.

::: mobile/android/chrome/content/browser.js
@@ +204,5 @@
>  // the "debug" priority and a log tag.
>  let dump = Log.d.bind(null, "Browser");
>  
> +try {
> +  Cu.import("resource://gre/modules/DeviceLogin.jsm", {});

Make this a lazy loaded module defined earlier in the file.  Call DeviceLogin.init() late in the load process in browser.js at [1].  Basically, do exactly what WebcompatReporter does :)

http://mxr.mozilla.org/mozilla-central/source/mobile/android/chrome/content/browser.js#338

::: mobile/android/modules/DeviceLogin.jsm
@@ +7,5 @@
> +const Cu = Components.utils;
> +const Cc = Components.classes;
> +const Ci = Components.interfaces;
> +
> +this.EXPORTED_SYMBOLS = [];

I think you should export DeviceLogin.  Think of this as providing a library that a consumer will use to wire up these device login things.  What would a consumer (browser.js) want?  A way to register, to un-register, for sure; anything else?

@@ +21,5 @@
> +
> +XPCOMUtils.defineLazyServiceGetter(this, "ppmm", "@mozilla.org/parentprocessmessagemanager;1",
> +                                   "nsIMessageListenerManager");
> +
> +let deviceLoginNsIHttpHeaderVisitor = function ( ) {

This is a class, so give it a capitalized name, and use the constructor + prototype approach used elsewhere in modules/.  It's not exposed (in EXPORTED_SYMBOLS) so it doesn't need a particularly unique name, and it's not actually extending any nsIHttpHeaderVisitor so drop that entirely.  So:

function HeaderVisitor() {
}

HeaderVisitor.prototype = {
  ...
}

@@ +25,5 @@
> +let deviceLoginNsIHttpHeaderVisitor = function ( ) {
> +        this._found = null;
> +};
> +deviceLoginNsIHttpHeaderVisitor.prototype.visitHeader = function ( aHeader, aValue ) {
> +        if ((aHeader.indexOf( "X-Auto-Login" ) !== -1) || (aHeader.indexOf( "x-auto-login" ) !== -1)) {

These headers are case insensitive, right?  This is not sufficiently general.

@@ +35,5 @@
> +};
> +
> +let DeviceLogin = {
> +  unregister: function () {
> +    Services.obs.removeObserver( this, "http-on-examine-response" );

Extract these out into a const list of messages observed.

@@ +44,5 @@
> +    log.debug("init");
> +    this._requestMessages = {};
> +
> +    // Add listeners for all messages from DeviceLogin.js
> +    let messages = ["DeviceLogin:GetWebToken"];

Lift this to a const in the class.  Shouldn't you be unregistering these in unregister?

The fact that one message is a substring of the other is unnecessarily confusing.  Choose a name for the web content facing stuff

@@ +50,5 @@
> +      ppmm.addMessageListener(msgName, this);
> +    }.bind(this));
> +
> +    // Add listeners for all messages from DeviceLogin.java
> +    let returnMessages = ["Android:DeviceLogin:GetWebToken:Return:OK", "Android:DeviceLogin:GetWebToken:Return:KO"];

Lift this to a const in the class.  Shouldn't you be unregistering these in unregister?

@@ +56,5 @@
> +    returnMessages.forEach(function(msgName) {
> +      Services.obs.addObserver(this, msgName, false);
> +    }.bind(this));
> +
> +    Services.obs.addObserver(this, "http-on-examine-response", false);

Use the factored list of messages here.

@@ +68,5 @@
> +      log.error("oops " + e);
> +    }
> +  },
> +
> +  _sendMessageToJava: function(aMsg) {

These _sendMessage helpers aren't helping.  We have a standard pattern for this stuff: Messaging.sendRequestForResult.  It makes this Request -> Response ping-ponging simple.

@@ +99,5 @@
> +        aSubject.visitResponseHeaders( aVisitor );
> +        var value = aVisitor.found( );
> +        if (value) {
> +            uri = aSubject.URI;
> +            aSubject.cancel(Components.results.NS_BINDING_ABORTED);

Find a way to factor a helper out of this parsing.  How is this not already implemented somewhere in Gecko?

@@ +167,5 @@
> +      case "DeviceLogin:GetWebToken":
> +        this._sendMessageToJava({type: "Android:DeviceLogin:GetWebToken", data: aMessage.data});
> +        break;
> +
> +      case "DeviceLogin:RegisterForMessages":

This appears unused.

Comment 13

[Nick Alexander :nalexander [he/him]]

(In reply to Nick Alexander :nalexander from comment #12)

This is really
> quite interesting, and I want to see it implemented and how it works, so I'm
> going to apply locally and see if I can figure it out.  I'll update the
> ticket with any progress I make

I've got a version of this mostly working locally; I need to clean cruft before posting.  Tomorrow, I hope.

Comment 14

[Axel Nennker]

Thanks for the review.

Regarding "cruft and debug code": Will do. I am waiting for your working version and then clean up.

DeviceLogin.jsm is modelled after "mobile/android/modules/ContactService.jsm" 
which I thought has a similar call structure js -> java -> js
But there are differences of course: Contacts is content_js -> java -> content_js
while DeviceLogin is observer_js -> java -> observer_js

Regarding your comment: "do things late in startup and exactly like WebcompatReporter does":
 WebcompatReporter is initialized in a DomContentLoaded handler. This is too late for DeviceLogin
 because DeviceLogin is reacting to an HTTP-Header ages before DomContent is touched.

 I think that DeviceLogin.jsm should not export anything because the http observer has to be startet very early and is removed when the browser shuts down.
 There is no API needed. Maybe it should live in mobile/android/components/ ?

Regarding: How is this not already implemented somewhere in Gecko?
 The DeviceLogin observer is called from here:
  http://mxr.mozilla.org/mozilla-central/source/netwerk/protocol/http/nsHttpChannel.cpp#1414
 and parsing of http headers is done in nsHttpChannel too. Like e.g. GetWWWChallenges
  http://mxr.mozilla.org/mozilla-central/source/netwerk/protocol/http/nsHttpChannel.cpp#5153
 DeviceLogin impacts each and every http response: Not sure about js versus C++ performance these days... (on Android for now)

I did not touch every review comment in this reply. I will go through the list when the messaging to and from java works.

Thanks again. 


T(In reply to Nick Alexander :nalexander from comment #12)

Comment 15

[Nick Alexander :nalexander [he/him]]

Attachment: Try to act on X-Auto-Login header in Firefox for Android.

This:

* listens for the non-standard X-Auto-Login header;
* shuttles the header, which is form encoded, to Java;
* extracts the "realm" from the header;
* identifies Android Accounts with type the realm;
* requests an Android auth token of type "args" from the header;
* returns the resulting URL to JavaScript;
* tries to redirect to said URL.

Unfortunately, this approach is no longer viable.  I read what Axel
and others have written (I like
http://nelenkov.blogspot.ca/2012/11/sso-using-account-manager.html);
did a good deal of digging into what the stock Android browser
("Internet") and Google Chrome do; and read the original security
exploit presentations and (public) bugs.

I conclude that Google has disabled this approach: "weblogin:" tokens
no longer include a valid "uberauth" parameter, *except* when the
requesting App is Google Chrome.  (I expect, in fact, that Google's
Authenticator is either white-listing App IDs and or Google-signing
keys or both, but the Authenticator is not open source and I don't
care to reverse engineer further).

I'm posting this patch so that Axel can see how I arranged the code,
and can test locally.

Comment 16

[Jozsef Fejes]

Is there any other viable approach? I can't login to any Google property on my Galaxy Nexus phone in Firefox. I have 2-factor auth enabled. When the login form asks for the TOTP code, I switch to Google Authenticator, try to memorize the digits, and when I switch back to Firefox, either Firefox reloads the page and I get back to square one, or I don't have enough time to type the TOTP code. This is really bad.

Comment 17

[Nick Alexander :nalexander [he/him]]

(In reply to Jozsef Fejes from comment #16)
> Is there any other viable approach? I can't login to any Google property on
> my Galaxy Nexus phone in Firefox. I have 2-factor auth enabled. When the
> login form asks for the TOTP code, I switch to Google Authenticator, try to
> memorize the digits, and when I switch back to Firefox, either Firefox
> reloads the page and I get back to square one, or I don't have enough time
> to type the TOTP code. This is really bad.

Can you not copy/paste with the Google Authenticator?  A Galaxy Nexus is a device old enough that Gecko might be killed as soon as you load something else, which is unfortunate :/

There are other things that we can do, but they're specific to Google Authenticator.  IIRC, GA can be used by Android Apps in a generic way.  I'd explore writing an add-on that does the GA dance in the background and fills a form element or something like that as one way forward.

We could support this flow in Firefox: that is, it's technically possible; but it's totally non-standard and not well aligned with any identity/web login approach.  I'd investigate writing an add-on that uses the GA system, if possible.

Comment 18

[Axel Nennker]

That's all quite anoying.

Would be good to know why Google is doing this in this way.

I think that Google Authenticator etc are beside the point.
I want Firefox to be as priviledged and capable as Chrome.
With this patch Fennec is capable but not priviledged by Google's account system to provide a smooth UX like Chrome.

Another step to use x-auto-login would be with an FxAccount and an FxAccountAuthenticator.java that provides webtokens for Sync, MDN, Bugzilla...
http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/sync/setup/SyncAuthenticatorService.java#175
http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/fxa/authenticator/FxAccountAuthenticator.java#68

Comment 19

[Nick Alexander :nalexander [he/him]]

(In reply to Axel Nennker from comment #18)
> That's all quite anoying.

'tis.  I spent about two days digging into this, 'cuz it's a neat feature that I'd like to see get adopted.

> Would be good to know why Google is doing this in this way.

I'm speculating here, but I think "early Android" had a vision for Accounts and Authenticators that essentially hasn't come to fruition.  They got part way towards building native oauth-like flows into the OS, and then ran in to some legitimately difficult problems.  In this instance, they exposed a way for any App to request an oauth token.  The UI displaying the oauth grant request to the user is laughable; it's clearly not OK to allow the user to share their tokens through that UI.  So Google had to do something; they couldn't break their own software; and they couldn't rev the OS on most handsets; so they whitelisted the token grant.

For the record, you can see the details of (what I believe to be) the original attack at http://www.slideshare.net/duosecurity/bypassing-strong-auth-with-passwords-passwords13-26280672

> I think that Google Authenticator etc are beside the point.

Perhaps, but GA is probably the best Authenticator currently written.  They couldn't get it right:  The Android UI for OKing and reviewing grants does not meet security needs.

> I want Firefox to be as priviledged and capable as Chrome.
> With this patch Fennec is capable but not priviledged by Google's account
> system to provide a smooth UX like Chrome.

I see no path forward here other than having Google Authenticator bless org.mozilla.firefox and friends.  And then who's the next legitimate App who needs to be blessed?  Of course, I'm speculating about the whitelisting -- perhaps I'm missing something?

> Another step to use x-auto-login would be with an FxAccount and an
> FxAccountAuthenticator.java that provides webtokens for Sync, MDN,
> Bugzilla...
> http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/sync/setup/
> SyncAuthenticatorService.java#175
> http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/fxa/
> authenticator/FxAccountAuthenticator.java#68

Yes, we certainly could arrange this.  However, we'd have to build it; we'd have the same security issues Google has; we'd have the same dangerous UI; and at the end of the day there are very few sites to log in to at this time.

If we can't participate in the Google ecosystem, I don't think it's worth trying to push this approach :(

Comment 20

[Mark Finkle (:mfinkle) (use needinfo?)]

Re-summarizing to reflect the work

Comment 21

[Axel Nennker]

I talked to two Googlers and got the impression that they would like to help here. They promised to evaluated options how to make this possible. Of course they have to evaluatate what it would mean to allow Fennec to get tokens from Google's authenticator.

Maybe Mozillians could too find out what it would mean if users had a Mozilla (?) account on Android and the respective Authenticator would issue weblogin tokens not only to Fennec but to e.g. Chrome.
Imagine a Chrome user on Android visiting https://account.services.mozilla.com/ and being offered to login using the device account. 
http://mxr.mozilla.org/mozilla-central/source/mobile/android/base/fxa/authenticator/FxAccountAuthenticator.java#68

Maybe the user should manage the list of trusted apps that are allowed to get weblogin tokens...

Comment 22

[Nick Alexander :nalexander [he/him]]

Not the same technology, but the same idea: Bug 1158869.  Axel, why I'm here, do you have any updates on this approach?  I am ever hopeful...

Comment 23

[Axel Nennker]

I chatted with someone from Google who told me that they are about to replace x-auto-login.
In fact he thought that the code was gone from Chromium already..
He then said that he would create an issue in Chromium's bug tracker to track the removal.

Well, the code is still there and no new issue.
https://code.google.com/p/chromium/codesearch#chromium/src/components/auto_login_parser/auto_login_parser.cc&q=x-auto-login&sq=package:chromium&l=19
https://chromium.googlesource.com/chromium/src.git/+/master/components/auto_login_parser/auto_login_parser.cc

When I curl/wget https://accounts.google.com/ the x-auto-login http header is still there.

I could only guess what is going on inside of Google.

Regarding Bug 1158869: There are a ton of ways to specify this or similar login related features.
The methods vary on who has to adapt most or who has the most work. And of course who controls the identities.

X-auto-login is one way to transmit the relyingparty/service login-requirements to the client/browser and in the end to the identity provider.
By using an HTTP header this method relieves the services developers from handling most of the login-mechanics. And shifts the work to the service operators. 
Which is nice if a company has many services because then identity "work" is done by e.g. @mozservices and not so much by e.g. a service bugzilla.mozilla.org
Google falls in this category too. Many services and one accounts.google.com

An "Identity@Mozilla" workshop would help, I guess. 
Bringing together Fennec, Firefox, Marketplace, FirefoxOS and MozServices people.

Comment 24

[Axel Nennker]

FYI: Auto login code finally removed from chrome.

https://codereview.chromium.org/1305233002/

Comment 25

[BMO Automation]

We have completed our launch of our new Firefox on Android. The development of the new versions use GitHub for issue tracking. If the bug report still reproduces in a current version of [Firefox on Android nightly](https://play.google.com/store/apps/details?id=org.mozilla.fenix) an issue can be reported at the [Fenix GitHub project](https://github.com/mozilla-mobile/fenix/). If you want to discuss your report please use [Mozilla's chat](https://wiki.mozilla.org/Matrix#Connect_to_Matrix) server https://chat.mozilla.org and join the [#fenix](https://chat.mozilla.org/#/room/#fenix:mozilla.org) channel.