Use the id of the account instead of the application id #3108
Conversation
Fix for issue: #3106
| @@ -12,7 +12,7 @@ function validateAuthData(authData, options) { | |||
| client.auth_token_secret = authData.auth_token_secret; | |||
|
|
|||
| return client.get("/1.1/account/verify_credentials.json").then((data) => { | |||
| if (data && data.id == authData.id) { | |||
| if (data && data.id_str == authData.user_id) { | |||
There was a problem hiding this comment.
using authData.user_id is a breaking change, not sure why we should change that here.
There was a problem hiding this comment.
The response from twitter does include the data.id, but it is not always correct: https://dev.twitter.com/overview/api/twitter-ids-json-and-snowflake. This is a possible way to solve the problem as the response does include the id_strwhich is the account id (which is included when requesting a token).
The naming of authData.user_id is debatable since this is returned from hellojs (it can just stay id)
There was a problem hiding this comment.
I guest that, my point is why change authData.id to authData.user_id
There was a problem hiding this comment.
See my edits to make it clear. It might as well stay id (hellojs returns user_id)
There was a problem hiding this comment.
yeah but the parse SDK uses id for the twitter authData user ID so you can't change that...
There was a problem hiding this comment.
I agree, it was a quick fix without thinking about the consequences. Can I somehow edit this or do i need to create a new pull request?
|
@jonas-db updated the pull request - view changes |
| @@ -12,7 +12,7 @@ function validateAuthData(authData, options) { | |||
| client.auth_token_secret = authData.auth_token_secret; | |||
|
|
|||
| return client.get("/1.1/account/verify_credentials.json").then((data) => { | |||
| if (data && data.id == authData.id) { | |||
| if (data && data.id_str == authData.id) { | |||
There was a problem hiding this comment.
Given the twitter doc, where both id and id_str are provided, authData.id is not guaranteed to be neither a string nor a number. Given that, we should either type check, or use if (data && data.id_str == ''+authData.id) to make sure we're comparing the same types
There was a problem hiding this comment.
Indeed, I will wait for further instructions because I guess it is not up to me to decide that :D
I have tested my implementation again and I do have indeed a real twitter account where id and id_str differs, which makes it fails. Changing it to data.id_str works.
There was a problem hiding this comment.
idandid_strdiffers
Can you expand on that? How would they differ?
There was a problem hiding this comment.
Consider the twitter link I posted earlier, which explains why they can be different from each other
|
@jonas-db can you force the authData.id to be a string by using? |
|
@jonas-db updated the pull request - view changes |
|
@jonas-db Many thanks for the fix! |
…ity#3108) * Use the id of the account instead of the application id Fix for issue: parse-community#3106 * Update twitter.js * Update twitter.js
…ity#3108) * Use the id of the account instead of the application id Fix for issue: parse-community#3106 * Update twitter.js * Update twitter.js
Fix for issue: #3106