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

[Bug] Cookies not sent #1676

Open
sebastien-f opened this issue Feb 27, 2024 · 17 comments · May be fixed by #4038
Open

[Bug] Cookies not sent #1676

sebastien-f opened this issue Feb 27, 2024 · 17 comments · May be fixed by #4038
Assignees
Labels
bug Something isn't working help wanted Community contribution is welcome module-request

Comments

@sebastien-f
Copy link

Version: 1.9.0

I have a collection with 2 requests inside, login and foo. When running login (POST, with data) I get a response with a set_cookie header :

< set-cookie: access_token=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJsb2dpbiI6InMuZmlub3RAY29tbS1pdC5pbyIsImlhdCI6MTcwOTA1MTcwM30.OeXYsgE5_U4ClUgvGi2vK0tJOiBBc96EMHBTcNnboqM; Path=/; HttpOnly; Secure

When running request foo (GET, not data), cookie is not sent despite it being listed in the cookies visible with the bottom-left cookie and get rejected because of missing access_token.

If I manually add the cookie header to the foo request, the request is properly authorized.

Also, cookie is properly updated each time I call login.

Both requests uses same protocol + url + port.

Both Store and Send Cookies automatically are enabled. I also tried to deactivate+save then activate+save as I was upgrading from 1.2.0.

I also tried using the runner, in case that could have an impact, but same result.

On a side note, I tried looking at the documentation on how to use the cookies, but could not find any reference.

@codingskynet
Copy link

codingskynet commented Apr 3, 2024

Could you try API with http://? I found that Postman adds Secure cookies on header even on http:// request, but bruno strictly handles it. If trying API with http:// with Secure cookie, it does not send cookie on http:// request.

https://developer.mozilla.org/en-US/docs/Web/HTTP/Cookies#restrict_access_to_cookies

@sanjai0py sanjai0py added bug Something isn't working help wanted Community contribution is welcome labels Apr 3, 2024
@sebastien-f
Copy link
Author

@codingskynet thank you for the suggestion, but the api is hosted on https and there is no way to makes calls over http. Not sure what Postman has to do here. Did you meant Bruno ?

@codingskynet
Copy link

Actually, I experienced like this on https server(maybe supports auto-upgrade to https from http):
Postman: If I send request on http, SECURE cookie can be sent.
Bruno: If I send request on http, SECURE cookie cannot be sent. On https, working well.

@JoshuaHintze
Copy link

Just adding my thoughts here. It would be GREAT if Bruno sent secure cookies over a localhost so we don't have to either

a.) setup a secure certificate for localhost testing or
b.) conditionally set secure to off for local testing on a backend

Webrowser's already do this.

Browsers like Chrome recognize localhost as a secure origin by default, even if it's not using HTTPS. This means that if you're developing locally and accessing your site via localhost, you should be able to set and use Secure cookies.

But bruno does not.

@JoshuaHintze
Copy link

JoshuaHintze commented May 3, 2024

Ok looking quickly at the code, the issue I think is here:

https://github.com/usebruno/bruno/blob/main/packages/bruno-electron/src/utils/cookies.js#L14

specifically

const getCookiesForUrl = (url) => {
  return cookieJar.getCookiesSync(url);
};

Bruno uses tough-cookie. according to those docs you can send in an options opbject as a second argument.

By default it considers whether your connection is secure if you have https

secure - boolean - autodetect from URL - indicates if this is a "Secure" API. If the currentUrl starts with https: or wss: then this is defaulted to true, otherwise false.

So a suggest improvement might be to do something like this

const getCookiesForUrl = (url) => {
  const secure = url.startsWith('https:') || url.startsWith('wss:") || url.startsWith('http://localhost');
  return cookieJar.getCookiesSync(url, {secure});
};

Thoughts?

@dimasanwaraziz
Copy link

Ok looking quickly at the code, the issue I think is here:

https://github.com/usebruno/bruno/blob/main/packages/bruno-electron/src/utils/cookies.js#L14

specifically

const getCookiesForUrl = (url) => {
  return cookieJar.getCookiesSync(url);
};

Bruno uses tough-cookie. according to those docs you can send in an options opbject as a second argument.

By default it considers whether your connection is secure if you have https

secure - boolean - autodetect from URL - indicates if this is a "Secure" API. If the currentUrl starts with https: or wss: then this is defaulted to true, otherwise false.

So a suggest improvement might be to do something like this

const getCookiesForUrl = (url) => {
  const secure = url.startsWith('https:') || url.startsWith('wss:") || url.startsWith('http://localhost');
  return cookieJar.getCookiesSync(url, {secure});
};

Thoughts?

It's can be merge your suggestion to next release?

@KiritaniAyaka
Copy link

const getCookiesForUrl = (url) => {
  const secure = url.startsWith('https:') || url.startsWith('wss:") || url.startsWith('http://localhost');
  return cookieJar.getCookiesSync(url, {secure});
};

This implementation does not look comprehensive enough.

See this spec

It should do some checks with the host instead of simply checking the URL if starts with a specific string. Furthermore, it is also necessary to check if the URL is a local address by CIDR matching.

@JoshuaHintze
Copy link

Just saw this issue again with a different backend. It would sure be nice to send out SECURE cookies on localhost for local development.

How friendly is Bruno to accepting PRs outside the core dev team?

@thiagocavalcanti
Copy link

thiagocavalcanti commented Oct 1, 2024

For those with this problem using http with localhost, one temporary fix that I'm using ngrok to create a https endpoint and it worked fine

@sreelakshmi-bruno
Copy link
Collaborator

Hi, I'm not able to reproduce the issue on 1.37. Can you check and let me know if it still persists? Thanks!

@KiritaniAyaka
Copy link

Hi, I'm not able to reproduce the issue on 1.37. Can you check and let me know if it still persists? Thanks!

I upgraded my Bruno from 1.20 to 1.37 just now. It could be reproduced and behaves the same as 1.20.

We have to set the cookie as non-secure for Bruno to send cookies on localhost even though localhost is a secure context.

@Chriss4123
Copy link

Hi, I'm not able to reproduce the issue on 1.37. Can you check and let me know if it still persists? Thanks!

I can reproduce the issue too. If the cookie has secure set to True it will not send it to localhost or any http://

@tusharsnx
Copy link

tusharsnx commented Feb 9, 2025

Can we have an override for "localhost" to be considered "secure"?

We have to set the cookie as non-secure for Bruno to send cookies on localhost even though localhost is a secure context.

Localhost is a secure context only when you have a guarantee that it'll resolve to the loopback address (or the application does that on its own).

@KiritaniAyaka
Copy link

Localhost is a secure context only when you have a guarantee that it'll resolve to the loopback address (or the application does that on its own).

That's a nice point I missed. As this spec says, only treat localhost as secure context when [let localhost be localhost] is confirmed.

In simple terms, you're right.

@Chriss4123
Copy link

Hey, I opened a PR for this issue here that implements an RFC 6761 compliant URL trustworthiness check that allows secure cookies to be sent to localhost or another loopback address.

@JoshuaHintze
Copy link

Thats great @Chriss4123 maybe @sreelakshmi-bruno can review and get this in.

@sreelakshmi-bruno
Copy link
Collaborator

Hi, thanks for the contribution, we'll look into it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Community contribution is welcome module-request
Projects
None yet