Skip to content

[pull] master from mattermost:master #956

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
pull merged 1 commit into from
Sep 2, 2026
+105 −15
Merged

[pull] master from mattermost:master #956

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 21 additions & 8 deletions server/channels/app/plugin_requests.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package app

import (
"bytes"
"errors"
"io"
"net/http"
"path"
Expand Down Expand Up @@ -267,7 +268,18 @@ func (ch *Channels) servePluginRequest(w http.ResponseWriter, r *http.Request, h
return
}

if validateCSRFForPluginRequest(rctx, r, session, cookieAuth, *ch.cfgSvc.Config().ServiceSettings.ExperimentalStrictCSRFEnforcement) {
maxBodyBytes := *ch.cfgSvc.Config().ServiceSettings.MaximumPayloadSizeBytes + bytes.MinRead
csrfValid, err := validateCSRFForPluginRequest(rctx, w, r, session, cookieAuth, *ch.cfgSvc.Config().ServiceSettings.ExperimentalStrictCSRFEnforcement, maxBodyBytes)
if err != nil {
var maxBytesErr *http.MaxBytesError
if errors.As(err, &maxBytesErr) {
http.Error(w, http.StatusText(http.StatusRequestEntityTooLarge), http.StatusRequestEntityTooLarge)
return
}
handleInternalServerError(rctx, "Failed to read request body for plugin CSRF validation", err)
return
}
if csrfValid {
r.Header.Set("Mattermost-User-Id", session.UserId)
context.SessionId = session.Id
} else {
Expand All @@ -278,18 +290,19 @@ func (ch *Channels) servePluginRequest(w http.ResponseWriter, r *http.Request, h
}

// validateCSRFForPluginRequest validates CSRF token for plugin requests
func validateCSRFForPluginRequest(rctx request.CTX, r *http.Request, session *model.Session, cookieAuth bool, strictCSRFEnforcement bool) bool {
func validateCSRFForPluginRequest(rctx request.CTX, w http.ResponseWriter, r *http.Request, session *model.Session, cookieAuth bool, strictCSRFEnforcement bool, maxBodyBytes int64) (bool, error) {
// Skip CSRF check for non-cookie auth or GET requests
if !cookieAuth || r.Method == http.MethodGet {
return true
return true, nil
}

csrfTokenFromClient := r.Header.Get(model.HeaderCsrfToken)

if csrfTokenFromClient == "" {
r.Body = http.MaxBytesReader(w, r.Body, maxBodyBytes)
bodyBytes, err := io.ReadAll(r.Body)
if err != nil {
rctx.Logger().Warn("Failed to read request body for plugin request", mlog.Err(err))
return false, err
}
r.Body = io.NopCloser(bytes.NewBuffer(bodyBytes))
if err := r.ParseForm(); err != nil {
Expand All @@ -301,21 +314,21 @@ func validateCSRFForPluginRequest(rctx request.CTX, r *http.Request, session *mo

expectedToken := session.GetCSRF()
if csrfTokenFromClient == expectedToken {
return true
return true, nil
}

// ToDo(DSchalla) 2019/01/04: Remove after deprecation period and only allow CSRF Header (MM-13657)
if r.Header.Get(model.HeaderRequestedWith) == model.HeaderRequestedWithXML {
csrfErrorMessage := "CSRF Check failed for request - Please migrate your plugin to either send a CSRF Header or Form Field, XMLHttpRequest is deprecated"
if strictCSRFEnforcement {
rctx.Logger().Warn(csrfErrorMessage, mlog.String("session_id", session.Id))
return false
return false, nil
}

// Allow XMLHttpRequest for backward compatibility when not strict
rctx.Logger().Debug(csrfErrorMessage, mlog.String("session_id", session.Id))
return true
return true, nil
}

return false
return false, nil
}
91 changes: 84 additions & 7 deletions server/channels/app/plugin_requests_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -570,6 +570,31 @@ func TestServePluginRequest(t *testing.T) {
require.True(t, handlerCalled)
})

t.Run("oversized cookie auth POST without CSRF header returns 413 and is not forwarded", func(t *testing.T) {
th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.MaximumPayloadSizeBytes = 1 })
t.Cleanup(func() {
th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.MaximumPayloadSizeBytes = 300000 })
})

body := strings.NewReader(strings.Repeat("a", 2048))
req := httptest.NewRequest(http.MethodPost, "/plugins/testplugin/endpoint", body)
req = mux.SetURLVars(req, map[string]string{"plugin_id": "testplugin"})
req.AddCookie(&http.Cookie{
Name: model.SessionCookieToken,
Value: session.Token,
})
rr := httptest.NewRecorder()

handlerCalled := false
mockHandler := func(ctx *plugin.Context, w http.ResponseWriter, r *http.Request) {
handlerCalled = true
}

th.App.ch.servePluginRequest(rr, req, mockHandler)
assert.Equal(t, http.StatusRequestEntityTooLarge, rr.Code)
assert.False(t, handlerCalled)
})

t.Run("third-party use of Authorization header preserved", func(t *testing.T) {
req := httptest.NewRequest(http.MethodPost, "/plugins/testplugin/endpoint", nil)
req.Header.Set(model.HeaderAuth, "Bearer 3rd-party-token")
Expand All @@ -590,12 +615,14 @@ func TestServePluginRequest(t *testing.T) {
func TestValidateCSRFForPluginRequest(t *testing.T) {
th := Setup(t)

const testMaxBodyBytes = 1024 * 1024

t.Run("skip CSRF for non-cookie auth", func(t *testing.T) {
session := &model.Session{Id: "sessionid", UserId: "userid", Token: "token"}
session.GenerateCSRF()
req := httptest.NewRequest(http.MethodPost, "/test", nil)

result := validateCSRFForPluginRequest(th.Context, req, session, false, false)
result, _ := validateCSRFForPluginRequest(th.Context, httptest.NewRecorder(), req, session, false, false, testMaxBodyBytes)
assert.True(t, result)
})

Expand All @@ -604,7 +631,7 @@ func TestValidateCSRFForPluginRequest(t *testing.T) {
session.GenerateCSRF()
req := httptest.NewRequest(http.MethodGet, "/test", nil)

result := validateCSRFForPluginRequest(th.Context, req, session, true, false)
result, _ := validateCSRFForPluginRequest(th.Context, httptest.NewRecorder(), req, session, true, false, testMaxBodyBytes)
assert.True(t, result)
})

Expand All @@ -614,7 +641,7 @@ func TestValidateCSRFForPluginRequest(t *testing.T) {
req := httptest.NewRequest(http.MethodPost, "/test", nil)
req.Header.Set(model.HeaderCsrfToken, expectedToken)

result := validateCSRFForPluginRequest(th.Context, req, session, true, false)
result, _ := validateCSRFForPluginRequest(th.Context, httptest.NewRecorder(), req, session, true, false, testMaxBodyBytes)
assert.True(t, result)
})

Expand All @@ -624,7 +651,7 @@ func TestValidateCSRFForPluginRequest(t *testing.T) {
req := httptest.NewRequest(http.MethodPost, "/test", nil)
req.Header.Set(model.HeaderCsrfToken, "invalid-token")

result := validateCSRFForPluginRequest(th.Context, req, session, true, false)
result, _ := validateCSRFForPluginRequest(th.Context, httptest.NewRecorder(), req, session, true, false, testMaxBodyBytes)
assert.False(t, result)
})

Expand All @@ -635,10 +662,60 @@ func TestValidateCSRFForPluginRequest(t *testing.T) {
req := httptest.NewRequest(http.MethodPost, "/test", strings.NewReader(formData))
req.Header.Set("Content-Type", "application/x-www-form-urlencoded")

result := validateCSRFForPluginRequest(th.Context, req, session, true, false)
result, _ := validateCSRFForPluginRequest(th.Context, httptest.NewRecorder(), req, session, true, false, testMaxBodyBytes)
assert.True(t, result)
})

t.Run("oversized form body is rejected as too large", func(t *testing.T) {
const smallMaxBodyBytes = 128

session := &model.Session{Id: "sessionid", UserId: "userid", Token: "token"}
expectedToken := session.GenerateCSRF()
formData := "filler=" + strings.Repeat("a", smallMaxBodyBytes) + "&csrf=" + expectedToken
req := httptest.NewRequest(http.MethodPost, "/test", strings.NewReader(formData))
req.Header.Set("Content-Type", "application/x-www-form-urlencoded")

valid, err := validateCSRFForPluginRequest(th.Context, httptest.NewRecorder(), req, session, true, false, smallMaxBodyBytes)
assert.False(t, valid)
var maxBytesErr *http.MaxBytesError
assert.ErrorAs(t, err, &maxBytesErr)
})

t.Run("oversized form body with early csrf token is rejected, not accepted with truncated data", func(t *testing.T) {
const smallMaxBodyBytes = 64

session := &model.Session{Id: "sessionid", UserId: "userid", Token: "token"}
expectedToken := session.GenerateCSRF()
// A valid token up front must not let an over-limit body through with truncated data.
formData := "csrf=" + expectedToken + "&filler=" + strings.Repeat("a", smallMaxBodyBytes)
req := httptest.NewRequest(http.MethodPost, "/test", strings.NewReader(formData))
req.Header.Set("Content-Type", "application/x-www-form-urlencoded")

valid, err := validateCSRFForPluginRequest(th.Context, httptest.NewRecorder(), req, session, true, false, smallMaxBodyBytes)
assert.False(t, valid)
var maxBytesErr *http.MaxBytesError
assert.ErrorAs(t, err, &maxBytesErr)
})

t.Run("oversized body with valid csrf header is not read or capped", func(t *testing.T) {
const smallMaxBodyBytes = 64

session := &model.Session{Id: "sessionid", UserId: "userid", Token: "token"}
expectedToken := session.GenerateCSRF()
body := strings.Repeat("a", smallMaxBodyBytes*4)
req := httptest.NewRequest(http.MethodPost, "/test", strings.NewReader(body))
req.Header.Set(model.HeaderCsrfToken, expectedToken)

valid, err := validateCSRFForPluginRequest(th.Context, httptest.NewRecorder(), req, session, true, false, smallMaxBodyBytes)
assert.True(t, valid)
require.NoError(t, err)

// The header path must leave the body untouched for the plugin to stream.
read, err := io.ReadAll(req.Body)
require.NoError(t, err)
assert.Len(t, read, smallMaxBodyBytes*4)
})

t.Run("XMLHttpRequest with strict enforcement disabled", func(t *testing.T) {
th.App.UpdateConfig(func(cfg *model.Config) {
*cfg.ServiceSettings.ExperimentalStrictCSRFEnforcement = false
Expand All @@ -649,7 +726,7 @@ func TestValidateCSRFForPluginRequest(t *testing.T) {
req := httptest.NewRequest(http.MethodPost, "/test", nil)
req.Header.Set(model.HeaderRequestedWith, model.HeaderRequestedWithXML)

result := validateCSRFForPluginRequest(th.Context, req, session, true, false)
result, _ := validateCSRFForPluginRequest(th.Context, httptest.NewRecorder(), req, session, true, false, testMaxBodyBytes)
assert.True(t, result)
})

Expand All @@ -663,7 +740,7 @@ func TestValidateCSRFForPluginRequest(t *testing.T) {
req := httptest.NewRequest(http.MethodPost, "/test", nil)
req.Header.Set(model.HeaderRequestedWith, model.HeaderRequestedWithXML)

result := validateCSRFForPluginRequest(th.Context, req, session, true, true)
result, _ := validateCSRFForPluginRequest(th.Context, httptest.NewRecorder(), req, session, true, true, testMaxBodyBytes)
assert.False(t, result)
})
}
Loading