Handle set-cookie from a redirect response fixes #13

[maccyber]

TODO:

• Add maxRedirect, redirectCount
• Test better
• Handle if redirect: "manual" is sent through fetch options
• Handle when redirected if redirect: "error" is sent through fetch options
• Set response.redirected when redirected
• deepStrictEqual assert on user's init so we make sure to not accidentally mutate their init
• Do not forward sensitive headers to third-party domains

[jd1378]

can we not use spread operator for this and simply set redirect on interceptedInit directly ?
the reason is spread has a cost, and I prefer to not use it when It's not needed
even if we want to clone user's init, in this case which we should have, let's do it once and when we assign the interceptedInit
also probably change interceptedInit type to WrappedFetchRequestInit ?

[maccyber]

Agreed. Tried to refactor it, but it still doesn't feel right. Must be better ways to solve it.
I hope someone else can push this over the line.

[jd1378]

thanks for the effort
I agree, it does not feel right.
I'll work on it when I have some time

[jd1378]

same here about the spread
also shouldn't it be changing interceptedInit ?

[jd1378]

also maybe do a deepStrictEqual assert on user's init so we make sure to not accidentally mutate their init from now on ?

[jd1378]

I have committed some changes
the reason for deciding to not handle Request was that it was not following whatwg fetch specs
so I thought its better to only replace headers and redirect using our init instead and let fetch handle the Request object

also I think the naming was good the way it was, so I renamed them back, sorry

the only thing remaining from tasks is Handle deletion of cookie headers which I don't quite understand
can you explain it a bit more ?

[jd1378]

I'm ready to merge, the only remaining task which I did not understand was "Handle deletion of cookie headers"
CookieJar already deletes a cookie if an expired cookie is set, so I think that shouldn't be a problem
please explain so we can merge this

[maccyber]

Awesome. I updated the task to "Do not forward sensitive headers (i.e. headers.authorization and headers.cookie) to third-party domains" to clearify.

I think I can create a test for it later tonight

[jd1378]

since I still did not understand, I appreciate the incoming tests :D
Links/examples in this case for the expected behavior would be appreciated
my first guess is to add support for SameSite attribute of cookies ?
and then I guess maybe what you suggest is a privacy feature ? for more strictness ?
I would be happy to help on this as well

edit: my bad, SameSite attribute is unrelated, since its only happening in browser environment
edit 2: is this related to redirection feature ? if so, the fetch should handle it using credentials option in init I believe ?

[maccyber]

Implementation of this functionality in fetch-cookie: https://github.com/valeriangalliat/fetch-cookie/blob/master/src/index.ts#L161-L166

[jd1378]

from what I understood, we were already hitting the check here and resetting it always here

so I think adding a test should make it clear if It's as I'm thinking or we need to change anything

edit: sorry, I keep missing the point
I understood now whats happening
the "headers" not just "cookie" header
my bad again

[jd1378]

I would be happy if you took a look at the test and the code guarding what it should

the cookie header part is handled differently by resetting them for every request

if everything is fine, tell me so I can merge
if you want to add more tests, you are very welcome

[jd1378]

I've also noticed I should have used hostname instead of host of URL object, because RFC 6265 under 8.5. Weak Confidentiality
So I also made that change to the code and tests and made server two use "localhost" vs "127.0.0.1" to differentiate them instead of relying on ports

[sonarqubecloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.0% Duplication

[maccyber]

Nice, looks good to me

[jd1378]

thanks a lot for this !