From 2a28edcd93724bb2bdea049c5c5a247e16acdf6f Mon Sep 17 00:00:00 2001 From: Agniva De Sarker Date: Thu, 16 Jan 2020 13:48:08 +0530 Subject: [PATCH] MM-21481: Fixed several issues from user marshalling (#13627) * MM-21481: Fixed several issues from user marshalling - Fixed the root cause for panic by properly converting the map to a User struct. - Added a check for type conversion for extra safety. - Fixed a somewhat unrelated issue of a pointer to pointer reference. * Fix tests --- api4/user_test.go | 6 ++---- app/user.go | 2 +- app/web_conn.go | 7 ++++++- model/websocket_message.go | 10 ++++++++++ model/websocket_message_test.go | 12 +++++++++--- 5 files changed, 28 insertions(+), 9 deletions(-) diff --git a/api4/user_test.go b/api4/user_test.go index 025c896c43..7deb221067 100644 --- a/api4/user_test.go +++ b/api4/user_test.go @@ -1687,11 +1687,9 @@ func assertExpectedWebsocketEvent(t *testing.T, client *model.WebSocketClient, e func assertWebsocketEventUserUpdatedWithEmail(t *testing.T, client *model.WebSocketClient, email string) { assertExpectedWebsocketEvent(t, client, model.WEBSOCKET_EVENT_USER_UPDATED, func(event *model.WebSocketEvent) { - eventUser, ok := event.GetData()["user"].(map[string]interface{}) + eventUser, ok := event.GetData()["user"].(*model.User) require.True(t, ok, "expected user") - userEmail, ok := eventUser["email"].(string) - require.Truef(t, ok, "expected email %s, but got nil", email) - assert.Equal(t, email, userEmail) + assert.Equal(t, email, eventUser.Email) }) } diff --git a/app/user.go b/app/user.go index 8f32e5e02d..a3b03b0b07 100644 --- a/app/user.go +++ b/app/user.go @@ -1111,7 +1111,7 @@ func (a *App) sendUpdatedUserEvent(user model.User) { adminCopyOfUser := user.DeepCopy() a.SanitizeProfile(adminCopyOfUser, true) adminMessage := model.NewWebSocketEvent(model.WEBSOCKET_EVENT_USER_UPDATED, "", "", "", nil) - adminMessage.Add("user", &adminCopyOfUser) + adminMessage.Add("user", adminCopyOfUser) adminMessage.GetBroadcast().ContainsSensitiveData = true a.Publish(adminMessage) diff --git a/app/web_conn.go b/app/web_conn.go index a605f3f151..8f7d82226f 100644 --- a/app/web_conn.go +++ b/app/web_conn.go @@ -305,7 +305,12 @@ func (wc *WebConn) shouldSendEventToGuest(msg *model.WebSocketEvent) bool { switch msg.EventType() { case model.WEBSOCKET_EVENT_USER_UPDATED: - userId = msg.GetData()["user"].(*model.User).Id + user, ok := msg.GetData()["user"].(*model.User) + if !ok { + mlog.Error("webhub.shouldSendEvent: user not found in message", mlog.Any("user", msg.GetData()["user"])) + return false + } + userId = user.Id case model.WEBSOCKET_EVENT_NEW_USER: userId = msg.GetData()["user_id"].(string) default: diff --git a/model/websocket_message.go b/model/websocket_message.go index 915488963f..4d69330197 100644 --- a/model/websocket_message.go +++ b/model/websocket_message.go @@ -4,6 +4,7 @@ package model import ( + "bytes" "encoding/json" "fmt" "io" @@ -194,6 +195,15 @@ func WebSocketEventFromJson(data io.Reader) *WebSocketEvent { return nil } ev.Event = o.Event + if u, ok := o.Data["user"]; ok { + // We need to convert to and from JSON again + // because the user is in the form of a map[string]interface{}. + buf, err := json.Marshal(u) + if err != nil { + return nil + } + o.Data["user"] = UserFromJson(bytes.NewReader(buf)) + } ev.Data = o.Data ev.Broadcast = o.Broadcast ev.Sequence = o.Sequence diff --git a/model/websocket_message_test.go b/model/websocket_message_test.go index da17736f0f..cbfb830cc4 100644 --- a/model/websocket_message_test.go +++ b/model/websocket_message_test.go @@ -12,14 +12,20 @@ import ( ) func TestWebSocketEvent(t *testing.T) { - m := NewWebSocketEvent("some_event", NewId(), NewId(), NewId(), nil) + userId := NewId() + m := NewWebSocketEvent("some_event", NewId(), NewId(), userId, nil) m.Add("RootId", NewId()) + user := &User{ + Id: userId, + } + m.Add("user", user) json := m.ToJson() result := WebSocketEventFromJson(strings.NewReader(json)) require.True(t, m.IsValid(), "should be valid") - require.Equal(t, m.GetBroadcast().TeamId, result.GetBroadcast().TeamId, "Ids do not match") - require.Equal(t, m.GetData()["RootId"], result.GetData()["RootId"], "Ids do not match") + require.Equal(t, m.GetBroadcast().TeamId, result.GetBroadcast().TeamId, "Team ids do not match") + require.Equal(t, m.GetData()["RootId"], result.GetData()["RootId"], "Root ids do not match") + require.Equal(t, m.GetData()["user"].(*User).Id, result.GetData()["user"].(*User).Id, "User ids do not match") } func TestWebSocketEventImmutable(t *testing.T) {