Refactor the utils package to other areas (#538)

* Refactor the utils package to other areas

Move cookieSession functions to cookie session store
& align the double implementation of SecretBytes to be
united and housed under encryption

* Remove unused Provider SessionFromCookie/CookieForSession

These implementations aren't used, these are handled in the cookie store.

* Add changelog entry for session/utils refactor

CHANGELOG.md

@@ -34,7 +34,7 @@
     Use `--prefer-email-to-user` to restore falling back to the Email in these cases.
 
 ## Changes since v5.1.1
-
+- [#538](https://github.com/oauth2-proxy/oauth2-proxy/pull/538) Refactor sessions/utils.go functionality to other areas (@NickMeves)
 - [#503](https://github.com/oauth2-proxy/oauth2-proxy/pull/503) Implements --real-client-ip-header option to select the header from which to obtain a proxied client's IP (@Izzette)
 - [#529](https://github.com/oauth2-proxy/oauth2-proxy/pull/529) Add local test environments for testing changes and new features (@JoelSpeed)
 - [#537](https://github.com/oauth2-proxy/oauth2-proxy/pull/537) Drop Fallback to Email if User not set (@JoelSpeed)

options.go

@@ -4,7 +4,6 @@ import (
 	"context"
 	"crypto"
 	"crypto/tls"
-	"encoding/base64"
 	"fmt"
 	"io/ioutil"
 	"net/http"
@@ -386,12 +385,12 @@ func (o *Options) Validate() error {
 	if o.PassAccessToken || o.SetAuthorization || o.PassAuthorization || (o.Cookie.Refresh != time.Duration(0)) {
 		validCookieSecretSize := false
 		for _, i := range []int{16, 24, 32} {
-			if len(secretBytes(o.Cookie.Secret)) == i {
+			if len(encryption.SecretBytes(o.Cookie.Secret)) == i {
 				validCookieSecretSize = true
 			}
 		}
 		var decoded bool
-		if string(secretBytes(o.Cookie.Secret)) != o.Cookie.Secret {
+		if string(encryption.SecretBytes(o.Cookie.Secret)) != o.Cookie.Secret {
 			decoded = true
 		}
 		if !validCookieSecretSize {
@@ -404,10 +403,10 @@ func (o *Options) Validate() error {
 					"to create an AES cipher when "+
 					"pass_access_token == true or "+
 					"cookie_refresh != 0, but is %d bytes.%s",
-				len(secretBytes(o.Cookie.Secret)), suffix))
+				len(encryption.SecretBytes(o.Cookie.Secret)), suffix))
 		} else {
 			var err error
-			cipher, err = encryption.NewCipher(secretBytes(o.Cookie.Secret))
+			cipher, err = encryption.NewCipher(encryption.SecretBytes(o.Cookie.Secret))
 			if err != nil {
 				msgs = append(msgs, fmt.Sprintf("cookie-secret error: %v", err))
 			}
@@ -643,29 +642,6 @@ func validateCookieName(o *Options, msgs []string) []string {
 	return msgs
 }
 
-func addPadding(secret string) string {
-	padding := len(secret) % 4
-	switch padding {
-	case 1:
-		return secret + "==="
-	case 2:
-		return secret + "=="
-	case 3:
-		return secret + "="
-	default:
-		return secret
-	}
-}
-
-// secretBytes attempts to base64 decode the secret, if that fails it treats the secret as binary
-func secretBytes(secret string) []byte {
-	b, err := base64.URLEncoding.DecodeString(addPadding(secret))
-	if err == nil {
-		return []byte(addPadding(string(b)))
-	}
-	return []byte(secret)
-}
-
 func setupLogger(o *Options, msgs []string) []string {
 	// Setup the log file
 	if len(o.LoggingFilename) > 0 {

pkg/encryption/cipher.go

@@ -17,6 +17,29 @@ import (
 	"time"
 )
 
+// SecretBytes attempts to base64 decode the secret, if that fails it treats the secret as binary
+func SecretBytes(secret string) []byte {
+	b, err := base64.URLEncoding.DecodeString(addPadding(secret))
+	if err == nil {
+		return []byte(addPadding(string(b)))
+	}
+	return []byte(secret)
+}
+
+func addPadding(secret string) string {
+	padding := len(secret) % 4
+	switch padding {
+	case 1:
+		return secret + "==="
+	case 2:
+		return secret + "=="
+	case 3:
+		return secret + "="
+	default:
+		return secret
+	}
+}
+
 // cookies are stored in a 3 part (value + timestamp + signature) to enforce that the values are as originally set.
 // additionally, the 'value' is encrypted so it's opaque to the browser
 

pkg/sessions/cookie/session_store.go

@@ -13,7 +13,6 @@ import (
 	"github.com/oauth2-proxy/oauth2-proxy/pkg/cookies"
 	"github.com/oauth2-proxy/oauth2-proxy/pkg/encryption"
 	"github.com/oauth2-proxy/oauth2-proxy/pkg/logger"
-	"github.com/oauth2-proxy/oauth2-proxy/pkg/sessions/utils"
 )
 
 const (
@@ -38,7 +37,7 @@ func (s *SessionStore) Save(rw http.ResponseWriter, req *http.Request, ss *sessi
 	if ss.CreatedAt.IsZero() {
 		ss.CreatedAt = time.Now()
 	}
-	value, err := utils.CookieForSession(ss, s.CookieCipher)
+	value, err := cookieForSession(ss, s.CookieCipher)
 	if err != nil {
 		return err
 	}
@@ -59,7 +58,7 @@ func (s *SessionStore) Load(req *http.Request) (*sessions.SessionState, error) {
 		return nil, errors.New("cookie signature not valid")
 	}
 
-	session, err := utils.SessionFromCookie(val, s.CookieCipher)
+	session, err := sessionFromCookie(val, s.CookieCipher)
 	if err != nil {
 		return nil, err
 	}
@@ -83,6 +82,16 @@ func (s *SessionStore) Clear(rw http.ResponseWriter, req *http.Request) error {
 	return nil
 }
 
+// cookieForSession serializes a session state for storage in a cookie
+func cookieForSession(s *sessions.SessionState, c *encryption.Cipher) (string, error) {
+	return s.EncodeSessionState(c)
+}
+
+// sessionFromCookie deserializes a session from a cookie value
+func sessionFromCookie(v string, c *encryption.Cipher) (s *sessions.SessionState, err error) {
+	return sessions.DecodeSessionState(v, c)
+}
+
 // setSessionCookie adds the user's session cookie to the response
 func (s *SessionStore) setSessionCookie(rw http.ResponseWriter, req *http.Request, val string, created time.Time) {
 	for _, c := range s.makeSessionCookie(req, val, created) {

pkg/sessions/session_store_test.go

@@ -18,7 +18,6 @@ import (
 	"github.com/oauth2-proxy/oauth2-proxy/pkg/sessions"
 	sessionscookie "github.com/oauth2-proxy/oauth2-proxy/pkg/sessions/cookie"
 	"github.com/oauth2-proxy/oauth2-proxy/pkg/sessions/redis"
-	"github.com/oauth2-proxy/oauth2-proxy/pkg/sessions/utils"
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
 )
@@ -365,7 +364,7 @@ var _ = Describe("NewSessionStore", func() {
 				_, err := rand.Read(secret)
 				Expect(err).ToNot(HaveOccurred())
 				cookieOpts.Secret = base64.URLEncoding.EncodeToString(secret)
-				cipher, err := encryption.NewCipher(utils.SecretBytes(cookieOpts.Secret))
+				cipher, err := encryption.NewCipher(encryption.SecretBytes(cookieOpts.Secret))
 				Expect(err).ToNot(HaveOccurred())
 				Expect(cipher).ToNot(BeNil())
 				opts.Cipher = cipher

pkg/sessions/utils/utils.go

@@ -1,41 +0,0 @@
-package utils
-
-import (
-	"encoding/base64"
-
-	"github.com/oauth2-proxy/oauth2-proxy/pkg/apis/sessions"
-	"github.com/oauth2-proxy/oauth2-proxy/pkg/encryption"
-)
-
-// CookieForSession serializes a session state for storage in a cookie
-func CookieForSession(s *sessions.SessionState, c *encryption.Cipher) (string, error) {
-	return s.EncodeSessionState(c)
-}
-
-// SessionFromCookie deserializes a session from a cookie value
-func SessionFromCookie(v string, c *encryption.Cipher) (s *sessions.SessionState, err error) {
-	return sessions.DecodeSessionState(v, c)
-}
-
-// SecretBytes attempts to base64 decode the secret, if that fails it treats the secret as binary
-func SecretBytes(secret string) []byte {
-	b, err := base64.URLEncoding.DecodeString(addPadding(secret))
-	if err == nil {
-		return []byte(addPadding(string(b)))
-	}
-	return []byte(secret)
-}
-
-func addPadding(secret string) string {
-	padding := len(secret) % 4
-	switch padding {
-	case 1:
-		return secret + "==="
-	case 2:
-		return secret + "=="
-	case 3:
-		return secret + "="
-	default:
-		return secret
-	}
-}

providers/provider_default.go

@@ -14,7 +14,6 @@ import (
 	"github.com/coreos/go-oidc"
 
 	"github.com/oauth2-proxy/oauth2-proxy/pkg/apis/sessions"
-	"github.com/oauth2-proxy/oauth2-proxy/pkg/encryption"
 )
 
 var _ Provider = (*ProviderData)(nil)
@@ -108,16 +107,6 @@ func (p *ProviderData) GetLoginURL(redirectURI, state string) string {
 	return a.String()
 }
 
-// CookieForSession serializes a session state for storage in a cookie
-func (p *ProviderData) CookieForSession(s *sessions.SessionState, c *encryption.Cipher) (string, error) {
-	return s.EncodeSessionState(c)
-}
-
-// SessionFromCookie deserializes a session from a cookie value
-func (p *ProviderData) SessionFromCookie(v string, c *encryption.Cipher) (s *sessions.SessionState, err error) {
-	return sessions.DecodeSessionState(v, c)
-}
-
 // GetEmailAddress returns the Account email address
 func (p *ProviderData) GetEmailAddress(ctx context.Context, s *sessions.SessionState) (string, error) {
 	return "", errors.New("not implemented")

providers/providers.go

@@ -5,7 +5,6 @@ import (
 
 	"github.com/coreos/go-oidc"
 	"github.com/oauth2-proxy/oauth2-proxy/pkg/apis/sessions"
-	"github.com/oauth2-proxy/oauth2-proxy/pkg/encryption"
 )
 
 // Provider represents an upstream identity provider implementation
@@ -19,8 +18,6 @@ type Provider interface {
 	ValidateSessionState(ctx context.Context, s *sessions.SessionState) bool
 	GetLoginURL(redirectURI, finalRedirect string) string
 	RefreshSessionIfNeeded(ctx context.Context, s *sessions.SessionState) (bool, error)
-	SessionFromCookie(string, *encryption.Cipher) (*sessions.SessionState, error)
-	CookieForSession(*sessions.SessionState, *encryption.Cipher) (string, error)
 	CreateSessionStateFromBearerToken(ctx context.Context, rawIDToken string, idToken *oidc.IDToken) (*sessions.SessionState, error)
 }