summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorJoram Wilander <jwawilander@gmail.com>2018-04-20 08:44:18 -0400
committerGitHub <noreply@github.com>2018-04-20 08:44:18 -0400
commit283f34b9c6d207f0a103e7b4c7f6da2c7481c3ef (patch)
tree9a9d0dfb9f536d37e9817e3407c32e7ec0c11cdf
parent7987c95fcd7f7a9e6d4d174be403bf170f7b9115 (diff)
downloadchat-283f34b9c6d207f0a103e7b4c7f6da2c7481c3ef.tar.gz
chat-283f34b9c6d207f0a103e7b4c7f6da2c7481c3ef.tar.bz2
chat-283f34b9c6d207f0a103e7b4c7f6da2c7481c3ef.zip
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
-rw-r--r--app/apptestlib.go10
-rw-r--r--app/user.go9
-rw-r--r--app/web_conn.go22
-rw-r--r--app/web_conn_test.go82
-rw-r--r--model/user.go17
-rw-r--r--model/user_test.go32
-rw-r--r--model/utils.go8
-rw-r--r--model/utils_test.go12
-rw-r--r--model/websocket_message.go10
9 files changed, 197 insertions, 5 deletions
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)
+ }
+}
diff --git a/model/user.go b/model/user.go
index efe8f8db9..6fbdb09e8 100644
--- a/model/user.go
+++ b/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 {
diff --git a/model/user_test.go b/model/user_test.go
index c1d1dafbd..645eaadff 100644
--- a/model/user_test.go
+++ b/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()
diff --git a/model/utils.go b/model/utils.go
index 2d61b49f6..9f8ef90e1 100644
--- a/model/utils.go
+++ b/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)
diff --git a/model/utils_test.go b/model/utils_test.go
index 92354c0a1..b7f5dc628 100644
--- a/model/utils_test.go
+++ b/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)
diff --git a/model/websocket_message.go b/model/websocket_message.go
index 4a547bb6a..08c238480 100644
--- a/model/websocket_message.go
+++ b/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
- UserId string `json:"user_id"` // broadcast only occurs for this user
- 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
+ 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
+ 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
+ ContainsSanitizedData bool `json:"-"`
+ ContainsSensitiveData bool `json:"-"`
}
type precomputedWebSocketEventJSON struct {