diff --git a/e2e-tests/cypress/tests/support/api/cloud_default_config.json b/e2e-tests/cypress/tests/support/api/cloud_default_config.json index 0209904c502..290fb6b0f65 100644 --- a/e2e-tests/cypress/tests/support/api/cloud_default_config.json +++ b/e2e-tests/cypress/tests/support/api/cloud_default_config.json @@ -79,7 +79,7 @@ "EnableAPITriggerAdminNotifications": false, "EnableAPIUserDeletion": false, "ExperimentalEnableHardenedMode": false, - "StrictCSRFEnforcement": false, + "ExperimentalStrictCSRFEnforcement": false, "EnableEmailInvitations": true, "DisableBotsWhenOwnerIsDeactivated": true, "EnableBotAccountCreation": true, diff --git a/e2e-tests/cypress/tests/support/api/on_prem_default_config.json b/e2e-tests/cypress/tests/support/api/on_prem_default_config.json index de1f4605930..30325f83d88 100644 --- a/e2e-tests/cypress/tests/support/api/on_prem_default_config.json +++ b/e2e-tests/cypress/tests/support/api/on_prem_default_config.json @@ -81,7 +81,7 @@ "EnableAPITriggerAdminNotifications": false, "EnableAPIUserDeletion": false, "ExperimentalEnableHardenedMode": false, - "StrictCSRFEnforcement": false, + "ExperimentalStrictCSRFEnforcement": false, "EnableEmailInvitations": true, "DisableBotsWhenOwnerIsDeactivated": true, "EnableBotAccountCreation": true, diff --git a/e2e-tests/playwright/lib/src/server/default_config.ts b/e2e-tests/playwright/lib/src/server/default_config.ts index e804d342bbd..9b126a56963 100644 --- a/e2e-tests/playwright/lib/src/server/default_config.ts +++ b/e2e-tests/playwright/lib/src/server/default_config.ts @@ -170,7 +170,7 @@ const defaultServerConfig: AdminConfig = { EnableAPIPostDeletion: false, EnableDesktopLandingPage: true, ExperimentalEnableHardenedMode: false, - StrictCSRFEnforcement: true, + ExperimentalStrictCSRFEnforcement: false, EnableEmailInvitations: false, DisableBotsWhenOwnerIsDeactivated: true, EnableBotAccountCreation: false, diff --git a/server/channels/app/plugin_requests.go b/server/channels/app/plugin_requests.go index 447aaa7756d..e3213726e49 100644 --- a/server/channels/app/plugin_requests.go +++ b/server/channels/app/plugin_requests.go @@ -236,7 +236,7 @@ func (ch *Channels) servePluginRequest(w http.ResponseWriter, r *http.Request, h return } - if validateCSRFForPluginRequest(rctx, r, session, cookieAuth, *ch.cfgSvc.Config().ServiceSettings.StrictCSRFEnforcement) { + if validateCSRFForPluginRequest(rctx, r, session, cookieAuth, *ch.cfgSvc.Config().ServiceSettings.ExperimentalStrictCSRFEnforcement) { r.Header.Set("Mattermost-User-Id", session.UserId) context.SessionId = session.Id } else { diff --git a/server/channels/app/plugin_requests_test.go b/server/channels/app/plugin_requests_test.go index e97227cf026..7faf3a5219e 100644 --- a/server/channels/app/plugin_requests_test.go +++ b/server/channels/app/plugin_requests_test.go @@ -577,7 +577,7 @@ func TestValidateCSRFForPluginRequest(t *testing.T) { t.Run("XMLHttpRequest with strict enforcement disabled", func(t *testing.T) { th.App.UpdateConfig(func(cfg *model.Config) { - *cfg.ServiceSettings.StrictCSRFEnforcement = false + *cfg.ServiceSettings.ExperimentalStrictCSRFEnforcement = false }) session := &model.Session{Id: "sessionid", UserId: "userid", Token: "token"} @@ -591,7 +591,7 @@ func TestValidateCSRFForPluginRequest(t *testing.T) { t.Run("XMLHttpRequest with strict enforcement enabled", func(t *testing.T) { th.App.UpdateConfig(func(cfg *model.Config) { - *cfg.ServiceSettings.StrictCSRFEnforcement = true + *cfg.ServiceSettings.ExperimentalStrictCSRFEnforcement = true }) session := &model.Session{Id: "sessionid", UserId: "userid", Token: "token"} diff --git a/server/channels/web/handlers.go b/server/channels/web/handlers.go index fb25f5f1225..c967e1c0495 100644 --- a/server/channels/web/handlers.go +++ b/server/channels/web/handlers.go @@ -494,7 +494,7 @@ func (h *Handler) checkCSRFToken(c *Context, r *http.Request, tokenLocation app. mlog.String("user_id", session.UserId), } - if *c.App.Config().ServiceSettings.StrictCSRFEnforcement { + if *c.App.Config().ServiceSettings.ExperimentalStrictCSRFEnforcement { c.Logger.Warn(csrfErrorMessage, fields...) } else { c.Logger.Debug(csrfErrorMessage, fields...) diff --git a/server/channels/web/handlers_test.go b/server/channels/web/handlers_test.go index cc24b43f8db..1ce4153362d 100644 --- a/server/channels/web/handlers_test.go +++ b/server/channels/web/handlers_test.go @@ -227,7 +227,7 @@ func TestHandlerServeCSRFToken(t *testing.T) { // Fallback Behavior Used - Success expected // ToDo (DSchalla) 2019/01/04: Remove once legacy CSRF Handling is removed th.App.UpdateConfig(func(config *model.Config) { - *config.ServiceSettings.StrictCSRFEnforcement = false + *config.ServiceSettings.ExperimentalStrictCSRFEnforcement = false }) request = httptest.NewRequest("POST", "/api/v4/test", nil) request.AddCookie(cookie) @@ -244,7 +244,7 @@ func TestHandlerServeCSRFToken(t *testing.T) { // Fallback Behavior Used with Strict Enforcement - Failure Expected // ToDo (DSchalla) 2019/01/04: Remove once legacy CSRF Handling is removed th.App.UpdateConfig(func(config *model.Config) { - *config.ServiceSettings.StrictCSRFEnforcement = true + *config.ServiceSettings.ExperimentalStrictCSRFEnforcement = true }) response = httptest.NewRecorder() handler.ServeHTTP(response, request) @@ -587,12 +587,15 @@ func TestHandlerServeInvalidToken(t *testing.T) { func TestHandlerServeCSRFFailureClearsAuthCookie(t *testing.T) { testCases := []struct { - Description string - SiteURL string - ExpectedSetCookieHeaderRegexp string + Description string + SiteURL string + ExpectedSetCookieHeaderRegexp string + ExperimentalStrictCSRFEnforcement bool }{ - {"no subpath", "http://localhost:8065", "^MMAUTHTOKEN=; Path=/"}, - {"subpath", "http://localhost:8065/subpath", "^MMAUTHTOKEN=; Path=/subpath"}, + {"no subpath", "http://localhost:8065", "^MMAUTHTOKEN=; Path=/", false}, + {"subpath", "http://localhost:8065/subpath", "^MMAUTHTOKEN=; Path=/subpath", false}, + {"no subpath", "http://localhost:8065", "^MMAUTHTOKEN=; Path=/", true}, + {"subpath", "http://localhost:8065/subpath", "^MMAUTHTOKEN=; Path=/subpath", true}, } for _, tc := range testCases { @@ -601,6 +604,7 @@ func TestHandlerServeCSRFFailureClearsAuthCookie(t *testing.T) { th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.SiteURL = tc.SiteURL + *cfg.ServiceSettings.ExperimentalStrictCSRFEnforcement = tc.ExperimentalStrictCSRFEnforcement }) session := &model.Session{ @@ -635,10 +639,14 @@ func TestHandlerServeCSRFFailureClearsAuthCookie(t *testing.T) { request.Header.Add(model.HeaderRequestedWith, model.HeaderRequestedWithXML) response := httptest.NewRecorder() handler.ServeHTTP(response, request) - require.Equal(t, http.StatusUnauthorized, response.Code) - cookies := response.Header().Get("Set-Cookie") - assert.Regexp(t, tc.ExpectedSetCookieHeaderRegexp, cookies) + if tc.ExperimentalStrictCSRFEnforcement { + require.Equal(t, http.StatusUnauthorized, response.Code) + cookies := response.Header().Get("Set-Cookie") + assert.Regexp(t, tc.ExpectedSetCookieHeaderRegexp, cookies) + } else { + require.Equal(t, http.StatusOK, response.Code) + } }) } } @@ -674,7 +682,7 @@ func TestCheckCSRFToken(t *testing.T) { assert.Nil(t, c.Err) }) - t.Run("should not allow a POST request with an X-Requested-With header", func(t *testing.T) { + t.Run("should allow a POST request with an X-Requested-With header", func(t *testing.T) { th := SetupWithStoreMock(t) h := &Handler{ @@ -700,6 +708,56 @@ func TestCheckCSRFToken(t *testing.T) { checked, passed := h.checkCSRFToken(c, r, tokenLocation, session) + assert.True(t, checked) + assert.True(t, passed) + assert.Nil(t, c.Err) + }) + + t.Run("should not allow a POST request with an X-Requested-With header with strict CSRF enforcement enabled", func(t *testing.T) { + th := SetupWithStoreMock(t) + + mockStore := th.App.Srv().Store().(*mocks.Store) + mockUserStore := mocks.UserStore{} + mockUserStore.On("Count", mock.Anything).Return(int64(10), nil) + mockPostStore := mocks.PostStore{} + mockPostStore.On("GetMaxPostSize").Return(65535, nil) + mockSystemStore := mocks.SystemStore{} + mockSystemStore.On("GetByName", "UpgradedFromTE").Return(&model.System{Name: "UpgradedFromTE", Value: "false"}, nil) + mockSystemStore.On("GetByName", "InstallationDate").Return(&model.System{Name: "InstallationDate", Value: "10"}, nil) + mockSystemStore.On("GetByName", "FirstServerRunTimestamp").Return(&model.System{Name: "FirstServerRunTimestamp", Value: "10"}, nil) + + mockStore.On("User").Return(&mockUserStore) + mockStore.On("Post").Return(&mockPostStore) + mockStore.On("System").Return(&mockSystemStore) + mockStore.On("GetDBSchemaVersion").Return(1, nil) + + th.App.UpdateConfig(func(cfg *model.Config) { + *cfg.ServiceSettings.ExperimentalStrictCSRFEnforcement = true + }) + + h := &Handler{ + RequireSession: true, + TrustRequester: false, + } + + token := "token" + tokenLocation := app.TokenLocationCookie + + c := &Context{ + App: th.App, + Logger: th.App.Log(), + AppContext: th.Context, + } + r, _ := http.NewRequest(http.MethodPost, "", nil) + r.Header.Set(model.HeaderRequestedWith, model.HeaderRequestedWithXML) + session := &model.Session{ + Props: map[string]string{ + "csrf": token, + }, + } + + checked, passed := h.checkCSRFToken(c, r, tokenLocation, session) + assert.True(t, checked) assert.False(t, passed) assert.Nil(t, c.Err) diff --git a/server/public/model/config.go b/server/public/model/config.go index 6caa09d2340..5465c7308fd 100644 --- a/server/public/model/config.go +++ b/server/public/model/config.go @@ -416,7 +416,7 @@ type ServiceSettings struct { EnableAPIPostDeletion *bool EnableDesktopLandingPage *bool ExperimentalEnableHardenedMode *bool `access:"experimental_features"` - StrictCSRFEnforcement *bool `access:"experimental_features,write_restrictable,cloud_restrictable"` + ExperimentalStrictCSRFEnforcement *bool `access:"experimental_features,write_restrictable,cloud_restrictable"` EnableEmailInvitations *bool `access:"authentication_signup"` DisableBotsWhenOwnerIsDeactivated *bool `access:"integrations_bot_accounts"` EnableBotAccountCreation *bool `access:"integrations_bot_accounts"` @@ -849,8 +849,8 @@ func (s *ServiceSettings) SetDefaults(isUpdate bool) { s.ExperimentalEnableHardenedMode = NewPointer(false) } - if s.StrictCSRFEnforcement == nil { - s.StrictCSRFEnforcement = NewPointer(true) + if s.ExperimentalStrictCSRFEnforcement == nil { + s.ExperimentalStrictCSRFEnforcement = NewPointer(false) } if s.DisableBotsWhenOwnerIsDeactivated == nil { diff --git a/webapp/platform/types/src/config.ts b/webapp/platform/types/src/config.ts index 47815f14a90..7f27320f515 100644 --- a/webapp/platform/types/src/config.ts +++ b/webapp/platform/types/src/config.ts @@ -385,7 +385,7 @@ export type ServiceSettings = { EnableAPITriggerAdminNotifications: boolean; EnableAPIUserDeletion: boolean; ExperimentalEnableHardenedMode: boolean; - StrictCSRFEnforcement: boolean; + ExperimentalStrictCSRFEnforcement: boolean; EnableEmailInvitations: boolean; DisableBotsWhenOwnerIsDeactivated: boolean; EnableBotAccountCreation: boolean;