From 2ab833b16e01460a0b449f5a222a7fc6c4541625 Mon Sep 17 00:00:00 2001 From: Andrew Watkins Date: Thu, 13 Apr 2017 14:06:53 -0700 Subject: [PATCH 01/16] Fix function application bug --- ui/src/shared/components/MultiSelectDropdown.js | 8 -------- 1 file changed, 8 deletions(-) diff --git a/ui/src/shared/components/MultiSelectDropdown.js b/ui/src/shared/components/MultiSelectDropdown.js index 6009803445..020a72b67f 100644 --- a/ui/src/shared/components/MultiSelectDropdown.js +++ b/ui/src/shared/components/MultiSelectDropdown.js @@ -30,14 +30,6 @@ class MultiSelectDropdown extends Component { this.onApplyFunctions = ::this.onApplyFunctions } - componentWillReceiveProps(nextProps) { - if (!_.isEqual(this.state.localSelectedItems, nextProps.selectedItems)) { - this.setState({ - localSelectedItems: nextProps.selectedItems, - }) - } - } - handleClickOutside() { this.setState({isOpen: false}) } From 9ad5f282ee1ffae49c8a8d4e42fd46b4c647437a Mon Sep 17 00:00:00 2001 From: Andrew Watkins Date: Thu, 13 Apr 2017 15:36:57 -0700 Subject: [PATCH 02/16] Update CHANGELOG --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 79ea3beafa..b80a955d33 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -32,6 +32,7 @@ 1. [#1223](https://github.com/influxdata/chronograf/pull/1223): Use vhost as Chronograf's proxy to Kapacitor 1. [#1205](https://github.com/influxdata/chronograf/pull/1205): Allow initial source to be an InfluxEnterprise source 1. [#1244](https://github.com/influxdata/chronograf/pull/1244): Fix env var name for Google client secret + 1. [#1257](https://github.com/influxdata/chronograf/issues/1257): Fix function selection in query builder ### Features 1. [#1112](https://github.com/influxdata/chronograf/pull/1112): Add ability to delete a dashboard From dc4574fa3943a13ba5d627f4aa1b52c8d2b7d98b Mon Sep 17 00:00:00 2001 From: Andrew Watkins Date: Thu, 13 Apr 2017 17:49:53 -0700 Subject: [PATCH 03/16] Add default RP to queryConfig when no RP is present This will only happen if the qC is fully qualified. A fully qualified query is one that has a db, measurement, and field. --- server/queries.go | 53 +++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 49 insertions(+), 4 deletions(-) diff --git a/server/queries.go b/server/queries.go index 6e05f157f2..c98e500b7d 100644 --- a/server/queries.go +++ b/server/queries.go @@ -2,8 +2,11 @@ package server import ( "encoding/json" + "fmt" "net/http" + "golang.org/x/net/context" + "github.com/influxdata/chronograf" "github.com/influxdata/chronograf/influx/queries" ) @@ -37,7 +40,8 @@ func (s *Service) Queries(w http.ResponseWriter, r *http.Request) { } ctx := r.Context() - if _, err = s.SourcesStore.Get(ctx, srcID); err != nil { + src, err := s.SourcesStore.Get(ctx, srcID) + if err != nil { notFound(w, srcID, s.Logger) return } @@ -54,16 +58,57 @@ func (s *Service) Queries(w http.ResponseWriter, r *http.Request) { for i, q := range req.Queries { qr := QueryResponse{ - ID: q.ID, - Query: q.Query, - QueryConfig: ToQueryConfig(q.Query), + ID: q.ID, + Query: q.Query, } + + qc := ToQueryConfig(q.Query) + if err := s.DefaultRP(ctx, &qc, &src); err != nil { + Error(w, http.StatusBadRequest, err.Error(), s.Logger) + return + } + qr.QueryConfig = qc + if stmt, err := queries.ParseSelect(q.Query); err == nil { qr.QueryAST = stmt } + qr.QueryConfig.ID = q.ID res.Queries[i] = qr } encodeJSON(w, http.StatusOK, res, s.Logger) } + +// DefaultRP will add the default retention policy to the QC if one has not been specified +func (s *Service) DefaultRP(ctx context.Context, qc *chronograf.QueryConfig, src *chronograf.Source) error { + // Only need to find the default RP IFF the qc's rp is empty + if qc.RetentionPolicy != "" { + return nil + } + + // For queries without databases, measurements, or fields we will not + // be able to find an RP + if qc.Database == "" || qc.Measurement == "" || len(qc.Fields) == 0 { + return nil + } + + db := s.Databases + if err := db.Connect(ctx, src); err != nil { + return fmt.Errorf("Unable to connect to source: %v", err) + } + + rps, err := db.AllRP(ctx, qc.Database) + if err != nil { + return fmt.Errorf("Unable to load RPs from DB %s: %v", qc.Database, err) + } + + for _, rp := range rps { + if rp.Default { + qc.RetentionPolicy = rp.Name + return nil + } + } + + return nil +} From 7291d7a5bdb875707eaec675649401cd2882ee81 Mon Sep 17 00:00:00 2001 From: Andrew Watkins Date: Thu, 13 Apr 2017 17:52:18 -0700 Subject: [PATCH 04/16] Make people happy --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index e652c38441..d06c9265c8 100644 --- a/Makefile +++ b/Makefile @@ -29,7 +29,7 @@ define CHRONOGIRAFFE ,"" _\_ ," ## | 0 0. ," ## ,-\__ `. - ," / `--._;) + ," / `--._;) - "HAI, I'm Chronogiraffe. Will you be my friend?" ," ## / ," ## / endef From eeb693896ab9064202c9a564002108f0eb92e26e Mon Sep 17 00:00:00 2001 From: Andrew Watkins Date: Thu, 13 Apr 2017 17:57:09 -0700 Subject: [PATCH 05/16] Update CHANGELOG to correct versions --- CHANGELOG.md | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b80a955d33..abd54f2c56 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,7 +1,13 @@ ## v1.2.0 [unreleased] ### Bug Fixes + 1. [#1257](https://github.com/influxdata/chronograf/issues/1257): Fix function selection in query builder + 1. [#1244](https://github.com/influxdata/chronograf/pull/1244): Fix env var name for Google client secret + 1. [#1269](https://github.com/influxdata/chronograf/issues/1269): Add more functionality to query config generation + ### Features + 1. [#1232](https://github.com/influxdata/chronograf/pull/1232): Fuse the query builder and raw query editor + ### UI Improvements 1. [#1259](https://github.com/influxdata/chronograf/pull/1259): Add default display for empty dashboard 1. [#1258](https://github.com/influxdata/chronograf/pull/1258): Display Kapacitor alert endpoint options as radio button group @@ -31,8 +37,6 @@ 1. [#1209](https://github.com/influxdata/chronograf/pull/1209): Ask for the HipChat subdomain instead of the entire HipChat URL in the HipChat Kapacitor configuration 1. [#1223](https://github.com/influxdata/chronograf/pull/1223): Use vhost as Chronograf's proxy to Kapacitor 1. [#1205](https://github.com/influxdata/chronograf/pull/1205): Allow initial source to be an InfluxEnterprise source - 1. [#1244](https://github.com/influxdata/chronograf/pull/1244): Fix env var name for Google client secret - 1. [#1257](https://github.com/influxdata/chronograf/issues/1257): Fix function selection in query builder ### Features 1. [#1112](https://github.com/influxdata/chronograf/pull/1112): Add ability to delete a dashboard @@ -46,7 +50,6 @@ 1. [#1212](https://github.com/influxdata/chronograf/pull/1212): Add meta query templates and loading animation to the RawQueryEditor 1. [#1221](https://github.com/influxdata/chronograf/pull/1221): Remove the default query from empty cells on dashboards 1. [#1101](https://github.com/influxdata/chronograf/pull/1101): Compress InfluxQL responses with gzip - 1. [#1232](https://github.com/influxdata/chronograf/pull/1232): Fuse the query builder and raw query editor ### UI Improvements 1. [#1132](https://github.com/influxdata/chronograf/pull/1132): Show blue strip next to active tab on the sidebar From 5b692bdef3f41aceb132ba75af4b49bed367e820 Mon Sep 17 00:00:00 2001 From: Chris Goller Date: Fri, 14 Apr 2017 02:12:52 -0500 Subject: [PATCH 06/16] 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 7bc701cfb2..698198cd22 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 e925c3c4c9..c65e56e8b5 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 0236ecdffc..8b7fc302f0 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 e7757573c4..d2ba0ef6ea 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 da5cda9d91..3636b31e38 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 1fabed503963f64c0b29861843f485961cb95cd4 Mon Sep 17 00:00:00 2001 From: Chris Goller Date: Fri, 14 Apr 2017 02:35:30 -0500 Subject: [PATCH 07/16] 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 e2303aafa6..3803b3366e 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 987b06532f..b85dfb6149 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 acdf81c013..7d4fc41d27 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 a5bc026cd5..9692e50215 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 6dd7336e55..fc15d7f3f9 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 f95e6f451c23a1f19d50871592dfd79980923c73 Mon Sep 17 00:00:00 2001 From: Chris Goller Date: Mon, 17 Apr 2017 10:38:46 -0500 Subject: [PATCH 08/16] 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 0000000000..826a287575 --- /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 f6aedc85a0390b42c08c2df02617fa1fdcc7d797 Mon Sep 17 00:00:00 2001 From: Chris Goller Date: Mon, 17 Apr 2017 10:39:01 -0500 Subject: [PATCH 09/16] 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 698198cd22..8a5e80489c 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 c65e56e8b5..d14bd7782a 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 8b7fc302f0..52537ac5b7 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 778cb19f570a7103e9503a88c33f1f58dcf5c396 Mon Sep 17 00:00:00 2001 From: Chris Goller Date: Mon, 17 Apr 2017 10:57:33 -0500 Subject: [PATCH 10/16] 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 d14bd7782a..0eab590bb4 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 826a287575..529e1c4b70 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 45e9508d311cdde2be1b6ba9450b772a50f28605 Mon Sep 17 00:00:00 2001 From: Chris Goller Date: Mon, 17 Apr 2017 11:49:45 -0500 Subject: [PATCH 11/16] 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 8a5e80489c..980ec1d307 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 3803b3366e..73ea86db94 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 d2ba0ef6ea..58668485e2 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 9692e50215..48e4929ce0 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 3636b31e38..eb58681094 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 fc15d7f3f9..a0d482de36 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 89ef827f94e7b12956c53c9a164a7556aa205201 Mon Sep 17 00:00:00 2001 From: Chris Goller Date: Mon, 17 Apr 2017 12:22:49 -0500 Subject: [PATCH 12/16] 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 980ec1d307..f87e839ba4 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 9b40e397753116b88dff15813f94ca7593e01518 Mon Sep 17 00:00:00 2001 From: Chris Goller Date: Mon, 17 Apr 2017 14:49:05 -0500 Subject: [PATCH 13/16] 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 73ea86db94..cfa47bcc5e 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 b85dfb6149..992522cda1 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 87e87d902aa9ecdc3315e68344547f35b9ce9a0a Mon Sep 17 00:00:00 2001 From: Chris Goller Date: Mon, 17 Apr 2017 15:25:07 -0500 Subject: [PATCH 14/16] Update CHANGELOG to mention refreshing JWTs --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index abd54f2c56..8a7776913b 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 From f17cf6bb0966fd41e5077a0acb207e7a99f2e876 Mon Sep 17 00:00:00 2001 From: Chris Goller Date: Wed, 19 Apr 2017 11:03:53 -0500 Subject: [PATCH 15/16] Add InfluxQL template rendering --- influx/templates.go | 53 +++++++++++++++ influx/templates_test.go | 139 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 192 insertions(+) create mode 100644 influx/templates.go create mode 100644 influx/templates_test.go diff --git a/influx/templates.go b/influx/templates.go new file mode 100644 index 0000000000..24f5338baa --- /dev/null +++ b/influx/templates.go @@ -0,0 +1,53 @@ +package influx + +import "strings" + +// TempValue is a value use to replace a template in an InfluxQL query +type TempValue struct { + Value string `json:"value"` + Type string `json:"type"` +} + +// TempVar is a named variable within an InfluxQL query to be replaced with Values +type TempVar struct { + Var string `json:"tempVar"` + Values []TempValue `json:"values"` +} + +// String converts the template variable into a correct InfluxQL string based +// on its type +func (t TempVar) String() string { + if len(t.Values) == 0 { + return "" + } + switch t.Values[0].Type { + case "tagKey", "fieldKey": + return `"` + t.Values[0].Value + `"` + case "tagValue": + return `'` + t.Values[0].Value + `'` + case "csv": + return t.Values[0].Value + default: + return "" + } +} + +// TempVars are template variables to replace within an InfluxQL query +type TempVars struct { + Vars []TempVar `json:"tempVars"` +} + +// TemplateReplace replaces templates with values within the query string +func TemplateReplace(query string, templates TempVars) string { + replacements := []string{} + for _, v := range templates.Vars { + newVal := v.String() + if newVal != "" { + replacements = append(replacements, v.Var, newVal) + } + } + + replacer := strings.NewReplacer(replacements...) + replaced := replacer.Replace(query) + return replaced +} diff --git a/influx/templates_test.go b/influx/templates_test.go new file mode 100644 index 0000000000..ba0ba26407 --- /dev/null +++ b/influx/templates_test.go @@ -0,0 +1,139 @@ +package influx + +import ( + "testing" +) + +func TestTemplateReplace(t *testing.T) { + tests := []struct { + name string + query string + vars TempVars + want string + }{ + { + name: "select with parameters", + query: "$METHOD field1, $field FROM $measurement WHERE temperature > $temperature", + vars: TempVars{ + Vars: []TempVar{ + { + Var: "$temperature", + Values: []TempValue{ + { + Type: "csv", + Value: "10", + }, + }, + }, + { + Var: "$field", + Values: []TempValue{ + { + Type: "fieldKey", + Value: "field2", + }, + }, + }, + { + Var: "$METHOD", + Values: []TempValue{ + { + Type: "csv", + Value: "SELECT", + }, + }, + }, + { + Var: "$measurement", + Values: []TempValue{ + { + Type: "csv", + Value: `"cpu"`, + }, + }, + }, + }, + }, + want: `SELECT field1, "field2" FROM "cpu" WHERE temperature > 10`, + }, + { + name: "select with parameters and aggregates", + query: `SELECT mean($field) FROM "cpu" WHERE $tag = $value GROUP BY $tag`, + vars: TempVars{ + Vars: []TempVar{ + { + Var: "$value", + Values: []TempValue{ + { + Type: "tagValue", + Value: "howdy.com", + }, + }, + }, + { + Var: "$tag", + Values: []TempValue{ + { + Type: "tagKey", + Value: "host", + }, + }, + }, + { + Var: "$field", + Values: []TempValue{ + { + Type: "fieldKey", + Value: "field", + }, + }, + }, + }, + }, + want: `SELECT mean("field") FROM "cpu" WHERE "host" = 'howdy.com' GROUP BY "host"`, + }, + { + name: "Non-existant parameters", + query: `SELECT $field FROM "cpu"`, + want: `SELECT $field FROM "cpu"`, + }, + { + name: "var without a value", + query: `SELECT $field FROM "cpu"`, + vars: TempVars{ + Vars: []TempVar{ + { + Var: "$field", + }, + }, + }, + want: `SELECT $field FROM "cpu"`, + }, + { + name: "var with unknown type", + query: `SELECT $field FROM "cpu"`, + vars: TempVars{ + Vars: []TempVar{ + { + Var: "$field", + Values: []TempValue{ + { + Type: "who knows?", + Value: "field", + }, + }, + }, + }, + }, + want: `SELECT $field FROM "cpu"`, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := TemplateReplace(tt.query, tt.vars) + if got != tt.want { + t.Errorf("TestParse %s =\n%s\nwant\n%s", tt.name, got, tt.want) + } + }) + } +} From c4465863da0c72ee1854404d175ddb0a92f1b001 Mon Sep 17 00:00:00 2001 From: Chris Goller Date: Wed, 19 Apr 2017 11:08:14 -0500 Subject: [PATCH 16/16] Update template variable naming to Template rather than Temp --- influx/templates.go | 22 +++++++++++----------- influx/templates_test.go | 34 +++++++++++++++++----------------- 2 files changed, 28 insertions(+), 28 deletions(-) diff --git a/influx/templates.go b/influx/templates.go index 24f5338baa..3cdded2159 100644 --- a/influx/templates.go +++ b/influx/templates.go @@ -2,21 +2,21 @@ package influx import "strings" -// TempValue is a value use to replace a template in an InfluxQL query -type TempValue struct { +// TemplateValue is a value use to replace a template in an InfluxQL query +type TemplateValue struct { Value string `json:"value"` Type string `json:"type"` } -// TempVar is a named variable within an InfluxQL query to be replaced with Values -type TempVar struct { - Var string `json:"tempVar"` - Values []TempValue `json:"values"` +// TemplateVar is a named variable within an InfluxQL query to be replaced with Values +type TemplateVar struct { + Var string `json:"tempVar"` + Values []TemplateValue `json:"values"` } // String converts the template variable into a correct InfluxQL string based // on its type -func (t TempVar) String() string { +func (t TemplateVar) String() string { if len(t.Values) == 0 { return "" } @@ -32,13 +32,13 @@ func (t TempVar) String() string { } } -// TempVars are template variables to replace within an InfluxQL query -type TempVars struct { - Vars []TempVar `json:"tempVars"` +// TemplateVars are template variables to replace within an InfluxQL query +type TemplateVars struct { + Vars []TemplateVar `json:"tempVars"` } // TemplateReplace replaces templates with values within the query string -func TemplateReplace(query string, templates TempVars) string { +func TemplateReplace(query string, templates TemplateVars) string { replacements := []string{} for _, v := range templates.Vars { newVal := v.String() diff --git a/influx/templates_test.go b/influx/templates_test.go index ba0ba26407..794b8522ee 100644 --- a/influx/templates_test.go +++ b/influx/templates_test.go @@ -8,17 +8,17 @@ func TestTemplateReplace(t *testing.T) { tests := []struct { name string query string - vars TempVars + vars TemplateVars want string }{ { name: "select with parameters", query: "$METHOD field1, $field FROM $measurement WHERE temperature > $temperature", - vars: TempVars{ - Vars: []TempVar{ + vars: TemplateVars{ + Vars: []TemplateVar{ { Var: "$temperature", - Values: []TempValue{ + Values: []TemplateValue{ { Type: "csv", Value: "10", @@ -27,7 +27,7 @@ func TestTemplateReplace(t *testing.T) { }, { Var: "$field", - Values: []TempValue{ + Values: []TemplateValue{ { Type: "fieldKey", Value: "field2", @@ -36,7 +36,7 @@ func TestTemplateReplace(t *testing.T) { }, { Var: "$METHOD", - Values: []TempValue{ + Values: []TemplateValue{ { Type: "csv", Value: "SELECT", @@ -45,7 +45,7 @@ func TestTemplateReplace(t *testing.T) { }, { Var: "$measurement", - Values: []TempValue{ + Values: []TemplateValue{ { Type: "csv", Value: `"cpu"`, @@ -59,11 +59,11 @@ func TestTemplateReplace(t *testing.T) { { name: "select with parameters and aggregates", query: `SELECT mean($field) FROM "cpu" WHERE $tag = $value GROUP BY $tag`, - vars: TempVars{ - Vars: []TempVar{ + vars: TemplateVars{ + Vars: []TemplateVar{ { Var: "$value", - Values: []TempValue{ + Values: []TemplateValue{ { Type: "tagValue", Value: "howdy.com", @@ -72,7 +72,7 @@ func TestTemplateReplace(t *testing.T) { }, { Var: "$tag", - Values: []TempValue{ + Values: []TemplateValue{ { Type: "tagKey", Value: "host", @@ -81,7 +81,7 @@ func TestTemplateReplace(t *testing.T) { }, { Var: "$field", - Values: []TempValue{ + Values: []TemplateValue{ { Type: "fieldKey", Value: "field", @@ -100,8 +100,8 @@ func TestTemplateReplace(t *testing.T) { { name: "var without a value", query: `SELECT $field FROM "cpu"`, - vars: TempVars{ - Vars: []TempVar{ + vars: TemplateVars{ + Vars: []TemplateVar{ { Var: "$field", }, @@ -112,11 +112,11 @@ func TestTemplateReplace(t *testing.T) { { name: "var with unknown type", query: `SELECT $field FROM "cpu"`, - vars: TempVars{ - Vars: []TempVar{ + vars: TemplateVars{ + Vars: []TemplateVar{ { Var: "$field", - Values: []TempValue{ + Values: []TemplateValue{ { Type: "who knows?", Value: "field",