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
2025-03-13 12:38:29 +05:30
--- 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
+}
--- 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)
+	})
+}
--- 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."