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

Do not prepend oauth_ to additional parameters for oauth_signature #373

Merged
merged 2 commits into from Jun 6, 2016

Conversation

Projects
None yet
2 participants
@jimhester
Member

jimhester commented Jun 1, 2016

The flickr API for instance needs all query parameters to be included in
the signature. (https://www.flickr.com/services/api/auth.oauth.html#yui_3_11_0_1_1464787470505_317)

With this change I am able to get a 200 response with the following code (app secret redacted)

f <- oauth_app("r to flickr", "ed724990662ac2b8c236726a34226ea9", "<redacted>")

ep <- oauth_endpoint(
  request = "https://www.flickr.com/services/oauth/request_token",
  authorize = "https://www.flickr.com/services/oauth/authorize",
  access = "https://www.flickr.com/services/oauth/access_token"
)

tok <- oauth1.0_token(ep, f, cache = T)

signature <- oauth_signature("https://api.flickr.com/services/rest", app =
  tok$app, token = tok$credentials$oauth_token, token_secret =
  tok$credentials$oauth_token_secret,
  other_params = list(method = "flickr.test.login", format = "json", "nojsoncallback" = 1))

GET("https://api.flickr.com/services/rest", query = signature)

@jimhester jimhester force-pushed the jimhester:bugfix/oauth_signature branch 2 times, most recently from ca6def7 to 0b39c49 Jun 1, 2016

Do not prepend oauth_ to additional parameters for oauth_signature
The flickr API for instance needs all query parameters to be included in
the signature

@jimhester jimhester force-pushed the jimhester:bugfix/oauth_signature branch from 0b39c49 to 5e49c92 Jun 1, 2016

jimhester referenced this pull request Jun 1, 2016

@@ -31,11 +31,11 @@ init_oauth1.0 <- function(endpoint, app, permission = NULL,
authorize_url <- modify_url(endpoint$authorize, query = list(
oauth_token = token,
permission = "read"))
verifier <- oauth_listener(authorize_url, is_interactive)[[1]]
verifier <- oauth_listener(authorize_url, is_interactive)$oauth_verifier

This comment has been minimized.

@hadley

hadley Jun 1, 2016

Member

I think the use of position was deliberate here

This comment has been minimized.

@jimhester

jimhester Jun 1, 2016

Member

Yeah it was changed with 7261a52#commitcomment-17702567, unfortunatly subsetting by position breaks when authenticating with flickr at least. I don't know if they are not returning the proper response or if it needs to be reverted.

This comment has been minimized.

@jimhester

jimhester Jun 2, 2016

Member

Ok I have updated it as suggested with da2bef0

# 3. Request access token
response <- POST(endpoint$access,
oauth_sig(endpoint$access, "POST", token, secret, verifier = verifier, private_key = private_key),
oauth_sig(endpoint$access, "POST", token, secret, oauth_verifier = verifier, private_key = private_key),

This comment has been minimized.

@hadley

hadley Jun 1, 2016

Member

Are you sure it doesn't need to be oauth_private_key?

This comment has been minimized.

@jimhester

jimhester Jun 1, 2016

Member

private_key is a named parameter to the oauth_sig() function defined above and doesn't get passed in the new other_params argument. It looks like it is used at https://github.com/hadley/httr/pull/373/files#diff-196777e55557c984260eab22d25ac411L45 for the "RSA-SHA1" method rather than as an oauth parameter.

This comment has been minimized.

@hadley

hadley Jun 1, 2016

Member

Oh ooops, yeah.

Use oauth_verifier if it is returned, otherwise use first item
Some APIs simply return the first item as the verifier token, others
return a named oauth_verifier (which may not be first) along with other
information.

@jimhester jimhester force-pushed the jimhester:bugfix/oauth_signature branch from 24ef81a to da2bef0 Jun 2, 2016

@jimhester

This comment has been minimized.

Member

jimhester commented Jun 6, 2016

Ok this was modified as discussed to check for oauth_verifier in the returned JSON and otherwise take the first item.

@hadley hadley merged commit dac3b49 into r-lib:master Jun 6, 2016

1 check failed

continuous-integration/travis-ci/pr The Travis CI build could not complete due to an error
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment