Move cipher creation to options and away from oauth2_proxy.go

pkg/apis/options/sessions.go

@@ -1,9 +1,13 @@
 package options
 
+import (
+	"github.com/pusher/oauth2_proxy/cookie"
+)
+
 // SessionOptions contains configuration options for the SessionStore providers.
 type SessionOptions struct {
-	Type         string `flag:"session-store-type" cfg:"session_store_type" env:"OAUTH2_PROXY_SESSION_STORE_TYPE"`
-	EnableCipher bool   // Allow the user to choose encryption or not
+	Type   string `flag:"session-store-type" cfg:"session_store_type" env:"OAUTH2_PROXY_SESSION_STORE_TYPE"`
+	Cipher *cookie.Cipher
 	CookieStoreOptions
 }
 
@@ -12,6 +16,4 @@ type SessionOptions struct {
 var CookieSessionStoreType = "cookie"
 
 // CookieStoreOptions contains configuration options for the CookieSessionStore.
-type CookieStoreOptions struct {
-	EnableCipher bool // Allow the user to choose encryption or not
-}
+type CookieStoreOptions struct{}

pkg/sessions/cookie/session_store.go

@@ -118,18 +118,9 @@ 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.CookieStoreOptions, cookieOpts *options.CookieOptions) (sessions.SessionStore, error) {
-	var cipher *cookie.Cipher
-	if opts.EnableCipher {
-		var err error
-		cipher, err = cookie.NewCipher(utils.SecretBytes(cookieOpts.CookieSecret))
-		if err != nil {
-			return nil, fmt.Errorf("unable to create cipher: %v", err)
-		}
-	}
-
+func NewCookieSessionStore(opts *options.SessionOptions, cookieOpts *options.CookieOptions) (sessions.SessionStore, error) {
 	return &SessionStore{
-		CookieCipher:  cipher,
+		CookieCipher:  opts.Cipher,
 		CookieOptions: cookieOpts,
 	}, nil
 }

pkg/sessions/session_store.go

@@ -12,9 +12,7 @@ import (
 func NewSessionStore(opts *options.SessionOptions, cookieOpts *options.CookieOptions) (sessions.SessionStore, error) {
 	switch opts.Type {
 	case options.CookieSessionStoreType:
-		// Ensure EnableCipher is propogated from the parent option
-		opts.CookieStoreOptions.EnableCipher = opts.EnableCipher
-		return cookie.NewCookieSessionStore(opts.CookieStoreOptions, cookieOpts)
+		return cookie.NewCookieSessionStore(opts, cookieOpts)
 	default:
 		return nil, fmt.Errorf("unknown session store type '%s'", opts.Type)
 	}

pkg/sessions/session_store_test.go

@@ -12,11 +12,13 @@ import (
 
 	. "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"
-	"github.com/pusher/oauth2_proxy/pkg/sessions/cookie"
+	sessionscookie "github.com/pusher/oauth2_proxy/pkg/sessions/cookie"
+	"github.com/pusher/oauth2_proxy/pkg/sessions/utils"
 )
 
 func TestSessionStore(t *testing.T) {
@@ -200,13 +202,16 @@ var _ = Describe("NewSessionStore", func() {
 			SessionStoreInterfaceTests()
 		})
 
-		Context("with encryption enabled", func() {
+		Context("with a cipher", func() {
 			BeforeEach(func() {
 				secret := make([]byte, 32)
 				_, err := rand.Read(secret)
 				Expect(err).ToNot(HaveOccurred())
 				cookieOpts.CookieSecret = base64.URLEncoding.EncodeToString(secret)
-				opts.EnableCipher = true
+				cipher, err := cookie.NewCipher(utils.SecretBytes(cookieOpts.CookieSecret))
+				Expect(err).ToNot(HaveOccurred())
+				Expect(cipher).ToNot(BeNil())
+				opts.Cipher = cipher
 
 				ss, err = sessions.NewSessionStore(opts, cookieOpts)
 				Expect(err).ToNot(HaveOccurred())
@@ -214,19 +219,6 @@ var _ = Describe("NewSessionStore", func() {
 
 			SessionStoreInterfaceTests()
 		})
-
-		Context("with encryption enabled, but no secret", func() {
-			BeforeEach(func() {
-				opts.EnableCipher = true
-			})
-
-			It("returns an error", func() {
-				ss, err := sessions.NewSessionStore(opts, cookieOpts)
-				Expect(err).To(HaveOccurred())
-				Expect(err.Error()).To(Equal("unable to create cipher: crypto/aes: invalid key size 0"))
-				Expect(ss).To(BeNil())
-			})
-		})
 	}
 
 	BeforeEach(func() {
@@ -264,7 +256,7 @@ var _ = Describe("NewSessionStore", func() {
 		It("creates a cookie.SessionStore", func() {
 			ss, err := sessions.NewSessionStore(opts, cookieOpts)
 			Expect(err).NotTo(HaveOccurred())
-			Expect(ss).To(BeAssignableToTypeOf(&cookie.SessionStore{}))
+			Expect(ss).To(BeAssignableToTypeOf(&sessionscookie.SessionStore{}))
 		})
 
 		Context("the cookie.SessionStore", func() {