Switch to in session store initialisation

oauthproxy_test.go

@@ -22,7 +22,6 @@ import (
 	"github.com/mbland/hmacauth"
 	"github.com/oauth2-proxy/oauth2-proxy/pkg/apis/options"
 	"github.com/oauth2-proxy/oauth2-proxy/pkg/apis/sessions"
-	"github.com/oauth2-proxy/oauth2-proxy/pkg/encryption"
 	"github.com/oauth2-proxy/oauth2-proxy/pkg/logger"
 	"github.com/oauth2-proxy/oauth2-proxy/pkg/sessions/cookie"
 	"github.com/oauth2-proxy/oauth2-proxy/pkg/validation"
@@ -1605,9 +1604,7 @@ func TestClearSplitCookie(t *testing.T) {
 	opts.Cookie.Secret = base64CookieSecret
 	opts.Cookie.Name = "oauth2"
 	opts.Cookie.Domains = []string{"abc"}
-	cipher, err := encryption.NewBase64Cipher(encryption.NewCFBCipher, encryption.SecretBytes(opts.Cookie.Secret))
+	store, err := cookie.NewCookieSessionStore(&opts.Session, &opts.Cookie)
-	assert.Equal(t, nil, err)
-	store, err := cookie.NewCookieSessionStore(&opts.Session, &opts.Cookie, cipher)
 	assert.Equal(t, nil, err)
 	p := OAuthProxy{CookieName: opts.Cookie.Name, CookieDomains: opts.Cookie.Domains, sessionStore: store}
 	var rw = httptest.NewRecorder()
@@ -1636,9 +1633,7 @@ func TestClearSingleCookie(t *testing.T) {
 	opts := baseTestOptions()
 	opts.Cookie.Name = "oauth2"
 	opts.Cookie.Domains = []string{"abc"}
-	cipher, err := encryption.NewBase64Cipher(encryption.NewCFBCipher, encryption.SecretBytes(opts.Cookie.Secret))
+	store, err := cookie.NewCookieSessionStore(&opts.Session, &opts.Cookie)
-	assert.Equal(t, nil, err)
-	store, err := cookie.NewCookieSessionStore(&opts.Session, &opts.Cookie, cipher)
 	assert.Equal(t, nil, err)
 	p := OAuthProxy{CookieName: opts.Cookie.Name, CookieDomains: opts.Cookie.Domains, sessionStore: store}
 	var rw = httptest.NewRecorder()

pkg/sessions/cookie/session_store.go

@@ -126,7 +126,12 @@ func (s *SessionStore) makeCookie(req *http.Request, name string, value string,
 
 // NewCookieSessionStore initialises a new instance of the SessionStore from
 // the configuration given
-func NewCookieSessionStore(opts *options.SessionOptions, cookieOpts *options.CookieOptions, cipher encryption.Cipher) (sessions.SessionStore, error) {
+func NewCookieSessionStore(opts *options.SessionOptions, cookieOpts *options.CookieOptions) (sessions.SessionStore, error) {
+	cipher, err := encryption.NewBase64Cipher(encryption.NewCFBCipher, encryption.SecretBytes(cookieOpts.Secret))
+	if err != nil {
+		return nil, fmt.Errorf("error initialising cipher: %v", err)
+	}
+
 	return &SessionStore{
 		CookieCipher:  cipher,
 		CookieOptions: cookieOpts,

pkg/sessions/redis/redis_store.go

@@ -39,7 +39,12 @@ type SessionStore struct {
 
 // NewRedisSessionStore initialises a new instance of the SessionStore from
 // the configuration given
-func NewRedisSessionStore(opts *options.SessionOptions, cookieOpts *options.CookieOptions, cipher encryption.Cipher) (sessions.SessionStore, error) {
+func NewRedisSessionStore(opts *options.SessionOptions, cookieOpts *options.CookieOptions) (sessions.SessionStore, error) {
+	cipher, err := encryption.NewBase64Cipher(encryption.NewCFBCipher, encryption.SecretBytes(cookieOpts.Secret))
+	if err != nil {
+		return nil, fmt.Errorf("error initialising cipher: %v", err)
+	}
+
 	client, err := newRedisCmdable(opts.Redis)
 	if err != nil {
 		return nil, fmt.Errorf("error constructing redis client: %v", err)

pkg/sessions/redis/redis_store_test.go

@@ -11,7 +11,6 @@ import (
 	"github.com/alicebob/miniredis/v2"
 	"github.com/oauth2-proxy/oauth2-proxy/pkg/apis/options"
 	"github.com/oauth2-proxy/oauth2-proxy/pkg/apis/sessions"
-	"github.com/oauth2-proxy/oauth2-proxy/pkg/encryption"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
@@ -21,9 +20,6 @@ func TestRedisStore(t *testing.T) {
 	_, err := rand.Read(secret)
 	assert.NoError(t, err)
 
-	cipher, err := encryption.NewBase64Cipher(encryption.NewCFBCipher, encryption.SecretBytes(string(secret)))
-	assert.NoError(t, err)
-
 	t.Run("save session on redis standalone", func(t *testing.T) {
 		redisServer, err := miniredis.Run()
 		require.NoError(t, err)
@@ -34,7 +30,9 @@ func TestRedisStore(t *testing.T) {
 			Host:   redisServer.Addr(),
 		}
 		opts.Session.Redis.ConnectionURL = redisURL.String()
-		redisStore, err := NewRedisSessionStore(&opts.Session, &opts.Cookie, cipher)
+
+		opts.Cookie.Secret = string(secret)
+		redisStore, err := NewRedisSessionStore(&opts.Session, &opts.Cookie)
 		require.NoError(t, err)
 		err = redisStore.Save(
 			httptest.NewRecorder(),
@@ -58,7 +56,9 @@ func TestRedisStore(t *testing.T) {
 		opts.Session.Redis.SentinelConnectionURLs = []string{sentinelURL.String()}
 		opts.Session.Redis.UseSentinel = true
 		opts.Session.Redis.SentinelMasterName = sentinel.MasterInfo().Name
-		redisStore, err := NewRedisSessionStore(&opts.Session, &opts.Cookie, cipher)
+
+		opts.Cookie.Secret = string(secret)
+		redisStore, err := NewRedisSessionStore(&opts.Session, &opts.Cookie)
 		require.NoError(t, err)
 		err = redisStore.Save(
 			httptest.NewRecorder(),

pkg/sessions/session_store.go

@@ -5,22 +5,17 @@ import (
 
 	"github.com/oauth2-proxy/oauth2-proxy/pkg/apis/options"
 	"github.com/oauth2-proxy/oauth2-proxy/pkg/apis/sessions"
-	"github.com/oauth2-proxy/oauth2-proxy/pkg/encryption"
 	"github.com/oauth2-proxy/oauth2-proxy/pkg/sessions/cookie"
 	"github.com/oauth2-proxy/oauth2-proxy/pkg/sessions/redis"
 )
 
 // NewSessionStore creates a SessionStore from the provided configuration
 func NewSessionStore(opts *options.SessionOptions, cookieOpts *options.CookieOptions) (sessions.SessionStore, error) {
-	cipher, err := encryption.NewBase64Cipher(encryption.NewCFBCipher, encryption.SecretBytes(cookieOpts.Secret))
-	if err != nil {
-		return nil, fmt.Errorf("error initialising cipher: %v", err)
-	}
 	switch opts.Type {
 	case options.CookieSessionStoreType:
-		return cookie.NewCookieSessionStore(opts, cookieOpts, cipher)
+		return cookie.NewCookieSessionStore(opts, cookieOpts)
 	case options.RedisSessionStoreType:
-		return redis.NewRedisSessionStore(opts, cookieOpts, cipher)
+		return redis.NewRedisSessionStore(opts, cookieOpts)
 	default:
 		return nil, fmt.Errorf("unknown session store type '%s'", opts.Type)
 	}

pkg/sessions/session_store_test.go

@@ -417,6 +417,19 @@ var _ = Describe("NewSessionStore", func() {
 		Context("the cookie.SessionStore", func() {
 			RunSessionTests(false)
 		})
+
+		Context("with an invalid cookie secret", func() {
+			BeforeEach(func() {
+				cookieOpts.Secret = "invalid"
+			})
+
+			It("returns an error", func() {
+				ss, err := sessions.NewSessionStore(opts, cookieOpts)
+				Expect(err).To(HaveOccurred())
+				Expect(err.Error()).To(Equal("error initialising cipher: crypto/aes: invalid key size 7"))
+				Expect(ss).To(BeNil())
+			})
+		})
 	})
 
 	Context("with type 'redis'", func() {
@@ -441,20 +454,6 @@ var _ = Describe("NewSessionStore", func() {
 		Context("the redis.SessionStore", func() {
 			RunSessionTests(true)
 		})
-	})
-
-	Context("with an invalid type", func() {
-		BeforeEach(func() {
-			opts.Type = "invalid-type"
-		})
-
-		It("returns an error", func() {
-			ss, err := sessions.NewSessionStore(opts, cookieOpts)
-			Expect(err).To(HaveOccurred())
-			Expect(err.Error()).To(Equal("unknown session store type 'invalid-type'"))
-			Expect(ss).To(BeNil())
-		})
-	})
 
 		Context("with an invalid cookie secret", func() {
 			BeforeEach(func() {
@@ -469,3 +468,17 @@ var _ = Describe("NewSessionStore", func() {
 			})
 		})
 	})
+
+	Context("with an invalid type", func() {
+		BeforeEach(func() {
+			opts.Type = "invalid-type"
+		})
+
+		It("returns an error", func() {
+			ss, err := sessions.NewSessionStore(opts, cookieOpts)
+			Expect(err).To(HaveOccurred())
+			Expect(err.Error()).To(Equal("unknown session store type 'invalid-type'"))
+			Expect(ss).To(BeNil())
+		})
+	})
+})