Softdelete and race condition on logout

[0x0ece]

Hi, currently express-session + connect-typeorm suffer from a race condition on logout:

1. ReqA starts and maybe modifies the session
2. ReqB arrives and logs out
3. ReqA terminates after ReqB and overrides, by resaving the session in the db

One possible solution is to add an extra column destroyed to the schema (default 0). Destroying a session would only flag the record as destroyed=1. All updates should only update when destroyed=0.

In short, it's a soft delete. It relies on the current cleanup to purge sessions. By adding the extra condition on updates, it'd prevent the race condition.

I can file a PR but since it's a pretty profound change, I wanted to check in first. Thoughts? Thank you.

[nykula]

Hi! Asking the users to add a column to their entity is a major API change, but it's time to bump the major anyway. Old typeorm has known dependency vulnerabilities, so before any new release I should bump the version requirements and remove the code paths no longer used. I don't know when I'll have time to work on this, so I welcome your pull requests. #7 (test cases for custom type-safe session fields) and #24 (debug tediousjs Bigint issue) have been on my todo list for quite a while, too.

[0x0ece]

ok thanks - I'll work on it. Definitely agree it's a major API change.

[0x0ece]

FYI, in the meantime we've released our modified version of express-session:
https://github.com/saasform/saasform/tree/main/packages/express-session

The new MemoryStore prevents resaving destroyed sessions, i.e. does what I'm planning to implement here.

We also added tests to validate that the original express session suffers from the race condition, while in the new implementation the race condition is fixed.

[0x0ece]

I opened a PR - it turned out to be much more simple and at the same time much more complex than expected. Simple because typeorm has the soft delete. Complex because just the soft delete didn't solve the update issue.

Please take a look at the comments in the PR and lmk your thoughts. If this is too big of a change we can also publish this as @saasform/typeorm-connect like we did for express-session.

[nykula]

LGTM so merged, thank you! To anyone also reading this, please see if you can suggest how to work around the need for (session as any) cast with recent express-session typings.

[0x0ece]

Great, thank you! Please lmk when you file a new release so I'll note the version.

[jdhuntington]

Thanks for this PR!

I noticed that with this change, any soft deletes are likely to persist forever. There might be a subtle timing issue if a logout happened close in time to when a session would expire, but I believe #27 is a useful fix for most use cases.

[freshgiammi]

Merged.