MM-64531: [Shared Channels] Users on different remote servers should not communicate unless the remotes have established secure connection. (#30985) (#33434)

Automatic Merge

server/channels/api4/shared_channel.go

@@ -15,6 +15,7 @@ func (api *API) InitSharedChannels() {
 	api.BaseRoutes.SharedChannels.Handle("/{team_id:[A-Za-z0-9]+}", api.APISessionRequired(getSharedChannels)).Methods(http.MethodGet)
 	api.BaseRoutes.SharedChannels.Handle("/remote_info/{remote_id:[A-Za-z0-9]+}", api.APISessionRequired(getRemoteClusterInfo)).Methods(http.MethodGet)
 	api.BaseRoutes.SharedChannels.Handle("/{channel_id:[A-Za-z0-9]+}/remotes", api.APISessionRequired(getSharedChannelRemotes)).Methods(http.MethodGet)
+	api.BaseRoutes.SharedChannels.Handle("/users/{user_id:[A-Za-z0-9]+}/can_dm/{other_user_id:[A-Za-z0-9]+}", api.APISessionRequired(canUserDirectMessage)).Methods(http.MethodGet)
 
 	api.BaseRoutes.SharedChannelRemotes.Handle("", api.APISessionRequired(getSharedChannelRemotesByRemoteCluster)).Methods(http.MethodGet)
 	api.BaseRoutes.ChannelForRemote.Handle("/invite", api.APISessionRequired(inviteRemoteClusterToChannel)).Methods(http.MethodPost)
@@ -294,3 +295,57 @@ func getSharedChannelRemotes(c *Context, w http.ResponseWriter, r *http.Request)
 		c.Logger.Warn("Error while writing response", mlog.Err(err))
 	}
 }
+
+func canUserDirectMessage(c *Context, w http.ResponseWriter, r *http.Request) {
+	c.RequireUserId().RequireOtherUserId()
+	if c.Err != nil {
+		return
+	}
+
+	// Check if the user can see the other user at all
+	canSee, err := c.App.UserCanSeeOtherUser(c.AppContext, c.Params.UserId, c.Params.OtherUserId)
+	if err != nil {
+		c.Err = err
+		return
+	}
+	if !canSee {
+		result := map[string]bool{"can_dm": false}
+		if err := json.NewEncoder(w).Encode(result); err != nil {
+			c.Logger.Warn("Error encoding JSON response", mlog.Err(err))
+		}
+		return
+	}
+
+	canDM := true
+
+	// Get shared channel sync service for remote user checks
+	scs := c.App.Srv().GetSharedChannelSyncService()
+	if scs != nil {
+		otherUser, otherErr := c.App.GetUser(c.Params.OtherUserId)
+		if otherErr != nil {
+			canDM = false
+		} else {
+			originalRemoteId := otherUser.GetOriginalRemoteID()
+
+			// Check if the other user is from a remote cluster
+			if otherUser.IsRemote() {
+				// If original remote ID is unknown, fall back to current RemoteId as best guess
+				if originalRemoteId == model.UserOriginalRemoteIdUnknown {
+					originalRemoteId = otherUser.GetRemoteID()
+				}
+
+				// For DMs, we require a direct connection to the ORIGINAL remote cluster
+				isDirectlyConnected := scs.IsRemoteClusterDirectlyConnected(originalRemoteId)
+
+				if !isDirectlyConnected {
+					canDM = false
+				}
+			}
+		}
+	}
+
+	result := map[string]bool{"can_dm": canDM}
+	if err := json.NewEncoder(w).Encode(result); err != nil {
+		c.Logger.Warn("Error encoding JSON response", mlog.Err(err))
+	}
+}

server/channels/app/shared_channel_service_iface.go

@@ -28,6 +28,7 @@ type SharedChannelServiceIFace interface {
 	CheckChannelIsShared(channelID string) error
 	CheckCanInviteToSharedChannel(channelId string) error
 	HandleMembershipChange(channelID, userID string, isAdd bool, remoteID string)
+	IsRemoteClusterDirectlyConnected(remoteId string) bool
 	TransformMentionsOnReceiveForTesting(ctx request.CTX, post *model.Post, targetChannel *model.Channel, rc *model.RemoteCluster, mentionTransforms map[string]string)
 }
 
@@ -100,3 +101,11 @@ func (mrcs *mockSharedChannelService) HandleMembershipChange(channelID, userID s
 		mrcs.SharedChannelServiceIFace.HandleMembershipChange(channelID, userID, isAdd, remoteID)
 	}
 }
+
+func (mrcs *mockSharedChannelService) IsRemoteClusterDirectlyConnected(remoteId string) bool {
+	if mrcs.SharedChannelServiceIFace != nil {
+		return mrcs.SharedChannelServiceIFace.IsRemoteClusterDirectlyConnected(remoteId)
+	}
+	// Default behavior for mock: Local server is always connected
+	return remoteId == ""
+}

server/channels/app/user_test.go

@@ -9,6 +9,7 @@ import (
 	"database/sql"
 	"encoding/json"
 	"errors"
+	"os"
 	"path/filepath"
 	"strings"
 	"testing"
@@ -26,6 +27,7 @@ import (
 	"github.com/mattermost/mattermost/server/v8/channels/utils/testutils"
 	"github.com/mattermost/mattermost/server/v8/einterfaces"
 	"github.com/mattermost/mattermost/server/v8/einterfaces/mocks"
+	"github.com/mattermost/mattermost/server/v8/platform/services/sharedchannel"
 )
 
 func TestCreateOAuthUser(t *testing.T) {
@@ -2411,3 +2413,74 @@ func TestGetUsersForReporting(t *testing.T) {
 		require.NotNil(t, userReports)
 	})
 }
+
+// Helper functions for remote user testing
+func setupRemoteClusterTest(t *testing.T) (*TestHelper, store.Store) {
+	os.Setenv("MM_FEATUREFLAGS_ENABLESHAREDCHANNELSDMS", "true")
+	t.Cleanup(func() { os.Unsetenv("MM_FEATUREFLAGS_ENABLESHAREDCHANNELSDMS") })
+	th := setupSharedChannels(t).InitBasic()
+	t.Cleanup(th.TearDown)
+	return th, th.App.Srv().Store()
+}
+
+func createTestRemoteCluster(t *testing.T, th *TestHelper, ss store.Store, name, siteURL string, confirmed bool) *model.RemoteCluster {
+	cluster := &model.RemoteCluster{
+		RemoteId:   model.NewId(),
+		Name:       name,
+		SiteURL:    siteURL,
+		CreateAt:   model.GetMillis(),
+		LastPingAt: model.GetMillis(),
+		Token:      model.NewId(),
+		CreatorId:  th.BasicUser.Id,
+	}
+	if confirmed {
+		cluster.RemoteToken = model.NewId()
+	}
+	savedCluster, err := ss.RemoteCluster().Save(cluster)
+	require.NoError(t, err)
+	return savedCluster
+}
+
+func createRemoteUser(t *testing.T, th *TestHelper, remoteCluster *model.RemoteCluster) *model.User {
+	user := th.CreateUser()
+	user.RemoteId = &remoteCluster.RemoteId
+	updatedUser, appErr := th.App.UpdateUser(th.Context, user, false)
+	require.Nil(t, appErr)
+	return updatedUser
+}
+
+func ensureRemoteClusterConnected(t *testing.T, ss store.Store, cluster *model.RemoteCluster, connected bool) {
+	if connected {
+		cluster.SiteURL = "https://example.com"
+		cluster.RemoteToken = model.NewId()
+		cluster.LastPingAt = model.GetMillis()
+	} else {
+		cluster.SiteURL = model.SiteURLPending + "example.com"
+		cluster.RemoteToken = ""
+	}
+	_, err := ss.RemoteCluster().Update(cluster)
+	require.NoError(t, err)
+}
+
+// TestRemoteUserDirectChannelCreation tests direct channel creation with remote users
+func TestRemoteUserDirectChannelCreation(t *testing.T) {
+	th, ss := setupRemoteClusterTest(t)
+
+	connectedRC := createTestRemoteCluster(t, th, ss, "connected-cluster", "https://example-connected.com", true)
+
+	user1 := createRemoteUser(t, th, connectedRC)
+
+	t.Run("Can create DM with user from connected remote", func(t *testing.T) {
+		ensureRemoteClusterConnected(t, ss, connectedRC, true)
+
+		scs := th.App.Srv().GetSharedChannelSyncService()
+		service, ok := scs.(*sharedchannel.Service)
+		require.True(t, ok)
+		require.True(t, service.IsRemoteClusterDirectlyConnected(connectedRC.RemoteId))
+
+		channel, appErr := th.App.GetOrCreateDirectChannel(th.Context, th.BasicUser.Id, user1.Id)
+		assert.NotNil(t, channel)
+		assert.Nil(t, appErr)
+		assert.Equal(t, model.ChannelTypeDirect, channel.Type)
+	})
+}

server/channels/web/context.go

@@ -300,6 +300,17 @@ func (c *Context) RequireUserId() *Context {
 	return c
 }
 
+func (c *Context) RequireOtherUserId() *Context {
+	if c.Err != nil {
+		return c
+	}
+
+	if !model.IsValidId(c.Params.OtherUserId) {
+		c.SetInvalidURLParam("other_user_id")
+	}
+	return c
+}
+
 func (c *Context) RequireTeamId() *Context {
 	if c.Err != nil {
 		return c

server/channels/web/params.go

@@ -26,6 +26,7 @@ const (
 
 type Params struct {
 	UserId                             string
+	OtherUserId                        string
 	TeamId                             string
 	InviteId                           string
 	TokenId                            string
@@ -129,6 +130,7 @@ func ParamsFromRequest(r *http.Request) *Params {
 	query := r.URL.Query()
 
 	params.UserId = props["user_id"]
+	params.OtherUserId = props["other_user_id"]
 	params.TeamId = props["team_id"]
 	params.CategoryId = props["category_id"]
 	params.InviteId = props["invite_id"]

server/platform/services/sharedchannel/service.go

@@ -329,6 +329,29 @@ func (scs *Service) postUnshareNotification(channelID string, creatorID string,
 	}
 }
 
+// IsRemoteClusterDirectlyConnected checks if a remote cluster has a direct connection to the current server
+func (scs *Service) IsRemoteClusterDirectlyConnected(remoteId string) bool {
+	if remoteId == "" {
+		return true // Local server is always "directly connected"
+	}
+
+	// Check if the remote cluster exists and confirmed
+	rc, err := scs.server.GetStore().RemoteCluster().Get(remoteId, false)
+	if err != nil {
+		return false
+	}
+
+	isConfirmed := rc.IsConfirmed()
+	hasCreator := rc.CreatorId != ""
+
+	// For a direct connection, the remote cluster must be confirmed AND have a creator
+	// (someone on this server initiated or accepted the connection)
+	// Remote clusters known only through synthetic users won't have a creator
+	directConnection := isConfirmed && hasCreator
+
+	return directConnection
+}
+
 // OnReceiveSyncMessageForTesting is a wrapper to expose onReceiveSyncMessage for testing purposes
 // isGlobalUserSyncEnabled checks if the global user sync feature is enabled
 func (scs *Service) isGlobalUserSyncEnabled() bool {

server/platform/services/sharedchannel/sync_recv.go

@@ -298,7 +298,15 @@ func (scs *Service) upsertSyncUser(c request.CTX, user *model.User, channel *mod
 	var userSaved *model.User
 	if euser == nil {
 		// new user.  Make sure the remoteID is correct and insert the record
+		// Preserve original remote ID before overwriting RemoteId
+		originalRemoteId := user.GetRemoteID()
 		user.RemoteId = model.NewPointer(rc.RemoteId)
+		if user.Props == nil || user.Props[model.UserPropsKeyOriginalRemoteId] == "" {
+			if originalRemoteId == "" {
+				originalRemoteId = rc.RemoteId // If no original RemoteId, use current sync sender
+			}
+			user.SetProp(model.UserPropsKeyOriginalRemoteId, originalRemoteId)
+		}
 		if userSaved, err = scs.insertSyncUser(c, user, channel, rc); err != nil {
 			return nil, err
 		}

server/public/model/shared_channel.go

@@ -12,8 +12,10 @@ import (
 )
 
 const (
-	UserPropsKeyRemoteUsername = "RemoteUsername"
-	UserPropsKeyRemoteEmail    = "RemoteEmail"
+	UserPropsKeyRemoteUsername   = "RemoteUsername"
+	UserPropsKeyRemoteEmail      = "RemoteEmail"
+	UserPropsKeyOriginalRemoteId = "OriginalRemoteId"
+	UserOriginalRemoteIdUnknown  = "UNKNOWN"
 )
 
 var (

server/public/model/user.go

@@ -936,6 +936,22 @@ func (u *User) GetRemoteID() string {
 	return SafeDereference(u.RemoteId)
 }
 
+func (u *User) GetOriginalRemoteID() string {
+	if u.Props == nil {
+		if u.IsRemote() {
+			return UserOriginalRemoteIdUnknown
+		}
+		return "" // Local user
+	}
+	if originalId, exists := u.Props[UserPropsKeyOriginalRemoteId]; exists && originalId != "" {
+		return originalId
+	}
+	if u.IsRemote() {
+		return UserOriginalRemoteIdUnknown
+	}
+	return "" // Local user
+}
+
 func (u *User) GetAuthData() string {
 	return SafeDereference(u.AuthData)
 }