Skip to content

Commit

Permalink
Sanitize RemoteEmail user prop (#27170) (#27516)
Browse files Browse the repository at this point in the history
Automatic Merge
  • Loading branch information
mattermost-build authored Jul 2, 2024
1 parent 98e51c9 commit 068c953
Show file tree
Hide file tree
Showing 6 changed files with 32 additions and 9 deletions.
2 changes: 0 additions & 2 deletions server/platform/services/sharedchannel/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,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`.
Expand Down
12 changes: 6 additions & 6 deletions server/platform/services/sharedchannel/sync_recv.go
Original file line number Diff line number Diff line change
Expand Up @@ -254,8 +254,8 @@ func (scs *Service) insertSyncUser(user *model.User, channel *model.Channel, rc
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
Expand Down Expand Up @@ -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
Expand All @@ -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++ {
Expand Down
2 changes: 1 addition & 1 deletion server/platform/services/sharedchannel/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
5 changes: 5 additions & 0 deletions server/public/model/shared_channel.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
1 change: 1 addition & 0 deletions server/public/model/user.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 = ""
Expand Down
19 changes: 19 additions & 0 deletions server/public/model/user_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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: "[email protected]",
Props: StringMap{UserPropsKeyRemoteEmail: "[email protected]"},
}

user.SanitizeProfile(nil)

require.Equal(t, "[email protected]", user.Email)
require.Equal(t, "[email protected]", user.Props[UserPropsKeyRemoteEmail])

user.SanitizeProfile(map[string]bool{"email": false})

require.Empty(t, user.Email)
require.Empty(t, user.Props[UserPropsKeyRemoteEmail])
})
}

0 comments on commit 068c953

Please sign in to comment.