From f8202309cec7044e459db8757068699461070ed0 Mon Sep 17 00:00:00 2001 From: Miguel de la Cruz Date: Thu, 12 Sep 2024 13:55:11 +0200 Subject: [PATCH] Adds logical deletes to shared channel remotes and remote clusters (#28159) * Adds logical deletes to shared channel remotes and remote clusters Instead of physically deleting the shared channel remote and remote clusters records when a channel is unshared, a remote uninvited or a remote cluster is deleted, now those have a logical `DeleteAt` field that is set. This allows us to safely restore shared channels between two remote clusters (as of now resetting the cursor without backfilling their contents) and to know which connections were established in the past and now are severed. * Delete the index in remoteclusters before adding the new column * Fix bad error check --- api/v4/source/definitions.yaml | 6 + api/v4/source/remoteclusters.yaml | 5 + api/v4/source/sharedchannels.yaml | 5 + server/channels/api4/remote_cluster.go | 1 + server/channels/api4/remote_cluster_test.go | 47 +++- server/channels/api4/shared_channel.go | 11 +- server/channels/api4/shared_channel_test.go | 39 +++- server/channels/app/remote_cluster.go | 23 +- server/channels/app/remote_cluster_test.go | 11 +- server/channels/app/session.go | 2 +- .../app/slashcommands/command_remote.go | 9 +- .../app/slashcommands/command_share.go | 3 + server/channels/db/migrations/migrations.list | 4 + ...aredchannels_remotes_add_deleteat.down.sql | 1 + ...sharedchannels_remotes_add_deleteat.up.sql | 44 ++++ ...aredchannels_remotes_add_deleteat.down.sql | 1 + ...sharedchannels_remotes_add_deleteat.up.sql | 4 + .../store/sqlstore/remote_cluster_store.go | 43 +++- .../store/sqlstore/shared_channel_store.go | 36 ++- .../store/storetest/remote_cluster_store.go | 119 +++++++--- .../store/storetest/shared_channel_store.go | 64 +++++- server/i18n/en.json | 6 +- .../services/sharedchannel/attachment.go | 6 +- .../services/sharedchannel/channelinvite.go | 209 +++++++++++------- .../sharedchannel/channelinvite_test.go | 101 ++++++++- .../services/sharedchannel/service_api.go | 2 +- .../services/sharedchannel/sync_send.go | 3 + .../sharedchannel/sync_send_remote.go | 3 + server/public/model/client4.go | 8 +- server/public/model/remote_cluster.go | 5 + server/public/model/shared_channel.go | 2 + 31 files changed, 645 insertions(+), 178 deletions(-) create mode 100644 server/channels/db/migrations/mysql/000126_sharedchannels_remotes_add_deleteat.down.sql create mode 100644 server/channels/db/migrations/mysql/000126_sharedchannels_remotes_add_deleteat.up.sql create mode 100644 server/channels/db/migrations/postgres/000126_sharedchannels_remotes_add_deleteat.down.sql create mode 100644 server/channels/db/migrations/postgres/000126_sharedchannels_remotes_add_deleteat.up.sql diff --git a/api/v4/source/definitions.yaml b/api/v4/source/definitions.yaml index ed31359058..06968850e0 100644 --- a/api/v4/source/definitions.yaml +++ b/api/v4/source/definitions.yaml @@ -3505,6 +3505,9 @@ components: create_at: description: Time in milliseconds that the remote cluster was created type: integer + delete_at: + description: Time in milliseconds that the remote cluster record was deleted + type: integer last_ping_at: description: Time in milliseconds when the last ping to the remote cluster was run type: integer @@ -3553,6 +3556,9 @@ components: update_at: description: Time in milliseconds that the shared channel remote record was last updated type: integer + delete_at: + description: Time in milliseconds that the shared chanenl remote record was deleted + type: integer is_invite_accepted: description: Indicates if the invite has been accepted by the remote type: boolean diff --git a/api/v4/source/remoteclusters.yaml b/api/v4/source/remoteclusters.yaml index a3852b6e6f..fcec494b47 100644 --- a/api/v4/source/remoteclusters.yaml +++ b/api/v4/source/remoteclusters.yaml @@ -50,6 +50,11 @@ description: Select only remote clusters that don't belong to a plugin schema: type: boolean + - name: include_deleted + in: query + description: Include those remote clusters that have been deleted + schema: + type: boolean responses: "200": description: Remote clusters fetch successful. Result might be empty. diff --git a/api/v4/source/sharedchannels.yaml b/api/v4/source/sharedchannels.yaml index 3c2637d743..cd34a43e71 100644 --- a/api/v4/source/sharedchannels.yaml +++ b/api/v4/source/sharedchannels.yaml @@ -75,6 +75,11 @@ description: Show only those Shared channel remotes that were shared from this server schema: type: boolean + - name: include_deleted + in: query + description: Include those Shared channel remotes that have been deleted + schema: + type: boolean - name: page in: query description: The page to select diff --git a/server/channels/api4/remote_cluster.go b/server/channels/api4/remote_cluster.go index 8360e45287..993ef3139b 100644 --- a/server/channels/api4/remote_cluster.go +++ b/server/channels/api4/remote_cluster.go @@ -325,6 +325,7 @@ func getRemoteClusters(c *Context, w http.ResponseWriter, r *http.Request) { PluginID: c.Params.PluginId, OnlyPlugins: c.Params.OnlyPlugins, ExcludePlugins: c.Params.ExcludePlugins, + IncludeDeleted: c.Params.IncludeDeleted, } rcs, appErr := c.App.GetAllRemoteClusters(c.Params.Page, c.Params.PerPage, filter) diff --git a/server/channels/api4/remote_cluster_test.go b/server/channels/api4/remote_cluster_test.go index 4a6606152f..7bd5d33bff 100644 --- a/server/channels/api4/remote_cluster_test.go +++ b/server/channels/api4/remote_cluster_test.go @@ -5,7 +5,6 @@ package api4 import ( "context" - "database/sql" "encoding/base64" "testing" @@ -48,6 +47,13 @@ func TestGetRemoteClusters(t *testing.T) { CreatorId: th.SystemAdminUser.Id, PluginID: model.NewId(), }, + { + RemoteId: model.NewId(), + Name: "remote4", + SiteURL: "http://example4.com", + CreatorId: th.SystemAdminUser.Id, + DeleteAt: 123, + }, } for _, rc := range newRCs { @@ -94,6 +100,16 @@ func TestGetRemoteClusters(t *testing.T) { ExpectedError: false, ExpectedNames: []string{"remote1", "remote2", "remote3"}, }, + { + Name: "Should return all remote clusters including deleted", + Client: th.SystemAdminClient, + Page: 0, + PerPage: 999999, + Filter: model.RemoteClusterQueryFilter{IncludeDeleted: true}, + ExpectedStatusCode: 200, + ExpectedError: false, + ExpectedNames: []string{"remote1", "remote2", "remote3", "remote4"}, + }, { Name: "Should return all remote clusters but those belonging to plugins", Client: th.SystemAdminClient, @@ -104,6 +120,16 @@ func TestGetRemoteClusters(t *testing.T) { ExpectedError: false, ExpectedNames: []string{"remote1", "remote2"}, }, + { + Name: "Should return all remote clusters but those belonging to plugins, including deleted", + Client: th.SystemAdminClient, + Page: 0, + PerPage: 999999, + Filter: model.RemoteClusterQueryFilter{ExcludePlugins: true, IncludeDeleted: true}, + ExpectedStatusCode: 200, + ExpectedError: false, + ExpectedNames: []string{"remote1", "remote2", "remote4"}, + }, { Name: "Should return only remote clusters belonging to plugins", Client: th.SystemAdminClient, @@ -572,18 +598,19 @@ func TestDeleteRemoteCluster(t *testing.T) { }) t.Run("should correctly delete the remote cluster", func(t *testing.T) { + // ensure the remote cluster is not deleted + initialRC, appErr := th.App.GetRemoteCluster(rc.RemoteId) + require.Nil(t, appErr) + require.NotEmpty(t, initialRC) + require.Zero(t, initialRC.DeleteAt) + resp, err := th.SystemAdminClient.DeleteRemoteCluster(context.Background(), rc.RemoteId) CheckNoContentStatus(t, resp) require.NoError(t, err) - deletedRC, err := th.App.GetRemoteCluster(rc.RemoteId) - require.ErrorIs(t, err, sql.ErrNoRows) - require.Empty(t, deletedRC) - }) - - t.Run("should return not found if the remote cluster is already deleted", func(t *testing.T) { - resp, err := th.SystemAdminClient.DeleteRemoteCluster(context.Background(), rc.RemoteId) - CheckNotFoundStatus(t, resp) - require.Error(t, err) + deletedRC, appErr := th.App.GetRemoteCluster(rc.RemoteId) + require.Nil(t, appErr) + require.NotEmpty(t, deletedRC) + require.NotZero(t, deletedRC.DeleteAt) }) } diff --git a/server/channels/api4/shared_channel.go b/server/channels/api4/shared_channel.go index 20d2bd9ba7..2d0274f594 100644 --- a/server/channels/api4/shared_channel.go +++ b/server/channels/api4/shared_channel.go @@ -116,9 +116,10 @@ func getSharedChannelRemotesByRemoteCluster(c *Context, w http.ResponseWriter, r } filter := model.SharedChannelRemoteFilterOpts{ - RemoteId: c.Params.RemoteId, - ExcludeHome: c.Params.ExcludeHome, - ExcludeRemote: c.Params.ExcludeRemote, + RemoteId: c.Params.RemoteId, + ExcludeHome: c.Params.ExcludeHome, + ExcludeRemote: c.Params.ExcludeRemote, + IncludeDeleted: c.Params.IncludeDeleted, } sharedChannelRemotes, err := c.App.GetSharedChannelRemotes(c.Params.Page, c.Params.PerPage, filter) if err != nil { @@ -153,7 +154,7 @@ func inviteRemoteClusterToChannel(c *Context, w http.ResponseWriter, r *http.Req return } - if _, appErr := c.App.GetRemoteCluster(c.Params.RemoteId); appErr != nil { + if rc, appErr := c.App.GetRemoteCluster(c.Params.RemoteId); appErr != nil || rc.DeleteAt != 0 { c.SetInvalidRemoteIdError(c.Params.RemoteId) return } @@ -200,7 +201,7 @@ func uninviteRemoteClusterToChannel(c *Context, w http.ResponseWriter, r *http.R return } - if _, appErr := c.App.GetRemoteCluster(c.Params.RemoteId); appErr != nil { + if rc, appErr := c.App.GetRemoteCluster(c.Params.RemoteId); appErr != nil || rc.DeleteAt != 0 { c.SetInvalidRemoteIdError(c.Params.RemoteId) return } diff --git a/server/channels/api4/shared_channel_test.go b/server/channels/api4/shared_channel_test.go index 8bab899251..866272d29c 100644 --- a/server/channels/api4/shared_channel_test.go +++ b/server/channels/api4/shared_channel_test.go @@ -299,6 +299,20 @@ func TestGetSharedChannelRemotesByRemoteCluster(t *testing.T) { _, err = th.App.ShareChannel(th.Context, sc3) require.NoError(t, err) + c4 := th.CreateChannelWithClientAndTeam(th.Client, model.ChannelTypeOpen, th.BasicTeam.Id) + sc4 := &model.SharedChannel{ + ChannelId: c4.Id, + TeamId: th.BasicTeam.Id, + ShareName: "shared_4", + ShareDisplayName: "Shared Channel 4", + CreatorId: th.BasicUser.Id, + RemoteId: rc1.RemoteId, + Home: false, + } + + _, err = th.App.ShareChannel(th.Context, sc4) + require.NoError(t, err) + // for the pagination test, we need to get the channelId of the // second SharedChannelRemote that belongs to RC1, sorted by ID, // so we accumulate those SharedChannelRemotes on creation and @@ -307,7 +321,7 @@ func TestGetSharedChannelRemotesByRemoteCluster(t *testing.T) { sharedChannelRemotesFromRC1 := []*model.SharedChannelRemote{} // create the shared channel remotes - for _, sc := range []*model.SharedChannel{sc1, sc2, sc3} { + for _, sc := range []*model.SharedChannel{sc1, sc2, sc3, sc4} { scr := &model.SharedChannelRemote{ Id: model.NewId(), ChannelId: sc.ChannelId, @@ -324,6 +338,14 @@ func TestGetSharedChannelRemotesByRemoteCluster(t *testing.T) { } } + // we delete the shared channel remote for sc4 + scr4, err := th.App.GetSharedChannelRemoteByIds(sc4.ChannelId, sc4.RemoteId) + require.NoError(t, err) + + deleted, err := th.App.DeleteSharedChannelRemote(scr4.Id) + require.NoError(t, err) + require.True(t, deleted) + sort.Slice(sharedChannelRemotesFromRC1, func(i, j int) bool { return sharedChannelRemotesFromRC1[i].Id < sharedChannelRemotesFromRC1[j].Id }) @@ -335,6 +357,7 @@ func TestGetSharedChannelRemotesByRemoteCluster(t *testing.T) { RemoteId string ExcludeHome bool ExcludeRemote bool + IncludeDeleted bool Page int PerPage int ExpectedStatusCode int @@ -369,6 +392,17 @@ func TestGetSharedChannelRemotesByRemoteCluster(t *testing.T) { ExpectedError: false, ExpectedIds: []string{sc1.ChannelId, sc2.ChannelId}, }, + { + Name: "should return the complete list of shared channel remotes for a remote cluster, including deleted", + Client: th.SystemAdminClient, + RemoteId: rc1.RemoteId, + IncludeDeleted: true, + Page: 0, + PerPage: 100, + ExpectedStatusCode: http.StatusOK, + ExpectedError: false, + ExpectedIds: []string{sc1.ChannelId, sc2.ChannelId, sc4.ChannelId}, + }, { Name: "should return only the shared channel remotes homed localy", Client: th.SystemAdminClient, @@ -395,6 +429,7 @@ func TestGetSharedChannelRemotesByRemoteCluster(t *testing.T) { Name: "should correctly paginate the results", Client: th.SystemAdminClient, RemoteId: rc1.RemoteId, + IncludeDeleted: true, Page: 1, PerPage: 1, ExpectedStatusCode: http.StatusOK, @@ -405,7 +440,7 @@ func TestGetSharedChannelRemotesByRemoteCluster(t *testing.T) { for _, tc := range testCases { t.Run(tc.Name, func(t *testing.T) { - scrs, resp, err := tc.Client.GetSharedChannelRemotesByRemoteCluster(context.Background(), tc.RemoteId, tc.ExcludeHome, tc.ExcludeRemote, tc.Page, tc.PerPage) + scrs, resp, err := tc.Client.GetSharedChannelRemotesByRemoteCluster(context.Background(), tc.RemoteId, tc.ExcludeHome, tc.ExcludeRemote, tc.IncludeDeleted, tc.Page, tc.PerPage) checkHTTPStatus(t, resp, tc.ExpectedStatusCode) if tc.ExpectedError { require.Error(t, err) diff --git a/server/channels/app/remote_cluster.go b/server/channels/app/remote_cluster.go index 34d9d8b146..c2355faa8a 100644 --- a/server/channels/app/remote_cluster.go +++ b/server/channels/app/remote_cluster.go @@ -30,10 +30,20 @@ func (a *App) RegisterPluginForSharedChannels(rctx request.CTX, opts model.Regis // if plugin is already registered then treat this as an update. if rc != nil { - rctx.Logger().Debug("Plugin already registered for Shared Channels", - mlog.String("plugin_id", opts.PluginID), - mlog.String("remote_id", rc.RemoteId), - ) + // plugin was deleted at some point + if rc.DeleteAt != 0 { + rctx.Logger().Debug("Restoring plugin registration for Shared Channels", + mlog.String("plugin_id", opts.PluginID), + mlog.String("remote_id", rc.RemoteId), + ) + + rc.DeleteAt = 0 + } else { + rctx.Logger().Debug("Plugin already registered for Shared Channels", + mlog.String("plugin_id", opts.PluginID), + mlog.String("remote_id", rc.RemoteId), + ) + } rc.DisplayName = opts.Displayname rc.Options = opts.GetOptionFlags() @@ -82,6 +92,11 @@ func (a *App) UnregisterPluginForSharedChannels(pluginID string) error { return err } + if rc.DeleteAt != 0 { + // plugin already unregistered, nothing to do + return nil + } + _, appErr := a.DeleteRemoteCluster(rc.RemoteId) if appErr != nil { return appErr diff --git a/server/channels/app/remote_cluster_test.go b/server/channels/app/remote_cluster_test.go index 46082a3558..ac20a9f8ff 100644 --- a/server/channels/app/remote_cluster_test.go +++ b/server/channels/app/remote_cluster_test.go @@ -6,11 +6,9 @@ package app import ( "testing" - "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/mattermost/mattermost/server/public/model" - "github.com/mattermost/mattermost/server/public/shared/i18n" ) func setupRemoteCluster(tb testing.TB) *TestHelper { @@ -38,8 +36,7 @@ func TestAddRemoteCluster(t *testing.T) { remoteCluster.RemoteId = model.NewId() _, err = th.App.AddRemoteCluster(remoteCluster) - 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")) + require.Nil(t, err, "Adding a duplicate remote cluster should work fine") }) } @@ -74,8 +71,7 @@ func TestUpdateRemoteCluster(t *testing.T) { savedRemoteClustered.SiteURL = remoteCluster.SiteURL _, 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")) + require.Nil(t, err, "Updating remote cluster with duplicate site url should work fine") }) t.Run("update remote cluster with an already existing site url, is not allowed", func(t *testing.T) { @@ -106,7 +102,6 @@ func TestUpdateRemoteCluster(t *testing.T) { // Same site url anotherExistingRemoteClustered.SiteURL = existingRemoteCluster.SiteURL _, err = th.App.UpdateRemoteCluster(anotherExistingRemoteClustered) - require.NotNil(t, err, "Updating remote cluster should error") - assert.Contains(t, err.Error(), i18n.T("api.remote_cluster.update_not_unique.app_error")) + require.Nil(t, err, "Updating remote cluster should work fine") }) } diff --git a/server/channels/app/session.go b/server/channels/app/session.go index 1b3577f35a..480e623010 100644 --- a/server/channels/app/session.go +++ b/server/channels/app/session.go @@ -69,7 +69,7 @@ func (a *App) GetCloudSession(token string) (*model.Session, *model.AppError) { func (a *App) GetRemoteClusterSession(token string, remoteId string) (*model.Session, *model.AppError) { rc, appErr := a.GetRemoteCluster(remoteId) - if appErr == nil && subtle.ConstantTimeCompare([]byte(rc.Token), []byte(token)) == 1 { + if appErr == nil && rc.DeleteAt == 0 && subtle.ConstantTimeCompare([]byte(rc.Token), []byte(token)) == 1 { // Need a bare-bones session object for later checks session := &model.Session{ Token: token, diff --git a/server/channels/app/slashcommands/command_remote.go b/server/channels/app/slashcommands/command_remote.go index cdc729af22..a5be4e3a92 100644 --- a/server/channels/app/slashcommands/command_remote.go +++ b/server/channels/app/slashcommands/command_remote.go @@ -219,7 +219,7 @@ func (rp *RemoteProvider) doRemove(a *app.App, args *model.CommandArgs, margs ma // doStatus displays connection status for all remote clusters. func (rp *RemoteProvider) doStatus(a *app.App, args *model.CommandArgs, _ map[string]string) *model.CommandResponse { - list, err := a.GetAllRemoteClusters(0, 999999, model.RemoteClusterQueryFilter{}) + list, err := a.GetAllRemoteClusters(0, 999999, model.RemoteClusterQueryFilter{IncludeDeleted: true}) if err != nil { responsef(args.T("api.command_remote.fetch_status.error", map[string]any{"Error": err.Error()})) } @@ -230,15 +230,16 @@ func (rp *RemoteProvider) doStatus(a *app.App, args *model.CommandArgs, _ map[st var sb strings.Builder fmt.Fprintf(&sb, args.T("api.command_remote.remote_table_header")+" \n") - // | Secure Connection | Display name | ConnectionID | Site URL | Invite accepted | Online | Last ping | - fmt.Fprintf(&sb, "| :---- | :---- | :---- | :---- | :---- | :---- | :---- | \n") + // | Secure Connection | Display name | ConnectionID | Site URL | Default Team | Invite accepted | Online | Last ping | Deleted | + fmt.Fprintf(&sb, "| :---- | :---- | :---- | :---- | :---- | :---- | :---- | :---- | | :---- |\n") for _, rc := range list { accepted := formatBool(args.T, rc.IsConfirmed()) online := formatBool(args.T, isOnline(rc.LastPingAt)) lastPing := formatTimestamp(rc.LastPingAt) + deleted := formatBool(args.T, rc.DeleteAt != 0) - fmt.Fprintf(&sb, "| %s | %s | %s | %s | %s | %s | %s |\n", rc.Name, rc.DisplayName, rc.RemoteId, rc.GetSiteURL(), accepted, online, lastPing) + fmt.Fprintf(&sb, "| %s | %s | %s | %s | %s | %s | %s | %s | %s |\n", rc.Name, rc.DisplayName, rc.RemoteId, rc.GetSiteURL(), rc.DefaultTeamId, accepted, online, lastPing, deleted) } return responsef(sb.String()) } diff --git a/server/channels/app/slashcommands/command_share.go b/server/channels/app/slashcommands/command_share.go index 3b3d2a4348..1de9bacd83 100644 --- a/server/channels/app/slashcommands/command_share.go +++ b/server/channels/app/slashcommands/command_share.go @@ -252,6 +252,9 @@ func (sp *ShareProvider) doInviteRemote(a *app.App, c request.CTX, args *model.C if appErr != nil { return responsef(args.T("api.command_share.remote_id_invalid.error", map[string]any{"Error": appErr.Error()})) } + if rc.DeleteAt != 0 { + return responsef(args.T("api.command_share.remote_id_invalid.error", map[string]any{"Error": "entity is deleted"})) + } if err = a.InviteRemoteToChannel(args.ChannelId, remoteID, args.UserId, true); err != nil { return responsef(args.T("api.command_share.invite_remote_to_channel.error", map[string]any{"Error": err.Error()})) diff --git a/server/channels/db/migrations/migrations.list b/server/channels/db/migrations/migrations.list index 79123a7709..d17b261f32 100644 --- a/server/channels/db/migrations/migrations.list +++ b/server/channels/db/migrations/migrations.list @@ -247,6 +247,8 @@ channels/db/migrations/mysql/000124_remove_manage_team_permission.down.sql channels/db/migrations/mysql/000124_remove_manage_team_permission.up.sql channels/db/migrations/mysql/000125_remoteclusters_add_default_team_id.down.sql channels/db/migrations/mysql/000125_remoteclusters_add_default_team_id.up.sql +channels/db/migrations/mysql/000126_sharedchannels_remotes_add_deleteat.down.sql +channels/db/migrations/mysql/000126_sharedchannels_remotes_add_deleteat.up.sql channels/db/migrations/postgres/000001_create_teams.down.sql channels/db/migrations/postgres/000001_create_teams.up.sql channels/db/migrations/postgres/000002_create_team_members.down.sql @@ -495,3 +497,5 @@ channels/db/migrations/postgres/000124_remove_manage_team_permission.down.sql channels/db/migrations/postgres/000124_remove_manage_team_permission.up.sql channels/db/migrations/postgres/000125_remoteclusters_add_default_team_id.down.sql channels/db/migrations/postgres/000125_remoteclusters_add_default_team_id.up.sql +channels/db/migrations/postgres/000126_sharedchannels_remotes_add_deleteat.down.sql +channels/db/migrations/postgres/000126_sharedchannels_remotes_add_deleteat.up.sql diff --git a/server/channels/db/migrations/mysql/000126_sharedchannels_remotes_add_deleteat.down.sql b/server/channels/db/migrations/mysql/000126_sharedchannels_remotes_add_deleteat.down.sql new file mode 100644 index 0000000000..4743bd6462 --- /dev/null +++ b/server/channels/db/migrations/mysql/000126_sharedchannels_remotes_add_deleteat.down.sql @@ -0,0 +1 @@ +-- Skipping it because the forward migrations are destructive diff --git a/server/channels/db/migrations/mysql/000126_sharedchannels_remotes_add_deleteat.up.sql b/server/channels/db/migrations/mysql/000126_sharedchannels_remotes_add_deleteat.up.sql new file mode 100644 index 0000000000..e0343a248c --- /dev/null +++ b/server/channels/db/migrations/mysql/000126_sharedchannels_remotes_add_deleteat.up.sql @@ -0,0 +1,44 @@ +SET @preparedStatement = (SELECT IF( + ( + SELECT COUNT(*) FROM INFORMATION_SCHEMA.STATISTICS + WHERE table_name = 'RemoteClusters' + AND table_schema = DATABASE() + AND index_name = 'remote_clusters_site_url_unique' + ) > 0, + 'DROP INDEX remote_clusters_site_url_unique ON RemoteClusters;', + 'SELECT 1' +)); + +PREPARE removeIndexIfExists FROM @preparedStatement; +EXECUTE removeIndexIfExists; +DEALLOCATE PREPARE removeIndexIfExists; + +SET @preparedStatement = (SELECT IF( + ( + SELECT COUNT(*) FROM INFORMATION_SCHEMA.COLUMNS + WHERE table_name = 'RemoteClusters' + AND table_schema = DATABASE() + AND column_name = 'DeleteAt' + ) > 0, + 'SELECT 1', + 'ALTER TABLE RemoteClusters ADD DeleteAt bigint(20) DEFAULT 0;' +)); + +PREPARE alterIfNotExists FROM @preparedStatement; +EXECUTE alterIfNotExists; +DEALLOCATE PREPARE alterIfNotExists; + +SET @preparedStatement = (SELECT IF( + ( + SELECT COUNT(*) FROM INFORMATION_SCHEMA.COLUMNS + WHERE table_name = 'SharedChannelRemotes' + AND table_schema = DATABASE() + AND column_name = 'DeleteAt' + ) > 0, + 'SELECT 1', + 'ALTER TABLE SharedChannelRemotes ADD DeleteAt bigint(20) DEFAULT 0;' +)); + +PREPARE alterIfNotExists FROM @preparedStatement; +EXECUTE alterIfNotExists; +DEALLOCATE PREPARE alterIfNotExists; diff --git a/server/channels/db/migrations/postgres/000126_sharedchannels_remotes_add_deleteat.down.sql b/server/channels/db/migrations/postgres/000126_sharedchannels_remotes_add_deleteat.down.sql new file mode 100644 index 0000000000..7e9ccde63d --- /dev/null +++ b/server/channels/db/migrations/postgres/000126_sharedchannels_remotes_add_deleteat.down.sql @@ -0,0 +1 @@ +-- Skipping it because the forward migrations are destructive \ No newline at end of file diff --git a/server/channels/db/migrations/postgres/000126_sharedchannels_remotes_add_deleteat.up.sql b/server/channels/db/migrations/postgres/000126_sharedchannels_remotes_add_deleteat.up.sql new file mode 100644 index 0000000000..7b6cc5f0ad --- /dev/null +++ b/server/channels/db/migrations/postgres/000126_sharedchannels_remotes_add_deleteat.up.sql @@ -0,0 +1,4 @@ +DROP INDEX IF EXISTS remote_clusters_site_url_unique; +ALTER TABLE remoteclusters ADD COLUMN IF NOT EXISTS deleteat bigint DEFAULT 0; + +ALTER TABLE sharedchannelremotes ADD COLUMN IF NOT EXISTS deleteat bigint DEFAULT 0; diff --git a/server/channels/store/sqlstore/remote_cluster_store.go b/server/channels/store/sqlstore/remote_cluster_store.go index 4f3864af27..7db7ee2388 100644 --- a/server/channels/store/sqlstore/remote_cluster_store.go +++ b/server/channels/store/sqlstore/remote_cluster_store.go @@ -36,6 +36,7 @@ func remoteClusterFields(prefix string) []string { prefix + "SiteURL", prefix + "DefaultTeamId", prefix + "CreateAt", + prefix + "DeleteAt", prefix + "LastPingAt", prefix + "Token", prefix + "RemoteToken", @@ -67,10 +68,10 @@ func (s sqlRemoteClusterStore) Save(remoteCluster *model.RemoteCluster) (*model. query := `INSERT INTO RemoteClusters (RemoteId, RemoteTeamId, Name, DisplayName, SiteURL, DefaultTeamId, CreateAt, - LastPingAt, Token, RemoteToken, Topics, CreatorId, PluginID, Options) + DeleteAt, LastPingAt, Token, RemoteToken, Topics, CreatorId, PluginID, Options) VALUES (:RemoteId, :RemoteTeamId, :Name, :DisplayName, :SiteURL, :DefaultTeamId, :CreateAt, - :LastPingAt, :Token, :RemoteToken, :Topics, :CreatorId, :PluginID, :Options)` + :DeleteAt, :LastPingAt, :Token, :RemoteToken, :Topics, :CreatorId, :PluginID, :Options)` if _, err := s.GetMasterX().NamedExec(query, remoteCluster); err != nil { return nil, errors.Wrap(err, "failed to save RemoteCluster") @@ -89,6 +90,7 @@ func (s sqlRemoteClusterStore) Update(remoteCluster *model.RemoteCluster) (*mode SET Token = :Token, RemoteTeamId = :RemoteTeamId, CreateAt = :CreateAt, + DeleteAt = :DeleteAt, LastPingAt = :LastPingAt, RemoteToken = :RemoteToken, CreatorId = :CreatorId, @@ -107,24 +109,53 @@ func (s sqlRemoteClusterStore) Update(remoteCluster *model.RemoteCluster) (*mode } func (s sqlRemoteClusterStore) Delete(remoteId string) (bool, error) { + transaction, err := s.GetMasterX().Beginx() + if err != nil { + return false, errors.Wrap(err, "DeleteRemoteCluster: begin_transaction") + } + defer finalizeTransactionX(transaction, &err) + + curTime := model.GetMillis() + + // we delete the remote cluster itself squery, args, err := s.getQueryBuilder(). - Delete("RemoteClusters"). + Update("RemoteClusters"). + Set("DeleteAt", curTime). Where(sq.Eq{"RemoteId": remoteId}). ToSql() if err != nil { return false, errors.Wrap(err, "delete_remote_cluster_tosql") } - result, err := s.GetMasterX().Exec(squery, args...) + result, err := transaction.Exec(squery, args...) if err != nil { return false, errors.Wrap(err, "failed to delete RemoteCluster") } + // also remove the shared channel remotes for the cluster (if any) + squery, args, err = s.getQueryBuilder(). + Update("SharedChannelRemotes"). + Set("UpdateAt", curTime). + Set("DeleteAt", curTime). + Where(sq.Eq{"RemoteId": remoteId}). + ToSql() + if err != nil { + return false, errors.Wrap(err, "delete_shared_channel_remotes_for_remote_cluster_tosql") + } + + if _, err = transaction.Exec(squery, args...); err != nil { + return false, errors.Wrap(err, "failed to delete SharedChannelRemotes for RemoteCluster") + } + count, err := result.RowsAffected() if err != nil { return false, errors.Wrap(err, "failed to determine rows affected") } + if err = transaction.Commit(); err != nil { + return false, errors.Wrap(err, "commit_transaction") + } + return count > 0, nil } @@ -213,6 +244,10 @@ func (s sqlRemoteClusterStore) GetAll(offset, limit int, filter model.RemoteClus query = query.Where(sq.NotEq{fmt.Sprintf("(rc.Options & %d)", filter.RequireOptions): 0}) } + if !filter.IncludeDeleted { + query = query.Where(sq.Eq{"DeleteAt": 0}) + } + if filter.Topic != "" { trimmed := strings.TrimSpace(filter.Topic) if trimmed == "" || trimmed == "*" { diff --git a/server/channels/store/sqlstore/shared_channel_store.go b/server/channels/store/sqlstore/shared_channel_store.go index 309d096246..c6ccf916da 100644 --- a/server/channels/store/sqlstore/shared_channel_store.go +++ b/server/channels/store/sqlstore/shared_channel_store.go @@ -269,8 +269,10 @@ func (s SqlSharedChannelStore) Update(sc *model.SharedChannel) (*model.SharedCha return sc, nil } -// Delete deletes a single shared channel plus associated SharedChannelRemotes. -// Returns true if shared channel found and deleted, false if not found. +// Delete deletes a single shared channel as deleted, plus marks as +// deleted associated SharedChannelRemotes. +// Returns true if shared channel found and deleted, false if not +// found. func (s SqlSharedChannelStore) Delete(channelId string) (ok bool, err error) { transaction, err := s.GetMasterX().Beginx() if err != nil { @@ -291,17 +293,20 @@ func (s SqlSharedChannelStore) Delete(channelId string) (ok bool, err error) { return false, errors.Wrap(err, "failed to delete SharedChannel") } + curTime := model.GetMillis() + // Also remove remotes from SharedChannelRemotes (if any). squery, args, err = s.getQueryBuilder(). - Delete("SharedChannelRemotes"). + Update("SharedChannelRemotes"). + Set("UpdateAt", curTime). + Set("DeleteAt", curTime). Where(sq.Eq{"ChannelId": channelId}). ToSql() if err != nil { return false, errors.Wrap(err, "delete_shared_channel_remotes_tosql") } - _, err = transaction.Exec(squery, args...) - if err != nil { + if _, err = transaction.Exec(squery, args...); err != nil { return false, errors.Wrap(err, "failed to delete SharedChannelRemotes") } @@ -337,10 +342,10 @@ func (s SqlSharedChannelStore) SaveRemote(remote *model.SharedChannelRemote) (*m } query, args, err := s.getQueryBuilder().Insert("SharedChannelRemotes"). - Columns("Id", "ChannelId", "CreatorId", "CreateAt", "UpdateAt", "IsInviteAccepted", "IsInviteConfirmed", "RemoteId", + Columns("Id", "ChannelId", "CreatorId", "CreateAt", "UpdateAt", "DeleteAt", "IsInviteAccepted", "IsInviteConfirmed", "RemoteId", "LastPostCreateAt", "LastPostCreateId", "LastPostUpdateAt", "LastPostId"). - Values(remote.Id, remote.ChannelId, remote.CreatorId, remote.CreateAt, remote.UpdateAt, remote.IsInviteAccepted, remote.IsInviteConfirmed, remote.RemoteId, - remote.LastPostCreateAt, remote.LastPostCreateID, remote.LastPostUpdateAt, remote.LastPostUpdateID). + Values(remote.Id, remote.ChannelId, remote.CreatorId, remote.CreateAt, remote.UpdateAt, remote.DeleteAt, remote.IsInviteAccepted, remote.IsInviteConfirmed, + remote.RemoteId, remote.LastPostCreateAt, remote.LastPostCreateID, remote.LastPostUpdateAt, remote.LastPostUpdateID). ToSql() if err != nil { return nil, errors.Wrapf(err, "savesharedchannelremote_tosql") @@ -362,6 +367,7 @@ func (s SqlSharedChannelStore) UpdateRemote(remote *model.SharedChannelRemote) ( Set("CreatorId", remote.CreatorId). Set("CreateAt", remote.CreateAt). Set("UpdateAt", remote.UpdateAt). + Set("DeleteAt", remote.DeleteAt). Set("IsInviteAccepted", remote.IsInviteAccepted). Set("IsInviteConfirmed", remote.IsInviteConfirmed). Set("RemoteId", remote.RemoteId). @@ -403,6 +409,7 @@ func sharedChannelRemoteFields(prefix string) []string { prefix + "CreatorId", prefix + "CreateAt", prefix + "UpdateAt", + prefix + "DeleteAt", prefix + "IsInviteAccepted", prefix + "IsInviteConfirmed", prefix + "RemoteId", @@ -504,6 +511,10 @@ func (s SqlSharedChannelStore) GetRemotes(offset, limit int, opts model.SharedCh query = query.Offset(uint64(offset)).Limit(uint64(limit)) + if !opts.IncludeDeleted { + query = query.Where(sq.Eq{"DeleteAt": 0}) + } + squery, args, err := query.ToSql() if err != nil { return nil, errors.Wrapf(err, "get_shared_channel_remotes_tosql") @@ -526,6 +537,7 @@ func (s SqlSharedChannelStore) HasRemote(channelID string, remoteId string) (boo From("SharedChannelRemotes"). Where(sq.Eq{"RemoteId": remoteId}). Where(sq.Eq{"ChannelId": channelID}). + Where(sq.Eq{"DeleteAt": 0}). Suffix(")") query, args, err := builder.ToSql() @@ -549,6 +561,7 @@ func (s SqlSharedChannelStore) GetRemoteForUser(remoteId string, userId string) Join("SharedChannelRemotes AS scr ON rc.RemoteId = scr.RemoteId"). Join("ChannelMembers AS cm ON scr.ChannelId = cm.ChannelId"). Where(sq.Eq{"rc.RemoteId": remoteId}). + Where(sq.Eq{"scr.DeleteAt": 0}). Where(sq.Eq{"cm.UserId": userId}) query, args, err := builder.ToSql() @@ -614,8 +627,12 @@ func (s SqlSharedChannelStore) UpdateRemoteCursor(id string, cursor model.GetPos // DeleteRemote deletes a single shared channel remote. // Returns true if remote found and deleted, false if not found. func (s SqlSharedChannelStore) DeleteRemote(id string) (bool, error) { + curTime := model.GetMillis() + squery, args, err := s.getQueryBuilder(). - Delete("SharedChannelRemotes"). + Update("SharedChannelRemotes"). + Set("DeleteAt", curTime). + Set("UpdateAt", curTime). Where(sq.Eq{"Id": id}). ToSql() if err != nil { @@ -644,6 +661,7 @@ func (s SqlSharedChannelStore) GetRemotesStatus(channelId string) ([]*model.Shar Select("scr.ChannelId, rc.DisplayName, rc.SiteURL, rc.LastPingAt, sc.ReadOnly, scr.IsInviteAccepted"). From("SharedChannelRemotes scr, RemoteClusters rc, SharedChannels sc"). Where("scr.RemoteId = rc.RemoteId"). + Where("scr.DeleteAt = 0"). Where("scr.ChannelId = sc.ChannelId"). Where(sq.Eq{"scr.ChannelId": channelId}) diff --git a/server/channels/store/storetest/remote_cluster_store.go b/server/channels/store/storetest/remote_cluster_store.go index 2fe2db9605..782f96bef8 100644 --- a/server/channels/store/storetest/remote_cluster_store.go +++ b/server/channels/store/storetest/remote_cluster_store.go @@ -145,7 +145,7 @@ func testRemoteClusterSave(t *testing.T, _ request.CTX, ss store.Store) { }) } -func testRemoteClusterDelete(t *testing.T, _ request.CTX, ss store.Store) { +func testRemoteClusterDelete(t *testing.T, rctx request.CTX, ss store.Store) { t.Run("Delete", func(t *testing.T) { rc := &model.RemoteCluster{ Name: "shortlived_remote", @@ -158,6 +158,57 @@ func testRemoteClusterDelete(t *testing.T, _ request.CTX, ss store.Store) { deleted, err := ss.RemoteCluster().Delete(rcSaved.RemoteId) require.NoError(t, err) require.True(t, deleted) + + deletedRC, err := ss.RemoteCluster().Get(rcSaved.RemoteId) + require.NoError(t, err) + require.NotZero(t, deletedRC.DeleteAt) + }) + + t.Run("Delete with shared channel remotes", func(t *testing.T) { + rc := &model.RemoteCluster{ + Name: "shortlived_remote", + SiteURL: makeSiteURL(), + CreatorId: model.NewId(), + } + rcSaved, err := ss.RemoteCluster().Save(rc) + require.NoError(t, err) + + // we create a shared channel remote for the remote cluster + channel, err := createTestChannel(ss, rctx, "test_delete") + require.NoError(t, err) + + sc := &model.SharedChannel{ + ChannelId: channel.Id, + TeamId: channel.TeamId, + CreatorId: model.NewId(), + ShareName: "testshare", + RemoteId: model.NewId(), + } + + _, err = ss.SharedChannel().Save(sc) + require.NoError(t, err, "couldn't save shared channel", err) + + scr := &model.SharedChannelRemote{ + ChannelId: channel.Id, + CreatorId: model.NewId(), + RemoteId: rc.RemoteId, + } + scrSaved, err := ss.SharedChannel().SaveRemote(scr) + require.NoError(t, err) + + // and then we delete the cluster, expecting the shared + // channel remote to be deleted as well + deleted, err := ss.RemoteCluster().Delete(rcSaved.RemoteId) + require.NoError(t, err) + require.True(t, deleted) + + deletedRC, err := ss.RemoteCluster().Get(rcSaved.RemoteId) + require.NoError(t, err) + require.NotZero(t, deletedRC.DeleteAt) + + deletedSCR, err := ss.SharedChannel().GetRemote(scrSaved.Id) + require.NoError(t, err) + require.NotZero(t, deletedSCR.DeleteAt) }) t.Run("Delete nonexistent", func(t *testing.T) { @@ -218,7 +269,7 @@ func testRemoteClusterGetByPluginID(t *testing.T, _ request.CTX, ss store.Store) } func testRemoteClusterGetAll(t *testing.T, _ request.CTX, ss store.Store) { - require.NoError(t, clearRemoteClusters(ss)) + ss.DropAllTables() userId := model.NewId() now := model.GetMillis() @@ -232,9 +283,11 @@ func testRemoteClusterGetAll(t *testing.T, _ request.CTX, ss store.Store) { {Name: "brand_new_offline_remote", CreatorId: userId, SiteURL: "", LastPingAt: 0, Topics: " bogus shared stuff "}, {Name: "offline_plugin_remote", CreatorId: model.NewId(), SiteURL: makeSiteURL(), PluginID: model.NewId(), LastPingAt: 0, Topics: " pluginshare "}, {Name: "online_plugin_remote", CreatorId: model.NewId(), SiteURL: makeSiteURL(), PluginID: model.NewId(), LastPingAt: now, Topics: " pluginshare "}, + {Name: "deleted_remote", CreatorId: model.NewId(), SiteURL: "", LastPingAt: 0, DeleteAt: 123}, } idsAll := make([]string, 0) + idsNotDeleted := make([]string, 0) idsOnline := make([]string, 0) idsShareTopic := make([]string, 0) idsPlugin := make([]string, 0) @@ -246,24 +299,29 @@ func testRemoteClusterGetAll(t *testing.T, _ request.CTX, ss store.Store) { saved, err := ss.RemoteCluster().Save(item) require.NoError(t, err) idsAll = append(idsAll, saved.RemoteId) - if online { - idsOnline = append(idsOnline, saved.RemoteId) - } - if strings.Contains(saved.Topics, " shared ") { - idsShareTopic = append(idsShareTopic, saved.RemoteId) - } - if item.PluginID != "" { - idsPlugin = append(idsPlugin, saved.RemoteId) - } else { - idsNotPlugin = append(idsNotPlugin, saved.RemoteId) - } - if item.SiteURL != "" { - idsConfirmed = append(idsConfirmed, saved.RemoteId) + if item.DeleteAt == 0 { + idsNotDeleted = append(idsNotDeleted, saved.RemoteId) + + // only include non-deleted items in other counts + if online { + idsOnline = append(idsOnline, saved.RemoteId) + } + if strings.Contains(saved.Topics, " shared ") { + idsShareTopic = append(idsShareTopic, saved.RemoteId) + } + if item.PluginID != "" { + idsPlugin = append(idsPlugin, saved.RemoteId) + } else { + idsNotPlugin = append(idsNotPlugin, saved.RemoteId) + } + if item.SiteURL != "" { + idsConfirmed = append(idsConfirmed, saved.RemoteId) + } } } t.Run("GetAll", func(t *testing.T) { - filter := model.RemoteClusterQueryFilter{} + filter := model.RemoteClusterQueryFilter{IncludeDeleted: true} remotes, err := ss.RemoteCluster().GetAll(0, 999999, filter) require.NoError(t, err) // make sure all the test data remotes were returned. @@ -271,6 +329,15 @@ func testRemoteClusterGetAll(t *testing.T, _ request.CTX, ss store.Store) { assert.ElementsMatch(t, ids, idsAll) }) + t.Run("GetAllNotDeleted", func(t *testing.T) { + filter := model.RemoteClusterQueryFilter{} + remotes, err := ss.RemoteCluster().GetAll(0, 999999, filter) + require.NoError(t, err) + // make sure all the test data remotes were returned. + ids := getIds(remotes) + assert.ElementsMatch(t, ids, idsNotDeleted) + }) + t.Run("GetAll online only", func(t *testing.T) { filter := model.RemoteClusterQueryFilter{ ExcludeOffline: true, @@ -375,7 +442,7 @@ func testRemoteClusterGetAllInChannel(t *testing.T, rctx request.CTX, ss store.S testPluginID_2 = "com.sample.bloop" ) - require.NoError(t, clearRemoteClusters(ss)) + ss.DropAllTables() now := model.GetMillis() userId := model.NewId() @@ -485,7 +552,7 @@ func testRemoteClusterGetAllInChannel(t *testing.T, rctx request.CTX, ss store.S } func testRemoteClusterGetAllNotInChannel(t *testing.T, rctx request.CTX, ss store.Store) { - require.NoError(t, clearRemoteClusters(ss)) + ss.DropAllTables() userId := model.NewId() @@ -590,7 +657,7 @@ func getIds(remotes []*model.RemoteCluster) []string { } func testRemoteClusterGetByTopic(t *testing.T, _ request.CTX, ss store.Store) { - require.NoError(t, clearRemoteClusters(ss)) + ss.DropAllTables() rcData := []*model.RemoteCluster{ {Name: "AAAA_Inc", CreatorId: model.NewId(), SiteURL: "aaaa.com", RemoteId: model.NewId(), Topics: ""}, @@ -670,17 +737,3 @@ func testRemoteClusterUpdateTopics(t *testing.T, _ request.CTX, ss store.Store) require.Equal(t, tt.expected, rcUpdated.Topics) } } - -func clearRemoteClusters(ss store.Store) error { - list, err := ss.RemoteCluster().GetAll(0, 999999, model.RemoteClusterQueryFilter{}) - if err != nil { - return err - } - - for _, rc := range list { - if _, err := ss.RemoteCluster().Delete(rc.RemoteId); err != nil { - return err - } - } - return nil -} diff --git a/server/channels/store/storetest/shared_channel_store.go b/server/channels/store/storetest/shared_channel_store.go index c95aa62aa1..60dcc26e8a 100644 --- a/server/channels/store/storetest/shared_channel_store.go +++ b/server/channels/store/storetest/shared_channel_store.go @@ -390,9 +390,10 @@ func testDeleteSharedChannel(t *testing.T, rctx request.CTX, ss store.Store) { // add some remotes for i := 0; i < 10; i++ { remote := &model.SharedChannelRemote{ - ChannelId: channel.Id, - CreatorId: model.NewId(), - RemoteId: model.NewId(), + ChannelId: channel.Id, + CreatorId: model.NewId(), + RemoteId: model.NewId(), + IsInviteConfirmed: true, // to avoid adding the InclUnconfirmed filter } _, err := ss.SharedChannel().SaveRemote(remote) require.NoError(t, err, "couldn't add remote", err) @@ -407,10 +408,14 @@ func testDeleteSharedChannel(t *testing.T, rctx request.CTX, ss store.Store) { require.Error(t, err) require.Nil(t, sc) - // make sure the remotes were deleted. + // make sure the remotes were marked as deleted. remotes, err := ss.SharedChannel().GetRemotes(0, 999999, model.SharedChannelRemoteFilterOpts{ChannelId: channel.Id}) require.NoError(t, err) - require.Len(t, remotes, 0, "expected empty remotes list") + require.Len(t, remotes, 0) + + deletedRemotes, err := ss.SharedChannel().GetRemotes(0, 999999, model.SharedChannelRemoteFilterOpts{ChannelId: channel.Id, IncludeDeleted: true}) + require.NoError(t, err) + require.Len(t, deletedRemotes, 10) // ensure channel's Shared flag is unset channelMod, err := ss.Channel().Get(channel.Id, false) @@ -571,6 +576,17 @@ func testGetSharedChannelRemoteByIds(t *testing.T, rctx request.CTX, ss store.St require.Error(t, err) require.Nil(t, r) }) + + t.Run("Get deleted shared channel remote by ids", func(t *testing.T) { + deleted, err := ss.SharedChannel().DeleteRemote(remoteSaved.Id) + require.NoError(t, err) + require.True(t, deleted) + + r, err := ss.SharedChannel().GetRemoteByIds(remoteSaved.ChannelId, remoteSaved.RemoteId) + require.NoError(t, err) + require.Equal(t, remoteSaved.Id, r.Id) + require.NotZero(t, r.DeleteAt) + }) } func testGetSharedChannelRemotes(t *testing.T, rctx request.CTX, ss store.Store) { @@ -587,6 +603,7 @@ func testGetSharedChannelRemotes(t *testing.T, rctx request.CTX, ss store.Store) {ChannelId: channel.Id, CreatorId: creator, RemoteId: remoteId2, IsInviteConfirmed: true}, {CreatorId: creator, RemoteId: remoteId, IsInviteConfirmed: true}, {CreatorId: creator, RemoteId: remoteId, IsInviteConfirmed: true}, + {CreatorId: creator, RemoteId: remoteId, IsInviteConfirmed: true, DeleteAt: 123}, {CreatorId: creator, RemoteId: remoteId}, } @@ -641,6 +658,25 @@ func testGetSharedChannelRemotes(t *testing.T, rctx request.CTX, ss store.Store) } }) + t.Run("Get shared channel remotes by remote_id including deleted", func(t *testing.T) { + opts := model.SharedChannelRemoteFilterOpts{ + RemoteId: remoteId, + IncludeDeleted: true, + } + remotes, err := ss.SharedChannel().GetRemotes(0, 999999, opts) + require.NoError(t, err, "should not error", err) + require.Len(t, remotes, 3) // only confirmed invitations + deletedCount := 0 + for _, r := range remotes { + require.Equal(t, remoteId, r.RemoteId) + require.True(t, r.IsInviteConfirmed) + if r.DeleteAt != 0 { + deletedCount++ + } + } + require.Equal(t, 1, deletedCount) + }) + t.Run("Get shared channel remotes by invalid remote_id", func(t *testing.T) { opts := model.SharedChannelRemoteFilterOpts{ RemoteId: model.NewId(), @@ -751,6 +787,20 @@ func testHasRemote(t *testing.T, rctx request.CTX, ss store.Store) { require.NoError(t, err) assert.False(t, has) }) + + t.Run("deleted remote", func(t *testing.T) { + scr, err := ss.SharedChannel().GetRemoteByIds(channel.Id, remote1) + require.NoError(t, err) + require.NotEmpty(t, scr.Id) + + deleted, err := ss.SharedChannel().DeleteRemote(scr.Id) + require.NoError(t, err) + require.True(t, deleted) + + has, err := ss.SharedChannel().HasRemote(channel.Id, remote1) + require.NoError(t, err) + assert.False(t, has) + }) } func testGetRemoteForUser(t *testing.T, rctx request.CTX, ss store.Store) { @@ -888,8 +938,8 @@ func testDeleteSharedChannelRemote(t *testing.T, rctx request.CTX, ss store.Stor require.True(t, deleted, "expected true from delete remote") r, err := ss.SharedChannel().GetRemote(remoteSaved.Id) - require.Error(t, err) - require.Nil(t, r) + require.NoError(t, err) + require.NotZero(t, r.DeleteAt) }) t.Run("Delete non-existent shared channel remote", func(t *testing.T) { diff --git a/server/i18n/en.json b/server/i18n/en.json index 2ad0d00a5b..101b75238d 100644 --- a/server/i18n/en.json +++ b/server/i18n/en.json @@ -1339,7 +1339,7 @@ }, { "id": "api.command_remote.remote_table_header", - "translation": "| Secure connection | Display name | Connection ID | Site URL | Invite accepted | Online | Last ping |" + "translation": "| Secure connection | Display name | Connection ID | Site URL | Default Team | Invite accepted | Online | Last ping | Deleted |" }, { "id": "api.command_remote.remotes_not_found", @@ -3758,6 +3758,10 @@ "id": "api.upgrade_to_enterprise_status.signature.app_error", "translation": "Mattermost was unable to upgrade to Enterprise Edition. The digital signature of the downloaded binary file could not be verified." }, + { + "id": "api.upload.create.upload_channel_not_shared_with_remote.app_error", + "translation": "Failed to upload file. Upload channel is not shared with remote." + }, { "id": "api.upload.create.upload_too_large.app_error", "translation": "Unable to upload file. File is too large." diff --git a/server/platform/services/sharedchannel/attachment.go b/server/platform/services/sharedchannel/attachment.go index 7102e942f3..717e0a8455 100644 --- a/server/platform/services/sharedchannel/attachment.go +++ b/server/platform/services/sharedchannel/attachment.go @@ -169,9 +169,13 @@ func (scs *Service) onReceiveUploadCreate(msg model.RemoteClusterMsg, rc *model. } // make sure channel is shared for the remote sender - if _, err := scs.server.GetStore().SharedChannel().GetRemoteByIds(us.ChannelId, rc.RemoteId); err != nil { + hasRemote, err := scs.server.GetStore().SharedChannel().HasRemote(us.ChannelId, rc.RemoteId) + if err != nil { return fmt.Errorf("could not validate upload session for remote: %w", err) } + if !hasRemote { + return model.NewAppError("createUpload", "api.upload.create.upload_channel_not_shared_with_remote.app_error", nil, "", http.StatusBadRequest) + } // make sure file attachments are enabled if scs.server.Config().FileSettings.EnableFileAttachments == nil || !*scs.server.Config().FileSettings.EnableFileAttachments { diff --git a/server/platform/services/sharedchannel/channelinvite.go b/server/platform/services/sharedchannel/channelinvite.go index 53505aca95..2a45467e1b 100644 --- a/server/platform/services/sharedchannel/channelinvite.go +++ b/server/platform/services/sharedchannel/channelinvite.go @@ -6,12 +6,14 @@ package sharedchannel import ( "context" "encoding/json" + "errors" "fmt" "strings" "github.com/mattermost/mattermost/server/public/model" "github.com/mattermost/mattermost/server/public/shared/mlog" "github.com/mattermost/mattermost/server/public/shared/request" + "github.com/mattermost/mattermost/server/v8/channels/store" "github.com/mattermost/mattermost/server/v8/platform/services/remotecluster" ) @@ -78,19 +80,48 @@ func (scs *Service) SendChannelInvite(channel *model.Channel, userId string, rc return } - scr := &model.SharedChannelRemote{ - ChannelId: sc.ChannelId, - CreatorId: userId, - RemoteId: rc.RemoteId, - IsInviteAccepted: true, - IsInviteConfirmed: true, - LastPostCreateAt: model.GetMillis(), - LastPostUpdateAt: model.GetMillis(), - } - if _, err = scs.server.GetStore().SharedChannel().SaveRemote(scr); err != nil { - scs.sendEphemeralPost(channel.Id, userId, fmt.Sprintf("Error confirming channel invite for %s: %v", rc.DisplayName, err)) + existingScr, err := scs.server.GetStore().SharedChannel().GetRemoteByIds(sc.ChannelId, rc.RemoteId) + var errNotFound *store.ErrNotFound + if err != nil && !errors.As(err, &errNotFound) { + scs.sendEphemeralPost(channel.Id, userId, fmt.Sprintf("Error sending channel invite for %s: %s", rc.DisplayName, err)) return } + + curTime := model.GetMillis() + if existingScr != nil { + if existingScr.DeleteAt == 0 { + // the shared channel remote exists and is not + // deleted, nothing to do here + return + } + + // the shared channel remote was deleted in the past, so + // with the new invite we restore it + existingScr.DeleteAt = 0 + existingScr.UpdateAt = curTime + existingScr.LastPostCreateAt = curTime + existingScr.LastPostUpdateAt = curTime + if _, sErr := scs.server.GetStore().SharedChannel().UpdateRemote(existingScr); sErr != nil { + scs.sendEphemeralPost(channel.Id, userId, fmt.Sprintf("Error confirming channel invite for %s: %v", rc.DisplayName, sErr)) + return + } + } else { + // the shared channel remote doesn't exists, so we create it + scr := &model.SharedChannelRemote{ + ChannelId: sc.ChannelId, + CreatorId: userId, + RemoteId: rc.RemoteId, + IsInviteAccepted: true, + IsInviteConfirmed: true, + LastPostCreateAt: curTime, + LastPostUpdateAt: curTime, + } + if _, err = scs.server.GetStore().SharedChannel().SaveRemote(scr); err != nil { + scs.sendEphemeralPost(channel.Id, userId, fmt.Sprintf("Error confirming channel invite for %s: %v", rc.DisplayName, err)) + return + } + } + scs.NotifyChannelChanged(sc.ChannelId) scs.sendEphemeralPost(channel.Id, userId, fmt.Sprintf("`%s` has been added to channel.", rc.DisplayName)) } @@ -105,28 +136,7 @@ func (scs *Service) SendChannelInvite(channel *model.Channel, userId string, rc ctx, cancel := context.WithTimeout(context.Background(), remotecluster.SendTimeout) defer cancel() - return rcs.SendMsg(ctx, msg, rc, func(msg model.RemoteClusterMsg, rc *model.RemoteCluster, resp *remotecluster.Response, err error) { - if err != nil || !resp.IsSuccess() { - scs.sendEphemeralPost(channel.Id, userId, fmt.Sprintf("Error sending channel invite for %s: %s", rc.DisplayName, combineErrors(err, resp.Err))) - return - } - - scr := &model.SharedChannelRemote{ - ChannelId: sc.ChannelId, - CreatorId: userId, - RemoteId: rc.RemoteId, - IsInviteAccepted: true, - IsInviteConfirmed: true, - LastPostCreateAt: model.GetMillis(), - LastPostUpdateAt: model.GetMillis(), - } - if _, err = scs.server.GetStore().SharedChannel().SaveRemote(scr); err != nil { - scs.sendEphemeralPost(channel.Id, userId, fmt.Sprintf("Error confirming channel invite for %s: %v", rc.DisplayName, err)) - return - } - scs.NotifyChannelChanged(sc.ChannelId) - scs.sendEphemeralPost(channel.Id, userId, fmt.Sprintf("`%s` has been added to channel.", rc.DisplayName)) - }) + return rcs.SendMsg(ctx, msg, rc, onInvite) } func combineErrors(err error, serror string) string { @@ -162,43 +172,63 @@ func (scs *Service) onReceiveChannelInvite(msg model.RemoteClusterMsg, rc *model ) // check if channel already exists + existingScr, err := scs.server.GetStore().SharedChannel().GetRemoteByIds(invite.ChannelId, rc.RemoteId) + var errNotFound *store.ErrNotFound + if err != nil && !errors.As(err, &errNotFound) { + return fmt.Errorf("cannot get deleted shared channel remote (channel_id=%s): %w", invite.ChannelId, err) + } + + if existingScr != nil && existingScr.DeleteAt == 0 { + // the channel is already shared, nothing to do + return nil + } + var channel *model.Channel var created bool - _, err := scs.server.GetStore().Channel().Get(invite.ChannelId, true) - if err == nil { - // the channel already exists on this server; could be the remote is trying to re-share it (not allowed at this time). - // If the channel is already shared with the remote, it will remain so. - return fmt.Errorf("cannot create shared channel (channel_id=%s): %w", invite.ChannelId, model.ErrChannelAlreadyExists) - } + if existingScr == nil { + var err error + _, err = scs.server.GetStore().Channel().Get(invite.ChannelId, true) + if err == nil { + // the channel already exists on this server and was not + // previously shared, so we reject the invite + return fmt.Errorf("cannot create new shared channel (channel_id=%s): %w", invite.ChannelId, model.ErrChannelAlreadyExists) + } - // create new local channel to sync with the remote channel - if channel, created, err = scs.handleChannelCreation(invite, rc); err != nil { - return err - } + // create new local channel to sync with the remote channel + if channel, created, err = scs.handleChannelCreation(invite, rc); err != nil { + return err + } - // sanity check to ensure the channel returned has the expected id. Otherwise sync will not work as expected and will fail - // silently. - if invite.ChannelId != channel.Id { - // as of this writing, this scenario should only be possible if the invite included a DM channel invitation with a - // combination of two user ids (one remote, one local) that already have a DM on this server. Very unlikely unless - // the remote is compromised AND has knowledge of the local user id. - // Another possibility would be an actual user ID collision between two servers, where the likelihood is - // infinitesimally small - scs.server.Log().Log(mlog.LvlSharedChannelServiceError, "Channel invite failed - channel created/fetched with wrong id", - mlog.String("remote", rc.DisplayName), - mlog.String("channel_id", invite.ChannelId), - mlog.String("channel_type", invite.Type), - mlog.String("channel_name", invite.Name), - mlog.String("team_id", invite.TeamId), - mlog.Array("dm_partics", invite.DirectParticipantIDs), - ) - return fmt.Errorf("cannot create shared channel (DM channel_id=%s): %w", invite.ChannelId, model.ErrChannelAlreadyExists) - } + // sanity check to ensure the channel returned has the expected id. Otherwise sync will not work as expected and will fail + // silently. + if invite.ChannelId != channel.Id { + // as of this writing, this scenario should only be possible if the invite included a DM channel invitation with a + // combination of two user ids (one remote, one local) that already have a DM on this server. Very unlikely unless + // the remote is compromised AND has knowledge of the local user id. + // Another possibility would be an actual user ID collision between two servers, where the likelihood is + // infinitesimally small + scs.server.Log().Log(mlog.LvlSharedChannelServiceError, "Channel invite failed - channel created/fetched with wrong id", + mlog.String("remote", rc.DisplayName), + mlog.String("channel_id", invite.ChannelId), + mlog.String("channel_type", invite.Type), + mlog.String("channel_name", invite.Name), + mlog.String("team_id", invite.TeamId), + mlog.Array("dm_partics", invite.DirectParticipantIDs), + ) + return fmt.Errorf("cannot create shared channel (DM channel_id=%s): %w", invite.ChannelId, model.ErrChannelAlreadyExists) + } - // mark the newly created channel read-only if requested in the invite - if invite.ReadOnly { - if err := scs.makeChannelReadOnly(channel); err != nil { - return fmt.Errorf("cannot make channel readonly `%s`: %w", invite.ChannelId, err) + // mark the newly created channel read-only if requested in the invite + if invite.ReadOnly { + if err := scs.makeChannelReadOnly(channel); err != nil { + return fmt.Errorf("cannot make channel readonly `%s`: %w", invite.ChannelId, err) + } + } + } else { + var err error + channel, err = scs.server.GetStore().Channel().Get(invite.ChannelId, true) + if err != nil { + return fmt.Errorf("cannot get channel (channel_id=%s) to restore a shared channel remote: %w", invite.ChannelId, err) } } @@ -206,7 +236,7 @@ func (scs *Service) onReceiveChannelInvite(msg model.RemoteClusterMsg, rc *model ChannelId: channel.Id, TeamId: channel.TeamId, Home: false, - ReadOnly: invite.ReadOnly, + ReadOnly: existingScr == nil && invite.ReadOnly, // only set read only flag for new shares ShareName: channel.Name, ShareDisplayName: channel.DisplayName, SharePurpose: channel.Purpose, @@ -224,25 +254,36 @@ func (scs *Service) onReceiveChannelInvite(msg model.RemoteClusterMsg, rc *model return fmt.Errorf("cannot create shared channel (channel_id=%s): %w", invite.ChannelId, err) } - sharedChannelRemote := &model.SharedChannelRemote{ - Id: model.NewId(), - ChannelId: channel.Id, - CreatorId: channel.CreatorId, - IsInviteAccepted: true, - IsInviteConfirmed: true, - RemoteId: rc.RemoteId, - LastPostCreateAt: model.GetMillis(), - LastPostUpdateAt: model.GetMillis(), - } - - if _, err := scs.server.GetStore().SharedChannel().SaveRemote(sharedChannelRemote); err != nil { - // delete the newly created channel since we could not create a SharedChannelRemote record for it, - // and delete the newly created SharedChannel record as well. - if created { - scs.app.PermanentDeleteChannel(request.EmptyContext(scs.server.Log()), channel) + curTime := model.GetMillis() + if existingScr != nil { + existingScr.DeleteAt = 0 + existingScr.UpdateAt = curTime + existingScr.LastPostCreateAt = curTime + existingScr.LastPostUpdateAt = curTime + if _, err := scs.server.GetStore().SharedChannel().UpdateRemote(existingScr); err != nil { + return fmt.Errorf("cannot restore deleted shared channel remote (channel_id=%s): %w", invite.ChannelId, err) + } + } else { + scr := &model.SharedChannelRemote{ + Id: model.NewId(), + ChannelId: channel.Id, + CreatorId: channel.CreatorId, + IsInviteAccepted: true, + IsInviteConfirmed: true, + RemoteId: rc.RemoteId, + LastPostCreateAt: model.GetMillis(), + LastPostUpdateAt: model.GetMillis(), + } + + if _, err := scs.server.GetStore().SharedChannel().SaveRemote(scr); err != nil { + // delete the newly created channel since we could not create a SharedChannelRemote record for it, + // and delete the newly created SharedChannel record as well. + if created { + scs.app.PermanentDeleteChannel(request.EmptyContext(scs.server.Log()), channel) + } + scs.server.GetStore().SharedChannel().Delete(sharedChannel.ChannelId) + return fmt.Errorf("cannot create shared channel remote (channel_id=%s): %w", invite.ChannelId, err) } - scs.server.GetStore().SharedChannel().Delete(sharedChannel.ChannelId) - return fmt.Errorf("cannot create shared channel remote (channel_id=%s): %w", invite.ChannelId, err) } return nil } diff --git a/server/platform/services/sharedchannel/channelinvite_test.go b/server/platform/services/sharedchannel/channelinvite_test.go index 063e2d7b35..d9c271822d 100644 --- a/server/platform/services/sharedchannel/channelinvite_test.go +++ b/server/platform/services/sharedchannel/channelinvite_test.go @@ -62,7 +62,7 @@ func TestOnReceiveChannelInvite(t *testing.T) { } mockStore := &mocks.Store{} - remoteCluster := &model.RemoteCluster{Name: "test", DefaultTeamId: model.NewId()} + remoteCluster := &model.RemoteCluster{RemoteId: model.NewId(), Name: "test", DefaultTeamId: model.NewId()} invitation := channelInviteMsg{ ChannelId: model.NewId(), TeamId: model.NewId(), @@ -83,6 +83,7 @@ func TestOnReceiveChannelInvite(t *testing.T) { Type: invitation.Type, } + mockSharedChannelStore.On("GetRemoteByIds", invitation.ChannelId, remoteCluster.RemoteId).Return(nil, store.NewErrNotFound("SharedChannelRemote", "")) mockChannelStore.On("Get", invitation.ChannelId, true).Return(nil, &store.ErrNotFound{}) mockSharedChannelStore.On("Save", mock.Anything).Return(nil, nil) mockSharedChannelStore.On("SaveRemote", mock.Anything).Return(nil, nil) @@ -127,7 +128,7 @@ func TestOnReceiveChannelInvite(t *testing.T) { } mockStore := &mocks.Store{} - remoteCluster := &model.RemoteCluster{Name: "test2"} + remoteCluster := &model.RemoteCluster{RemoteId: model.NewId(), Name: "test2"} invitation := channelInviteMsg{ ChannelId: model.NewId(), TeamId: model.NewId(), @@ -148,11 +149,14 @@ func TestOnReceiveChannelInvite(t *testing.T) { team := &model.Team{ Id: model.NewId(), } + mockSharedChannelStore := mocks.SharedChannelStore{} + mockSharedChannelStore.On("GetRemoteByIds", invitation.ChannelId, remoteCluster.RemoteId).Return(nil, store.NewErrNotFound("SharedChannelRemote", "")) mockChannelStore.On("Get", invitation.ChannelId, true).Return(nil, &store.ErrNotFound{}) mockTeamStore.On("GetAllPage", 0, 1, mock.Anything).Return([]*model.Team{team}, nil) mockStore.On("Channel").Return(&mockChannelStore) mockStore.On("Team").Return(&mockTeamStore) + mockStore.On("SharedChannel").Return(&mockSharedChannelStore) mockServer = scs.server.(*MockServerIface) mockServer.On("GetStore").Return(mockStore) @@ -167,6 +171,96 @@ func TestOnReceiveChannelInvite(t *testing.T) { assert.Equal(t, fmt.Sprintf("cannot make channel readonly `%s`: foo: bar, boom", invitation.ChannelId), err.Error()) }) + t.Run("When invitation points to a deleted shared channel remote", func(t *testing.T) { + mockServer := &MockServerIface{} + logger := mlog.CreateConsoleTestLogger(t) + mockServer.On("Log").Return(logger) + mockApp := &MockAppIface{} + scs := &Service{ + server: mockServer, + app: mockApp, + } + + mockStore := &mocks.Store{} + remoteCluster := &model.RemoteCluster{RemoteId: model.NewId(), Name: "test", DefaultTeamId: model.NewId()} + invitation := channelInviteMsg{ + ChannelId: model.NewId(), + TeamId: model.NewId(), + Type: model.ChannelTypeOpen, + } + payload, err := json.Marshal(invitation) + require.NoError(t, err) + + msg := model.RemoteClusterMsg{ + Payload: payload, + } + mockChannelStore := mocks.ChannelStore{} + mockSharedChannelStore := mocks.SharedChannelStore{} + channel := &model.Channel{ + Id: invitation.ChannelId, + TeamId: invitation.TeamId, + Type: invitation.Type, + } + sharedChannelRemote := &model.SharedChannelRemote{ + ChannelId: invitation.ChannelId, + RemoteId: remoteCluster.RemoteId, + DeleteAt: 1234, + } + + mockSharedChannelStore.On("GetRemoteByIds", invitation.ChannelId, mock.Anything).Return(sharedChannelRemote, nil) + mockChannelStore.On("Get", invitation.ChannelId, true).Return(channel, nil) + mockSharedChannelStore.On("Save", mock.Anything).Return(nil, nil) + mockSharedChannelStore.On("UpdateRemote", mock.Anything).Return(nil, nil) + mockStore.On("Channel").Return(&mockChannelStore) + mockStore.On("SharedChannel").Return(&mockSharedChannelStore) + + mockServer.On("GetStore").Return(mockStore) + defer mockApp.AssertExpectations(t) + + err = scs.onReceiveChannelInvite(msg, remoteCluster, nil) + require.NoError(t, err) + }) + + t.Run("When invitation points to an existing shared channel remote", func(t *testing.T) { + mockServer := &MockServerIface{} + logger := mlog.CreateConsoleTestLogger(t) + mockServer.On("Log").Return(logger) + mockApp := &MockAppIface{} + scs := &Service{ + server: mockServer, + app: mockApp, + } + + mockStore := &mocks.Store{} + remoteCluster := &model.RemoteCluster{RemoteId: model.NewId(), Name: "test", DefaultTeamId: model.NewId()} + invitation := channelInviteMsg{ + ChannelId: model.NewId(), + TeamId: model.NewId(), + Type: model.ChannelTypeOpen, + } + payload, err := json.Marshal(invitation) + require.NoError(t, err) + + msg := model.RemoteClusterMsg{ + Payload: payload, + } + mockSharedChannelStore := mocks.SharedChannelStore{} + sharedChannelRemote := &model.SharedChannelRemote{ + ChannelId: invitation.ChannelId, + RemoteId: remoteCluster.RemoteId, + DeleteAt: 0, + } + + mockServer.On("GetStore").Return(mockStore) + mockSharedChannelStore.On("GetRemoteByIds", invitation.ChannelId, remoteCluster.RemoteId).Return(sharedChannelRemote, nil) + mockStore.On("SharedChannel").Return(&mockSharedChannelStore) + + defer mockApp.AssertExpectations(t) + + err = scs.onReceiveChannelInvite(msg, remoteCluster, nil) + require.NoError(t, err) + }) + t.Run("DM channels", func(t *testing.T) { var testRemoteID = model.NewId() testCases := []struct { @@ -196,7 +290,7 @@ func TestOnReceiveChannelInvite(t *testing.T) { } mockStore := &mocks.Store{} - remoteCluster := &model.RemoteCluster{Name: "test3", CreatorId: model.NewId(), RemoteId: testRemoteID} + remoteCluster := &model.RemoteCluster{RemoteId: testRemoteID, Name: "test3", CreatorId: model.NewId()} invitation := channelInviteMsg{ ChannelId: model.NewId(), TeamId: model.NewId(), @@ -225,6 +319,7 @@ func TestOnReceiveChannelInvite(t *testing.T) { mockChannelStore.On("Get", invitation.ChannelId, true).Return(nil, errors.New("boom")) mockChannelStore.On("GetByName", "", mockTypeString, true).Return(nil, &store.ErrNotFound{}) + mockSharedChannelStore.On("GetRemoteByIds", invitation.ChannelId, remoteCluster.RemoteId).Return(nil, store.NewErrNotFound("SharedChannelRemote", "")) mockSharedChannelStore.On("Save", mock.Anything).Return(nil, nil) mockSharedChannelStore.On("SaveRemote", mock.Anything).Return(nil, nil) mockStore.On("Channel").Return(&mockChannelStore) diff --git a/server/platform/services/sharedchannel/service_api.go b/server/platform/services/sharedchannel/service_api.go index aaf7410625..05b782044f 100644 --- a/server/platform/services/sharedchannel/service_api.go +++ b/server/platform/services/sharedchannel/service_api.go @@ -170,7 +170,7 @@ func (scs *Service) InviteRemoteToChannel(channelID, remoteID, userID string, sh func (scs *Service) UninviteRemoteFromChannel(channelID, remoteID string) error { scr, err := scs.server.GetStore().SharedChannel().GetRemoteByIds(channelID, remoteID) - if err != nil || scr.ChannelId != channelID { + if err != nil || scr.ChannelId != channelID || scr.DeleteAt != 0 { return model.NewAppError("UninviteRemoteFromChannel", "api.command_share.channel_remote_id_not_exists", map[string]any{"RemoteId": remoteID}, "", http.StatusInternalServerError) } diff --git a/server/platform/services/sharedchannel/sync_send.go b/server/platform/services/sharedchannel/sync_send.go index 82c4f4cfdb..220e01e216 100644 --- a/server/platform/services/sharedchannel/sync_send.go +++ b/server/platform/services/sharedchannel/sync_send.go @@ -343,6 +343,9 @@ func (scs *Service) processTask(task syncTask) error { if err != nil { return err } + if rc.DeleteAt != 0 { + return fmt.Errorf("Processing task for a deleted remote cluster '%s'", task.remoteID) + } if !rc.IsOnline() { return fmt.Errorf("Failed updating shared channel '%s' for offline remote cluster '%s'", task.channelID, rc.DisplayName) } diff --git a/server/platform/services/sharedchannel/sync_send_remote.go b/server/platform/services/sharedchannel/sync_send_remote.go index d888469d3a..5521e9c75d 100644 --- a/server/platform/services/sharedchannel/sync_send_remote.go +++ b/server/platform/services/sharedchannel/sync_send_remote.go @@ -119,6 +119,9 @@ func (scs *Service) syncForRemote(task syncTask, rc *model.RemoteCluster) error mlog.String("remote", rc.DisplayName), mlog.String("channel_id", task.channelID), ) + } else if err == nil && scr.DeleteAt != 0 { + // if SharedChannelRemote is deleted, regardless of the autoinvite flag, do nothing + return nil } else if err != nil { return err } diff --git a/server/public/model/client4.go b/server/public/model/client4.go index d168791a68..38f9c908f9 100644 --- a/server/public/model/client4.go +++ b/server/public/model/client4.go @@ -8781,6 +8781,9 @@ func (c *Client4) GetRemoteClusters(ctx context.Context, page, perPage int, filt if filter.ExcludePlugins { v.Set("exclude_plugins", "true") } + if filter.IncludeDeleted { + v.Set("include_deleted", "true") + } url := c.remoteClusterRoute() if len(v) > 0 { url += "?" + v.Encode() @@ -8894,7 +8897,7 @@ func (c *Client4) DeleteRemoteCluster(ctx context.Context, remoteClusterId strin return BuildResponse(r), nil } -func (c *Client4) GetSharedChannelRemotesByRemoteCluster(ctx context.Context, remoteId string, excludeHome, excludeRemote bool, page, perPage int) ([]*SharedChannelRemote, *Response, error) { +func (c *Client4) GetSharedChannelRemotesByRemoteCluster(ctx context.Context, remoteId string, excludeHome, excludeRemote, includeDeleted bool, page, perPage int) ([]*SharedChannelRemote, *Response, error) { v := url.Values{} if excludeHome { v.Set("exclude_home", "true") @@ -8902,6 +8905,9 @@ func (c *Client4) GetSharedChannelRemotesByRemoteCluster(ctx context.Context, re if excludeRemote { v.Set("exclude_remote", "true") } + if includeDeleted { + v.Set("include_deleted", "true") + } if page != 0 { v.Set("page", fmt.Sprintf("%d", page)) } diff --git a/server/public/model/remote_cluster.go b/server/public/model/remote_cluster.go index 25959dd025..83304cc8de 100644 --- a/server/public/model/remote_cluster.go +++ b/server/public/model/remote_cluster.go @@ -57,6 +57,7 @@ type RemoteCluster struct { SiteURL string `json:"site_url"` DefaultTeamId string `json:"default_team_id"` CreateAt int64 `json:"create_at"` + DeleteAt int64 `json:"delete_at"` LastPingAt int64 `json:"last_ping_at"` Token string `json:"token"` RemoteToken string `json:"remote_token"` @@ -75,6 +76,7 @@ func (rc *RemoteCluster) Auditable() map[string]interface{} { "site_url": rc.SiteURL, "default_team_id": rc.DefaultTeamId, "create_at": rc.CreateAt, + "delete_at": rc.DeleteAt, "last_ping_at": rc.LastPingAt, "creator_id": rc.CreatorId, "plugin_id": rc.PluginID, @@ -271,6 +273,7 @@ func (rc *RemoteCluster) ToRemoteClusterInfo() RemoteClusterInfo { Name: rc.Name, DisplayName: rc.DisplayName, CreateAt: rc.CreateAt, + DeleteAt: rc.DeleteAt, LastPingAt: rc.LastPingAt, } } @@ -284,6 +287,7 @@ type RemoteClusterInfo struct { Name string `json:"name"` DisplayName string `json:"display_name"` CreateAt int64 `json:"create_at"` + DeleteAt int64 `json:"delete_at"` LastPingAt int64 `json:"last_ping_at"` } @@ -457,4 +461,5 @@ type RemoteClusterQueryFilter struct { OnlyPlugins bool ExcludePlugins bool RequireOptions Bitmask + IncludeDeleted bool } diff --git a/server/public/model/shared_channel.go b/server/public/model/shared_channel.go index ac870fa4c0..6ddd975a5e 100644 --- a/server/public/model/shared_channel.go +++ b/server/public/model/shared_channel.go @@ -110,6 +110,7 @@ type SharedChannelRemote struct { CreatorId string `json:"creator_id"` CreateAt int64 `json:"create_at"` UpdateAt int64 `json:"update_at"` + DeleteAt int64 `json:"delete_at"` IsInviteAccepted bool `json:"is_invite_accepted"` IsInviteConfirmed bool `json:"is_invite_confirmed"` RemoteId string `json:"remote_id"` @@ -265,6 +266,7 @@ type SharedChannelRemoteFilterOpts struct { InclUnconfirmed bool ExcludeHome bool ExcludeRemote bool + IncludeDeleted bool } // SyncMsg represents a change in content (post add/edit/delete, reaction add/remove, users).