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
Этот коммит содержится в:
Agniva De Sarker
2020-01-16 13:48:08 +05:30
коммит произвёл Claudio Costa
родитель 87eb7697f9
Коммит 2a28edcd93
5 изменённых файлов: 28 добавлений и 9 удалений

Просмотреть файл

@@ -1687,11 +1687,9 @@ func assertExpectedWebsocketEvent(t *testing.T, client *model.WebSocketClient, e
func assertWebsocketEventUserUpdatedWithEmail(t *testing.T, client *model.WebSocketClient, email string) { func assertWebsocketEventUserUpdatedWithEmail(t *testing.T, client *model.WebSocketClient, email string) {
assertExpectedWebsocketEvent(t, client, model.WEBSOCKET_EVENT_USER_UPDATED, func(event *model.WebSocketEvent) { 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") require.True(t, ok, "expected user")
userEmail, ok := eventUser["email"].(string) assert.Equal(t, email, eventUser.Email)
require.Truef(t, ok, "expected email %s, but got nil", email)
assert.Equal(t, email, userEmail)
}) })
} }

Просмотреть файл

@@ -1111,7 +1111,7 @@ func (a *App) sendUpdatedUserEvent(user model.User) {
adminCopyOfUser := user.DeepCopy() adminCopyOfUser := user.DeepCopy()
a.SanitizeProfile(adminCopyOfUser, true) a.SanitizeProfile(adminCopyOfUser, true)
adminMessage := model.NewWebSocketEvent(model.WEBSOCKET_EVENT_USER_UPDATED, "", "", "", nil) adminMessage := model.NewWebSocketEvent(model.WEBSOCKET_EVENT_USER_UPDATED, "", "", "", nil)
adminMessage.Add("user", &adminCopyOfUser) adminMessage.Add("user", adminCopyOfUser)
adminMessage.GetBroadcast().ContainsSensitiveData = true adminMessage.GetBroadcast().ContainsSensitiveData = true
a.Publish(adminMessage) a.Publish(adminMessage)

Просмотреть файл

@@ -305,7 +305,12 @@ func (wc *WebConn) shouldSendEventToGuest(msg *model.WebSocketEvent) bool {
switch msg.EventType() { switch msg.EventType() {
case model.WEBSOCKET_EVENT_USER_UPDATED: 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: case model.WEBSOCKET_EVENT_NEW_USER:
userId = msg.GetData()["user_id"].(string) userId = msg.GetData()["user_id"].(string)
default: default:

Просмотреть файл

@@ -4,6 +4,7 @@
package model package model
import ( import (
"bytes"
"encoding/json" "encoding/json"
"fmt" "fmt"
"io" "io"
@@ -194,6 +195,15 @@ func WebSocketEventFromJson(data io.Reader) *WebSocketEvent {
return nil return nil
} }
ev.Event = o.Event 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.Data = o.Data
ev.Broadcast = o.Broadcast ev.Broadcast = o.Broadcast
ev.Sequence = o.Sequence ev.Sequence = o.Sequence

Просмотреть файл

@@ -12,14 +12,20 @@ import (
) )
func TestWebSocketEvent(t *testing.T) { 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()) m.Add("RootId", NewId())
user := &User{
Id: userId,
}
m.Add("user", user)
json := m.ToJson() json := m.ToJson()
result := WebSocketEventFromJson(strings.NewReader(json)) result := WebSocketEventFromJson(strings.NewReader(json))
require.True(t, m.IsValid(), "should be valid") 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.GetBroadcast().TeamId, result.GetBroadcast().TeamId, "Team ids do not match")
require.Equal(t, m.GetData()["RootId"], result.GetData()["RootId"], "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) { func TestWebSocketEventImmutable(t *testing.T) {