Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Cordova] Google will disallow OAuth login in embedded WebView in April 2017 #8253

Closed
realyze opened this issue Jan 13, 2017 · 41 comments
Closed

Comments

@realyze
Copy link
Contributor

realyze commented Jan 13, 2017

See https://developers.googleblog.com/2016/08/modernizing-oauth-interactions-in-native-apps.html .

Quote:

On April 20, 2017, we will start blocking OAuth requests using web-views for all OAuth clients on platforms where viable alternatives exist.

What's the plan to deal with this please? Current Meteor Accounts seem always use the popup login style overriding redirect.

@stubailo
Copy link
Contributor

Presumably redirect will have the same problem right?

@realyze
Copy link
Contributor Author

realyze commented Jan 15, 2017

Right, correct (I didn't stop to think that redirect doesn't mean redirect outside of webview :-) ).

So what do we do? There's https://github.com/EddyVerbruggen/cordova-plugin-googleplus but I haven't investigated how to use it with Accounts yet.

@stubailo
Copy link
Contributor

I think the first step would be to find out if the Cordova community has a solution for this -- how do people usually do OAuth there, is it through these various plugins?

If there are adequate solutions it might be as simple as just passing the data from that into a login method. What Meteor is doing is in no way unique, but from what I understand there isn't a public API that will let you easily integrate with such a plugin.

@realyze
Copy link
Contributor Author

realyze commented Jan 16, 2017

@ramezrafla
Copy link
Contributor

ramezrafla commented Jan 22, 2017

Seems the community is still looking for an answer. Google likely wants to make sure to install super cookies in user browser to push their ads and collect data.

Can it not be as easy as altering the useragent of the oauth request?

@realyze
Copy link
Contributor Author

realyze commented Feb 1, 2017

Soo, I don't want to be pushy, but is anyone from MDG looking into this? When Google flips the switch in April, all Meteor Cordova apps that use Google OAuth will break.

Kinda sounds like a big deal.

@ramezrafla
Copy link
Contributor

Agreed -- this could be a major issue in April. @martijnwalraven sorry to pull you into this. Your opinion would be appreciated (including maybe just setting the useragent, how does Google know if it's a webview except from useragent).

I expect a backlash from many mobile developers on this major change.

@martijnwalraven
Copy link
Contributor

@ramezrafla: It seems Ionic recommends https://github.com/EddyVerbruggen/cordova-plugin-googleplus, so it would be great if someone could try that out and see how it could be integrated with a Meteor login method.

@ramezrafla
Copy link
Contributor

Thanks! I checked and they even have a Meteor package: https://atmospherejs.com/hedcet/cordova-google-plus-native-sign-in

Will try it shortly.

@realyze
Copy link
Contributor Author

realyze commented Feb 27, 2017

@ramezrafla Any progress please?

Less than two months left till Google breaks all Cordova apps.

panic

@ramezrafla
Copy link
Contributor

@realyze I haven't yet, but will do so within a week's time. The challenge here is to keep the token so the rest of the workflow proceeds as planned, then I don't foresee an issue.

@realyze
Copy link
Contributor Author

realyze commented Feb 27, 2017

Thanks, much appreciated! I'm nagging because updating the apps on AppStore takes some time so it's safer to plan ahead.

@realyze
Copy link
Contributor Author

realyze commented Mar 7, 2017

@ramezrafla I hate to nag you again but - any progress please?

@realyze
Copy link
Contributor Author

realyze commented Mar 14, 2017

@martijnwalraven @ramezrafla Hi guys, one month left till all Meteor Cordova apps using Google OAuth will break. Any updates please?

Should we start looking into this ourselves? If you don't plan to address this issue, I'll freak out but at least please let us (and the community) know so that we know we should take over!

@martijnwalraven
Copy link
Contributor

@realyze: I won't have time to work on this, so it would be great if you could look into this yourselves. It seems to me this isn't a Meteor-specific issue however, so I would recommend investigating how the rest of the Cordova community deals with this and adapt that solution to the Meteor situation. The plugin mentioned in the Ionic blog post I linked to above sounds promising and might be a good starting point.

@realyze
Copy link
Contributor Author

realyze commented Mar 17, 2017

@martijnwalraven OK, thanks for the heads-up, I'll look into it.

I feel like I should say that I find it disturbing that MDG won't be looking into this though. I know Meteor is an OS project and I appreciate your hard work. But this makes me feel like balls are being dropped.

@abernix
Copy link
Contributor

abernix commented Mar 17, 2017

@realyze Let me know if you run into any road blocks that preventing you from progressing or anything at all I can do to help you look into this? We've recently updated the Development Guide with many details about working with Meteor Core.

A contribution to help with this would be greatly appreciated, especially as it seems you might be better equipped to test.

@realyze
Copy link
Contributor Author

realyze commented Mar 31, 2017

@benjamn Thanks for looking into this! And sorry for not looking into this myself, I was out for the last two weeks.

@benjamn
Copy link
Contributor

benjamn commented Mar 31, 2017

@realyze No worries! I would very much appreciate any feedback you have time to provide.

benjamn pushed a commit that referenced this issue Apr 3, 2017
benjamn added a commit that referenced this issue Apr 3, 2017
* Support Google Sign-In in google-oauth package.

Addresses #8253.

* Use Meteor.startup instead of listening for deviceready event.

* Fix mobile-config.js typo.

* Bump accounts-google and google-oauth package versions.

I'm only bumping the patch versions, even though the recent changes to
these packages may seem significant, for two reasons:

1. Bumping the minor versions would force Meteor 1.4.3 developers to
   upgrade to Meteor 1.4.4 if they wanted to use these changes.

2. The accounts-google and google-oauth packages without these changes
   will stop working completely in two weeks, which is much worse than the
   risks of upgrading.
benjamn pushed a commit to meteor/cordova-plugin-googleplus that referenced this issue Apr 4, 2017
This implements a suggestion made by @djett41:
EddyVerbruggen#352

I'm not sure if this is the most appropriate way to fix Keychain Sharing
for all relevant versions of Cordova iOS, but it works for the version of
Cordova that Meteor is using (6.4.0), and should therefore help with
meteor/meteor#8253.
benjamn pushed a commit to meteor/cordova-plugin-googleplus that referenced this issue Apr 4, 2017
This implements a suggestion made by @djett41:
EddyVerbruggen#352

I'm not sure if this is the most appropriate way to fix Keychain Sharing
for all relevant versions of Cordova iOS, but it works for the version of
Cordova that Meteor is using (6.4.0), and should therefore help with
meteor/meteor#8253.
@realyze
Copy link
Contributor Author

realyze commented Apr 6, 2017

@benjamn So, everything seems to work OK on iOS, but I'm hitting EddyVerbruggen/cordova-plugin-googleplus#368 on Android which is a showstopper (can't build the app).

@abernix
Copy link
Contributor

abernix commented Apr 6, 2017

@realyze What version of the Android SDKs (Platform, etc.) are you using? A screenshot of your SDK Manager may be helpful. 😄

@realyze
Copy link
Contributor Author

realyze commented Apr 6, 2017

@abernix
screen shot 2017-04-06 at 10 08 51 pm

But I think my particular dependency hell is caused by the combo of cordova plugins we're using in our app.

Since we're on Meteor@1.3, we can't use the latest versions (because we depend on the Cordova version bundled with Meteor).

Here's the offending plugins:

cordova.plugins.diagnostic@3.0.0
cordova-plugin-googleplus@5.1.1
phonegap-plugin-push@1.4.5
urbanairship-cordova@5.4.0

And here's what they cause:

  • urbanairship > 5.4.0 causes the "Multiple dex files" error.
  • phonegap-plugin-push > 1.5 with cordova-plugin-googleplus causes the duplicate symbol error
  • cordova.plugins.diagnostic > 3.0.0 causes the No resource found that matches the given name 'android:TextAppearance.Material.Widget.Button.Borderless.Colored' (I think it wants to use SDK 24)

Oh the joys of Cordova development! :-)

@abernix
Copy link
Contributor

abernix commented Apr 6, 2017

@realyze

Since we're on Meteor@1.3, we can't use the latest versions (because we depend on the Cordova version bundled with Meteor).

Is there anything specifically keeping you from using newer versions of Meteor (and thus Cordova), such as Meteor 1.4.3.2?

@realyze
Copy link
Contributor Author

realyze commented Apr 7, 2017

@abernix It's mostly just good old "if it ain't broke, don't fix it". :-) But also:

  1. Node 4 has lazy GC and since we're running on Heroku and some of our worker dynos use quite a lot of RAM, we might need to fiddle with the memory settings..which is something I've been avoiding since the server load is tricky to replicate in staging env (and I'm concerned about suddenly getting OOM errors).
  2. Meteor@1.4 has newer mongo driver which has a different API (and we're accessing the raw collections to run e.g. findAndModify in a few places on the server).

@realyze
Copy link
Contributor Author

realyze commented Apr 7, 2017

Oh and if we upgrade Meteor & Cordova & bump cordova plugin versions, then there's an unrelated issue where we'd need to build with XCode8 which changed code signing so it's now a pain for us to build (so we've been building with XCode7 for now).

@abernix
Copy link
Contributor

abernix commented Apr 7, 2017

@realyze

It's mostly just good old "if it ain't broke, don't fix it". :-)

I can certainly understand that! But there's also the case where the older software gets, the harder it is to update – which is what is happening now! (Sorry, but I had to say it 🙊). Additionally, Node 0.10 is no longer receiving security updates since it's been "End of Life"'d by Node.

If you have an idea how to fix the issue you've reported in the cordova-plugin-googleplus repo, we're happy to try and review that and land it in our own fork of that project, but we'd need your help solving it. Unfortunately, our hands are a bit tied since you're at the mercy of other conflicting plugins. With that said, the rest of my update is going to be along the lines of "you might want to try upgrading", and trying to make you feel better about trying. 😄

Node 4 has lazy GC and since we're running on Heroku and some of our worker dynos use quite a lot of RAM, we might need to fiddle with the memory settings..which is something I've been avoiding since the server load is tricky to replicate in staging env (and I'm concerned about suddenly getting OOM errors).

Memory performance was greatly increased in Node via many V8 improvements and a number of additional configurations options becoming available in node --v8-options. Regardless, you can pass --expose-gc (on Heroku you can configure this in your Procfile) and call global.gc(); to trigger GC when you'd like (for example, after a worker finishes a job, maybe?). All in all, you should have the ability to do better memory management. I'd recommend trying before discounting it as a problem!

Meteor@1.4 has newer mongo driver which has a different API (and we're accessing the raw collections to run e.g. findAndModify in a few places on the server).

I don't believe the API for findAndModify (which is Mongo server functionality) changed in the Node Mongo driver itself so unless you change your actual Mongo server version you shouldn't have a problem here, right?

Oh and if we upgrade Meteor & Cordova & bump cordova plugin versions, then there's an unrelated issue where we'd need to build with XCode8 which changed code signing so it's now a pain for us to build (so we've been building with XCode7 for now).

I believe we've fixed any Meteor problems with this (which were fixed upstream by Cordova). Happy to try to help with that too, but we don't have any open issues about this right now I don't think.

@realyze
Copy link
Contributor Author

realyze commented Apr 7, 2017

@abernix Thanks for the tips, much appreciated!

It's not that we never plan to upgrade to 1.4, it's just that we've been postponing it until the "right time". :-)

Re: the cordova plugin issues: Not sure if it makes sense for you to do anything there. There might be any number of incompatible (versions of) plugins and you can't fix all of them. Plus it's possible that new versions of those plugins use cocoa pods and solve some of those issues already.

Re: findAndModify: The driver API actually did change. The value passed in callback in v1.4 is the doc itself while in 2.2 it's a findAndModifyWriteOpResultObject object.

Re: build issues with XCode8: That's an issue on our end, we're using Fastlane to do our ios builds and the automatic provisioning introduced in XCode8 broke our scripts so they require a manual step (which obviously sucks for CI). Anyhow, we'll fix that once it becomes painful enough (which it will once we upgrade to meteor@1.4 ;) ).

@realyze
Copy link
Contributor Author

realyze commented Apr 12, 2017

@abernix So I have a question about the new implementation. My brain might just be tired but I don't see how it's supposed to work.

In https://github.com/meteor/meteor/blob/devel/packages/google-oauth/google_server.js unless I'm completely off, when I sign in with Google Plus cordova plugin, the Accounts.registerLoginHandler callback will get called, but not the OAuth.registerService callback. Unless this changed between Meteor@1.3 and 1.4.

If I'm right, the registerLoginHandler won't store user's name, refresh_token and many other important fields (because the https://www.googleapis.com/oauth2/v3/tokeninfo endpoint doesn't return those).

@realyze
Copy link
Contributor Author

realyze commented Apr 12, 2017

I filed #8586 for that; I just checked on Meteor@1.4 and newly created user documents are "corrupted" (in that they're missing important fields).

@adamgins
Copy link

I just noticed that they were missing User name too (or atleast my code was)... I have not debugged yet to see if a different return structure from Google

@bsbechtel
Copy link

My team is still seeing this issue. When we build on iOS, things seem to work fine. However, when we build on Android, we are getting errors when trying to call loginWithGoogle. We are using crosswalk, which is no longer supported. I suspect this might be causing problems since Google's new auth flow uses the device browser instead of a webview. Has anyone else experienced this?

@realyze
Copy link
Contributor Author

realyze commented Apr 19, 2017

@bsbechtel Can you share what errors are you getting? What's the error message from Google?

@abernix
Copy link
Contributor

abernix commented Apr 20, 2017

@bsbechtel More so, if you could create a basic reproduction application of what you're trying it could be helpful – even if it does include (the deprecated) Crosswalk plugin. No need to include your API keys - someone else can provide those.

@bsbechtel
Copy link

We are seeing a 12501 error from Google (see: EddyVerbruggen/cordova-plugin-googleplus#307). My coworker told me he is seeing logs on our Galaxy server related to this issue: #8613 which has an open PR from @abernix to fix it. It sounds like fixing issue 8613 could fix our 12501 error too.

@abernix
Copy link
Contributor

abernix commented Apr 20, 2017

@bsbechtel Could you try google-oauth@1.2.4?

@realyze
Copy link
Contributor Author

realyze commented Apr 20, 2017

@bsbechtel Did you create an Android oauth key in console.developers.google.com with the right SHA fingerprint? I was getting 12501 when I forgot to create the oauth key.

@chrisknu
Copy link

@realyze @abernix (I work with @bsbechtel) Yes, the key was set for the parent project, and/or the projectId was added to the whitelist for the 'staging' project in the firebase console.
You can only use one SHA per project, but whitelisting lets you share..

#8613 fixed the issue we were seeing

@realyze
Copy link
Contributor Author

realyze commented May 27, 2017

@abernix Should we close this one?

@abernix
Copy link
Contributor

abernix commented May 31, 2017

Thanks, @realyze. Yes, I think this can be closed now as the date has passed and the changes from #8549, #8604 and #8588 seem to have done the trick! Thanks again for your help with this!

@madc0w
Copy link

madc0w commented Jul 4, 2019

Not sure if anyone is still reading this, but for me the issue is definitely still present.

We tried the solution suggested here
https://github.com/meteor/meteor/wiki/OAuth-for-mobile-Meteor-clients
namely to use loginStyle: "redirect", which had the effect of showing an error message (rather than the silent failure we were seeing). The message is, however, utterly unhelpful:
"SignInResult is null"

Note that Google signin works great in the web app. Not so much with Cordova.

google-oauth@1.2.6
METEOR@1.8.1

@lakshya-8
Copy link

Right, correct (I didn't stop to think that redirect doesn't mean redirect outside of webview :-) ).

So what do we do? There's https://github.com/EddyVerbruggen/cordova-plugin-googleplus but I haven't investigated how to use it with Accounts yet.

Were you able to confirm if it's using the WebView or WKWebView ( iOS ) internally when sending the OAuth requests. This issue was opened in 2017, and I got an email from Google in March 28th, 2023 so assume the support for WebView had not be deprecated in 2017?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests