diff --git a/CHANGELOG.md b/CHANGELOG.md index 7b4e7a9..4f3fc3d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,16 @@ - [#175](https://github.com/pusher/outh2_proxy/pull/175) Bump go-oidc to v2.0.0 (@aeijdenberg). - Includes fix for potential signature checking issue when OIDC discovery is skipped. +- [#155](https://github.com/pusher/outh2_proxy/pull/155) Add RedisSessionStore implementation (@brianv0, @JoelSpeed) + - Implement flags to configure the redis session store + - `-session-store-type=redis` Sets the store type to redis + - `-redis-connection-url` Sets the Redis connection URL + - `-redis-use-sentinel=true` Enables Redis Sentinel support + - `-redis-sentinel-master-name` Sets the Sentinel master name, if sentinel is enabled + - `-redis-sentinel-connection-urls` Defines the Redis Sentinel Connection URLs, if sentinel is enabled + - Introduces the concept of a session ticket. Tickets are composed of the cookie name, a session ID, and a secret. + - Redis Sessions are stored encrypted with a per-session secret + - Added tests for server based session stores - [#168](https://github.com/pusher/outh2_proxy/pull/168) Drop Go 1.11 support in Travis (@JoelSpeed) - [#169](https://github.com/pusher/outh2_proxy/pull/169) Update Alpine to 3.9 (@kskewes) - [#148](https://github.com/pusher/outh2_proxy/pull/148) Implement SessionStore interface within proxy (@JoelSpeed) diff --git a/Gopkg.lock b/Gopkg.lock index ffbd76a..89ba394 100644 --- a/Gopkg.lock +++ b/Gopkg.lock @@ -17,6 +17,25 @@ revision = "b26d9c308763d68093482582cea63d69be07a0f0" version = "v0.3.0" +[[projects]] + branch = "master" + digest = "1:3cce78d5d0090e3f1162945fba60ba74e72e8422e8e41bb9c701afb67237bb65" + name = "github.com/alicebob/gopher-json" + packages = ["."] + pruneopts = "" + revision = "5a6b3ba71ee69b77cf64febf8b5a7526ca5eaef0" + +[[projects]] + digest = "1:18a07506ddaa87b1612bfd69eef03f510faf122398df3da774d46dcfe751a060" + name = "github.com/alicebob/miniredis" + packages = [ + ".", + "server", + ] + pruneopts = "" + revision = "3d7aa1333af56ab862d446678d93aaa6803e0938" + version = "v2.7.0" + [[projects]] digest = "1:512883404c2a99156e410e9880e3bb35ecccc0c07c1159eb204b5f3ef3c431b3" name = "github.com/bitly/go-simplejson" @@ -49,6 +68,22 @@ revision = "06ea1031745cb8b3dab3f6a236daf2b0aa468b7e" version = "v3.2.0" +[[projects]] + digest = "1:8c7410dae63c74bd92db09bf33af7e0698b635ab6a397fd8e9e10dfcce3138ac" + name = "github.com/go-redis/redis" + packages = [ + ".", + "internal", + "internal/consistenthash", + "internal/hashtag", + "internal/pool", + "internal/proto", + "internal/util", + ] + pruneopts = "" + revision = "d22fde8721cc915a55aeb6b00944a76a92bfeb6e" + version = "v6.15.2" + [[projects]] branch = "master" digest = "1:3b760d3b93f994df8eb1d9ebfad17d3e9e37edcb7f7efaa15b427c0d7a64f4e4" @@ -57,6 +92,17 @@ pruneopts = "" revision = "1e59b77b52bf8e4b449a57e6f79f21226d571845" +[[projects]] + digest = "1:dcf8316121302735c0ac84e05f4686e3b34e284444435e9a206da48d8be18cb1" + name = "github.com/gomodule/redigo" + packages = [ + "internal", + "redis", + ] + pruneopts = "" + revision = "9c11da706d9b7902c6da69c592f75637793fe121" + version = "v2.0.0" + [[projects]] digest = "1:b3c5b95e56c06f5aa72cb2500e6ee5f44fcd122872d4fec2023a488e561218bc" name = "github.com/hpcloud/tail" @@ -173,6 +219,19 @@ pruneopts = "" revision = "1d66fa95c997864ba4d8479f56609620fe542928" +[[projects]] + branch = "master" + digest = "1:378d29a839ff770e9d9150580b4c01ff0a513a296b0487558a7af7c18adab98e" + name = "github.com/yuin/gopher-lua" + packages = [ + ".", + "ast", + "parse", + "pm", + ] + pruneopts = "" + revision = "8bfc7677f583b35a5663a9dd934c08f3b5774bbb" + [[projects]] branch = "master" digest = "1:f6a006d27619a4d93bf9b66fe1999b8c8d1fa62bdc63af14f10fbe6fcaa2aa1a" @@ -341,9 +400,11 @@ analyzer-version = 1 input-imports = [ "github.com/BurntSushi/toml", + "github.com/alicebob/miniredis", "github.com/bitly/go-simplejson", "github.com/coreos/go-oidc", "github.com/dgrijalva/jwt-go", + "github.com/go-redis/redis", "github.com/mbland/hmacauth", "github.com/mreiferson/go-options", "github.com/onsi/ginkgo", diff --git a/Gopkg.toml b/Gopkg.toml index cfb10a7..1263b95 100644 --- a/Gopkg.toml +++ b/Gopkg.toml @@ -46,3 +46,11 @@ [[constraint]] name = "gopkg.in/natefinch/lumberjack.v2" version = "2.1.0" + +[[constraint]] + name = "github.com/go-redis/redis" + version = "v6.15.2" + +[[constraint]] + name = "github.com/alicebob/miniredis" + version = "2.7.0" diff --git a/docs/configuration/configuration.md b/docs/configuration/configuration.md index fd33d37..d631eaf 100644 --- a/docs/configuration/configuration.md +++ b/docs/configuration/configuration.md @@ -75,6 +75,10 @@ Usage of oauth2_proxy: -pubjwk-url string: JWK pubkey access endpoint: required by login.gov -redeem-url string: Token redemption endpoint -redirect-url string: the OAuth Redirect URL. ie: "https://internalapp.yourcompany.com/oauth2/callback" + -redis-connection-url string: URL of redis server for redis session storage (eg: redis://HOST[:PORT]) + -redis-sentinel-master-name string: Redis sentinel master name. Used in conjuction with --redis-use-sentinel + -redis-sentinel-connection-urls: List of Redis sentinel conneciton URLs (eg redis://HOST[:PORT]). Used in conjuction with --redis-use-sentinel + -redis-use-sentinel: Connect to redis via sentinels. Must set --redis-sentinel-master-name and --redis-sentinel-connection-urls to use this feature (default: false) -request-logging: Log requests to stdout (default true) -request-logging-format: Template for request log lines (see "Logging Configuration" paragraph below) -resource string: The resource that is protected (Azure AD only) diff --git a/docs/configuration/sessions.md b/docs/configuration/sessions.md index 6e9d9d7..0ffe392 100644 --- a/docs/configuration/sessions.md +++ b/docs/configuration/sessions.md @@ -16,6 +16,7 @@ data in one of the available session storage backends. At present the available backends are (as passed to `--session-store-type`): - [cookie](cookie-storage) (default) +- [redis](redis-storage) ### Cookie Storage @@ -32,3 +33,35 @@ The following should be known when using this implementation: - Since multiple requests can be made concurrently to the OAuth2 Proxy, this session implementation cannot lock sessions and while updating and refreshing sessions, there can be conflicts which force users to re-authenticate + + +### Redis Storage + +The Redis Storage backend stores sessions, encrypted, in redis. Instead sending all the information +back the the client for storage, as in the [Cookie storage](cookie-storage), a ticket is sent back +to the user as the cookie value instead. + +A ticket is composed as the following: + +`{CookieName}-{ticketID}.{secret}` + +Where: + +- The `CookieName` is the OAuth2 cookie name (_oauth2_proxy by default) +- The `ticketID` is a 128 bit random number, hex-encoded +- The `secret` is a 128 bit random number, base64url encoded (no padding). The secret is unique for every session. +- The pair of `{CookieName}-{ticketID}` comprises a ticket handle, and thus, the redis key +to which the session is stored. The encoded session is encrypted with the secret and stored +in redis via the `SETEX` command. + +Encrypting every session uniquely protects the refresh/access/id tokens stored in the session from +disclosure. + +#### Usage + +When using the redis store, specify `--session-store-type=redis` as well as the Redis connection URL, via +`--redis-connection-url=redis://host[:port][/db-number]`. + +You may also configure the store for Redis Sentinel. In this case, you will want to use the +`--redis-use-sentinel=true` flag, as well as configure the flags `--redis-sentinel-master-name` +and `--redis-sentinel-connection-urls` appropriately. diff --git a/main.go b/main.go index a74c245..a66c4fc 100644 --- a/main.go +++ b/main.go @@ -24,6 +24,7 @@ func main() { upstreams := StringArray{} skipAuthRegex := StringArray{} googleGroups := StringArray{} + redisSentinelConnectionURLs := StringArray{} config := flagSet.String("config", "", "path to config file") showVersion := flagSet.Bool("version", false, "print version string") @@ -76,6 +77,10 @@ func main() { flagSet.Bool("cookie-httponly", true, "set HttpOnly cookie flag") flagSet.String("session-store-type", "cookie", "the session storage provider to use") + flagSet.String("redis-connection-url", "", "URL of redis server for redis session storage (eg: redis://HOST[:PORT])") + flagSet.Bool("redis-use-sentinel", false, "Connect to redis via sentinels. Must set --redis-sentinel-master-name and --redis-sentinel-connection-urls to use this feature") + flagSet.String("redis-sentinel-master-name", "", "Redis sentinel master name. Used in conjuction with --redis-use-sentinel") + flagSet.Var(&redisSentinelConnectionURLs, "redis-sentinel-connection-urls", "List of Redis sentinel connection URLs (eg redis://HOST[:PORT]). Used in conjuction with --redis-use-sentinel") flagSet.String("logging-filename", "", "File to log requests to, empty for stdout") flagSet.Int("logging-max-size", 100, "Maximum size in megabytes of the log file before rotation") diff --git a/pkg/apis/options/sessions.go b/pkg/apis/options/sessions.go index 7d33f14..c72da3d 100644 --- a/pkg/apis/options/sessions.go +++ b/pkg/apis/options/sessions.go @@ -9,6 +9,7 @@ type SessionOptions struct { Type string `flag:"session-store-type" cfg:"session_store_type" env:"OAUTH2_PROXY_SESSION_STORE_TYPE"` Cipher *cookie.Cipher CookieStoreOptions + RedisStoreOptions } // CookieSessionStoreType is used to indicate the CookieSessionStore should be @@ -17,3 +18,15 @@ var CookieSessionStoreType = "cookie" // CookieStoreOptions contains configuration options for the CookieSessionStore. type CookieStoreOptions struct{} + +// RedisSessionStoreType is used to indicate the RedisSessionStore should be +// used for storing sessions. +var RedisSessionStoreType = "redis" + +// RedisStoreOptions contains configuration options for the RedisSessionStore. +type RedisStoreOptions struct { + RedisConnectionURL string `flag:"redis-connection-url" cfg:"redis_connection_url" env:"OAUTH2_PROXY_REDIS_CONNECTION_URL"` + UseSentinel bool `flag:"redis-use-sentinel" cfg:"redis_use_sentinel" env:"OAUTH2_PROXY_REDIS_USE_SENTINEL"` + SentinelMasterName string `flag:"redis-sentinel-master-name" cfg:"redis_sentinel_master_name" env:"OAUTH2_PROXY_REDIS_SENTINEL_MASTER_NAME"` + SentinelConnectionURLs []string `flag:"redis-sentinel-connection-urls" cfg:"redis_sentinel_connection_urls" env:"OAUTH2_PROXY_REDIS_SENTINEL_CONNECTION_URLS"` +} diff --git a/pkg/apis/sessions/session_state.go b/pkg/apis/sessions/session_state.go index e085976..01789ff 100644 --- a/pkg/apis/sessions/session_state.go +++ b/pkg/apis/sessions/session_state.go @@ -203,14 +203,14 @@ func DecodeSessionState(v string, c *cookie.Cipher) (*SessionState, error) { User: ss.User, } } else { - // Backward compatibility with using unecrypted Email + // Backward compatibility with using unencrypted Email if ss.Email != "" { decryptedEmail, errEmail := c.Decrypt(ss.Email) if errEmail == nil { ss.Email = decryptedEmail } } - // Backward compatibility with using unecrypted User + // Backward compatibility with using unencrypted User if ss.User != "" { decryptedUser, errUser := c.Decrypt(ss.User) if errUser == nil { diff --git a/pkg/sessions/redis/redis_store.go b/pkg/sessions/redis/redis_store.go new file mode 100644 index 0000000..82e941e --- /dev/null +++ b/pkg/sessions/redis/redis_store.go @@ -0,0 +1,305 @@ +package redis + +import ( + "crypto/aes" + "crypto/cipher" + "crypto/rand" + "encoding/base64" + "encoding/hex" + "fmt" + "io" + "net/http" + "strings" + "time" + + "github.com/go-redis/redis" + "github.com/pusher/oauth2_proxy/cookie" + "github.com/pusher/oauth2_proxy/pkg/apis/options" + "github.com/pusher/oauth2_proxy/pkg/apis/sessions" + "github.com/pusher/oauth2_proxy/pkg/cookies" +) + +// TicketData is a structure representing the ticket used in server session storage +type TicketData struct { + TicketID string + Secret []byte +} + +// SessionStore is an implementation of the sessions.SessionStore +// interface that stores sessions in redis +type SessionStore struct { + CookieCipher *cookie.Cipher + CookieOptions *options.CookieOptions + Client *redis.Client +} + +// NewRedisSessionStore initialises a new instance of the SessionStore from +// the configuration given +func NewRedisSessionStore(opts *options.SessionOptions, cookieOpts *options.CookieOptions) (sessions.SessionStore, error) { + client, err := newRedisClient(opts.RedisStoreOptions) + if err != nil { + return nil, fmt.Errorf("error constructing redis client: %v", err) + } + + rs := &SessionStore{ + Client: client, + CookieCipher: opts.Cipher, + CookieOptions: cookieOpts, + } + return rs, nil + +} + +func newRedisClient(opts options.RedisStoreOptions) (*redis.Client, error) { + if opts.UseSentinel { + client := redis.NewFailoverClient(&redis.FailoverOptions{ + MasterName: opts.SentinelMasterName, + SentinelAddrs: opts.SentinelConnectionURLs, + }) + return client, nil + } + + opt, err := redis.ParseURL(opts.RedisConnectionURL) + if err != nil { + return nil, fmt.Errorf("unable to parse redis url: %s", err) + } + + client := redis.NewClient(opt) + return client, nil +} + +// Save takes a sessions.SessionState and stores the information from it +// to redies, and adds a new ticket cookie on the HTTP response writer +func (store *SessionStore) Save(rw http.ResponseWriter, req *http.Request, s *sessions.SessionState) error { + if s.CreatedAt.IsZero() { + s.CreatedAt = time.Now() + } + + // Old sessions that we are refreshing would have a request cookie + // New sessions don't, so we ignore the error. storeValue will check requestCookie + requestCookie, _ := req.Cookie(store.CookieOptions.CookieName) + value, err := s.EncodeSessionState(store.CookieCipher) + if err != nil { + return err + } + ticketString, err := store.storeValue(value, store.CookieOptions.CookieExpire, requestCookie) + if err != nil { + return err + } + + ticketCookie := store.makeCookie( + req, + ticketString, + store.CookieOptions.CookieExpire, + s.CreatedAt, + ) + + http.SetCookie(rw, ticketCookie) + return nil +} + +// Load reads sessions.SessionState information from a ticket +// cookie within the HTTP request object +func (store *SessionStore) Load(req *http.Request) (*sessions.SessionState, error) { + requestCookie, err := req.Cookie(store.CookieOptions.CookieName) + if err != nil { + return nil, fmt.Errorf("error loading session: %s", err) + } + + val, _, ok := cookie.Validate(requestCookie, store.CookieOptions.CookieSecret, store.CookieOptions.CookieExpire) + if !ok { + return nil, fmt.Errorf("Cookie Signature not valid") + } + session, err := store.loadSessionFromString(val) + if err != nil { + return nil, fmt.Errorf("error loading session: %s", err) + } + return session, nil +} + +// loadSessionFromString loads the session based on the ticket value +func (store *SessionStore) loadSessionFromString(value string) (*sessions.SessionState, error) { + ticket, err := decodeTicket(store.CookieOptions.CookieName, value) + if err != nil { + return nil, err + } + + result, err := store.Client.Get(ticket.asHandle(store.CookieOptions.CookieName)).Result() + if err != nil { + return nil, err + } + + resultBytes := []byte(result) + block, err := aes.NewCipher(ticket.Secret) + if err != nil { + return nil, err + } + // Use secret as the IV too, because each entry has it's own key + stream := cipher.NewCFBDecrypter(block, ticket.Secret) + stream.XORKeyStream(resultBytes, resultBytes) + + session, err := sessions.DecodeSessionState(string(resultBytes), store.CookieCipher) + if err != nil { + return nil, err + } + return session, nil +} + +// Clear clears any saved session information for a given ticket cookie +// from redis, and then clears the session +func (store *SessionStore) Clear(rw http.ResponseWriter, req *http.Request) error { + // We go ahead and clear the cookie first, always. + clearCookie := store.makeCookie( + req, + "", + time.Hour*-1, + time.Now(), + ) + http.SetCookie(rw, clearCookie) + + // If there was an existing cookie we should clear the session in redis + requestCookie, err := req.Cookie(store.CookieOptions.CookieName) + if err != nil && err == http.ErrNoCookie { + // No existing cookie so can't clear redis + return nil + } else if err != nil { + return fmt.Errorf("error retrieving cookie: %v", err) + } + + val, _, ok := cookie.Validate(requestCookie, store.CookieOptions.CookieSecret, store.CookieOptions.CookieExpire) + if !ok { + return fmt.Errorf("Cookie Signature not valid") + } + + // We only return an error if we had an issue with redis + // If there's an issue decoding the ticket, ignore it + ticket, _ := decodeTicket(store.CookieOptions.CookieName, val) + if ticket != nil { + _, err := store.Client.Del(ticket.asHandle(store.CookieOptions.CookieName)).Result() + if err != nil { + return fmt.Errorf("error clearing cookie from redis: %s", err) + } + } + return nil +} + +// makeCookie makes a cookie, signing the value if present +func (store *SessionStore) makeCookie(req *http.Request, value string, expires time.Duration, now time.Time) *http.Cookie { + if value != "" { + value = cookie.SignedValue(store.CookieOptions.CookieSecret, store.CookieOptions.CookieName, value, now) + } + return cookies.MakeCookieFromOptions( + req, + store.CookieOptions.CookieName, + value, + store.CookieOptions, + expires, + now, + ) +} + +func (store *SessionStore) storeValue(value string, expiration time.Duration, requestCookie *http.Cookie) (string, error) { + ticket, err := store.getTicket(requestCookie) + if err != nil { + return "", fmt.Errorf("error getting ticket: %v", err) + } + + ciphertext := make([]byte, len(value)) + block, err := aes.NewCipher(ticket.Secret) + if err != nil { + return "", fmt.Errorf("error initiating cipher block %s", err) + } + + // Use secret as the Initialization Vector too, because each entry has it's own key + stream := cipher.NewCFBEncrypter(block, ticket.Secret) + stream.XORKeyStream(ciphertext, []byte(value)) + + handle := ticket.asHandle(store.CookieOptions.CookieName) + err = store.Client.Set(handle, ciphertext, expiration).Err() + if err != nil { + return "", err + } + return ticket.encodeTicket(store.CookieOptions.CookieName), nil +} + +// getTicket retrieves an existing ticket from the cookie if present, +// or creates a new ticket +func (store *SessionStore) getTicket(requestCookie *http.Cookie) (*TicketData, error) { + if requestCookie == nil { + return newTicket() + } + + // An existing cookie exists, try to retrieve the ticket + val, _, ok := cookie.Validate(requestCookie, store.CookieOptions.CookieSecret, store.CookieOptions.CookieExpire) + if !ok { + // Cookie is invalid, create a new ticket + return newTicket() + } + + // Valid cookie, decode the ticket + ticket, err := decodeTicket(store.CookieOptions.CookieName, val) + if err != nil { + // If we can't decode the ticket we have to create a new one + return newTicket() + } + return ticket, nil +} + +func newTicket() (*TicketData, error) { + rawID := make([]byte, 16) + if _, err := io.ReadFull(rand.Reader, rawID); err != nil { + return nil, fmt.Errorf("failed to create new ticket ID %s", err) + } + // ticketID is hex encoded + ticketID := fmt.Sprintf("%x", rawID) + + secret := make([]byte, aes.BlockSize) + if _, err := io.ReadFull(rand.Reader, secret); err != nil { + return nil, fmt.Errorf("failed to create initialization vector %s", err) + } + ticket := &TicketData{ + TicketID: ticketID, + Secret: secret, + } + return ticket, nil +} + +func (ticket *TicketData) asHandle(prefix string) string { + return fmt.Sprintf("%s-%s", prefix, ticket.TicketID) +} + +func decodeTicket(cookieName string, ticketString string) (*TicketData, error) { + prefix := cookieName + "-" + if !strings.HasPrefix(ticketString, prefix) { + return nil, fmt.Errorf("failed to decode ticket handle") + } + trimmedTicket := strings.TrimPrefix(ticketString, prefix) + + ticketParts := strings.Split(trimmedTicket, ".") + if len(ticketParts) != 2 { + return nil, fmt.Errorf("failed to decode ticket") + } + ticketID, secretBase64 := ticketParts[0], ticketParts[1] + + // ticketID must be a hexadecimal string + _, err := hex.DecodeString(ticketID) + if err != nil { + return nil, fmt.Errorf("server ticket failed sanity checks") + } + + secret, err := base64.RawURLEncoding.DecodeString(secretBase64) + if err != nil { + return nil, fmt.Errorf("failed to decode initialization vector %s", err) + } + ticketData := &TicketData{ + TicketID: ticketID, + Secret: secret, + } + return ticketData, nil +} + +func (ticket *TicketData) encodeTicket(prefix string) string { + handle := ticket.asHandle(prefix) + ticketString := handle + "." + base64.RawURLEncoding.EncodeToString(ticket.Secret) + return ticketString +} diff --git a/pkg/sessions/session_store.go b/pkg/sessions/session_store.go index ec84dd7..17ef21c 100644 --- a/pkg/sessions/session_store.go +++ b/pkg/sessions/session_store.go @@ -6,6 +6,7 @@ import ( "github.com/pusher/oauth2_proxy/pkg/apis/options" "github.com/pusher/oauth2_proxy/pkg/apis/sessions" "github.com/pusher/oauth2_proxy/pkg/sessions/cookie" + "github.com/pusher/oauth2_proxy/pkg/sessions/redis" ) // NewSessionStore creates a SessionStore from the provided configuration @@ -13,6 +14,8 @@ func NewSessionStore(opts *options.SessionOptions, cookieOpts *options.CookieOpt switch opts.Type { case options.CookieSessionStoreType: return cookie.NewCookieSessionStore(opts, cookieOpts) + case options.RedisSessionStoreType: + return redis.NewRedisSessionStore(opts, cookieOpts) default: return nil, fmt.Errorf("unknown session store type '%s'", opts.Type) } diff --git a/pkg/sessions/session_store_test.go b/pkg/sessions/session_store_test.go index b407d58..47ad4b7 100644 --- a/pkg/sessions/session_store_test.go +++ b/pkg/sessions/session_store_test.go @@ -10,6 +10,7 @@ import ( "testing" "time" + "github.com/alicebob/miniredis" . "github.com/onsi/ginkgo" . "github.com/onsi/gomega" "github.com/pusher/oauth2_proxy/cookie" @@ -18,6 +19,7 @@ import ( "github.com/pusher/oauth2_proxy/pkg/cookies" "github.com/pusher/oauth2_proxy/pkg/sessions" sessionscookie "github.com/pusher/oauth2_proxy/pkg/sessions/cookie" + "github.com/pusher/oauth2_proxy/pkg/sessions/redis" "github.com/pusher/oauth2_proxy/pkg/sessions/utils" ) @@ -34,6 +36,7 @@ var _ = Describe("NewSessionStore", func() { var response *httptest.ResponseRecorder var session *sessionsapi.SessionState var ss sessionsapi.SessionStore + var mr *miniredis.Miniredis CheckCookieOptions := func() { Context("the cookies returned", func() { @@ -89,19 +92,113 @@ var _ = Describe("NewSessionStore", func() { }) } - SessionStoreInterfaceTests := func() { - Context("when Save is called", func() { + // The following should only be for server stores + PersistentSessionStoreTests := func() { + Context("when Clear is called on a persistent store", func() { + var resultCookies []*http.Cookie + BeforeEach(func() { - err := ss.Save(response, request, session) + req := httptest.NewRequest("GET", "http://example.com/", nil) + saveResp := httptest.NewRecorder() + err := ss.Save(saveResp, req, session) + Expect(err).ToNot(HaveOccurred()) + + resultCookies = saveResp.Result().Cookies() + for _, c := range resultCookies { + request.AddCookie(c) + } + err = ss.Clear(response, request) Expect(err).ToNot(HaveOccurred()) }) - It("sets a `set-cookie` header in the response", func() { - Expect(response.Header().Get("set-cookie")).ToNot(BeEmpty()) + Context("attempting to Load", func() { + var loadedAfterClear *sessionsapi.SessionState + var loadErr error + + BeforeEach(func() { + loadReq := httptest.NewRequest("GET", "http://example.com/", nil) + for _, c := range resultCookies { + loadReq.AddCookie(c) + } + + loadedAfterClear, loadErr = ss.Load(loadReq) + }) + + It("returns an empty session", func() { + Expect(loadedAfterClear).To(BeNil()) + }) + + It("returns an error", func() { + Expect(loadErr).To(HaveOccurred()) + }) }) - It("Ensures the session CreatedAt is not zero", func() { - Expect(session.CreatedAt.IsZero()).To(BeFalse()) + CheckCookieOptions() + }) + } + + SessionStoreInterfaceTests := func(persistent bool) { + Context("when Save is called", func() { + Context("with no existing session", func() { + BeforeEach(func() { + err := ss.Save(response, request, session) + Expect(err).ToNot(HaveOccurred()) + }) + + It("sets a `set-cookie` header in the response", func() { + Expect(response.Header().Get("set-cookie")).ToNot(BeEmpty()) + }) + + It("Ensures the session CreatedAt is not zero", func() { + Expect(session.CreatedAt.IsZero()).To(BeFalse()) + }) + }) + + Context("with a broken session", func() { + BeforeEach(func() { + By("Using a valid cookie with a different providers session encoding") + broken := "BrokenSessionFromADifferentSessionImplementation" + value := cookie.SignedValue(cookieOpts.CookieSecret, cookieOpts.CookieName, broken, time.Now()) + cookie := cookies.MakeCookieFromOptions(request, cookieOpts.CookieName, value, cookieOpts, cookieOpts.CookieExpire, time.Now()) + request.AddCookie(cookie) + + err := ss.Save(response, request, session) + Expect(err).ToNot(HaveOccurred()) + }) + + It("sets a `set-cookie` header in the response", func() { + Expect(response.Header().Get("set-cookie")).ToNot(BeEmpty()) + }) + + It("Ensures the session CreatedAt is not zero", func() { + Expect(session.CreatedAt.IsZero()).To(BeFalse()) + }) + }) + + Context("with an expired saved session", func() { + var err error + BeforeEach(func() { + By("saving a session") + req := httptest.NewRequest("GET", "http://example.com/", nil) + saveResp := httptest.NewRecorder() + err = ss.Save(saveResp, req, session) + Expect(err).ToNot(HaveOccurred()) + + By("and clearing the session") + for _, c := range saveResp.Result().Cookies() { + request.AddCookie(c) + } + clearResp := httptest.NewRecorder() + err = ss.Clear(clearResp, request) + Expect(err).ToNot(HaveOccurred()) + + By("then saving a request with the cleared session") + err = ss.Save(response, request, session) + }) + + It("no error should occur", func() { + Expect(err).ToNot(HaveOccurred()) + }) }) CheckCookieOptions() @@ -109,18 +206,15 @@ var _ = Describe("NewSessionStore", func() { Context("when Clear is called", func() { BeforeEach(func() { - cookie := cookies.MakeCookie(request, - cookieOpts.CookieName, - "foo", - cookieOpts.CookiePath, - cookieOpts.CookieDomain, - cookieOpts.CookieHTTPOnly, - cookieOpts.CookieSecure, - cookieOpts.CookieExpire, - time.Now(), - ) - request.AddCookie(cookie) - err := ss.Clear(response, request) + req := httptest.NewRequest("GET", "http://example.com/", nil) + saveResp := httptest.NewRecorder() + err := ss.Save(saveResp, req, session) + Expect(err).ToNot(HaveOccurred()) + + for _, c := range saveResp.Result().Cookies() { + request.AddCookie(c) + } + err = ss.Clear(response, request) Expect(err).ToNot(HaveOccurred()) }) @@ -132,7 +226,38 @@ var _ = Describe("NewSessionStore", func() { }) Context("when Load is called", func() { - var loadedSession *sessionsapi.SessionState + LoadSessionTests := func() { + var loadedSession *sessionsapi.SessionState + BeforeEach(func() { + var err error + loadedSession, err = ss.Load(request) + Expect(err).ToNot(HaveOccurred()) + }) + + It("loads a session equal to the original session", func() { + if cookieOpts.CookieSecret == "" { + // Only Email and User stored in session when encrypted + Expect(loadedSession.Email).To(Equal(session.Email)) + Expect(loadedSession.User).To(Equal(session.User)) + } else { + // All fields stored in session if encrypted + + // Can't compare time.Time using Equal() so remove ExpiresOn from sessions + l := *loadedSession + l.CreatedAt = time.Time{} + l.ExpiresOn = time.Time{} + s := *session + s.CreatedAt = time.Time{} + s.ExpiresOn = time.Time{} + Expect(l).To(Equal(s)) + + // Compare time.Time separately + Expect(loadedSession.CreatedAt.Equal(session.CreatedAt)).To(BeTrue()) + Expect(loadedSession.ExpiresOn.Equal(session.ExpiresOn)).To(BeTrue()) + } + }) + } + BeforeEach(func() { req := httptest.NewRequest("GET", "http://example.com/", nil) resp := httptest.NewRecorder() @@ -142,36 +267,57 @@ var _ = Describe("NewSessionStore", func() { for _, cookie := range resp.Result().Cookies() { request.AddCookie(cookie) } - loadedSession, err = ss.Load(request) - Expect(err).ToNot(HaveOccurred()) }) - It("loads a session equal to the original session", func() { - if cookieOpts.CookieSecret == "" { - // Only Email and User stored in session when encrypted - Expect(loadedSession.Email).To(Equal(session.Email)) - Expect(loadedSession.User).To(Equal(session.User)) - } else { - // All fields stored in session if encrypted - - // Can't compare time.Time using Equal() so remove ExpiresOn from sessions - l := *loadedSession - l.CreatedAt = time.Time{} - l.ExpiresOn = time.Time{} - s := *session - s.CreatedAt = time.Time{} - s.ExpiresOn = time.Time{} - Expect(l).To(Equal(s)) - - // Compare time.Time separately - Expect(loadedSession.CreatedAt.Equal(session.CreatedAt)).To(BeTrue()) - Expect(loadedSession.ExpiresOn.Equal(session.ExpiresOn)).To(BeTrue()) - } + Context("before the refresh period", func() { + LoadSessionTests() }) + + // Test TTLs and cleanup of persistent session storage + // For non-persistent we rely on the browser cookie lifecycle + if persistent { + Context("after the refresh period, but before the cookie expire period", func() { + BeforeEach(func() { + switch ss.(type) { + case *redis.SessionStore: + mr.FastForward(cookieOpts.CookieRefresh + time.Minute) + } + }) + + LoadSessionTests() + }) + + Context("after the cookie expire period", func() { + var loadedSession *sessionsapi.SessionState + var err error + + BeforeEach(func() { + switch ss.(type) { + case *redis.SessionStore: + mr.FastForward(cookieOpts.CookieExpire + time.Minute) + } + + loadedSession, err = ss.Load(request) + Expect(err).To(HaveOccurred()) + }) + + It("returns an error loading the session", func() { + Expect(err).To(HaveOccurred()) + }) + + It("returns an empty session", func() { + Expect(loadedSession).To(BeNil()) + }) + }) + } }) + + if persistent { + PersistentSessionStoreTests() + } } - RunSessionTests := func() { + RunSessionTests := func(persistent bool) { Context("with default options", func() { BeforeEach(func() { var err error @@ -179,7 +325,7 @@ var _ = Describe("NewSessionStore", func() { Expect(err).ToNot(HaveOccurred()) }) - SessionStoreInterfaceTests() + SessionStoreInterfaceTests(persistent) }) Context("with non-default options", func() { @@ -188,7 +334,7 @@ var _ = Describe("NewSessionStore", func() { CookieName: "_cookie_name", CookiePath: "/path", CookieExpire: time.Duration(72) * time.Hour, - CookieRefresh: time.Duration(3600), + CookieRefresh: time.Duration(2) * time.Hour, CookieSecure: false, CookieHTTPOnly: false, CookieDomain: "example.com", @@ -199,7 +345,7 @@ var _ = Describe("NewSessionStore", func() { Expect(err).ToNot(HaveOccurred()) }) - SessionStoreInterfaceTests() + SessionStoreInterfaceTests(persistent) }) Context("with a cipher", func() { @@ -217,7 +363,7 @@ var _ = Describe("NewSessionStore", func() { Expect(err).ToNot(HaveOccurred()) }) - SessionStoreInterfaceTests() + SessionStoreInterfaceTests(persistent) }) } @@ -230,7 +376,7 @@ var _ = Describe("NewSessionStore", func() { CookieName: "_oauth2_proxy", CookiePath: "/", CookieExpire: time.Duration(168) * time.Hour, - CookieRefresh: time.Duration(0), + CookieRefresh: time.Duration(1) * time.Hour, CookieSecure: true, CookieHTTPOnly: true, } @@ -260,7 +406,31 @@ var _ = Describe("NewSessionStore", func() { }) Context("the cookie.SessionStore", func() { - RunSessionTests() + RunSessionTests(false) + }) + }) + + Context("with type 'redis'", func() { + BeforeEach(func() { + var err error + mr, err = miniredis.Run() + Expect(err).ToNot(HaveOccurred()) + opts.Type = options.RedisSessionStoreType + opts.RedisConnectionURL = "redis://" + mr.Addr() + }) + + AfterEach(func() { + mr.Close() + }) + + It("creates a redis.SessionStore", func() { + ss, err := sessions.NewSessionStore(opts, cookieOpts) + Expect(err).NotTo(HaveOccurred()) + Expect(ss).To(BeAssignableToTypeOf(&redis.SessionStore{})) + }) + + Context("the redis.SessionStore", func() { + RunSessionTests(true) }) })