Semantics for SessionStore::save method #182
-
What should happen if the save method finds that the session does not already exist? These are the two choices I see: Store the record regardlessStoring the record seems really bad as it would allow a user to set a session id cookie manually and then make a request that causes a change to session data which would then causing the server to store session data. This sounds like it is begging for a compromise since it's allowing an angle for an attacker to store data for a arbitrary id. Also, the MemoryStore just stores data for whatever record is handed to the save method. I seems like the store should refuse to insert a session whose id does not already exist in the store. That seems like a security issue waiting to happen. Also, memorystore should probably implement the correct semantics as an example to other store implementations. Generate a new sessionGenerating a new session without showing some kind error (presumably a 5xx) to the user seems not possible with the current interface. Is the plan to change this after the work for detecting session id collisions. |
Beta Was this translation helpful? Give feedback.
Replies: 1 comment
-
It's a good question and I thought about this a bit while we considered the collision issue. Ultimately it's undefined behavior that's left up to the store implementation. It's certainly not wrong to throw an error. ( If we assume that only tower-sessions is allowed to mutate the persisted session state (which is what we expect) then really it shouldn't be possible to save before create--that could be a bug in the crate. However, if an application decides to do something else and does mutate that stored state then we really can't make any guarantees. |
Beta Was this translation helpful? Give feedback.
It's a good question and I thought about this a bit while we considered the collision issue.
Ultimately it's undefined behavior that's left up to the store implementation. It's certainly not wrong to throw an error. (
Error::Backend
with a meaningful message.) Or issue a warning and save via upsert. Or simply upsert transparently.If we assume that only tower-sessions is allowed to mutate the persisted session state (which is what we expect) then really it shouldn't be possible to save before create--that could be a bug in the crate.
However, if an application decides to do something else and does mutate that stored state then we really can't make any guarantees.