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

set-cookie seems to override the cookie header instead of appending the new cookies #6207

Closed
robespmun opened this issue Oct 10, 2018 · 11 comments

Comments

@robespmun
Copy link

robespmun commented Oct 10, 2018

I'm trying to test my website where users authenticate via session cookie without having to automate the whole manual logging process using the Chrome debugger solution suggested here https://github.com/GoogleChrome/lighthouse/blob/master/docs/readme.md#testing-on-a-site-with-authentication, so I'm trying to directly set the cookies using the --extra-headers flag.

Provide the steps to reproduce

  1. Run LH on an authenticated via cookie website:
    lighthouse https://authenticated-website.com --chrome-flags="--headless" --extra-headers=./headers.json

The content of headers.json is as follows:

{ "Cookie": "session_id=whatever-session-id" }

What is the current behavior?

When lighthouse accesses the website, there's a call to an authentication API that returns a set-cookie header with a csrf token. From that request onwards, the user is unauthenticated. All of the requests are sending the cookie with the csrf token (returned via set-cookie) but are completely disregarding the headers I set in the beginning, overriding the original "Cookie" header instead of appending the value. I enabled extra logging and see on the network responses that the headers received are not the expected ones:

{ "method": "Network.responseReceived", "params": { "requestId": "1000019523.17", "loaderId": "5D025634B8EEDC4CF37CCC23430142BB", "timestamp": 52153.046111, "type": "Fetch", "response": { "url": "https://authenticated-website.com/whatever.json", "status": 200, "statusText": "", "headers": { "date": "Tue, 09 Oct 2018 16:44:06 GMT", "content-encoding": "gzip", "x-content-type-options": "nosniff", "x-permitted-cross-domain-policies": "none", "status": "200", "vary": "Accept-Encoding", "content-length": "219", "x-xss-protection": "1; mode=block", "pragma": "no-cache", "last-modified": "Tue, 09 Oct 2018 15:52:20 GMT", "server": "none", "x-frame-options": "SAMEORIGIN", "x-download-options": "noopen", "strict-transport-security": "max-age=31536000; includeSubdomains; preload\nmax-age=31536000; includeSubdomains;", "content-type": "application/json", "region": "EU", "cache-control": "max-age=0, no-cache, no-store, must-revalidate", "accept-ranges": "bytes", "x-robots-tag": "none", "expires": "Wed, 11 Jan 1984 05:00:00 GMT" }, "mimeType": "application/json", "requestHeaders": { ":path": "/whatever.json", "accept-encoding": "gzip, deflate", "user-agent": "Mozilla/5.0 (Linux; Android 6.0.1; Nexus 5 Build/MRA58N) AppleWebKit/537.36(KHTML, like Gecko) Chrome/71.0.3559.0 Mobile Safari/537.36", "accept": "*/*", "referer": "https://authenticated-website.com", ":authority": "authenticated-website.com", "cookie": "csrf_token=whatever-csrf-token", ":scheme": "https", ":method": "GET" }, "connectionReused": true, "connectionId": 9 ....

What is the expected behavior?

Cookie returned in set-cookie should be appended, making the headers from that point onwards look like this:

{ "Cookie": "session_id=whatever-session-id; csrf_token=whatever-session-token" }

Environment Information

  • Affected Channels: CLI
  • Lighthouse version: 3.2.1
  • Node.js version: 9.5.0
  • Operating System: Mac OS X (also reproduced on Ubuntu 18.04.1)

I'm running Chrome 69.0.3497.100.

Related issues
#1418

@robespmun robespmun changed the title set-cookie seems to override the cookie header instead of appending set-cookie seems to override the cookie header instead of appending the new cookies Oct 10, 2018
@patrickhulce
Copy link
Collaborator

Thanks for filing @robespmun!

Just so I get this straight...

  1. You have a page that needs both a dynamic csrf_token and a session_id
  2. You are using --extra-headers to override the session_id but you still want to let the csrf_token go through
  3. --extra-headers is clobbering the csrf_token and you're just getting the session_id (or is it vice-versa)?

Is that an accurate description of what's happening?

@robespmun
Copy link
Author

The whole process is:

  1. I use --extra-headers to set a valid session_id
  2. The website makes the corresponding call to see if the user is authenticated, correctly forwarding the cookie (everything fine up to this point).
  3. The server responds with a set-cookie header meant to be appended to the cookie (if I'm not wrong this is standard cookie behaviour, at least in JS, so the cookie would look like { "Cookie": "session_id=whatever-session-id; csrf_token=whatever-session-token" })
  4. On calls after that, the cookie that is being forwarded only includes the csrf_token and not the session_id, making it look like set-cookie is overriding the cookies instead of appending the new value.

@patrickhulce
Copy link
Collaborator

Ah, I see. Yeah that's some peculiar behavior 😕 I wouldn't expect setExtraHTTPHeaders to ever be able to merge the values though so just one of them was going to be able to win.

My only suggestion for an immediate workaround would be to use puppeteer to set a real cookie as soon as the page loads or include a CSRF token with the headers.json.

We'll have to decide what the correct behavior really should be. My guess is that from the Chrome-perspective this is WAI and Chrome should not be automatically setting this cookie on all domains just because the Cookie header has been overridden.

@RynatSibahatau
Copy link

RynatSibahatau commented Jun 9, 2019

I think this issue breaks the possibility of using lighthouse on any enterprise level project on CLI mode (i.e in CI) and I add +1 for the criticality of this issue. I came across exactly the same behavior: any Set-Cookie response from backend overwrites the Cookie header from extra-headers.

  1. lighthouse http://localhost/ --extra-headers "{\"Cookie\":\"test=true\"}"
  2. Request header: Cookie: test=true
    Response header: Set-Cookie: locale=en;
  3. All subsequent requests headers:
    Actual request headers : Cookie: locale=en
    Excpeted request headers: Cookie: test=true; locale=en

So I decided to play with the code RynatSibahatau@5a8b8ce and have introduced a new CLI parameter:

lighthouse http://localhost/ --extra-cookies "[{\"name\":\"test\", \"value\": \"true\", \"url\": \"http://localhost\"}]"

This command works as expected behaviour.

If you support me to have this parameter in the lighthouse's master branch, I can do extra polishing/add extra tests and create a PR.

Thank you!

@patrickhulce
Copy link
Collaborator

Thanks @RynatSibahatau that looks like a great starting point for a PR already! Personally, I'm fairly convinced this is a worthwhile use case to support in aid of #1418 without resorting to #3837, especially in CI cases. I'd be happy to assist with your feature PR and make the case to the rest of the team.

The only issues I think I currently see is how this behavior is intended to interact with other cookie settings and the specific JSON format expected, but we can discuss those details in the PR if you wish to proceed :)

@deadlyhifi
Copy link

Is there anything I can do to help push this along? The pull request has been open for 2 months and is currently “waiting4commiter”. I could really do with this feature.

@kkdev163
Copy link

kkdev163 commented Jul 29, 2019

Hi, in my test case, I fond the main reason that why rest request will losing the cookie in --extra-headers is not the evil of set cookies.

The main reason is when chrome exists real cookie, the cookie in --extra-headers will be ignored.
And the network panel of chrome-devtools can't display real cookie sent to server.

In my test case:
real cookie: WM_NI=xxx;WM_NIKE=xxx;WM_TID=xxx;
extra-header:

{
   "Cookie": "MUSIC_U=xxxx"
}

we can use lighthouse --port to use an existing chrome instance, and set real cookie, then, we will get the test result:

image

Summary, set cookies -> real cookie -> cookie in extra-headers be ignored, and chrome' devtools may have display bug.

@patrickhulce
Copy link
Collaborator

Thanks very much for the additional debug info @kkdev163 that's super helpful! This should be addressed by #9170 if we can get it across the finish line.

@patrickhulce
Copy link
Collaborator

We'll plan to solve this use case as part of Fraggle Rock work #11313 but we're not going to solve this specific CLI flag issue.

@hellosean1025
Copy link

hellosean1025 commented Mar 9, 2021

try use chrome-remote-interface Newtork setCookie

const port = 9222
  const chrome = await chromeLauncher.launch({chromeFlags: [''], port});
  const options = {
    logLevel: 'info', 
    output: 'html', 
    onlyCategories: ['performance'], 
    port,
  };

  const remoteInterface = require('chrome-remote-interface');
  remoteInterface((launcher)=>{
    cookies.forEach(item=>{
      launcher.Network.setCookie(item)
    })
  })

  const runnerResult = await lighthouse('https://xxxxxx.com', options);

@Poliyawan
Copy link

try use chrome-remote-interface Newtork setCookie

const port = 9222
  const chrome = await chromeLauncher.launch({chromeFlags: [''], port});
  const options = {
    logLevel: 'info', 
    output: 'html', 
    onlyCategories: ['performance'], 
    port,
  };

  const remoteInterface = require('chrome-remote-interface');
  remoteInterface((launcher)=>{
    cookies.forEach(item=>{
      launcher.Network.setCookie(item)
    })
  })

  const runnerResult = await lighthouse('https://xxxxxx.com', options);

想问一下,这个remoteInterface不需要和上面生成的chrome变量联动吗?以及cookies.forEach里面的cookie是什么格式?是["_did=xxx", "key=value"]这样吗?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants