Restricting groupmsg command to only allow to create group chats with know people (#12148)

* Restricting groupmsg command to only allow to create group chats with know people

* More generic response to the users about user he can't see

* Making the code more clean

app/command_groupmsg.go

@@ -47,14 +47,26 @@ func (me *groupmsgProvider) DoCommand(a *App, args *model.CommandArgs, message s
 	for _, username := range users {
 		username = strings.TrimSpace(username)
 		username = strings.TrimPrefix(username, "@")
-		if targetUser, err := a.Srv.Store.User().GetByUsername(username); err != nil {
+		targetUser, err := a.Srv.Store.User().GetByUsername(username)
+		if err != nil {
 			invalidUsernames = append(invalidUsernames, username)
-		} else {
+			continue
-			_, exists := targetUsers[targetUser.Id]
+		}
-			if !exists && targetUser.Id != args.UserId {
+
-				targetUsers[targetUser.Id] = targetUser
+		canSee, err := a.UserCanSeeOtherUser(args.UserId, targetUser.Id)
-				targetUsersSlice = append(targetUsersSlice, targetUser.Id)
+		if err != nil {
-			}
+			return &model.CommandResponse{Text: args.T("api.command_groupmsg.fail.app_error"), ResponseType: model.COMMAND_RESPONSE_TYPE_EPHEMERAL}
+		}
+
+		if !canSee {
+			invalidUsernames = append(invalidUsernames, username)
+			continue
+		}
+
+		_, exists := targetUsers[targetUser.Id]
+		if !exists && targetUser.Id != args.UserId {
+			targetUsers[targetUser.Id] = targetUser
+			targetUsersSlice = append(targetUsersSlice, targetUser.Id)
 		}
 	}
 

app/command_groupmsg_test.go

@@ -52,46 +52,67 @@ func TestGroupMsgProvider(t *testing.T) {
 	th.LinkUserToTeam(th.BasicUser, team)
 	cmd := &groupmsgProvider{}
 
-	// Check without permission to create a GM channel.
+	t.Run("Check without permission to create a GM channel.", func(t *testing.T) {
-	resp := cmd.DoCommand(th.App, &model.CommandArgs{
+		resp := cmd.DoCommand(th.App, &model.CommandArgs{
-		T:       i18n.IdentityTfunc(),
+			T:       i18n.IdentityTfunc(),
-		SiteURL: "http://test.url",
+			SiteURL: "http://test.url",
-		TeamId:  team.Id,
+			TeamId:  team.Id,
-		UserId:  th.BasicUser.Id,
+			UserId:  th.BasicUser.Id,
-		Session: model.Session{
+			Session: model.Session{
-			Roles: "",
+				Roles: "",
-		},
+			},
-	}, targetUsers+"hello")
+		}, targetUsers+"hello")
 
-	channelName := model.GetGroupNameFromUserIds([]string{th.BasicUser.Id, th.BasicUser2.Id, user3.Id})
+		assert.Equal(t, "api.command_groupmsg.permission.app_error", resp.Text)
-	assert.Equal(t, "api.command_groupmsg.permission.app_error", resp.Text)
+		assert.Equal(t, "", resp.GotoLocation)
-	assert.Equal(t, "", resp.GotoLocation)
+	})
 
-	// Check with permission to create a GM channel.
+	t.Run("Check without permissions to view a user in the list.", func(t *testing.T) {
-	resp = cmd.DoCommand(th.App, &model.CommandArgs{
+		th.RemovePermissionFromRole(model.PERMISSION_VIEW_MEMBERS.Id, model.SYSTEM_USER_ROLE_ID)
-		T:       i18n.IdentityTfunc(),
+		defer th.AddPermissionToRole(model.PERMISSION_VIEW_MEMBERS.Id, model.SYSTEM_USER_ROLE_ID)
-		SiteURL: "http://test.url",
+		resp := cmd.DoCommand(th.App, &model.CommandArgs{
-		TeamId:  team.Id,
+			T:       i18n.IdentityTfunc(),
-		UserId:  th.BasicUser.Id,
+			SiteURL: "http://test.url",
-		Session: model.Session{
+			TeamId:  team.Id,
-			Roles: model.SYSTEM_USER_ROLE_ID,
+			UserId:  th.BasicUser.Id,
-		},
+			Session: model.Session{
-	}, targetUsers+"hello")
+				Roles: model.SYSTEM_USER_ROLE_ID,
+			},
+		}, targetUsers+"hello")
 
-	assert.Equal(t, "", resp.Text)
+		assert.Equal(t, "api.command_groupmsg.invalid_user.app_error", resp.Text)
-	assert.Equal(t, "http://test.url/"+team.Name+"/channels/"+channelName, resp.GotoLocation)
+		assert.Equal(t, "", resp.GotoLocation)
+	})
 
-	// Check without permission to post to an existing GM channel.
+	t.Run("Check with permission to create a GM channel.", func(t *testing.T) {
-	resp = cmd.DoCommand(th.App, &model.CommandArgs{
+		resp := cmd.DoCommand(th.App, &model.CommandArgs{
-		T:       i18n.IdentityTfunc(),
+			T:       i18n.IdentityTfunc(),
-		SiteURL: "http://test.url",
+			SiteURL: "http://test.url",
-		TeamId:  team.Id,
+			TeamId:  team.Id,
-		UserId:  th.BasicUser.Id,
+			UserId:  th.BasicUser.Id,
-		Session: model.Session{
+			Session: model.Session{
-			Roles: "",
+				Roles: model.SYSTEM_USER_ROLE_ID,
-		},
+			},
-	}, targetUsers+"hello")
+		}, targetUsers+"hello")
 
-	assert.Equal(t, "", resp.Text)
+		channelName := model.GetGroupNameFromUserIds([]string{th.BasicUser.Id, th.BasicUser2.Id, user3.Id})
-	assert.Equal(t, "http://test.url/"+team.Name+"/channels/"+channelName, resp.GotoLocation)
+		assert.Equal(t, "", resp.Text)
+		assert.Equal(t, "http://test.url/"+team.Name+"/channels/"+channelName, resp.GotoLocation)
+	})
+
+	t.Run("Check without permission to post to an existing GM channel.", func(t *testing.T) {
+		resp := cmd.DoCommand(th.App, &model.CommandArgs{
+			T:       i18n.IdentityTfunc(),
+			SiteURL: "http://test.url",
+			TeamId:  team.Id,
+			UserId:  th.BasicUser.Id,
+			Session: model.Session{
+				Roles: "",
+			},
+		}, targetUsers+"hello")
+
+		channelName := model.GetGroupNameFromUserIds([]string{th.BasicUser.Id, th.BasicUser2.Id, user3.Id})
+		assert.Equal(t, "", resp.Text)
+		assert.Equal(t, "http://test.url/"+team.Name+"/channels/"+channelName, resp.GotoLocation)
+	})
 }