Closed Bug 750568 Opened 9 years ago Closed 9 years ago
Cannot use twitter or google accounts within the desktop runtime - Needs to be whitelisted on origins allowed
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).
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.
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.
Whiteboard: [topapps] → [topapps], [marketplace-beta?]
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?
Whiteboard: [topapps], [marketplace-beta?] → [topapps], [marketplace-beta+]
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
(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.
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.
Assignee: nobody → edilee
Status: NEW → ASSIGNED
Attachment #620203 - Flags: review?(myk)
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 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.
Attachment #620203 - Flags: review?(myk) → review+
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.
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
The change did land before the build infrastructure started building Wednesday's nightlies, so it'll appear in them.
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.
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.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Yup. Looks like.. http://lanyrd.com -> http://lanyrd.com/twitter/signin/ -> https://twitter.com/oauth/authenticate
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..
Attachment #620371 - Flags: review?(myk)
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.
Attachment #620371 - Flags: review?(myk) → review+
Status: REOPENED → RESOLVED
Closed: 9 years ago → 9 years ago
Resolution: --- → FIXED
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).
Whiteboard: [topapps], [marketplace-beta+] → [topapps], [marketplace-beta=]
I can verify Bug 750568 is fixed, my twitter account didn't indicate any error. Windows 7, nightly build 20120504
Whiteboard: [topapps], [marketplace-beta=] → [topapps], [marketplace-beta=] [testday-20120502]
(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?
(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.
(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.
Status: RESOLVED → VERIFIED
Whiteboard: [topapps], [marketplace-beta=] [testday-20120502] → [topapps], [marketplace-beta=] [testday-20120504]]
Component: Desktop Runtime → Webapp Runtime
Product: Web Apps → Firefox
You need to log in before you can comment on or make changes to this bug.