fix: make session clearing resilient

[nerdoza]

This PR addresses an issue in the session manager utility which results in an already cleared session returning when useSession, session.update, getSession, updateSession, or sealSession are used after calling session.clear or clearSession.

This ensures that after clearing the session, a new blank session is initialized on the event context so that subsequent session mutation calls cause updates on the newly initialized session, rather than restoring the original session from the event context.

There are still edge-cases where the session can be restored, but they require subverting the use of the provided methods and session manager. This fix largely relies on the understanding that the event.context.sessions object is the source of truth for expected session state, but in the absence of a valid session in the context, the original session will still be reconstructed.

The documentation for the session management has also been updated to add a section on session rotation, which should lend in resolving issues with the downstream nuxt-auth-utils issues.

Closes #1004

[sandros94]

Just out of curiosity: I imagine this will have a cost on memory 🤔

  // Initialize a new empty session object
  event.context.sessions![sessionName] = {
    id: generateId(config),
    createdAt: Date.now(),
    data: new EmptyObject(),
  } satisfies Session;

PS: I messed up the comment 🙄

[nerdoza]

Just out of curiosity: I imagine this will have a cost on memory 🤔

This is a very small object that is already instantiated on the event.context.sessions object in almost all other conditions, so it doesn't really use any more memory than before, though it might slightly reduce memory savings by clearing the session compared to the current implementation.

As I looked at the code, though, I did realize that it deletes a property (something that normally should be avoided) and then redeclares the same property for the new session object. As such, the 370ce91 commit essentially combines the calls to make it a clobber operation. Ultimately, I expect it will have 0% impact on the final runtime performance, but it looks a little bit cleaner this way 😝.

[nerdoza]

@pi0, this PR is ready for review and possibly merge.

[pi0]

Thanks for the well-written explanations.

For session rotation, I believe we might need a new util or at least flag on clear, normal clear() means logout and should actually clear all data. Also, by having an explicit way of calling rotate, we can make user usage easier to not make them cache previous value and restore with 3 lines of code but only renew createdAt + id

The refactors of this PR look nice addition, btw. Can you please convert this PR to a refactor only and then a subsequent to draft idea for explicit rotation?

[nerdoza]

@pi0, updates to remove documentation about session rotation have been applied. I agree with your point about clear() and for clarity this implementation does remove the data and does setup a new session stub to prevent session restoration by the next session call, but because it bypasses the cookie setting phase it won't actually propagate a new session until updateSession() is called.

[pi0]

This should be reverted to allow this PR move forward as a refactor step.

[nerdoza]

I'm a bit confused @pi0. This line is the main point of this PR. If you want to isolate changes I'd be more apt to revert the refactor changes on this PR and apply them on a separate PR intended for refactor changes. Thoughts?

[pi0]

We can do this as part of #1022 for session rotate. clear purpose is to destroy session fully not to create a new one.

Alternatively we can set a _destroyed flag to previous object to indicate it is invalid.

[nerdoza]

@pi0, I'm in complete agreement on that point, but the implementation at the code level is not 1:1 with the user experience. When the user calls clear() they want the session cleared, and then if they call update() they would expect that this creates a new session (or at least that's what I would assume). Because the update() call is eagerly restoring the session from the request, calling update() returns a mutation of the same session that was previously cleared.

TLDR; If a new session is not set on the context.sessions[sessionName] when clear() is called, then the session isn't actually cleared, it's just temporarily reset (and will be restored on next access call).