Questioning the plugin responsibility on checking secure connection

[qnp]

Prerequisites

• I have written a descriptive issue title
• I have searched existing issues to ensure the issue has not already been raised

Issue

Hello,

I'm questioning the relevance of checking whether the connection is secure before responding with a secure session cookie, as done here.

To my understanding, this is not the responsibility of the server to choose whether a secure cookie should be sent or not. It is the browser that own the responsibility of using cookies according to their policies. Hence, the cookie should always be set to the response, even though the subsequent unsecure connections cannot use the cookie. Am I missing something ?

References:

• https://developer.mozilla.org/en-US/docs/Web/HTTP/Cookies#block_access_to_your_cookies
• https://security.stackexchange.com/questions/251675/should-the-server-send-secure-cookies-on-unsecured-http-response

Best regards

[krlwlfrt]

It's not just questionable, but it causes confusing behavior, when fastify is behind a reverse proxy. The reverse does not use a "secure" connection - obviously, but simply use HTTP without the S to proxy the requests to the fastify instance.

When using true for secure that causes the session cookie to not be set by the plugin precisely because of this check.

Using 'auto' as value for secure circumvents this issue, but it's confusing behavior and took me quite some time to debug. And I'd rather use true to be explicit about it.

[pluspol]

We stumbled over the same issue. auto helped (for now) however it also leads to the cookie not having secure set which is also not ideal.

express-session also checks for a secure connection but respects trustProxy as seen here:
https://github.com/expressjs/session/blob/master/index.js#L648

[mcollina]

PRs are welcome ;)

[BaliBalo]

I has a similar issue to @krlwlfrt and @pluspol where my server was behind a load balancer acting as a proxy, so the connection was considered insecure and cookies were not set.

I was about to submit a PR to allow a trustConnection option (in the format 'always' | 'simple' | 'trust-proxy' | custom-function), but just realized that the more appropriate way to deal with this is actually to use the global Fastify option trustProxy, which updates the request.protocol value based on the proxy headers (some code related to proxies actually got removed in #238 in favor of using that global option, so probably best to not re-introduce something similar)

The original question still remains (whether this plugin should check on secure connections at all), but is somewhat irrelevant for people like me using a proxy, since Fastify's trustProxy should properly reflect the proxied request - this is mentioned in this repo's Readme but I had missed it