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

The cookies object recevied in the handlers varies depending on the placement of the handler within the setupServer function. #2126

Open
4 tasks done
NZepeda opened this issue Apr 5, 2024 · 5 comments
Labels
bug Something isn't working needs:triage Issues that have not been investigated yet. scope:node Related to MSW running in Node

Comments

@NZepeda
Copy link

NZepeda commented Apr 5, 2024

Prerequisites

Environment check

  • I'm using the latest msw version
  • I'm using Node.js version 18 or higher

Node.js version

18.12.1

Reproduction repository

https://github.com/NZepeda/msw-cookies-example

Reproduction steps

  1. git clone https://github.com/NZepeda/msw-cookies-example
  2. cd msw-cookies-example
  3. pnpm install
  4. pnpm test mock

Current behavior

In my test, I've set the cookie to be sent with the request via the store.add function of @mswjs/cookies. I have a few handlers for my server that check the received cookie from the request and test it against a static string authorizedSessionCookieValue.

The first test passes as the received cookie value is the same as authorizedSessionCookieValue, but the second test fails. This seems to be due to the placement of the handler within the setupServer function. It seems like the first handler (which the first test hits) receives the correct cookie value, in my case { MyCookie: 'ABCD1234' }, however, the third handler (which the second test hits) receives a cookie value of { MyCookie: 'ABCD1234, MyCookie=ABCD1234' }, which causes the test to fail.

Expected behavior

I would expect the received cookies object from each handler to be the same.

@NZepeda NZepeda added bug Something isn't working needs:triage Issues that have not been investigated yet. scope:node Related to MSW running in Node labels Apr 5, 2024
@kettanaito
Copy link
Member

Hi, @NZepeda!

It may not be the best idea to use @mswjs/cookie that way. You are interfering with how MSW picks up cookies that way, it seems. If you want to have an initial cookie set, mutate document.cookie instead. In other words, do what your app would do.

I also have a concern that you aren't testing anything in your test: https://github.com/NZepeda/msw-cookies-example/blob/d7d900d6e0202c8b8a522b1c88b4958b8782c181/src/test/mock.test.ts. What the test cases assert right now is that MSW handles cookies correctly. If this is a trimmed test case you created to illustrate the issue then it's okay. But if it's not, don't test third-party libraries, test your code instead.

@antoinetissier
Copy link

Hi @kettanaito,

What we are trying to simulate is a prod setup with an Http-Only, Secure cookie.
In a browser Http-Only are not readable/mutable through document.cookie.

Is there no way to set this up in msw + jsdom ?

This is related to #1586

@kettanaito
Copy link
Member

@antoinetissier 👋

Since you cannot read the HttpOnly cookie in JavaScript, what would you test then? Nothing in your application can depend on it. I suspect you are talking about forwarding that cookie to the server request (i.e. credentials)? If that's the case, it's done by the browser for you.

Please tell me more about your test case.

@antoinetissier
Copy link

antoinetissier commented Apr 19, 2024

We use a cookie to authenticate a WebSocket connection to the server.
We get that cookie by sending a request to the server with a JWT, and if it is valid the server responds with a Set-Cookie header where the cookie is HttpOnly and Secure.
The cookie is stored in the browser securely (as it's not accessible through JavaScript with document.cookie), and sent upon further server requests (we adapt the credentials request property if necessary for cross-domain deployments).
As mentioned above, this allows to then open a WebSocket in particular, but the cookie can also be used to authenticate any other further HTTP request to the server (our Java uses Spring security and this is the default behavior).

When the UI assets (JS, HTML, css, etc.) are served by the server itself (which is the deployment that we recommended), then when you try connecting to the UI, you must first pass the Spring security layer before the UI assets can be fetched.
In that case, by the time any JavaScript is run, the session cookie is already set in the user's browser.

We are looking to test all this authentication flow with msw.

@NZepeda
Copy link
Author

NZepeda commented Apr 19, 2024

To add on to this, @kettanaito, as you recommended above, I've removed usage of the @mswjs/cookie package to set the cookie and instead, I'm directly mutating document.cookie. The same error persists. I've updated the linked repo to demonstrate the issue.

For the test, I've set up a server with three handlers, two of which depend on the cookies object given by the resolver function. These handlers check the cookie similarly by comparing cookies[sessionCookieName] to an expected cookie value.

Within the test suite itself, I set the session cookie once by mutating document.cookie within the beforeAll. There are two tests, each hitting a different handler. The first test passes because the cookie has the expected value. The second test fails because the cookie value is now unexpected.

This seems like a bug within msw because I expect that both tests should pass because the cookie was only mutated once in the beforeAll. I do not expect the cookies object given by the resolver to change between handlers simply due to their order within the setupServer function.

For more clarity:
This is the value of the cookies object given by the resolver in the first handler:

{ MyCookie: 'ABCD1234' } <- This is good

This is the value of the cookies object given the resolver in the third handler:

{ MyCookie: 'ABCD1234, MyCookie=ABCD1234' } <- This is bad

Can you confirm whether or not this is expected?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working needs:triage Issues that have not been investigated yet. scope:node Related to MSW running in Node
Projects
None yet
Development

No branches or pull requests

3 participants