feat: @fastify/cookie responsible for the signing and unsigning

[Uzlopak]

This PR couples @fastify/cookie stronger to @fastify/session. So instead of duplicating signing and unsigning logic in @fastify/session, we expect that setting the signing related stuff in @fastify/cookie and we just consume the logic in @fastify/session.

@rclmenezes
I think before we merge other patches, we should first merge this.

To review this PR i think, it is easier by going through the commits. I could not break it down into much smaller PRs as it is basically one big change.

Checklist

• run npm run test and npm run benchmark
• tests and/or benchmarks are included
• documentation is changed or added
• commit message and code follows the Developer's Certification of Origin
  and the Code of conduct

[climba03003]

By comparing express-session, the secret option should stay in here.

[climba03003]

I would move it back to devDependency.

The user should explicitly install and register @fastify/cookie in their application.

[SimenB]

can fastify-plugin ensure the version of @fastify/cookie is at least 7.4? or is that out of scope for that module, and it might fail at runtime when accessing non-existent methods?

[mcollina]

You can require for a decorator to exist: https://github.com/fastify/fastify-plugin#dependencies

[SimenB]

Ah, right! should add that 👍

[Uzlopak]

I think my approach is much cleaner, as we just need to setup fastify-cookie correct and then can use the actual methods instead of taking also care of the secrect management.

[climba03003]

I think my approach is much cleaner, as we just need to setup fastify-cookie correct and then can use the actual methods instead of taking also care of the secrect management.

It require the user to be clear that they shouldn't forget the secret in registering @fastify-cookie.
There is no way to block them and remind them to add one before application startup.

It is totally different than the way before and may expose user in vulnerability.

[mcollina]

Here is a few questions:

1. What happens if there is no secret set to both @fastify/cookie and @fastify/session? Let's make sure an error is thrown at some point.

2. Make sure to throw if secret is passed to @fastify/session.

[Uzlopak]

@mcollina
Created fastify/fastify-cookie#203

So when no secret is set, we can feature detect if the decorators exist. If they dont we can throw a secret not set error. Obviously I can only write corresponding unit tests if the PR is merged and a new release of @fastify/cookie was published.

[mcollina]

lgtm

[mcollina]

@Uzlopak please wait for @climba03003 and @SimenB

[Uzlopak]

will do

[Uzlopak]

@mcollina

I still have to add that the plugin throws an error if a secret was provided.

[climba03003]

It does not convince me to provide secret in @fastify/cookie.

1. It leaks of length check, user can use weak secret which can't currently.
2. User can not switch from express-session to @fastify/session with just config copy and paste.

Point 2. is not as important as 1..
If it the concern is code de-duplication, why can't we use the signer factory from @fastify/cookie?

[rclmenezes]

My last few patches have been all about making @fastify/session compatible with express-session.

This PR creates a new incompatibility: what if someone provides a different secret to both express-cookie and express-session? They probably shouldn't, but the effect in express would be that non-session-related cookies have one secret and sessions have another.

But honestly, I can't think of a real use case to have differing secrets here. And I think good code / practices is more important than express compatibility.

I suspect we should tell users in this scenario that they should migrate to one secret. They can do so by setting Express's cookieParser to: cookieParser([sessionSecret, oldSecret])

[Uzlopak]

Dont worry. Working on a different branch.

[Uzlopak]

Closing in favor of #129