750568 - Cannot use twitter or google accounts within the desktop runtime - Needs to be whitelisted on origins allowed

Status: VERIFIED FIXED

(Firefox Graveyard :: Webapp Runtime, defect)

Description

[Jason Smith [:jsmith]]

Google and twitter accounts cannot be used within the desktop runtime currently due to the implementation of bug 707836. Logging in with these accounts within an app that redirected to login will send you directly to the browser, making login by these accounts, currently not possible within an application. We need temporarily whitelist the origins to allow account access to twitter and google.

In the long run, we really need to support origin whitelisting for apps, as this is repeatedly becoming problematic (we're putting band-aids on the real problem).

Comment 1

[Jason Smith [:jsmith]]

Example Top

Comment 2

[Jason Smith [:jsmith]]

Example top app: GIS Cloud

Example Steps:

1. Install and launch GIS Cloud
2. Click sign in
3. Select the twitter or google account logo
4. Login with your creds for twitter or google account respectively

Expected:

After a successful login, you should redirect back to GIS Cloud.

Actual:

The browser starts up. Google accounts prompt for a login again, twitter indicates an error.

Comment 3

[Jason Smith [:jsmith]]

For the marketplace beta release, I think we need to define what are allowed_origins for this release carefully, then allow them in the code for now.

Comment 4

[Myk Melez [:myk] [@mykmelez]]

Mardak: is this as simple as adding two more origins to the list of allowed origins?

Jason: do you know the specific origins to which those apps are directing users?

Comment 5

[Jennifer Arguello :ticachica]

To give more information about the opening comment of the bug, this will be a bandaid fix for the Marketplace Beta release. We will need to put more time into developing a solution to this problem. The blocking bug for K9o is bug 749415

Comment 6

[Ed Lee :Mardak]

(In reply to Myk Melez [:myk] [@mykmelez] from comment #4)
> Mardak: is this as simple as adding two more origins to the list of allowed
> origins?
Should be. At least for GIS Cloud, they seem to redirect to..

https://api.twitter.com
https://www.google.com -> https://accounts.google.com

However, there's currently no distinction between whitelisting for popups vs main page navigation.

Comment 7

[Ed Lee :Mardak]

Seems like both www and accounts are needed for Google, but there's no good way to go back from the Google login flow without restarting the app.

Twitter has a cancel button, but I wasn't able to fully test if just api.twitter.com was necessary.

These two flow differ from the Persona and Facebook logins where they use a separate window whereas these use the main window.

Comment 8

[Ed Lee :Mardak]

Attachment: v1

Comment 9

[Ed Lee :Mardak]

This fix doesn't quite fix GIS Cloud twitter login because some reason it redirects to http://www.giscloud.com/app instead of https://www.giscloud.com/app

So that ends up opening the non-https page in the default browser. But this is probably something GIS Cloud needs to fix on their side.

Comment 10

[Myk Melez [:myk] [@mykmelez]]

Comment on attachment 620203 [details] [diff] [review]
v1

Looks good, the obvious fix, and works, provided apps integrate with the identity providers correctly.  We obviously really need a better long-term solution sooner than later, but this, like the similar fix we landed last week, is sufficient in the short term.

Comment 11

[Myk Melez [:myk] [@mykmelez]]

I had a conversation with glandium on IRC about getting approval for this patch to land, concluding that it has the "automatic approval to land" granted to files that are "not part of the build on Windows" <https://wiki.mozilla.org/Tree_Rules#mozilla-central_.28Nightly_channel.29> because it is a JS-only change.

So I landed it with a=js-only, and glandium is going to get edmorley to update the Tree Rules page to explicitly specify a=js-only (and others, like a=chrome-only and a=npotb) in the set of automatic approvals.

https://tbpl.mozilla.org/?noignore=1&rev=b13bfc70bc44

If it landed before the build infrastructure started building Wednesday's nightlies, it'll appear in those nightlies (which may or may not include Windows, given the Windows build bustage that is responsible for the tree restrictions).  Otherwise, it'll appear in Thursday's nightlies.

Comment 12

[Myk Melez [:myk] [@mykmelez]]

The change did land before the build infrastructure started building Wednesday's nightlies, so it'll appear in them.

Comment 13

[Jason Smith [:jsmith]]

On the try build, I was successfully able to login with my google account on GIS Cloud. The twitter account works except for the issue specified, which is known with the app. I'll want to find a test case where twitter is used correctly to make sure it works as expected.

Comment 14

[Jason Smith [:jsmith]]

Nope, this does not work always for twitter. Fails with lanyrd mobile, a top app, since it's using twitter.com as its origin, not api.twitter.com. If you try to login in lanyrd mobile (can be found on marketplace.mozilla.org or https://apps.mozillalabs.com/appdir/?db=db/apps-preview.json), it will redirect to twitter.com to ask for auth. Logging in will allow you access to lanyrd mobile within the browser, not the application itself.

Comment 15

[Ed Lee :Mardak]

Yup. Looks like..

http://lanyrd.com ->
http://lanyrd.com/twitter/signin/ ->
https://twitter.com/oauth/authenticate

Comment 16

[Ed Lee :Mardak]

Attachment: followup v1

Potentially someone could incorrectly link to https://www.twitter.com too which would then redirect to https://twitter.com but only after being kicked to the default browser..

Comment 17

[Myk Melez [:myk] [@mykmelez]]

Comment on attachment 620371 [details] [diff] [review]
followup v1

Since inbound is closed (and wouldn't be guaranteed to merge into central in time for tomorrow's nightly even if open), please land this on central with r=myk, a=js-only.

Comment 18

[Ed Lee :Mardak]

https://hg.mozilla.org/mozilla-central/rev/1bc4679ac516

Comment 19

[Jason Smith [:jsmith]]

For a typical user, this fix will not work to use lanyrd mobile on an app, as the default is to use http://twitter.com, not https://twitter.com. For Ed, https was likely used since he logged in with a twitter account that always requires https. However, Ed's argument is valid that for our temporary solution for allowed_origins, allowing http is risky. I do want to see a verification that the https version works though for lanyrd mobile. Still trying to figure out how to verify this with https for twitter though (I've had no luck so far).

Comment 20

[Gabriela [:gaby2300]]

I can verify Bug 750568 is fixed, my twitter account didn't indicate any error.
Windows 7, nightly build 20120504

Comment 21

[Ed Lee :Mardak]

(In reply to Gabriela from comment #20)
> I can verify Bug 750568 is fixed, my twitter account didn't indicate any
> error.
Curious, which app did you use to test?

Comment 22

[Gabriela [:gaby2300]]

(In reply to Edward Lee :Mardak from comment #21)
> (In reply to Gabriela from comment #20)
> > I can verify Bug 750568 is fixed, my twitter account didn't indicate any
> > error.
> Curious, which app did you use to test?

I used GIS Cloud, as the example above.

Comment 23

[Jason Smith [:jsmith]]

(In reply to Gabriela from comment #22)
> (In reply to Edward Lee :Mardak from comment #21)
> > (In reply to Gabriela from comment #20)
> > > I can verify Bug 750568 is fixed, my twitter account didn't indicate any
> > > error.
> > Curious, which app did you use to test?
> 
> I used GIS Cloud, as the example above.

Confirmed. Marking as verified.