update for adding multiple members (#25128)

* update for adding multiple members * fix unit test * more test fixes * add another unit test * fix object passed by client4 * revert package-lock.json * revert package-lock.json * add length check * limit size of lists in API requests * revert package-lock * add batching to front end * add batching to front end * fix bad merge * update return type * remove unnecessary permisssion check, add unit test * fixes and add tests from review * revert changes adding limits to other apis * fixes * clean-up from code review * fix unit test call * revert back to interface{}, fix unit test --------- Co-authored-by: Mattermost Build <build@mattermost.com>
2024-06-25 12:25:28 -06:00
--- a/server/channels/api4/channel.go
+++ b/server/channels/api4/channel.go
@@ -16,6 +16,8 @@ import (
 	"github.com/mattermost/mattermost/server/v8/channels/audit"
 )

+const maxListSize = 1000
+
 func (api *API) InitChannel() {
 	api.BaseRoutes.Channels.Handle("", api.APISessionRequired(getAllChannels)).Methods("GET")
 	api.BaseRoutes.Channels.Handle("", api.APISessionRequired(createChannel)).Methods("POST")
@@ -1725,43 +1727,47 @@ func addChannelMember(c *Context, w http.ResponseWriter, r *http.Request) {
 	}

 	props := model.StringInterfaceFromJSON(r.Body)
-	userId, ok := props["user_id"].(string)
-	if !ok || !model.IsValidId(userId) {
-		c.SetInvalidParam("user_id")
-		return
-	}

-	auditRec := c.MakeAuditRecord("addChannelMember", audit.Fail)
-	defer c.LogAuditRec(auditRec)
-	audit.AddEventParameter(auditRec, "user_id", userId)
-	audit.AddEventParameter(auditRec, "channel_id", c.Params.ChannelId)
-
-	member := &model.ChannelMember{
-		ChannelId: c.Params.ChannelId,
-		UserId:    userId,
+	var userIds []string
+	interfaceIds, ok := props["user_ids"].([]interface{})
+	if ok {
+		if len(interfaceIds) > maxListSize {
+			c.SetInvalidParam("user_ids")
+			return
+		}
+		for _, userId := range interfaceIds {
+			userIds = append(userIds, userId.(string))
+		}
+	} else {
+		userId, ok2 := props["user_id"].(string)
+		if !ok2 || !model.IsValidId(userId) {
+			c.SetInvalidParam("user_id or user_ids")
+			return
+		}
+		userIds = append(userIds, userId)
 	}

 	postRootId, ok := props["post_root_id"].(string)
-	if ok && postRootId != "" && !model.IsValidId(postRootId) {
-		c.SetInvalidParam("post_root_id")
-		return
-	}
+	if ok && postRootId != "" {
+		if !model.IsValidId(postRootId) {
+			c.SetInvalidParam("post_root_id")
+			return
+		}

-	audit.AddEventParameter(auditRec, "post_root_id", postRootId)
-
-	if ok && len(postRootId) == 26 {
 		rootPost, err := c.App.GetSinglePost(c.AppContext, postRootId, false)
 		if err != nil {
 			c.Err = err
 			return
 		}
-		if rootPost.ChannelId != member.ChannelId {
+		if rootPost.ChannelId != c.Params.ChannelId {
 			c.SetInvalidParam("post_root_id")
 			return
 		}
+	} else if !ok {
+		postRootId = ""
 	}

-	channel, err := c.App.GetChannel(c.AppContext, member.ChannelId)
+	channel, err := c.App.GetChannel(c.AppContext, c.Params.ChannelId)
 	if err != nil {
 		c.Err = err
 		return
@@ -1772,54 +1778,28 @@ func addChannelMember(c *Context, w http.ResponseWriter, r *http.Request) {
 		return
 	}

-	isNewMembership := false
-	if _, err = c.App.GetChannelMember(c.AppContext, member.ChannelId, member.UserId); err != nil {
-		if err.Id == app.MissingChannelMemberError {
-			isNewMembership = true
-		} else {
-			c.Err = err
-			return
-		}
-	}
-
-	isSelfAdd := member.UserId == c.AppContext.Session().UserId
-
+	canAddSelf := false
+	canAddOthers := false
 	if channel.Type == model.ChannelTypeOpen {
-		if isSelfAdd && isNewMembership {
-			if !c.App.SessionHasPermissionToTeam(*c.AppContext.Session(), channel.TeamId, model.PermissionJoinPublicChannels) {
-				c.SetPermissionError(model.PermissionJoinPublicChannels)
-				return
-			}
-		} else if isSelfAdd && !isNewMembership {
-			// nothing to do, since already in the channel
-		} else if !isSelfAdd {
-			if !c.App.SessionHasPermissionToChannel(c.AppContext, *c.AppContext.Session(), channel.Id, model.PermissionManagePublicChannelMembers) {
-				c.SetPermissionError(model.PermissionManagePublicChannelMembers)
-				return
-			}
+		if c.App.SessionHasPermissionToTeam(*c.AppContext.Session(), channel.TeamId, model.PermissionJoinPublicChannels) {
+			canAddSelf = true
+		}
+		if c.App.SessionHasPermissionToChannel(c.AppContext, *c.AppContext.Session(), channel.Id, model.PermissionManagePublicChannelMembers) {
+			canAddOthers = true
 		}
 	}

 	if channel.Type == model.ChannelTypePrivate {
-		if isSelfAdd && isNewMembership {
-			if !c.App.SessionHasPermissionToChannel(c.AppContext, *c.AppContext.Session(), channel.Id, model.PermissionManagePrivateChannelMembers) {
-				c.SetPermissionError(model.PermissionManagePrivateChannelMembers)
-				return
-			}
-		} else if isSelfAdd && !isNewMembership {
-			// nothing to do, since already in the channel
-		} else if !isSelfAdd {
-			if !c.App.SessionHasPermissionToChannel(c.AppContext, *c.AppContext.Session(), channel.Id, model.PermissionManagePrivateChannelMembers) {
-				c.SetPermissionError(model.PermissionManagePrivateChannelMembers)
-				return
-			}
+		if !c.App.SessionHasPermissionToChannel(c.AppContext, *c.AppContext.Session(), channel.Id, model.PermissionManagePrivateChannelMembers) {
+			c.SetPermissionError(model.PermissionManagePrivateChannelMembers)
+			return
 		}
 	}

 	if channel.IsGroupConstrained() {
-		nonMembers, err := c.App.FilterNonGroupChannelMembers([]string{member.UserId}, channel)
+		nonMembers, err := c.App.FilterNonGroupChannelMembers(userIds, channel)
 		if err != nil {
-			if v, ok := err.(*model.AppError); ok {
+			if v, ok2 := err.(*model.AppError); ok2 {
 				c.Err = v
 			} else {
 				c.Err = model.NewAppError("addChannelMember", "api.channel.add_members.error", nil, "", http.StatusBadRequest).Wrap(err)
@@ -1832,32 +1812,98 @@ func addChannelMember(c *Context, w http.ResponseWriter, r *http.Request) {
 		}
 	}

-	cm, err := c.App.AddChannelMember(c.AppContext, member.UserId, channel, app.ChannelMemberOpts{
-		UserRequestorID: c.AppContext.Session().UserId,
-		PostRootID:      postRootId,
-	})
-	if err != nil {
-		c.Err = err
+	var lastError *model.AppError
+	var newChannelMembers []model.ChannelMember
+	for _, userId := range userIds {
+		if !model.IsValidId(userId) {
+			c.Logger.Warn("Error adding channel member, invalid UserId", mlog.String("UserId", userId), mlog.String("ChannelId", channel.Id))
+			c.SetInvalidParam("user_id")
+			lastError = c.Err
+			continue
+		}
+
+		auditRec := c.MakeAuditRecord("addChannelMember", audit.Fail)
+		defer c.LogAuditRec(auditRec)
+		audit.AddEventParameter(auditRec, "user_id", userId)
+		audit.AddEventParameter(auditRec, "channel_id", c.Params.ChannelId)
+		audit.AddEventParameter(auditRec, "post_root_id", postRootId)
+
+		member := &model.ChannelMember{
+			ChannelId: c.Params.ChannelId,
+			UserId:    userId,
+		}
+
+		existingMember, err := c.App.GetChannelMember(c.AppContext, member.ChannelId, member.UserId)
+		if err != nil {
+			if err.Id != app.MissingChannelMemberError {
+				c.Logger.Warn("Error adding channel member, error getting channel member", mlog.String("UserId", userId), mlog.String("ChannelId", channel.Id), mlog.Err(err))
+				lastError = err
+				continue
+			}
+		} else {
+			// user is already a member, go to next
+			c.Logger.Warn("User is already a channel member, skipping", mlog.String("UserId", userId), mlog.String("ChannelId", channel.Id))
+			newChannelMembers = append(newChannelMembers, *existingMember)
+			continue
+		}
+
+		if channel.Type == model.ChannelTypeOpen {
+			isSelfAdd := member.UserId == c.AppContext.Session().UserId
+			if isSelfAdd && !canAddSelf {
+				c.Logger.Warn("Error adding channel member, Invalid Permission to add self", mlog.String("UserId", userId), mlog.String("ChannelId", channel.Id))
+				c.SetPermissionError(model.PermissionJoinPublicChannels)
+				lastError = c.Err
+				continue
+			} else if !isSelfAdd && !canAddOthers {
+				c.Logger.Warn("Error adding channel member, Invalid Permission to add others", mlog.String("UserId", userId), mlog.String("ChannelId", channel.Id))
+				c.SetPermissionError(model.PermissionManagePublicChannelMembers)
+				lastError = c.Err
+				continue
+			}
+		}
+
+		cm, err := c.App.AddChannelMember(c.AppContext, member.UserId, channel, app.ChannelMemberOpts{
+			UserRequestorID: c.AppContext.Session().UserId,
+			PostRootID:      postRootId,
+		})
+		if err != nil {
+			c.Logger.Warn("Error adding channel member", mlog.String("UserId", userId), mlog.String("ChannelId", channel.Id), mlog.Err(err))
+			lastError = err
+			continue
+		}
+		newChannelMembers = append(newChannelMembers, *cm)
+
+		if postRootId != "" {
+			err := c.App.UpdateThreadFollowForUserFromChannelAdd(c.AppContext, cm.UserId, channel.TeamId, postRootId)
+			if err != nil {
+				c.Logger.Warn("Error adding channel member, error updating thread", mlog.String("UserId", userId), mlog.String("ChannelId", channel.Id), mlog.Err(err))
+				lastError = err
+				continue
+			}
+		}
+
+		auditRec.Success()
+		auditRec.AddEventResultState(cm)
+		auditRec.AddEventObjectType("channel_member")
+		auditRec.AddMeta("add_user_id", cm.UserId)
+		c.LogAudit("name=" + channel.Name + " user_id=" + cm.UserId)
+	}
+
+	if lastError != nil && len(newChannelMembers) == 0 {
+		c.Err = lastError
 		return
 	}

-	if postRootId != "" {
-		err := c.App.UpdateThreadFollowForUserFromChannelAdd(c.AppContext, cm.UserId, channel.TeamId, postRootId)
-		if err != nil {
-			c.Err = err
-			return
-		}
-	}
-
-	auditRec.Success()
-	auditRec.AddEventResultState(cm)
-	auditRec.AddEventObjectType("channel_member")
-	auditRec.AddMeta("add_user_id", cm.UserId)
-	c.LogAudit("name=" + channel.Name + " user_id=" + cm.UserId)
-
 	w.WriteHeader(http.StatusCreated)
-	if err := json.NewEncoder(w).Encode(cm); err != nil {
-		c.Logger.Warn("Error while writing response", mlog.Err(err))
+	userId, ok := props["user_id"]
+	if ok && len(newChannelMembers) == 1 && newChannelMembers[0].UserId == userId {
+		if err := json.NewEncoder(w).Encode(newChannelMembers[0]); err != nil {
+			c.Logger.Warn("Error while writing response", mlog.Err(err))
+		}
+	} else {
+		if err := json.NewEncoder(w).Encode(newChannelMembers); err != nil {
+			c.Logger.Warn("Error while writing response", mlog.Err(err))
+		}
 	}
 }

--- a/server/channels/api4/channel_test.go
+++ b/server/channels/api4/channel_test.go
@@ -3294,6 +3294,36 @@ func TestAddChannelMember(t *testing.T) {
 	})
 }

+func TestAddChannelMembers(t *testing.T) {
+	th := Setup(t).InitBasic()
+	defer th.TearDown()
+	client := th.Client
+	user := th.BasicUser
+	user2 := th.BasicUser2
+	team := th.BasicTeam
+	publicChannel := th.CreatePublicChannel()
+	privateChannel := th.CreatePrivateChannel()
+
+	user3 := th.CreateUserWithClient(th.SystemAdminClient)
+	_, _, err := th.SystemAdminClient.AddTeamMember(context.Background(), team.Id, user3.Id)
+	require.NoError(t, err)
+
+	cm, resp, err := client.AddChannelMembers(context.Background(), publicChannel.Id, "", []string{user.Id, user2.Id, user3.Id})
+	require.NoError(t, err)
+	CheckCreatedStatus(t, resp)
+	require.Equal(t, publicChannel.Id, cm[0].ChannelId, "should have returned exact channel")
+	require.Equal(t, user.Id, cm[0].UserId, "should have returned exact user added to public channel")
+	require.Equal(t, user2.Id, cm[1].UserId, "should have returned exact user added to public channel")
+	require.Equal(t, user3.Id, cm[2].UserId, "should have returned exact user added to public channel")
+
+	cm, _, err = client.AddChannelMembers(context.Background(), privateChannel.Id, "", []string{user.Id, user2.Id, user3.Id})
+	require.NoError(t, err)
+	require.Equal(t, privateChannel.Id, cm[0].ChannelId, "should have returned exact channel")
+	require.Equal(t, user.Id, cm[0].UserId, "should have returned exact user added to public channel")
+	require.Equal(t, user2.Id, cm[1].UserId, "should have returned exact user added to public channel")
+	require.Equal(t, user3.Id, cm[2].UserId, "should have returned exact user added to public channel")
+}
+
 func TestAddChannelMemberFromThread(t *testing.T) {
 	t.Skip("MM-41285")
 	th := Setup(t).InitBasic()
@@ -4784,6 +4814,13 @@ func TestGetChannelsMemberCount(t *testing.T) {
 		_, _, err := client.GetChannelsMemberCount(context.Background(), channelIDs)
 		require.NoError(t, err)
 	})
+
+	t.Run("Should fail for private channels that the user is not a member of", func(t *testing.T) {
+		th.LoginBasic2()
+		channelIDs := []string{channel2.Id}
+		_, _, err := client.GetChannelsMemberCount(context.Background(), channelIDs)
+		require.Error(t, err)
+	})
 }

 func TestMoveChannel(t *testing.T) {
--- a/server/public/model/client4.go
+++ b/server/public/model/client4.go
@@ -3730,6 +3730,23 @@ func (c *Client4) AddChannelMember(ctx context.Context, channelId, userId string
 	return ch, BuildResponse(r), nil
 }

+// AddChannelMembers adds users to a channel and return an array of channel members.
+func (c *Client4) AddChannelMembers(ctx context.Context, channelId, postRootId string, userIds []string) ([]*ChannelMember, *Response, error) {
+	requestBody := map[string]any{"user_ids": userIds, "post_root_id": postRootId}
+	r, err := c.DoAPIPost(ctx, c.channelMembersRoute(channelId)+"", StringInterfaceToJSON(requestBody))
+	if err != nil {
+		return nil, BuildResponse(r), err
+	}
+	defer closeBody(r)
+
+	var ch []*ChannelMember
+	err = json.NewDecoder(r.Body).Decode(&ch)
+	if err != nil {
+		return nil, BuildResponse(r), NewAppError("AddChannelMembers", "api.marshal_error", nil, "", http.StatusInternalServerError).Wrap(err)
+	}
+	return ch, BuildResponse(r), nil
+}
+
 // AddChannelMemberWithRootId adds user to channel and return a channel member. Post add to channel message has the postRootId.
 func (c *Client4) AddChannelMemberWithRootId(ctx context.Context, channelId, userId, postRootId string) (*ChannelMember, *Response, error) {
 	requestBody := map[string]string{"user_id": userId, "post_root_id": postRootId}