From 8781c36eb3efb26658ff66a4b60334dd0fb892c8 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jes=C3=BAs=20Espino?= <jespinog@gmail.com>
Date: Tue, 28 Apr 2020 12:58:34 +0200
Subject: [PATCH] Fixing system messages about non-visible users (#14254)

* Fixing system messages about non-visible users

* Adding unit tests to verify the new behavior

* Regenerating app layers

Co-authored-by: mattermod <mattermod@users.noreply.github.com>
---
 app/app_iface.go         |  1 +
 app/notification.go      | 20 +++++++++++++++++++-
 app/notification_test.go | 21 +++++++++++++++++++++
 app/opentracing_layer.go | 22 ++++++++++++++++++++++
 4 files changed, 63 insertions(+), 1 deletion(-)

diff --git a/app/app_iface.go b/app/app_iface.go
index e09b2f5656..bae6bc8c88 100644
--- a/app/app_iface.go
+++ b/app/app_iface.go
@@ -463,6 +463,7 @@ type AppIface interface {
 	FileExists(path string) (bool, *model.AppError)
 	FillInChannelProps(channel *model.Channel) *model.AppError
 	FillInChannelsProps(channelList *model.ChannelList) *model.AppError
+	FilterUsersByVisible(viewer *model.User, otherUsers []*model.User) ([]*model.User, *model.AppError)
 	FindTeamByName(name string) bool
 	GenerateMfaSecret(userId string) (*model.MfaSecret, *model.AppError)
 	GeneratePublicLink(siteURL string, info *model.FileInfo) string
diff --git a/app/notification.go b/app/notification.go
index d652d2b0ec..6167633a5c 100644
--- a/app/notification.go
+++ b/app/notification.go
@@ -439,6 +439,20 @@ func (a *App) sendOutOfChannelMentions(sender *model.User, post *model.Post, cha
 	return true, nil
 }
 
+func (a *App) FilterUsersByVisible(viewer *model.User, otherUsers []*model.User) ([]*model.User, *model.AppError) {
+	result := []*model.User{}
+	for _, user := range otherUsers {
+		canSee, err := a.UserCanSeeOtherUser(viewer.Id, user.Id)
+		if err != nil {
+			return nil, err
+		}
+		if canSee {
+			result = append(result, user)
+		}
+	}
+	return result, nil
+}
+
 func (a *App) filterOutOfChannelMentions(sender *model.User, post *model.Post, channel *model.Channel, potentialMentions []string) ([]*model.User, []*model.User, error) {
 	if post.IsSystemMessage() {
 		return nil, nil, nil
@@ -460,6 +474,10 @@ func (a *App) filterOutOfChannelMentions(sender *model.User, post *model.Post, c
 	// Filter out inactive users and bots
 	allUsers := model.UserSlice(users).FilterByActive(true)
 	allUsers = allUsers.FilterWithoutBots()
+	allUsers, err = a.FilterUsersByVisible(sender, allUsers)
+	if err != nil {
+		return nil, nil, err
+	}
 
 	if len(allUsers) == 0 {
 		return nil, nil, nil
@@ -477,7 +495,7 @@ func (a *App) filterOutOfChannelMentions(sender *model.User, post *model.Post, c
 		outOfChannelUsers = allUsers.FilterWithoutID(nonMemberIDs)
 		outOfGroupsUsers = allUsers.FilterByID(nonMemberIDs)
 	} else {
-		outOfChannelUsers = users
+		outOfChannelUsers = allUsers
 	}
 
 	return outOfChannelUsers, outOfGroupsUsers, nil
diff --git a/app/notification_test.go b/app/notification_test.go
index 027d4db1ec..43d37e8bbe 100644
--- a/app/notification_test.go
+++ b/app/notification_test.go
@@ -162,7 +162,16 @@ func TestFilterOutOfChannelMentions(t *testing.T) {
 	user1 := th.BasicUser
 	user2 := th.BasicUser2
 	user3 := th.CreateUser()
+	guest := th.CreateGuest()
+	user4 := th.CreateUser()
+	guestAndUser4Channel := th.CreateChannel(th.BasicTeam)
+	defer th.App.PermanentDeleteUser(guest)
 	th.LinkUserToTeam(user3, th.BasicTeam)
+	th.LinkUserToTeam(user4, th.BasicTeam)
+	th.LinkUserToTeam(guest, th.BasicTeam)
+	th.App.AddUserToChannel(guest, channel)
+	th.App.AddUserToChannel(user4, guestAndUser4Channel)
+	th.App.AddUserToChannel(guest, guestAndUser4Channel)
 
 	t.Run("should return users not in the channel", func(t *testing.T) {
 		post := &model.Post{}
@@ -177,6 +186,18 @@ func TestFilterOutOfChannelMentions(t *testing.T) {
 		assert.Nil(t, outOfGroupUsers)
 	})
 
+	t.Run("should return only visible users not in the channel (for guests)", func(t *testing.T) {
+		post := &model.Post{}
+		potentialMentions := []string{user2.Username, user3.Username, user4.Username}
+
+		outOfChannelUsers, outOfGroupUsers, err := th.App.filterOutOfChannelMentions(guest, post, channel, potentialMentions)
+
+		require.Nil(t, err)
+		require.Len(t, outOfChannelUsers, 1)
+		assert.Equal(t, user4.Id, outOfChannelUsers[0].Id)
+		assert.Nil(t, outOfGroupUsers)
+	})
+
 	t.Run("should not return results for a system message", func(t *testing.T) {
 		post := &model.Post{
 			Type: model.POST_ADD_REMOVE,
diff --git a/app/opentracing_layer.go b/app/opentracing_layer.go
index 8838276a79..85be8bf41e 100644
--- a/app/opentracing_layer.go
+++ b/app/opentracing_layer.go
@@ -3525,6 +3525,28 @@ func (a *OpenTracingAppLayer) FilterNonGroupTeamMembers(userIds []string, team *
 	return resultVar0, resultVar1
 }
 
+func (a *OpenTracingAppLayer) FilterUsersByVisible(viewer *model.User, otherUsers []*model.User) ([]*model.User, *model.AppError) {
+	origCtx := a.ctx
+	span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.FilterUsersByVisible")
+
+	a.ctx = newCtx
+	a.app.Srv().Store.SetContext(newCtx)
+	defer func() {
+		a.app.Srv().Store.SetContext(origCtx)
+		a.ctx = origCtx
+	}()
+
+	defer span.Finish()
+	resultVar0, resultVar1 := a.app.FilterUsersByVisible(viewer, otherUsers)
+
+	if resultVar1 != nil {
+		span.LogFields(spanlog.Error(resultVar1))
+		ext.Error.Set(span, true)
+	}
+
+	return resultVar0, resultVar1
+}
+
 func (a *OpenTracingAppLayer) FindTeamByName(name string) bool {
 	origCtx := a.ctx
 	span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.FindTeamByName")