From cf79fd9e4c3ced4f4d201e101be6885b875908d4 Mon Sep 17 00:00:00 2001
From: Mike Bland <michael.bland@gsa.gov>
Date: Sun, 5 Apr 2015 09:43:40 -0400
Subject: [PATCH] Refactor pass_access_token+cookie_secret check

Moves the check from NewOauthProxy() to Options.Validate() and adds a test.
---
 oauthproxy.go   | 19 ++-----------------
 options.go      | 17 +++++++++++++++++
 options_test.go | 19 +++++++++++++++++++
 3 files changed, 38 insertions(+), 17 deletions(-)

diff --git a/oauthproxy.go b/oauthproxy.go
index 0b5141e..5b0fece 100644
--- a/oauthproxy.go
+++ b/oauthproxy.go
@@ -47,7 +47,6 @@ type OauthProxy struct {
 	DisplayHtpasswdForm bool
 	serveMux            http.Handler
 	PassBasicAuth       bool
-	PassAccessToken     bool
 	AesCipher           cipher.Block
 	skipAuthRegex       []string
 	compiledRegex       []*regexp.Regexp
@@ -121,20 +120,7 @@ func NewOauthProxy(opts *Options, validator func(string) bool) *OauthProxy {
 	log.Printf("Cookie settings: secure (https):%v httponly:%v expiry:%s domain:%s", opts.CookieSecure, opts.CookieHttpOnly, opts.CookieExpire, domain)
 
 	var aes_cipher cipher.Block
-
-	if opts.PassAccessToken == true {
-		valid_cookie_secret_size := false
-		for _, i := range []int{16, 24, 32} {
-			if len(opts.CookieSecret) == i {
-				valid_cookie_secret_size = true
-			}
-		}
-		if valid_cookie_secret_size == false {
-			log.Fatal("cookie_secret must be 16, 24, or 32 bytes " +
-				"to create an AES cipher when " +
-				"pass_access_token == true")
-		}
-
+	if opts.PassAccessToken {
 		var err error
 		aes_cipher, err = aes.NewCipher([]byte(opts.CookieSecret))
 		if err != nil {
@@ -163,7 +149,6 @@ func NewOauthProxy(opts *Options, validator func(string) bool) *OauthProxy {
 		skipAuthRegex:      opts.SkipAuthRegex,
 		compiledRegex:      opts.CompiledRegex,
 		PassBasicAuth:      opts.PassBasicAuth,
-		PassAccessToken:    opts.PassAccessToken,
 		AesCipher:          aes_cipher,
 		templates:          loadTemplates(opts.CustomTemplatesDir),
 	}
@@ -441,7 +426,7 @@ func (p *OauthProxy) ServeHTTP(rw http.ResponseWriter, req *http.Request) {
 		if p.Validator(email) {
 			log.Printf("%s authenticating %s completed", remoteAddr, email)
 			encoded_token := ""
-			if p.PassAccessToken {
+			if p.AesCipher != nil {
 				encoded_token, err = encodeAccessToken(p.AesCipher, access_token)
 				if err != nil {
 					log.Printf("error encoding access token: %s", err)
diff --git a/options.go b/options.go
index e02cfb8..bbfd466 100644
--- a/options.go
+++ b/options.go
@@ -117,6 +117,23 @@ func (o *Options) Validate() error {
 	}
 	msgs = parseProviderInfo(o, msgs)
 
+	if o.PassAccessToken {
+		valid_cookie_secret_size := false
+		for _, i := range []int{16, 24, 32} {
+			if len(o.CookieSecret) == i {
+				valid_cookie_secret_size = true
+			}
+		}
+		if valid_cookie_secret_size == false {
+			msgs = append(msgs, fmt.Sprintf(
+				"cookie_secret must be 16, 24, or 32 bytes "+
+					"to create an AES cipher when "+
+					"pass_access_token == true, "+
+					"but is %d bytes",
+				len(o.CookieSecret)))
+		}
+	}
+
 	if len(msgs) != 0 {
 		return fmt.Errorf("Invalid configuration:\n  %s",
 			strings.Join(msgs, "\n  "))
diff --git a/options_test.go b/options_test.go
index 515c1c8..dcb5421 100644
--- a/options_test.go
+++ b/options_test.go
@@ -102,3 +102,22 @@ func TestDefaultProviderApiSettings(t *testing.T) {
 	assert.Equal(t, "", p.ProfileUrl.String())
 	assert.Equal(t, "profile email", p.Scope)
 }
+
+func TestPassAccessTokenRequiresSpecificCookieSecretLengths(t *testing.T) {
+	o := testOptions()
+	assert.Equal(t, nil, o.Validate())
+
+	assert.Equal(t, false, o.PassAccessToken)
+	o.PassAccessToken = true
+	o.CookieSecret = "cookie of invalid length-"
+	assert.NotEqual(t, nil, o.Validate())
+
+	o.CookieSecret = "16 bytes AES-128"
+	assert.Equal(t, nil, o.Validate())
+
+	o.CookieSecret = "24 byte secret AES-192--"
+	assert.Equal(t, nil, o.Validate())
+
+	o.CookieSecret = "32 byte secret for AES-256------"
+	assert.Equal(t, nil, o.Validate())
+}