MM-15485: In-channel system behaviour for at-mentions and slash commands for group-constrained channels. (#10855)

* MM-15108: Prevent non-group-members from being added to group-constrained channels.

* MM-15485: Updates ephemeral message when non-group member is invited to group-constrained channel.

* MM-15485: Prevent group-permitted members from being removed from group-constrained channels. Show custom ephemeral when attempting /kick or /remove.

api4/channel.go

@@ -1166,7 +1166,7 @@ func addChannelMember(c *Context, w http.ResponseWriter, r *http.Request) {
 		}
 	}
 
-	if channel.GroupConstrained != nil && *channel.GroupConstrained {
+	if channel.IsGroupConstrained() {
 		nonMembers, err := c.App.FilterNonGroupChannelMembers([]string{member.UserId}, channel)
 		if err != nil {
 			if v, ok := err.(*model.AppError); ok {
@@ -1210,7 +1210,7 @@ func removeChannelMember(c *Context, w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	if channel.GroupConstrained != nil && *channel.GroupConstrained && (c.Params.UserId != c.App.Session.UserId) {
+	if channel.IsGroupConstrained() && (c.Params.UserId != c.App.Session.UserId) {
 		c.Err = model.NewAppError("removeChannelMember", "api.channel.remove_member.group_constrained.app_error", nil, "", http.StatusBadRequest)
 		return
 	}

api4/team.go

@@ -452,7 +452,7 @@ func addTeamMember(c *Context, w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	if team.GroupConstrained != nil && *team.GroupConstrained {
+	if team.IsGroupConstrained() {
 		nonMembers, err := c.App.FilterNonGroupTeamMembers([]string{member.UserId}, team)
 		if err != nil {
 			if v, ok := err.(*model.AppError); ok {
@@ -533,7 +533,7 @@ func addTeamMembers(c *Context, w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	if team.GroupConstrained != nil && *team.GroupConstrained {
+	if team.IsGroupConstrained() {
 		nonMembers, err := c.App.FilterNonGroupTeamMembers(memberIDs, team)
 		if err != nil {
 			if v, ok := err.(*model.AppError); ok {
@@ -599,7 +599,7 @@ func removeTeamMember(c *Context, w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	if team.GroupConstrained != nil && *team.GroupConstrained && (c.Params.UserId != c.App.Session.UserId) {
+	if team.IsGroupConstrained() && (c.Params.UserId != c.App.Session.UserId) {
 		c.Err = model.NewAppError("removeTeamMember", "api.team.remove_member.group_constrained.app_error", nil, "", http.StatusBadRequest)
 		return
 	}

app/channel.go

@@ -897,6 +897,16 @@ func (a *App) addUserToChannel(user *model.User, channel *model.Channel, teamMem
 		return channelMember, nil
 	}
 
+	if channel.IsGroupConstrained() {
+		nonMembers, err := a.FilterNonGroupChannelMembers([]string{user.Id}, channel)
+		if err != nil {
+			return nil, model.NewAppError("addUserToChannel", "api.channel.add_user_to_channel.type.app_error", nil, "", http.StatusInternalServerError)
+		}
+		if len(nonMembers) > 0 {
+			return nil, model.NewAppError("addUserToChannel", "api.channel.add_members.user_denied", map[string]interface{}{"UserIDs": nonMembers}, "", http.StatusBadRequest)
+		}
+	}
+
 	newMember := &model.ChannelMember{
 		ChannelId:   channel.Id,
 		UserId:      user.Id,
@@ -1593,6 +1603,16 @@ func (a *App) removeUserFromChannel(userIdToRemove string, removerUserId string,
 		return model.NewAppError("RemoveUserFromChannel", "api.channel.remove.default.app_error", map[string]interface{}{"Channel": model.DEFAULT_CHANNEL}, "", http.StatusBadRequest)
 	}
 
+	if channel.IsGroupConstrained() && userIdToRemove != removerUserId {
+		nonMembers, err := a.FilterNonGroupChannelMembers([]string{userIdToRemove}, channel)
+		if err != nil {
+			return model.NewAppError("removeUserFromChannel", "api.channel.remove_user_from_channel.app_error", nil, "", http.StatusInternalServerError)
+		}
+		if len(nonMembers) == 0 {
+			return model.NewAppError("removeUserFromChannel", "api.channel.remove_members.denied", map[string]interface{}{"UserIDs": nonMembers}, "", http.StatusBadRequest)
+		}
+	}
+
 	cm, err := a.GetChannelMember(channel.Id, userIdToRemove)
 	if err != nil {
 		return err

app/command_invite.go

@@ -142,8 +142,14 @@ func (me *InviteProvider) DoCommand(a *App, args *model.CommandArgs, message str
 	}
 
 	if _, err := a.AddChannelMember(userProfile.Id, channelToJoin, args.Session.UserId, ""); err != nil {
+		var text string
+		if err.Id == "api.channel.add_members.user_denied" {
+			text = args.T("api.command_invite.group_constrained_user_denied")
+		} else {
+			text = args.T("api.command_invite.fail.app_error")
+		}
 		return &model.CommandResponse{
-			Text:         args.T("api.command_invite.fail.app_error"),
+			Text:         text,
 			ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL,
 		}
 	}

app/command_remove.go

@@ -135,10 +135,16 @@ func doCommand(a *App, args *model.CommandArgs, message string) *model.CommandRe
 	}
 
 	if err = a.RemoveUserFromChannel(userProfile.Id, args.UserId, channel); err != nil {
-		return &model.CommandResponse{
-			Text: args.T(err.Id, map[string]interface{}{
+		var text string
+		if err.Id == "api.channel.remove_members.denied" {
+			text = args.T("api.command_remove.group_constrained_user_denied")
+		} else {
+			text = args.T(err.Id, map[string]interface{}{
 				"Channel": model.DEFAULT_CHANNEL,
-			}),
+			})
+		}
+		return &model.CommandResponse{
+			Text:         text,
 			ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL,
 		}
 	}

app/notification.go

@@ -135,7 +135,7 @@ func (a *App) SendNotifications(post *model.Post, team *model.Team, channel *mod
 				var outOfChannelMentions model.UserSlice
 				var outOfGroupsMentions model.UserSlice
 
-				if channel.GroupConstrained != nil && *channel.GroupConstrained {
+				if channel.IsGroupConstrained() {
 					nonMemberIDs, err := a.FilterNonGroupChannelMembers(channelMentions.IDs(), channel)
 					if err != nil {
 						return nil, err

app/team.go

@@ -437,7 +437,7 @@ func (a *App) AddUserToTeamByToken(userId string, tokenId string) (*model.Team,
 	}
 	team := result.Data.(*model.Team)
 
-	if team.GroupConstrained != nil && *team.GroupConstrained {
+	if team.IsGroupConstrained() {
 		return nil, model.NewAppError("AddUserToTeamByToken", "app.team.invite_token.group_constrained.error", nil, "", http.StatusForbidden)
 	}
 
@@ -789,7 +789,7 @@ func (a *App) AddTeamMemberByInviteId(inviteId, userId string) (*model.TeamMembe
 		return nil, err
 	}
 
-	if team.GroupConstrained != nil && *team.GroupConstrained {
+	if team.IsGroupConstrained() {
 		return nil, model.NewAppError("AddTeamMemberByInviteId", "app.team.invite_id.group_constrained.error", nil, "", http.StatusForbidden)
 	}
 

cmd/mattermost/commands/group.go

@@ -177,7 +177,7 @@ func channelGroupStatusCmdF(command *cobra.Command, args []string) error {
 		return errors.New("Unable to find channel '" + args[0] + "'")
 	}
 
-	if channel.GroupConstrained != nil && *channel.GroupConstrained {
+	if channel.IsGroupConstrained() {
 		fmt.Println("Enabled")
 	} else {
 		fmt.Println("Disabled")
@@ -271,7 +271,7 @@ func teamGroupStatusCmdF(command *cobra.Command, args []string) error {
 		return errors.New("Unable to find team '" + args[0] + "'")
 	}
 
-	if team.GroupConstrained != nil && *team.GroupConstrained {
+	if team.IsGroupConstrained() {
 		fmt.Println("Enabled")
 	} else {
 		fmt.Println("Disabled")

i18n/en.json

@@ -307,6 +307,14 @@
     "id": "api.channel.remove_member.removed",
     "translation": "%v removed from the channel."
   },
+  {
+    "id": "api.channel.remove_members.denied",
+    "translation": "Channel membership removal denied to the following users because of group constraints: {{ .UserIDs }}"
+  },
+  {
+    "id": "api.channel.remove_user_from_channel.app_error",
+    "translation": "Can not remove user from this channel type"
+  },
   {
     "id": "api.channel.rename_channel.cant_rename_direct_messages.app_error",
     "translation": "You cannot rename a direct message channel"
@@ -706,6 +714,10 @@
     "id": "api.command_invite.fail.app_error",
     "translation": "An error occurred while joining the channel."
   },
+  {
+    "id": "api.command_invite.group_constrained_user_denied",
+    "translation": "User cannot be added to this channel because it is constrained to group members only."
+  },
   {
     "id": "api.command_invite.hint",
     "translation": "@[username] ~[channel]"
@@ -910,6 +922,10 @@
     "id": "api.command_remove.direct_group.app_error",
     "translation": "You can't remove someone from a direct message channel."
   },
+  {
+    "id": "api.command_remove.group_constrained_user_denied",
+    "translation": "User cannot be removed from the channel by you because they are a member of the groups linked to this channel. To remove them from this channel, they must be removed from the linked groups."
+  },
   {
     "id": "api.command_remove.hint",
     "translation": "@[username]"
@@ -1458,11 +1474,11 @@
   },
   {
     "id": "api.post.check_for_out_of_channel_groups_mentions.message.multiple",
-    "translation": "@{{.Usernames}} and @{{.LastUsername}} did not get notified by this mention because they are not in the channel. They are also not a member of the groups linked to this channel."
+    "translation": "@{{.Usernames}} and @{{.LastUsername}} did not get notified by this mention because they are not in the channel. They cannot be added to the channel because they are not a member of the linked groups. To add them to this channel, they must be added to the linked groups."
   },
   {
     "id": "api.post.check_for_out_of_channel_groups_mentions.message.one",
-    "translation": "@{{.Username}} did not get notified by this mention because they are not in the channel. They are also not a member of the groups linked to this channel."
+    "translation": "@{{.Username}} did not get notified by this mention because they are not in the channel. They cannot be added to the channel because they are not a member of the linked groups. To add them to this channel, they must be added to the linked groups."
   },
   {
     "id": "api.post.check_for_out_of_channel_mentions.message.multiple",

model/channel.go

@@ -206,6 +206,10 @@ func (o *Channel) AddProp(key string, value interface{}) {
 	o.Props[key] = value
 }
 
+func (o *Channel) IsGroupConstrained() bool {
+	return o.GroupConstrained != nil && *o.GroupConstrained
+}
+
 func GetDMNameFromIds(userId1, userId2 string) string {
 	if userId1 > userId2 {
 		return userId2 + "__" + userId1

model/team.go

@@ -280,6 +280,10 @@ func (t *Team) Patch(patch *TeamPatch) {
 	}
 }
 
+func (t *Team) IsGroupConstrained() bool {
+	return t.GroupConstrained != nil && *t.GroupConstrained
+}
+
 func (t *TeamPatch) ToJson() string {
 	b, err := json.Marshal(t)
 	if err != nil {