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

Adds generic support additional OAuth providers #247

Merged
merged 3 commits into from
Feb 17, 2016
Merged

Adds generic support additional OAuth providers #247

merged 3 commits into from
Feb 17, 2016

Conversation

flovilmart
Copy link
Contributor

Proposed fix for #241 and #40

Adds additional OAuth providers, may need a refactor to reduct the boilerplate request code.

No coverage of the network request, needs to be tested against real keys and tokens unfortunately.

Because Facebook auth validation is refactored, the methodology should work for all other providers.

@flovilmart flovilmart changed the title Fix 241 Adds generic support additional OAuth providers Feb 5, 2016
@facebook-github-bot
Copy link

@flovilmart updated the pull request.

@flovilmart
Copy link
Contributor Author

Twitter is a bit trickier because it requires header signature with OAuth1.

@facebook-github-bot
Copy link

@flovilmart updated the pull request.

2 similar comments
@facebook-github-bot
Copy link

@flovilmart updated the pull request.

@facebook-github-bot
Copy link

@flovilmart updated the pull request.

@flovilmart
Copy link
Contributor Author

@nlutsenko there seems to be a problem with the test suite as it randomly fails after downloading a certain version of mongo-db... Not sure what's happening here.

@flovilmart flovilmart mentioned this pull request Feb 5, 2016
@facebook-github-bot
Copy link

@flovilmart updated the pull request.

4 similar comments
@facebook-github-bot
Copy link

@flovilmart updated the pull request.

@facebook-github-bot
Copy link

@flovilmart updated the pull request.

@facebook-github-bot
Copy link

@flovilmart updated the pull request.

@facebook-github-bot
Copy link

@flovilmart updated the pull request.

@flovilmart
Copy link
Contributor Author

Twitter is giving me some pain... Stupid OAuth 1.0

@facebook-github-bot
Copy link

@flovilmart updated the pull request.

@gfosco
Copy link
Contributor

gfosco commented Feb 6, 2016

This is beautiful. 👍

@flovilmart
Copy link
Contributor Author

@gfosco, I'll refactor to pass just authData and options to both validation calls, it will be cleaner and more extensible

@facebook-github-bot
Copy link

@flovilmart updated the pull request.

@flovilmart
Copy link
Contributor Author

@gfosco refactor is done, adds ability also to pass custom modules as validators :) that will help with extensibility. And obviously is covered by tests.

@nitrag
Copy link

nitrag commented Feb 8, 2016

+1

@talkaboutdesign
Copy link

  • YES! Please merge already.

@meilers
Copy link

meilers commented Feb 9, 2016

+1

@kkaradag2
Copy link

hi @flovilmart
I have a problem about twitter aouth.

I using like that.

curl -X POST
-H "X-Parse-Application-Id: xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"
-H "X-Parse-REST-API-Key: yyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyy"
-H "X-Parse-Revocable-Session: 1"
-H "Content-Type: application/json"
-d '{
"authData": {
"twitter": {
"id": "12345678",
"screen_name": "ParseIt",
"consumer_key": "SaMpLeId3X7eLjjLgWEw",
"consumer_secret": "SaMpLew55QbMR0vTdtOACfPXa5UdO2THX1JrxZ9s3c",
"auth_token": "12345678-SaMpLeTuo3m2avZxh5cjJmIrAfx4ZYyamdofM7IjU",
"auth_token_secret": "SaMpLeEb13SpRzQ4DAIzutEkCE2LBIm2ZQDsP3WUU"
}
}
}'
http://localhost:1337/users

response come like that

{
"code": 200
"error": "bad or missing username"
}

What is my wrong ?

Thank you

@flovilmart
Copy link
Contributor Author

@kkaradag2 I didn't test with the REST API, I'll add the according Unit tests to make sure that work. please stay tuned!

@facebook-github-bot
Copy link

@flovilmart updated the pull request.

@kkaradag2
Copy link

@flovilmart I finaly run twitter register. I notice that, git clone come someting is wrong or miss. I read carefully your comments and changes so I created a running version on m local.
Now I can work with parse-server but I want to say http://localhost:1337/users registration for twitter accounts need for check user already has. Because I can create one than more same user.
Thank you for your response.

@flovilmart
Copy link
Contributor Author

@kkaradag2 does your bug reproduces with Facebook as well? this is maybe a more generalized bug

@kkaradag2
Copy link

I didn't try to create a user with Facebok. May be yes this is a general problem with oauth. Because normal registration (username and password one) say
{
"code": 202
"error": "Account already exists for this username"
}

oauth registration needs same control.

Thank you

@FridaySG
Copy link

Hey total beginner here. How might one utilize this via the iOS/Android Client SDK's?
Does this give the ability for my users to continue using Twitter login as seamlessly as Facebook post-migration?
If not, what would have to be done to implement these advantages in the Parse Client SDK's?

@flovilmart
Copy link
Contributor Author

@FridayDevGroup there is a gist there: https://gist.github.com/flovilmart/68a6c538496953408bb1 with the example for the twitter login

@facebook-github-bot
Copy link

@flovilmart updated the pull request.

Refactors facebook login into oauth generic login

Adds additional oauth2 providers

adds ability to pass an oAuth validator in the config

Adds Twitter validation support + OAuth 1 client

Support auth_token instead of access_token for twitter

Improves code coverage of OAuth

Adds validation of oauth provider structures

Better coverage of the OAuth spec

100% coverage of OAuth1.js

Adds passing auth_token_secret for Twitter auth.

Refactors auth validation methods to include authData parameter

- Adds ability to extens oauth validator through configuration
- Adds ability to extend oauth validator through external module (file or package)
- Adds more tests
- Adds tests to login with custom auth provider

Adds more tests for REST API

fixes twitter auth_token

f
@facebook-github-bot
Copy link

@flovilmart updated the pull request.

@FridaySG
Copy link

@flovilmart Thank you very very much. That example is gorgeous. So for the Twitter users currently registered to my app, this will query and return the user account created pre-migration? Apologies if the answer to that is ubiquitous.

@flovilmart
Copy link
Contributor Author

@FridayDevGroup it definitely should. The original SDK also pass the consumer_key and consumer_secret alongside the authData, you can pass them also but that's optional as the token verification is made based on the oauth configuration as described in the readme.

@FridaySG
Copy link

@flovilmart Awesome, thank you. I'm going to get going with all of this soon. About the readme, I tried to take a look at the example links you dropped in there earlier and they're all returning 404. I think they may have gotten moved around when the Parse-Server folder restructuring pr got merged, but I couldn't track down the new locations.

@flovilmart
Copy link
Contributor Author

@FridayDevGroup it's because it's not merged yet :) On another point, you should not need those links :)

@FridaySG
Copy link

@flovilmart Yeah.. I had a feeling it was my own misunderstanding haha. Thanks again for the guidance.

@flovilmart
Copy link
Contributor Author

no problemo!

@tanmays
Copy link

tanmays commented Feb 17, 2016

Hello @flovilmart

I'm not sure if this issue is related with oAuth specifically or the parse save function but here's what I'm facing.

I'm trying to link a Twitter account to an existing parse account using Cloud Code and I'm getting a code: 206, message: 'cannot modify user error.

It seems a request.user.save() function when run within Cloud Code does not pass the sessionToken in this case.

Here's my original request:

POST /parse/functions/LinkTwitterAccount { host: 'af3098d9.ngrok.io',
  'content-type': 'application/json',
  'content-length': '349',
  'x-parse-session-token': 'r:<snipped>',
  accept: '*/*',
  'user-agent': 'Pinglar/0.2 (iPhone; iOS 9.2.1; Scale/2.00)',
  'x-parse-application-id': '<snipped>',
  'accept-language': 'en-IN;q=1',
  'accept-encoding': 'gzip, deflate',
  'x-forwarded-proto': 'https',
  'x-forwarded-for': '120.88.191.119' } {
  "twitterID": <snipped>,
  "socialAuthData": {
    "twitter": {
      "auth_token": "<snipped>",
      "id": "<snipped>",
      "screen_name": "<snipped>",
      "consumer_key": "<snipped>",
      "consumer_secret": "<snipped>",
      "auth_token_secret": "<snipped>"
    }
  }
}

Here's the request made on request.user.save() in cloud code

PUT /parse/classes/_User/NSME8tT0tf { 'user-agent': 'node-XMLHttpRequest, Parse/js1.7.0 (NodeJS 4.2.6)',
  accept: '*/*',
  'content-type': 'text/plain',
  host: 'localhost:1337',
  'content-length': '500',
  connection: 'close' } {
  "authData": {
    "twitter": {
      "auth_token": "<snipped>",
      "id": "<snipped>",
      "screen_name": "<snipped>",
      "consumer_key": "<snipped>",
      "consumer_secret": "<snipped>",
      "auth_token_secret": "<snipped>"
    }
  }
}

Here's my cloud code

< Code to cleanup a few here is omitted >

request.user.set("authData", socialAuthData);
// Save user
request.user.save({useMasterKey:true}).then(function(userObject) {
  response.success(userObject);
  }, function(error) {
    console.log("Unable to save current user" + error.message);
    response.error("There was a problem creating your account, please try again");
});

Specifying a master key too doesn't help and I still get the 206 error code which is for missing sessionToken.

@flovilmart
Copy link
Contributor Author

I don't think you can explicitly set authData like that. What does the JS SDK suggest for linking?

@tanmays
Copy link

tanmays commented Feb 17, 2016

Just tried it again with useMasterKey:true and it worked. I think any changes made to request.user within a cloud code function will still need to use the masterkey. I think one shouldn't be required to use masterKey to modify request.user objects, what do you think? Should I open this as a separate issue?

Anyhow this issue thankfully isn't related to this pull request, hope this gets merged asap.

@flovilmart
Copy link
Contributor Author

Can you try on the master branch with a Facebook auth data? See if it's a regression or not.

@drew-gross
Copy link
Contributor

Looks awesome! Thanks a lot @flovilmart

drew-gross added a commit that referenced this pull request Feb 17, 2016
Adds generic support additional OAuth providers
@drew-gross drew-gross merged commit ccc1d02 into parse-community:master Feb 17, 2016
@flovilmart
Copy link
Contributor Author

🎉 !

@flovilmart flovilmart deleted the fix-241 branch February 17, 2016 17:46
@nlutsenko
Copy link
Contributor

Whee!! We finally have it!

@flovilmart
Copy link
Contributor Author

Ahah! I'm drinking a beer to cheer on that! 🍻 for everyone at Parse HQ!

@ellemedit
Copy link

ellemedit commented Jul 7, 2016

What about adding documentation for this?

@acegreen
Copy link

acegreen commented Mar 4, 2017

Lots going on here but not enough documentation, so is the new syntax for index.js as such:

auth: { facebook: { appIds: ["XXXXXXXXXX"] } }, oauth: { twitter: { consumer_key: "XXXXXXXXXX", consumer_secret: "XXXXXXXXXX" },

@flovilmart
Copy link
Contributor Author

oauth is deprecated in favor of auth.

@acegreen
Copy link

acegreen commented Mar 4, 2017

Does that mean Parse Server (2.3.0+) doesn't support Twitter login? I thought this was covered in this PR

@flovilmart
Copy link
Contributor Author

It just means that parse-server prefers the auth option instead of oauth. It doesn't change what's supported, just the option name.

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.