hyper
hyper copied to clipboard
Retief/cookiestore
Added a session store for storing session info in an encrypted cookie, and added tests for session stores. This does include a breaking change to the SessionStore class (put needs to return the updated id), but that seems reasonable to me -- it's a one line fix.
Looks like travis might be complaining about simple.JSON? I can swap it out easily enough, but I'm not sure what I should replace it with.
Hi, thanks for the PR!
Regarding the session IDs: I haven't had time to look that closely, but it seems inconsistent with the SessionStore
class. If a consumer of the session store use its operation to create a new ID, that would be a useless string, as the "real ID" would be the encrypted cookie with this store. That seems like a very confusing API.
I suggest we either adapt the type class to fit a broader use, or separate it into two different responsibilities, e.g. the existing SessionStore
for a key-value-like database (Redis, Memcache, in-memory) which might be used by many session backends, and some other SessionStrategy
or SessionBackend
that works for both the cookie and database cases. This is just my first observation, might be misunderstanding something here.
@paluh input?
Thanks!
The new definition of SessionStore should still work just fine with key-value like backends. Just return the id that was passed in and you are all set (see my changes to InMemorySessionStore). SessionStore users need to make sure to use the most recent key, but that doesn't add that much complexity. For example, saveSession needed a two line change to work with the new definition.
Once I updated the SessionStore definition, CookieStore plays nicely with that definition. Yes, newSessionID doesn't do very much. However, consumers don't need to care -- as long as they use the latest key, it works just like a standard key/value store.
That said, if you want to rework this code in some manner, be my guest. This was a learning exercise for me more than anything else, and if you want to redo or skip parts of it, have fun. You probably want to keep the bugfix on line 151 of Session.purs, and I think SessionSpec.purs does a decent job of testing InMemorySessionStore/getSession/saveSession/deleteSession even if you strip out the CookieStore tests. Beyond that, do what you want with the code.