From 2aff84a72e6515c2e0674c0271ae6a19c4bde5f1 Mon Sep 17 00:00:00 2001 From: Miguel de la Cruz Date: Wed, 26 Jun 2024 19:48:31 +0200 Subject: [PATCH] Sanitize RemoteEmail user prop (#27170) * Sanitize RemoteEmail user prop If the server is configured to hide user emails, the "RemoteEmail" user property will be sanitized as well, effectively hiding the real email of remote users. * fix merge conflict --------- Co-authored-by: Doug Lauder Co-authored-by: Mattermost Build --- .../services/sharedchannel/service.go | 2 -- .../services/sharedchannel/sync_recv.go | 12 ++++++------ .../platform/services/sharedchannel/util.go | 2 +- server/public/model/shared_channel.go | 5 +++++ server/public/model/user.go | 1 + server/public/model/user_test.go | 19 +++++++++++++++++++ 6 files changed, 32 insertions(+), 9 deletions(-) diff --git a/server/platform/services/sharedchannel/service.go b/server/platform/services/sharedchannel/service.go index 4b7edc988b..918d53d3ea 100644 --- a/server/platform/services/sharedchannel/service.go +++ b/server/platform/services/sharedchannel/service.go @@ -30,8 +30,6 @@ const ( NotifyMinimumDelay = time.Second * 2 MaxUpsertRetries = 25 ProfileImageSyncTimeout = time.Second * 5 - KeyRemoteUsername = "RemoteUsername" - KeyRemoteEmail = "RemoteEmail" ) // Mocks can be re-generated with `make sharedchannel-mocks`. diff --git a/server/platform/services/sharedchannel/sync_recv.go b/server/platform/services/sharedchannel/sync_recv.go index 25898445c8..5e7f172ec3 100644 --- a/server/platform/services/sharedchannel/sync_recv.go +++ b/server/platform/services/sharedchannel/sync_recv.go @@ -254,8 +254,8 @@ func (scs *Service) insertSyncUser(rctx request.CTX, user *model.User, _ *model. user = sanitizeUserForSync(user) // save the original username and email in props - user.SetProp(KeyRemoteUsername, user.Username) - user.SetProp(KeyRemoteEmail, user.Email) + user.SetProp(model.UserPropsKeyRemoteUsername, user.Username) + user.SetProp(model.UserPropsKeyRemoteEmail, user.Email) // Apply a suffix to the username until it is unique. Collisions will be quite // rare since we are joining a username that is unique at a remote site with a unique @@ -300,8 +300,8 @@ func (scs *Service) updateSyncUser(rctx request.CTX, patch *model.UserPatch, use // preserve existing real username/email since Patch will over-write them; // the real username/email in props can be updated if they don't contain colons, // meaning the update is coming from the user's origin server (not munged). - realUsername, _ := user.GetProp(KeyRemoteUsername) - realEmail, _ := user.GetProp(KeyRemoteEmail) + realUsername, _ := user.GetProp(model.UserPropsKeyRemoteUsername) + realEmail, _ := user.GetProp(model.UserPropsKeyRemoteEmail) if patch.Username != nil && !strings.Contains(*patch.Username, ":") { realUsername = *patch.Username @@ -312,8 +312,8 @@ func (scs *Service) updateSyncUser(rctx request.CTX, patch *model.UserPatch, use user.Patch(patch) user = sanitizeUserForSync(user) - user.SetProp(KeyRemoteUsername, realUsername) - user.SetProp(KeyRemoteEmail, realEmail) + user.SetProp(model.UserPropsKeyRemoteUsername, realUsername) + user.SetProp(model.UserPropsKeyRemoteEmail, realEmail) // Apply a suffix to the username until it is unique. for i := 1; i <= MaxUpsertRetries; i++ { diff --git a/server/platform/services/sharedchannel/util.go b/server/platform/services/sharedchannel/util.go index ae42e68868..31b6af979a 100644 --- a/server/platform/services/sharedchannel/util.go +++ b/server/platform/services/sharedchannel/util.go @@ -18,7 +18,7 @@ func fixMention(post *model.Post, mentionMap model.UserMentionMap, user *model.U return } - realUsername, ok := user.GetProp(KeyRemoteUsername) + realUsername, ok := user.GetProp(model.UserPropsKeyRemoteUsername) if !ok { return } diff --git a/server/public/model/shared_channel.go b/server/public/model/shared_channel.go index 63632b93c1..94f9f8b7a4 100644 --- a/server/public/model/shared_channel.go +++ b/server/public/model/shared_channel.go @@ -11,6 +11,11 @@ import ( "github.com/pkg/errors" ) +const ( + UserPropsKeyRemoteUsername = "RemoteUsername" + UserPropsKeyRemoteEmail = "RemoteEmail" +) + var ( ErrChannelAlreadyShared = errors.New("channel is already shared") ErrChannelHomedOnRemote = errors.New("channel is homed on a remote cluster") diff --git a/server/public/model/user.go b/server/public/model/user.go index 8b1b0cfc1e..e29e43d46f 100644 --- a/server/public/model/user.go +++ b/server/public/model/user.go @@ -637,6 +637,7 @@ func (u *User) Sanitize(options map[string]bool) { if len(options) != 0 && !options["email"] { u.Email = "" + delete(u.Props, UserPropsKeyRemoteEmail) } if len(options) != 0 && !options["fullname"] { u.FirstName = "" diff --git a/server/public/model/user_test.go b/server/public/model/user_test.go index 2141a52cb0..0ffd20d024 100644 --- a/server/public/model/user_test.go +++ b/server/public/model/user_test.go @@ -384,3 +384,22 @@ func TestValidateCustomStatus(t *testing.T) { assert.True(t, user0.ValidateCustomStatus()) }) } + +func TestSanitizeProfile(t *testing.T) { + t.Run("should correctly sanitize email and remote email", func(t *testing.T) { + user := &User{ + Email: "john@doe.com", + Props: StringMap{UserPropsKeyRemoteEmail: "remote@doe.com"}, + } + + user.SanitizeProfile(nil) + + require.Equal(t, "john@doe.com", user.Email) + require.Equal(t, "remote@doe.com", user.Props[UserPropsKeyRemoteEmail]) + + user.SanitizeProfile(map[string]bool{"email": false}) + + require.Empty(t, user.Email) + require.Empty(t, user.Props[UserPropsKeyRemoteEmail]) + }) +}