a25fef7 · typeable/oauth2-proxy

git.iko.soy/typeable/oauth2-proxy

Merge pull request #2697 from matpen-wi/feat/max-age-instead-of-expires

pkg/cookies: use 'Max-Age' instead of 'Expires' for cookie expiration

commita25fef7cbf9bcff62e0985547d32013a67674ecf authorJoel Speed <Joel.speed@hotmail.co.uk> dateMon 24 Feb 2025, 18:27 (2 years ago) parent80189a9 a393ea3

10 files changed+40−32

compare with master

unifiedsplit

CHANGELOG.md+1 pkg/cookies/cookies.go+5−3 pkg/cookies/cookies_suite_test.go−7 pkg/cookies/cookies_test.go+24−5 pkg/cookies/csrf.go−2 pkg/cookies/csrf_per_request_test.go+3−4 pkg/cookies/csrf_test.go+3−4 pkg/sessions/cookie/session_store.go+3−4 pkg/sessions/persistence/ticket.go−2 pkg/sessions/tests/session_store_tests.go+1−1

CHANGELOG.md+1

@@ -9,6 +9,7 @@

99## Changes since v7.8.1

1010

1111- [#2927](https://github.com/oauth2-proxy/oauth2-proxy/pull/2927) chore(deps/build): bump golang to 1.23 and use go.mod as single point of truth for all build files (@tuunit)

12+- [#2697](https://github.com/oauth2-proxy/oauth2-proxy/pull/2697) Use `Max-Age` instead of `Expires` for cookie expiration (@matpen-wi)

1213

1314# V7.8.1

1415

pkg/cookies/cookies.go+5−3

@@ -14,7 +14,7 @@ import (

1414

1515// MakeCookieFromOptions constructs a cookie based on the given *options.CookieOptions,

1616// value and creation time

17−func MakeCookieFromOptions(req *http.Request, name string, value string, opts *options.Cookie, expiration time.Duration, now time.Time) *http.Cookie {

17+func MakeCookieFromOptions(req *http.Request, name string, value string, opts *options.Cookie, expiration time.Duration) *http.Cookie {

1818 domain := GetCookieDomain(req, opts.Domains)

1919 // If nothing matches, create the cookie with the shortest domain

2020 if domain == "" && len(opts.Domains) > 0 {

@@ -35,8 +35,10 @@ func MakeCookieFromOptions(req *http.Request, name string, value string, opts *o

3535 SameSite: ParseSameSite(opts.SameSite),

3636 }

3737

38− if expiration != time.Duration(0) {

39− c.Expires = now.Add(expiration)

38+ if expiration > time.Duration(0) {

39+ c.MaxAge = int(expiration.Seconds())

40+ } else if expiration < time.Duration(0) {

41+ c.MaxAge = -1

4042 }

4143

4244 warnInvalidDomain(c, req)

pkg/cookies/cookies_suite_test.go−7

@@ -1,9 +1,7 @@

11package cookies

33import (

4− "net/http"

54 "testing"

6− "time"

86 "github.com/oauth2-proxy/oauth2-proxy/v7/pkg/logger"

97 . "github.com/onsi/ginkgo/v2"

@@ -28,8 +26,3 @@ func TestProviderSuite(t *testing.T) {

2826 RegisterFailHandler(Fail)

2927 RunSpecs(t, "Cookies")

3028}

31−

32−func testCookieExpires(exp time.Time) string {

33− var buf [len(http.TimeFormat)]byte

34− return string(exp.UTC().AppendFormat(buf[:0], http.TimeFormat))

35−}

pkg/cookies/cookies_test.go+24−5

@@ -87,7 +87,7 @@ var _ = Describe("Cookie Tests", func() {

8787 opts options.Cookie

8888 expiration time.Duration

8989 now time.Time

90− expectedOutput time.Time

90+ expectedOutput int

9191 }

9292

9393 validName := "_oauth2_proxy"

@@ -95,7 +95,7 @@ var _ = Describe("Cookie Tests", func() {

9595 domains := []string{"www.cookies.test"}

9696

9797 now := time.Now()

98− var expectedExpires time.Time

98+ var expectedMaxAge int

9999

100100 DescribeTable("should return cookies with or without expiration",

101101 func(in makeCookieFromOptionsTableInput) {

@@ -106,7 +106,7 @@ var _ = Describe("Cookie Tests", func() {

106106 )

107107 Expect(err).ToNot(HaveOccurred())

108108

109− Expect(MakeCookieFromOptions(req, in.name, in.value, &in.opts, in.expiration, in.now).Expires).To(Equal(in.expectedOutput))

109+ Expect(MakeCookieFromOptions(req, in.name, in.value, &in.opts, in.expiration).MaxAge).To(Equal(in.expectedOutput))

110110 },

111111 Entry("persistent cookie", makeCookieFromOptionsTableInput{

112112 host: "www.cookies.test",

@@ -125,7 +125,26 @@ var _ = Describe("Cookie Tests", func() {

125125 },

126126 expiration: 15 * time.Minute,

127127 now: now,

128− expectedOutput: now.Add(15 * time.Minute),

128+ expectedOutput: int((15 * time.Minute).Seconds()),

129+ }),

130+ Entry("persistent cookie to be cleared", makeCookieFromOptionsTableInput{

131+ host: "www.cookies.test",

132+ name: validName,

133+ value: "1",

134+ opts: options.Cookie{

135+ Name: validName,

136+ Secret: validSecret,

137+ Domains: domains,

138+ Path: "",

139+ Expire: time.Hour * -1,

140+ Refresh: 15 * time.Minute,

141+ Secure: true,

142+ HTTPOnly: false,

143+ SameSite: "",

144+ },

145+ expiration: time.Hour * -1,

146+ now: now,

147+ expectedOutput: -1,

129148 }),

130149 Entry("session cookie", makeCookieFromOptionsTableInput{

131150 host: "www.cookies.test",

@@ -144,7 +163,7 @@ var _ = Describe("Cookie Tests", func() {

144163 },

145164 expiration: 0,

146165 now: now,

147− expectedOutput: expectedExpires,

166+ expectedOutput: expectedMaxAge,

148167 }),

149168 )

150169 })

pkg/cookies/csrf.go−2

@@ -145,7 +145,6 @@ func (c *csrf) SetCookie(rw http.ResponseWriter, req *http.Request) (*http.Cooki

145145 encoded,

146146 c.cookieOpts,

147147 c.cookieOpts.CSRFExpire,

148− c.time.Now(),

149148 )

150149 http.SetCookie(rw, cookie)

151150

@@ -160,7 +159,6 @@ func (c *csrf) ClearCookie(rw http.ResponseWriter, req *http.Request) {

160159 "",

161160 c.cookieOpts,

162161 time.Hour*-1,

163− c.time.Now(),

164162 ))

165163}

166164

pkg/cookies/csrf_per_request_test.go+3−4

@@ -159,10 +159,10 @@ var _ = Describe("CSRF Cookie with non-fixed name Tests", func() {

159159 ))

160160 Expect(rw.Header().Get("Set-Cookie")).To(ContainSubstring(

161161 fmt.Sprintf(

162− "; Path=%s; Domain=%s; Expires=%s; HttpOnly; Secure",

162+ "; Path=%s; Domain=%s; Max-Age=%d; HttpOnly; Secure",

163163 cookiePath,

164164 cookieDomain,

165− testCookieExpires(testNow.Add(cookieOpts.CSRFExpire)),

165+ int(cookieOpts.CSRFExpire.Seconds()),

166166 ),

167167 ))

168168 })

@@ -176,11 +176,10 @@ var _ = Describe("CSRF Cookie with non-fixed name Tests", func() {

176176

177177 Expect(rw.Header().Get("Set-Cookie")).To(Equal(

178178 fmt.Sprintf(

179− "%s=; Path=%s; Domain=%s; Expires=%s; HttpOnly; Secure",

179+ "%s=; Path=%s; Domain=%s; Max-Age=0; HttpOnly; Secure",

180180 privateCSRF.cookieName(),

181181 cookiePath,

182182 cookieDomain,

183− testCookieExpires(testNow.Add(time.Hour*-1)),

184183 ),

185184 ))

186185 })

pkg/cookies/csrf_test.go+3−4

@@ -161,10 +161,10 @@ var _ = Describe("CSRF Cookie Tests", func() {

161161 ))

162162 Expect(rw.Header().Get("Set-Cookie")).To(ContainSubstring(

163163 fmt.Sprintf(

164− "; Path=%s; Domain=%s; Expires=%s; HttpOnly; Secure",

164+ "; Path=%s; Domain=%s; Max-Age=%d; HttpOnly; Secure",

165165 cookiePath,

166166 cookieDomain,

167− testCookieExpires(testNow.Add(cookieOpts.CSRFExpire)),

167+ int(cookieOpts.CSRFExpire.Seconds()),

168168 ),

169169 ))

170170 })

@@ -239,11 +239,10 @@ var _ = Describe("CSRF Cookie Tests", func() {

239239

240240 Expect(rw.Header().Get("Set-Cookie")).To(Equal(

241241 fmt.Sprintf(

242− "%s=; Path=%s; Domain=%s; Expires=%s; HttpOnly; Secure",

242+ "%s=; Path=%s; Domain=%s; Max-Age=0; HttpOnly; Secure",

243243 privateCSRF.cookieName(),

244244 cookiePath,

245245 cookieDomain,

246− testCookieExpires(testNow.Add(time.Hour*-1)),

247246 ),

248247 ))

249248 })

pkg/sessions/cookie/session_store.go+3−4

@@ -74,7 +74,7 @@ func (s *SessionStore) Clear(rw http.ResponseWriter, req *http.Request) error {

7474

7575 for _, c := range req.Cookies() {

7676 if cookieNameRegex.MatchString(c.Name) {

77− clearCookie := s.makeCookie(req, c.Name, "", time.Hour*-1, time.Now())

77+ clearCookie := s.makeCookie(req, c.Name, "", time.Hour*-1)

7878

7979 http.SetCookie(rw, clearCookie)

8080 }

@@ -126,21 +126,20 @@ func (s *SessionStore) makeSessionCookie(req *http.Request, value []byte, now ti

126126 return nil, err

127127 }

128128 }

129− c := s.makeCookie(req, s.Cookie.Name, strValue, s.Cookie.Expire, now)

129+ c := s.makeCookie(req, s.Cookie.Name, strValue, s.Cookie.Expire)

130130 if len(c.String()) > maxCookieLength {

131131 return splitCookie(c), nil

132132 }

133133 return []*http.Cookie{c}, nil

134134}

135135

136−func (s *SessionStore) makeCookie(req *http.Request, name string, value string, expiration time.Duration, now time.Time) *http.Cookie {

136+func (s *SessionStore) makeCookie(req *http.Request, name string, value string, expiration time.Duration) *http.Cookie {

137137 return pkgcookies.MakeCookieFromOptions(

138138 req,

139139 name,

140140 value,

141141 s.Cookie,

142142 expiration,

143− now,

144143 )

145144}

146145

pkg/sessions/persistence/ticket.go−2

@@ -223,7 +223,6 @@ func (t *ticket) clearCookie(rw http.ResponseWriter, req *http.Request) {

223223 "",

224224 t.options,

225225 time.Hour*-1,

226− time.Now(),

227226 ))

228227}

229228

@@ -242,7 +241,6 @@ func (t *ticket) makeCookie(req *http.Request, value string, expires time.Durati

242241 value,

243242 t.options,

244243 expires,

245− now,

246244 ), nil

247245}

248246

pkg/sessions/tests/session_store_tests.go+1−1

@@ -385,7 +385,7 @@ func SessionStoreInterfaceTests(in *testInput) {

385385 broken := "BrokenSessionFromADifferentSessionImplementation"

386386 value, err := encryption.SignedValue(in.cookieOpts.Secret, in.cookieOpts.Name, []byte(broken), time.Now())

387387 Expect(err).ToNot(HaveOccurred())

388− cookie := cookiesapi.MakeCookieFromOptions(in.request, in.cookieOpts.Name, value, in.cookieOpts, in.cookieOpts.Expire, time.Now())

388+ cookie := cookiesapi.MakeCookieFromOptions(in.request, in.cookieOpts.Name, value, in.cookieOpts, in.cookieOpts.Expire)

389389 in.request.AddCookie(cookie)

390390

391391 err = in.ss().Save(in.response, in.request, in.session)