Marks the RemoteTeamId field of RemoteClusters as deprecated (#27364)

* Marks the RemoteTeamId field of RemoteClusters as deprecated

The `RemoteTeamId` was used both in the `RemoteCluster` model and as
part of remote invites. It existed so two different remotes could have
multiple secure connections between them, and have each of those
connections scoped to a team, sharing through each only the channels
that belong to their corresponding team.

The way that we're thinking on the feature currently only contemplates
one secure connection between two servers, and shares all the
channels through that secure connection, so this field is no longer
needed.

As we don't have a system in place for the user to choose in which
team a channel should be created from an invite, this change adds a
mechanism that checks the invite for a teamId, and if it's not
present, fetches a team from the database to create the channel
into. This makes the change backwards compatible for secure
connections that already have an established behavior and allows us to
move forward with the implementation of an alternative.

* Mark invite teamId field as deprecated

---------

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

server/channels/api4/shared_channel_test.go

@@ -108,11 +108,10 @@ func TestGetRemoteClusterById(t *testing.T) {
 
 	// create a remote cluster
 	rc := &model.RemoteCluster{
-		RemoteId:     model.NewId(),
+		RemoteId:  model.NewId(),
-		Name:         "Test1",
+		Name:      "Test1",
-		RemoteTeamId: model.NewId(),
+		SiteURL:   model.NewId(),
-		SiteURL:      model.NewId(),
+		CreatorId: model.NewId(),
-		CreatorId:    model.NewId(),
 	}
 	rc, appErr := th.App.AddRemoteCluster(rc)
 	require.Nil(t, appErr)

server/channels/app/remote_cluster_test.go

@@ -23,15 +23,14 @@ func TestAddRemoteCluster(t *testing.T) {
 	th := setupRemoteCluster(t).InitBasic()
 	defer th.TearDown()
 
-	t.Run("adding remote cluster with duplicate site url and remote team id", func(t *testing.T) {
+	t.Run("adding remote cluster with duplicate site url", func(t *testing.T) {
 		remoteCluster := &model.RemoteCluster{
-			RemoteTeamId: model.NewId(),
+			Name:        "test1",
-			Name:         "test1",
+			SiteURL:     "http://www1.example.com:8065",
-			SiteURL:      "http://www1.example.com:8065",
+			Token:       model.NewId(),
-			Token:        model.NewId(),
+			RemoteToken: model.NewId(),
-			RemoteToken:  model.NewId(),
+			Topics:      "",
-			Topics:       "",
+			CreatorId:   th.BasicUser.Id,
-			CreatorId:    th.BasicUser.Id,
 		}
 
 		_, err := th.App.AddRemoteCluster(remoteCluster)
@@ -42,60 +41,29 @@ func TestAddRemoteCluster(t *testing.T) {
 		require.NotNil(t, err, "Adding a duplicate remote cluster should error")
 		assert.Contains(t, err.Error(), i18n.T("api.remote_cluster.save_not_unique.app_error"))
 	})
-
-	t.Run("adding remote cluster with duplicate site url or remote team id is allowed", func(t *testing.T) {
-		remoteCluster := &model.RemoteCluster{
-			RemoteTeamId: model.NewId(),
-			Name:         "test2",
-			SiteURL:      "http://www2.example.com:8065",
-			Token:        model.NewId(),
-			RemoteToken:  model.NewId(),
-			Topics:       "",
-			CreatorId:    th.BasicUser.Id,
-		}
-
-		existingRemoteCluster, err := th.App.AddRemoteCluster(remoteCluster)
-		require.Nil(t, err, "Adding a remote cluster should not error")
-
-		// Same site url but different remote team id
-		remoteCluster.RemoteId = model.NewId()
-		remoteCluster.RemoteTeamId = model.NewId()
-		remoteCluster.SiteURL = existingRemoteCluster.SiteURL
-		_, err = th.App.AddRemoteCluster(remoteCluster)
-		assert.Nil(t, err, "Adding a remote cluster should not error")
-
-		// Same remote team id but different site url
-		remoteCluster.RemoteId = model.NewId()
-		remoteCluster.RemoteTeamId = existingRemoteCluster.RemoteTeamId
-		remoteCluster.SiteURL = existingRemoteCluster.SiteURL + "/new"
-		_, err = th.App.AddRemoteCluster(remoteCluster)
-		assert.Nil(t, err, "Adding a remote cluster should not error")
-	})
 }
 
 func TestUpdateRemoteCluster(t *testing.T) {
 	th := setupRemoteCluster(t).InitBasic()
 	defer th.TearDown()
 
-	t.Run("update remote cluster with an already existing site url and team id", func(t *testing.T) {
+	t.Run("update remote cluster with an already existing site url", func(t *testing.T) {
 		remoteCluster := &model.RemoteCluster{
-			RemoteTeamId: model.NewId(),
+			Name:        "test3",
-			Name:         "test3",
+			SiteURL:     "http://www3.example.com:8065",
-			SiteURL:      "http://www3.example.com:8065",
+			Token:       model.NewId(),
-			Token:        model.NewId(),
+			RemoteToken: model.NewId(),
-			RemoteToken:  model.NewId(),
+			Topics:      "",
-			Topics:       "",
+			CreatorId:   th.BasicUser.Id,
-			CreatorId:    th.BasicUser.Id,
 		}
 
 		otherRemoteCluster := &model.RemoteCluster{
-			RemoteTeamId: model.NewId(),
+			Name:        "test4",
-			Name:         "test4",
+			SiteURL:     "http://www4.example.com:8066",
-			SiteURL:      "http://www4.example.com:8066",
+			Token:       model.NewId(),
-			Token:        model.NewId(),
+			RemoteToken: model.NewId(),
-			RemoteToken:  model.NewId(),
+			Topics:      "",
-			Topics:       "",
+			CreatorId:   th.BasicUser.Id,
-			CreatorId:    th.BasicUser.Id,
 		}
 
 		_, err := th.App.AddRemoteCluster(remoteCluster)
@@ -105,31 +73,28 @@ func TestUpdateRemoteCluster(t *testing.T) {
 		require.Nil(t, err, "Adding a remote cluster should not error")
 
 		savedRemoteClustered.SiteURL = remoteCluster.SiteURL
-		savedRemoteClustered.RemoteTeamId = remoteCluster.RemoteTeamId
 		_, err = th.App.UpdateRemoteCluster(savedRemoteClustered)
 		require.NotNil(t, err, "Updating remote cluster with duplicate site url should error")
 		assert.Contains(t, err.Error(), i18n.T("api.remote_cluster.update_not_unique.app_error"))
 	})
 
-	t.Run("update remote cluster with an already existing site url or team id, is allowed", func(t *testing.T) {
+	t.Run("update remote cluster with an already existing site url, is not allowed", func(t *testing.T) {
 		remoteCluster := &model.RemoteCluster{
-			RemoteTeamId: model.NewId(),
+			Name:        "test5",
-			Name:         "test5",
+			SiteURL:     "http://www5.example.com:8065",
-			SiteURL:      "http://www5.example.com:8065",
+			Token:       model.NewId(),
-			Token:        model.NewId(),
+			RemoteToken: model.NewId(),
-			RemoteToken:  model.NewId(),
+			Topics:      "",
-			Topics:       "",
+			CreatorId:   th.BasicUser.Id,
-			CreatorId:    th.BasicUser.Id,
 		}
 
 		otherRemoteCluster := &model.RemoteCluster{
-			RemoteTeamId: model.NewId(),
+			Name:        "test6",
-			Name:         "test6",
+			SiteURL:     "http://www6.example.com:8065",
-			SiteURL:      "http://www6.example.com:8065",
+			Token:       model.NewId(),
-			Token:        model.NewId(),
+			RemoteToken: model.NewId(),
-			RemoteToken:  model.NewId(),
+			Topics:      "",
-			Topics:       "",
+			CreatorId:   th.BasicUser.Id,
-			CreatorId:    th.BasicUser.Id,
 		}
 
 		existingRemoteCluster, err := th.App.AddRemoteCluster(remoteCluster)
@@ -138,16 +103,10 @@ func TestUpdateRemoteCluster(t *testing.T) {
 		anotherExistingRemoteClustered, err := th.App.AddRemoteCluster(otherRemoteCluster)
 		require.Nil(t, err, "Adding a remote cluster should not error")
 
-		// Same site url but different remote team id
+		// Same site url
 		anotherExistingRemoteClustered.SiteURL = existingRemoteCluster.SiteURL
-		anotherExistingRemoteClustered.RemoteTeamId = model.NewId()
 		_, err = th.App.UpdateRemoteCluster(anotherExistingRemoteClustered)
-		assert.Nil(t, err, "Updating remote cluster should not error")
+		require.NotNil(t, err, "Updating remote cluster should error")
-
+		assert.Contains(t, err.Error(), i18n.T("api.remote_cluster.update_not_unique.app_error"))
-		// Same remote team id but different site url
-		anotherExistingRemoteClustered.SiteURL = existingRemoteCluster.SiteURL + "/new"
-		anotherExistingRemoteClustered.RemoteTeamId = existingRemoteCluster.RemoteTeamId
-		_, err = th.App.UpdateRemoteCluster(anotherExistingRemoteClustered)
-		assert.Nil(t, err, "Updating remote cluster should not error")
 	})
 }

server/channels/app/session_test.go

@@ -370,11 +370,10 @@ func TestGetRemoteClusterSession(t *testing.T) {
 	remoteID := model.NewId()
 
 	rc := model.RemoteCluster{
-		RemoteId:     remoteID,
+		RemoteId:  remoteID,
-		RemoteTeamId: model.NewId(),
+		Name:      "test",
-		Name:         "test",
+		Token:     token,
-		Token:        token,
+		CreatorId: model.NewId(),
-		CreatorId:    model.NewId(),
 	}
 
 	_, err := th.GetSqlStore().RemoteCluster().Save(&rc)

server/channels/app/slashcommands/command_remote.go

@@ -145,10 +145,9 @@ func (rp *RemoteProvider) doCreate(a *app.App, args *model.CommandArgs, margs ma
 
 	// Display the encrypted invitation
 	invite := &model.RemoteClusterInvite{
-		RemoteId:     rcSaved.RemoteId,
+		RemoteId: rcSaved.RemoteId,
-		RemoteTeamId: args.TeamId,
+		SiteURL:  url,
-		SiteURL:      url,
+		Token:    rcSaved.Token,
-		Token:        rcSaved.Token,
 	}
 	encrypted, err := invite.Encrypt(password)
 	if err != nil {

server/channels/store/storetest/shared_channel_store.go

@@ -698,13 +698,12 @@ func testHasRemote(t *testing.T, rctx request.CTX, ss store.Store) {
 
 func testGetRemoteForUser(t *testing.T, rctx request.CTX, ss store.Store) {
 	// add remotes, and users to simulated shared channels.
-	teamId := model.NewId()
 	channel, err := createSharedTestChannel(ss, rctx, "share_test_channel", true, nil)
 	require.NoError(t, err)
 	remotes := []*model.RemoteCluster{
-		{RemoteId: model.NewId(), SiteURL: model.NewId(), CreatorId: model.NewId(), RemoteTeamId: teamId, Name: "Test_Remote_1"},
+		{RemoteId: model.NewId(), SiteURL: model.NewId(), CreatorId: model.NewId(), Name: "Test_Remote_1"},
-		{RemoteId: model.NewId(), SiteURL: model.NewId(), CreatorId: model.NewId(), RemoteTeamId: teamId, Name: "Test_Remote_2"},
+		{RemoteId: model.NewId(), SiteURL: model.NewId(), CreatorId: model.NewId(), Name: "Test_Remote_2"},
-		{RemoteId: model.NewId(), SiteURL: model.NewId(), CreatorId: model.NewId(), RemoteTeamId: teamId, Name: "Test_Remote_3"},
+		{RemoteId: model.NewId(), SiteURL: model.NewId(), CreatorId: model.NewId(), Name: "Test_Remote_3"},
 	}
 	for _, rc := range remotes {
 		_, err := ss.RemoteCluster().Save(rc)

server/platform/services/remotecluster/invitation.go

@@ -14,14 +14,13 @@ import (
 // AcceptInvitation is called when accepting an invitation to connect with a remote cluster.
 func (rcs *Service) AcceptInvitation(invite *model.RemoteClusterInvite, name string, displayName, creatorId string, teamId string, siteURL string) (*model.RemoteCluster, error) {
 	rc := &model.RemoteCluster{
-		RemoteId:     invite.RemoteId,
+		RemoteId:    invite.RemoteId,
-		RemoteTeamId: invite.RemoteTeamId,
+		Name:        name,
-		Name:         name,
+		DisplayName: displayName,
-		DisplayName:  displayName,
+		Token:       model.NewId(),
-		Token:        model.NewId(),
+		RemoteToken: invite.Token,
-		RemoteToken:  invite.Token,
+		SiteURL:     invite.SiteURL,
-		SiteURL:      invite.SiteURL,
+		CreatorId:   creatorId,
-		CreatorId:    creatorId,
 	}
 
 	rcSaved, err := rcs.server.GetStore().RemoteCluster().Save(rc)
@@ -63,10 +62,9 @@ func (rcs *Service) AcceptInvitation(invite *model.RemoteClusterInvite, name str
 
 func makeConfirmFrame(rc *model.RemoteCluster, teamId string, siteURL string) (*model.RemoteClusterFrame, error) {
 	confirm := model.RemoteClusterInvite{
-		RemoteId:     rc.RemoteId,
+		RemoteId: rc.RemoteId,
-		RemoteTeamId: teamId,
+		SiteURL:  siteURL,
-		SiteURL:      siteURL,
+		Token:    rc.Token,
-		Token:        rc.Token,
 	}
 	confirmRaw, err := json.Marshal(confirm)
 	if err != nil {

server/platform/services/remotecluster/recv.go

@@ -62,7 +62,6 @@ func (rcs *Service) ReceiveInviteConfirmation(confirm model.RemoteClusterInvite)
 		return nil, fmt.Errorf("cannot accept invite confirmation for remote %s: %w", confirm.RemoteId, err)
 	}
 
-	rc.RemoteTeamId = confirm.RemoteTeamId
 	rc.SiteURL = confirm.SiteURL
 	rc.RemoteToken = confirm.Token
 

server/platform/services/sharedchannel/channelinvite.go

@@ -18,7 +18,7 @@ import (
 // channelInviteMsg represents an invitation for a remote cluster to start sharing a channel.
 type channelInviteMsg struct {
 	ChannelId            string            `json:"channel_id"`
-	TeamId               string            `json:"team_id"`
+	TeamId               string            `json:"team_id"` // Deprecated: this field is no longer used. It's only kept for backwards compatibility.
 	ReadOnly             bool              `json:"read_only"`
 	Name                 string            `json:"name"`
 	DisplayName          string            `json:"display_name"`
@@ -52,7 +52,6 @@ func (scs *Service) SendChannelInvite(channel *model.Channel, userId string, rc
 
 	invite := channelInviteMsg{
 		ChannelId:   channel.Id,
-		TeamId:      rc.RemoteTeamId,
 		ReadOnly:    sc.ReadOnly,
 		Name:        channel.Name,
 		DisplayName: sc.ShareDisplayName,
@@ -256,9 +255,21 @@ func (scs *Service) handleChannelCreation(invite channelInviteMsg, rc *model.Rem
 		return scs.createDirectChannel(invite, rc)
 	}
 
+	teamId := invite.TeamId
+	// if the invite doesn't have a teamId associated and until the
+	// acceptance of an invite includes selecting a team, we use the
+	// first team of the list
+	if teamId == "" {
+		teams, err := scs.server.GetStore().Team().GetAllPage(0, 1, nil)
+		if err != nil {
+			return nil, fmt.Errorf("cannot get team to create the channel `%s`: %w", invite.ChannelId, err)
+		}
+		teamId = teams[0].Id
+	}
+
 	channelNew := &model.Channel{
 		Id:          invite.ChannelId,
-		TeamId:      invite.TeamId,
+		TeamId:      teamId,
 		Type:        invite.Type,
 		DisplayName: invite.DisplayName,
 		Name:        invite.Name,

server/public/model/auditconv.go

@@ -750,7 +750,6 @@ func newRemoteCluster(r *RemoteCluster) auditRemoteCluster {
 	var rc auditRemoteCluster
 	if r != nil {
 		rc.RemoteId = r.RemoteId
-		rc.RemoteTeamId = r.RemoteTeamId
 		rc.Name = r.Name
 		rc.DisplayName = r.DisplayName
 		rc.SiteURL = r.SiteURL

server/public/model/remote_cluster.go

@@ -51,7 +51,7 @@ func (bm *Bitmask) UnsetBit(flag Bitmask) {
 
 type RemoteCluster struct {
 	RemoteId     string  `json:"remote_id"`
-	RemoteTeamId string  `json:"remote_team_id"`
+	RemoteTeamId string  `json:"remote_team_id"` // Deprecated: this field is no longer used. It's only kept for backwards compatibility.
 	Name         string  `json:"name"`
 	DisplayName  string  `json:"display_name"`
 	SiteURL      string  `json:"site_url"`
@@ -312,7 +312,7 @@ type RemoteClusterPing struct {
 // RemoteClusterInvite represents an invitation to establish a simple trust with a remote cluster.
 type RemoteClusterInvite struct {
 	RemoteId     string `json:"remote_id"`
-	RemoteTeamId string `json:"remote_team_id"`
+	RemoteTeamId string `json:"remote_team_id"` // Deprecated: this field is no longer used. It's only kept for backwards compatibility.
 	SiteURL      string `json:"site_url"`
 	Token        string `json:"token"`
 }

server/public/model/remote_cluster_test.go

@@ -130,10 +130,9 @@ func TestRemoteClusterInviteEncryption(t *testing.T) {
 
 func makeInvite(url string) RemoteClusterInvite {
 	return RemoteClusterInvite{
-		RemoteId:     NewId(),
+		RemoteId: NewId(),
-		RemoteTeamId: NewId(),
+		SiteURL:  url,
-		SiteURL:      url,
+		Token:    NewId(),
-		Token:        NewId(),
 	}
 }