From e9fe9f50dd5839a7cf0926a2d97e88a19c9b831e Mon Sep 17 00:00:00 2001 From: Jesse Hallam Date: Tue, 2 Jan 2018 11:41:23 -0500 Subject: [PLT-8173] Add username and profile picture to webhook set up pages (#8002) --- app/webhook.go | 33 +++++- app/webhook_test.go | 283 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 314 insertions(+), 2 deletions(-) (limited to 'app') 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() -- cgit v1.2.3-1-g7c22