Session-Cookie Support (#1713)

* Create session cookie when cookie-expire set 0

* Fix format

* add test

* fix lint error

* fix test code

* fix conflicted test case

* update test case of cookie expiration

* update tests of csrf cookies

* update docs

* Update docs/docs/configuration/overview.md

Co-authored-by: Joel Speed <Joel.speed@hotmail.co.uk>

---------

Co-authored-by: tanuki884 <morkazuk@fsi.co.jp>
Co-authored-by: Joel Speed <Joel.speed@hotmail.co.uk>

pkg/cookies/cookies.go

@@ -30,12 +30,15 @@ func MakeCookieFromOptions(req *http.Request, name string, value string, opts *o
 		Value:    value,
 		Path:     opts.Path,
 		Domain:   domain,
-		Expires:  now.Add(expiration),
 		HttpOnly: opts.HTTPOnly,
 		Secure:   opts.Secure,
 		SameSite: ParseSameSite(opts.SameSite),
 	}
 
+	if expiration != time.Duration(0) {
+		c.Expires = now.Add(expiration)
+	}
+
 	warnInvalidDomain(c, req)
 
 	return c

pkg/cookies/cookies_test.go

@@ -3,6 +3,9 @@ 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"
@@ -76,4 +79,74 @@ var _ = Describe("Cookie Tests", func() {
 			}),
 		)
 	})
+
+	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 cookies with or without expiration",
+			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/cookies/csrf_test.go

@@ -31,6 +31,7 @@ var _ = Describe("CSRF Cookie Tests", func() {
 			Secure:         true,
 			HTTPOnly:       true,
 			CSRFPerRequest: false,
+			CSRFExpire:     time.Hour,
 		}
 
 		var err error

pkg/encryption/utils.go

@@ -58,7 +58,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)) {
+		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 {

pkg/encryption/utils_test.go

@@ -7,8 +7,11 @@ import (
 	"encoding/base64"
 	"fmt"
 	"io"
+	"net/http"
+	"strconv"
 	"strings"
 	"testing"
+	"time"
 	"unicode"
 
 	"github.com/stretchr/testify/assert"
@@ -103,6 +106,30 @@ func TestSignAndValidate(t *testing.T) {
 	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)
+}
+
 func TestGenerateRandomASCIIString(t *testing.T) {
 	randomString, err := GenerateRandomASCIIString(96)
 	assert.NoError(t, err)

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 {