added the custom icon and username for the outgoing webhook and its response (#9141)

* 8272 added the username and icon as part of the model and persisted the same

* 8272 added the custome icon and name when set to the web hook response

* 8272 changed the infinte loop to timeout after 5 seconds

* 8272  fixed review comments

api4/webhook_test.go

@@ -256,7 +256,7 @@ func TestCreateOutgoingWebhook(t *testing.T) {
 	th.AddPermissionToRole(model.PERMISSION_MANAGE_WEBHOOKS.Id, model.TEAM_ADMIN_ROLE_ID)
 	th.RemovePermissionFromRole(model.PERMISSION_MANAGE_WEBHOOKS.Id, model.TEAM_USER_ROLE_ID)
 
-	hook := &model.OutgoingWebhook{ChannelId: th.BasicChannel.Id, TeamId: th.BasicChannel.TeamId, CallbackURLs: []string{"http://nowhere.com"}}
+	hook := &model.OutgoingWebhook{ChannelId: th.BasicChannel.Id, TeamId: th.BasicChannel.TeamId, CallbackURLs: []string{"http://nowhere.com"}, Username: "some-user-name", IconURL: "http://some-icon-url/"}
 
 	rhook, resp := th.SystemAdminClient.CreateOutgoingWebhook(hook)
 	CheckNoError(t, resp)

app/webhook.go

@@ -133,7 +133,13 @@ func (a *App) TriggerWebhook(payload *model.OutgoingWebhookPayload, hook *model.
 						if len(webhookResp.Attachments) > 0 {
 							webhookResp.Props["attachments"] = webhookResp.Attachments
 						}
+						if a.Config().ServiceSettings.EnablePostUsernameOverride && hook.Username != "" && webhookResp.Username == "" {
+							webhookResp.Username = hook.Username
+						}
 
+						if a.Config().ServiceSettings.EnablePostIconOverride && hook.IconURL != "" && webhookResp.IconURL == "" {
+							webhookResp.IconURL = hook.IconURL
+						}
 						if _, err := a.CreateWebhookPost(hook.CreatorId, channel, text, webhookResp.Username, webhookResp.IconURL, webhookResp.Props, webhookResp.Type, postRootId); err != nil {
 							mlog.Error(fmt.Sprintf("Failed to create response post, err=%v", err))
 						}

app/webhook_test.go

@@ -11,6 +11,9 @@ import (
 	"github.com/stretchr/testify/require"
 
 	"github.com/mattermost/mattermost-server/model"
+	"net/http"
+	"net/http/httptest"
+	"time"
 )
 
 func TestCreateIncomingWebhookForChannel(t *testing.T) {
@@ -470,3 +473,181 @@ func TestSplitWebhookPost(t *testing.T) {
 		})
 	}
 }
+
+func TestCreateOutGoingWebhookWithUsernameAndIconURL(t *testing.T) {
+	th := Setup().InitBasic()
+	defer th.TearDown()
+
+	outgoingWebhook := model.OutgoingWebhook{
+		ChannelId:    th.BasicChannel.Id,
+		TeamId:       th.BasicChannel.TeamId,
+		CallbackURLs: []string{"http://nowhere.com"},
+		Username:     "some-user-name",
+		IconURL:      "http://some-icon/",
+		DisplayName:  "some-display-name",
+		Description:  "some-description",
+		CreatorId:    th.BasicUser.Id,
+	}
+
+	th.App.UpdateConfig(func(cfg *model.Config) { cfg.ServiceSettings.EnableOutgoingWebhooks = true })
+
+	createdHook, err := th.App.CreateOutgoingWebhook(&outgoingWebhook)
+
+	if err != nil {
+		t.Fatalf("should not have failed: %v", err.Error())
+	}
+
+	assert.NotNil(t, createdHook, "should not be null")
+
+	assert.Equal(t, createdHook.ChannelId, outgoingWebhook.ChannelId)
+	assert.Equal(t, createdHook.TeamId, outgoingWebhook.TeamId)
+	assert.Equal(t, createdHook.CallbackURLs, outgoingWebhook.CallbackURLs)
+	assert.Equal(t, createdHook.Username, outgoingWebhook.Username)
+	assert.Equal(t, createdHook.IconURL, outgoingWebhook.IconURL)
+	assert.Equal(t, createdHook.DisplayName, outgoingWebhook.DisplayName)
+	assert.Equal(t, createdHook.Description, outgoingWebhook.Description)
+
+}
+
+func TestTriggerOutGoingWebhookWithUsernameAndIconURL(t *testing.T) {
+
+	getPayload := func(hook *model.OutgoingWebhook, th *TestHelper, channel *model.Channel) *model.OutgoingWebhookPayload {
+		return &model.OutgoingWebhookPayload{
+			Token:       hook.Token,
+			TeamId:      hook.TeamId,
+			TeamDomain:  th.BasicTeam.Name,
+			ChannelId:   channel.Id,
+			ChannelName: channel.Name,
+			Timestamp:   th.BasicPost.CreateAt,
+			UserId:      th.BasicPost.UserId,
+			UserName:    th.BasicUser.Username,
+			PostId:      th.BasicPost.Id,
+			Text:        th.BasicPost.Message,
+			TriggerWord: "Abracadabra",
+			FileIds:     strings.Join(th.BasicPost.FileIds, ","),
+		}
+	}
+
+	waitUntilWebhookResposeIsCreatedAsPost := func(channel *model.Channel, th *TestHelper, t *testing.T, createdPost chan *model.Post) {
+		go func() {
+			for i := 0; i < 5; i++ {
+				time.Sleep(time.Second)
+				posts, _ := th.App.GetPosts(channel.Id, 0, 5)
+				if len(posts.Posts) > 0 {
+					for _, post := range posts.Posts {
+						createdPost <- post
+						return
+					}
+				}
+			}
+		}()
+	}
+
+	type TestCaseOutgoing struct {
+		EnablePostUsernameOverride bool
+		EnablePostIconOverride     bool
+		ExpectedUsername           string
+		ExpectedIconUrl            string
+		WebhookResponse            *model.OutgoingWebhookResponse
+	}
+
+	createOutgoingWebhook := func(channel *model.Channel, testCallBackUrl string, th *TestHelper) (*model.OutgoingWebhook, *model.AppError) {
+		outgoingWebhook := model.OutgoingWebhook{
+			ChannelId:    channel.Id,
+			TeamId:       channel.TeamId,
+			CallbackURLs: []string{testCallBackUrl},
+			Username:     "some-user-name",
+			IconURL:      "http://some-icon/",
+			DisplayName:  "some-display-name",
+			Description:  "some-description",
+			CreatorId:    th.BasicUser.Id,
+			TriggerWords: []string{"Abracadabra"},
+			ContentType:  "application/json",
+		}
+
+		return th.App.CreateOutgoingWebhook(&outgoingWebhook)
+	}
+
+	getTestCases := func() map[string]TestCaseOutgoing {
+
+		webHookResponse := "sample response text from test server"
+		testCasesOutgoing := map[string]TestCaseOutgoing{
+
+			"Should override username and Icon": {
+				EnablePostUsernameOverride: true,
+				EnablePostIconOverride:     true,
+				ExpectedUsername:           "some-user-name",
+				ExpectedIconUrl:            "http://some-icon/",
+			},
+			"Should not override username and Icon": {
+				EnablePostUsernameOverride: false,
+				EnablePostIconOverride:     false,
+			},
+			"Should not override username and Icon if the webhook response already has it": {
+				EnablePostUsernameOverride: true,
+				EnablePostIconOverride:     true,
+				ExpectedUsername:           "webhookuser",
+				ExpectedIconUrl:            "http://webhok/icon",
+				WebhookResponse:            &model.OutgoingWebhookResponse{Text: &webHookResponse, Username: "webhookuser", IconURL: "http://webhok/icon"},
+			},
+		}
+		return testCasesOutgoing
+	}
+
+	th := Setup().InitBasic()
+	defer th.TearDown()
+
+	th.App.UpdateConfig(func(cfg *model.Config) {
+		*cfg.ServiceSettings.AllowedUntrustedInternalConnections = "localhost 127.0.0.1"
+	})
+	createdPost := make(chan *model.Post)
+
+	for name, testCase := range getTestCases() {
+		t.Run(name, func(t *testing.T) {
+
+			th.App.UpdateConfig(func(cfg *model.Config) {
+				cfg.ServiceSettings.EnableOutgoingWebhooks = true
+				cfg.ServiceSettings.EnablePostUsernameOverride = testCase.EnablePostUsernameOverride
+				cfg.ServiceSettings.EnablePostIconOverride = testCase.EnablePostIconOverride
+			})
+
+			ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+				if testCase.WebhookResponse != nil {
+					w.Write([]byte(testCase.WebhookResponse.ToJson()))
+				} else {
+					w.Write([]byte(`{"text": "sample response text from test server"}`))
+				}
+			}))
+			defer ts.Close()
+
+			channel := th.CreateChannel(th.BasicTeam)
+			hook, _ := createOutgoingWebhook(channel, ts.URL, th)
+			payload := getPayload(hook, th, channel)
+
+			th.App.TriggerWebhook(payload, hook, th.BasicPost, channel)
+
+			waitUntilWebhookResposeIsCreatedAsPost(channel, th, t, createdPost)
+
+			select {
+			case webhookPost := <-createdPost:
+				assert.Equal(t, webhookPost.Message, "sample response text from test server")
+				assert.Equal(t, webhookPost.Props["from_webhook"], "true")
+				if testCase.ExpectedIconUrl != "" {
+					assert.Equal(t, webhookPost.Props["override_icon_url"], testCase.ExpectedIconUrl)
+				} else {
+					assert.Nil(t, webhookPost.Props["override_icon_url"])
+				}
+
+				if testCase.ExpectedUsername != "" {
+					assert.Equal(t, webhookPost.Props["override_username"], testCase.ExpectedUsername)
+				} else {
+					assert.Nil(t, webhookPost.Props["override_username"])
+				}
+			case <-time.After(5 * time.Second):
+				t.Fatal("Timeout, webhook response not created as post")
+			}
+
+		})
+	}
+
+}

i18n/en.json

@@ -4454,6 +4454,13 @@
     "id": "model.outgoing_hook.is_valid.words.app_error",
     "translation": "Invalid trigger words"
   },
+  {
+    "id": "model.outgoing_hook.username.app_error",
+    "translation": "Invalid username"
+  },{
+    "id": "model.outgoing_hook.icon_url.app_error",
+    "translation": "Invalid icon"
+  },
   {
     "id": "model.plugin_command.error.app_error",
     "translation": "An error occurred while trying to execute this command."

model/outgoing_webhook.go

@@ -28,6 +28,8 @@ type OutgoingWebhook struct {
 	DisplayName  string      `json:"display_name"`
 	Description  string      `json:"description"`
 	ContentType  string      `json:"content_type"`
+	Username     string      `json:"username"`
+	IconURL      string      `json:"icon_url"`
 }
 
 type OutgoingWebhookPayload struct {
@@ -181,6 +183,14 @@ func (o *OutgoingWebhook) IsValid() *AppError {
 		return NewAppError("OutgoingWebhook.IsValid", "model.outgoing_hook.is_valid.content_type.app_error", nil, "", http.StatusBadRequest)
 	}
 
+	if len(o.Username) > 64 {
+		return NewAppError("OutgoingWebhook.IsValid", "model.outgoing_hook.username.app_error", nil, "", http.StatusBadRequest)
+	}
+
+	if len(o.IconURL) > 1024 {
+		return NewAppError("OutgoingWebhook.IsValid", "model.outgoing_hook.icon_url.app_error", nil, "", http.StatusBadRequest)
+	}
+
 	return nil
 }
 

model/outgoing_webhook_test.go

@@ -121,6 +121,26 @@ func TestOutgoingWebhookIsValid(t *testing.T) {
 	if err := o.IsValid(); err != nil {
 		t.Fatal(err)
 	}
+
+	o.Username = strings.Repeat("1", 65)
+	if err := o.IsValid(); err == nil {
+		t.Fatal("should be invalid")
+	}
+
+	o.Username = strings.Repeat("1", 64)
+	if err := o.IsValid(); err != nil {
+		t.Fatal("should be invalid")
+	}
+
+	o.IconURL = strings.Repeat("1", 1025)
+	if err := o.IsValid(); err == nil {
+		t.Fatal(err)
+	}
+
+	o.IconURL = strings.Repeat("1", 1024)
+	if err := o.IsValid(); err != nil {
+		t.Fatal(err)
+	}
 }
 
 func TestOutgoingWebhookPayloadToFormValues(t *testing.T) {

store/sqlstore/upgrade.go

@@ -476,7 +476,8 @@ func UpgradeDatabaseToVersion51(sqlStore SqlStore) {
 func UpgradeDatabaseToVersion52(sqlStore SqlStore) {
 	// TODO: Uncomment following condition when version 5.2.0 is released
 	// if shouldPerformUpgrade(sqlStore, VERSION_5_1_0, VERSION_5_2_0) {
-
+	sqlStore.CreateColumnIfNotExists("OutgoingWebhooks", "Username", "varchar(64)", "varchar(64)", "")
+	sqlStore.CreateColumnIfNotExists("OutgoingWebhooks", "IconURL", "varchar(1024)", "varchar(1024)", "")
 	// 	saveSchemaVersion(sqlStore, VERSION_5_2_0)
 	// }
 }

store/sqlstore/webhook_store.go

@@ -61,6 +61,8 @@ func NewSqlWebhookStore(sqlStore SqlStore, metrics einterfaces.MetricsInterface)
 		tableo.ColMap("Description").SetMaxSize(128)
 		tableo.ColMap("ContentType").SetMaxSize(128)
 		tableo.ColMap("TriggerWhen").SetMaxSize(1)
+		tableo.ColMap("Username").SetMaxSize(64)
+		tableo.ColMap("IconURL").SetMaxSize(1024)
 	}
 
 	return s

store/storetest/webhook_store.go

@@ -239,6 +239,8 @@ func testWebhookStoreSaveOutgoing(t *testing.T, ss store.Store) {
 	o1.CreatorId = model.NewId()
 	o1.TeamId = model.NewId()
 	o1.CallbackURLs = []string{"http://nowhere.com/"}
+	o1.Username = "test-user-name"
+	o1.IconURL = "http://nowhere.com/icon"
 
 	if err := (<-ss.Webhook().SaveOutgoing(&o1)).Err; err != nil {
 		t.Fatal("couldn't save item", err)
@@ -255,6 +257,8 @@ func testWebhookStoreGetOutgoing(t *testing.T, ss store.Store) {
 	o1.CreatorId = model.NewId()
 	o1.TeamId = model.NewId()
 	o1.CallbackURLs = []string{"http://nowhere.com/"}
+	o1.Username = "test-user-name"
+	o1.IconURL = "http://nowhere.com/icon"
 
 	o1 = (<-ss.Webhook().SaveOutgoing(o1)).Data.(*model.OutgoingWebhook)
 
@@ -461,10 +465,13 @@ func testWebhookStoreUpdateOutgoing(t *testing.T, ss store.Store) {
 	o1.CreatorId = model.NewId()
 	o1.TeamId = model.NewId()
 	o1.CallbackURLs = []string{"http://nowhere.com/"}
+	o1.Username = "test-user-name"
+	o1.IconURL = "http://nowhere.com/icon"
 
 	o1 = (<-ss.Webhook().SaveOutgoing(o1)).Data.(*model.OutgoingWebhook)
 
 	o1.Token = model.NewId()
+	o1.Username = "another-test-user-name"
 
 	if r2 := <-ss.Webhook().UpdateOutgoing(o1); r2.Err != nil {
 		t.Fatal(r2.Err)