Skip to content

Commit afa45ff

Browse files
committed
Merge pull request #1 from 18F/cookie_renew_115
Reproduce cookie refresh bugs, validate fixes
2 parents 3ff03ca + 21f67c7 commit afa45ff

File tree

3 files changed

+36
-1
lines changed

3 files changed

+36
-1
lines changed

cookies.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,10 @@ func validateCookie(cookie *http.Cookie, seed string, expiration time.Duration)
2727
if err != nil {
2828
return
2929
}
30+
// The expiration timestamp set when the cookie was created
31+
// isn't sent back by the browser. Hence, we check whether the
32+
// creation timestamp stored in the cookie falls within the
33+
// window defined by (Now()-expiration, Now()].
3034
t = time.Unix(int64(ts), 0)
3135
if t.After(time.Now().Add(expiration*-1)) && t.Before(time.Now().Add(time.Minute*5)) {
3236
// it's a valid cookie. now get the contents

oauthproxy.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -239,7 +239,7 @@ func (p *OauthProxy) ProcessCookie(rw http.ResponseWriter, req *http.Request) (e
239239
if err != nil {
240240
log.Printf(err.Error())
241241
ok = false
242-
} else if ok && p.CookieRefresh != time.Duration(0) && value != "" {
242+
} else if ok && p.CookieRefresh != time.Duration(0) {
243243
refresh := timestamp.Add(p.CookieRefresh)
244244
if refresh.Before(time.Now()) {
245245
ok = p.Validator(email) && p.provider.ValidateToken(access_token)

oauthproxy_test.go

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -457,6 +457,37 @@ func TestProcessCookieRefreshThresholdNotCrossed(t *testing.T) {
457457
assert.Equal(t, []string(nil), pc_test.rw.HeaderMap["Set-Cookie"])
458458
}
459459

460+
func TestProcessCookieFailIfCookieExpired(t *testing.T) {
461+
pc_test := NewProcessCookieTestWithDefaults()
462+
pc_test.proxy.CookieExpire = time.Duration(24) * time.Hour
463+
reference := time.Now().Add(time.Duration(25) * time.Hour * -1)
464+
cookie := pc_test.MakeCookie("[email protected]", "my_access_token", reference)
465+
pc_test.req.AddCookie(cookie)
466+
467+
if _, _, _, ok := pc_test.ProcessCookie(); ok {
468+
t.Error("ProcessCookie() should have failed")
469+
}
470+
if set_cookie := pc_test.rw.HeaderMap["Set-Cookie"]; set_cookie != nil {
471+
t.Error("expected Set-Cookie to be nil, instead was: ", set_cookie)
472+
}
473+
}
474+
475+
func TestProcessCookieFailIfRefreshSetAndCookieExpired(t *testing.T) {
476+
pc_test := NewProcessCookieTestWithDefaults()
477+
pc_test.proxy.CookieExpire = time.Duration(24) * time.Hour
478+
reference := time.Now().Add(time.Duration(25) * time.Hour * -1)
479+
cookie := pc_test.MakeCookie("[email protected]", "my_access_token", reference)
480+
pc_test.req.AddCookie(cookie)
481+
482+
pc_test.proxy.CookieRefresh = time.Hour
483+
if _, _, _, ok := pc_test.ProcessCookie(); ok {
484+
t.Error("ProcessCookie() should have failed")
485+
}
486+
if set_cookie := pc_test.rw.HeaderMap["Set-Cookie"]; set_cookie != nil {
487+
t.Error("expected Set-Cookie to be nil, instead was: ", set_cookie)
488+
}
489+
}
490+
460491
func TestProcessCookieFailIfRefreshSetAndTokenNoLongerValid(t *testing.T) {
461492
pc_test := NewProcessCookieTest(ProcessCookieTestOpts{
462493
provider_validate_cookie_response: false,

0 commit comments

Comments
 (0)