Merge pull request #24 from philips-forks/sessioncookiepr

creating oauth proxy cookie as session cookie

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

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,

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,
+			}),
+		)
+	})
+})

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

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)
+}

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(),

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 {