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

Concurrency issue with credentials #29

Open
elseano opened this issue Oct 23, 2015 · 0 comments
Open

Concurrency issue with credentials #29

elseano opened this issue Oct 23, 2015 · 0 comments

Comments

@elseano
Copy link

elseano commented Oct 23, 2015

We encountered an issue with how the accessToken structure is stored in the request, which results in details from one request overwriting details of another request.

This comes up specifically when:

  1. You're handling more than one request at a time
  2. You're referencing the req.oauth2.accessToken structure after a call to an external service.

In our case, we have a rate limiter middleware which references the accessToken, and then we make a HTTP request after that to some external resource. The issue is caused by the fact that req.oauth2 actually is always the same one instance.

req.oauth2 = _self

Here's how it plays out:

  1. Two requests come into our app, say they're for bearer tokens A and B.
  2. Node handles the token A request, and hits the rate limiter (which calls to redis for example).
  3. While waiting for a response from redis, node begins handling token B.
  4. At this point, the accessToken structure on the shared req.oauth2 object is changed.
  5. The token B request hits the rate limiter.
  6. While waiting for a response for the token B request, the token A rate limiter request has returned so node starts working on that again.
  7. The token A request now actually contains authentication information from the token B request.
  8. We call out to the external resource with the token B credentials for the token A request.

If your API is providing information specific to a user, then potentially sensitive information can be provided to the wrong person.

We found that by making the change in this commit the problem no longer occurs.

We will have a proper PR coming, but in the meantime wanted to ensure this issue is surfaced.

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

No branches or pull requests

1 participant