MM-10007 Send an admin and regular WS events when a user is updated (#8588)

* Add user.DeepCopy() function

* Add omit admins/non-admins to WS broadcast and use for updating users

* Updates per feedback and adding unit test for ShouldSendEvent

app/apptestlib.go

@@ -31,6 +31,8 @@ type TestHelper struct {
 	BasicChannel *model.Channel
 	BasicPost    *model.Post
 
+	SystemAdminUser *model.User
+
 	tempConfigPath string
 	tempWorkspace  string
 	pluginHooks    map[string]plugin.Hooks
@@ -143,6 +145,14 @@ func (me *TestHelper) InitBasic() *TestHelper {
 	return me
 }
 
+func (me *TestHelper) InitSystemAdmin() *TestHelper {
+	me.SystemAdminUser = me.CreateUser()
+	me.App.UpdateUserRoles(me.SystemAdminUser.Id, model.SYSTEM_USER_ROLE_ID+" "+model.SYSTEM_ADMIN_ROLE_ID, false)
+	me.SystemAdminUser, _ = me.App.GetUser(me.SystemAdminUser.Id)
+
+	return me
+}
+
 func (me *TestHelper) MakeEmail() string {
 	return "success_" + model.NewId() + "@simulator.amazonses.com"
 }

app/user.go

@@ -1021,10 +1021,17 @@ func (a *App) UpdateUserAuth(userId string, userAuth *model.UserAuth) (*model.Us
 }
 
 func (a *App) sendUpdatedUserEvent(user model.User) {
-	a.SanitizeProfile(&user, false)
+	adminCopyOfUser := user.DeepCopy()
+	a.SanitizeProfile(adminCopyOfUser, true)
+	adminMessage := model.NewWebSocketEvent(model.WEBSOCKET_EVENT_USER_UPDATED, "", "", "", nil)
+	adminMessage.Add("user", *adminCopyOfUser)
+	adminMessage.Broadcast.ContainsSensitiveData = true
+	a.Publish(adminMessage)
 
+	a.SanitizeProfile(&user, false)
 	message := model.NewWebSocketEvent(model.WEBSOCKET_EVENT_USER_UPDATED, "", "", "", nil)
 	message.Add("user", user)
+	message.Broadcast.ContainsSanitizedData = true
 	a.Publish(message)
 }
 

app/web_conn.go

@@ -287,6 +287,28 @@ func (webCon *WebConn) ShouldSendEvent(msg *model.WebSocketEvent) bool {
 		return false
 	}
 
+	// If the event contains sanitized data, only send to users that don't have permission to
+	// see sensitive data. Prevents admin clients from receiving events with bad data
+	var hasReadPrivateDataPermission *bool
+	if msg.Broadcast.ContainsSanitizedData {
+		hasReadPrivateDataPermission = model.NewBool(webCon.App.RolesGrantPermission(webCon.GetSession().GetUserRoles(), model.PERMISSION_MANAGE_SYSTEM.Id))
+
+		if *hasReadPrivateDataPermission {
+			return false
+		}
+	}
+
+	// If the event contains sensitive data, only send to users with permission to see it
+	if msg.Broadcast.ContainsSensitiveData {
+		if hasReadPrivateDataPermission == nil {
+			hasReadPrivateDataPermission = model.NewBool(webCon.App.RolesGrantPermission(webCon.GetSession().GetUserRoles(), model.PERMISSION_MANAGE_SYSTEM.Id))
+		}
+
+		if !*hasReadPrivateDataPermission {
+			return false
+		}
+	}
+
 	// If the event is destined to a specific user
 	if len(msg.Broadcast.UserId) > 0 {
 		if webCon.UserId == msg.Broadcast.UserId {

app/web_conn_test.go

@@ -0,0 +1,82 @@
+// Copyright (c) 2016-present Mattermost, Inc. All Rights Reserved.
+// See License.txt for license information.
+
+package app
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	"github.com/mattermost/mattermost-server/model"
+	"github.com/mattermost/mattermost-server/utils"
+)
+
+func TestWebConnShouldSendEvent(t *testing.T) {
+	th := Setup().InitBasic().InitSystemAdmin()
+	defer th.TearDown()
+
+	session, err := th.App.CreateSession(&model.Session{UserId: th.BasicUser.Id, Roles: th.BasicUser.GetRawRoles()})
+	require.Nil(t, err)
+
+	basicUserWc := &WebConn{
+		App:    th.App,
+		UserId: th.BasicUser.Id,
+		T:      utils.T,
+	}
+
+	basicUserWc.SetSession(session)
+	basicUserWc.SetSessionToken(session.Token)
+	basicUserWc.SetSessionExpiresAt(session.ExpiresAt)
+
+	session2, err := th.App.CreateSession(&model.Session{UserId: th.BasicUser2.Id, Roles: th.BasicUser2.GetRawRoles()})
+	require.Nil(t, err)
+
+	basicUser2Wc := &WebConn{
+		App:    th.App,
+		UserId: th.BasicUser2.Id,
+		T:      utils.T,
+	}
+
+	basicUser2Wc.SetSession(session2)
+	basicUser2Wc.SetSessionToken(session2.Token)
+	basicUser2Wc.SetSessionExpiresAt(session2.ExpiresAt)
+
+	session3, err := th.App.CreateSession(&model.Session{UserId: th.SystemAdminUser.Id, Roles: th.SystemAdminUser.GetRawRoles()})
+	require.Nil(t, err)
+
+	adminUserWc := &WebConn{
+		App:    th.App,
+		UserId: th.SystemAdminUser.Id,
+		T:      utils.T,
+	}
+
+	adminUserWc.SetSession(session3)
+	adminUserWc.SetSessionToken(session3.Token)
+	adminUserWc.SetSessionExpiresAt(session3.ExpiresAt)
+
+	cases := []struct {
+		Description   string
+		Broadcast     *model.WebsocketBroadcast
+		User1Expected bool
+		User2Expected bool
+		AdminExpected bool
+	}{
+		{"should send to all", &model.WebsocketBroadcast{}, true, true, true},
+		{"should only send to basic user", &model.WebsocketBroadcast{UserId: th.BasicUser.Id}, true, false, false},
+		{"should omit basic user 2", &model.WebsocketBroadcast{OmitUsers: map[string]bool{th.BasicUser2.Id: true}}, true, false, true},
+		{"should only send to admin", &model.WebsocketBroadcast{ContainsSensitiveData: true}, false, false, true},
+		{"should only send to non-admins", &model.WebsocketBroadcast{ContainsSanitizedData: true}, true, true, false},
+		{"should send to nobody", &model.WebsocketBroadcast{ContainsSensitiveData: true, ContainsSanitizedData: true}, false, false, false},
+		// needs more cases to get full coverage
+	}
+
+	event := &model.WebSocketEvent{Event: "some_event"}
+	for _, c := range cases {
+		event.Broadcast = c.Broadcast
+		assert.Equal(t, c.User1Expected, basicUserWc.ShouldSendEvent(event), c.Description)
+		assert.Equal(t, c.User2Expected, basicUser2Wc.ShouldSendEvent(event), c.Description)
+		assert.Equal(t, c.AdminExpected, adminUserWc.ShouldSendEvent(event), c.Description)
+	}
+}

model/user.go

@@ -96,6 +96,23 @@ type UserAuth struct {
 	AuthService string  `json:"auth_service,omitempty"`
 }
 
+func (u *User) DeepCopy() *User {
+	copyUser := *u
+	if u.AuthData != nil {
+		copyUser.AuthData = NewString(*u.AuthData)
+	}
+	if u.Props != nil {
+		copyUser.Props = CopyStringMap(u.Props)
+	}
+	if u.NotifyProps != nil {
+		copyUser.NotifyProps = CopyStringMap(u.NotifyProps)
+	}
+	if u.Timezone != nil {
+		copyUser.Timezone = CopyStringMap(u.Timezone)
+	}
+	return &copyUser
+}
+
 // IsValid validates the user and returns an error if it isn't configured
 // correctly.
 func (u *User) IsValid() *AppError {

model/user_test.go

@@ -8,6 +8,8 @@ import (
 	"net/http"
 	"strings"
 	"testing"
+
+	"github.com/stretchr/testify/assert"
 )
 
 func TestPasswordHash(t *testing.T) {
@@ -22,6 +24,36 @@ func TestPasswordHash(t *testing.T) {
 	}
 }
 
+func TestUserDeepCopy(t *testing.T) {
+	id := NewId()
+	authData := "authdata"
+	mapKey := "key"
+	mapValue := "key"
+
+	user := &User{Id: id, AuthData: NewString(authData), Props: map[string]string{}, NotifyProps: map[string]string{}, Timezone: map[string]string{}}
+	user.Props[mapKey] = mapValue
+	user.NotifyProps[mapKey] = mapValue
+	user.Timezone[mapKey] = mapValue
+
+	copyUser := user.DeepCopy()
+	copyUser.Id = "someid"
+	*copyUser.AuthData = "changed"
+	copyUser.Props[mapKey] = "changed"
+	copyUser.NotifyProps[mapKey] = "changed"
+	copyUser.Timezone[mapKey] = "changed"
+
+	assert.Equal(t, id, user.Id)
+	assert.Equal(t, authData, *user.AuthData)
+	assert.Equal(t, mapValue, user.Props[mapKey])
+	assert.Equal(t, mapValue, user.NotifyProps[mapKey])
+	assert.Equal(t, mapValue, user.Timezone[mapKey])
+
+	user = &User{Id: id}
+	copyUser = user.DeepCopy()
+
+	assert.Equal(t, id, user.Id)
+}
+
 func TestUserJson(t *testing.T) {
 	user := User{Id: NewId(), Username: NewId()}
 	json := user.ToJson()

model/utils.go

@@ -148,6 +148,14 @@ func GetMillis() int64 {
 	return time.Now().UnixNano() / int64(time.Millisecond)
 }
 
+func CopyStringMap(originalMap map[string]string) map[string]string {
+	copyMap := make(map[string]string)
+	for k, v := range originalMap {
+		copyMap[k] = v
+	}
+	return copyMap
+}
+
 // MapToJson converts a map to a json string
 func MapToJson(objmap map[string]string) string {
 	b, _ := json.Marshal(objmap)

model/utils_test.go

@@ -8,6 +8,7 @@ import (
 	"strings"
 	"testing"
 
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
 
@@ -47,6 +48,17 @@ func TestAppErrorJunk(t *testing.T) {
 	}
 }
 
+func TestCopyStringMap(t *testing.T) {
+	itemKey := "item1"
+	originalMap := make(map[string]string)
+	originalMap[itemKey] = "val1"
+
+	copyMap := CopyStringMap(originalMap)
+	copyMap[itemKey] = "changed"
+
+	assert.Equal(t, "val1", originalMap[itemKey])
+}
+
 func TestMapJson(t *testing.T) {
 
 	m := make(map[string]string)

model/websocket_message.go

@@ -57,10 +57,12 @@ type WebSocketMessage interface {
 }
 
 type WebsocketBroadcast struct {
-	OmitUsers map[string]bool `json:"omit_users"` // broadcast is omitted for users listed here
+	OmitUsers             map[string]bool `json:"omit_users"` // broadcast is omitted for users listed here
-	UserId    string          `json:"user_id"`    // broadcast only occurs for this user
+	UserId                string          `json:"user_id"`    // broadcast only occurs for this user
-	ChannelId string          `json:"channel_id"` // broadcast only occurs for users in this channel
+	ChannelId             string          `json:"channel_id"` // broadcast only occurs for users in this channel
-	TeamId    string          `json:"team_id"`    // broadcast only occurs for users in this team
+	TeamId                string          `json:"team_id"`    // broadcast only occurs for users in this team
+	ContainsSanitizedData bool            `json:"-"`
+	ContainsSensitiveData bool            `json:"-"`
 }
 
 type precomputedWebSocketEventJSON struct {