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

OAuth parameters are not refreshed following a 302 redirect #1573

Closed
marshall007 opened this issue May 11, 2015 · 6 comments · Fixed by #1574
Closed

OAuth parameters are not refreshed following a 302 redirect #1573

marshall007 opened this issue May 11, 2015 · 6 comments · Fixed by #1574

Comments

@marshall007
Copy link

After following a 302 Redirect, it appears the subsequent request is signed using the same OAuth parameters as the original request.

@simov
Copy link
Member

simov commented May 11, 2015

Is that an actual bug? Can you point us to the exact part of the RFC regarding this behavior?

@marshall007
Copy link
Author

@simov the RFC does not outline behavior for this case specifically, but from Section 3.3:

The nonce value MUST be unique across all requests with the same timestamp, client credentials, and token combinations.

I would assume "all requests" would include redirects. Here's a real-world example of where this becomes an issue with the Bitbucket API: this endpoint responds with a 302 and after following the redirect using the same parameters I get a 401.

@marshall007
Copy link
Author

@simov I was working on a PR for this, do you agree it warrants a fix?

When request.init() is called from Redirect.prototype.onResponse it doesn't pass any parameters, so self.oauth() on request.js#L630 which would have refreshed the credentials doesn't get called. I think the solution is to either change that if statement to include the case where options are already set on self._oauth or to handle it in Redirect.prototype.onResponse. Do you have a preference?

@marshall007
Copy link
Author

I also just realized that none of the OAuth params are stored on the self._oauth object, so it's going to be kind of tricky to "refresh" them either way.

simov added a commit to simov/request that referenced this issue May 13, 2015
- Cache the initial oauth options passed to request in _oauth.params
- On subsequent calls to init() use the cached _oauth.params
  to invoke the oauth params generation logic again
simov added a commit to simov/request that referenced this issue May 13, 2015
- Cache the initial oauth options passed to request in _oauth.params
- On subsequent calls to init() use the cached _oauth.params
  to invoke the oauth params generation logic again
@simov
Copy link
Member

simov commented May 13, 2015

@marshall007 I was able to reproduce the bug and submitted a PR #1574 (I hope you don't mind)

Hopefully we're interpreting the specification correctly. I can't remember having that issue before, but I can't say if I had any redirects when making oauth requests either.

@marshall007
Copy link
Author

@simov looks good, thanks! I'm pretty confident in our interpretation given that there are APIs in the wild that implement this behavior.

simov added a commit that referenced this issue May 17, 2015
Refresh the oauth_nonce on redirect (#1573)
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 a pull request may close this issue.

2 participants