Fix: Session middleware issues

[Spedoske]

Fix: Session.Regenerate does not set Session.fresh to be true.
Fix: Session should be regenerated if the session can not be found in the storage.
This PR is intended to close #1395 issue and #1408 issue.

Tests have been added.
Some old tests have been modified.
If there are some questions, I will answer them.

[welcome]

Thanks for opening this pull request! 🎉 Please check out our contributing guidelines. If you need help or want to chat with us, join us on Discord https://gofiber.io/discord

[ReneWerner87]

Thx

Since these bugs were not found by the tests, can you please add some to ensure that future code changes do not cause this problem again?

[hi019]

Hey, thanks for the PR! Some comments:

Fix: Session.Regenerate does not set Session.fresh to be true.

I think the current behaviour is sufficent. Session.Regenerate only resets the session ID. The data inside the sessions still persists, so technically the session is not new.

I was working on a similar problem a few days ago, where Session.SetExpiry only sets the session expiry if the session is fresh. Instead of setting the session to fresh in the function, I added a new private field to Session called shouldSetSession. Here is my commit (it's on a testing branch, I haven't created a PR yet): 3d821f4. Do you think we could do a similar thing here?

[Spedoske]

It’s true that we need to set session for user after Regenerate and SetExpiry since the session id and the expiration time changes, which are stored in the user’s browser.

I agree that you should set shouldSetSession to be true when SetExpiry for a session, because the session is not changed, only the expiration time changes.

I am curious which is a more appropriate definition of regenerate ID, here are two versions:

1. Remove all data associating with the old session, and assign a new session to the user.
2. Assign a new session to the user, copy the data for the new session from the old session, and finally destroy the old session.

For version 1, the new session do not have any data. So the session is fresh.
For version 2, the data are transferred from the old session to the new session. So the session is not fresh, but session id changes.

Both of these two versions requires fiber to set session for user.

According to the main branch of fiber, the Regenerate implements the version 1, the function destroys all data in the storage for the old session, and assigns a new ID for the session with no data. So I think we should set fresh to be true.

If you think that version 2 is the more appropriate implementation, then we should set shouldSetSession to be true.

[hi019]

My mistake. I looked at the Regenerate code again, and it does delete the session data in addition to regenerating the ID. The PR looks fine as it is, and I'll test it out tomorrow!

[welcome]

Congrats on merging your first pull request! 🎉 We here at Fiber are proud of you! If you need help or want to chat with us, join us on Discord https://gofiber.io/discord