From 16ff8a182bf2823bcc2435e7929a86706f4165f3 Mon Sep 17 00:00:00 2001 From: Bob Shannon Date: Mon, 19 Nov 2018 13:15:18 -0500 Subject: [PATCH] Re-organize packages and add basic auth test --- pkg/{util/auth.go => api/basic_auth.go} | 4 +-- pkg/api/basic_auth_test.go | 45 +++++++++++++++++++++++++ pkg/api/http_server.go | 9 ++--- pkg/api/http_server_test.go | 30 +++++++++++++++++ 4 files changed, 82 insertions(+), 6 deletions(-) rename pkg/{util/auth.go => api/basic_auth.go} (84%) create mode 100644 pkg/api/basic_auth_test.go create mode 100644 pkg/api/http_server_test.go diff --git a/pkg/util/auth.go b/pkg/api/basic_auth.go similarity index 84% rename from pkg/util/auth.go rename to pkg/api/basic_auth.go index 723cc79e244..376cfb24c91 100644 --- a/pkg/util/auth.go +++ b/pkg/api/basic_auth.go @@ -1,11 +1,11 @@ -package util +package api import ( "crypto/subtle" macaron "gopkg.in/macaron.v1" ) -// BasicAuthenticated parses the provided HTTP request for basic authentication credentials +// BasicAuthenticatedRequest parses the provided HTTP request for basic authentication credentials // and returns true if the provided credentials match the expected username and password. // Returns false if the request is unauthenticated. // Uses constant-time comparison in order to mitigate timing attacks. diff --git a/pkg/api/basic_auth_test.go b/pkg/api/basic_auth_test.go new file mode 100644 index 00000000000..0b5051c3e2a --- /dev/null +++ b/pkg/api/basic_auth_test.go @@ -0,0 +1,45 @@ +package api + +import ( + "encoding/base64" + "fmt" + "net/http" + "testing" + + . "github.com/smartystreets/goconvey/convey" + "gopkg.in/macaron.v1" +) + +func TestBasicAuthenticatedRequest(t *testing.T) { + expectedUser := "prometheus" + expectedPass := "password" + + Convey("Given a valid set of basic auth credentials", t, func() { + httpReq, err := http.NewRequest("GET", "http://localhost:3000/metrics", nil) + So(err, ShouldBeNil) + req := macaron.Request{ + Request: httpReq, + } + encodedCreds := encodeBasicAuthCredentials(expectedUser, expectedPass) + req.Header.Add("Authorization", fmt.Sprintf("Basic %s", encodedCreds)) + authenticated := BasicAuthenticatedRequest(req, expectedUser, expectedPass) + So(authenticated, ShouldBeTrue) + }) + + Convey("Given an invalid set of basic auth credentials", t, func() { + httpReq, err := http.NewRequest("GET", "http://localhost:3000/metrics", nil) + So(err, ShouldBeNil) + req := macaron.Request{ + Request: httpReq, + } + encodedCreds := encodeBasicAuthCredentials("invaliduser", "invalidpass") + req.Header.Add("Authorization", fmt.Sprintf("Basic %s", encodedCreds)) + authenticated := BasicAuthenticatedRequest(req, expectedUser, expectedPass) + So(authenticated, ShouldBeFalse) + }) +} + +func encodeBasicAuthCredentials(user, pass string) string { + creds := fmt.Sprintf("%s:%s", user, pass) + return base64.StdEncoding.EncodeToString([]byte(creds)) +} diff --git a/pkg/api/http_server.go b/pkg/api/http_server.go index 70feb44268d..d4d7b41bec5 100644 --- a/pkg/api/http_server.go +++ b/pkg/api/http_server.go @@ -32,7 +32,6 @@ import ( "github.com/grafana/grafana/pkg/services/hooks" "github.com/grafana/grafana/pkg/services/rendering" "github.com/grafana/grafana/pkg/setting" - "github.com/grafana/grafana/pkg/util" ) func init() { @@ -246,9 +245,7 @@ func (hs *HTTPServer) metricsEndpoint(ctx *macaron.Context) { return } - if hs.Cfg.MetricsEndpointBasicAuthUsername != "" && - hs.Cfg.MetricsEndpointBasicAuthPassword != "" && - !util.BasicAuthenticatedRequest(ctx.Req, hs.Cfg.MetricsEndpointBasicAuthUsername, hs.Cfg.MetricsEndpointBasicAuthPassword) { + if hs.metricsEndpointBasicAuthEnabled() && !BasicAuthenticatedRequest(ctx.Req, hs.Cfg.MetricsEndpointBasicAuthUsername, hs.Cfg.MetricsEndpointBasicAuthPassword) { ctx.Resp.WriteHeader(http.StatusUnauthorized) return } @@ -307,3 +304,7 @@ func (hs *HTTPServer) mapStatic(m *macaron.Macaron, rootDir string, dir string, }, )) } + +func (hs *HTTPServer) metricsEndpointBasicAuthEnabled() bool { + return hs.Cfg.MetricsEndpointBasicAuthUsername != "" && hs.Cfg.MetricsEndpointBasicAuthPassword != "" +} diff --git a/pkg/api/http_server_test.go b/pkg/api/http_server_test.go new file mode 100644 index 00000000000..0f99ae82db5 --- /dev/null +++ b/pkg/api/http_server_test.go @@ -0,0 +1,30 @@ +package api + +import ( + "testing" + + "github.com/grafana/grafana/pkg/setting" + . "github.com/smartystreets/goconvey/convey" +) + +func TestHTTPServer(t *testing.T) { + Convey("Given a HTTPServer", t, func() { + ts := &HTTPServer{ + Cfg: setting.NewCfg(), + } + + Convey("Given that basic auth on the metrics endpoint is enabled", func() { + ts.Cfg.MetricsEndpointBasicAuthUsername = "foo" + ts.Cfg.MetricsEndpointBasicAuthPassword = "bar" + + So(ts.metricsEndpointBasicAuthEnabled(), ShouldBeTrue) + }) + + Convey("Given that basic auth on the metrics endpoint is disabled", func() { + ts.Cfg.MetricsEndpointBasicAuthUsername = "" + ts.Cfg.MetricsEndpointBasicAuthPassword = "" + + So(ts.metricsEndpointBasicAuthEnabled(), ShouldBeFalse) + }) + }) +}