From 0bf098051b803e21e89526facb26804ee010e10e Mon Sep 17 00:00:00 2001 From: Jehiah Czebotar Date: Mon, 22 Jun 2015 15:10:08 -0400 Subject: [PATCH] fix cookie refresh code --- contrib/oauth2_proxy.cfg.example | 21 +++++++++-------- cookies.go | 20 ++++++++++------ oauthproxy.go | 22 ++++++++---------- oauthproxy_test.go | 39 +++++++++++++++++--------------- 4 files changed, 55 insertions(+), 47 deletions(-) diff --git a/contrib/oauth2_proxy.cfg.example b/contrib/oauth2_proxy.cfg.example index 1ba1c27ba..4006850a4 100644 --- a/contrib/oauth2_proxy.cfg.example +++ b/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 = "" diff --git a/cookies.go b/cookies.go index 8d7ce632f..2d538cd0a 100644 --- a/cookies.go +++ b/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 diff --git a/oauthproxy.go b/oauthproxy.go index 6db668fc1..4c76e75f7 100644 --- a/oauthproxy.go +++ b/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,16 +214,16 @@ 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) { @@ -231,19 +231,17 @@ func (p *OauthProxy) ProcessCookie(rw http.ResponseWriter, req *http.Request) (e 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) diff --git a/oauthproxy_test.go b/oauthproxy_test.go index f53d31e4d..afa4ccb3d 100644 --- a/oauthproxy_test.go +++ b/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,7 +414,7 @@ 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) } @@ -424,7 +422,8 @@ func TestProcessCookieFailIfParsingCookieValueFails(t *testing.T) { func TestProcessCookieRefreshNotSet(t *testing.T) { pc_test := NewProcessCookieTestWithDefaults() pc_test.proxy.CookieExpire = time.Duration(23) * time.Hour - cookie := pc_test.MakeCookie("michael.bland@gsa.gov", "") + reference := time.Now().Add(time.Duration(-2) * time.Hour) + cookie := pc_test.MakeCookie("michael.bland@gsa.gov", "", reference) pc_test.req.AddCookie(cookie) _, _, _, ok := pc_test.ProcessCookie() @@ -435,10 +434,11 @@ 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("michael.bland@gsa.gov", "my_access_token") + reference := time.Now().Add(time.Duration(-2) * time.Hour) + cookie := pc_test.MakeCookie("michael.bland@gsa.gov", "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"]) @@ -446,11 +446,12 @@ func TestProcessCookieRefresh(t *testing.T) { func TestProcessCookieRefreshThresholdNotCrossed(t *testing.T) { pc_test := NewProcessCookieTestWithDefaults() - pc_test.proxy.CookieExpire = time.Duration(25) * time.Hour - cookie := pc_test.MakeCookie("michael.bland@gsa.gov", "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("michael.bland@gsa.gov", "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("michael.bland@gsa.gov", "my_access_token") + reference := time.Now().Add(time.Duration(-24) * time.Hour) + cookie := pc_test.MakeCookie("michael.bland@gsa.gov", "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("michael.bland@gsa.gov", "my_access_token") + reference := time.Now().Add(time.Duration(-2) * time.Hour) + cookie := pc_test.MakeCookie("michael.bland@gsa.gov", "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"])