From bd08d8ce8e5fa42969500464be794dea1f27fe8d Mon Sep 17 00:00:00 2001
From: Kyle Brandt <kyle@grafana.com>
Date: Tue, 18 Jun 2019 14:24:23 -0400
Subject: [PATCH] middleware: fix Strict-Transport-Security header (#17644)

fixes #17641
---
 pkg/middleware/middleware.go      |  8 ++++----
 pkg/middleware/middleware_test.go | 33 +++++++++++++++++++++++++++++++
 2 files changed, 37 insertions(+), 4 deletions(-)

diff --git a/pkg/middleware/middleware.go b/pkg/middleware/middleware.go
index bec6df6c7b3..6b71d75f6e0 100644
--- a/pkg/middleware/middleware.go
+++ b/pkg/middleware/middleware.go
@@ -255,14 +255,14 @@ func AddDefaultResponseHeaders() macaron.Handler {
 // AddSecurityHeaders adds various HTTP(S) response headers that enable various security protections behaviors in the client's browser.
 func AddSecurityHeaders(w macaron.ResponseWriter) {
 	if setting.Protocol == setting.HTTPS && setting.StrictTransportSecurity {
-		strictHeader := "Strict-Transport-Security"
-		w.Header().Add(strictHeader, fmt.Sprintf("max-age=%v", setting.StrictTransportSecurityMaxAge))
+		strictHeaderValues := []string{fmt.Sprintf("max-age=%v", setting.StrictTransportSecurityMaxAge)}
 		if setting.StrictTransportSecurityPreload {
-			w.Header().Add(strictHeader, "preload")
+			strictHeaderValues = append(strictHeaderValues, "preload")
 		}
 		if setting.StrictTransportSecuritySubDomains {
-			w.Header().Add(strictHeader, "includeSubDomains")
+			strictHeaderValues = append(strictHeaderValues, "includeSubDomains")
 		}
+		w.Header().Add("Strict-Transport-Security", strings.Join(strictHeaderValues, "; "))
 	}
 
 	if setting.ContentTypeProtectionHeader {
diff --git a/pkg/middleware/middleware_test.go b/pkg/middleware/middleware_test.go
index c50960569b6..e3687f6057d 100644
--- a/pkg/middleware/middleware_test.go
+++ b/pkg/middleware/middleware_test.go
@@ -21,6 +21,39 @@ import (
 	"gopkg.in/macaron.v1"
 )
 
+func TestMiddleWareSecurityHeaders(t *testing.T) {
+	setting.ERR_TEMPLATE_NAME = "error-template"
+
+	Convey("Given the grafana middleware", t, func() {
+
+		middlewareScenario(t, "middleware should get correct x-xss-protection header", func(sc *scenarioContext) {
+			setting.XSSProtectionHeader = true
+			sc.fakeReq("GET", "/api/").exec()
+			So(sc.resp.Header().Get("X-XSS-Protection"), ShouldEqual, "1; mode=block")
+		})
+
+		middlewareScenario(t, "middleware should not get x-xss-protection when disabled", func(sc *scenarioContext) {
+			setting.XSSProtectionHeader = false
+			sc.fakeReq("GET", "/api/").exec()
+			So(sc.resp.Header().Get("X-XSS-Protection"), ShouldBeEmpty)
+		})
+
+		middlewareScenario(t, "middleware should add correct Strict-Transport-Security header", func(sc *scenarioContext) {
+			setting.StrictTransportSecurity = true
+			setting.Protocol = setting.HTTPS
+			setting.StrictTransportSecurityMaxAge = 64000
+			sc.fakeReq("GET", "/api/").exec()
+			So(sc.resp.Header().Get("Strict-Transport-Security"), ShouldEqual, "max-age=64000")
+			setting.StrictTransportSecurityPreload = true
+			sc.fakeReq("GET", "/api/").exec()
+			So(sc.resp.Header().Get("Strict-Transport-Security"), ShouldEqual, "max-age=64000; preload")
+			setting.StrictTransportSecuritySubDomains = true
+			sc.fakeReq("GET", "/api/").exec()
+			So(sc.resp.Header().Get("Strict-Transport-Security"), ShouldEqual, "max-age=64000; preload; includeSubDomains")
+		})
+	})
+}
+
 func TestMiddlewareContext(t *testing.T) {
 	setting.ERR_TEMPLATE_NAME = "error-template"