fix cookie refresh code

contrib/oauth2_proxy.cfg.example

@@ -54,16 +54,17 @@
 # custom_templates_dir = ""
 
 ## Cookie Settings
-## Name    - the cookie name
-## Secret - the seed string for secure cookies; should be 16, 24, or 32 bytes
-##          for use with an AES cipher when cookie_refresh or pass_access_token
-##          is set
-## Domain - (optional) cookie domain to force cookies to (ie: .yourcompany.com)
-## Expire - (duration) expire timeframe for cookie
-## Refresh - (duration) refresh the cookie when less than this much time remains before
-##           expiration; should be less than cookie_expire; set to 0 to disable.
-##           Refresh revalidated the OAuth token to ensure it is still valid. ie: 24h
-## Secure - secure cookies are only sent by the browser of a HTTPS connection (recommended)
+## Name     - the cookie name
+## Secret   - the seed string for secure cookies; should be 16, 24, or 32 bytes
+##            for use with an AES cipher when cookie_refresh or pass_access_token
+##            is set
+## Domain   - (optional) cookie domain to force cookies to (ie: .yourcompany.com)
+## Expire   - (duration) expire timeframe for cookie
+## Refresh  - (duration) refresh the cookie when duration has elapsed after cookie was initially set.
+##            Should be less than cookie_expire; set to 0 to disable.
+##            On refresh, OAuth token is re-validated. 
+##            (ie: 1h means tokens are refreshed on request 1hr+ after it was set)
+## Secure   - secure cookies are only sent by the browser of a HTTPS connection (recommended)
 ## HttpOnly - httponly cookies are not readable by javascript (recommended)
 # cookie_name = "_oauth2_proxy"
 # cookie_secret = ""

cookies.go

@@ -15,29 +15,35 @@ import (
 	"time"
 )
 
-func validateCookie(cookie *http.Cookie, seed string) (string, time.Time, bool) {
+func validateCookie(cookie *http.Cookie, seed string, expiration time.Duration) (value string, t time.Time, ok bool) {
 	// value, timestamp, sig
 	parts := strings.Split(cookie.Value, "|")
 	if len(parts) != 3 {
-		return "", time.Unix(0, 0), false
+		return
 	}
 	sig := cookieSignature(seed, cookie.Name, parts[0], parts[1])
 	if checkHmac(parts[2], sig) {
 		ts, err := strconv.Atoi(parts[1])
-		if err == nil && int64(ts) > time.Now().Add(time.Duration(24)*7*time.Hour*-1).Unix() {
+		if err != nil {
+			return
+		}
+		t = time.Unix(int64(ts), 0)
+		if 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 {
-				return string(rawValue), time.Unix(int64(ts), 0), true
+				value = string(rawValue)
+				ok = true
+				return
 			}
 		}
 	}
-	return "", time.Unix(0, 0), false
+	return
 }
 
-func signedCookieValue(seed string, key string, value string) string {
+func signedCookieValue(seed string, key string, value string, now time.Time) string {
 	encodedValue := base64.URLEncoding.EncodeToString([]byte(value))
-	timeStr := fmt.Sprintf("%d", time.Now().Unix())
+	timeStr := fmt.Sprintf("%d", now.Unix())
 	sig := cookieSignature(seed, key, encodedValue, timeStr)
 	cookieVal := fmt.Sprintf("%s|%s|%s", encodedValue, timeStr, sig)
 	return cookieVal

oauthproxy.go

@@ -191,7 +191,7 @@ func (p *OauthProxy) redeemCode(host, code string) (string, string, error) {
 	return access_token, email, nil
 }
 
-func (p *OauthProxy) MakeCookie(req *http.Request, value string, expiration time.Duration) *http.Cookie {
+func (p *OauthProxy) MakeCookie(req *http.Request, value string, expiration time.Duration, now time.Time) *http.Cookie {
 	domain := req.Host
 	if h, _, err := net.SplitHostPort(domain); err == nil {
 		domain = h
@@ -204,7 +204,7 @@ func (p *OauthProxy) MakeCookie(req *http.Request, value string, expiration time
 	}
 
 	if value != "" {
-		value = signedCookieValue(p.CookieSeed, p.CookieName, value)
+		value = signedCookieValue(p.CookieSeed, p.CookieName, value, now)
 	}
 
 	return &http.Cookie{
@@ -214,36 +214,34 @@ func (p *OauthProxy) MakeCookie(req *http.Request, value string, expiration time
 		Domain:   domain,
 		HttpOnly: p.CookieHttpOnly,
 		Secure:   p.CookieSecure,
-		Expires:  time.Now().Add(expiration),
+		Expires:  now.Add(expiration),
 	}
 }
 
 func (p *OauthProxy) ClearCookie(rw http.ResponseWriter, req *http.Request) {
-	http.SetCookie(rw, p.MakeCookie(req, "", time.Duration(1)*time.Hour*-1))
+	http.SetCookie(rw, p.MakeCookie(req, "", time.Hour*-1, time.Now()))
 }
 
 func (p *OauthProxy) SetCookie(rw http.ResponseWriter, req *http.Request, val string) {
-	http.SetCookie(rw, p.MakeCookie(req, val, p.CookieExpire))
+	http.SetCookie(rw, p.MakeCookie(req, val, p.CookieExpire, time.Now()))
 }
 
 func (p *OauthProxy) ProcessCookie(rw http.ResponseWriter, req *http.Request) (email, user, access_token string, ok bool) {
 	var value string
 	var timestamp time.Time
 	cookie, err := req.Cookie(p.CookieName)
 	if err == nil {
-		value, timestamp, ok = validateCookie(cookie, p.CookieSeed)
+		value, timestamp, ok = validateCookie(cookie, p.CookieSeed, p.CookieExpire)
 		if ok {
-			email, user, access_token, err = parseCookieValue(
-				value, p.AesCipher)
+			email, user, access_token, err = parseCookieValue(value, p.AesCipher)
 		}
 	}
 	if err != nil {
 		log.Printf(err.Error())
 		ok = false
-	} else if p.CookieRefresh != time.Duration(0) {
-		expires := timestamp.Add(p.CookieExpire)
-		refresh_threshold := time.Now().Add(p.CookieRefresh)
-		if refresh_threshold.Unix() > expires.Unix() {
+	} else if ok && p.CookieRefresh != time.Duration(0) && value != "" {
+		refresh := timestamp.Add(p.CookieRefresh)
+		if refresh.Before(time.Now()) {
 			ok = p.Validator(email) && p.provider.ValidateToken(access_token)
 			if ok {
 				p.SetCookie(rw, req, value)

oauthproxy_test.go

@@ -348,13 +348,12 @@ func NewProcessCookieTest(opts ProcessCookieTestOpts) *ProcessCookieTest {
 
 	pc_test.opts = NewOptions()
 	pc_test.opts.Upstreams = append(pc_test.opts.Upstreams, "unused")
-	pc_test.opts.CookieSecret = "foobar"
 	pc_test.opts.ClientID = "bazquux"
 	pc_test.opts.ClientSecret = "xyzzyplugh"
 	pc_test.opts.CookieSecret = "0123456789abcdef"
 	// First, set the CookieRefresh option so proxy.AesCipher is created,
 	// needed to encrypt the access_token.
-	pc_test.opts.CookieRefresh = time.Duration(24) * time.Hour
+	pc_test.opts.CookieRefresh = time.Hour
 	pc_test.opts.Validate()
 
 	pc_test.proxy = NewOauthProxy(pc_test.opts, func(email string) bool {
@@ -379,14 +378,13 @@ func NewProcessCookieTestWithDefaults() *ProcessCookieTest {
 	})
 }
 
-func (p *ProcessCookieTest) MakeCookie(value, access_token string) *http.Cookie {
-	cookie_value, _ := buildCookieValue(
-		value, p.proxy.AesCipher, access_token)
-	return p.proxy.MakeCookie(p.req, cookie_value, p.opts.CookieExpire)
+func (p *ProcessCookieTest) MakeCookie(value, access_token string, ref time.Time) *http.Cookie {
+	cookie_value, _ := buildCookieValue(value, p.proxy.AesCipher, access_token)
+	return p.proxy.MakeCookie(p.req, cookie_value, p.opts.CookieExpire, ref)
 }
 
 func (p *ProcessCookieTest) AddCookie(value, access_token string) {
-	p.req.AddCookie(p.MakeCookie(value, access_token))
+	p.req.AddCookie(p.MakeCookie(value, access_token, time.Now()))
 }
 
 func (p *ProcessCookieTest) ProcessCookie() (email, user, access_token string, ok bool) {
@@ -416,15 +414,16 @@ func TestProcessCookieFailIfParsingCookieValueFails(t *testing.T) {
 		pc_test.proxy.AesCipher, "my_access_token")
 	pc_test.req.AddCookie(pc_test.proxy.MakeCookie(
 		pc_test.req, value+"some bogus bytes",
-		pc_test.opts.CookieExpire))
+		pc_test.opts.CookieExpire, time.Now()))
 	_, _, _, ok := pc_test.ProcessCookie()
 	assert.Equal(t, false, ok)
 }
 
 func TestProcessCookieRefreshNotSet(t *testing.T) {
 	pc_test := NewProcessCookieTestWithDefaults()
 	pc_test.proxy.CookieExpire = time.Duration(23) * time.Hour
-	cookie := pc_test.MakeCookie("[email protected]", "")
+	reference := time.Now().Add(time.Duration(-2) * time.Hour)
+	cookie := pc_test.MakeCookie("[email protected]", "", reference)
 	pc_test.req.AddCookie(cookie)
 
 	_, _, _, ok := pc_test.ProcessCookie()
@@ -435,22 +434,24 @@ func TestProcessCookieRefreshNotSet(t *testing.T) {
 func TestProcessCookieRefresh(t *testing.T) {
 	pc_test := NewProcessCookieTestWithDefaults()
 	pc_test.proxy.CookieExpire = time.Duration(23) * time.Hour
-	cookie := pc_test.MakeCookie("[email protected]", "my_access_token")
+	reference := time.Now().Add(time.Duration(-2) * time.Hour)
+	cookie := pc_test.MakeCookie("[email protected]", "my_access_token", reference)
 	pc_test.req.AddCookie(cookie)
 
-	pc_test.proxy.CookieRefresh = time.Duration(24) * time.Hour
+	pc_test.proxy.CookieRefresh = time.Hour
 	_, _, _, ok := pc_test.ProcessCookie()
 	assert.Equal(t, true, ok)
 	assert.NotEqual(t, []string(nil), pc_test.rw.HeaderMap["Set-Cookie"])
 }
 
 func TestProcessCookieRefreshThresholdNotCrossed(t *testing.T) {
 	pc_test := NewProcessCookieTestWithDefaults()
-	pc_test.proxy.CookieExpire = time.Duration(25) * time.Hour
-	cookie := pc_test.MakeCookie("[email protected]", "my_access_token")
+	pc_test.proxy.CookieExpire = time.Duration(23) * time.Hour
+	reference := time.Now().Add(time.Duration(-30) * time.Minute)
+	cookie := pc_test.MakeCookie("[email protected]", "my_access_token", reference)
 	pc_test.req.AddCookie(cookie)
 
-	pc_test.proxy.CookieRefresh = time.Duration(24) * time.Hour
+	pc_test.proxy.CookieRefresh = time.Hour
 	_, _, _, ok := pc_test.ProcessCookie()
 	assert.Equal(t, true, ok)
 	assert.Equal(t, []string(nil), pc_test.rw.HeaderMap["Set-Cookie"])
@@ -461,10 +462,11 @@ func TestProcessCookieFailIfRefreshSetAndTokenNoLongerValid(t *testing.T) {
 		provider_validate_cookie_response: false,
 	})
 	pc_test.proxy.CookieExpire = time.Duration(23) * time.Hour
-	cookie := pc_test.MakeCookie("[email protected]", "my_access_token")
+	reference := time.Now().Add(time.Duration(-24) * time.Hour)
+	cookie := pc_test.MakeCookie("[email protected]", "my_access_token", reference)
 	pc_test.req.AddCookie(cookie)
 
-	pc_test.proxy.CookieRefresh = time.Duration(24) * time.Hour
+	pc_test.proxy.CookieRefresh = time.Hour
 	_, _, _, ok := pc_test.ProcessCookie()
 	assert.Equal(t, false, ok)
 	assert.Equal(t, []string(nil), pc_test.rw.HeaderMap["Set-Cookie"])
@@ -475,10 +477,11 @@ func TestProcessCookieFailIfRefreshSetAndUserNoLongerValid(t *testing.T) {
 	pc_test.validate_user = false
 
 	pc_test.proxy.CookieExpire = time.Duration(23) * time.Hour
-	cookie := pc_test.MakeCookie("[email protected]", "my_access_token")
+	reference := time.Now().Add(time.Duration(-2) * time.Hour)
+	cookie := pc_test.MakeCookie("[email protected]", "my_access_token", reference)
 	pc_test.req.AddCookie(cookie)
 
-	pc_test.proxy.CookieRefresh = time.Duration(24) * time.Hour
+	pc_test.proxy.CookieRefresh = time.Hour
 	_, _, _, ok := pc_test.ProcessCookie()
 	assert.Equal(t, false, ok)
 	assert.Equal(t, []string(nil), pc_test.rw.HeaderMap["Set-Cookie"])