alexedwards / scs

HTTP Session Management for Go
MIT License
2.02k stars 165 forks source link

Redis example is using a different Redis library #173

Closed robvdl closed 11 months ago

robvdl commented 11 months ago

This page https://github.com/alexedwards/scs/tree/master/redisstore

Seems to be using github.com/gomodule/redigo/redis, I was using github.com/redis/go-redis/v9 which seems to be the official bindings and probably should be the preferred library to use now.

robvdl commented 11 months ago

I think I have two choices, either copy the file redistore.go into my project and update it to use go-redis.

Or switch from go-redis to redigo.

There isn't a lot of code in redistore.go so I could probably easily do my own based on go-redis/v9

robvdl commented 11 months ago

Interesting comparison: https://redis.uptrace.dev/guide/go-redis-vs-redigo.html

I do like the looks of go-redis from that page so would prefer to continue using that.

robvdl commented 11 months ago

I wonder for the scs project if it would make more sense to switch to the official redis-go v9 library by redis themselves, and drop redigo.

If I was to submit code for the newer go-redis v9 library, if the old code needs to stay around or if it should be deleted.

jum commented 11 months ago

There are already two adapters for redis in the SCS repo, so you can use either redigo or go-redis, whatever suits you better. Look into the goredisstore directory for the other redis driver.

robvdl commented 11 months ago

Thank you, somehow I hadn't seen that.

robvdl commented 11 months ago

I see now what happened.

This table is the first thing I jumped to in the README and it pointed only to the one redis adapter: https://github.com/alexedwards/scs#configuring-the-session-store

I didn't dig deep enough to realise there was another one "goredis".

Do you think the README perhaps could list both?

robvdl commented 11 months ago

Also one last question sorry. The session data in Redis doesn't seem to be encrypted and I couldn't find anywhere to put the session secret.

Unless I'm missing something, would that need to be implemented in the store level?

robvdl commented 11 months ago

OK I'm closing this because I'm going offtopic.

I've checked the code of scs and I see no code anywhere that encrypts session data at all. The codebase isn't too big so it is pretty easy to verify that. Which might not sound like a big deal, but these sort of things won't get an application past a security review. Because you can peek into other peoples sessions without even having the encryption key because the session data is unencrypted.

So if someone forgets to properly lock down a Redis server with a password, then that could be a way for all the unencrypted sessions to be leaked. It's hypothetical, but still, session encryption is kind of a must have feature.

I've looked at ways to do this, and have made a successful prototype against the Codec interface in scs. It encrypts after gobbing, and decrypts before ungobbing.