diff --git a/CHANGELOG.md b/CHANGELOG.md index 03d6ccd7..548d200d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ - [#1906](https://github.com/oauth2-proxy/oauth2-proxy/pull/1906) Fix PKCE code verifier generation to never use UTF-8 characters - [#1839](https://github.com/oauth2-proxy/oauth2-proxy/pull/1839) Add readiness checks for deeper health checks (@kobim) - [#1927](https://github.com/oauth2-proxy/oauth2-proxy/pull/1927) Fix default scope settings for none oidc providers +- [#1713](https://github.com/oauth2-proxy/oauth2-proxy/pull/1713) Add session cookie support (@t-katsumura @tanuki884) - [#1951](https://github.com/oauth2-proxy/oauth2-proxy/pull/1951) Fix validate URL, check if query string marker (?) or separator (&) needs to be appended (@miguelborges99) - [#1920](https://github.com/oauth2-proxy/oauth2-proxy/pull/1920) Make sure emailClaim is not overriden if userIDClaim is not set - [#2010](https://github.com/oauth2-proxy/oauth2-proxy/pull/2010) Log the difference between invalid email and not authorized session diff --git a/docs/docs/configuration/overview.md b/docs/docs/configuration/overview.md index 98447864..b120337a 100644 --- a/docs/docs/configuration/overview.md +++ b/docs/docs/configuration/overview.md @@ -88,7 +88,7 @@ An example [oauth2-proxy.cfg](https://github.com/oauth2-proxy/oauth2-proxy/blob/ | `--code-challenge-method` | string | use PKCE code challenges with the specified method. Either 'plain' or 'S256' (recommended) | | | `--config` | string | path to config file | | | `--cookie-domain` | string \| list | Optional cookie domains to force cookies to (e.g. `.yourcompany.com`). The longest domain matching the request's host will be used (or the shortest cookie domain if there is no match). | | -| `--cookie-expire` | duration | expire timeframe for cookie | 168h0m0s | +| `--cookie-expire` | duration | expire timeframe for cookie. If set to 0, cookie becomes a session-cookie which will expire when the browser is closed. | 168h0m0s | | `--cookie-httponly` | bool | set HttpOnly cookie flag | true | | `--cookie-name` | string | the name of the cookie that the oauth_proxy creates. Should be changed to use a [cookie prefix](https://developer.mozilla.org/en-US/docs/Web/HTTP/Cookies#cookie_prefixes) (`__Host-` or `__Secure-`) if `--cookie-secure` is set. | `"_oauth2_proxy"` | | `--cookie-path` | string | an optional cookie path to force cookies to (e.g. `/poc/`) | `"/"` | diff --git a/go.mod b/go.mod index 23797ca4..2954b2b5 100644 --- a/go.mod +++ b/go.mod @@ -20,7 +20,7 @@ require ( github.com/oauth2-proxy/mockoidc v0.0.0-20220221072942-e3afe97dec43 github.com/oauth2-proxy/tools/reference-gen v0.0.0-20210118095127-56ffd7384404 github.com/onsi/ginkgo v1.16.5 - github.com/onsi/gomega v1.27.2 + github.com/onsi/gomega v1.27.6 github.com/pierrec/lz4/v4 v4.1.17 github.com/prometheus/client_golang v1.14.0 github.com/redis/go-redis/v9 v9.0.2 @@ -30,7 +30,7 @@ require ( github.com/stretchr/testify v1.8.1 github.com/vmihailenco/msgpack/v5 v5.3.5 golang.org/x/crypto v0.7.0 - golang.org/x/exp v0.0.0-20230307190834-24139beb5833 + golang.org/x/exp v0.0.0-20230425010034-47ecfdc1ba53 golang.org/x/net v0.8.0 golang.org/x/oauth2 v0.6.0 golang.org/x/sync v0.1.0 @@ -52,7 +52,7 @@ require ( github.com/go-jose/go-jose/v3 v3.0.0 // indirect github.com/go-logr/logr v1.2.3 // indirect github.com/golang/groupcache v0.0.0-20210331224755-41bb18bfe9da // indirect - github.com/golang/protobuf v1.5.2 // indirect + github.com/golang/protobuf v1.5.3 // indirect github.com/google/go-cmp v0.5.9 // indirect github.com/googleapis/enterprise-certificate-proxy v0.2.3 // indirect github.com/googleapis/gax-go/v2 v2.7.0 // indirect diff --git a/go.sum b/go.sum index dba34096..46b8bfc5 100644 --- a/go.sum +++ b/go.sum @@ -127,6 +127,7 @@ github.com/go-logr/logr v1.2.3/go.mod h1:jdQByPbusPIv2/zmleS9BjJVeZ6kBagPoEUsqbV github.com/go-stack/stack v1.8.0/go.mod h1:v0f6uXyyMGvRgIKkXu+yp6POWl0qKG85gN/melR3HDY= github.com/go-task/slim-sprig v0.0.0-20210107165309-348f09dbbbc0 h1:p104kn46Q8WdvHunIJ9dAyjPVtrBPhSr3KT2yUst43I= github.com/go-task/slim-sprig v0.0.0-20210107165309-348f09dbbbc0/go.mod h1:fyg7847qk6SyHyPtNmDHnmrv/HOrqktSC+C9fM+CJOE= +github.com/go-task/slim-sprig v0.0.0-20230315185526-52ccab3ef572 h1:tfuBGBXKqDEevZMzYi5KSi8KkcZtzBcTgAUUtapy0OI= github.com/gogo/protobuf v1.1.1/go.mod h1:r8qH/GZQm5c6nD/R0oafs1akxWv10x8SbQlK7atdtwQ= github.com/gogo/protobuf v1.2.1/go.mod h1:hp+jE20tsWTFYpLwKvXlhS1hjn+gTNwPg2I6zVXpSg4= github.com/golang-jwt/jwt v3.2.2+incompatible h1:IfV12K8xAKAnZqdXVzCZ+TOjboZ2keLg81eXfW3O+oY= @@ -162,6 +163,8 @@ github.com/golang/protobuf v1.4.3/go.mod h1:oDoupMAO8OvCJWAcko0GGGIgR6R6ocIYbsSw github.com/golang/protobuf v1.5.0/go.mod h1:FsONVRAS9T7sI+LIUmWTfcYkHO4aIWwzhcaSAoJOfIk= github.com/golang/protobuf v1.5.2 h1:ROPKBNFfQgOUMifHyP+KYbvpjbdoFNs+aK7DXlji0Tw= github.com/golang/protobuf v1.5.2/go.mod h1:XVQd3VNwM+JqD3oG2Ue2ip4fOMUkwXdXDdiuN0vRsmY= +github.com/golang/protobuf v1.5.3 h1:KhyjKVUg7Usr/dYsdSqoFveMYd5ko72D+zANwlG1mmg= +github.com/golang/protobuf v1.5.3/go.mod h1:XVQd3VNwM+JqD3oG2Ue2ip4fOMUkwXdXDdiuN0vRsmY= github.com/gomodule/redigo v1.7.1-0.20190322064113-39e2c31b7ca3 h1:6amM4HsNPOvMLVc2ZnyqrjeQ92YAVWn7T4WBKK87inY= github.com/gomodule/redigo v1.7.1-0.20190322064113-39e2c31b7ca3/go.mod h1:B4C85qUVwatsJoIUNIfCRsp7qO0iAmpGFZ4EELWSbC4= github.com/google/btree v0.0.0-20180813153112-4030bb1f1f0c/go.mod h1:lNA+9X1NB3Zf8V7Ke586lFgjr2dZNuvo3lPJSGZ5JPQ= @@ -272,10 +275,13 @@ github.com/onsi/ginkgo v1.12.1/go.mod h1:zj2OWP4+oCPe1qIXoGWkgMRwljMUYCdkwsT2108 github.com/onsi/ginkgo v1.16.5 h1:8xi0RTUf59SOSfEtZMvwTvXYMzG4gV23XVHOZiXNtnE= github.com/onsi/ginkgo v1.16.5/go.mod h1:+E8gABHa3K6zRBolWtd+ROzc/U5bkGt0FwiG042wbpU= github.com/onsi/ginkgo/v2 v2.8.4 h1:gf5mIQ8cLFieruNLAdgijHF1PYfLphKm2dxxcUtcqK0= +github.com/onsi/ginkgo/v2 v2.9.2 h1:BA2GMJOtfGAfagzYtrAlufIP0lq6QERkFmHLMLPwFSU= github.com/onsi/gomega v1.7.1/go.mod h1:XdKZgCCFLUoM/7CFJVPcG8C1xQ1AJ0vpAezJrB7JYyY= github.com/onsi/gomega v1.10.1/go.mod h1:iN09h71vgCQne3DLsj+A5owkum+a2tYe+TOCB1ybHNo= github.com/onsi/gomega v1.27.2 h1:SKU0CXeKE/WVgIV1T61kSa3+IRE8Ekrv9rdXDwwTqnY= github.com/onsi/gomega v1.27.2/go.mod h1:5mR3phAHpkAVIDkHEUBY6HGVsU+cpcEscrGPB4oPlZI= +github.com/onsi/gomega v1.27.6 h1:ENqfyGeS5AX/rlXDd/ETokDz93u0YufY1Pgxuy/PvWE= +github.com/onsi/gomega v1.27.6/go.mod h1:PIQNjfQwkP3aQAH7lf7j87O/5FiNr+ZR8+ipb+qQlhg= github.com/pelletier/go-toml v1.2.0 h1:T5zMGML61Wp+FlcbWjRDT7yAxhJNAiPPLOFECq181zc= github.com/pelletier/go-toml v1.2.0/go.mod h1:5z9KED0ma1S8pY6P1sdut58dfprrGBbd/94hg7ilaic= github.com/pelletier/go-toml/v2 v2.0.7 h1:muncTPStnKRos5dpVKULv2FVd4bMOhNePj9CjgDb8Us= @@ -403,6 +409,8 @@ golang.org/x/exp v0.0.0-20200207192155-f17229e696bd/go.mod h1:J/WKrq2StrnmMY6+EH golang.org/x/exp v0.0.0-20200224162631-6cc2880d07d6/go.mod h1:3jZMyOhIsHpP37uCMkUooju7aAi5cS1Q23tOzKc+0MU= golang.org/x/exp v0.0.0-20230307190834-24139beb5833 h1:SChBja7BCQewoTAU7IgvucQKMIXrEpFxNMs0spT3/5s= golang.org/x/exp v0.0.0-20230307190834-24139beb5833/go.mod h1:CxIveKay+FTh1D0yPZemJVgC/95VzuuOLq5Qi4xnoYc= +golang.org/x/exp v0.0.0-20230425010034-47ecfdc1ba53 h1:5llv2sWeaMSnA3w2kS57ouQQ4pudlXrR0dCgw51QK9o= +golang.org/x/exp v0.0.0-20230425010034-47ecfdc1ba53/go.mod h1:V1LtkGg67GoY2N1AnLN78QLrzxkLyJw7RJb1gzOOz9w= golang.org/x/image v0.0.0-20190227222117-0694c2d4d067/go.mod h1:kZ7UVZpmo3dzQBMxlp+ypCbDeSB+sBbTgSJuh5dn5js= golang.org/x/image v0.0.0-20190802002840-cff245a6509b/go.mod h1:FeLwcggjj3mMvU+oOTbSwawSJRM1uh48EjtB4UJZlP0= golang.org/x/lint v0.0.0-20181026193005-c67002cb31c3/go.mod h1:UVdnD1Gm6xHRNCYTkRU2/jEulfH38KcIWyp/GAMgvoE= @@ -616,6 +624,7 @@ golang.org/x/tools v0.0.0-20210108195828-e2f9c7f1fc8e/go.mod h1:emZCQorbCU4vsT4f golang.org/x/tools v0.1.0/go.mod h1:xkSsbof2nBLbhDlRMhhhyNLN/zl3eTqcnHD5viDpcZ0= golang.org/x/tools v0.1.12/go.mod h1:hNGJHUnrk76NpqgfD5Aqm5Crs+Hm0VOH/i9J2+nxYbc= golang.org/x/tools v0.6.0 h1:BOw41kyTf3PuCW1pVQf8+Cyg8pMlkYB1oo9iJ6D/lKM= +golang.org/x/tools v0.7.0 h1:W4OVu8VVOaIO0yzWMNdepAulS7YfoS3Zabrm8DOXXU4= golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= diff --git a/pkg/cookies/cookies.go b/pkg/cookies/cookies.go index dc1bff76..1e017366 100644 --- a/pkg/cookies/cookies.go +++ b/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 diff --git a/pkg/cookies/cookies_test.go b/pkg/cookies/cookies_test.go index 7f6c0319..c01617a1 100644 --- a/pkg/cookies/cookies_test.go +++ b/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, + }), + ) + }) }) diff --git a/pkg/cookies/csrf_test.go b/pkg/cookies/csrf_test.go index c4a461f0..97c1b496 100644 --- a/pkg/cookies/csrf_test.go +++ b/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 diff --git a/pkg/encryption/utils.go b/pkg/encryption/utils.go index aec52532..426a3131 100644 --- a/pkg/encryption/utils.go +++ b/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 { diff --git a/pkg/encryption/utils_test.go b/pkg/encryption/utils_test.go index cc341ec4..9e69df84 100644 --- a/pkg/encryption/utils_test.go +++ b/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) 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 {