Skip to content

Conversation

@jperl
Copy link
Collaborator

@jperl jperl commented Jun 9, 2014

Features of this PR:

  • Make phonegap-oauth work with Meteor >= v0.8.2 while maintaining support for the browser-policy package, details below.
  • Close the InAppBrowser when the auth flow is cancelled by the user.
  • Close the InAppBrowser on the exit event. This is triggered if the back button is pressed and there is no history (right after it is opened).
  • Wait until InAppBrowser is loaded before patching the window. This fixes a bug if the meteor application is loaded before cordova is ready -- this will happen if the app is included on the phone locally.
  • A better workaround for the InAppBrowser eventListener callback not firing, which also fixes the bug on Android where Cordova plugin callbacks are not processed until the next message.

To support Meteor's latest oauth I patch OAuth._endOfLoginResponse to append the credentialToken and credentialSecret to the url hash -- and use them to call OAuth._handleCredentialSecret and Accounts.oauth.tryLoginAfterPopupClosed.

This method works even if the meteor app is using the browser-policy package which prevents inline
scripts from being executed. The executeScript method used in PR #3 will not work under these restrictions since it uses inline scripts.

This PR will only work with Meteor >= v0.8.2, as it relies on OAuth._endOfLoginResponse.

More context here https://groups.google.com/forum/#!topic/meteor-core/Ma3XTZk4Kqg

If meteor/meteor#2220 is accepted we can get rid of the OAuth._endOfLoginResponse patch.

@jperl
Copy link
Collaborator Author

jperl commented Jun 9, 2014

@faceyspacey I did not include that regex modification to make it work for twitter because it broke the LinkedIn oauth. Do you have one that works for all providers?

@faceyspacey
Copy link
Contributor

@jperl no but it's simple enough to add another OR clause

jperl added 3 commits June 10, 2014 10:20
…uthentication.

Remove android show setInterval hack because it breaks the behavior of the InAppBrowser closing when the user presses back.
Since loadstop is called without it it is not necessary.
@jperl
Copy link
Collaborator Author

jperl commented Jun 10, 2014

@faceyspacey K I checked in some code that should work. Would you mind testing it to see if it works for you?

jperl added 3 commits June 11, 2014 15:21
Also handle exit event -- if the user presses back, close the window.
otherwise it will throw an exception
@jperl
Copy link
Collaborator Author

jperl commented Jun 16, 2014

@AdamBrodzinski Have you had a chance to check this out yet?

@AdamBrodzinski
Copy link
Owner

@jperl Not yet, it's on my todo list for tonight or tomorrow though!

@AdamBrodzinski
Copy link
Owner

@jperl I'll try to finish testing on Android tonight... not sure how that will go, my eclipse was botched the last time I tried to compile.

There's a copy running in debug mode on http://oauth-demo.meteor.com with this patch (minus the require mentioned above) if anyone wants to try it out.

I can confirm that if I comment out/change the IAB require, the LinkedIn OAuth and Twitter OAuth are working on iOS7 👍 🍻

@jperl
Copy link
Collaborator Author

jperl commented Jun 18, 2014

@AdamBrodzinski Glad it works on twitter. I didn't even test that 😃

@jperl jperl changed the title Work with vNext Meteor auth Work with 0.8.2 meteor auth Jun 18, 2014
jperl added 2 commits June 19, 2014 22:33
Replace setInterval show hack with setInterval exec hack. This will solve problems for other plugins as well.
@jperl
Copy link
Collaborator Author

jperl commented Jun 20, 2014

@AdamBrodzinski FYI just committed an improved hack for fixing the event listener callbacks never firing.

@AdamBrodzinski
Copy link
Owner

@jperl Oh wow, I wish I would have seen that a month ago! I lost a lot of hair trying to get the video capture to work. It was only firing the callback on the 2nd capture. 😦 I'm hoping to test on the Galaxy S3 tonight and if all goes well on the Genymotion emulators. I def. want to ship this when 0.8.2 is out!

@jperl
Copy link
Collaborator Author

jperl commented Jun 20, 2014

@AdamBrodzinski Yeah I was running into the same problem w the dialog prompt until I tried this. It works on my nexus 5, I hope it works on the S3. fingers crossed

@faceyspacey
Copy link
Contributor

@jperl @AdamBrodzinski can you guys send me the code. I was running into the same issue with capture callbacks. I'd like to use it since I know Adam is really busy.

@jperl
Copy link
Collaborator Author

jperl commented Jun 20, 2014

@faceyspacey Not sure what you mean exactly. Send you the code that is in this PR? The code that fixes the plugin callbacks is here https://github.com/jperl/meteor-phonegap-oauth/blob/master/patch_window.js#L20-L29

@faceyspacey
Copy link
Contributor

oops my bad. i didnt see that i can get it. awesome brother. thanks!

@jperl
Copy link
Collaborator Author

jperl commented Jun 20, 2014

@faceyspacey NP. Let me know if it works for you.

@tstuber
Copy link

tstuber commented Jun 22, 2014

hey guys, I'll test that in short as well. But did I got it right, that this will not working with cordova 3.3? Only with 3.4 and 3.5?

Cheers

@AdamBrodzinski
Copy link
Owner

It will work if you make your own local package and then change the require line to use '.InAppBrowser' instead of the lowercased one (on mobile now so I can't reference it). Phone gap recently changed their JS code in 3.4 or 3.5—
Sent from Mailbox

On Sun, Jun 22, 2014 at 8:58 AM, tstuber [email protected] wrote:

hey guys, I'll test that in short as well. But did I got it right, that this will not working with cordova 3.3? Only with 3.4 and 3.5?

Cheers

Reply to this email directly or view it on GitHub:
#5 (comment)

AdamBrodzinski added a commit that referenced this pull request Jun 23, 2014
adds functionality to work with new 0.8.2 meteor auth
@AdamBrodzinski AdamBrodzinski merged commit 31201fe into AdamBrodzinski:master Jun 23, 2014
@AdamBrodzinski
Copy link
Owner

@jperl @faceyspacey thanks for the PRs 🍻

Just tested on Android 4.4 Motox and 4.3 Galaxy S3 and it's working great on both LinkedIn and Twitter 😄

@jperl
Copy link
Collaborator Author

jperl commented Jun 24, 2014

Awesome!

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants