Revert "MM-13657: Set ExperimentalStrictCSRFEnforcement to true by default (#33444)" (#34112)

* Revert "MM-13657: Set ExperimentalStrictCSRFEnforcement to true by default (#33444)"

This reverts commit 257eec43ed.

* Fix call to checkCSRFToken

* Adapt test that relied on strict CSRF enforcement

This test was added after
https://github.com/mattermost/mattermost/pull/33444, so it assumed
strict CSRF enforcement to be enabled. When reverting that PR, we need
to adapt the test to account for both cases.

* Fix newer tests to use older setting

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,

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,

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,

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 {

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"}

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...)

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)

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 {

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;