From a3e38a68c99edb54b5d5c19ca34f6148f2659c49 Mon Sep 17 00:00:00 2001 From: Chris Goller Date: Mon, 24 Jul 2017 22:01:32 -0500 Subject: [PATCH 1/4] Update influx Authorization Headers for write and query path --- influx/authorization.go | 93 +++++++++++++++++++ influx/{jwt_test.go => authorization_test.go} | 0 influx/jwt.go | 41 -------- 3 files changed, 93 insertions(+), 41 deletions(-) create mode 100644 influx/authorization.go rename influx/{jwt_test.go => authorization_test.go} (100%) delete mode 100644 influx/jwt.go diff --git a/influx/authorization.go b/influx/authorization.go new file mode 100644 index 0000000000..2f5b7f3e43 --- /dev/null +++ b/influx/authorization.go @@ -0,0 +1,93 @@ +package influx + +import ( + "fmt" + "net/http" + "time" + + jwt "github.com/dgrijalva/jwt-go" + "github.com/influxdata/chronograf" +) + +// Authorization adds optional authorization header to request +type Authorization interface { + // Set may manipulate the request by adding the Authorization header + Set(req *http.Request) error +} + +// NoAuthorization does not add any authorization headers +type NoAuthorization struct{} + +// Set does not add authorization +func (n *NoAuthorization) Set(req *http.Request) error { return nil } + +// DefaultAuthorization creates either a shared JWT builder, basic auth or Noop +func DefaultAuthorization(src *chronograf.Source) Authorization { + // Optionally, add the shared secret JWT token creation + if src.Username != "" && src.SharedSecret != "" { + return &BearerJWT{ + Username: src.Username, + SharedSecret: src.SharedSecret, + } + } else if src.Username != "" && src.Password != "" { + return &BasicAuth{ + Username: src.Username, + Password: src.Password, + } + } + return &NoAuthorization{} +} + +// BasicAuth adds Authorization: Basic to the request header +type BasicAuth struct { + Username string + Password string +} + +// Set adds the basic auth headers to the request +func (b *BasicAuth) Set(r *http.Request) error { + r.SetBasicAuth(b.Username, b.Password) + return nil +} + +// BearerJWT is the default Bearer for InfluxDB +type BearerJWT struct { + Username string + SharedSecret string +} + +// Set adds an Authorization Bearer to the request if has a shared secret +func (b *BearerJWT) Set(r *http.Request) error { + if b.SharedSecret != "" && b.Username != "" { + token, err := b.Token(b.Username) + if err != nil { + return fmt.Errorf("Unable to create token") + } + r.Header.Set("Authorization", "Bearer "+token) + } + return nil +} + +// Token returns the expected InfluxDB JWT signed with the sharedSecret +func (b *BearerJWT) Token(username string) (string, error) { + return JWT(username, b.SharedSecret, time.Now) +} + +// Now returns the current time +type Now func() time.Time + +// JWT returns a token string accepted by InfluxDB using the sharedSecret as an Authorization: Bearer header +func JWT(username, sharedSecret string, now Now) (string, error) { + token := &jwt.Token{ + Header: map[string]interface{}{ + "typ": "JWT", + "alg": jwt.SigningMethodHS512.Alg(), + }, + Claims: jwt.MapClaims{ + "username": username, + "exp": now().Add(time.Minute).Unix(), + }, + Method: jwt.SigningMethodHS512, + } + return token.SignedString([]byte(sharedSecret)) +} diff --git a/influx/jwt_test.go b/influx/authorization_test.go similarity index 100% rename from influx/jwt_test.go rename to influx/authorization_test.go diff --git a/influx/jwt.go b/influx/jwt.go deleted file mode 100644 index 7d89b21392..0000000000 --- a/influx/jwt.go +++ /dev/null @@ -1,41 +0,0 @@ -package influx - -import ( - "time" - - jwt "github.com/dgrijalva/jwt-go" -) - -// Bearer generates tokens for Authorization: Bearer -type Bearer interface { - Token(username string) (string, error) -} - -// BearerJWT is the default Bearer for InfluxDB -type BearerJWT struct { - SharedSecret string -} - -// Token returns the expected InfluxDB JWT signed with the sharedSecret -func (b *BearerJWT) Token(username string) (string, error) { - return JWT(username, b.SharedSecret, time.Now) -} - -// Now returns the current time -type Now func() time.Time - -// JWT returns a token string accepted by InfluxDB using the sharedSecret as an Authorization: Bearer header -func JWT(username, sharedSecret string, now Now) (string, error) { - token := &jwt.Token{ - Header: map[string]interface{}{ - "typ": "JWT", - "alg": jwt.SigningMethodHS512.Alg(), - }, - Claims: jwt.MapClaims{ - "username": username, - "exp": now().Add(time.Minute).Unix(), - }, - Method: jwt.SigningMethodHS512, - } - return token.SignedString([]byte(sharedSecret)) -} From cef0e66df0d60433d6a798838c133165c402a4f1 Mon Sep 17 00:00:00 2001 From: Chris Goller Date: Mon, 24 Jul 2017 22:01:32 -0500 Subject: [PATCH 2/4] Update influx Authorization Headers for write and query path --- influx/influx.go | 25 ++++++---------------- influx/influx_test.go | 50 +++++++++++++++++++++++++++---------------- server/influx.go | 8 +++---- 3 files changed, 43 insertions(+), 40 deletions(-) diff --git a/influx/influx.go b/influx/influx.go index c1f0baef9e..13aa040610 100644 --- a/influx/influx.go +++ b/influx/influx.go @@ -28,7 +28,7 @@ var ( // Client is a device for retrieving time series data from an InfluxDB instance type Client struct { URL *url.URL - Bearer Bearer + Authorization Authorization InsecureSkipVerify bool Logger chronograf.Logger } @@ -72,13 +72,11 @@ func (c *Client) query(u *url.URL, q chronograf.Query) (chronograf.Response, err params.Set("epoch", "ms") // TODO(timraymond): set this based on analysis req.URL.RawQuery = params.Encode() - if c.Bearer != nil && u.User != nil { - token, err := c.Bearer.Token(u.User.Username()) - if err != nil { - logs.Error("Error creating token", err) - return nil, fmt.Errorf("Unable to create token") + if c.Authorization != nil { + if err := c.Authorization.Set(req); err != nil { + logs.Error("Error setting authorization header ", err) + return nil, err } - req.Header.Set("Authorization", "Bearer "+token) } hc := &http.Client{} @@ -156,22 +154,13 @@ func (c *Client) Connect(ctx context.Context, src *chronograf.Source) error { if err != nil { return err } - u.User = url.UserPassword(src.Username, src.Password) + c.Authorization = DefaultAuthorization(src) // Only allow acceptance of all certs if the scheme is https AND the user opted into to the setting. if u.Scheme == "https" && src.InsecureSkipVerify { c.InsecureSkipVerify = src.InsecureSkipVerify } - c.URL = u - // Optionally, add the shared secret JWT token creation - if src.Username != "" && src.SharedSecret != "" { - c.Bearer = &BearerJWT{ - src.SharedSecret, - } - } else { - // Clear out the bearer if not needed - c.Bearer = nil - } + c.URL = u return nil } diff --git a/influx/influx_test.go b/influx/influx_test.go index 3218681a18..e2c7f532bb 100644 --- a/influx/influx_test.go +++ b/influx/influx_test.go @@ -62,38 +62,52 @@ func Test_Influx_MakesRequestsToQueryEndpoint(t *testing.T) { } } -type MockBearer struct { +type MockAuthorization struct { Bearer string Error error } -func (m *MockBearer) Token(username string) (string, error) { - return m.Bearer, m.Error +func (m *MockAuthorization) Set(req *http.Request) error { + return m.Error } func Test_Influx_AuthorizationBearer(t *testing.T) { t.Parallel() - want := "Bearer ********" ts := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { rw.WriteHeader(http.StatusOK) rw.Write([]byte(`{}`)) - got := r.Header.Get("Authorization") - if got != want { - t.Errorf("Test_Influx_AuthorizationBearer got %s want %s", got, want) + auth := r.Header.Get("Authorization") + tokenString := strings.Split(auth, " ")[1] + token, err := gojwt.Parse(tokenString, func(token *gojwt.Token) (interface{}, error) { + if _, ok := token.Method.(*gojwt.SigningMethodHMAC); !ok { + return nil, fmt.Errorf("Unexpected signing method: %v", token.Header["alg"]) + } + return []byte("42"), nil + }) + if err != nil { + t.Errorf("Invalid token %v", err) } + + if claims, ok := token.Claims.(gojwt.MapClaims); ok && token.Valid { + got := claims["username"] + want := "AzureDiamond" + if got != want { + t.Errorf("Test_Influx_AuthorizationBearer got %s want %s", got, want) + } + return + } + t.Errorf("Invalid token %v", token) })) defer ts.Close() - bearer := &MockBearer{ - Bearer: "********", + src := &chronograf.Source{ + Username: "AzureDiamond", + URL: ts.URL, + SharedSecret: "42", } - - u, _ := url.Parse(ts.URL) - u.User = url.UserPassword("AzureDiamond", "hunter2") series := &influx.Client{ - URL: u, - Bearer: bearer, Logger: log.New(log.DebugLevel), } + series.Connect(context.Background(), src) query := chronograf.Query{ Command: "show databases", @@ -161,16 +175,16 @@ func Test_Influx_AuthorizationBearerCtx(t *testing.T) { func Test_Influx_AuthorizationBearerFailure(t *testing.T) { t.Parallel() - bearer := &MockBearer{ + bearer := &MockAuthorization{ Error: fmt.Errorf("cracked1337"), } u, _ := url.Parse("http://haxored.net") u.User = url.UserPassword("AzureDiamond", "hunter2") series := &influx.Client{ - URL: u, - Bearer: bearer, - Logger: log.New(log.DebugLevel), + URL: u, + Authorization: bearer, + Logger: log.New(log.DebugLevel), } query := chronograf.Query{ diff --git a/server/influx.go b/server/influx.go index ce3145134a..1d2dd84831 100644 --- a/server/influx.go +++ b/server/influx.go @@ -8,6 +8,7 @@ import ( "net/url" "github.com/influxdata/chronograf" + "github.com/influxdata/chronograf/influx" ) // ValidInfluxRequest checks if queries specify a command. @@ -106,10 +107,9 @@ func (h *Service) Write(w http.ResponseWriter, r *http.Request) { req.Host = u.Host req.URL = u // Because we are acting as a proxy, influxdb needs to have the - // basic auth information set as a header directly - if src.Username != "" && src.Password != "" { - req.SetBasicAuth(src.Username, src.Password) - } + // basic auth or bearer token information set as a header directly + auth := influx.DefaultAuthorization(&src) + auth.Set(req) } proxy := &httputil.ReverseProxy{ Director: director, From 4720df245b18042d860bf4c88e07b885178273b0 Mon Sep 17 00:00:00 2001 From: Nathan Haugo Date: Tue, 25 Jul 2017 09:57:01 -0500 Subject: [PATCH 3/4] Update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3f6787643f..c8c88654e1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ 1. [#1751](https://github.com/influxdata/chronograf/pull/1751): Fix typo that may have affected PagerDuty node creation in Kapacitor 1. [#1756](https://github.com/influxdata/chronograf/pull/1756): Prevent 'auto' GROUP BY as option in Kapacitor rule builder when applying a function to a field 1. [#1773](https://github.com/influxdata/chronograf/pull/1773): Prevent clipped buttons in Rule Builder, Data Explorer, and Configuration pages +1. [#1776](https://github.com/influxdata/chronograf/pull/1776): Fix JWT for the write path ### Features 1. [#1717](https://github.com/influxdata/chronograf/pull/1717): View server generated TICKscripts From 38f172f84d4fc6a726781b19f685ff70da313b9b Mon Sep 17 00:00:00 2001 From: Chris Goller Date: Tue, 25 Jul 2017 10:03:23 -0500 Subject: [PATCH 4/4] Update interface Authorization to Authorizer --- influx/authorization.go | 6 +++--- influx/influx.go | 8 ++++---- influx/influx_test.go | 6 +++--- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/influx/authorization.go b/influx/authorization.go index 2f5b7f3e43..86ed486654 100644 --- a/influx/authorization.go +++ b/influx/authorization.go @@ -9,8 +9,8 @@ import ( "github.com/influxdata/chronograf" ) -// Authorization adds optional authorization header to request -type Authorization interface { +// Authorizer adds optional authorization header to request +type Authorizer interface { // Set may manipulate the request by adding the Authorization header Set(req *http.Request) error } @@ -22,7 +22,7 @@ type NoAuthorization struct{} func (n *NoAuthorization) Set(req *http.Request) error { return nil } // DefaultAuthorization creates either a shared JWT builder, basic auth or Noop -func DefaultAuthorization(src *chronograf.Source) Authorization { +func DefaultAuthorization(src *chronograf.Source) Authorizer { // Optionally, add the shared secret JWT token creation if src.Username != "" && src.SharedSecret != "" { return &BearerJWT{ diff --git a/influx/influx.go b/influx/influx.go index 13aa040610..8bc92c09ea 100644 --- a/influx/influx.go +++ b/influx/influx.go @@ -28,7 +28,7 @@ var ( // Client is a device for retrieving time series data from an InfluxDB instance type Client struct { URL *url.URL - Authorization Authorization + Authorizer Authorizer InsecureSkipVerify bool Logger chronograf.Logger } @@ -72,8 +72,8 @@ func (c *Client) query(u *url.URL, q chronograf.Query) (chronograf.Response, err params.Set("epoch", "ms") // TODO(timraymond): set this based on analysis req.URL.RawQuery = params.Encode() - if c.Authorization != nil { - if err := c.Authorization.Set(req); err != nil { + if c.Authorizer != nil { + if err := c.Authorizer.Set(req); err != nil { logs.Error("Error setting authorization header ", err) return nil, err } @@ -154,7 +154,7 @@ func (c *Client) Connect(ctx context.Context, src *chronograf.Source) error { if err != nil { return err } - c.Authorization = DefaultAuthorization(src) + c.Authorizer = DefaultAuthorization(src) // Only allow acceptance of all certs if the scheme is https AND the user opted into to the setting. if u.Scheme == "https" && src.InsecureSkipVerify { c.InsecureSkipVerify = src.InsecureSkipVerify diff --git a/influx/influx_test.go b/influx/influx_test.go index e2c7f532bb..a6dce9e118 100644 --- a/influx/influx_test.go +++ b/influx/influx_test.go @@ -182,9 +182,9 @@ func Test_Influx_AuthorizationBearerFailure(t *testing.T) { u, _ := url.Parse("http://haxored.net") u.User = url.UserPassword("AzureDiamond", "hunter2") series := &influx.Client{ - URL: u, - Authorization: bearer, - Logger: log.New(log.DebugLevel), + URL: u, + Authorizer: bearer, + Logger: log.New(log.DebugLevel), } query := chronograf.Query{