From 7c048e8135d262261be3cf504e50038c6cb9ddeb Mon Sep 17 00:00:00 2001 From: Chris Goller Date: Fri, 14 Apr 2017 02:12:52 -0500 Subject: [PATCH 1/9] Add JWT refresh on validation. JWTs will only life five minutes into the future. Any time the server receives an authenicated request, the JWT's expire at will be extended into the future. --- oauth2/cookies.go | 83 +++++++++++++++++++++++--------- oauth2/jwt.go | 118 +++++++++++++--------------------------------- oauth2/mux.go | 15 ++++-- oauth2/oauth2.go | 21 +++++---- server/auth.go | 2 +- 5 files changed, 117 insertions(+), 122 deletions(-) diff --git a/oauth2/cookies.go b/oauth2/cookies.go index 7bc701cfb..698198cd2 100644 --- a/oauth2/cookies.go +++ b/oauth2/cookies.go @@ -9,24 +9,28 @@ import ( const ( // DefaultCookieName is the name of the stored cookie DefaultCookieName = "session" + // DefaultInactivityDuration is the duration a token is valid without any new activity + DefaultInactivityDuration = 5 * time.Minute ) var _ Authenticator = &cookie{} // cookie represents the location and expiration time of new cookies. type cookie struct { - Name string - Duration time.Duration - Now func() time.Time - Tokens Tokenizer + Name string // Name is the name of the cookie stored on the browser + Lifespan time.Duration // Lifespan is the expiration date of the cookie. 0 means session cookie + Inactivity time.Duration // Inactivity is the length of time a token is valid if there is no activity + Now func() time.Time + Tokens Tokenizer } // NewCookieJWT creates an Authenticator that uses cookies for auth -func NewCookieJWT(secret string, duration time.Duration) Authenticator { +func NewCookieJWT(secret string, lifespan time.Duration) Authenticator { return &cookie{ - Name: DefaultCookieName, - Duration: duration, - Now: time.Now, + Name: DefaultCookieName, + Lifespan: lifespan, + Inactivity: DefaultInactivityDuration, + Now: time.Now, Tokens: &JWT{ Secret: secret, Now: time.Now, @@ -35,47 +39,80 @@ func NewCookieJWT(secret string, duration time.Duration) Authenticator { } // Validate returns Principal of the Cookie if the Token is valid. -func (c *cookie) Validate(ctx context.Context, r *http.Request) (Principal, error) { +func (c *cookie) Validate(ctx context.Context, w http.ResponseWriter, r *http.Request) (Principal, error) { cookie, err := r.Cookie(c.Name) if err != nil { return Principal{}, ErrAuthentication } - return c.Tokens.ValidPrincipal(ctx, Token(cookie.Value), c.Duration) + + p, err := c.Tokens.ValidPrincipal(ctx, Token(cookie.Value), c.Lifespan) + if err != nil { + return Principal{}, ErrAuthentication + } + + // Refresh the token by extending its life another Inactivity duration + p, err = c.Tokens.ExtendPrincipal(ctx, p, c.Inactivity) + if err != nil { + return Principal{}, ErrAuthentication + } + + token, err := c.Tokens.Create(ctx, p) + if err != nil { + return Principal{}, ErrAuthentication + } + + // Cookie lifespan can be indirectly figured out by taking the token's + // issued at time and adding the lifespan setting The token's issued at + // time happens to correspond to the cookie's original issued at time. + exp := p.IssuedAt.Add(c.Lifespan) + // Once the token has been extended, write it out as a new cookie. + c.setCookie(w, string(token), exp) + + return p, nil } // Authorize will create cookies containing token information. It'll create // a token with cookie.Duration of life to be stored as the cookie's value. func (c *cookie) Authorize(ctx context.Context, w http.ResponseWriter, p Principal) error { - token, err := c.Tokens.Create(ctx, p, c.Duration) + // Principal will be issued at Now() and will expire + // c.Inactivity into the future + now := c.Now().UTC() + p.IssuedAt = now + p.ExpiresAt = now.Add(c.Inactivity) + + token, err := c.Tokens.Create(ctx, p) if err != nil { return err } + + // The time when the cookie expires + exp := now.Add(c.Lifespan) + c.setCookie(w, string(token), exp) + + return nil +} + +// setCookie creates a cookie with value expiring at exp and writes it as a cookie into the response +func (c *cookie) setCookie(w http.ResponseWriter, value string, exp time.Time) { // Cookie has a Token baked into it cookie := http.Cookie{ Name: DefaultCookieName, - Value: string(token), + Value: value, HttpOnly: true, Path: "/", } // Only set a cookie to be persistent (endure beyond the browser session) // if auth duration is greater than zero - if c.Duration > 0 { - cookie.Expires = c.Now().UTC().Add(c.Duration) + if c.Lifespan > 0 || exp.Before(c.Now().UTC()) { + cookie.Expires = exp } http.SetCookie(w, &cookie) - return nil } // Expire returns a cookie that will expire an existing cookie func (c *cookie) Expire(w http.ResponseWriter) { - cookie := http.Cookie{ - Name: DefaultCookieName, - Value: "none", - HttpOnly: true, - Path: "/", - Expires: c.Now().UTC().Add(-1 * time.Hour), // to expire cookie set the time in the past - } - http.SetCookie(w, &cookie) + // to expire cookie set the time in the past + c.setCookie(w, "none", c.Now().UTC().Add(-1*time.Hour)) } diff --git a/oauth2/jwt.go b/oauth2/jwt.go index e925c3c4c..c65e56e8b 100644 --- a/oauth2/jwt.go +++ b/oauth2/jwt.go @@ -44,47 +44,9 @@ func (c *Claims) Valid() error { return nil } -// EverlastingClaims extends jwt.StandardClaims' Valid to make sure claims has -// a subject, and it ignores the expiration -type EverlastingClaims struct { - gojwt.StandardClaims - Now func() time.Time -} - -// Valid time based claims "iat, nbf". -// There is no accounting for clock skew. -// As well, if any of the above claims are not in the token, it will still -// be considered a valid claim. -func (c *EverlastingClaims) Valid() error { - vErr := new(gojwt.ValidationError) - now := c.Now().Unix() - - // The claims below are optional, by default, so if they are set to the - // default value in Go, let's not fail the verification for them. - - if c.VerifyIssuedAt(now, false) == false { - vErr.Inner = fmt.Errorf("Token used before issued") - vErr.Errors |= gojwt.ValidationErrorIssuedAt - } - - if c.VerifyNotBefore(now, false) == false { - vErr.Inner = fmt.Errorf("token is not valid yet") - vErr.Errors |= gojwt.ValidationErrorNotValidYet - } - - if c.Subject == "" { - return fmt.Errorf("claim has no subject") - } - - if vErr.Errors > 0 { - return vErr - } - - return nil -} - -// ValidPrincipal checks if the jwtToken is signed correctly and validates with Claims. -func (j *JWT) ValidPrincipal(ctx context.Context, jwtToken Token, duration time.Duration) (Principal, error) { +// ValidPrincipal checks if the jwtToken is signed correctly and validates with Claims. lifespan is the +// maximum valid lifetime of a token. +func (j *JWT) ValidPrincipal(ctx context.Context, jwtToken Token, lifespan time.Duration) (Principal, error) { gojwt.TimeFunc = j.Now // Check for expected signing method. @@ -95,20 +57,16 @@ func (j *JWT) ValidPrincipal(ctx context.Context, jwtToken Token, duration time. return []byte(j.Secret), nil } - if duration == 0 { - return j.ValidEverlastingClaims(jwtToken, alg) - } - - return j.ValidClaims(jwtToken, duration, alg) + return j.ValidClaims(jwtToken, lifespan, alg) } // ValidClaims validates a token with StandardClaims -func (j *JWT) ValidClaims(jwtToken Token, duration time.Duration, alg gojwt.Keyfunc) (Principal, error) { +func (j *JWT) ValidClaims(jwtToken Token, lifespan time.Duration, alg gojwt.Keyfunc) (Principal, error) { // 1. Checks for expired tokens // 2. Checks if time is after the issued at // 3. Check if time is after not before (nbf) // 4. Check if subject is not empty - // 5. Check if duration matches auth duration + // 5. Check if duration less than auth lifespan token, err := gojwt.ParseWithClaims(string(jwtToken), &Claims{}, alg) if err != nil { return Principal{}, err @@ -123,56 +81,35 @@ func (j *JWT) ValidClaims(jwtToken Token, duration time.Duration, alg gojwt.Keyf return Principal{}, fmt.Errorf("unable to convert claims to standard claims") } - if time.Duration(claims.ExpiresAt-claims.IssuedAt)*time.Second != duration { - return Principal{}, fmt.Errorf("claims duration is different from auth duration") + exp := time.Unix(claims.ExpiresAt, 0) + iat := time.Unix(claims.IssuedAt, 0) + + // If the duration of the claim is longer than the auth lifespan then this is + // an invalid claim because server assumes that lifespan is the maximum possible + // duration + if exp.Sub(iat) > lifespan { + return Principal{}, fmt.Errorf("claims duration is different from auth lifespan") } return Principal{ - Subject: claims.Subject, - Issuer: claims.Issuer, + Subject: claims.Subject, + Issuer: claims.Issuer, + ExpiresAt: exp, + IssuedAt: iat, }, nil } -// ValidEverlastingClaims validates a token with EverlastingClaims -func (j *JWT) ValidEverlastingClaims(jwtToken Token, alg gojwt.Keyfunc) (Principal, error) { - // 1. Checks if time is after the issued at - // 2. Check if time is after not before (nbf) - // 3. Check if subject is not empty - token, err := gojwt.ParseWithClaims(string(jwtToken), &EverlastingClaims{ - Now: j.Now, - }, alg) - if err != nil { - return Principal{}, err - // at time of this writing and researching the docs, token.Valid seems to be always true - } else if !token.Valid { - return Principal{}, err - } - - // at time of this writing and researching the docs, there will always be claims - claims, ok := token.Claims.(*EverlastingClaims) - if !ok { - return Principal{}, fmt.Errorf("unable to convert claims to everlasting claims") - } - - return Principal{ - Subject: claims.Subject, - Issuer: claims.Issuer, - }, nil -} - -// Create creates a signed JWT token from user that expires at Now + duration -func (j *JWT) Create(ctx context.Context, user Principal, duration time.Duration) (Token, error) { - gojwt.TimeFunc = j.Now +// Create creates a signed JWT token from user that expires at Principal's ExpireAt time. +func (j *JWT) Create(ctx context.Context, user Principal) (Token, error) { // Create a new token object, specifying signing method and the claims // you would like it to contain. - now := j.Now().UTC() claims := &Claims{ gojwt.StandardClaims{ Subject: user.Subject, Issuer: user.Issuer, - ExpiresAt: now.Add(duration).Unix(), - IssuedAt: now.Unix(), - NotBefore: now.Unix(), + ExpiresAt: user.ExpiresAt.Unix(), + IssuedAt: user.IssuedAt.Unix(), + NotBefore: user.IssuedAt.Unix(), }, } token := gojwt.NewWithClaims(gojwt.SigningMethodHS256, claims) @@ -184,3 +121,12 @@ func (j *JWT) Create(ctx context.Context, user Principal, duration time.Duration } return Token(t), nil } + +// ExtendPrincipal sets the expires at to be the current time plus the extention into the future +func (j *JWT) ExtendPrincipal(ctx context.Context, principal Principal, extension time.Duration) (Principal, error) { + // Extend the time of expiration. Do not change IssuedAt as the + // lifetime of the token is extended, but, NOT the original time + // of issue. This is used to enforce a maximum lifetime of a token + principal.ExpiresAt = j.Now().UTC().Add(extension) + return principal, nil +} diff --git a/oauth2/mux.go b/oauth2/mux.go index 0236ecdff..8b7fc302f 100644 --- a/oauth2/mux.go +++ b/oauth2/mux.go @@ -23,6 +23,7 @@ func NewAuthMux(p Provider, a Authenticator, t Tokenizer, l chronograf.Logger) * Logger: l, SuccessURL: "/", FailureURL: "/login", + Now: time.Now, } } @@ -38,6 +39,7 @@ type AuthMux struct { Logger chronograf.Logger // Logger is used to give some more information about the OAuth2 process SuccessURL string // SuccessURL is redirect location after successful authorization FailureURL string // FailureURL is redirect location after authorization failure + Now func() time.Time // Now returns the current time (for testing) } // Login uses a Cookie with a random string as the state validation method. JWTs are @@ -52,12 +54,17 @@ func (j *AuthMux) Login() http.Handler { // oauth2 provider's password. // If the callback is not received within 10 minutes, then authorization will fail. csrf := randomString(32) // 32 is not important... just long - p := Principal{ - Subject: csrf, - } + now := j.Now().UTC() + // This token will be valid for 10 minutes. Any chronograf server will // be able to validate this token. - token, err := j.Tokens.Create(r.Context(), p, TenMinutes) + p := Principal{ + Subject: csrf, + IssuedAt: now, + ExpiresAt: now.Add(TenMinutes), + } + token, err := j.Tokens.Create(r.Context(), p) + // This is likely an internal server error if err != nil { j.Logger. diff --git a/oauth2/oauth2.go b/oauth2/oauth2.go index e7757573c..d2ba0ef6e 100644 --- a/oauth2/oauth2.go +++ b/oauth2/oauth2.go @@ -30,8 +30,10 @@ var ( // Principal is any entity that can be authenticated type Principal struct { - Subject string - Issuer string + Subject string + Issuer string + ExpiresAt time.Time + IssuedAt time.Time } /* Interfaces */ @@ -62,8 +64,9 @@ type Mux interface { // Authenticator represents a service for authenticating users. type Authenticator interface { // Validate returns Principal associated with authenticated and authorized - // entity if successful. - Validate(context.Context, *http.Request) (Principal, error) + // entity if successful. ResponseWriter is passed if the Validation + // wishes to manipulate the response (i.e. refreshing a cookie) + Validate(context.Context, http.ResponseWriter, *http.Request) (Principal, error) // Authorize will grant privileges to a Principal Authorize(context.Context, http.ResponseWriter, Principal) error // Expire revokes privileges from a Principal @@ -78,9 +81,11 @@ type Token string // non-sensitive equivalent, referred to as a token, that has no extrinsic // or exploitable meaning or value. type Tokenizer interface { - // Create uses a token lasting duration with Principal data - Create(context.Context, Principal, time.Duration) (Token, error) + // Create issues a token at Principal's IssuedAt that lasts until Principal's ExpireAt + Create(context.Context, Principal) (Token, error) // ValidPrincipal checks if the token has a valid Principal and requires - // a duration to ensure it complies with possible server runtime arguments. - ValidPrincipal(context.Context, Token, time.Duration) (Principal, error) + // a lifespan duration to ensure it complies with possible server runtime arguments. + ValidPrincipal(ctx context.Context, token Token, lifespan time.Duration) (Principal, error) + // ExtendPrincipal adds the extention to the principal's lifespan. + ExtendPrincipal(ctx context.Context, principal Principal, extension time.Duration) (Principal, error) } diff --git a/server/auth.go b/server/auth.go index da5cda9d9..3636b31e3 100644 --- a/server/auth.go +++ b/server/auth.go @@ -23,7 +23,7 @@ func AuthorizedToken(auth oauth2.Authenticator, logger chronograf.Logger, next h ctx := r.Context() // We do not check the validity of the principal. Those // served further down the chain should do so. - principal, err := auth.Validate(ctx, r) + principal, err := auth.Validate(ctx, w, r) if err != nil { log.Error("Invalid principal") w.WriteHeader(http.StatusForbidden) From 017b01d3848ebbf6459242bcdcaa764362b883f9 Mon Sep 17 00:00:00 2001 From: Chris Goller Date: Fri, 14 Apr 2017 02:35:30 -0500 Subject: [PATCH 2/9] Update tests for refreshing jwts --- oauth2/cookies_test.go | 16 +++++++++++----- oauth2/jwt_test.go | 39 ++++++++++++++++++++++----------------- oauth2/mux_test.go | 9 +++++---- oauth2/oauth2_test.go | 6 +++++- server/auth_test.go | 2 +- 5 files changed, 44 insertions(+), 28 deletions(-) diff --git a/oauth2/cookies_test.go b/oauth2/cookies_test.go index e2303aafa..3803b3366 100644 --- a/oauth2/cookies_test.go +++ b/oauth2/cookies_test.go @@ -22,10 +22,14 @@ func (m *MockTokenizer) ValidPrincipal(ctx context.Context, token Token, duratio return m.Principal, m.ValidErr } -func (m *MockTokenizer) Create(ctx context.Context, p Principal, t time.Duration) (Token, error) { +func (m *MockTokenizer) Create(ctx context.Context, p Principal) (Token, error) { return m.Token, m.CreateErr } +func (m *MockTokenizer) ExtendPrincipal(ctx context.Context, principal Principal, extension time.Duration) (Principal, error) { + return principal, nil +} + func TestCookieAuthorize(t *testing.T) { var test = []struct { Desc string @@ -48,7 +52,7 @@ func TestCookieAuthorize(t *testing.T) { } for _, test := range test { cook := cookie{ - Duration: 1 * time.Second, + Lifespan: 1 * time.Second, Now: func() time.Time { return time.Unix(0, 0) }, @@ -121,8 +125,9 @@ func TestCookieValidate(t *testing.T) { }) cook := cookie{ - Name: test.Lookup, - Duration: 1 * time.Second, + Name: test.Lookup, + Lifespan: 1 * time.Second, + Inactivity: DefaultInactivityDuration, Now: func() time.Time { return time.Unix(0, 0) }, @@ -133,7 +138,8 @@ func TestCookieValidate(t *testing.T) { ValidErr: test.ValidErr, }, } - principal, err := cook.Validate(context.Background(), req) + w := httptest.NewRecorder() + principal, err := cook.Validate(context.Background(), w, req) if err != test.Err { t.Errorf("Cookie extract error; expected %v actual %v", test.Err, err) } diff --git a/oauth2/jwt_test.go b/oauth2/jwt_test.go index 987b06532..b85dfb614 100644 --- a/oauth2/jwt_test.go +++ b/oauth2/jwt_test.go @@ -10,6 +10,7 @@ import ( ) func TestAuthenticate(t *testing.T) { + history := time.Unix(-446774400, 0) var tests = []struct { Desc string Secret string @@ -33,7 +34,9 @@ func TestAuthenticate(t *testing.T) { Token: "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIvY2hyb25vZ3JhZi92MS91c2Vycy8xIiwibmFtZSI6IkRvYyBCcm93biIsImlhdCI6LTQ0Njc3NDQwMCwiZXhwIjotNDQ2Nzc0Mzk5LCJuYmYiOi00NDY3NzQ0MDB9.Ga0zGXWTT2CBVnnIhIO5tUAuBEVk4bKPaT4t4MU1ngo", Duration: time.Second, Principal: oauth2.Principal{ - Subject: "/chronograf/v1/users/1", + Subject: "/chronograf/v1/users/1", + ExpiresAt: history.Add(time.Second), + IssuedAt: history, }, }, { @@ -42,7 +45,9 @@ func TestAuthenticate(t *testing.T) { Token: "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIvY2hyb25vZ3JhZi92MS91c2Vycy8xIiwibmFtZSI6IkRvYyBCcm93biIsImlhdCI6LTQ0Njc3NDQwMCwiZXhwIjotNDQ2Nzc0NDAxLCJuYmYiOi00NDY3NzQ0MDB9.vWXdm0-XQ_pW62yBpSISFFJN_yz0vqT9_INcUKTp5Q8", Duration: time.Second, Principal: oauth2.Principal{ - Subject: "", + Subject: "", + ExpiresAt: history.Add(time.Second), + IssuedAt: history, }, Err: errors.New("token is expired by 1s"), }, @@ -52,7 +57,9 @@ func TestAuthenticate(t *testing.T) { Token: "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIvY2hyb25vZ3JhZi92MS91c2Vycy8xIiwibmFtZSI6IkRvYyBCcm93biIsImlhdCI6LTQ0Njc3NDQwMCwiZXhwIjotNDQ2Nzc0NDAwLCJuYmYiOi00NDY3NzQzOTl9.TMGAhv57u1aosjc4ywKC7cElP1tKyQH7GmRF2ToAxlE", Duration: time.Second, Principal: oauth2.Principal{ - Subject: "", + Subject: "", + ExpiresAt: history.Add(time.Second), + IssuedAt: history, }, Err: errors.New("token is not valid yet"), }, @@ -62,7 +69,9 @@ func TestAuthenticate(t *testing.T) { Token: "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpYXQiOi00NDY3NzQ0MDAsImV4cCI6LTQ0Njc3NDQwMCwibmJmIjotNDQ2Nzc0NDAwfQ.gxsA6_Ei3s0f2I1TAtrrb8FmGiO25OqVlktlF_ylhX4", Duration: time.Second, Principal: oauth2.Principal{ - Subject: "", + Subject: "", + ExpiresAt: history.Add(time.Second), + IssuedAt: history, }, Err: errors.New("claim has no subject"), }, @@ -72,18 +81,12 @@ func TestAuthenticate(t *testing.T) { Token: "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIvY2hyb25vZ3JhZi92MS91c2Vycy8xIiwibmFtZSI6IkRvYyBCcm93biIsImlhdCI6LTQ0Njc3NDQwMCwiZXhwIjotNDQ2Nzc0NDAwLCJuYmYiOi00NDY3NzQ0MDB9._rZ4gOIei9PizHOABH6kLcJTA3jm8ls0YnDxtz1qeUI", Duration: 500 * time.Hour, Principal: oauth2.Principal{ - Subject: "/chronograf/v1/users/1", + Subject: "/chronograf/v1/users/1", + ExpiresAt: history, + IssuedAt: history, }, Err: errors.New("claims duration is different from auth duration"), }, - { - Desc: "Test valid EverlastingClaim", - Secret: "secret", - Token: "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIvY2hyb25vZ3JhZi92MS91c2Vycy8xIiwibmFtZSI6IkRvYyBCcm93biIsImlhdCI6LTQ0Njc3NDQwMCwiZXhwIjotNDQ2Nzc0Mzk5LCJuYmYiOi00NDY3NzQ0MDB9.Ga0zGXWTT2CBVnnIhIO5tUAuBEVk4bKPaT4t4MU1ngo", - Principal: oauth2.Principal{ - Subject: "/chronograf/v1/users/1", - }, - }, } for _, test := range tests { j := oauth2.JWT{ @@ -107,18 +110,20 @@ func TestAuthenticate(t *testing.T) { } func TestToken(t *testing.T) { - duration := time.Second expected := oauth2.Token("eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJleHAiOi00NDY3NzQzOTksImlhdCI6LTQ0Njc3NDQwMCwibmJmIjotNDQ2Nzc0NDAwLCJzdWIiOiIvY2hyb25vZ3JhZi92MS91c2Vycy8xIn0.ofQM6yTmrmve5JeEE0RcK4_euLXuZ_rdh6bLAbtbC9M") + history := time.Unix(-446774400, 0) j := oauth2.JWT{ Secret: "secret", Now: func() time.Time { - return time.Unix(-446774400, 0) + return history }, } p := oauth2.Principal{ - Subject: "/chronograf/v1/users/1", + Subject: "/chronograf/v1/users/1", + ExpiresAt: history.Add(time.Second), + IssuedAt: history, } - if token, err := j.Create(context.Background(), p, duration); err != nil { + if token, err := j.Create(context.Background(), p); err != nil { t.Errorf("Error creating token for principal: %v", err) } else if token != expected { t.Errorf("Error creating token; expected: %s actual: %s", expected, token) diff --git a/oauth2/mux_test.go b/oauth2/mux_test.go index acdf81c01..7d4fc41d2 100644 --- a/oauth2/mux_test.go +++ b/oauth2/mux_test.go @@ -30,10 +30,11 @@ func setupMuxTest(selector func(*AuthMux) http.Handler) (*http.Client, *httptest mp := &MockProvider{"biff@example.com", provider.URL} mt := &YesManTokenizer{} auth := &cookie{ - Name: DefaultCookieName, - Duration: 1 * time.Hour, - Now: now, - Tokens: mt, + Name: DefaultCookieName, + Lifespan: 1 * time.Hour, + Inactivity: DefaultInactivityDuration, + Now: now, + Tokens: mt, } jm := NewAuthMux(mp, auth, mt, clog.New(clog.ParseLevel("debug"))) diff --git a/oauth2/oauth2_test.go b/oauth2/oauth2_test.go index a5bc026cd..9692e5021 100644 --- a/oauth2/oauth2_test.go +++ b/oauth2/oauth2_test.go @@ -63,10 +63,14 @@ func (y *YesManTokenizer) ValidPrincipal(ctx context.Context, token Token, durat }, nil } -func (y *YesManTokenizer) Create(ctx context.Context, p Principal, t time.Duration) (Token, error) { +func (y *YesManTokenizer) Create(ctx context.Context, p Principal) (Token, error) { return Token("HELLO?!MCFLY?!ANYONEINTHERE?!"), nil } +func (y *YesManTokenizer) ExtendPrincipal(ctx context.Context, p Principal, ext time.Duration) (Principal, error) { + return p, nil +} + func NewTestTripper(log chronograf.Logger, ts *httptest.Server, rt http.RoundTripper) (*TestTripper, error) { url, err := url.Parse(ts.URL) if err != nil { diff --git a/server/auth_test.go b/server/auth_test.go index 6dd7336e5..fc15d7f3f 100644 --- a/server/auth_test.go +++ b/server/auth_test.go @@ -18,7 +18,7 @@ type MockAuthenticator struct { Serialized string } -func (m *MockAuthenticator) Validate(context.Context, *http.Request) (oauth2.Principal, error) { +func (m *MockAuthenticator) Validate(context.Context, http.ResponseWriter, *http.Request) (oauth2.Principal, error) { return m.Principal, m.ValidateErr } func (m *MockAuthenticator) Authorize(ctx context.Context, w http.ResponseWriter, p oauth2.Principal) error { From d2012e4c8e7d77b9090a500e49969742c55a8528 Mon Sep 17 00:00:00 2001 From: Chris Goller Date: Mon, 17 Apr 2017 10:38:46 -0500 Subject: [PATCH 3/9] Add default now time func to return UTC --- oauth2/time.go | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 oauth2/time.go diff --git a/oauth2/time.go b/oauth2/time.go new file mode 100644 index 000000000..826a28757 --- /dev/null +++ b/oauth2/time.go @@ -0,0 +1,6 @@ +package oauth2 + +import "time" + +// DefaultNowTime returns UTC time at the present moment +const DefaultNowTime = func() time.Time { return time.Now().UTC() } From db9a15bbe4872a61f9f1b9a9a455e60d8d4f832b Mon Sep 17 00:00:00 2001 From: Chris Goller Date: Mon, 17 Apr 2017 10:39:01 -0500 Subject: [PATCH 4/9] Update oauth2 now time calculation to ensure UTC time --- oauth2/cookies.go | 10 +++++----- oauth2/jwt.go | 4 ++-- oauth2/mux.go | 4 ++-- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/oauth2/cookies.go b/oauth2/cookies.go index 698198cd2..8a5e80489 100644 --- a/oauth2/cookies.go +++ b/oauth2/cookies.go @@ -30,10 +30,10 @@ func NewCookieJWT(secret string, lifespan time.Duration) Authenticator { Name: DefaultCookieName, Lifespan: lifespan, Inactivity: DefaultInactivityDuration, - Now: time.Now, + Now: DefaultNowTime, Tokens: &JWT{ Secret: secret, - Now: time.Now, + Now: DefaultNowTime, }, } } @@ -76,7 +76,7 @@ func (c *cookie) Validate(ctx context.Context, w http.ResponseWriter, r *http.Re func (c *cookie) Authorize(ctx context.Context, w http.ResponseWriter, p Principal) error { // Principal will be issued at Now() and will expire // c.Inactivity into the future - now := c.Now().UTC() + now := c.Now() p.IssuedAt = now p.ExpiresAt = now.Add(c.Inactivity) @@ -104,7 +104,7 @@ func (c *cookie) setCookie(w http.ResponseWriter, value string, exp time.Time) { // Only set a cookie to be persistent (endure beyond the browser session) // if auth duration is greater than zero - if c.Lifespan > 0 || exp.Before(c.Now().UTC()) { + if c.Lifespan > 0 || exp.Before(c.Now()) { cookie.Expires = exp } @@ -114,5 +114,5 @@ func (c *cookie) setCookie(w http.ResponseWriter, value string, exp time.Time) { // Expire returns a cookie that will expire an existing cookie func (c *cookie) Expire(w http.ResponseWriter) { // to expire cookie set the time in the past - c.setCookie(w, "none", c.Now().UTC().Add(-1*time.Hour)) + c.setCookie(w, "none", c.Now().Add(-1*time.Hour)) } diff --git a/oauth2/jwt.go b/oauth2/jwt.go index c65e56e8b..d14bd7782 100644 --- a/oauth2/jwt.go +++ b/oauth2/jwt.go @@ -21,7 +21,7 @@ type JWT struct { func NewJWT(secret string) *JWT { return &JWT{ Secret: secret, - Now: time.Now, + Now: DefaultNowTime, } } @@ -127,6 +127,6 @@ func (j *JWT) ExtendPrincipal(ctx context.Context, principal Principal, extensio // Extend the time of expiration. Do not change IssuedAt as the // lifetime of the token is extended, but, NOT the original time // of issue. This is used to enforce a maximum lifetime of a token - principal.ExpiresAt = j.Now().UTC().Add(extension) + principal.ExpiresAt = j.Now().Add(extension) return principal, nil } diff --git a/oauth2/mux.go b/oauth2/mux.go index 8b7fc302f..52537ac5b 100644 --- a/oauth2/mux.go +++ b/oauth2/mux.go @@ -23,7 +23,7 @@ func NewAuthMux(p Provider, a Authenticator, t Tokenizer, l chronograf.Logger) * Logger: l, SuccessURL: "/", FailureURL: "/login", - Now: time.Now, + Now: DefaultNowTime, } } @@ -54,7 +54,7 @@ func (j *AuthMux) Login() http.Handler { // oauth2 provider's password. // If the callback is not received within 10 minutes, then authorization will fail. csrf := randomString(32) // 32 is not important... just long - now := j.Now().UTC() + now := j.Now() // This token will be valid for 10 minutes. Any chronograf server will // be able to validate this token. From f5930fd4b51b46f9aa7495678b5e2da5e7bd22b9 Mon Sep 17 00:00:00 2001 From: Chris Goller Date: Mon, 17 Apr 2017 10:57:33 -0500 Subject: [PATCH 5/9] Update JWT to use Extended rather than Extend --- oauth2/jwt.go | 4 ++-- oauth2/time.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/oauth2/jwt.go b/oauth2/jwt.go index d14bd7782..0eab590bb 100644 --- a/oauth2/jwt.go +++ b/oauth2/jwt.go @@ -122,8 +122,8 @@ func (j *JWT) Create(ctx context.Context, user Principal) (Token, error) { return Token(t), nil } -// ExtendPrincipal sets the expires at to be the current time plus the extention into the future -func (j *JWT) ExtendPrincipal(ctx context.Context, principal Principal, extension time.Duration) (Principal, error) { +// ExtendedPrincipal sets the expires at to be the current time plus the extention into the future +func (j *JWT) ExtendedPrincipal(ctx context.Context, principal Principal, extension time.Duration) (Principal, error) { // Extend the time of expiration. Do not change IssuedAt as the // lifetime of the token is extended, but, NOT the original time // of issue. This is used to enforce a maximum lifetime of a token diff --git a/oauth2/time.go b/oauth2/time.go index 826a28757..529e1c4b7 100644 --- a/oauth2/time.go +++ b/oauth2/time.go @@ -3,4 +3,4 @@ package oauth2 import "time" // DefaultNowTime returns UTC time at the present moment -const DefaultNowTime = func() time.Time { return time.Now().UTC() } +var DefaultNowTime = func() time.Time { return time.Now().UTC() } From 3c6f0db6230f468deb5722e3a3e8a396a108002c Mon Sep 17 00:00:00 2001 From: Chris Goller Date: Mon, 17 Apr 2017 11:49:45 -0500 Subject: [PATCH 6/9] Update oauth2 Authenticator signatures to use extend --- oauth2/cookies.go | 14 +++++++------- oauth2/cookies_test.go | 5 ++--- oauth2/oauth2.go | 11 ++++++----- oauth2/oauth2_test.go | 2 +- server/auth.go | 13 +++++++++++-- server/auth_test.go | 13 ++++++++++++- 6 files changed, 39 insertions(+), 19 deletions(-) diff --git a/oauth2/cookies.go b/oauth2/cookies.go index 8a5e80489..980ec1d30 100644 --- a/oauth2/cookies.go +++ b/oauth2/cookies.go @@ -39,19 +39,19 @@ func NewCookieJWT(secret string, lifespan time.Duration) Authenticator { } // Validate returns Principal of the Cookie if the Token is valid. -func (c *cookie) Validate(ctx context.Context, w http.ResponseWriter, r *http.Request) (Principal, error) { +func (c *cookie) Validate(ctx context.Context, r *http.Request) (Principal, error) { cookie, err := r.Cookie(c.Name) if err != nil { return Principal{}, ErrAuthentication } + return c.Tokens.ValidPrincipal(ctx, Token(cookie.Value), c.Lifespan) +} - p, err := c.Tokens.ValidPrincipal(ctx, Token(cookie.Value), c.Lifespan) - if err != nil { - return Principal{}, ErrAuthentication - } - +// Extend will extend the lifetime of the Token by the Inactivity time. Assumes +// Principal is already valid. +func (c *cookie) Extend(ctx context.Context, w http.ResponseWriter, p Principal) (Principal, error) { // Refresh the token by extending its life another Inactivity duration - p, err = c.Tokens.ExtendPrincipal(ctx, p, c.Inactivity) + p, err := c.Tokens.ExtendedPrincipal(ctx, p, c.Inactivity) if err != nil { return Principal{}, ErrAuthentication } diff --git a/oauth2/cookies_test.go b/oauth2/cookies_test.go index 3803b3366..73ea86db9 100644 --- a/oauth2/cookies_test.go +++ b/oauth2/cookies_test.go @@ -26,7 +26,7 @@ func (m *MockTokenizer) Create(ctx context.Context, p Principal) (Token, error) return m.Token, m.CreateErr } -func (m *MockTokenizer) ExtendPrincipal(ctx context.Context, principal Principal, extension time.Duration) (Principal, error) { +func (m *MockTokenizer) ExtendedPrincipal(ctx context.Context, principal Principal, extension time.Duration) (Principal, error) { return principal, nil } @@ -138,8 +138,7 @@ func TestCookieValidate(t *testing.T) { ValidErr: test.ValidErr, }, } - w := httptest.NewRecorder() - principal, err := cook.Validate(context.Background(), w, req) + principal, err := cook.Validate(context.Background(), req) if err != test.Err { t.Errorf("Cookie extract error; expected %v actual %v", test.Err, err) } diff --git a/oauth2/oauth2.go b/oauth2/oauth2.go index d2ba0ef6e..58668485e 100644 --- a/oauth2/oauth2.go +++ b/oauth2/oauth2.go @@ -64,11 +64,12 @@ type Mux interface { // Authenticator represents a service for authenticating users. type Authenticator interface { // Validate returns Principal associated with authenticated and authorized - // entity if successful. ResponseWriter is passed if the Validation - // wishes to manipulate the response (i.e. refreshing a cookie) - Validate(context.Context, http.ResponseWriter, *http.Request) (Principal, error) + // entity if successful. + Validate(context.Context, *http.Request) (Principal, error) // Authorize will grant privileges to a Principal Authorize(context.Context, http.ResponseWriter, Principal) error + // Extend will extend the lifetime of a already validated Principal + Extend(context.Context, http.ResponseWriter, Principal) (Principal, error) // Expire revokes privileges from a Principal Expire(http.ResponseWriter) } @@ -86,6 +87,6 @@ type Tokenizer interface { // ValidPrincipal checks if the token has a valid Principal and requires // a lifespan duration to ensure it complies with possible server runtime arguments. ValidPrincipal(ctx context.Context, token Token, lifespan time.Duration) (Principal, error) - // ExtendPrincipal adds the extention to the principal's lifespan. - ExtendPrincipal(ctx context.Context, principal Principal, extension time.Duration) (Principal, error) + // ExtendedPrincipal adds the extention to the principal's lifespan. + ExtendedPrincipal(ctx context.Context, principal Principal, extension time.Duration) (Principal, error) } diff --git a/oauth2/oauth2_test.go b/oauth2/oauth2_test.go index 9692e5021..48e4929ce 100644 --- a/oauth2/oauth2_test.go +++ b/oauth2/oauth2_test.go @@ -67,7 +67,7 @@ func (y *YesManTokenizer) Create(ctx context.Context, p Principal) (Token, error return Token("HELLO?!MCFLY?!ANYONEINTHERE?!"), nil } -func (y *YesManTokenizer) ExtendPrincipal(ctx context.Context, p Principal, ext time.Duration) (Principal, error) { +func (y *YesManTokenizer) ExtendedPrincipal(ctx context.Context, p Principal, ext time.Duration) (Principal, error) { return p, nil } diff --git a/server/auth.go b/server/auth.go index 3636b31e3..eb5868109 100644 --- a/server/auth.go +++ b/server/auth.go @@ -21,15 +21,24 @@ func AuthorizedToken(auth oauth2.Authenticator, logger chronograf.Logger, next h WithField("url", r.URL) ctx := r.Context() - // We do not check the validity of the principal. Those + // We do not check the authorization of the principal. Those // served further down the chain should do so. - principal, err := auth.Validate(ctx, w, r) + principal, err := auth.Validate(ctx, r) if err != nil { log.Error("Invalid principal") w.WriteHeader(http.StatusForbidden) return } + // If the principal is valid we will extend its lifespan + // into the future + principal, err = auth.Extend(ctx, w, principal) + if err != nil { + log.Error("Unable to extend principal") + w.WriteHeader(http.StatusForbidden) + return + } + // Send the principal to the next handler ctx = context.WithValue(ctx, oauth2.PrincipalKey, principal) next.ServeHTTP(w, r.WithContext(ctx)) diff --git a/server/auth_test.go b/server/auth_test.go index fc15d7f3f..a0d482de3 100644 --- a/server/auth_test.go +++ b/server/auth_test.go @@ -15,19 +15,30 @@ import ( type MockAuthenticator struct { Principal oauth2.Principal ValidateErr error + ExtendErr error Serialized string } -func (m *MockAuthenticator) Validate(context.Context, http.ResponseWriter, *http.Request) (oauth2.Principal, error) { +func (m *MockAuthenticator) Validate(context.Context, *http.Request) (oauth2.Principal, error) { return m.Principal, m.ValidateErr } + +func (m *MockAuthenticator) Extend(ctx context.Context, w http.ResponseWriter, p oauth2.Principal) (oauth2.Principal, error) { + cookie := http.Cookie{} + + http.SetCookie(w, &cookie) + return m.Principal, m.ExtendErr +} + func (m *MockAuthenticator) Authorize(ctx context.Context, w http.ResponseWriter, p oauth2.Principal) error { cookie := http.Cookie{} http.SetCookie(w, &cookie) return nil } + func (m *MockAuthenticator) Expire(http.ResponseWriter) {} + func (m *MockAuthenticator) ValidAuthorization(ctx context.Context, serializedAuthorization string) (oauth2.Principal, error) { return oauth2.Principal{}, nil } From 8804d9d4fb61c19edd2648834b91dbc63792288b Mon Sep 17 00:00:00 2001 From: Chris Goller Date: Mon, 17 Apr 2017 12:22:49 -0500 Subject: [PATCH 7/9] Add comment about tokens in extending --- oauth2/cookies.go | 1 + 1 file changed, 1 insertion(+) diff --git a/oauth2/cookies.go b/oauth2/cookies.go index 980ec1d30..f87e839ba 100644 --- a/oauth2/cookies.go +++ b/oauth2/cookies.go @@ -56,6 +56,7 @@ func (c *cookie) Extend(ctx context.Context, w http.ResponseWriter, p Principal) return Principal{}, ErrAuthentication } + // Creating a new token with the extended principal token, err := c.Tokens.Create(ctx, p) if err != nil { return Principal{}, ErrAuthentication From 7b26eb3f00e26f598f9059faa9486ae24ef710b1 Mon Sep 17 00:00:00 2001 From: Chris Goller Date: Mon, 17 Apr 2017 14:49:05 -0500 Subject: [PATCH 8/9] Add tests for token extend --- oauth2/cookies_test.go | 119 ++++++++++++++++++++++++++++++++++++++++- oauth2/jwt_test.go | 54 +++++++++++++++++++ 2 files changed, 172 insertions(+), 1 deletion(-) diff --git a/oauth2/cookies_test.go b/oauth2/cookies_test.go index 73ea86db9..cfa47bcc5 100644 --- a/oauth2/cookies_test.go +++ b/oauth2/cookies_test.go @@ -6,6 +6,7 @@ import ( "log" "net/http" "net/http/httptest" + "reflect" "strings" "testing" "time" @@ -16,6 +17,7 @@ type MockTokenizer struct { ValidErr error Token Token CreateErr error + ExtendErr error } func (m *MockTokenizer) ValidPrincipal(ctx context.Context, token Token, duration time.Duration) (Principal, error) { @@ -27,7 +29,7 @@ func (m *MockTokenizer) Create(ctx context.Context, p Principal) (Token, error) } func (m *MockTokenizer) ExtendedPrincipal(ctx context.Context, principal Principal, extension time.Duration) (Principal, error) { - return principal, nil + return principal, m.ExtendErr } func TestCookieAuthorize(t *testing.T) { @@ -155,3 +157,118 @@ func TestNewCookieJWT(t *testing.T) { t.Errorf("NewCookieJWT() did not create cookie Authenticator") } } + +func TestCookieExtend(t *testing.T) { + history := time.Unix(-446774400, 0) + type fields struct { + Name string + Lifespan time.Duration + Inactivity time.Duration + Now func() time.Time + Tokens Tokenizer + } + type args struct { + ctx context.Context + w *httptest.ResponseRecorder + p Principal + } + tests := []struct { + name string + fields fields + args args + want Principal + wantErr bool + }{ + { + name: "Successful extention", + want: Principal{ + Subject: "subject", + }, + fields: fields{ + Name: "session", + Lifespan: time.Second, + Inactivity: time.Second, + Now: func() time.Time { + return history + }, + Tokens: &MockTokenizer{ + Principal: Principal{ + Subject: "subject", + }, + Token: "token", + ExtendErr: nil, + }, + }, + args: args{ + ctx: context.Background(), + w: httptest.NewRecorder(), + p: Principal{ + Subject: "subject", + }, + }, + }, + { + name: "Unable to extend", + wantErr: true, + fields: fields{ + Tokens: &MockTokenizer{ + ExtendErr: fmt.Errorf("bad extend"), + }, + }, + args: args{ + ctx: context.Background(), + w: httptest.NewRecorder(), + p: Principal{ + Subject: "subject", + }, + }, + }, + { + name: "Unable to create", + wantErr: true, + fields: fields{ + Tokens: &MockTokenizer{ + CreateErr: fmt.Errorf("bad extend"), + }, + }, + args: args{ + ctx: context.Background(), + w: httptest.NewRecorder(), + p: Principal{ + Subject: "subject", + }, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + c := &cookie{ + Name: tt.fields.Name, + Lifespan: tt.fields.Lifespan, + Inactivity: tt.fields.Inactivity, + Now: tt.fields.Now, + Tokens: tt.fields.Tokens, + } + got, err := c.Extend(tt.args.ctx, tt.args.w, tt.args.p) + if (err != nil) != tt.wantErr { + t.Errorf("cookie.Extend() error = %v, wantErr %v", err, tt.wantErr) + return + } + if tt.wantErr == false { + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("cookie.Extend() = %v, want %v", got, tt.want) + } + + cookies := tt.args.w.HeaderMap["Set-Cookie"] + if len(cookies) == 0 { + t.Fatal("Expected some cookies but got zero") + } + log.Printf("%s", cookies) + want := fmt.Sprintf("%s=%s", DefaultCookieName, "token") + if !strings.Contains(cookies[0], want) { + t.Errorf("cookie.Extend() = %v, want %v", cookies[0], want) + } + } + }) + } +} diff --git a/oauth2/jwt_test.go b/oauth2/jwt_test.go index b85dfb614..992522cda 100644 --- a/oauth2/jwt_test.go +++ b/oauth2/jwt_test.go @@ -3,6 +3,7 @@ package oauth2_test import ( "context" "errors" + "reflect" "testing" "time" @@ -139,3 +140,56 @@ func TestSigningMethod(t *testing.T) { t.Errorf("Error wanted 'unexpected signing method', got %s", err.Error()) } } + +func TestJWT_ExtendedPrincipal(t *testing.T) { + history := time.Unix(-446774400, 0) + type fields struct { + Now func() time.Time + } + type args struct { + ctx context.Context + principal oauth2.Principal + extension time.Duration + } + tests := []struct { + name string + fields fields + args args + want oauth2.Principal + wantErr bool + }{ + { + name: "Extend principal by one hour", + fields: fields{ + Now: func() time.Time { + return history + }, + }, + args: args{ + ctx: context.Background(), + principal: oauth2.Principal{ + ExpiresAt: history, + }, + extension: time.Hour, + }, + want: oauth2.Principal{ + ExpiresAt: history.Add(time.Hour), + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + j := &oauth2.JWT{ + Now: tt.fields.Now, + } + got, err := j.ExtendedPrincipal(tt.args.ctx, tt.args.principal, tt.args.extension) + if (err != nil) != tt.wantErr { + t.Errorf("JWT.ExtendedPrincipal() error = %v, wantErr %v", err, tt.wantErr) + return + } + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("JWT.ExtendedPrincipal() = %v, want %v", got, tt.want) + } + }) + } +} From 5f32d8d471ad82f6b68fcc4d3f888795813e3ec2 Mon Sep 17 00:00:00 2001 From: Chris Goller Date: Mon, 17 Apr 2017 15:25:07 -0500 Subject: [PATCH 9/9] Update CHANGELOG to mention refreshing JWTs --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index abd54f2c5..8a7776913 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,8 @@ ### Features 1. [#1232](https://github.com/influxdata/chronograf/pull/1232): Fuse the query builder and raw query editor + 1. [#1286](https://github.com/influxdata/chronograf/pull/1286): Add refreshing JWTs for authentication + ### UI Improvements 1. [#1259](https://github.com/influxdata/chronograf/pull/1259): Add default display for empty dashboard