From e881612ea622dfed39265796076973ae72acfe9b Mon Sep 17 00:00:00 2001
From: Brian Van Klaveren <bvan@slac.stanford.edu>
Date: Wed, 8 May 2019 12:35:15 -0700
Subject: [PATCH 01/28] Fix session_state type

---
 pkg/apis/sessions/session_state.go | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

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 {

From b1bd3280dbf080cde82e1c2e832125bc21e05494 Mon Sep 17 00:00:00 2001
From: Brian Van Klaveren <bvan@slac.stanford.edu>
Date: Thu, 9 May 2019 16:09:22 -0700
Subject: [PATCH 02/28] Add support for a redis session store

---
 Gopkg.lock                         |  61 +++++++
 Gopkg.toml                         |   8 +
 pkg/apis/options/sessions.go       |  10 ++
 pkg/sessions/redis/redis_store.go  | 271 +++++++++++++++++++++++++++++
 pkg/sessions/session_store.go      |   3 +
 pkg/sessions/session_store_test.go |  88 ++++++++--
 6 files changed, 427 insertions(+), 14 deletions(-)
 create mode 100644 pkg/sessions/redis/redis_store.go

diff --git a/Gopkg.lock b/Gopkg.lock
index 01af4d2..e8bb95e 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 732bbcb..7c90984 100644
--- a/Gopkg.toml
+++ b/Gopkg.toml
@@ -46,3 +46,11 @@
 [[constraint]]
   name = "gopkg.in/natefinch/lumberjack.v2"
   version = "2.1.0"
+
+[[constraint]]
+  branch = "master"
+  name = "github.com/go-redis/redis"
+
+[[constraint]]
+  name = "github.com/alicebob/miniredis"
+  version = "2.7.0"
diff --git a/pkg/apis/options/sessions.go b/pkg/apis/options/sessions.go
index 7d33f14..591cc37 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,12 @@ var CookieSessionStoreType = "cookie"
 
 // CookieStoreOptions contains configuration options for the CookieSessionStore.
 type CookieStoreOptions struct{}
+
+// RedisSessionStoreType is used to indicate the CookieSessionStore should be
+// used for storing sessions.
+var RedisSessionStoreType = "redis"
+
+// RedisStoreOptions contains configuration options for the CookieSessionStore.
+type RedisStoreOptions struct {
+	RedisConnectionURL string `flag:"redis-connection-url" cfg:"redis_connection_url" env:"OAUTH2_PROXY_REDIS_CONNECTION_URL"`
+}
diff --git a/pkg/sessions/redis/redis_store.go b/pkg/sessions/redis/redis_store.go
new file mode 100644
index 0000000..74cb1c0
--- /dev/null
+++ b/pkg/sessions/redis/redis_store.go
@@ -0,0 +1,271 @@
+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"
+	"github.com/pusher/oauth2_proxy/pkg/sessions/utils"
+)
+
+// TicketData is a structure representing a 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
+	CookieDomain   string
+	CookieExpire   time.Duration
+	CookieHTTPOnly bool
+	CookieName     string
+	CookiePath     string
+	CookieSecret   string
+	CookieSecure   bool
+	Client         *redis.Client
+}
+
+// NewRedisSessionStore initialises a new instance of the SessionStore from
+// the configuration given
+func NewRedisSessionStore(opts options.RedisStoreOptions, cookieOpts *options.CookieOptions) (sessions.SessionStore, error) {
+	opt, err := redis.ParseURL(opts.RedisConnectionURL)
+	if err != nil {
+		return nil, fmt.Errorf("unable to parse redis url: %s", err)
+	}
+
+	var cookieCipher *cookie.Cipher
+	if len(cookieOpts.CookieSecret) > 0 {
+		var err error
+		cookieCipher, err = cookie.NewCipher(utils.SecretBytes(cookieOpts.CookieSecret))
+		if err != nil {
+			return nil, fmt.Errorf("unable to create cookieCipher: %v", err)
+		}
+	}
+
+	client := redis.NewClient(opt)
+
+	rs := &SessionStore{
+		Client:         client,
+		CookieCipher:   cookieCipher,
+		CookieDomain:   cookieOpts.CookieDomain,
+		CookieExpire:   cookieOpts.CookieExpire,
+		CookieHTTPOnly: cookieOpts.CookieHTTPOnly,
+		CookieName:     cookieOpts.CookieName,
+		CookiePath:     cookieOpts.CookiePath,
+		CookieSecret:   cookieOpts.CookieSecret,
+		CookieSecure:   cookieOpts.CookieSecure,
+	}
+	return rs, 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 {
+	requestCookie, _ := req.Cookie(store.CookieName)
+	value, err := s.EncodeSessionState(store.CookieCipher)
+	if err != nil {
+		return err
+	}
+	ticketString, err := store.storeValue(value, s.ExpiresOn, requestCookie)
+	if err != nil {
+		return err
+	}
+
+	ticketCookie := cookies.MakeCookie(
+		req,
+		store.CookieName,
+		ticketString,
+		store.CookiePath,
+		store.CookieDomain,
+		store.CookieHTTPOnly,
+		store.CookieSecure,
+		store.CookieExpire,
+		time.Now(),
+	)
+
+	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, _ := req.Cookie(store.CookieName)
+	// No cookie validation necessary
+	session, err := store.LoadSessionFromString(requestCookie.Value)
+	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.CookieName, value)
+	if err != nil {
+		return nil, err
+	}
+
+	result, err := store.Client.Get(ticket.asHandle(store.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 {
+	requestCookie, _ := req.Cookie(store.CookieName)
+
+	// We go ahead and clear the cookie first, always.
+	clearCookie := cookies.MakeCookie(
+		req,
+		store.CookieName,
+		"",
+		store.CookiePath,
+		store.CookieDomain,
+		store.CookieHTTPOnly,
+		store.CookieSecure,
+		time.Hour*-1,
+		time.Now(),
+	)
+	http.SetCookie(rw, clearCookie)
+
+	// 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.CookieName, requestCookie.Value)
+	if ticket != nil {
+		deleted, err := store.Client.Del(ticket.asHandle(store.CookieName)).Result()
+		fmt.Println("delted %n", deleted)
+		if err != nil {
+			return fmt.Errorf("error clearing cookie from redis: %s", err)
+		}
+	}
+	return nil
+}
+
+func (store *SessionStore) storeValue(value string, expiresOn time.Time, requestCookie *http.Cookie) (string, error) {
+	var ticket *TicketData
+	if requestCookie != nil {
+		var err error
+		ticket, err = decodeTicket(store.CookieName, requestCookie.Value)
+		if err != nil {
+			return "", err
+		}
+	} else {
+		var err error
+		ticket, err = newTicket()
+		if err != nil {
+			return "", fmt.Errorf("error creating new ticket: %s", 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 IV too, because each entry has it's own key
+	stream := cipher.NewCFBEncrypter(block, ticket.Secret)
+	stream.XORKeyStream(ciphertext, []byte(value))
+
+	handle := ticket.asHandle(store.CookieName)
+	expires := expiresOn.Sub(time.Now())
+	err = store.Client.Set(handle, ciphertext, expires).Err()
+	if err != nil {
+		return "", err
+	}
+	return ticket.encodeTicket(store.CookieName), 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")
+		// s is not a valid
+	}
+
+	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..e8e229a 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.RedisStoreOptions, 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..47780f6 100644
--- a/pkg/sessions/session_store_test.go
+++ b/pkg/sessions/session_store_test.go
@@ -10,14 +10,15 @@ import (
 	"testing"
 	"time"
 
+	"github.com/alicebob/miniredis"
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
 	"github.com/pusher/oauth2_proxy/cookie"
 	"github.com/pusher/oauth2_proxy/pkg/apis/options"
 	sessionsapi "github.com/pusher/oauth2_proxy/pkg/apis/sessions"
-	"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"
 )
 
@@ -109,22 +110,18 @@ 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())
+
+				resultCookie := saveResp.Result().Cookies()[0]
+				request.AddCookie(resultCookie)
+				err = ss.Clear(response, request)
 				Expect(err).ToNot(HaveOccurred())
 			})
 
-			It("sets a `set-cookie` header in the response", func() {
+			It("sets a `set-cookie` header in the response and session to not be loadable after clear", func() {
 				Expect(response.Header().Get("Set-Cookie")).ToNot(BeEmpty())
 			})
 
@@ -171,6 +168,35 @@ var _ = Describe("NewSessionStore", func() {
 		})
 	}
 
+	PersistentSessionStoreTests := func() {
+		Context("when Clear is called the session should not be recoverable", func() {
+			var loadedAfterClear *sessionsapi.SessionState
+			BeforeEach(func() {
+				req := httptest.NewRequest("GET", "http://example.com/", nil)
+				saveResp := httptest.NewRecorder()
+				err := ss.Save(saveResp, req, session)
+				Expect(err).ToNot(HaveOccurred())
+
+				resultCookie := saveResp.Result().Cookies()[0]
+				request.AddCookie(resultCookie)
+				err = ss.Clear(response, request)
+				Expect(err).ToNot(HaveOccurred())
+
+				// The following should only be for server stores
+				loadReq := httptest.NewRequest("GET", "http://example.com/", nil)
+				loadReq.AddCookie(resultCookie)
+				loadedAfterClear, err = ss.Load(loadReq)
+			})
+
+			It("sets a `set-cookie` header in the response and session to not be loadable after clear", func() {
+				Expect(response.Header().Get("Set-Cookie")).ToNot(BeEmpty())
+				Expect(loadedAfterClear).To(BeNil())
+			})
+
+			CheckCookieOptions()
+		})
+	}
+
 	RunSessionTests := func() {
 		Context("with default options", func() {
 			BeforeEach(func() {
@@ -221,6 +247,18 @@ var _ = Describe("NewSessionStore", func() {
 		})
 	}
 
+	RunPersistentSessionStoreTests := func() {
+		Context("with default options", func() {
+			BeforeEach(func() {
+				var err error
+				ss, err = sessions.NewSessionStore(opts, cookieOpts)
+				Expect(err).ToNot(HaveOccurred())
+			})
+
+			PersistentSessionStoreTests()
+		})
+	}
+
 	BeforeEach(func() {
 		ss = nil
 		opts = &options.SessionOptions{}
@@ -264,6 +302,28 @@ var _ = Describe("NewSessionStore", func() {
 		})
 	})
 
+	Context("with type 'redis'", func() {
+		BeforeEach(func() {
+			mr, err := miniredis.Run()
+			if err != nil {
+				panic(err)
+			}
+			opts.Type = options.RedisSessionStoreType
+			opts.RedisConnectionURL = "redis://" + mr.Addr()
+		})
+
+		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()
+			RunPersistentSessionStoreTests()
+		})
+	})
+
 	Context("with an invalid type", func() {
 		BeforeEach(func() {
 			opts.Type = "invalid-type"

From 42731f061702dbba5dea52c4c81bb822afd48054 Mon Sep 17 00:00:00 2001
From: Brian Van Klaveren <bvan@slac.stanford.edu>
Date: Mon, 13 May 2019 11:54:06 -0700
Subject: [PATCH 03/28] Check cookie error and doc on cookie handling

---
 pkg/sessions/redis/redis_store.go | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/pkg/sessions/redis/redis_store.go b/pkg/sessions/redis/redis_store.go
index 74cb1c0..c240071 100644
--- a/pkg/sessions/redis/redis_store.go
+++ b/pkg/sessions/redis/redis_store.go
@@ -20,7 +20,7 @@ import (
 	"github.com/pusher/oauth2_proxy/pkg/sessions/utils"
 )
 
-// TicketData is a structure representing a used in server session storage
+// TicketData is a structure representing the ticket used in server session storage
 type TicketData struct {
 	TicketID string
 	Secret   []byte
@@ -77,6 +77,8 @@ func NewRedisSessionStore(opts options.RedisStoreOptions, cookieOpts *options.Co
 // 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 {
+	// 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.CookieName)
 	value, err := s.EncodeSessionState(store.CookieCipher)
 	if err != nil {
@@ -106,7 +108,10 @@ func (store *SessionStore) Save(rw http.ResponseWriter, req *http.Request, s *se
 // 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, _ := req.Cookie(store.CookieName)
+	requestCookie, err := req.Cookie(store.CookieName)
+	if err != nil {
+		return nil, fmt.Errorf("error loading session: %s", err)
+	}
 	// No cookie validation necessary
 	session, err := store.LoadSessionFromString(requestCookie.Value)
 	if err != nil {

From f2562e897362398e677b96d554dc69e4250956c5 Mon Sep 17 00:00:00 2001
From: Brian Van Klaveren <bvan@slac.stanford.edu>
Date: Mon, 13 May 2019 11:54:22 -0700
Subject: [PATCH 04/28] Pin version of go-redis

---
 Gopkg.toml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Gopkg.toml b/Gopkg.toml
index 7c90984..bdc0ae9 100644
--- a/Gopkg.toml
+++ b/Gopkg.toml
@@ -50,6 +50,7 @@
 [[constraint]]
   branch = "master"
   name = "github.com/go-redis/redis"
+  version = "v6.15.2"
 
 [[constraint]]
   name = "github.com/alicebob/miniredis"

From 296d989e5871be8e0274f2cb8540d53c497ee6e8 Mon Sep 17 00:00:00 2001
From: Joel Speed <Joel.speed@hotmail.co.uk>
Date: Wed, 15 May 2019 17:06:05 +0100
Subject: [PATCH 05/28] Simplify redis store options

---
 pkg/sessions/redis/redis_store.go | 78 ++++++++++---------------------
 pkg/sessions/session_store.go     |  2 +-
 2 files changed, 26 insertions(+), 54 deletions(-)

diff --git a/pkg/sessions/redis/redis_store.go b/pkg/sessions/redis/redis_store.go
index c240071..ec55a03 100644
--- a/pkg/sessions/redis/redis_store.go
+++ b/pkg/sessions/redis/redis_store.go
@@ -17,7 +17,6 @@ import (
 	"github.com/pusher/oauth2_proxy/pkg/apis/options"
 	"github.com/pusher/oauth2_proxy/pkg/apis/sessions"
 	"github.com/pusher/oauth2_proxy/pkg/cookies"
-	"github.com/pusher/oauth2_proxy/pkg/sessions/utils"
 )
 
 // TicketData is a structure representing the ticket used in server session storage
@@ -29,46 +28,25 @@ type TicketData struct {
 // SessionStore is an implementation of the sessions.SessionStore
 // interface that stores sessions in redis
 type SessionStore struct {
-	CookieCipher   *cookie.Cipher
-	CookieDomain   string
-	CookieExpire   time.Duration
-	CookieHTTPOnly bool
-	CookieName     string
-	CookiePath     string
-	CookieSecret   string
-	CookieSecure   bool
-	Client         *redis.Client
+	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.RedisStoreOptions, cookieOpts *options.CookieOptions) (sessions.SessionStore, error) {
-	opt, err := redis.ParseURL(opts.RedisConnectionURL)
+func NewRedisSessionStore(opts *options.SessionOptions, cookieOpts *options.CookieOptions) (sessions.SessionStore, error) {
+	opt, err := redis.ParseURL(opts.RedisStoreOptions.RedisConnectionURL)
 	if err != nil {
 		return nil, fmt.Errorf("unable to parse redis url: %s", err)
 	}
 
-	var cookieCipher *cookie.Cipher
-	if len(cookieOpts.CookieSecret) > 0 {
-		var err error
-		cookieCipher, err = cookie.NewCipher(utils.SecretBytes(cookieOpts.CookieSecret))
-		if err != nil {
-			return nil, fmt.Errorf("unable to create cookieCipher: %v", err)
-		}
-	}
-
 	client := redis.NewClient(opt)
 
 	rs := &SessionStore{
-		Client:         client,
-		CookieCipher:   cookieCipher,
-		CookieDomain:   cookieOpts.CookieDomain,
-		CookieExpire:   cookieOpts.CookieExpire,
-		CookieHTTPOnly: cookieOpts.CookieHTTPOnly,
-		CookieName:     cookieOpts.CookieName,
-		CookiePath:     cookieOpts.CookiePath,
-		CookieSecret:   cookieOpts.CookieSecret,
-		CookieSecure:   cookieOpts.CookieSecure,
+		Client:        client,
+		CookieCipher:  opts.Cipher,
+		CookieOptions: cookieOpts,
 	}
 	return rs, nil
 
@@ -79,7 +57,7 @@ func NewRedisSessionStore(opts options.RedisStoreOptions, cookieOpts *options.Co
 func (store *SessionStore) Save(rw http.ResponseWriter, req *http.Request, s *sessions.SessionState) error {
 	// 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.CookieName)
+	requestCookie, _ := req.Cookie(store.CookieOptions.CookieName)
 	value, err := s.EncodeSessionState(store.CookieCipher)
 	if err != nil {
 		return err
@@ -89,15 +67,12 @@ func (store *SessionStore) Save(rw http.ResponseWriter, req *http.Request, s *se
 		return err
 	}
 
-	ticketCookie := cookies.MakeCookie(
+	ticketCookie := cookies.MakeCookieFromOptions(
 		req,
-		store.CookieName,
+		store.CookieOptions.CookieName,
 		ticketString,
-		store.CookiePath,
-		store.CookieDomain,
-		store.CookieHTTPOnly,
-		store.CookieSecure,
-		store.CookieExpire,
+		store.CookieOptions,
+		store.CookieOptions.CookieExpire,
 		time.Now(),
 	)
 
@@ -108,7 +83,7 @@ func (store *SessionStore) Save(rw http.ResponseWriter, req *http.Request, s *se
 // 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.CookieName)
+	requestCookie, err := req.Cookie(store.CookieOptions.CookieName)
 	if err != nil {
 		return nil, fmt.Errorf("error loading session: %s", err)
 	}
@@ -122,12 +97,12 @@ func (store *SessionStore) Load(req *http.Request) (*sessions.SessionState, erro
 
 // LoadSessionFromString loads the session based on the ticket value
 func (store *SessionStore) LoadSessionFromString(value string) (*sessions.SessionState, error) {
-	ticket, err := decodeTicket(store.CookieName, value)
+	ticket, err := decodeTicket(store.CookieOptions.CookieName, value)
 	if err != nil {
 		return nil, err
 	}
 
-	result, err := store.Client.Get(ticket.asHandle(store.CookieName)).Result()
+	result, err := store.Client.Get(ticket.asHandle(store.CookieOptions.CookieName)).Result()
 	if err != nil {
 		return nil, err
 	}
@@ -151,17 +126,14 @@ func (store *SessionStore) LoadSessionFromString(value string) (*sessions.Sessio
 // 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 {
-	requestCookie, _ := req.Cookie(store.CookieName)
+	requestCookie, _ := req.Cookie(store.CookieOptions.CookieName)
 
 	// We go ahead and clear the cookie first, always.
-	clearCookie := cookies.MakeCookie(
+	clearCookie := cookies.MakeCookieFromOptions(
 		req,
-		store.CookieName,
+		store.CookieOptions.CookieName,
 		"",
-		store.CookiePath,
-		store.CookieDomain,
-		store.CookieHTTPOnly,
-		store.CookieSecure,
+		store.CookieOptions,
 		time.Hour*-1,
 		time.Now(),
 	)
@@ -169,9 +141,9 @@ func (store *SessionStore) Clear(rw http.ResponseWriter, req *http.Request) erro
 
 	// 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.CookieName, requestCookie.Value)
+	ticket, _ := decodeTicket(store.CookieOptions.CookieName, requestCookie.Value)
 	if ticket != nil {
-		deleted, err := store.Client.Del(ticket.asHandle(store.CookieName)).Result()
+		deleted, err := store.Client.Del(ticket.asHandle(store.CookieOptions.CookieName)).Result()
 		fmt.Println("delted %n", deleted)
 		if err != nil {
 			return fmt.Errorf("error clearing cookie from redis: %s", err)
@@ -184,7 +156,7 @@ func (store *SessionStore) storeValue(value string, expiresOn time.Time, request
 	var ticket *TicketData
 	if requestCookie != nil {
 		var err error
-		ticket, err = decodeTicket(store.CookieName, requestCookie.Value)
+		ticket, err = decodeTicket(store.CookieOptions.CookieName, requestCookie.Value)
 		if err != nil {
 			return "", err
 		}
@@ -206,13 +178,13 @@ func (store *SessionStore) storeValue(value string, expiresOn time.Time, request
 	stream := cipher.NewCFBEncrypter(block, ticket.Secret)
 	stream.XORKeyStream(ciphertext, []byte(value))
 
-	handle := ticket.asHandle(store.CookieName)
+	handle := ticket.asHandle(store.CookieOptions.CookieName)
 	expires := expiresOn.Sub(time.Now())
 	err = store.Client.Set(handle, ciphertext, expires).Err()
 	if err != nil {
 		return "", err
 	}
-	return ticket.encodeTicket(store.CookieName), nil
+	return ticket.encodeTicket(store.CookieOptions.CookieName), nil
 }
 
 func newTicket() (*TicketData, error) {
diff --git a/pkg/sessions/session_store.go b/pkg/sessions/session_store.go
index e8e229a..17ef21c 100644
--- a/pkg/sessions/session_store.go
+++ b/pkg/sessions/session_store.go
@@ -15,7 +15,7 @@ func NewSessionStore(opts *options.SessionOptions, cookieOpts *options.CookieOpt
 	case options.CookieSessionStoreType:
 		return cookie.NewCookieSessionStore(opts, cookieOpts)
 	case options.RedisSessionStoreType:
-		return redis.NewRedisSessionStore(opts.RedisStoreOptions, cookieOpts)
+		return redis.NewRedisSessionStore(opts, cookieOpts)
 	default:
 		return nil, fmt.Errorf("unknown session store type '%s'", opts.Type)
 	}

From 2c566a5f5b65c3c6294a848ed51ddc7451a77d20 Mon Sep 17 00:00:00 2001
From: Joel Speed <Joel.speed@hotmail.co.uk>
Date: Wed, 15 May 2019 17:08:15 +0100
Subject: [PATCH 06/28] Use session CreatedAt for cookie timings

---
 pkg/sessions/redis/redis_store.go | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/pkg/sessions/redis/redis_store.go b/pkg/sessions/redis/redis_store.go
index ec55a03..01b3053 100644
--- a/pkg/sessions/redis/redis_store.go
+++ b/pkg/sessions/redis/redis_store.go
@@ -55,6 +55,10 @@ func NewRedisSessionStore(opts *options.SessionOptions, cookieOpts *options.Cook
 // 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)
@@ -73,7 +77,7 @@ func (store *SessionStore) Save(rw http.ResponseWriter, req *http.Request, s *se
 		ticketString,
 		store.CookieOptions,
 		store.CookieOptions.CookieExpire,
-		time.Now(),
+		s.CreatedAt,
 	)
 
 	http.SetCookie(rw, ticketCookie)

From b255ed56ef400fdd9b9e17439aa3463c1869bfe7 Mon Sep 17 00:00:00 2001
From: Joel Speed <Joel.speed@hotmail.co.uk>
Date: Wed, 15 May 2019 17:20:32 +0100
Subject: [PATCH 07/28] Sign cookies in the Redis Session store

---
 pkg/sessions/redis/redis_store.go | 41 ++++++++++++++++++++++---------
 1 file changed, 30 insertions(+), 11 deletions(-)

diff --git a/pkg/sessions/redis/redis_store.go b/pkg/sessions/redis/redis_store.go
index 01b3053..05ff5ec 100644
--- a/pkg/sessions/redis/redis_store.go
+++ b/pkg/sessions/redis/redis_store.go
@@ -71,11 +71,9 @@ func (store *SessionStore) Save(rw http.ResponseWriter, req *http.Request, s *se
 		return err
 	}
 
-	ticketCookie := cookies.MakeCookieFromOptions(
+	ticketCookie := store.makeCookie(
 		req,
-		store.CookieOptions.CookieName,
 		ticketString,
-		store.CookieOptions,
 		store.CookieOptions.CookieExpire,
 		s.CreatedAt,
 	)
@@ -91,8 +89,12 @@ func (store *SessionStore) Load(req *http.Request) (*sessions.SessionState, erro
 	if err != nil {
 		return nil, fmt.Errorf("error loading session: %s", err)
 	}
-	// No cookie validation necessary
-	session, err := store.LoadSessionFromString(requestCookie.Value)
+
+	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)
 	}
@@ -132,12 +134,15 @@ func (store *SessionStore) LoadSessionFromString(value string) (*sessions.Sessio
 func (store *SessionStore) Clear(rw http.ResponseWriter, req *http.Request) error {
 	requestCookie, _ := req.Cookie(store.CookieOptions.CookieName)
 
+	val, _, ok := cookie.Validate(requestCookie, store.CookieOptions.CookieSecret, store.CookieOptions.CookieExpire)
+	if !ok {
+		return fmt.Errorf("Cookie Signature not valid")
+	}
+
 	// We go ahead and clear the cookie first, always.
-	clearCookie := cookies.MakeCookieFromOptions(
+	clearCookie := store.makeCookie(
 		req,
-		store.CookieOptions.CookieName,
 		"",
-		store.CookieOptions,
 		time.Hour*-1,
 		time.Now(),
 	)
@@ -145,10 +150,9 @@ func (store *SessionStore) Clear(rw http.ResponseWriter, req *http.Request) erro
 
 	// 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, requestCookie.Value)
+	ticket, _ := decodeTicket(store.CookieOptions.CookieName, val)
 	if ticket != nil {
-		deleted, err := store.Client.Del(ticket.asHandle(store.CookieOptions.CookieName)).Result()
-		fmt.Println("delted %n", deleted)
+		_, err := store.Client.Del(ticket.asHandle(store.CookieOptions.CookieName)).Result()
 		if err != nil {
 			return fmt.Errorf("error clearing cookie from redis: %s", err)
 		}
@@ -156,6 +160,21 @@ func (store *SessionStore) Clear(rw http.ResponseWriter, req *http.Request) erro
 	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, expiresOn time.Time, requestCookie *http.Cookie) (string, error) {
 	var ticket *TicketData
 	if requestCookie != nil {

From 7a1fc52e3375ced61d7bbf4b066827dcd0b9535c Mon Sep 17 00:00:00 2001
From: Joel Speed <Joel.speed@hotmail.co.uk>
Date: Wed, 15 May 2019 17:24:06 +0100
Subject: [PATCH 08/28] Fix go-redis version pin

---
 Gopkg.toml | 1 -
 1 file changed, 1 deletion(-)

diff --git a/Gopkg.toml b/Gopkg.toml
index bdc0ae9..6778c97 100644
--- a/Gopkg.toml
+++ b/Gopkg.toml
@@ -48,7 +48,6 @@
   version = "2.1.0"
 
 [[constraint]]
-  branch = "master"
   name = "github.com/go-redis/redis"
   version = "v6.15.2"
 

From 130d03758d0cdb8be52d792174c4c15503ed48b7 Mon Sep 17 00:00:00 2001
From: Joel Speed <Joel.speed@hotmail.co.uk>
Date: Thu, 16 May 2019 17:03:38 +0100
Subject: [PATCH 09/28] Fix comments on Redis options

---
 pkg/apis/options/sessions.go | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/pkg/apis/options/sessions.go b/pkg/apis/options/sessions.go
index 591cc37..6787ec9 100644
--- a/pkg/apis/options/sessions.go
+++ b/pkg/apis/options/sessions.go
@@ -19,11 +19,11 @@ var CookieSessionStoreType = "cookie"
 // CookieStoreOptions contains configuration options for the CookieSessionStore.
 type CookieStoreOptions struct{}
 
-// RedisSessionStoreType is used to indicate the CookieSessionStore should be
+// RedisSessionStoreType is used to indicate the RedisSessionStore should be
 // used for storing sessions.
 var RedisSessionStoreType = "redis"
 
-// RedisStoreOptions contains configuration options for the CookieSessionStore.
+// 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"`
 }

From f435fa68abfea7602ab279e9458ab40247ce879e Mon Sep 17 00:00:00 2001
From: Joel Speed <Joel.speed@hotmail.co.uk>
Date: Thu, 16 May 2019 17:06:13 +0100
Subject: [PATCH 10/28] Make loadSessionFromString private

---
 pkg/sessions/redis/redis_store.go | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/pkg/sessions/redis/redis_store.go b/pkg/sessions/redis/redis_store.go
index 05ff5ec..a428033 100644
--- a/pkg/sessions/redis/redis_store.go
+++ b/pkg/sessions/redis/redis_store.go
@@ -94,15 +94,15 @@ func (store *SessionStore) Load(req *http.Request) (*sessions.SessionState, erro
 	if !ok {
 		return nil, fmt.Errorf("Cookie Signature not valid")
 	}
-	session, err := store.LoadSessionFromString(val)
+	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) {
+// 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

From 2f61e42c37687193a2f3fe52a7f56e3943b31693 Mon Sep 17 00:00:00 2001
From: Joel Speed <Joel.speed@hotmail.co.uk>
Date: Thu, 16 May 2019 17:07:43 +0100
Subject: [PATCH 11/28] More obvious comment on CFB

---
 pkg/sessions/redis/redis_store.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/pkg/sessions/redis/redis_store.go b/pkg/sessions/redis/redis_store.go
index a428033..154b839 100644
--- a/pkg/sessions/redis/redis_store.go
+++ b/pkg/sessions/redis/redis_store.go
@@ -197,7 +197,7 @@ func (store *SessionStore) storeValue(value string, expiresOn time.Time, request
 		return "", fmt.Errorf("error initiating cipher block %s", err)
 	}
 
-	// Use secret as the IV too, because each entry has it's own key
+	// 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))
 

From a6b8f7bde2987b77fe1127892a0098c793ff077c Mon Sep 17 00:00:00 2001
From: Joel Speed <Joel.speed@hotmail.co.uk>
Date: Thu, 16 May 2019 17:08:10 +0100
Subject: [PATCH 12/28] Rename expire -> expiration

---
 pkg/sessions/redis/redis_store.go | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/pkg/sessions/redis/redis_store.go b/pkg/sessions/redis/redis_store.go
index 154b839..fd71c4a 100644
--- a/pkg/sessions/redis/redis_store.go
+++ b/pkg/sessions/redis/redis_store.go
@@ -202,8 +202,8 @@ func (store *SessionStore) storeValue(value string, expiresOn time.Time, request
 	stream.XORKeyStream(ciphertext, []byte(value))
 
 	handle := ticket.asHandle(store.CookieOptions.CookieName)
-	expires := expiresOn.Sub(time.Now())
-	err = store.Client.Set(handle, ciphertext, expires).Err()
+	expiration := expiresOn.Sub(time.Now())
+	err = store.Client.Set(handle, ciphertext, expiration).Err()
 	if err != nil {
 		return "", err
 	}

From 93df7d913236a46fbbbe6270940c1ae0267ec09f Mon Sep 17 00:00:00 2001
From: Joel Speed <Joel.speed@hotmail.co.uk>
Date: Thu, 16 May 2019 17:08:59 +0100
Subject: [PATCH 13/28] Remove spurious comment

---
 pkg/sessions/redis/redis_store.go | 1 -
 1 file changed, 1 deletion(-)

diff --git a/pkg/sessions/redis/redis_store.go b/pkg/sessions/redis/redis_store.go
index fd71c4a..c55e9c7 100644
--- a/pkg/sessions/redis/redis_store.go
+++ b/pkg/sessions/redis/redis_store.go
@@ -250,7 +250,6 @@ func decodeTicket(cookieName string, ticketString string) (*TicketData, error) {
 	_, err := hex.DecodeString(ticketID)
 	if err != nil {
 		return nil, fmt.Errorf("server ticket failed sanity checks")
-		// s is not a valid
 	}
 
 	secret, err := base64.RawURLEncoding.DecodeString(secretBase64)

From a7693cc72ad7322b624d60c96ebfd868170d2294 Mon Sep 17 00:00:00 2001
From: Joel Speed <Joel.speed@hotmail.co.uk>
Date: Thu, 16 May 2019 17:13:14 +0100
Subject: [PATCH 14/28] Tranfser all cookies in tests

---
 pkg/sessions/session_store_test.go | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/pkg/sessions/session_store_test.go b/pkg/sessions/session_store_test.go
index 47780f6..ff074e2 100644
--- a/pkg/sessions/session_store_test.go
+++ b/pkg/sessions/session_store_test.go
@@ -115,8 +115,9 @@ var _ = Describe("NewSessionStore", func() {
 				err := ss.Save(saveResp, req, session)
 				Expect(err).ToNot(HaveOccurred())
 
-				resultCookie := saveResp.Result().Cookies()[0]
-				request.AddCookie(resultCookie)
+				for _, c := range saveResp.Result().Cookies() {
+					request.AddCookie(c)
+				}
 				err = ss.Clear(response, request)
 				Expect(err).ToNot(HaveOccurred())
 			})
@@ -177,14 +178,18 @@ var _ = Describe("NewSessionStore", func() {
 				err := ss.Save(saveResp, req, session)
 				Expect(err).ToNot(HaveOccurred())
 
-				resultCookie := saveResp.Result().Cookies()[0]
-				request.AddCookie(resultCookie)
+				resultCookies := saveResp.Result().Cookies()
+				for _, c := range resultCookies {
+					request.AddCookie(c)
+				}
 				err = ss.Clear(response, request)
 				Expect(err).ToNot(HaveOccurred())
 
 				// The following should only be for server stores
 				loadReq := httptest.NewRequest("GET", "http://example.com/", nil)
-				loadReq.AddCookie(resultCookie)
+				for _, c := range resultCookies {
+					loadReq.AddCookie(c)
+				}
 				loadedAfterClear, err = ss.Load(loadReq)
 			})
 

From 42f14a41d906b3cf9a7f1bb5fa92f8978794b79f Mon Sep 17 00:00:00 2001
From: Joel Speed <Joel.speed@hotmail.co.uk>
Date: Thu, 16 May 2019 17:25:41 +0100
Subject: [PATCH 15/28] Clean up persistent SessionStore tests

---
 pkg/sessions/session_store_test.go | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/pkg/sessions/session_store_test.go b/pkg/sessions/session_store_test.go
index ff074e2..b4e36ca 100644
--- a/pkg/sessions/session_store_test.go
+++ b/pkg/sessions/session_store_test.go
@@ -122,7 +122,7 @@ var _ = Describe("NewSessionStore", func() {
 				Expect(err).ToNot(HaveOccurred())
 			})
 
-			It("sets a `set-cookie` header in the response and session to not be loadable after clear", func() {
+			It("sets a `set-cookie` header in the response", func() {
 				Expect(response.Header().Get("Set-Cookie")).ToNot(BeEmpty())
 			})
 
@@ -169,8 +169,9 @@ var _ = Describe("NewSessionStore", func() {
 		})
 	}
 
+	// The following should only be for server stores
 	PersistentSessionStoreTests := func() {
-		Context("when Clear is called the session should not be recoverable", func() {
+		Context("when Clear is called on a persistent store", func() {
 			var loadedAfterClear *sessionsapi.SessionState
 			BeforeEach(func() {
 				req := httptest.NewRequest("GET", "http://example.com/", nil)
@@ -185,16 +186,21 @@ var _ = Describe("NewSessionStore", func() {
 				err = ss.Clear(response, request)
 				Expect(err).ToNot(HaveOccurred())
 
-				// The following should only be for server stores
 				loadReq := httptest.NewRequest("GET", "http://example.com/", nil)
 				for _, c := range resultCookies {
 					loadReq.AddCookie(c)
 				}
+
 				loadedAfterClear, err = ss.Load(loadReq)
+				// If we have cleared the session, Load should fail
+				Expect(err).To(HaveOccurred())
 			})
 
-			It("sets a `set-cookie` header in the response and session to not be loadable after clear", func() {
+			It("sets a `set-cookie` header in the response", func() {
 				Expect(response.Header().Get("Set-Cookie")).ToNot(BeEmpty())
+			})
+
+			It("attempting to Load returns an empty session", func() {
 				Expect(loadedAfterClear).To(BeNil())
 			})
 

From bc3d75a2ed6c34883de232362757e91b2479bd95 Mon Sep 17 00:00:00 2001
From: Joel Speed <Joel.speed@hotmail.co.uk>
Date: Thu, 16 May 2019 17:29:53 +0100
Subject: [PATCH 16/28] Run persistent tests with multiple option groups

---
 pkg/sessions/session_store_test.go | 109 +++++++++++++----------------
 1 file changed, 50 insertions(+), 59 deletions(-)

diff --git a/pkg/sessions/session_store_test.go b/pkg/sessions/session_store_test.go
index b4e36ca..c21987e 100644
--- a/pkg/sessions/session_store_test.go
+++ b/pkg/sessions/session_store_test.go
@@ -90,7 +90,46 @@ var _ = Describe("NewSessionStore", func() {
 		})
 	}
 
-	SessionStoreInterfaceTests := func() {
+	// The following should only be for server stores
+	PersistentSessionStoreTests := func() {
+		Context("when Clear is called on a persistent store", func() {
+			var loadedAfterClear *sessionsapi.SessionState
+			BeforeEach(func() {
+				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())
+
+				loadReq := httptest.NewRequest("GET", "http://example.com/", nil)
+				for _, c := range resultCookies {
+					loadReq.AddCookie(c)
+				}
+
+				loadedAfterClear, err = ss.Load(loadReq)
+				// If we have cleared the session, Load should fail
+				Expect(err).To(HaveOccurred())
+			})
+
+			It("sets a `set-cookie` header in the response", func() {
+				Expect(response.Header().Get("Set-Cookie")).ToNot(BeEmpty())
+			})
+
+			It("attempting to Load returns an empty session", func() {
+				Expect(loadedAfterClear).To(BeNil())
+			})
+
+			CheckCookieOptions()
+		})
+	}
+
+	SessionStoreInterfaceTests := func(persistent bool) {
 		Context("when Save is called", func() {
 			BeforeEach(func() {
 				err := ss.Save(response, request, session)
@@ -167,48 +206,13 @@ var _ = Describe("NewSessionStore", func() {
 				}
 			})
 		})
+
+		if persistent {
+			PersistentSessionStoreTests()
+		}
 	}
 
-	// The following should only be for server stores
-	PersistentSessionStoreTests := func() {
-		Context("when Clear is called on a persistent store", func() {
-			var loadedAfterClear *sessionsapi.SessionState
-			BeforeEach(func() {
-				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())
-
-				loadReq := httptest.NewRequest("GET", "http://example.com/", nil)
-				for _, c := range resultCookies {
-					loadReq.AddCookie(c)
-				}
-
-				loadedAfterClear, err = ss.Load(loadReq)
-				// If we have cleared the session, Load should fail
-				Expect(err).To(HaveOccurred())
-			})
-
-			It("sets a `set-cookie` header in the response", func() {
-				Expect(response.Header().Get("Set-Cookie")).ToNot(BeEmpty())
-			})
-
-			It("attempting to Load returns an empty session", func() {
-				Expect(loadedAfterClear).To(BeNil())
-			})
-
-			CheckCookieOptions()
-		})
-	}
-
-	RunSessionTests := func() {
+	RunSessionTests := func(persistent bool) {
 		Context("with default options", func() {
 			BeforeEach(func() {
 				var err error
@@ -216,7 +220,7 @@ var _ = Describe("NewSessionStore", func() {
 				Expect(err).ToNot(HaveOccurred())
 			})
 
-			SessionStoreInterfaceTests()
+			SessionStoreInterfaceTests(persistent)
 		})
 
 		Context("with non-default options", func() {
@@ -236,7 +240,7 @@ var _ = Describe("NewSessionStore", func() {
 				Expect(err).ToNot(HaveOccurred())
 			})
 
-			SessionStoreInterfaceTests()
+			SessionStoreInterfaceTests(persistent)
 		})
 
 		Context("with a cipher", func() {
@@ -254,19 +258,7 @@ var _ = Describe("NewSessionStore", func() {
 				Expect(err).ToNot(HaveOccurred())
 			})
 
-			SessionStoreInterfaceTests()
-		})
-	}
-
-	RunPersistentSessionStoreTests := func() {
-		Context("with default options", func() {
-			BeforeEach(func() {
-				var err error
-				ss, err = sessions.NewSessionStore(opts, cookieOpts)
-				Expect(err).ToNot(HaveOccurred())
-			})
-
-			PersistentSessionStoreTests()
+			SessionStoreInterfaceTests(persistent)
 		})
 	}
 
@@ -309,7 +301,7 @@ var _ = Describe("NewSessionStore", func() {
 		})
 
 		Context("the cookie.SessionStore", func() {
-			RunSessionTests()
+			RunSessionTests(false)
 		})
 	})
 
@@ -330,8 +322,7 @@ var _ = Describe("NewSessionStore", func() {
 		})
 
 		Context("the redis.SessionStore", func() {
-			RunSessionTests()
-			RunPersistentSessionStoreTests()
+			RunSessionTests(true)
 		})
 	})
 

From 7e7bfb5daffb3ad7beb103d1fc1af3236828d397 Mon Sep 17 00:00:00 2001
From: Joel Speed <Joel.speed@hotmail.co.uk>
Date: Thu, 16 May 2019 17:32:54 +0100
Subject: [PATCH 17/28] Stop miniredis after each test

---
 pkg/sessions/session_store_test.go | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/pkg/sessions/session_store_test.go b/pkg/sessions/session_store_test.go
index c21987e..8c5202b 100644
--- a/pkg/sessions/session_store_test.go
+++ b/pkg/sessions/session_store_test.go
@@ -306,15 +306,19 @@ var _ = Describe("NewSessionStore", func() {
 	})
 
 	Context("with type 'redis'", func() {
+		var mr *miniredis.Miniredis
 		BeforeEach(func() {
-			mr, err := miniredis.Run()
-			if err != nil {
-				panic(err)
-			}
+			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())

From 4f5dbace9f6dfd08f4a53cd6b2ecf69a258df2dd Mon Sep 17 00:00:00 2001
From: Joel Speed <Joel.speed@hotmail.co.uk>
Date: Thu, 16 May 2019 17:38:42 +0100
Subject: [PATCH 18/28] Refactor persistent tests with more Context

---
 pkg/sessions/session_store_test.go | 38 +++++++++++++++++-------------
 1 file changed, 22 insertions(+), 16 deletions(-)

diff --git a/pkg/sessions/session_store_test.go b/pkg/sessions/session_store_test.go
index 8c5202b..76722df 100644
--- a/pkg/sessions/session_store_test.go
+++ b/pkg/sessions/session_store_test.go
@@ -93,36 +93,42 @@ var _ = Describe("NewSessionStore", func() {
 	// The following should only be for server stores
 	PersistentSessionStoreTests := func() {
 		Context("when Clear is called on a persistent store", func() {
-			var loadedAfterClear *sessionsapi.SessionState
+			var resultCookies []*http.Cookie
+
 			BeforeEach(func() {
 				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()
+				resultCookies = saveResp.Result().Cookies()
 				for _, c := range resultCookies {
 					request.AddCookie(c)
 				}
 				err = ss.Clear(response, request)
 				Expect(err).ToNot(HaveOccurred())
-
-				loadReq := httptest.NewRequest("GET", "http://example.com/", nil)
-				for _, c := range resultCookies {
-					loadReq.AddCookie(c)
-				}
-
-				loadedAfterClear, err = ss.Load(loadReq)
-				// If we have cleared the session, Load should fail
-				Expect(err).To(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
 
-			It("attempting to Load returns an empty session", func() {
-				Expect(loadedAfterClear).To(BeNil())
+				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())
+				})
 			})
 
 			CheckCookieOptions()

From 5095c3647d9433ca1bf1e65a64b89b780c8e73aa Mon Sep 17 00:00:00 2001
From: Joel Speed <Joel.speed@hotmail.co.uk>
Date: Fri, 17 May 2019 13:16:43 +0100
Subject: [PATCH 19/28] Add redis-connection-url flag

---
 main.go | 1 +
 1 file changed, 1 insertion(+)

diff --git a/main.go b/main.go
index a74c245..f647651 100644
--- a/main.go
+++ b/main.go
@@ -76,6 +76,7 @@ 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.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")

From fc06e2dbef3389e9ba71c72f9569566902fff0aa Mon Sep 17 00:00:00 2001
From: Brian Van Klaveren <bvan@slac.stanford.edu>
Date: Mon, 20 May 2019 14:46:38 -0700
Subject: [PATCH 20/28] Update documentation and changelog for redis store

---
 CHANGELOG.md                        |  6 ++++++
 docs/configuration/configuration.md |  1 +
 docs/configuration/sessions.md      | 24 ++++++++++++++++++++++++
 main.go                             |  2 +-
 4 files changed, 32 insertions(+), 1 deletion(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 2b3c69b..8c85c9b 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -14,6 +14,12 @@
 
 ## Changes since v3.2.0
 
+- [#155](https://github.com/pusher/outh2_proxy/pull/155) Add RedisSessionStore implementation (@brianv0, @JoelSpeed)
+  - Implement flags to configure the redis session store
+    -  `-redis-connection-url`
+  - Introduces the concept of a session ticket. Tickets are composed of the cookie name, a session ID, and a secret. 
+  - Sessions are stored encrypted with a per-session secret 
+  - Added Some tests for a Server based session store
 - [#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/docs/configuration/configuration.md b/docs/configuration/configuration.md
index fd33d37..82b45a3 100644
--- a/docs/configuration/configuration.md
+++ b/docs/configuration/configuration.md
@@ -75,6 +75,7 @@ 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 type (eg: redis://HOST[:PORT])
   -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..103d424 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,26 @@ 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.
\ No newline at end of file
diff --git a/main.go b/main.go
index f647651..e649eba 100644
--- a/main.go
+++ b/main.go
@@ -76,7 +76,7 @@ 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.String("redis-connection-url", "", "URL of redis server for redis session storage type (eg: redis://HOST[:PORT])")
 
 	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")

From 518c1d3e8ed0fbebac48074518314acc5e65e6bc Mon Sep 17 00:00:00 2001
From: Joel Speed <Joel.speed@hotmail.co.uk>
Date: Fri, 24 May 2019 17:32:55 +0100
Subject: [PATCH 21/28] Add Redis sentinel compatibility

(cherry picked from commit ff36b61f8cee4ecf0b91a90b5e1b651b526bb6b6)
---
 main.go                           |  6 +++++-
 pkg/apis/options/sessions.go      |  5 ++++-
 pkg/sessions/redis/redis_store.go | 24 ++++++++++++++++++++----
 3 files changed, 29 insertions(+), 6 deletions(-)

diff --git a/main.go b/main.go
index e649eba..7f230f4 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,7 +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 type (eg: redis://HOST[:PORT])")
+	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")
diff --git a/pkg/apis/options/sessions.go b/pkg/apis/options/sessions.go
index 6787ec9..c72da3d 100644
--- a/pkg/apis/options/sessions.go
+++ b/pkg/apis/options/sessions.go
@@ -25,5 +25,8 @@ 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"`
+	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/sessions/redis/redis_store.go b/pkg/sessions/redis/redis_store.go
index c55e9c7..61aa1c7 100644
--- a/pkg/sessions/redis/redis_store.go
+++ b/pkg/sessions/redis/redis_store.go
@@ -36,13 +36,11 @@ type SessionStore struct {
 // NewRedisSessionStore initialises a new instance of the SessionStore from
 // the configuration given
 func NewRedisSessionStore(opts *options.SessionOptions, cookieOpts *options.CookieOptions) (sessions.SessionStore, error) {
-	opt, err := redis.ParseURL(opts.RedisStoreOptions.RedisConnectionURL)
+	client, err := newRedisClient(opts.RedisStoreOptions)
 	if err != nil {
-		return nil, fmt.Errorf("unable to parse redis url: %s", err)
+		return nil, fmt.Errorf("error constructing redis client: %v", err)
 	}
 
-	client := redis.NewClient(opt)
-
 	rs := &SessionStore{
 		Client:        client,
 		CookieCipher:  opts.Cipher,
@@ -52,6 +50,24 @@ func NewRedisSessionStore(opts *options.SessionOptions, cookieOpts *options.Cook
 
 }
 
+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 {

From ae0258a20385b59a401f64e99d3febd2ff43f179 Mon Sep 17 00:00:00 2001
From: Brian Van Klaveren <bvan@slac.stanford.edu>
Date: Tue, 28 May 2019 13:26:40 -0700
Subject: [PATCH 22/28] Documentation updates around Redis and Redis Sentinel
 use

---
 docs/configuration/configuration.md |  5 ++++-
 docs/configuration/sessions.md      | 11 ++++++++++-
 main.go                             |  4 ++--
 3 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/docs/configuration/configuration.md b/docs/configuration/configuration.md
index 82b45a3..d631eaf 100644
--- a/docs/configuration/configuration.md
+++ b/docs/configuration/configuration.md
@@ -75,7 +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 type (eg: redis://HOST[:PORT])
+  -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 103d424..0ffe392 100644
--- a/docs/configuration/sessions.md
+++ b/docs/configuration/sessions.md
@@ -55,4 +55,13 @@ to which the session is stored. The encoded session is encrypted with the secret
 in redis via the `SETEX` command.
 
 Encrypting every session uniquely protects the refresh/access/id tokens stored in the session from
-disclosure.
\ No newline at end of file
+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 7f230f4..a66c4fc 100644
--- a/main.go
+++ b/main.go
@@ -78,9 +78,9 @@ func main() {
 
 	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.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 conneciton URLs (eg redis://HOST[:PORT]). 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")

From 2e2327af6c941c46d61e2169b99389f3b16fce21 Mon Sep 17 00:00:00 2001
From: Joel Speed <Joel.speed@hotmail.co.uk>
Date: Wed, 29 May 2019 11:59:58 +0100
Subject: [PATCH 23/28] Check SaveSession works when an existing session is
 present

(cherry picked from commit 9dc1a96d817741632cb476456755a645b732db7d)
---
 pkg/sessions/redis/redis_store.go  |  9 +++++-
 pkg/sessions/session_store_test.go | 44 ++++++++++++++++++++++++------
 2 files changed, 44 insertions(+), 9 deletions(-)

diff --git a/pkg/sessions/redis/redis_store.go b/pkg/sessions/redis/redis_store.go
index 61aa1c7..b32d5c2 100644
--- a/pkg/sessions/redis/redis_store.go
+++ b/pkg/sessions/redis/redis_store.go
@@ -195,7 +195,14 @@ func (store *SessionStore) storeValue(value string, expiresOn time.Time, request
 	var ticket *TicketData
 	if requestCookie != nil {
 		var err error
-		ticket, err = decodeTicket(store.CookieOptions.CookieName, requestCookie.Value)
+		val, _, ok := cookie.Validate(requestCookie, store.CookieOptions.CookieSecret, store.CookieOptions.CookieExpire)
+		if !ok {
+			ticket, err = newTicket()
+			if err != nil {
+				return "", fmt.Errorf("error creating new ticket: %s", err)
+			}
+		}
+		ticket, err = decodeTicket(store.CookieOptions.CookieName, val)
 		if err != nil {
 			return "", err
 		}
diff --git a/pkg/sessions/session_store_test.go b/pkg/sessions/session_store_test.go
index 76722df..85e63b3 100644
--- a/pkg/sessions/session_store_test.go
+++ b/pkg/sessions/session_store_test.go
@@ -137,17 +137,45 @@ var _ = Describe("NewSessionStore", func() {
 
 	SessionStoreInterfaceTests := func(persistent bool) {
 		Context("when Save is called", func() {
-			BeforeEach(func() {
-				err := ss.Save(response, request, session)
-				Expect(err).ToNot(HaveOccurred())
+			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())
+				})
 			})
 
-			It("sets a `set-cookie` header in the response", func() {
-				Expect(response.Header().Get("set-cookie")).ToNot(BeEmpty())
-			})
+			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())
 
-			It("Ensures the session CreatedAt is not zero", func() {
-				Expect(session.CreatedAt.IsZero()).To(BeFalse())
+					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()

From 3155ada287da8f3602ea1f68df385d0cfd78ee22 Mon Sep 17 00:00:00 2001
From: Joel Speed <Joel.speed@hotmail.co.uk>
Date: Wed, 29 May 2019 15:25:56 +0100
Subject: [PATCH 24/28] Ensure sessions are refreshable in redis session store

(cherry picked from commit 48edce3003d187a3eadc4ea96236845271dd9360)
---
 pkg/sessions/redis/redis_store.go  |   5 +-
 pkg/sessions/session_store_test.go | 102 +++++++++++++++++++++--------
 2 files changed, 77 insertions(+), 30 deletions(-)

diff --git a/pkg/sessions/redis/redis_store.go b/pkg/sessions/redis/redis_store.go
index b32d5c2..6a5ffbb 100644
--- a/pkg/sessions/redis/redis_store.go
+++ b/pkg/sessions/redis/redis_store.go
@@ -82,7 +82,7 @@ func (store *SessionStore) Save(rw http.ResponseWriter, req *http.Request, s *se
 	if err != nil {
 		return err
 	}
-	ticketString, err := store.storeValue(value, s.ExpiresOn, requestCookie)
+	ticketString, err := store.storeValue(value, store.CookieOptions.CookieExpire, requestCookie)
 	if err != nil {
 		return err
 	}
@@ -191,7 +191,7 @@ func (store *SessionStore) makeCookie(req *http.Request, value string, expires t
 	)
 }
 
-func (store *SessionStore) storeValue(value string, expiresOn time.Time, requestCookie *http.Cookie) (string, error) {
+func (store *SessionStore) storeValue(value string, expiration time.Duration, requestCookie *http.Cookie) (string, error) {
 	var ticket *TicketData
 	if requestCookie != nil {
 		var err error
@@ -225,7 +225,6 @@ func (store *SessionStore) storeValue(value string, expiresOn time.Time, request
 	stream.XORKeyStream(ciphertext, []byte(value))
 
 	handle := ticket.asHandle(store.CookieOptions.CookieName)
-	expiration := expiresOn.Sub(time.Now())
 	err = store.Client.Set(handle, ciphertext, expiration).Err()
 	if err != nil {
 		return "", err
diff --git a/pkg/sessions/session_store_test.go b/pkg/sessions/session_store_test.go
index 85e63b3..2ffc0bd 100644
--- a/pkg/sessions/session_store_test.go
+++ b/pkg/sessions/session_store_test.go
@@ -35,6 +35,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() {
@@ -203,7 +204,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()
@@ -213,32 +245,49 @@ 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 {
@@ -263,7 +312,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",
@@ -305,7 +354,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,
 		}
@@ -340,7 +389,6 @@ var _ = Describe("NewSessionStore", func() {
 	})
 
 	Context("with type 'redis'", func() {
-		var mr *miniredis.Miniredis
 		BeforeEach(func() {
 			var err error
 			mr, err = miniredis.Run()

From 22199fa4173ca2f1b4b72f545231c7103ff86848 Mon Sep 17 00:00:00 2001
From: Joel Speed <Joel.speed@hotmail.co.uk>
Date: Thu, 30 May 2019 10:10:28 +0100
Subject: [PATCH 25/28] Fix ticket retrieval with an invalid ticket

(cherry picked from commit 66bbf146ec45d127bdd374120743aeef936894a7)
---
 pkg/sessions/redis/redis_store.go | 41 ++++++++++++++++---------------
 1 file changed, 21 insertions(+), 20 deletions(-)

diff --git a/pkg/sessions/redis/redis_store.go b/pkg/sessions/redis/redis_store.go
index 6a5ffbb..552b48d 100644
--- a/pkg/sessions/redis/redis_store.go
+++ b/pkg/sessions/redis/redis_store.go
@@ -192,26 +192,9 @@ func (store *SessionStore) makeCookie(req *http.Request, value string, expires t
 }
 
 func (store *SessionStore) storeValue(value string, expiration time.Duration, requestCookie *http.Cookie) (string, error) {
-	var ticket *TicketData
-	if requestCookie != nil {
-		var err error
-		val, _, ok := cookie.Validate(requestCookie, store.CookieOptions.CookieSecret, store.CookieOptions.CookieExpire)
-		if !ok {
-			ticket, err = newTicket()
-			if err != nil {
-				return "", fmt.Errorf("error creating new ticket: %s", err)
-			}
-		}
-		ticket, err = decodeTicket(store.CookieOptions.CookieName, val)
-		if err != nil {
-			return "", err
-		}
-	} else {
-		var err error
-		ticket, err = newTicket()
-		if err != nil {
-			return "", fmt.Errorf("error creating new ticket: %s", err)
-		}
+	ticket, err := store.getTicket(requestCookie)
+	if err != nil {
+		return "", fmt.Errorf("error getting ticket: %v", err)
 	}
 
 	ciphertext := make([]byte, len(value))
@@ -232,6 +215,24 @@ func (store *SessionStore) storeValue(value string, expiration time.Duration, re
 	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
+	return decodeTicket(store.CookieOptions.CookieName, val)
+}
+
 func newTicket() (*TicketData, error) {
 	rawID := make([]byte, 16)
 	if _, err := io.ReadFull(rand.Reader, rawID); err != nil {

From c1ae0ca807818b4bfdf7a49e59e064bafc05618c Mon Sep 17 00:00:00 2001
From: Joel Speed <Joel.speed@hotmail.co.uk>
Date: Thu, 30 May 2019 10:53:53 +0100
Subject: [PATCH 26/28] Make sure the cookie exists before we clear the session
 in redis

(cherry picked from commit 6d7f0ab57d554706425f76aed4df60717dd63ece)
---
 pkg/sessions/redis/redis_store.go | 21 ++++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/pkg/sessions/redis/redis_store.go b/pkg/sessions/redis/redis_store.go
index 552b48d..1d34d84 100644
--- a/pkg/sessions/redis/redis_store.go
+++ b/pkg/sessions/redis/redis_store.go
@@ -148,13 +148,6 @@ func (store *SessionStore) loadSessionFromString(value string) (*sessions.Sessio
 // 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 {
-	requestCookie, _ := req.Cookie(store.CookieOptions.CookieName)
-
-	val, _, ok := cookie.Validate(requestCookie, store.CookieOptions.CookieSecret, store.CookieOptions.CookieExpire)
-	if !ok {
-		return fmt.Errorf("Cookie Signature not valid")
-	}
-
 	// We go ahead and clear the cookie first, always.
 	clearCookie := store.makeCookie(
 		req,
@@ -164,6 +157,20 @@ func (store *SessionStore) Clear(rw http.ResponseWriter, req *http.Request) erro
 	)
 	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)

From 4721da02f2516a68f2504126a0f60cdcd0d35ac6 Mon Sep 17 00:00:00 2001
From: Joel Speed <Joel.speed@hotmail.co.uk>
Date: Thu, 30 May 2019 11:55:42 +0100
Subject: [PATCH 27/28] Ensure SessionStores can handle recieving cookies for
 the wrong implementation

(cherry picked from commit 131206cf41697543583751ac2714287898c19ad0)
---
 pkg/sessions/redis/redis_store.go  |  7 ++++++-
 pkg/sessions/session_store_test.go | 22 ++++++++++++++++++++++
 2 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/pkg/sessions/redis/redis_store.go b/pkg/sessions/redis/redis_store.go
index 1d34d84..82e941e 100644
--- a/pkg/sessions/redis/redis_store.go
+++ b/pkg/sessions/redis/redis_store.go
@@ -237,7 +237,12 @@ func (store *SessionStore) getTicket(requestCookie *http.Cookie) (*TicketData, e
 	}
 
 	// Valid cookie, decode the ticket
-	return decodeTicket(store.CookieOptions.CookieName, val)
+	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) {
diff --git a/pkg/sessions/session_store_test.go b/pkg/sessions/session_store_test.go
index 2ffc0bd..47ad4b7 100644
--- a/pkg/sessions/session_store_test.go
+++ b/pkg/sessions/session_store_test.go
@@ -16,6 +16,7 @@ import (
 	"github.com/pusher/oauth2_proxy/cookie"
 	"github.com/pusher/oauth2_proxy/pkg/apis/options"
 	sessionsapi "github.com/pusher/oauth2_proxy/pkg/apis/sessions"
+	"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"
@@ -153,6 +154,27 @@ var _ = Describe("NewSessionStore", func() {
 				})
 			})
 
+			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() {

From 405f9b3bb006b10fe16b8db854915781382db726 Mon Sep 17 00:00:00 2001
From: Brian Van Klaveren <bvan@slac.stanford.edu>
Date: Wed, 5 Jun 2019 00:02:49 -0700
Subject: [PATCH 28/28] Update CHANGELOG with descriptions about redis support
 Add updates from master

---
 CHANGELOG.md | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 8c85c9b..362cf1e 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -16,10 +16,14 @@
 
 - [#155](https://github.com/pusher/outh2_proxy/pull/155) Add RedisSessionStore implementation (@brianv0, @JoelSpeed)
   - Implement flags to configure the redis session store
-    -  `-redis-connection-url`
-  - Introduces the concept of a session ticket. Tickets are composed of the cookie name, a session ID, and a secret. 
-  - Sessions are stored encrypted with a per-session secret 
-  - Added Some tests for a Server based 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)