MM-58255 Ensure remote users do not get valid email addresses (#27421)

* remote users don't get valid email addresses; remote users cannot have access tokens

* block notification emails for remote users

---------

Co-authored-by: Mattermost Build <build@mattermost.com>

server/channels/api4/user.go

@@ -2415,6 +2415,12 @@ func createUserAccessToken(c *Context, w http.ResponseWriter, r *http.Request) {
 
 	audit.AddEventParameterAuditable(auditRec, "user", user)
 
+	if user.IsRemote() {
+		// remote/synthetic users cannot have access tokens
+		c.SetPermissionError(model.PermissionCreateUserAccessToken)
+		return
+	}
+
 	if c.AppContext.Session().IsOAuth {
 		c.SetPermissionError(model.PermissionCreateUserAccessToken)
 		c.Err.DetailedError += ", attempted access by oauth app"

server/channels/api4/user_test.go

@@ -22,6 +22,7 @@ import (
 	"github.com/stretchr/testify/require"
 
 	"github.com/mattermost/mattermost/server/public/model"
+	"github.com/mattermost/mattermost/server/public/shared/request"
 	"github.com/mattermost/mattermost/server/v8/channels/app"
 	"github.com/mattermost/mattermost/server/v8/channels/utils/testutils"
 	"github.com/mattermost/mattermost/server/v8/einterfaces/mocks"
@@ -4634,6 +4635,26 @@ func TestCreateUserAccessToken(t *testing.T) {
 		assertToken(t, th, rtoken, th.BasicUser.Id)
 	})
 
+	t.Run("create user access token for remote user as a system admin", func(t *testing.T) {
+		th := Setup(t).InitBasic()
+		defer th.TearDown()
+
+		th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.EnableUserAccessTokens = true })
+
+		// make a remote user
+		remoteUser, appErr := th.App.CreateUser(request.TestContext(t), &model.User{
+			Username: "remoteuser",
+			RemoteId: model.NewString(model.NewId()),
+			Password: model.NewId(),
+			Email:    "remoteuser@example.com",
+		})
+		require.Nil(t, appErr)
+
+		_, resp, err := th.SystemAdminClient.CreateUserAccessToken(context.Background(), remoteUser.Id, "test token")
+		require.Error(t, err)
+		CheckForbiddenStatus(t, resp) // remote users are not allowed to have access tokens
+	})
+
 	t.Run("create access token as oauth session", func(t *testing.T) {
 		th := Setup(t).InitBasic()
 		defer th.TearDown()

server/channels/app/notification.go

@@ -1102,8 +1102,8 @@ func max(a, b int64) int64 {
 }
 
 func (a *App) userAllowsEmail(c request.CTX, user *model.User, channelMemberNotificationProps model.StringMap, post *model.Post) bool {
-	// if user is a bot account, then we do not send email
+	// if user is a bot account or remote, then we do not send email
-	if user.IsBot {
+	if user.IsBot || user.IsRemote() {
 		return false
 	}
 

server/platform/services/sharedchannel/sync_recv.go

@@ -267,7 +267,7 @@ func (scs *Service) insertSyncUser(rctx request.CTX, user *model.User, _ *model.
 		}
 
 		user.Username = mungUsername(user.Username, rc.Name, suffix, model.UserNameMaxLength)
-		user.Email = mungEmail(rc.Name, model.UserEmailMaxLength)
+		user.Email = model.NewId()
 
 		if userSaved, err = scs.server.GetStore().User().Save(rctx, user); err != nil {
 			field, ok := isConflictError(err)
@@ -321,7 +321,7 @@ func (scs *Service) updateSyncUser(rctx request.CTX, patch *model.UserPatch, use
 			suffix = strconv.FormatInt(int64(i), 10)
 		}
 		user.Username = mungUsername(user.Username, rc.Name, suffix, model.UserNameMaxLength)
-		user.Email = mungEmail(rc.Name, model.UserEmailMaxLength)
+		user.Email = model.NewId()
 
 		if update, err = scs.server.GetStore().User().Update(rctx, user, false); err != nil {
 			field, ok := isConflictError(err)

server/platform/services/sharedchannel/util.go

@@ -95,15 +95,6 @@ func mungUsername(username string, remotename string, suffix string, maxLen int)
 	return fmt.Sprintf("%s%s%s:%s%s", username, suffix, userEllipses, remotename, remoteEllipses)
 }
 
-// mungEmail creates a unique email address using a UID and remote name.
-func mungEmail(remotename string, maxLen int) string {
-	s := fmt.Sprintf("%s@%s", model.NewId(), remotename)
-	if len(s) > maxLen {
-		s = s[:maxLen]
-	}
-	return s
-}
-
 func isConflictError(err error) (string, bool) {
 	if err == nil {
 		return "", false

server/public/model/user.go

@@ -346,7 +346,7 @@ func (u *User) IsValid() *AppError {
 		}
 	}
 
-	if len(u.Email) > UserEmailMaxLength || u.Email == "" || !IsValidEmail(u.Email) {
+	if len(u.Email) > UserEmailMaxLength || u.Email == "" || (!IsValidEmail(u.Email) && !u.IsRemote()) {
 		return InvalidUserError("email", u.Id, u.Email)
 	}
 

server/public/model/user_test.go

@@ -9,9 +9,10 @@ import (
 	"strings"
 	"testing"
 
-	"github.com/mattermost/mattermost/server/public/shared/mlog"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+
+	"github.com/mattermost/mattermost/server/public/shared/mlog"
 )
 
 func TestUserDeepCopy(t *testing.T) {
@@ -117,6 +118,13 @@ func TestUserIsValid(t *testing.T) {
 	user.LastName = ""
 	require.Nil(t, user.IsValid())
 
+	user.Email = NewId()
+	appErr = user.IsValid()
+	require.True(t, HasExpectedUserIsValidError(appErr, "email", user.Id, user.Email), "expected user is valid error: %s", appErr.Error())
+
+	user.RemoteId = NewString(NewId())
+	require.Nil(t, user.IsValid())
+
 	user.FirstName = strings.Repeat("a", 65)
 	appErr = user.IsValid()
 	require.True(t, HasExpectedUserIsValidError(appErr, "first_name", user.Id, user.FirstName), "expected user is valid error: %s", appErr.Error())