Add config to allow permanent session in cookie

[colinrotherham]

I'm currently working across two teams with long form journeys.

A quick prototyping scenario

1. Enter lots and lots of form data
2. Reach the Nth page in a journey
3. Edit a view or route

What happens?

1. The prototype is restarted by nodemon
2. All typed in session data is now lost 😭

What's in this pull request

I've added the following config:

{
  // Enable cookie-based session store (4KB limit, persists on restart)
  useCookieSessionStore: 'false'
}

When set to true it swaps express-session with Mozilla client-sessions so retains compatibility with req.session and persists all session data in a cookie.

[colinrotherham]

Rebased with latest from master

[colinrotherham]

Renamed the option to useCookieSessionStore

[colinrotherham]

As discussed on #472 there were some security concerns around file sessions stored on the server.

e.g. if Heroku was hacked, your encrypted sessions can be stolen and brute forced.

With this approach we avoid that:

1. Data is encrypted by Mozilla client-sessions
2. Data isn't stored on Heroku or on disk, it's in a cookie
3. Data is persisted when the prototype restarts
4. Data is persisted if the prototype is deleted and cloned again

[NickColley]

I have tested this by:

1. Going through the pass data example
2. Confirming the session has my inputted data
3. Restarting the server
4. Session is lost

---

1. Turning on 'useCookieSessionStore'
2. Going through previous 1-3
3. Session persists

I have also tested the 'clear data' function in both modes, and confirmed the session was removed.

[NickColley]

Great work!

I think the 4kb limitation is reasonable given the pros for this approach, and we can always consider the file system approach in the future if it does become and issue.

I will need a review from another team member before we can merge this.

[fofr]

This will be really useful for us at DfE 👍

[hannalaakso]

Tested to work correctly 🙂 Left a couple of comments.

[hannalaakso]

secret used to be randomised. Could it use sessionName instead?

[colinrotherham]

@hannalaakso Yep, no problem.

Had to remove the randomisation, otherwise you're locked out of your session on restart 😳

[hannalaakso]

More of a discussion but would it be worth adding to the comment that if you exceed the limit, there is no notification so you need to bear this in mind with large prototypes? In case people expect something to happen when you go over the limit.

I tested exceeding the limit by pasting in a super long string in vehicle-registration and (unsurprisingly) the value doesn't get retained which you find out in check-answers. If there is an existing value for that attribute in session data, it looks like the existing value is retained.

Like @NickColley said we might just want to go ahead with this and see if the limit becomes an issue in the future.

[colinrotherham]

@hannalaakso Yeah I'll add a comment.

Yup that's it. Node.js will happily send a cookie that's too big, but the browser will ignore it and keep the last cookie it had that was under the 4KB limit for the domain.

[colinrotherham]

Hi @hannalaakso, thanks for the review.

I've addressed both your comments and rebased from master.

In future (since the session is mainly text) a little gzip magic could be used to compress it down. It'll reduce cookie size down to about 65% of the original.

[hannalaakso]

Thanks @colinrotherham, this looks great 🙌 💯 We can look into approaches of reducing the cookie size if we see it becoming an issue 👍

[joelanman]

not sure if there's an advantage to using a predictable string for the 'secret' of the cookie? I think it should be somewhat secret?

[joelanman]

ah, is this because when the app restarts it won't know the randomly generated secret? Hmm.. maybe we could store the secret somehow. If needed

[colinrotherham]

Yeah that's the trade-off sadly.

The old approach re-hashed the secret on every restart. This would lock you out of your cookie causing it to be regenerated.

Sounds like an idea though.