Commit 4f259970 authored by Jesse Hallam's avatar Jesse Hallam Committed by GitHub

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
parent df6b8ff7
......@@ -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
......
......@@ -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")
})
}
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment