From 4f259970e6753b4406f0375fdcb2ac6350f8e3b0 Mon Sep 17 00:00:00 2001
From: Jesse Hallam <jesse.hallam@gmail.com>
Date: Wed, 27 Feb 2019 14:53:53 -0500
Subject: [PATCH] MM-14194: fix subpath csp directive until server restart
 (#10365)

* MM-14194: fix subpath csp directive until server restart

The SiteURL organically doesn't take effect until server restart, but in v5.8, the required CSP directive would change immediately. If changing from one subpath to another, the webapp would effectively be bricked until a server restart.

Avoid this by determining the CSP directive when the static handler is created.

* simplify access to config
---
 web/handlers.go      | 10 +++++++++-
 web/handlers_test.go | 21 +++++++++++++++++++--
 2 files changed, 28 insertions(+), 3 deletions(-)

diff --git a/web/handlers.go b/web/handlers.go
index 36e91bc86b..60aaef64f7 100644
--- a/web/handlers.go
+++ b/web/handlers.go
@@ -26,6 +26,10 @@ func (w *Web) NewHandler(h func(*Context, http.ResponseWriter, *http.Request)) h
 }
 
 func (w *Web) NewStaticHandler(h func(*Context, http.ResponseWriter, *http.Request)) http.Handler {
+	// Determine the CSP SHA directive needed for subpath support, if any. This value is fixed
+	// on server start and intentionally requires a restart to take effect.
+	subpath, _ := utils.GetSubpathFromConfig(w.ConfigService.Config())
+
 	return &Handler{
 		GetGlobalAppOptions: w.GetGlobalAppOptions,
 		HandleFunc:          h,
@@ -33,6 +37,8 @@ func (w *Web) NewStaticHandler(h func(*Context, http.ResponseWriter, *http.Reque
 		TrustRequester:      false,
 		RequireMfa:          false,
 		IsStatic:            true,
+
+		cspShaDirective: utils.GetSubpathScriptHash(subpath),
 	}
 }
 
@@ -43,6 +49,8 @@ type Handler struct {
 	TrustRequester      bool
 	RequireMfa          bool
 	IsStatic            bool
+
+	cspShaDirective string
 }
 
 func (h Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
@@ -79,7 +87,7 @@ func (h Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 		// Set content security policy. This is also specified in the root.html of the webapp in a meta tag.
 		w.Header().Set("Content-Security-Policy", fmt.Sprintf(
 			"frame-ancestors 'self'; script-src 'self' cdn.segment.com/analytics.js/%s",
-			utils.GetSubpathScriptHash(subpath),
+			h.cspShaDirective,
 		))
 	} else {
 		// All api response bodies will be JSON formatted by default
diff --git a/web/handlers_test.go b/web/handlers_test.go
index a6f618b2e2..ac3bf7e37c 100644
--- a/web/handlers_test.go
+++ b/web/handlers_test.go
@@ -243,7 +243,7 @@ func TestHandlerServeCSPHeader(t *testing.T) {
 		response := httptest.NewRecorder()
 		handler.ServeHTTP(response, request)
 		assert.Equal(t, 200, response.Code)
-		assert.Contains(t, response.Header()["Content-Security-Policy"], "frame-ancestors 'self'; script-src 'self' cdn.segment.com/analytics.js/")
+		assert.Equal(t, response.Header()["Content-Security-Policy"], []string{"frame-ancestors 'self'; script-src 'self' cdn.segment.com/analytics.js/"})
 	})
 
 	t.Run("static, with subpath", func(t *testing.T) {
@@ -269,6 +269,23 @@ func TestHandlerServeCSPHeader(t *testing.T) {
 		response := httptest.NewRecorder()
 		handler.ServeHTTP(response, request)
 		assert.Equal(t, 200, response.Code)
-		assert.Contains(t, response.Header()["Content-Security-Policy"], "frame-ancestors 'self'; script-src 'self' cdn.segment.com/analytics.js/ 'sha256-tPOjw+tkVs9axL78ZwGtYl975dtyPHB6LYKAO2R3gR4='")
+		assert.Equal(t, response.Header()["Content-Security-Policy"], []string{"frame-ancestors 'self'; script-src 'self' cdn.segment.com/analytics.js/"})
+
+		// TODO: It's hard to unit test this now that the CSP directive is effectively
+		// decided in Setup(). Circle back to this in master once the memory store is
+		// merged, allowing us to mock the desired initial config to take effect in Setup().
+		// assert.Contains(t, response.Header()["Content-Security-Policy"], "frame-ancestors 'self'; script-src 'self' cdn.segment.com/analytics.js/ 'sha256-tPOjw+tkVs9axL78ZwGtYl975dtyPHB6LYKAO2R3gR4='")
+
+		th.App.UpdateConfig(func(cfg *model.Config) {
+			*cfg.ServiceSettings.SiteURL = *cfg.ServiceSettings.SiteURL + "/subpath2"
+		})
+
+		request = httptest.NewRequest("POST", "/", nil)
+		response = httptest.NewRecorder()
+		handler.ServeHTTP(response, request)
+		assert.Equal(t, 200, response.Code)
+		assert.Equal(t, response.Header()["Content-Security-Policy"], []string{"frame-ancestors 'self'; script-src 'self' cdn.segment.com/analytics.js/"})
+		// TODO: See above.
+		// assert.Contains(t, response.Header()["Content-Security-Policy"], "frame-ancestors 'self'; script-src 'self' cdn.segment.com/analytics.js/ 'sha256-tPOjw+tkVs9axL78ZwGtYl975dtyPHB6LYKAO2R3gR4='", "csp header incorrectly changed after subpath changed")
 	})
 }