From 6e1b3b9660bf7968d175fcfdeffb677ad3a2f4fe Mon Sep 17 00:00:00 2001 From: Joel Speed Date: Sun, 28 Jun 2020 12:44:12 +0100 Subject: [PATCH] Switch to in session store initialisation --- oauthproxy_test.go | 9 ++---- pkg/sessions/cookie/session_store.go | 7 ++++- pkg/sessions/redis/redis_store.go | 7 ++++- pkg/sessions/redis/redis_store_test.go | 12 ++++---- pkg/sessions/session_store.go | 9 ++---- pkg/sessions/session_store_test.go | 39 +++++++++++++++++--------- 6 files changed, 48 insertions(+), 35 deletions(-) diff --git a/oauthproxy_test.go b/oauthproxy_test.go index 14a137ed..cd7e8e53 100644 --- a/oauthproxy_test.go +++ b/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)) - assert.Equal(t, nil, err) - store, err := cookie.NewCookieSessionStore(&opts.Session, &opts.Cookie, cipher) + store, err := cookie.NewCookieSessionStore(&opts.Session, &opts.Cookie) 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)) - assert.Equal(t, nil, err) - store, err := cookie.NewCookieSessionStore(&opts.Session, &opts.Cookie, cipher) + store, err := cookie.NewCookieSessionStore(&opts.Session, &opts.Cookie) assert.Equal(t, nil, err) p := OAuthProxy{CookieName: opts.Cookie.Name, CookieDomains: opts.Cookie.Domains, sessionStore: store} var rw = httptest.NewRecorder() diff --git a/pkg/sessions/cookie/session_store.go b/pkg/sessions/cookie/session_store.go index 77dc1504..00fb02b4 100644 --- a/pkg/sessions/cookie/session_store.go +++ b/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, diff --git a/pkg/sessions/redis/redis_store.go b/pkg/sessions/redis/redis_store.go index 36298b9f..2b79aebc 100644 --- a/pkg/sessions/redis/redis_store.go +++ b/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) diff --git a/pkg/sessions/redis/redis_store_test.go b/pkg/sessions/redis/redis_store_test.go index 70f35f42..e4bd7919 100644 --- a/pkg/sessions/redis/redis_store_test.go +++ b/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(), diff --git a/pkg/sessions/session_store.go b/pkg/sessions/session_store.go index c0054ba6..992d6ded 100644 --- a/pkg/sessions/session_store.go +++ b/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) } diff --git a/pkg/sessions/session_store_test.go b/pkg/sessions/session_store_test.go index ac2a4911..70aa4e52 100644 --- a/pkg/sessions/session_store_test.go +++ b/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,6 +454,19 @@ var _ = Describe("NewSessionStore", func() { Context("the redis.SessionStore", func() { RunSessionTests(true) }) + + 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 an invalid type", func() { @@ -455,17 +481,4 @@ var _ = Describe("NewSessionStore", func() { Expect(ss).To(BeNil()) }) }) - - 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()) - }) - }) })