From 84fa496c69760a5d4e60ca175a743c9232ca462b Mon Sep 17 00:00:00 2001 From: Harshil Sharma <18575143+harshilsharma63@users.noreply.github.com> Date: Thu, 13 Mar 2025 12:38:29 +0530 Subject: [PATCH] Added missed license check for channel banner in patch channel API (#30445) * Added missed license check for channel banner in patch channel API * Extractced permission check function --- server/channels/api4/channel.go | 21 +++- server/channels/api4/channel_test.go | 149 +++++++++++++++++++++++++++ server/i18n/en.json | 4 + 3 files changed, 171 insertions(+), 3 deletions(-) diff --git a/server/channels/api4/channel.go b/server/channels/api4/channel.go index dc74959b98..1ab0e8e6a2 100644 --- a/server/channels/api4/channel.go +++ b/server/channels/api4/channel.go @@ -377,9 +377,12 @@ func patchChannel(c *Context, w http.ResponseWriter, r *http.Request) { } } - if patch.BannerInfo != nil && (originalOldChannel.Type != model.ChannelTypeOpen && originalOldChannel.Type != model.ChannelTypePrivate) { - c.Err = model.NewAppError("patchChannel", "api.channel.update_channel.banner_info.channel_type.not_allowed", nil, "", http.StatusBadRequest) - return + if patch.BannerInfo != nil { + if channelBannerAppErr := canEditChannelBanner(c.App.License(), originalOldChannel); channelBannerAppErr != nil { + channelBannerAppErr.Where = "patchChannel" + c.Err = channelBannerAppErr + return + } } rchannel, appErr := c.App.PatchChannel(c.AppContext, oldChannel, patch, c.AppContext.Session().UserId) @@ -2439,3 +2442,15 @@ func convertGroupMessageToChannel(c *Context, w http.ResponseWriter, r *http.Req c.Logger.Warn("Error while writing response from convertGroupMessageToChannel", mlog.Err(err)) } } + +func canEditChannelBanner(license *model.License, originalChannel *model.Channel) *model.AppError { + if license == nil || !license.IsE20OrEnterprise() { + return model.NewAppError("", "license_error.feature_unavailable", nil, "feature is not available for the current license", http.StatusForbidden) + } + + if originalChannel.Type != model.ChannelTypeOpen && originalChannel.Type != model.ChannelTypePrivate { + return model.NewAppError("", "api.channel.update_channel.banner_info.channel_type.not_allowed", nil, "", http.StatusBadRequest) + } + + return nil +} diff --git a/server/channels/api4/channel_test.go b/server/channels/api4/channel_test.go index 876a5177f8..0f7834c357 100644 --- a/server/channels/api4/channel_test.go +++ b/server/channels/api4/channel_test.go @@ -584,9 +584,74 @@ func TestPatchChannel(t *testing.T) { CheckBadRequestStatus(t, resp) }) + t.Run("Should not be able to configure channel banner without a license", func(t *testing.T) { + client.Logout(context.Background()) + th.LoginBasic() + th.App.Srv().RemoveLicense() + + channel := &model.Channel{ + DisplayName: GenerateTestChannelName(), + Name: GenerateTestChannelName(), + Type: model.ChannelTypeOpen, + TeamId: team.Id, + } + var err error + channel, _, err = client.CreateChannel(context.Background(), channel) + require.NoError(t, err) + + patch := &model.ChannelPatch{ + BannerInfo: &model.ChannelBannerInfo{ + Enabled: model.NewPointer(true), + Text: model.NewPointer("banner text"), + BackgroundColor: model.NewPointer("color"), + }, + } + + patchedChannel, resp, err := client.PatchChannel(context.Background(), channel.Id, patch) + require.Error(t, err) + CheckForbiddenStatus(t, resp) + require.Nil(t, patchedChannel) + }) + + t.Run("Should not be able to configure channel banner with a professional license", func(t *testing.T) { + client.Logout(context.Background()) + th.LoginBasic() + th.App.Srv().SetLicense(model.NewTestLicenseSKU(model.LicenseShortSkuProfessional)) + defer func() { + th.App.Srv().RemoveLicense() + }() + + channel := &model.Channel{ + DisplayName: GenerateTestChannelName(), + Name: GenerateTestChannelName(), + Type: model.ChannelTypeOpen, + TeamId: team.Id, + } + var err error + channel, _, err = client.CreateChannel(context.Background(), channel) + require.NoError(t, err) + + patch := &model.ChannelPatch{ + BannerInfo: &model.ChannelBannerInfo{ + Enabled: model.NewPointer(true), + Text: model.NewPointer("banner text"), + BackgroundColor: model.NewPointer("color"), + }, + } + + patchedChannel, resp, err := client.PatchChannel(context.Background(), channel.Id, patch) + require.Error(t, err) + CheckForbiddenStatus(t, resp) + require.Nil(t, patchedChannel) + }) + t.Run("Should be able to configure channel banner on a channel", func(t *testing.T) { client.Logout(context.Background()) th.LoginBasic() + th.App.Srv().SetLicense(model.NewTestLicenseSKU(model.LicenseShortSkuEnterprise)) + defer func() { + th.App.Srv().RemoveLicense() + }() channel := &model.Channel{ DisplayName: GenerateTestChannelName(), @@ -618,6 +683,10 @@ func TestPatchChannel(t *testing.T) { t.Run("Cannot enable channel banner without configuring it", func(t *testing.T) { client.Logout(context.Background()) th.LoginBasic() + th.App.Srv().SetLicense(model.NewTestLicenseSKU(model.LicenseShortSkuEnterprise)) + defer func() { + th.App.Srv().RemoveLicense() + }() channel := &model.Channel{ DisplayName: GenerateTestChannelName(), @@ -674,6 +743,10 @@ func TestPatchChannel(t *testing.T) { t.Run("Cannot configure channel banner on a DM channel", func(t *testing.T) { client.Logout(context.Background()) th.LoginBasic() + th.App.Srv().SetLicense(model.NewTestLicenseSKU(model.LicenseShortSkuEnterprise)) + defer func() { + th.App.Srv().RemoveLicense() + }() dmChannel, resp, err := client.CreateDirectChannel(context.Background(), th.BasicUser.Id, th.BasicUser2.Id) require.NoError(t, err) @@ -697,6 +770,10 @@ func TestPatchChannel(t *testing.T) { t.Run("Cannot configure channel banner on a GM channel", func(t *testing.T) { client.Logout(context.Background()) th.LoginBasic() + th.App.Srv().SetLicense(model.NewTestLicenseSKU(model.LicenseShortSkuEnterprise)) + defer func() { + th.App.Srv().RemoveLicense() + }() user3 := th.CreateUser() gmChannel, resp, err := client.CreateGroupChannel(context.Background(), []string{th.BasicUser.Id, th.BasicUser2.Id, user3.Id}) @@ -5461,3 +5538,75 @@ func TestViewChannelWithoutCollapsedThreads(t *testing.T) { require.NoError(t, err) require.Zero(t, threads.TotalUnreadMentions) } + +func TestCanEditChannelBanner(t *testing.T) { + t.Run("when license is nil", func(t *testing.T) { + channel := &model.Channel{ + Type: model.ChannelTypeOpen, + } + + err := canEditChannelBanner(nil, channel) + + require.NotNil(t, err) + assert.Equal(t, "license_error.feature_unavailable", err.Id) + assert.Equal(t, http.StatusForbidden, err.StatusCode) + }) + + t.Run("when license is not E20 or Enterprise", func(t *testing.T) { + license := model.NewTestLicenseSKU(model.LicenseShortSkuProfessional) + channel := &model.Channel{ + Type: model.ChannelTypeOpen, + } + + err := canEditChannelBanner(license, channel) + + require.NotNil(t, err) + assert.Equal(t, "license_error.feature_unavailable", err.Id) + assert.Equal(t, http.StatusForbidden, err.StatusCode) + }) + + t.Run("when channel type is direct message", func(t *testing.T) { + license := model.NewTestLicenseSKU(model.LicenseShortSkuEnterprise) + channel := &model.Channel{ + Type: model.ChannelTypeDirect, + } + + err := canEditChannelBanner(license, channel) + + require.NotNil(t, err) + assert.Equal(t, "api.channel.update_channel.banner_info.channel_type.not_allowed", err.Id) + assert.Equal(t, http.StatusBadRequest, err.StatusCode) + }) + + t.Run("when channel type is group message", func(t *testing.T) { + license := model.NewTestLicenseSKU(model.LicenseShortSkuEnterprise) + channel := &model.Channel{ + Type: model.ChannelTypeGroup, + } + + err := canEditChannelBanner(license, channel) + require.NotNil(t, err) + assert.Equal(t, "api.channel.update_channel.banner_info.channel_type.not_allowed", err.Id) + assert.Equal(t, http.StatusBadRequest, err.StatusCode) + }) + + t.Run("when channel type is open and license is valid", func(t *testing.T) { + license := model.NewTestLicenseSKU(model.LicenseShortSkuEnterprise) + channel := &model.Channel{ + Type: model.ChannelTypeOpen, + } + + err := canEditChannelBanner(license, channel) + assert.Nil(t, err) + }) + + t.Run("when channel type is private and license is valid", func(t *testing.T) { + license := model.NewTestLicenseSKU(model.LicenseShortSkuEnterprise) + channel := &model.Channel{ + Type: model.ChannelTypePrivate, + } + + err := canEditChannelBanner(license, channel) + assert.Nil(t, err) + }) +} diff --git a/server/i18n/en.json b/server/i18n/en.json index b1f9fdb9d0..fbe85d6f4d 100644 --- a/server/i18n/en.json +++ b/server/i18n/en.json @@ -8376,6 +8376,10 @@ "id": "jobs.set_job_error.update.error", "translation": "Failed to set job status to error" }, + { + "id": "license_error.feature_unavailable", + "translation": "Feature is not available for the current license" + }, { "id": "manaultesting.manual_test.parse.app_error", "translation": "Unable to parse URL."