mirror of
https://github.com/oauth2-proxy/oauth2-proxy.git
synced 2025-04-27 12:32:10 +02:00
Add tests for cookie validation
This also removes the check for the decoded from the valid secret size check. The code was unreachable because encryption.SecretBytes will only return the decoded secret if it was the right length after decoding.
This commit is contained in:
Joel Speed
parent
900061b88a
commit
285c65a2d4
@ -55,15 +55,8 @@ func validateCookieSecret(secret string) []string {
|
||||
return []string{}
|
||||
}
|
||||
// Invalid secret size found, return a message
|
||||
|
||||
// If the secretBytes is different to the raw secret, it was decoded from Base64
|
||||
// Add a note to the error message
|
||||
var decodedSuffix string
|
||||
if string(secretBytes) != secret {
|
||||
decodedSuffix = " note: cookie secret was base64 decoded"
|
||||
}
|
||||
|
||||
return []string{fmt.Sprintf(
|
||||
"cookie_secret must be 16, 24, or 32 bytes to create an AES cipher, but is %d bytes.%s",
|
||||
len(secretBytes), decodedSuffix)}
|
||||
"cookie_secret must be 16, 24, or 32 bytes to create an AES cipher, but is %d bytes",
|
||||
len(secretBytes)),
|
||||
}
|
||||
}
|
||||
|
||||
250
pkg/validation/cookie_test.go
Normal file
250
pkg/validation/cookie_test.go
Normal file
@ -0,0 +1,250 @@
|
||||
package validation
|
||||
|
||||
import (
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"github.com/oauth2-proxy/oauth2-proxy/pkg/apis/options"
|
||||
. "github.com/onsi/gomega"
|
||||
)
|
||||
|
||||
func TestValidateCookie(t *testing.T) {
|
||||
validName := "_oauth2_proxy"
|
||||
invalidName := "_oauth2;proxy" // Separater character not allowed
|
||||
validSecret := "secretthirtytwobytes+abcdefghijk"
|
||||
invalidSecret := "abcdef" // 6 bytes is not a valid size
|
||||
validBase64Secret := "c2VjcmV0dGhpcnR5dHdvYnl0ZXMrYWJjZGVmZ2hpams" // Base64 encoding of "secretthirtytwobytes+abcdefghijk"
|
||||
invalidBase64Secret := "YWJjZGVmCg" // Base64 encoding of "abcdef"
|
||||
emptyDomains := []string{}
|
||||
domains := []string{
|
||||
"a.localhost",
|
||||
"ba.localhost",
|
||||
"ca.localhost",
|
||||
"cba.localhost",
|
||||
"a.cba.localhost",
|
||||
}
|
||||
|
||||
invalidNameMsg := "invalid cookie name: \"_oauth2;proxy\""
|
||||
missingSecretMsg := "missing setting: cookie-secret"
|
||||
invalidSecretMsg := "cookie_secret must be 16, 24, or 32 bytes to create an AES cipher, but is 6 bytes"
|
||||
invalidBase64SecretMsg := "cookie_secret must be 16, 24, or 32 bytes to create an AES cipher, but is 10 bytes"
|
||||
refreshLongerThanExpireMsg := "cookie_refresh (\"1h0m0s\") must be less than cookie_expire (\"15m0s\")"
|
||||
invalidSameSiteMsg := "cookie_samesite (\"invalid\") must be one of ['', 'lax', 'strict', 'none']"
|
||||
|
||||
testCases := []struct {
|
||||
name string
|
||||
cookie options.CookieOptions
|
||||
errStrings []string
|
||||
}{
|
||||
{
|
||||
name: "with valid configuration",
|
||||
cookie: options.CookieOptions{
|
||||
Name: validName,
|
||||
Secret: validSecret,
|
||||
Domains: domains,
|
||||
Path: "",
|
||||
Expire: time.Hour,
|
||||
Refresh: 15 * time.Minute,
|
||||
Secure: true,
|
||||
HTTPOnly: false,
|
||||
SameSite: "",
|
||||
},
|
||||
errStrings: []string{},
|
||||
},
|
||||
{
|
||||
name: "with no cookie secret",
|
||||
cookie: options.CookieOptions{
|
||||
Name: validName,
|
||||
Secret: "",
|
||||
Domains: emptyDomains,
|
||||
Path: "",
|
||||
Expire: time.Hour,
|
||||
Refresh: 15 * time.Minute,
|
||||
Secure: true,
|
||||
HTTPOnly: false,
|
||||
SameSite: "",
|
||||
},
|
||||
errStrings: []string{
|
||||
missingSecretMsg,
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "with an invalid cookie secret",
|
||||
cookie: options.CookieOptions{
|
||||
Name: validName,
|
||||
Secret: invalidSecret,
|
||||
Domains: emptyDomains,
|
||||
Path: "",
|
||||
Expire: time.Hour,
|
||||
Refresh: 15 * time.Minute,
|
||||
Secure: true,
|
||||
HTTPOnly: false,
|
||||
SameSite: "",
|
||||
},
|
||||
errStrings: []string{
|
||||
invalidSecretMsg,
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "with a valid Base64 secret",
|
||||
cookie: options.CookieOptions{
|
||||
Name: validName,
|
||||
Secret: validBase64Secret,
|
||||
Domains: emptyDomains,
|
||||
Path: "",
|
||||
Expire: time.Hour,
|
||||
Refresh: 15 * time.Minute,
|
||||
Secure: true,
|
||||
HTTPOnly: false,
|
||||
SameSite: "",
|
||||
},
|
||||
errStrings: []string{},
|
||||
},
|
||||
{
|
||||
name: "with an invalid Base64 secret",
|
||||
cookie: options.CookieOptions{
|
||||
Name: validName,
|
||||
Secret: invalidBase64Secret,
|
||||
Domains: emptyDomains,
|
||||
Path: "",
|
||||
Expire: time.Hour,
|
||||
Refresh: 15 * time.Minute,
|
||||
Secure: true,
|
||||
HTTPOnly: false,
|
||||
SameSite: "",
|
||||
},
|
||||
errStrings: []string{
|
||||
invalidBase64SecretMsg,
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "with an invalid name",
|
||||
cookie: options.CookieOptions{
|
||||
Name: invalidName,
|
||||
Secret: validSecret,
|
||||
Domains: emptyDomains,
|
||||
Path: "",
|
||||
Expire: time.Hour,
|
||||
Refresh: 15 * time.Minute,
|
||||
Secure: true,
|
||||
HTTPOnly: false,
|
||||
SameSite: "",
|
||||
},
|
||||
errStrings: []string{
|
||||
invalidNameMsg,
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "with refresh longer than expire",
|
||||
cookie: options.CookieOptions{
|
||||
Name: validName,
|
||||
Secret: validSecret,
|
||||
Domains: emptyDomains,
|
||||
Path: "",
|
||||
Expire: 15 * time.Minute,
|
||||
Refresh: time.Hour,
|
||||
Secure: true,
|
||||
HTTPOnly: false,
|
||||
SameSite: "",
|
||||
},
|
||||
errStrings: []string{
|
||||
refreshLongerThanExpireMsg,
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "with samesite \"none\"",
|
||||
cookie: options.CookieOptions{
|
||||
Name: validName,
|
||||
Secret: validSecret,
|
||||
Domains: emptyDomains,
|
||||
Path: "",
|
||||
Expire: time.Hour,
|
||||
Refresh: 15 * time.Minute,
|
||||
Secure: true,
|
||||
HTTPOnly: false,
|
||||
SameSite: "none",
|
||||
},
|
||||
errStrings: []string{},
|
||||
},
|
||||
{
|
||||
name: "with samesite \"lax\"",
|
||||
cookie: options.CookieOptions{
|
||||
Name: validName,
|
||||
Secret: validSecret,
|
||||
Domains: emptyDomains,
|
||||
Path: "",
|
||||
Expire: time.Hour,
|
||||
Refresh: 15 * time.Minute,
|
||||
Secure: true,
|
||||
HTTPOnly: false,
|
||||
SameSite: "none",
|
||||
},
|
||||
errStrings: []string{},
|
||||
},
|
||||
{
|
||||
name: "with samesite \"strict\"",
|
||||
cookie: options.CookieOptions{
|
||||
Name: validName,
|
||||
Secret: validSecret,
|
||||
Domains: emptyDomains,
|
||||
Path: "",
|
||||
Expire: time.Hour,
|
||||
Refresh: 15 * time.Minute,
|
||||
Secure: true,
|
||||
HTTPOnly: false,
|
||||
SameSite: "none",
|
||||
},
|
||||
errStrings: []string{},
|
||||
},
|
||||
{
|
||||
name: "with samesite \"invalid\"",
|
||||
cookie: options.CookieOptions{
|
||||
Name: validName,
|
||||
Secret: validSecret,
|
||||
Domains: emptyDomains,
|
||||
Path: "",
|
||||
Expire: time.Hour,
|
||||
Refresh: 15 * time.Minute,
|
||||
Secure: true,
|
||||
HTTPOnly: false,
|
||||
SameSite: "invalid",
|
||||
},
|
||||
errStrings: []string{
|
||||
invalidSameSiteMsg,
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "with a combination of configuration errors",
|
||||
cookie: options.CookieOptions{
|
||||
Name: invalidName,
|
||||
Secret: invalidSecret,
|
||||
Domains: domains,
|
||||
Path: "",
|
||||
Expire: 15 * time.Minute,
|
||||
Refresh: time.Hour,
|
||||
Secure: true,
|
||||
HTTPOnly: false,
|
||||
SameSite: "invalid",
|
||||
},
|
||||
errStrings: []string{
|
||||
invalidNameMsg,
|
||||
invalidSecretMsg,
|
||||
refreshLongerThanExpireMsg,
|
||||
invalidSameSiteMsg,
|
||||
},
|
||||
},
|
||||
}
|
||||
|
||||
for _, tc := range testCases {
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
errStrings := validateCookieOptions(tc.cookie)
|
||||
g := NewWithT(t)
|
||||
|
||||
g.Expect(errStrings).To(ConsistOf(tc.errStrings))
|
||||
// Check domains were sorted to the right lengths
|
||||
for i := 0; i < len(tc.cookie.Domains)-1; i++ {
|
||||
g.Expect(len(tc.cookie.Domains[i])).To(BeNumerically(">=", len(tc.cookie.Domains[i+1])))
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
@ -2,7 +2,6 @@ package validation
|
||||
|
||||
import (
|
||||
"crypto"
|
||||
"fmt"
|
||||
"io/ioutil"
|
||||
"net/url"
|
||||
"os"
|
||||
@ -291,20 +290,6 @@ func TestValidateSignatureKeyUnsupportedAlgorithm(t *testing.T) {
|
||||
" unsupported signature hash algorithm: "+o.SignatureKey)
|
||||
}
|
||||
|
||||
func TestValidateCookie(t *testing.T) {
|
||||
o := testOptions()
|
||||
o.Cookie.Name = "_valid_cookie_name"
|
||||
assert.Equal(t, nil, Validate(o))
|
||||
}
|
||||
|
||||
func TestValidateCookieBadName(t *testing.T) {
|
||||
o := testOptions()
|
||||
o.Cookie.Name = "_bad_cookie_name{}"
|
||||
err := Validate(o)
|
||||
assert.Equal(t, err.Error(), "invalid configuration:\n"+
|
||||
fmt.Sprintf(" invalid cookie name: %q", o.Cookie.Name))
|
||||
}
|
||||
|
||||
func TestSkipOIDCDiscovery(t *testing.T) {
|
||||
o := testOptions()
|
||||
o.ProviderType = "oidc"
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user