a25fef7 · typeable/oauth2-proxy

CHANGELOG.md+1

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

9## Changes since v7.8.1

10

11- [#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)

12

13

13# V7.8.1

14# V7.8.1

14

15

pkg/cookies/cookies.go+5−3

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

14

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

16// 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 {

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

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

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

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

35 SameSite: ParseSameSite(opts.SameSite),

36 }

37

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

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

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

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

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

41+ c.MaxAge = -1

40 }

42 }

41

43

42 warnInvalidDomain(c, req)

44 warnInvalidDomain(c, req)

pkg/cookies/cookies_suite_test.go−7

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

1package cookies

2

3import (

4− "net/http"

5 "testing"

4 "testing"

6− "time"

7

5

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

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

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

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

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

28 RegisterFailHandler(Fail)

26 RegisterFailHandler(Fail)

29 RunSpecs(t, "Cookies")

27 RunSpecs(t, "Cookies")

30}

28}

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

87 opts options.Cookie

88 expiration time.Duration

89 now time.Time

90− expectedOutput time.Time

90+ expectedOutput int

91 }

92

93 validName := "_oauth2_proxy"

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

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

96

97 now := time.Now()

98− var expectedExpires time.Time

98+ var expectedMaxAge int

99

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

101 func(in makeCookieFromOptionsTableInput) {

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

106 )

107 Expect(err).ToNot(HaveOccurred())

108

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

110 },

111 Entry("persistent cookie", makeCookieFromOptionsTableInput{

112 host: "www.cookies.test",

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

125 },

126 expiration: 15 * time.Minute,

127 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,

129 }),

148 }),

130 Entry("session cookie", makeCookieFromOptionsTableInput{

149 Entry("session cookie", makeCookieFromOptionsTableInput{

131 host: "www.cookies.test",

150 host: "www.cookies.test",

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

144 },

163 },

145 expiration: 0,

164 expiration: 0,

146 now: now,

165 now: now,

147− expectedOutput: expectedExpires,

166+ expectedOutput: expectedMaxAge,

148 }),

167 }),

149 )

168 )

150 })

169 })

pkg/cookies/csrf.go−2

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

145 encoded,

146 c.cookieOpts,

147 c.cookieOpts.CSRFExpire,

148− c.time.Now(),

149 )

148 )

150 http.SetCookie(rw, cookie)

149 http.SetCookie(rw, cookie)

151

150

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

160 "",

159 "",

161 c.cookieOpts,

160 c.cookieOpts,

162 time.Hour*-1,

161 time.Hour*-1,

163− c.time.Now(),

164 ))

162 ))

165}

163}

166

164

pkg/cookies/csrf_per_request_test.go+3−4

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

159 ))

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

161 fmt.Sprintf(

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

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

163 cookiePath,

164 cookieDomain,

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

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

166 ),

167 ))

168 })

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

176

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

178 fmt.Sprintf(

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

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

180 privateCSRF.cookieName(),

181 cookiePath,

182 cookieDomain,

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

184 ),

183 ),

185 ))

184 ))

186 })

185 })

pkg/cookies/csrf_test.go+3−4

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

161 ))

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

163 fmt.Sprintf(

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

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

165 cookiePath,

166 cookieDomain,

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

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

168 ),

169 ))

170 })

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

239

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

241 fmt.Sprintf(

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

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

243 privateCSRF.cookieName(),

244 cookiePath,

245 cookieDomain,

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

247 ),

246 ),

248 ))

247 ))

249 })

248 })

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

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

74

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

76 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)

78

79 http.SetCookie(rw, clearCookie)

80 }

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

126 return nil, err

127 }

128 }

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)

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

131 return splitCookie(c), nil

132 }

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

134}

135

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 {

137 return pkgcookies.MakeCookieFromOptions(

138 req,

139 name,

140 value,

141 s.Cookie,

142 expiration,

143− now,

144 )

143 )

145}

144}

146

145

pkg/sessions/persistence/ticket.go−2

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

223 "",

224 t.options,

225 time.Hour*-1,

226− time.Now(),

227 ))

226 ))

228}

227}

229

228

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

242 value,

241 value,

243 t.options,

242 t.options,

244 expires,

243 expires,

245− now,

246 ), nil

244 ), nil

247}

245}

248

246

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

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

385 broken := "BrokenSessionFromADifferentSessionImplementation"

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

387 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)

389 in.request.AddCookie(cookie)

390

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