Merge remote-tracking branch 'origin/master' into MM-22212

api4/group_test.go

@@ -7,6 +7,7 @@ import (
 	"fmt"
 	"net/http"
 	"testing"
+	"time"
 
 	"github.com/mattermost/mattermost-server/v5/model"
 	"github.com/stretchr/testify/assert"
@@ -236,6 +237,7 @@ func TestUnlinkGroupTeam(t *testing.T) {
 
 	response = th.Client.UnlinkGroupSyncable(g.Id, th.BasicTeam.Id, model.GroupSyncableTypeTeam)
 	assert.NotNil(t, response.Error)
+	time.Sleep(2 * time.Second) // A hack to let "go c.App.SyncRolesAndMembership" finish before moving on.
 	th.UpdateUserToTeamAdmin(th.BasicUser, th.BasicTeam)
 	ok, response := th.Client.Logout()
 	assert.True(t, ok)

app/channel.go

@@ -1164,7 +1164,8 @@ func (a *App) addUserToChannel(user *model.User, channel *model.Channel, teamMem
 		newMember.SchemeAdmin = userShouldBeAdmin
 	}
 
-	if _, err = a.Srv().Store.Channel().SaveMember(newMember); err != nil {
+	newMember, err = a.Srv().Store.Channel().SaveMember(newMember)
+	if err != nil {
 		mlog.Error("Failed to add member", mlog.String("user_id", user.Id), mlog.String("channel_id", channel.Id), mlog.Err(err))
 		return nil, model.NewAppError("AddUserToChannel", "api.channel.add_user.to.channel.failed.app_error", nil, "", http.StatusInternalServerError)
 	}

store/sqlstore/team_store.go

@@ -189,6 +189,8 @@ func (s SqlTeamStore) createIndexesIfNotExists() {
 	s.CreateIndexIfNotExists("idx_teammembers_delete_at", "TeamMembers", "DeleteAt")
 }
 
+// Save adds the team to the database if a team with the same name does not already
+// exist in the database. It returns the team added if the operation is successful.
 func (s SqlTeamStore) Save(team *model.Team) (*model.Team, *model.AppError) {
 	if len(team.Id) > 0 {
 		return nil, model.NewAppError("SqlTeamStore.Save",
@@ -210,6 +212,9 @@ func (s SqlTeamStore) Save(team *model.Team) (*model.Team, *model.AppError) {
 	return team, nil
 }
 
+// Update updates the details of the team passed as the parameter using the team Id
+// if the team exists in the database.
+// It returns the updated team if the operation is successful.
 func (s SqlTeamStore) Update(team *model.Team) (*model.Team, *model.AppError) {
 
 	team.PreUpdate()
@@ -529,6 +534,10 @@ func (s SqlTeamStore) getTeamMembersWithSchemeSelectQuery() sq.SelectBuilder {
 		LeftJoin("Schemes TeamScheme ON Teams.SchemeId = TeamScheme.Id")
 }
 
+// SaveMember adds a team member using the team Id of the member
+// if the member does not already exist in the database and if the number
+// of existing team members are less than the maximum allowed users per team.
+// It returns the team member added if the operation is successful.
 func (s SqlTeamStore) SaveMember(member *model.TeamMember, maxUsersPerTeam int) (*model.TeamMember, *model.AppError) {
 	defer s.InvalidateAllTeamIdsForUser(member.UserId)
 	if err := member.IsValid(); err != nil {
@@ -587,6 +596,8 @@ func (s SqlTeamStore) SaveMember(member *model.TeamMember, maxUsersPerTeam int)
 	return retrievedMember.ToModel(), nil
 }
 
+// UpdateMember updates the team member if the team member exists in the database.
+// It returns the updated team member if the operation is successful.
 func (s SqlTeamStore) UpdateMember(member *model.TeamMember) (*model.TeamMember, *model.AppError) {
 	member.PreUpdate()
 

store/sqlstore/upgrade.go

@@ -18,8 +18,7 @@ import (
 )
 
 const (
-	CURRENT_SCHEMA_VERSION   = VERSION_5_20_0
-	VERSION_5_22_0           = "5.22.0"
+	CURRENT_SCHEMA_VERSION   = VERSION_5_21_0
 	VERSION_5_21_0           = "5.21.0"
 	VERSION_5_20_0           = "5.20.0"
 	VERSION_5_19_0           = "5.19.0"
@@ -769,6 +768,7 @@ func upgradeDatabaseToVersion520(sqlStore SqlStore) {
 
 func upgradeDatabaseToVersion521(sqlStore SqlStore) {
 	if shouldPerformUpgrade(sqlStore, VERSION_5_20_0, VERSION_5_21_0) {
+		saveSchemaVersion(sqlStore, VERSION_5_21_0)
 	}
 }
 

web/webhook.go

@@ -5,6 +5,7 @@ package web
 
 import (
 	"io"
+	"mime"
 	"net/http"
 	"strings"
 
@@ -28,7 +29,11 @@ func incomingWebhook(c *Context, w http.ResponseWriter, r *http.Request) {
 
 	var err *model.AppError
 	incomingWebhookPayload := &model.IncomingWebhookRequest{}
-	contentType := r.Header.Get("Content-Type")
+	mediaType, _, mimeErr := mime.ParseMediaType(r.Header.Get("Content-Type"))
+	if mimeErr != nil && mimeErr != mime.ErrInvalidMediaParameter {
+		c.Err = model.NewAppError("incomingWebhook", "api.webhook.incoming.error", nil, mimeErr.Error(), http.StatusBadRequest)
+		return
+	}
 
 	defer func() {
 		if *c.App.Config().LogSettings.EnableWebhookDebugging {
@@ -38,7 +43,7 @@ func incomingWebhook(c *Context, w http.ResponseWriter, r *http.Request) {
 		}
 	}()
 
-	if strings.Split(contentType, "; ")[0] == "application/x-www-form-urlencoded" {
+	if mediaType == "application/x-www-form-urlencoded" {
 		payload := strings.NewReader(r.FormValue("payload"))
 
 		incomingWebhookPayload, err = decodePayload(payload)
@@ -46,7 +51,7 @@ func incomingWebhook(c *Context, w http.ResponseWriter, r *http.Request) {
 			c.Err = err
 			return
 		}
-	} else if strings.HasPrefix(contentType, "multipart/form-data") {
+	} else if mediaType == "multipart/form-data" {
 		r.ParseMultipartForm(0)
 
 		decoder := schema.NewDecoder()

web/webhook_test.go

@@ -87,14 +87,42 @@ func TestIncomingWebhook(t *testing.T) {
 		assert.Nil(t, err)
 		assert.True(t, resp.StatusCode == http.StatusOK)
 
+		resp, err = http.Post(url, "AppLicaTion/x-www-Form-urlencoded", strings.NewReader("payload={\"text\":\""+text+"\"}"))
+		assert.Nil(t, err)
+		assert.True(t, resp.StatusCode == http.StatusOK)
+
+		resp, err = http.Post(url, "application/x-www-form-urlencoded;charset=utf-8", strings.NewReader("payload={\"text\":\""+text+"\"}"))
+		assert.Nil(t, err)
+		assert.True(t, resp.StatusCode == http.StatusOK)
+
+		resp, err = http.Post(url, "application/x-www-form-urlencoded; charset=utf-8", strings.NewReader("payload={\"text\":\""+text+"\"}"))
+		assert.Nil(t, err)
+		assert.True(t, resp.StatusCode == http.StatusOK)
+
+		resp, err = http.Post(url, "application/x-www-form-urlencoded wrongtext", strings.NewReader("payload={\"text\":\""+text+"\"}"))
+		assert.Nil(t, err)
+		assert.True(t, resp.StatusCode == http.StatusBadRequest)
+
 		resp, err = http.Post(url, "application/json", strings.NewReader("{\"text\":\""+tooLongText+"\"}"))
 		require.Nil(t, err)
 		assert.True(t, resp.StatusCode == http.StatusOK)
 
+		resp, err = http.Post(url, "application/x-www-form-urlencoded", strings.NewReader("{\"text\":\""+tooLongText+"\"}"))
+		assert.Nil(t, err)
+		assert.True(t, resp.StatusCode == http.StatusBadRequest)
+
+		resp, err = http.Post(url, "application/json", strings.NewReader("payload={\"text\":\""+text+"\"}"))
+		assert.Nil(t, err)
+		assert.True(t, resp.StatusCode == http.StatusBadRequest)
+
 		payloadMultiPart := "------WebKitFormBoundary7MA4YWxkTrZu0gW\r\nContent-Disposition: form-data; name=\"username\"\r\n\r\nwebhook-bot\r\n------WebKitFormBoundary7MA4YWxkTrZu0gW\r\nContent-Disposition: form-data; name=\"text\"\r\n\r\nthis is a test :tada:\r\n------WebKitFormBoundary7MA4YWxkTrZu0gW--"
 		resp, err = http.Post(ApiClient.Url+"/hooks/"+hook.Id, "multipart/form-data; boundary=----WebKitFormBoundary7MA4YWxkTrZu0gW", strings.NewReader(payloadMultiPart))
 		require.Nil(t, err)
 		assert.True(t, resp.StatusCode == http.StatusOK)
+
+		resp, err = http.Post(url, "mimetype/wrong", strings.NewReader("payload={\"text\":\""+text+"\"}"))
+		assert.Nil(t, err)
+		assert.True(t, resp.StatusCode == http.StatusBadRequest)
 	})
 
 	t.Run("WebhookExperimentalReadOnly", func(t *testing.T) {