Refactor pass_access_token+cookie_secret check
Moves the check from NewOauthProxy() to Options.Validate() and adds a test.
This commit is contained in:
		
							parent
							
								
									ca32394c6f
								
							
						
					
					
						commit
						cf79fd9e4c
					
				|  | @ -47,7 +47,6 @@ type OauthProxy struct { | ||||||
| 	DisplayHtpasswdForm bool | 	DisplayHtpasswdForm bool | ||||||
| 	serveMux            http.Handler | 	serveMux            http.Handler | ||||||
| 	PassBasicAuth       bool | 	PassBasicAuth       bool | ||||||
| 	PassAccessToken     bool |  | ||||||
| 	AesCipher           cipher.Block | 	AesCipher           cipher.Block | ||||||
| 	skipAuthRegex       []string | 	skipAuthRegex       []string | ||||||
| 	compiledRegex       []*regexp.Regexp | 	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) | 	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 | 	var aes_cipher cipher.Block | ||||||
| 
 | 	if opts.PassAccessToken { | ||||||
| 	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") |  | ||||||
| 		} |  | ||||||
| 
 |  | ||||||
| 		var err error | 		var err error | ||||||
| 		aes_cipher, err = aes.NewCipher([]byte(opts.CookieSecret)) | 		aes_cipher, err = aes.NewCipher([]byte(opts.CookieSecret)) | ||||||
| 		if err != nil { | 		if err != nil { | ||||||
|  | @ -163,7 +149,6 @@ func NewOauthProxy(opts *Options, validator func(string) bool) *OauthProxy { | ||||||
| 		skipAuthRegex:      opts.SkipAuthRegex, | 		skipAuthRegex:      opts.SkipAuthRegex, | ||||||
| 		compiledRegex:      opts.CompiledRegex, | 		compiledRegex:      opts.CompiledRegex, | ||||||
| 		PassBasicAuth:      opts.PassBasicAuth, | 		PassBasicAuth:      opts.PassBasicAuth, | ||||||
| 		PassAccessToken:    opts.PassAccessToken, |  | ||||||
| 		AesCipher:          aes_cipher, | 		AesCipher:          aes_cipher, | ||||||
| 		templates:          loadTemplates(opts.CustomTemplatesDir), | 		templates:          loadTemplates(opts.CustomTemplatesDir), | ||||||
| 	} | 	} | ||||||
|  | @ -441,7 +426,7 @@ func (p *OauthProxy) ServeHTTP(rw http.ResponseWriter, req *http.Request) { | ||||||
| 		if p.Validator(email) { | 		if p.Validator(email) { | ||||||
| 			log.Printf("%s authenticating %s completed", remoteAddr, email) | 			log.Printf("%s authenticating %s completed", remoteAddr, email) | ||||||
| 			encoded_token := "" | 			encoded_token := "" | ||||||
| 			if p.PassAccessToken { | 			if p.AesCipher != nil { | ||||||
| 				encoded_token, err = encodeAccessToken(p.AesCipher, access_token) | 				encoded_token, err = encodeAccessToken(p.AesCipher, access_token) | ||||||
| 				if err != nil { | 				if err != nil { | ||||||
| 					log.Printf("error encoding access token: %s", err) | 					log.Printf("error encoding access token: %s", err) | ||||||
|  |  | ||||||
							
								
								
									
										17
									
								
								options.go
								
								
								
								
							
							
						
						
									
										17
									
								
								options.go
								
								
								
								
							|  | @ -117,6 +117,23 @@ func (o *Options) Validate() error { | ||||||
| 	} | 	} | ||||||
| 	msgs = parseProviderInfo(o, msgs) | 	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 { | 	if len(msgs) != 0 { | ||||||
| 		return fmt.Errorf("Invalid configuration:\n  %s", | 		return fmt.Errorf("Invalid configuration:\n  %s", | ||||||
| 			strings.Join(msgs, "\n  ")) | 			strings.Join(msgs, "\n  ")) | ||||||
|  |  | ||||||
|  | @ -102,3 +102,22 @@ func TestDefaultProviderApiSettings(t *testing.T) { | ||||||
| 	assert.Equal(t, "", p.ProfileUrl.String()) | 	assert.Equal(t, "", p.ProfileUrl.String()) | ||||||
| 	assert.Equal(t, "profile email", p.Scope) | 	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()) | ||||||
|  | } | ||||||
|  |  | ||||||
		Loading…
	
		Reference in New Issue