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

use state instead of cid for oauth1 callback #3354

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

reorx
Copy link

@reorx reorx commented May 23, 2022

Currently twitter oauth1.0a could not be authenticated, the error is:

OAuth1 callback failed because of insufficient user permissions

By investigating the code, I found that when /oauth1-credential/callback
is called, const credential = await getCredentialWithoutUser(credentialId)
failed because creadentialId from req.query.cid is nil.

This is because twitter api only allowes state parameter to be passed
round-trip, others like cid is ignored, which we passed eariler in
/oauth1-credential/auth handler. (Google does the same, AFAIK)

If you wish to include request-specific data in the callback URL, you can use the state parameter to store data that will be included after the user is redirected. It can either encode the data in the state parameter itself or use the parameter as a session ID to store the state on the server.

https://developer.twitter.com/en/docs/apps/callback-urls

In this PR, I simply changed the parameter name from cid to state
and tested that it can fix this issue.

Though state could be a JSON string to hold more data, it's currently
OK to just be the value of credentialId, if we need to add more
values, we can change state to JSON like what has been implemented in
/oauth2-credential.

Currently twitter oauth1.0a could not be authenticated, the error is:

    OAuth1 callback failed because of insufficient user permissions

By investigating the code, I found that when /oauth1-credential/callback
is called, `const credential = await getCredentialWithoutUser(credentialId)`
failed because `creadentialId` from `req.query.cid` is nil.

This is because twitter api only allowes `state` parameter to be passed
round-trip, others like `cid` is ignored, which we passed eariler in
/oauth1-credential/auth handler. (Google does the same, AFAIK)

In this PR, I simply changed the parameter name from `cid` to `state`
and tested that it can fix this issue.

Though `state` could be a JSON string to hold more data, it's currently
OK to just be the value of `credentialId`, if we need to add more
values, we can change `state` to JSON like what has been implemented in
/oauth2-credential.
@CLAassistant
Copy link

CLAassistant commented May 23, 2022

CLA assistant check
All committers have signed the CLA.

@n8n-assistant n8n-assistant bot added community Authored by a community member core Enhancement outside /nodes-base and /editor-ui labels May 23, 2022
@xdlrt
Copy link

xdlrt commented May 28, 2022

hope this will be merged soon

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community Authored by a community member core Enhancement outside /nodes-base and /editor-ui
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants