From f778c1d971c7a738758a587309eb2cde88d01e44 Mon Sep 17 00:00:00 2001 From: Marcus Efraimsson Date: Mon, 6 May 2019 09:22:59 +0200 Subject: [PATCH] Security: Responses from backend should not be cached (#16848) Currently all API requests set Cache-control: no-cache to avoid browsers caching sensitive data. This fixes so that all responses returned from backend not are cached using http headers. The exception is the data proxy where we don't add these http headers in case datasource backend needs to control whether data can be cached or not. Fixes #16845 --- pkg/api/http_server.go | 3 ++- pkg/middleware/middleware.go | 19 +++++++++++++------ pkg/middleware/middleware_test.go | 27 +++++++++++++++++++++++---- pkg/middleware/recovery_test.go | 2 +- 4 files changed, 39 insertions(+), 12 deletions(-) diff --git a/pkg/api/http_server.go b/pkg/api/http_server.go index 7c84d579e4a..4f368d8bb0a 100644 --- a/pkg/api/http_server.go +++ b/pkg/api/http_server.go @@ -225,6 +225,8 @@ func (hs *HTTPServer) addMiddlewaresAndStaticRoutes() { hs.mapStatic(m, hs.Cfg.ImagesDir, "", "/public/img/attachments") } + m.Use(middleware.AddDefaultResponseHeaders()) + m.Use(macaron.Renderer(macaron.RenderOptions{ Directory: path.Join(setting.StaticRootPath, "views"), IndentJSON: macaron.Env != macaron.PROD, @@ -245,7 +247,6 @@ func (hs *HTTPServer) addMiddlewaresAndStaticRoutes() { } m.Use(middleware.HandleNoCacheHeader()) - m.Use(middleware.AddDefaultResponseHeaders()) } func (hs *HTTPServer) metricsEndpoint(ctx *macaron.Context) { diff --git a/pkg/middleware/middleware.go b/pkg/middleware/middleware.go index be76c45d89d..63d929f34fa 100644 --- a/pkg/middleware/middleware.go +++ b/pkg/middleware/middleware.go @@ -4,6 +4,7 @@ import ( "net/http" "net/url" "strconv" + "strings" "time" "github.com/grafana/grafana/pkg/bus" @@ -231,11 +232,17 @@ func WriteSessionCookie(ctx *m.ReqContext, value string, maxLifetimeDays int) { } func AddDefaultResponseHeaders() macaron.Handler { - return func(ctx *m.ReqContext) { - if ctx.IsApiRequest() && ctx.Req.Method == "GET" { - ctx.Resp.Header().Add("Cache-Control", "no-cache") - ctx.Resp.Header().Add("Pragma", "no-cache") - ctx.Resp.Header().Add("Expires", "-1") - } + return func(ctx *macaron.Context) { + ctx.Resp.Before(func(w macaron.ResponseWriter) { + if !strings.HasPrefix(ctx.Req.URL.Path, "/api/datasources/proxy/") { + AddNoCacheHeaders(ctx.Resp) + } + }) } } + +func AddNoCacheHeaders(w macaron.ResponseWriter) { + w.Header().Add("Cache-Control", "no-cache") + w.Header().Add("Pragma", "no-cache") + w.Header().Add("Expires", "-1") +} diff --git a/pkg/middleware/middleware_test.go b/pkg/middleware/middleware_test.go index e59e017398a..95d30797739 100644 --- a/pkg/middleware/middleware_test.go +++ b/pkg/middleware/middleware_test.go @@ -10,6 +10,7 @@ import ( "testing" "time" + "github.com/grafana/grafana/pkg/api/dtos" "github.com/grafana/grafana/pkg/bus" "github.com/grafana/grafana/pkg/infra/remotecache" m "github.com/grafana/grafana/pkg/models" @@ -34,16 +35,34 @@ func TestMiddlewareContext(t *testing.T) { So(sc.resp.Code, ShouldEqual, 200) }) - middlewareScenario(t, "middleware should add Cache-Control header for GET requests to API", func(sc *scenarioContext) { + middlewareScenario(t, "middleware should add Cache-Control header for requests to API", func(sc *scenarioContext) { sc.fakeReq("GET", "/api/search").exec() So(sc.resp.Header().Get("Cache-Control"), ShouldEqual, "no-cache") So(sc.resp.Header().Get("Pragma"), ShouldEqual, "no-cache") So(sc.resp.Header().Get("Expires"), ShouldEqual, "-1") }) - middlewareScenario(t, "middleware should not add Cache-Control header to for non-API GET requests", func(sc *scenarioContext) { - sc.fakeReq("GET", "/").exec() + middlewareScenario(t, "middleware should not add Cache-Control header for requests to datasource proxy API", func(sc *scenarioContext) { + sc.fakeReq("GET", "/api/datasources/proxy/1/test").exec() So(sc.resp.Header().Get("Cache-Control"), ShouldBeEmpty) + So(sc.resp.Header().Get("Pragma"), ShouldBeEmpty) + So(sc.resp.Header().Get("Expires"), ShouldBeEmpty) + }) + + middlewareScenario(t, "middleware should add Cache-Control header for GET requests with html response", func(sc *scenarioContext) { + sc.handler(func(c *m.ReqContext) { + data := &dtos.IndexViewData{ + User: &dtos.CurrentUser{}, + Settings: map[string]interface{}{}, + NavTree: []*dtos.NavLink{}, + } + c.HTML(200, "index-template", data) + }) + sc.fakeReq("GET", "/").exec() + So(sc.resp.Code, ShouldEqual, 200) + So(sc.resp.Header().Get("Cache-Control"), ShouldEqual, "no-cache") + So(sc.resp.Header().Get("Pragma"), ShouldEqual, "no-cache") + So(sc.resp.Header().Get("Expires"), ShouldEqual, "-1") }) middlewareScenario(t, "Invalid api key", func(sc *scenarioContext) { @@ -413,6 +432,7 @@ func middlewareScenario(t *testing.T, desc string, fn scenarioFunc) { viewsPath, _ := filepath.Abs("../../public/views") sc.m = macaron.New() + sc.m.Use(AddDefaultResponseHeaders()) sc.m.Use(macaron.Renderer(macaron.RenderOptions{ Directory: viewsPath, Delims: macaron.Delims{Left: "[[", Right: "]]"}, @@ -424,7 +444,6 @@ func middlewareScenario(t *testing.T, desc string, fn scenarioFunc) { sc.m.Use(GetContextHandler(sc.userAuthTokenService, sc.remoteCacheService)) sc.m.Use(OrgRedirect()) - sc.m.Use(AddDefaultResponseHeaders()) sc.defaultHandler = func(c *m.ReqContext) { sc.context = c diff --git a/pkg/middleware/recovery_test.go b/pkg/middleware/recovery_test.go index f1a3dd31cb4..cc20fa6ffd0 100644 --- a/pkg/middleware/recovery_test.go +++ b/pkg/middleware/recovery_test.go @@ -59,6 +59,7 @@ func recoveryScenario(t *testing.T, desc string, url string, fn scenarioFunc) { sc.m = macaron.New() sc.m.Use(Recovery()) + sc.m.Use(AddDefaultResponseHeaders()) sc.m.Use(macaron.Renderer(macaron.RenderOptions{ Directory: viewsPath, Delims: macaron.Delims{Left: "[[", Right: "]]"}, @@ -70,7 +71,6 @@ func recoveryScenario(t *testing.T, desc string, url string, fn scenarioFunc) { sc.m.Use(GetContextHandler(sc.userAuthTokenService, sc.remoteCacheService)) // mock out gc goroutine sc.m.Use(OrgRedirect()) - sc.m.Use(AddDefaultResponseHeaders()) sc.defaultHandler = func(c *m.ReqContext) { sc.context = c