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

Twitter force_login and Facebook authType not working? #7584

Closed
MichaelJCole opened this issue Aug 4, 2016 · 3 comments · Fixed by #7933
Closed

Twitter force_login and Facebook authType not working? #7584

MichaelJCole opened this issue Aug 4, 2016 · 3 comments · Fixed by #7933
Labels
confirmed We want to fix or implement it Type:Bug

Comments

@MichaelJCole
Copy link
Contributor

MichaelJCole commented Aug 4, 2016

This is about OAuth logins, via these commits

Twitter offers a force_login parameter to ask the user to re-enter their password. This is useful for selecting a twitter account in the case someone has more than one account. It's coded in Meteor here

Facebook offers a similar feature through auth_type which seems to be encoded in Meteor as authType

I'm using the meteor-link-accounts package but neither request seems to be adding these parameters to the URL. (The link shows the code that calls into Meteor Framework).

Is there an example for this?

Here's a reproduction.

The expected behavior is that force_login is passed to twitter, but that doesn't appear to be happening.

abernix added a commit to abernix/meteor-docs that referenced this issue Oct 14, 2016
* Google supports many
* Facebook supports `auth_type`
* Twitter supports `force_login`

API URLs included for reference.

Related to meteor/meteor#7584 meteor/meteor#7078 and meteor/meteor#7820
abernix added a commit to abernix/meteor that referenced this issue Oct 14, 2016
According to the Facebook docs:

https://developers.facebook.com/docs/facebook-login/manually-build-a-login-flow

The parameter should be `auth_type`, not `authType`.  A cursory look through their API history didn't show anything about it being changed, but I can confirm that using this feature does not work with `authType` in the current implementation.

Related meteor/docs#94
Related meteor#7584
Closes meteor#7078
@abernix abernix added confirmed We want to fix or implement it Project:Accounts:OAuth Type:Bug labels Oct 14, 2016
@abernix
Copy link
Contributor

abernix commented Oct 14, 2016

Thanks for opening this issue.

For Facebook's auth_type, you're absolutely right – it's implemented incorrectly as authType in Meteor. I don't think it has ever been authType according to the Facebook docs (as you linked to). Additionally, a cursory look through their API history didn't show anything about it being changed. It's possible that somewhere in their full JS API (which is not used by Meteor) it's possible that it converts it automatically from authType to auth_type, but I can't tell. I've submitted PR #7928 to fix that.


As far as Twitter:

Yeah, the implementation of force_login is broken there too. sigh I'll try to check it out next week.

benjamn pushed a commit that referenced this issue Oct 15, 2016
According to the Facebook docs:

https://developers.facebook.com/docs/facebook-login/manually-build-a-login-flow

The parameter should be `auth_type`, not `authType`.  A cursory look through their API history didn't show anything about it being changed, but I can confirm that using this feature does not work with `authType` in the current implementation.

Related meteor/docs#94
Related #7584
Closes #7078
abernix added a commit to abernix/meteor that referenced this issue Oct 18, 2016
Using a the previously-supported ability to pass a function (versus a string) for an oAuth1 URL, this commit implements (and relocates) a function which safely applies whitelisted params to that URL.

This introduces a twitter_common.js file shared between server and client which indicates which Twitter-supported params are permitted on the authorize step.  The two params which Twitter supports right now are `force_login` and `screen_name`.  (See: https://dev.twitter.com/oauth/reference/get/oauth/authenticate)

This commit removes the non-functional implementation of `force_login` introduced by meteor#6987 and implements it via the aforementioned method.

As a precaution (and since neither `ecmascript` nor `es5-shim` are used by this package), I stuck with JS ES3.

Closes meteor#7584
benjamn pushed a commit that referenced this issue Oct 19, 2016
Using a the previously-supported ability to pass a function (versus a string) for an oAuth1 URL, this commit implements (and relocates) a function which safely applies whitelisted params to that URL.

This introduces a twitter_common.js file shared between server and client which indicates which Twitter-supported params are permitted on the authorize step.  The two params which Twitter supports right now are `force_login` and `screen_name`.  (See: https://dev.twitter.com/oauth/reference/get/oauth/authenticate)

This commit removes the non-functional implementation of `force_login` introduced by #6987 and implements it via the aforementioned method.

As a precaution (and since neither `ecmascript` nor `es5-shim` are used by this package), I stuck with JS ES3.

Closes #7584
@abernix
Copy link
Contributor

abernix commented Oct 20, 2016

@MichaelJCole Again, thanks for reporting this. This should be fixed in the release candidate right now (specifically, 1.4.2-rc.1). Hopefully this is finally working properly!

I tested it with your reproduction, and it did in-fact work. I did run into a bit of a problem getting the twitter package to update properly. I ended up having to manually change the versions of oauth1 to @1.1.11-rc.1 and twitter to @1.1.13-rc.1 in the .meteor/versions file. I believe this is unrelated though and might not happen on a fresh project. :)

Please report back if you still have problems with this!

@abernix abernix mentioned this issue Oct 20, 2016
@MichaelJCole
Copy link
Contributor Author

Right on, thanks!

abernix added a commit to abernix/meteor-docs that referenced this issue Oct 21, 2016
* Google supports many
* Facebook supports `auth_type`
* Twitter supports `force_login`

API URLs included for reference.

Related to meteor/meteor#7584 meteor/meteor#7078 and meteor/meteor#7820
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
confirmed We want to fix or implement it Type:Bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants