TriPSs / react-esc

Easy to use Server and Client configuration
MIT License
2 stars 0 forks source link

Sessions/cookies mix up #33

Closed kuatro closed 6 years ago

kuatro commented 6 years ago

Sessions/cookies mix up.

I'm experiencing a nasty tricky bug, and it's pretty hard to explain how to get it:

There is a page which show user's cookies (i.e Your auth token is: 12345678) Bob has a special cookie named token, which contains his authentication token (12345678). Sally doesn't have any cookies on her browser, so when she requests the page, she don't get any message.

But when Bob and Sally makes requests to the page at the same time, there is a chance for Sally to get Bob's credentials (the page will display: Your auth token is: 12345678)!

Steps to reproduce using react-esc-example:

  1. Edit AsyncComponent.js

import React from 'react' import PropTypes from 'prop-types' import Helmet from 'react-helmet' import { resolve } from 'react-esc-resolver' import Storage from 'react-esc-storage'

class Async extends React.Component { static propTypes = { fetchData: PropTypes.func.isRequired, jsonPlaceholder: PropTypes.object.isRequired, }

showToken () { const token = Storage.get(Storage.COOKIE, 'token')

if (token) {
  return (
    <div>token: {token}</div>
  )
}

}

render () { const { jsonPlaceholder } = this.props

return (
  <div>
    <Helmet title="Async" />

    <div>
      { JSON.stringify(jsonPlaceholder) }
    </div>

    <div>
      {this.showToken()}
    </div>

  </div>
)

} }

export default resolve('jsonPlaceholder', props => props.fetchData().then(action => action.payload))(Async)


2. Add the cookies to your browser

document.cookie="token=\"12345678\""


3. Refresh the page (`localhost:3000/async`) to make sure that you can see your token
<img width="454" alt="screen shot 2018-03-19 at 15 22 17" src="https://user-images.githubusercontent.com/5260027/37587367-587b2cb6-2b89-11e8-937a-5e8934a3fb5d.png">

4. To simulate Sally I used a simple bash script:

!/bin/bash

for i in {1..20} do curl -v --silent http://localhost:3000/async 2>&1 | grep token: done



5. Execute the script
6. Immediately reload the page (CMD + R)
7. Take a look at stdout. Sally have got Bob's auth token :(
<img width="701" alt="screen shot 2018-03-19 at 15 32 45" src="https://user-images.githubusercontent.com/5260027/37587904-d27be662-2b8a-11e8-9585-cffa57ebd982.png">
TriPSs commented 6 years ago

Some simpel questions:

  1. Which version of esc?
  2. Which version of esc-storage?

I have the feeling this happens because it's the same browser session.

kuatro commented 6 years ago

@TriPSs Just cloned react-esc-example (master branch) and successfully reproduced the glitch.

TriPSs commented 6 years ago

@kuatro I did not have the time yet to update that project :( Could you plz update esc to at-least 4.0.0-beta.10 and are you sure it does not have to do with the same browser session?

If not then i will check it tonight.

kuatro commented 6 years ago

@TriPSs np! are you sure it does not have to do with the same browser session? — absolutely sure.

TriPSs commented 6 years ago

@kuatro I updated the project in the cleanup branch.

Chrome: image

Terminal: image

As you can see i don't get a token in the response. Could you check it with that branch to? (Note remove node_modules and lock file)

kuatro commented 6 years ago

@TriPSs I tried with the new branch, but it didn't fix the problem. Could you please try to repeat the steps above?

TriPSs commented 6 years ago

@kuatro I tried it again: Chrome source code: image

Terminal: image

Firefox: image

I'm sorry but i can't reproduce the problem.

kuatro commented 6 years ago

@TriPSs You need to start the script and reload the page in the browser several times to catch the bug. And actually the script returns an output only in case of there is token: string is present on the page.

screen shot 2018-03-20 at 18 39 48 screen shot 2018-03-20 at 18 39 21
TriPSs commented 6 years ago

@kuatro Thanks! I see what you mean, i will investigate it!

TriPSs commented 6 years ago

@kuatro I think this has to do with how setup the Cookie for esc-storage

// Add Cookie to global so we can use it in the Storage module
global.cookie = new CookieStorage(ctx.cookies)

Will look into it on how we can solve this! If you have any further info or ideas let me know!

kuatro commented 6 years ago

@TriPSs I have a feeling that the problem is related to Koa caches the response for requests coming in at the same time. Still looking for the way to check this theory.

TriPSs commented 6 years ago

@kuatro Starting to suspect the same, i added a log that logs the token when processing the request. It logs the token once but the terminal and chrome both get the token.

Do you know if this is also a issue when you access it simultaneously on chrome and firefox?

TriPSs commented 6 years ago

@kuatro small update, i'm working on a project that extracted the source of esc to it's own project and i found out that this application does not have the bug.

So it is probably still something in esc.

kuatro commented 6 years ago

@TriPSs Found the way to prove that the problem isn't in Koa response cache. I've added a random number to the view, and in a moment of cookies mix-up the number was different in a browser and terminal.

Do you know if this is also a issue when you access it simultaneously on chrome and firefox?

I asked my friend to help me with this experiments. It's much harder, but we got the cookies mix up several times.

TriPSs commented 6 years ago

@kuatro Could you tell me a little more on how your project uses esc? What rendering type etc.

As i'm not able to reproduce the issue in my own projects and we are in the example, maybe we can pin point it more to a certain configuration and find out what is causing it.

kuatro commented 6 years ago

@TriPSs Actually I used an ejected first version of react-esc to make our internal tool. Some of my coworkers faced the problem, and it took a time to figure out, what is going on 😄. Then I just used react-esc-example to check if the problem still exists.

PS. Our project is a pretty regular react-redux app, which communicates with RESTful API using Axios. To authorize requests we use a token, which is stored in cookies.

PPS. Hit the wrong button, sorry

TriPSs commented 6 years ago

@kuatro I do the exact same thing with two other projects (they use esc package, they use 4.0 alpa and 3.5) and i can't reproduce it there, i also have one project that ejected the source and that one does also not contain the bug (project is ejected last week).

So i think it can take me a little while to find out why the example project has this problem.

TriPSs commented 6 years ago

@kuatro I found a solution to the problem!

It had to do with react-esc-storage, see this commit for the changes.

Plz reopen the issue if the problem still occurs after you applied these changes!

kuatro commented 6 years ago

@TriPSs Wow, I believe that was a long research. Thank you! I'll the solution a bit later.