diff --git a/oauthproxy.go b/oauthproxy.go index 7081f6e2..6fe7ee39 100644 --- a/oauthproxy.go +++ b/oauthproxy.go @@ -445,16 +445,21 @@ func (p *OAuthProxy) makeCookie(req *http.Request, name string, value string, ex } } - return &http.Cookie{ + c := &http.Cookie{ Name: name, Value: value, Path: p.CookiePath, Domain: cookieDomain, HttpOnly: p.CookieHTTPOnly, Secure: p.CookieSecure, - Expires: now.Add(expiration), SameSite: cookies.ParseSameSite(p.CookieSameSite), } + + if expiration != time.Duration(0) { + c.Expires = now.Add(expiration) + } + + return c } // ClearCSRFCookie creates a cookie to unset the CSRF cookie stored in the user's diff --git a/pkg/cookies/cookies.go b/pkg/cookies/cookies.go index 80453ab2..1a27a5d5 100644 --- a/pkg/cookies/cookies.go +++ b/pkg/cookies/cookies.go @@ -25,16 +25,22 @@ func MakeCookie(req *http.Request, name string, value string, path string, domai } } - return &http.Cookie{ + c := &http.Cookie{ Name: name, Value: value, Path: path, Domain: domain, HttpOnly: httpOnly, Secure: secure, - Expires: now.Add(expiration), + SameSite: sameSite, } + + if expiration != time.Duration(0) { + c.Expires = now.Add(expiration) + } + + return c } // MakeCookieFromOptions constructs a cookie based on the given *options.CookieOptions, diff --git a/pkg/cookies/cookies_test.go b/pkg/cookies/cookies_test.go new file mode 100644 index 00000000..524f6635 --- /dev/null +++ b/pkg/cookies/cookies_test.go @@ -0,0 +1,154 @@ +package cookies + +import ( + "fmt" + "net/http" + "time" + + "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/options" + + middlewareapi "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/middleware" + . "github.com/onsi/ginkgo" + . "github.com/onsi/ginkgo/extensions/table" + . "github.com/onsi/gomega" +) + +const cookiePath = "/cookie-tests" + +var _ = Describe("Cookie Tests", func() { + Context("GetCookieDomain", func() { + type getCookieDomainTableInput struct { + host string + xForwardedHost string + cookieDomains []string + expectedOutput string + } + + DescribeTable("should return expected results", + func(in getCookieDomainTableInput) { + req, err := http.NewRequest( + http.MethodGet, + fmt.Sprintf("https://%s/%s", in.host, cookiePath), + nil, + ) + Expect(err).ToNot(HaveOccurred()) + + if in.xForwardedHost != "" { + req.Header.Add("X-Forwarded-Host", in.xForwardedHost) + req = middlewareapi.AddRequestScope(req, &middlewareapi.RequestScope{ + ReverseProxy: true, + }) + } + + Expect(GetCookieDomain(req, in.cookieDomains)).To(Equal(in.expectedOutput)) + }, + Entry("a single exact match for the Host header", getCookieDomainTableInput{ + host: "www.cookies.test", + cookieDomains: []string{"www.cookies.test"}, + expectedOutput: "www.cookies.test", + }), + Entry("a single exact match for the X-Forwarded-Host header", getCookieDomainTableInput{ + host: "backend.cookies.internal", + xForwardedHost: "www.cookies.test", + cookieDomains: []string{"www.cookies.test"}, + expectedOutput: "www.cookies.test", + }), + Entry("a single suffix match for the Host header", getCookieDomainTableInput{ + host: "www.cookies.test", + cookieDomains: []string{".cookies.test"}, + expectedOutput: ".cookies.test", + }), + Entry("a single suffix match for the X-Forwarded-Host header", getCookieDomainTableInput{ + host: "backend.cookies.internal", + xForwardedHost: "www.cookies.test", + cookieDomains: []string{".cookies.test"}, + expectedOutput: ".cookies.test", + }), + Entry("the first match is used", getCookieDomainTableInput{ + host: "www.cookies.test", + cookieDomains: []string{"www.cookies.test", ".cookies.test"}, + expectedOutput: "www.cookies.test", + }), + Entry("the only match is used", getCookieDomainTableInput{ + host: "www.cookies.test", + cookieDomains: []string{".cookies.wrong", ".cookies.test"}, + expectedOutput: ".cookies.test", + }), + Entry("blank is returned for no matches", getCookieDomainTableInput{ + host: "www.cookies.test", + cookieDomains: []string{".cookies.wrong", ".cookies.false"}, + expectedOutput: "", + }), + ) + }) + + Context("MakeCookieFromOptions", func() { + type MakeCookieFromOptionsTableInput struct { + host string + name string + value string + opts options.Cookie + expiration time.Duration + now time.Time + expectedOutput time.Time + } + + validName := "_oauth2_proxy" + validSecret := "secretthirtytwobytes+abcdefghijk" + domains := []string{"www.cookies.test"} + + now := time.Now() + var expectedExpires time.Time + + DescribeTable("should return expected results", + func(in MakeCookieFromOptionsTableInput) { + req, err := http.NewRequest( + http.MethodGet, + fmt.Sprintf("https://%s/%s", in.host, cookiePath), + nil, + ) + Expect(err).ToNot(HaveOccurred()) + + Expect(MakeCookieFromOptions(req, in.name, in.value, &in.opts, in.expiration, in.now).Expires).To(Equal(in.expectedOutput)) + }, + Entry("persistent cookie", MakeCookieFromOptionsTableInput{ + host: "www.cookies.test", + name: validName, + value: "1", + opts: options.Cookie{ + Name: validName, + Secret: validSecret, + Domains: domains, + Path: "", + Expire: time.Hour, + Refresh: 15 * time.Minute, + Secure: true, + HTTPOnly: false, + SameSite: "", + }, + expiration: 15 * time.Minute, + now: now, + expectedOutput: now.Add(15 * time.Minute), + }), + Entry("session cookie", MakeCookieFromOptionsTableInput{ + host: "www.cookies.test", + name: validName, + value: "1", + opts: options.Cookie{ + Name: validName, + Secret: validSecret, + Domains: domains, + Path: "", + Expire: 0, + Refresh: 15 * time.Minute, + Secure: true, + HTTPOnly: false, + SameSite: "", + }, + expiration: 0, + now: now, + expectedOutput: expectedExpires, + }), + ) + }) +}) diff --git a/pkg/encryption/utils.go b/pkg/encryption/utils.go index c9d19249..2f4891e4 100644 --- a/pkg/encryption/utils.go +++ b/pkg/encryption/utils.go @@ -50,8 +50,7 @@ func Validate(cookie *http.Cookie, seed string, expiration time.Duration) (value // creation timestamp stored in the cookie falls within the // window defined by (Now()-expiration, Now()]. t = time.Unix(int64(ts), 0) - if t.After(time.Now().Add(expiration*-1)) && t.Before(time.Now().Add(time.Minute*5)) { - // it's a valid cookie. now get the contents + if (expiration == time.Duration(0)) || t.After(time.Now().Add(expiration*-1)) && t.Before(time.Now().Add(time.Minute*5)) { // it's a valid cookie. now get the contents rawValue, err := base64.URLEncoding.DecodeString(parts[0]) if err == nil { value = rawValue diff --git a/pkg/encryption/utils_test.go b/pkg/encryption/utils_test.go index 2500d4ab..753f9b7e 100644 --- a/pkg/encryption/utils_test.go +++ b/pkg/encryption/utils_test.go @@ -7,7 +7,10 @@ import ( "encoding/base64" "fmt" "io" + "net/http" + "strconv" "testing" + "time" "github.com/stretchr/testify/assert" ) @@ -100,3 +103,27 @@ func TestSignAndValidate(t *testing.T) { assert.False(t, checkSignature(sha256sig, seed, key, "tampered", epoch)) assert.False(t, checkSignature(sha1sig, seed, key, "tampered", epoch)) } + +func TestValidate(t *testing.T) { + seed := "0123456789abcdef" + key := "cookie-name" + value := base64.URLEncoding.EncodeToString([]byte("I am soooo encoded")) + epoch := int64(123456789) + epochStr := strconv.FormatInt(epoch, 10) + + sha256sig, err := cookieSignature(sha256.New, seed, key, value, epochStr) + assert.NoError(t, err) + + cookie := &http.Cookie{ + Name: key, + Value: value + "|" + epochStr + "|" + sha256sig, + } + + validValue, timestamp, ok := Validate(cookie, seed, 0) + assert.True(t, ok) + assert.Equal(t, timestamp, time.Unix(epoch, 0)) + + expectedValue, err := base64.URLEncoding.DecodeString(value) + assert.NoError(t, err) + assert.Equal(t, validValue, expectedValue) +} diff --git a/pkg/validation/cookie.go b/pkg/validation/cookie.go index 2984ac2e..b515809d 100644 --- a/pkg/validation/cookie.go +++ b/pkg/validation/cookie.go @@ -4,6 +4,7 @@ import ( "fmt" "net/http" "sort" + "time" "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/apis/options" "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/encryption" @@ -12,7 +13,7 @@ import ( func validateCookie(o options.Cookie) []string { msgs := validateCookieSecret(o.Secret) - if o.Refresh >= o.Expire { + if o.Expire != time.Duration(0) && o.Refresh >= o.Expire { msgs = append(msgs, fmt.Sprintf( "cookie_refresh (%q) must be less than cookie_expire (%q)", o.Refresh.String(), diff --git a/pkg/validation/cookie_test.go b/pkg/validation/cookie_test.go index b756daa8..1f0dc5cd 100644 --- a/pkg/validation/cookie_test.go +++ b/pkg/validation/cookie_test.go @@ -256,6 +256,21 @@ func TestValidateCookie(t *testing.T) { invalidSameSiteMsg, }, }, + { + name: "with session cookie configuration", + cookie: options.Cookie{ + Name: validName, + Secret: validSecret, + Domains: domains, + Path: "", + Expire: 0, + Refresh: 15 * time.Minute, + Secure: true, + HTTPOnly: false, + SameSite: "", + }, + errStrings: []string{}, + }, } for _, tc := range testCases {