MM-63912 - enhance validation for channel banner colors (#30981)

* MM-63912 - enhance validation for channel banner colors

* fix tests

* fix unit tests

server/channels/api4/channel_test.go

@@ -172,7 +172,7 @@ func TestCreateChannel(t *testing.T) {
 			BannerInfo: &model.ChannelBannerInfo{
 				Enabled:         model.NewPointer(true),
 				Text:            model.NewPointer("banner text"),
-				BackgroundColor: model.NewPointer("color"),
+				BackgroundColor: model.NewPointer("#dddddd"),
 			},
 		}
 
@@ -182,7 +182,7 @@ func TestCreateChannel(t *testing.T) {
 
 		require.True(t, *createdChannel.BannerInfo.Enabled)
 		require.Equal(t, "banner text", *createdChannel.BannerInfo.Text)
-		require.Equal(t, "color", *createdChannel.BannerInfo.BackgroundColor)
+		require.Equal(t, "#dddddd", *createdChannel.BannerInfo.BackgroundColor)
 	})
 
 	t.Run("Cannot create channel with banner enabled but not configured", func(t *testing.T) {
@@ -767,7 +767,7 @@ func TestPatchChannel(t *testing.T) {
 			BannerInfo: &model.ChannelBannerInfo{
 				Enabled:         model.NewPointer(true),
 				Text:            model.NewPointer("banner text"),
-				BackgroundColor: model.NewPointer("color"),
+				BackgroundColor: model.NewPointer("#dddddd"),
 			},
 		}
 
@@ -799,7 +799,7 @@ func TestPatchChannel(t *testing.T) {
 			BannerInfo: &model.ChannelBannerInfo{
 				Enabled:         model.NewPointer(true),
 				Text:            model.NewPointer("banner text"),
-				BackgroundColor: model.NewPointer("color"),
+				BackgroundColor: model.NewPointer("#dddddd"),
 			},
 		}
 
@@ -831,7 +831,7 @@ func TestPatchChannel(t *testing.T) {
 			BannerInfo: &model.ChannelBannerInfo{
 				Enabled:         model.NewPointer(true),
 				Text:            model.NewPointer("banner text"),
-				BackgroundColor: model.NewPointer("color"),
+				BackgroundColor: model.NewPointer("#dddddd"),
 			},
 		}
 
@@ -841,7 +841,7 @@ func TestPatchChannel(t *testing.T) {
 		require.NotNil(t, patchedChannel.BannerInfo)
 		require.True(t, *patchedChannel.BannerInfo.Enabled)
 		require.Equal(t, "banner text", *patchedChannel.BannerInfo.Text)
-		require.Equal(t, "color", *patchedChannel.BannerInfo.BackgroundColor)
+		require.Equal(t, "#dddddd", *patchedChannel.BannerInfo.BackgroundColor)
 	})
 
 	t.Run("Should not be able to configure channel banner on a channel as a non-admin channel member", func(t *testing.T) {
@@ -856,7 +856,7 @@ func TestPatchChannel(t *testing.T) {
 			BannerInfo: &model.ChannelBannerInfo{
 				Enabled:         model.NewPointer(true),
 				Text:            model.NewPointer("banner text"),
-				BackgroundColor: model.NewPointer("color"),
+				BackgroundColor: model.NewPointer("#dddddd"),
 			},
 		}
 
@@ -877,7 +877,7 @@ func TestPatchChannel(t *testing.T) {
 			BannerInfo: &model.ChannelBannerInfo{
 				Enabled:         model.NewPointer(true),
 				Text:            model.NewPointer("banner text"),
-				BackgroundColor: model.NewPointer("color"),
+				BackgroundColor: model.NewPointer("#dddddd"),
 			},
 		}
 
@@ -887,7 +887,7 @@ func TestPatchChannel(t *testing.T) {
 		require.NotNil(t, patchedChannel.BannerInfo)
 		require.True(t, *patchedChannel.BannerInfo.Enabled)
 		require.Equal(t, "banner text", *patchedChannel.BannerInfo.Text)
-		require.Equal(t, "color", *patchedChannel.BannerInfo.BackgroundColor)
+		require.Equal(t, "#dddddd", *patchedChannel.BannerInfo.BackgroundColor)
 	})
 
 	t.Run("Cannot enable channel banner without configuring it", func(t *testing.T) {
@@ -923,7 +923,7 @@ func TestPatchChannel(t *testing.T) {
 			BannerInfo: &model.ChannelBannerInfo{
 				Enabled:         nil,
 				Text:            model.NewPointer("banner text"),
-				BackgroundColor: model.NewPointer("color"),
+				BackgroundColor: model.NewPointer("#dddddd"),
 			},
 		}
 
@@ -933,7 +933,7 @@ func TestPatchChannel(t *testing.T) {
 		require.NotNil(t, patchedChannel.BannerInfo)
 		require.Nil(t, patchedChannel.BannerInfo.Enabled)
 		require.Equal(t, "banner text", *patchedChannel.BannerInfo.Text)
-		require.Equal(t, "color", *patchedChannel.BannerInfo.BackgroundColor)
+		require.Equal(t, "#dddddd", *patchedChannel.BannerInfo.BackgroundColor)
 
 		patch = &model.ChannelPatch{
 			BannerInfo: &model.ChannelBannerInfo{
@@ -947,7 +947,7 @@ func TestPatchChannel(t *testing.T) {
 		require.NotNil(t, patchedChannel.BannerInfo)
 		require.True(t, *patchedChannel.BannerInfo.Enabled)
 		require.Equal(t, "banner text", *patchedChannel.BannerInfo.Text)
-		require.Equal(t, "color", *patchedChannel.BannerInfo.BackgroundColor)
+		require.Equal(t, "#dddddd", *patchedChannel.BannerInfo.BackgroundColor)
 	})
 
 	t.Run("Cannot configure channel banner on a DM channel", func(t *testing.T) {
@@ -966,7 +966,7 @@ func TestPatchChannel(t *testing.T) {
 			BannerInfo: &model.ChannelBannerInfo{
 				Enabled:         model.NewPointer(true),
 				Text:            model.NewPointer("banner text"),
-				BackgroundColor: model.NewPointer("color"),
+				BackgroundColor: model.NewPointer("#dddddd"),
 			},
 		}
 
@@ -994,7 +994,7 @@ func TestPatchChannel(t *testing.T) {
 			BannerInfo: &model.ChannelBannerInfo{
 				Enabled:         model.NewPointer(true),
 				Text:            model.NewPointer("banner text"),
-				BackgroundColor: model.NewPointer("color"),
+				BackgroundColor: model.NewPointer("#dddddd"),
 			},
 		}
 

server/i18n/en.json

@@ -8660,6 +8660,10 @@
     "id": "model.channel.is_valid.banner_info.background_color.empty.app_error",
     "translation": "Channel banner color cannot be empty when channel banner is enabled"
   },
+  {
+    "id": "model.channel.is_valid.banner_info.background_color.invalid.app_error",
+    "translation": "Channel banner color must be a valid hex color (e.g., #FF0000 or #F00)"
+  },
   {
     "id": "model.channel.is_valid.banner_info.channel_type.app_error",
     "translation": "Channel banner can only be configured on Public and Private channels"

server/public/model/channel.go

@@ -17,6 +17,11 @@ import (
 	"unicode/utf8"
 )
 
+var (
+	// Validates both 3-digit (#RGB) and 6-digit (#RRGGBB) hex colors
+	channelHexColorRegex = regexp.MustCompile(`^#([0-9a-fA-F]{3}|[0-9a-fA-F]{6})$`)
+)
+
 type ChannelType string
 
 const (
@@ -312,6 +317,10 @@ func (o *Channel) IsValid() *AppError {
 		if o.BannerInfo.BackgroundColor == nil || len(*o.BannerInfo.BackgroundColor) == 0 {
 			return NewAppError("Channel.IsValid", "model.channel.is_valid.banner_info.background_color.empty.app_error", nil, "", http.StatusBadRequest)
 		}
+
+		if !channelHexColorRegex.MatchString(*o.BannerInfo.BackgroundColor) {
+			return NewAppError("Channel.IsValid", "model.channel.is_valid.banner_info.background_color.invalid.app_error", nil, "", http.StatusBadRequest)
+		}
 	}
 
 	return nil

server/public/model/channel_test.go

@@ -87,6 +87,68 @@ func TestChannelIsValid(t *testing.T) {
 	require.NotNil(t, o.IsValid())
 }
 
+func TestChannelBannerBackgroundColorValidation(t *testing.T) {
+	o := Channel{
+		Id:       NewId(),
+		CreateAt: GetMillis(),
+		UpdateAt: GetMillis(),
+		Name:     "valid-name",
+		Type:     ChannelTypeOpen,
+		Header:   "valid-header",
+		Purpose:  "valid-purpose",
+		BannerInfo: &ChannelBannerInfo{
+			Enabled: NewPointer(true),
+			Text:    NewPointer("Banner Text"),
+		},
+	}
+
+	// Test with nil background color
+	o.BannerInfo.BackgroundColor = nil
+	require.NotNil(t, o.IsValid())
+	require.Equal(t, "model.channel.is_valid.banner_info.background_color.empty.app_error", o.IsValid().Id)
+
+	// Test with empty background color
+	o.BannerInfo.BackgroundColor = NewPointer("")
+	require.NotNil(t, o.IsValid())
+	require.Equal(t, "model.channel.is_valid.banner_info.background_color.empty.app_error", o.IsValid().Id)
+
+	// Test with invalid background color (no # prefix)
+	o.BannerInfo.BackgroundColor = NewPointer("FF0000")
+	require.NotNil(t, o.IsValid())
+	require.Equal(t, "model.channel.is_valid.banner_info.background_color.invalid.app_error", o.IsValid().Id)
+
+	// Test with invalid background color (invalid characters)
+	o.BannerInfo.BackgroundColor = NewPointer("#GGGGGG")
+	require.NotNil(t, o.IsValid())
+	require.Equal(t, "model.channel.is_valid.banner_info.background_color.invalid.app_error", o.IsValid().Id)
+
+	// Test with invalid background color (wrong length)
+	o.BannerInfo.BackgroundColor = NewPointer("#FF00")
+	require.NotNil(t, o.IsValid())
+	require.Equal(t, "model.channel.is_valid.banner_info.background_color.invalid.app_error", o.IsValid().Id)
+
+	// Test with invalid background color (wrong length)
+	o.BannerInfo.BackgroundColor = NewPointer("#FF00000")
+	require.NotNil(t, o.IsValid())
+	require.Equal(t, "model.channel.is_valid.banner_info.background_color.invalid.app_error", o.IsValid().Id)
+
+	// Test with valid 6-digit hex color
+	o.BannerInfo.BackgroundColor = NewPointer("#FF0000")
+	require.Nil(t, o.IsValid())
+
+	// Test with valid 6-digit hex color (lowercase)
+	o.BannerInfo.BackgroundColor = NewPointer("#ff0000")
+	require.Nil(t, o.IsValid())
+
+	// Test with valid 3-digit hex color
+	o.BannerInfo.BackgroundColor = NewPointer("#F00")
+	require.Nil(t, o.IsValid())
+
+	// Test with valid 3-digit hex color (lowercase)
+	o.BannerInfo.BackgroundColor = NewPointer("#f00")
+	require.Nil(t, o.IsValid())
+}
+
 func TestChannelPreSave(t *testing.T) {
 	o := Channel{Name: "test"}
 	o.PreSave()