From 283f34b9c6d207f0a103e7b4c7f6da2c7481c3ef Mon Sep 17 00:00:00 2001 From: Joram Wilander Date: Fri, 20 Apr 2018 08:44:18 -0400 Subject: 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 | 10 +++++++ app/user.go | 9 +++++- app/web_conn.go | 22 ++++++++++++++ app/web_conn_test.go | 82 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 122 insertions(+), 1 deletion(-) create mode 100644 app/web_conn_test.go (limited to 'app') diff --git a/app/apptestlib.go b/app/apptestlib.go index 1b22831c9..a5c2db91c 100644 --- a/app/apptestlib.go +++ b/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" } diff --git a/app/user.go b/app/user.go index 80c8b6ef2..34076555e 100644 --- a/app/user.go +++ b/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) } diff --git a/app/web_conn.go b/app/web_conn.go index 33c285af3..9ae5505b2 100644 --- a/app/web_conn.go +++ b/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 { diff --git a/app/web_conn_test.go b/app/web_conn_test.go new file mode 100644 index 000000000..c378c8370 --- /dev/null +++ b/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) + } +} -- cgit v1.2.3-1-g7c22