[PLT-8173] Add username and profile picture to webhook set up pages (#8002)

2 files changed, 314 insertions, 2 deletions

diff --git a/app/webhook.go b/app/webhook.go
index ba513def5..5ed71e992 100644
--- a/app/webhook.go
+++ b/app/webhook.go
@@ -277,6 +277,17 @@ func (a *App) CreateIncomingWebhookForChannel(creatorId string, channel *model.C
 	hook.UserId = creatorId
 	hook.TeamId = channel.TeamId
 
+	if !a.Config().ServiceSettings.EnablePostUsernameOverride {
+		hook.PostUsername = ""
+	}
+	if !a.Config().ServiceSettings.EnablePostIconOverride {
+		hook.PostIconURL = ""
+	}
+
+	if hook.PostUsername != "" && !model.IsValidUsername(hook.PostUsername) {
+		return nil, model.NewAppError("CreateIncomingWebhookForChannel", "api.incoming_webhook.invalid_post_username.app_error", nil, "", http.StatusBadRequest)
+	}
+
 	if result := <-a.Srv.Store.Webhook().SaveIncoming(hook); result.Err != nil {
 		return nil, result.Err
 	} else {
@@ -289,6 +300,17 @@ func (a *App) UpdateIncomingWebhook(oldHook, updatedHook *model.IncomingWebhook)
 		return nil, model.NewAppError("UpdateIncomingWebhook", "api.incoming_webhook.disabled.app_error", nil, "", http.StatusNotImplemented)
 	}
 
+	if !a.Config().ServiceSettings.EnablePostUsernameOverride {
+		updatedHook.PostUsername = oldHook.PostUsername
+	}
+	if !a.Config().ServiceSettings.EnablePostIconOverride {
+		updatedHook.PostIconURL = oldHook.PostIconURL
+	}
+
+	if updatedHook.PostUsername != "" && !model.IsValidUsername(updatedHook.PostUsername) {
+		return nil, model.NewAppError("UpdateIncomingWebhook", "api.incoming_webhook.invalid_post_username.app_error", nil, "", http.StatusBadRequest)
+	}
+
 	updatedHook.Id = oldHook.Id
 	updatedHook.UserId = oldHook.UserId
 	updatedHook.CreateAt = oldHook.CreateAt
@@ -608,8 +630,15 @@ func (a *App) HandleIncomingWebhook(hookId string, req *model.IncomingWebhookReq
 		return model.NewAppError("HandleIncomingWebhook", "web.incoming_webhook.permissions.app_error", nil, "", http.StatusForbidden)
 	}
 
-	overrideUsername := req.Username
-	overrideIconUrl := req.IconURL
+	overrideUsername := hook.PostUsername
+	if req.Username != "" {
+		overrideUsername = req.Username
+	}
+
+	overrideIconUrl := hook.PostIconURL
+	if req.IconURL != "" {
+		overrideIconUrl = req.IconURL
+	}
 
 	_, err := a.CreateWebhookPost(hook.UserId, channel, text, overrideUsername, overrideIconUrl, req.Props, webhookType, "")
 	return err
diff --git a/app/webhook_test.go b/app/webhook_test.go
index bc72d4b6e..048c8361d 100644
--- a/app/webhook_test.go
+++ b/app/webhook_test.go
@@ -13,6 +13,289 @@ import (
 	"github.com/mattermost/mattermost-server/model"
 )
 
+func TestCreateIncomingWebhookForChannel(t *testing.T) {
+	th := Setup().InitBasic()
+	defer th.TearDown()
+
+	enableIncomingHooks := th.App.Config().ServiceSettings.EnableIncomingWebhooks
+	defer th.App.UpdateConfig(func(cfg *model.Config) { cfg.ServiceSettings.EnableIncomingWebhooks = enableIncomingHooks })
+	enablePostUsernameOverride := th.App.Config().ServiceSettings.EnablePostUsernameOverride
+	defer th.App.UpdateConfig(func(cfg *model.Config) { cfg.ServiceSettings.EnablePostUsernameOverride = enablePostUsernameOverride })
+	enablePostIconOverride := th.App.Config().ServiceSettings.EnablePostIconOverride
+	defer th.App.UpdateConfig(func(cfg *model.Config) { cfg.ServiceSettings.EnablePostIconOverride = enablePostIconOverride })
+
+	type TestCase struct {
+		EnableIncomingHooks        bool
+		EnablePostUsernameOverride bool
+		EnablePostIconOverride     bool
+		IncomingWebhook            model.IncomingWebhook
+
+		ExpectedError           bool
+		ExpectedIncomingWebhook *model.IncomingWebhook
+	}
+
+	for name, tc := range map[string]TestCase{
+		"webhooks not enabled": {
+			EnableIncomingHooks:        false,
+			EnablePostUsernameOverride: false,
+			EnablePostIconOverride:     false,
+			IncomingWebhook: model.IncomingWebhook{
+				DisplayName: "title",
+				Description: "description",
+				ChannelId:   th.BasicChannel.Id,
+			},
+
+			ExpectedError:           true,
+			ExpectedIncomingWebhook: nil,
+		},
+		"valid: username and post icon url ignored, since override not enabled": {
+			EnableIncomingHooks:        true,
+			EnablePostUsernameOverride: false,
+			EnablePostIconOverride:     false,
+			IncomingWebhook: model.IncomingWebhook{
+				DisplayName:  "title",
+				Description:  "description",
+				ChannelId:    th.BasicChannel.Id,
+				PostUsername: ":invalid and ignored:",
+				PostIconURL:  "ignored",
+			},
+
+			ExpectedError: false,
+			ExpectedIncomingWebhook: &model.IncomingWebhook{
+				DisplayName: "title",
+				Description: "description",
+				ChannelId:   th.BasicChannel.Id,
+			},
+		},
+		"invalid username, override enabled": {
+			EnableIncomingHooks:        true,
+			EnablePostUsernameOverride: true,
+			EnablePostIconOverride:     false,
+			IncomingWebhook: model.IncomingWebhook{
+				DisplayName:  "title",
+				Description:  "description",
+				ChannelId:    th.BasicChannel.Id,
+				PostUsername: ":invalid:",
+			},
+
+			ExpectedError:           true,
+			ExpectedIncomingWebhook: nil,
+		},
+		"valid, no username or post icon url provided": {
+			EnableIncomingHooks:        true,
+			EnablePostUsernameOverride: true,
+			EnablePostIconOverride:     true,
+			IncomingWebhook: model.IncomingWebhook{
+				DisplayName: "title",
+				Description: "description",
+				ChannelId:   th.BasicChannel.Id,
+			},
+
+			ExpectedError: false,
+			ExpectedIncomingWebhook: &model.IncomingWebhook{
+				DisplayName: "title",
+				Description: "description",
+				ChannelId:   th.BasicChannel.Id,
+			},
+		},
+		"valid, with username and post icon": {
+			EnableIncomingHooks:        true,
+			EnablePostUsernameOverride: true,
+			EnablePostIconOverride:     true,
+			IncomingWebhook: model.IncomingWebhook{
+				DisplayName:  "title",
+				Description:  "description",
+				ChannelId:    th.BasicChannel.Id,
+				PostUsername: "valid",
+				PostIconURL:  "http://example.com/icon",
+			},
+
+			ExpectedError: false,
+			ExpectedIncomingWebhook: &model.IncomingWebhook{
+				DisplayName:  "title",
+				Description:  "description",
+				ChannelId:    th.BasicChannel.Id,
+				PostUsername: "valid",
+				PostIconURL:  "http://example.com/icon",
+			},
+		},
+	} {
+		t.Run(name, func(t *testing.T) {
+			assert := assert.New(t)
+
+			th.App.UpdateConfig(func(cfg *model.Config) { cfg.ServiceSettings.EnableIncomingWebhooks = tc.EnableIncomingHooks })
+			th.App.UpdateConfig(func(cfg *model.Config) {
+				cfg.ServiceSettings.EnablePostUsernameOverride = tc.EnablePostUsernameOverride
+			})
+			th.App.UpdateConfig(func(cfg *model.Config) { cfg.ServiceSettings.EnablePostIconOverride = tc.EnablePostIconOverride })
+
+			createdHook, err := th.App.CreateIncomingWebhookForChannel(th.BasicUser.Id, th.BasicChannel, &tc.IncomingWebhook)
+			if tc.ExpectedError && err == nil {
+				t.Fatal("should have failed")
+			} else if !tc.ExpectedError && err != nil {
+				t.Fatalf("should not have failed: %v", err.Error())
+			}
+			if createdHook != nil {
+				defer th.App.DeleteIncomingWebhook(createdHook.Id)
+			}
+			if tc.ExpectedIncomingWebhook == nil {
+				assert.Nil(createdHook, "expected nil webhook")
+			} else if assert.NotNil(createdHook, "expected non-nil webhook") {
+				assert.Equal(tc.ExpectedIncomingWebhook.DisplayName, createdHook.DisplayName)
+				assert.Equal(tc.ExpectedIncomingWebhook.Description, createdHook.Description)
+				assert.Equal(tc.ExpectedIncomingWebhook.ChannelId, createdHook.ChannelId)
+				assert.Equal(tc.ExpectedIncomingWebhook.PostUsername, createdHook.PostUsername)
+				assert.Equal(tc.ExpectedIncomingWebhook.PostIconURL, createdHook.PostIconURL)
+			}
+		})
+	}
+}
+
+func TestUpdateIncomingWebhook(t *testing.T) {
+	th := Setup().InitBasic()
+	defer th.TearDown()
+
+	enableIncomingHooks := th.App.Config().ServiceSettings.EnableIncomingWebhooks
+	defer th.App.UpdateConfig(func(cfg *model.Config) { cfg.ServiceSettings.EnableIncomingWebhooks = enableIncomingHooks })
+	enablePostUsernameOverride := th.App.Config().ServiceSettings.EnablePostUsernameOverride
+	defer th.App.UpdateConfig(func(cfg *model.Config) { cfg.ServiceSettings.EnablePostUsernameOverride = enablePostUsernameOverride })
+	enablePostIconOverride := th.App.Config().ServiceSettings.EnablePostIconOverride
+	defer th.App.UpdateConfig(func(cfg *model.Config) { cfg.ServiceSettings.EnablePostIconOverride = enablePostIconOverride })
+
+	type TestCase struct {
+		EnableIncomingHooks        bool
+		EnablePostUsernameOverride bool
+		EnablePostIconOverride     bool
+		IncomingWebhook            model.IncomingWebhook
+
+		ExpectedError           bool
+		ExpectedIncomingWebhook *model.IncomingWebhook
+	}
+
+	for name, tc := range map[string]TestCase{
+		"webhooks not enabled": {
+			EnableIncomingHooks:        false,
+			EnablePostUsernameOverride: false,
+			EnablePostIconOverride:     false,
+			IncomingWebhook: model.IncomingWebhook{
+				DisplayName: "title",
+				Description: "description",
+				ChannelId:   th.BasicChannel.Id,
+			},
+
+			ExpectedError:           true,
+			ExpectedIncomingWebhook: nil,
+		},
+		"valid: username and post icon url ignored, since override not enabled": {
+			EnableIncomingHooks:        true,
+			EnablePostUsernameOverride: false,
+			EnablePostIconOverride:     false,
+			IncomingWebhook: model.IncomingWebhook{
+				DisplayName:  "title",
+				Description:  "description",
+				ChannelId:    th.BasicChannel.Id,
+				PostUsername: ":invalid and ignored:",
+				PostIconURL:  "ignored",
+			},
+
+			ExpectedError: false,
+			ExpectedIncomingWebhook: &model.IncomingWebhook{
+				DisplayName: "title",
+				Description: "description",
+				ChannelId:   th.BasicChannel.Id,
+			},
+		},
+		"invalid username, override enabled": {
+			EnableIncomingHooks:        true,
+			EnablePostUsernameOverride: true,
+			EnablePostIconOverride:     false,
+			IncomingWebhook: model.IncomingWebhook{
+				DisplayName:  "title",
+				Description:  "description",
+				ChannelId:    th.BasicChannel.Id,
+				PostUsername: ":invalid:",
+			},
+
+			ExpectedError:           true,
+			ExpectedIncomingWebhook: nil,
+		},
+		"valid, no username or post icon url provided": {
+			EnableIncomingHooks:        true,
+			EnablePostUsernameOverride: true,
+			EnablePostIconOverride:     true,
+			IncomingWebhook: model.IncomingWebhook{
+				DisplayName: "title",
+				Description: "description",
+				ChannelId:   th.BasicChannel.Id,
+			},
+
+			ExpectedError: false,
+			ExpectedIncomingWebhook: &model.IncomingWebhook{
+				DisplayName: "title",
+				Description: "description",
+				ChannelId:   th.BasicChannel.Id,
+			},
+		},
+		"valid, with username and post icon": {
+			EnableIncomingHooks:        true,
+			EnablePostUsernameOverride: true,
+			EnablePostIconOverride:     true,
+			IncomingWebhook: model.IncomingWebhook{
+				DisplayName:  "title",
+				Description:  "description",
+				ChannelId:    th.BasicChannel.Id,
+				PostUsername: "valid",
+				PostIconURL:  "http://example.com/icon",
+			},
+
+			ExpectedError: false,
+			ExpectedIncomingWebhook: &model.IncomingWebhook{
+				DisplayName:  "title",
+				Description:  "description",
+				ChannelId:    th.BasicChannel.Id,
+				PostUsername: "valid",
+				PostIconURL:  "http://example.com/icon",
+			},
+		},
+	} {
+		t.Run(name, func(t *testing.T) {
+			assert := assert.New(t)
+
+			th.App.UpdateConfig(func(cfg *model.Config) { cfg.ServiceSettings.EnableIncomingWebhooks = true })
+
+			hook, err := th.App.CreateIncomingWebhookForChannel(th.BasicUser.Id, th.BasicChannel, &model.IncomingWebhook{
+				ChannelId: th.BasicChannel.Id,
+			})
+			if err != nil {
+				t.Fatal(err.Error())
+			}
+			defer th.App.DeleteIncomingWebhook(hook.Id)
+
+			th.App.UpdateConfig(func(cfg *model.Config) { cfg.ServiceSettings.EnableIncomingWebhooks = tc.EnableIncomingHooks })
+			th.App.UpdateConfig(func(cfg *model.Config) {
+				cfg.ServiceSettings.EnablePostUsernameOverride = tc.EnablePostUsernameOverride
+			})
+			th.App.UpdateConfig(func(cfg *model.Config) { cfg.ServiceSettings.EnablePostIconOverride = tc.EnablePostIconOverride })
+
+			updatedHook, err := th.App.UpdateIncomingWebhook(hook, &tc.IncomingWebhook)
+			if tc.ExpectedError && err == nil {
+				t.Fatal("should have failed")
+			} else if !tc.ExpectedError && err != nil {
+				t.Fatalf("should not have failed: %v", err.Error())
+			}
+			if tc.ExpectedIncomingWebhook == nil {
+				assert.Nil(updatedHook, "expected nil webhook")
+			} else if assert.NotNil(updatedHook, "expected non-nil webhook") {
+				assert.Equal(tc.ExpectedIncomingWebhook.DisplayName, updatedHook.DisplayName)
+				assert.Equal(tc.ExpectedIncomingWebhook.Description, updatedHook.Description)
+				assert.Equal(tc.ExpectedIncomingWebhook.ChannelId, updatedHook.ChannelId)
+				assert.Equal(tc.ExpectedIncomingWebhook.PostUsername, updatedHook.PostUsername)
+				assert.Equal(tc.ExpectedIncomingWebhook.PostIconURL, updatedHook.PostIconURL)
+			}
+		})
+	}
+}
+
 func TestCreateWebhookPost(t *testing.T) {
 	th := Setup().InitBasic()
 	defer th.TearDown()