3155ada · typeable/oauth2-proxy

git.iko.soy/typeable/oauth2-proxy

on master help sign in

Ensure sessions are refreshable in redis session store

3155adaJoel Speed7 years ago

pkg/sessions/redis/redis_store.go+2−3

compare with master

unifiedsplit

@@ -82,7 +82,7 @@ func (store *SessionStore) Save(rw http.ResponseWriter, req *http.Request, s *se

82 if err != nil {

83 return err

84 }

85− ticketString, err := store.storeValue(value, s.ExpiresOn, requestCookie)

85+ ticketString, err := store.storeValue(value, store.CookieOptions.CookieExpire, requestCookie)

86 if err != nil {

87 return err

88 }

@@ -191,7 +191,7 @@ func (store *SessionStore) makeCookie(req *http.Request, value string, expires t

191 )

192}

193

194−func (store *SessionStore) storeValue(value string, expiresOn time.Time, requestCookie *http.Cookie) (string, error) {

194+func (store *SessionStore) storeValue(value string, expiration time.Duration, requestCookie *http.Cookie) (string, error) {

195 var ticket *TicketData

196 if requestCookie != nil {

197 var err error

@@ -225,7 +225,6 @@ func (store *SessionStore) storeValue(value string, expiresOn time.Time, request

225 stream.XORKeyStream(ciphertext, []byte(value))

226

227 handle := ticket.asHandle(store.CookieOptions.CookieName)

228− expiration := expiresOn.Sub(time.Now())

229 err = store.Client.Set(handle, ciphertext, expiration).Err()

228 err = store.Client.Set(handle, ciphertext, expiration).Err()

230 if err != nil {

229 if err != nil {

231 return "", err

230 return "", err

pkg/sessions/session_store_test.go+75−27

@@ -35,6 +35,7 @@ var _ = Describe("NewSessionStore", func() {

35 var response *httptest.ResponseRecorder

36 var session *sessionsapi.SessionState

37 var ss sessionsapi.SessionStore

38+ var mr *miniredis.Miniredis

39 CheckCookieOptions := func() {

40 CheckCookieOptions := func() {

40 Context("the cookies returned", func() {

41 Context("the cookies returned", func() {

@@ -203,7 +204,38 @@ var _ = Describe("NewSessionStore", func() {

203 })

204 })

204

205

205 Context("when Load is called", func() {

206 Context("when Load is called", func() {

207+ LoadSessionTests := func() {

206− var loadedSession *sessionsapi.SessionState

208+ var loadedSession *sessionsapi.SessionState

209+ BeforeEach(func() {

210+ var err error

211+ loadedSession, err = ss.Load(request)

212+ Expect(err).ToNot(HaveOccurred())

213+ })

214+

215+ It("loads a session equal to the original session", func() {

216+ if cookieOpts.CookieSecret == "" {

217+ // Only Email and User stored in session when encrypted

218+ Expect(loadedSession.Email).To(Equal(session.Email))

219+ Expect(loadedSession.User).To(Equal(session.User))

220+ } else {

221+ // All fields stored in session if encrypted

222+

223+ // Can't compare time.Time using Equal() so remove ExpiresOn from sessions

224+ l := *loadedSession

225+ l.CreatedAt = time.Time{}

226+ l.ExpiresOn = time.Time{}

227+ s := *session

228+ s.CreatedAt = time.Time{}

229+ s.ExpiresOn = time.Time{}

230+ Expect(l).To(Equal(s))

231+

232+ // Compare time.Time separately

233+ Expect(loadedSession.CreatedAt.Equal(session.CreatedAt)).To(BeTrue())

234+ Expect(loadedSession.ExpiresOn.Equal(session.ExpiresOn)).To(BeTrue())

235+ }

236+ })

237+ }

238+

207 BeforeEach(func() {

239 BeforeEach(func() {

208 req := httptest.NewRequest("GET", "http://example.com/", nil)

240 req := httptest.NewRequest("GET", "http://example.com/", nil)

209 resp := httptest.NewRecorder()

241 resp := httptest.NewRecorder()

@@ -213,32 +245,49 @@ var _ = Describe("NewSessionStore", func() {

213 for _, cookie := range resp.Result().Cookies() {

245 for _, cookie := range resp.Result().Cookies() {

214 request.AddCookie(cookie)

246 request.AddCookie(cookie)

215 }

247 }

216− loadedSession, err = ss.Load(request)

217− Expect(err).ToNot(HaveOccurred())

218 })

248 })

219

249

220− It("loads a session equal to the original session", func() {

250+ Context("before the refresh period", func() {

221− if cookieOpts.CookieSecret == "" {

251+ LoadSessionTests()

222− // Only Email and User stored in session when encrypted

223− Expect(loadedSession.Email).To(Equal(session.Email))

224− Expect(loadedSession.User).To(Equal(session.User))

225− } else {

226− // All fields stored in session if encrypted

227−

228− // Can't compare time.Time using Equal() so remove ExpiresOn from sessions

229− l := *loadedSession

230− l.CreatedAt = time.Time{}

231− l.ExpiresOn = time.Time{}

232− s := *session

233− s.CreatedAt = time.Time{}

234− s.ExpiresOn = time.Time{}

235− Expect(l).To(Equal(s))

236−

237− // Compare time.Time separately

238− Expect(loadedSession.CreatedAt.Equal(session.CreatedAt)).To(BeTrue())

239− Expect(loadedSession.ExpiresOn.Equal(session.ExpiresOn)).To(BeTrue())

240− }

241 })

252 })

253+

254+ // Test TTLs and cleanup of persistent session storage

255+ // For non-persistent we rely on the browser cookie lifecycle

256+ if persistent {

257+ Context("after the refresh period, but before the cookie expire period", func() {

258+ BeforeEach(func() {

259+ switch ss.(type) {

260+ case *redis.SessionStore:

261+ mr.FastForward(cookieOpts.CookieRefresh + time.Minute)

262+ }

263+ })

264+

265+ LoadSessionTests()

266+ })

267+

268+ Context("after the cookie expire period", func() {

269+ var loadedSession *sessionsapi.SessionState

270+ var err error

271+

272+ BeforeEach(func() {

273+ switch ss.(type) {

274+ case *redis.SessionStore:

275+ mr.FastForward(cookieOpts.CookieExpire + time.Minute)

276+ }

277+

278+ loadedSession, err = ss.Load(request)

279+ Expect(err).To(HaveOccurred())

280+ })

281+

282+ It("returns an error loading the session", func() {

283+ Expect(err).To(HaveOccurred())

284+ })

285+

286+ It("returns an empty session", func() {

287+ Expect(loadedSession).To(BeNil())

288+ })

289+ })

290+ }

242 })

291 })

243

292

244 if persistent {

293 if persistent {

@@ -263,7 +312,7 @@ var _ = Describe("NewSessionStore", func() {

263 CookieName: "_cookie_name",

312 CookieName: "_cookie_name",

264 CookiePath: "/path",

313 CookiePath: "/path",

265 CookieExpire: time.Duration(72) * time.Hour,

314 CookieExpire: time.Duration(72) * time.Hour,

266− CookieRefresh: time.Duration(3600),

315+ CookieRefresh: time.Duration(2) * time.Hour,

267 CookieSecure: false,

316 CookieSecure: false,

268 CookieHTTPOnly: false,

317 CookieHTTPOnly: false,

269 CookieDomain: "example.com",

318 CookieDomain: "example.com",

@@ -305,7 +354,7 @@ var _ = Describe("NewSessionStore", func() {

305 CookieName: "_oauth2_proxy",

354 CookieName: "_oauth2_proxy",

306 CookiePath: "/",

355 CookiePath: "/",

307 CookieExpire: time.Duration(168) * time.Hour,

356 CookieExpire: time.Duration(168) * time.Hour,

308− CookieRefresh: time.Duration(0),

357+ CookieRefresh: time.Duration(1) * time.Hour,

309 CookieSecure: true,

358 CookieSecure: true,

310 CookieHTTPOnly: true,

359 CookieHTTPOnly: true,

311 }

360 }

@@ -340,7 +389,6 @@ var _ = Describe("NewSessionStore", func() {

340 })

389 })

341

390

342 Context("with type 'redis'", func() {

391 Context("with type 'redis'", func() {

343− var mr *miniredis.Miniredis

344 BeforeEach(func() {

392 BeforeEach(func() {

345 var err error

393 var err error

346 mr, err = miniredis.Run()

394 mr, err = miniredis.Run()