Ensure SessionStores can handle recieving cookies for the wrong implementation

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",

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"

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-conneciton-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 conneciton 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")

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"`
+}

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 {

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
+}

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)
 	}

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() {
+	// The following should only be for server stores
-		Context("when Save is called", func() {
+	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() {
+			Context("attempting to Load", func() {
-				Expect(response.Header().Get("set-cookie")).ToNot(BeEmpty())
+				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() {
+			CheckCookieOptions()
-				Expect(session.CreatedAt.IsZero()).To(BeFalse())
+		})
+	}
+
+	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,
+				req := httptest.NewRequest("GET", "http://example.com/", nil)
-					cookieOpts.CookieName,
+				saveResp := httptest.NewRecorder()
-					"foo",
+				err := ss.Save(saveResp, req, session)
-					cookieOpts.CookiePath,
+				Expect(err).ToNot(HaveOccurred())
-					cookieOpts.CookieDomain,
+
-					cookieOpts.CookieHTTPOnly,
+				for _, c := range saveResp.Result().Cookies() {
-					cookieOpts.CookieSecure,
+					request.AddCookie(c)
-					cookieOpts.CookieExpire,
+				}
-					time.Now(),
+				err = ss.Clear(response, request)
-				)
-				request.AddCookie(cookie)
-				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() {
+			Context("before the refresh period", func() {
-				if cookieOpts.CookieSecret == "" {
+				LoadSessionTests()
-					// 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())
-				}
 			})
+
+			// 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)
 		})
 	})