From 35e459227bf0bf1b34fa1a4b4993b8e6f44451b7 Mon Sep 17 00:00:00 2001 From: Miguel de la Cruz Date: Tue, 15 Dec 2020 15:56:42 +0100 Subject: [PATCH] [MM-30016] Allows to create incoming and outgoing webhooks for a different user (#16425) * [MM-30016] Allows to create an incoming webhook for a different user * [MM-30021] Allows to create an outgoing webhook for a different user * Fix update test Co-authored-by: Mattermod --- api4/webhook.go | 36 +++++++++++++++-- api4/webhook_local.go | 91 +++++++++++++++++++++++++++++++++++++++++++ api4/webhook_test.go | 75 +++++++++++++++++++++++++++++++++-- 3 files changed, 195 insertions(+), 7 deletions(-) diff --git a/api4/webhook.go b/api4/webhook.go index 58595bfd29..b0b36a36f7 100644 --- a/api4/webhook.go +++ b/api4/webhook.go @@ -54,7 +54,23 @@ func createIncomingHook(c *Context, w http.ResponseWriter, r *http.Request) { return } - incomingHook, err := c.App.CreateIncomingWebhookForChannel(c.App.Session().UserId, channel, hook) + userId := c.App.Session().UserId + if hook.UserId != "" && hook.UserId != userId { + if !c.App.SessionHasPermissionToTeam(*c.App.Session(), channel.TeamId, model.PERMISSION_MANAGE_OTHERS_INCOMING_WEBHOOKS) { + c.LogAudit("fail - innapropriate permissions") + c.SetPermissionError(model.PERMISSION_MANAGE_OTHERS_INCOMING_WEBHOOKS) + return + } + + if _, err = c.App.GetUser(hook.UserId); err != nil { + c.Err = err + return + } + + userId = hook.UserId + } + + incomingHook, err := c.App.CreateIncomingWebhookForChannel(userId, channel, hook) if err != nil { c.Err = err return @@ -374,13 +390,27 @@ func createOutgoingHook(c *Context, w http.ResponseWriter, r *http.Request) { auditRec.AddMeta("hook_id", hook.Id) c.LogAudit("attempt") - hook.CreatorId = c.App.Session().UserId - if !c.App.SessionHasPermissionToTeam(*c.App.Session(), hook.TeamId, model.PERMISSION_MANAGE_OUTGOING_WEBHOOKS) { c.SetPermissionError(model.PERMISSION_MANAGE_OUTGOING_WEBHOOKS) return } + if hook.CreatorId == "" { + hook.CreatorId = c.App.Session().UserId + } else { + if !c.App.SessionHasPermissionToTeam(*c.App.Session(), hook.TeamId, model.PERMISSION_MANAGE_OTHERS_OUTGOING_WEBHOOKS) { + c.LogAudit("fail - innapropriate permissions") + c.SetPermissionError(model.PERMISSION_MANAGE_OTHERS_OUTGOING_WEBHOOKS) + return + } + + _, err := c.App.GetUser(hook.CreatorId) + if err != nil { + c.Err = err + return + } + } + rhook, err := c.App.CreateOutgoingWebhook(hook) if err != nil { c.LogAudit("fail") diff --git a/api4/webhook_local.go b/api4/webhook_local.go index 891f6be41d..a07d744346 100644 --- a/api4/webhook_local.go +++ b/api4/webhook_local.go @@ -3,14 +3,105 @@ package api4 +import ( + "net/http" + + "github.com/mattermost/mattermost-server/v5/audit" + "github.com/mattermost/mattermost-server/v5/model" +) + func (api *API) InitWebhookLocal() { + api.BaseRoutes.IncomingHooks.Handle("", api.ApiLocal(localCreateIncomingHook)).Methods("POST") api.BaseRoutes.IncomingHooks.Handle("", api.ApiLocal(getIncomingHooks)).Methods("GET") api.BaseRoutes.IncomingHook.Handle("", api.ApiLocal(getIncomingHook)).Methods("GET") api.BaseRoutes.IncomingHook.Handle("", api.ApiLocal(updateIncomingHook)).Methods("PUT") api.BaseRoutes.IncomingHook.Handle("", api.ApiLocal(deleteIncomingHook)).Methods("DELETE") + api.BaseRoutes.OutgoingHooks.Handle("", api.ApiLocal(localCreateOutgoingHook)).Methods("POST") api.BaseRoutes.OutgoingHooks.Handle("", api.ApiLocal(getOutgoingHooks)).Methods("GET") api.BaseRoutes.OutgoingHook.Handle("", api.ApiLocal(getOutgoingHook)).Methods("GET") api.BaseRoutes.OutgoingHook.Handle("", api.ApiLocal(updateOutgoingHook)).Methods("PUT") api.BaseRoutes.OutgoingHook.Handle("", api.ApiLocal(deleteOutgoingHook)).Methods("DELETE") } + +func localCreateIncomingHook(c *Context, w http.ResponseWriter, r *http.Request) { + hook := model.IncomingWebhookFromJson(r.Body) + if hook == nil { + c.SetInvalidParam("incoming_webhook") + return + } + + if hook.UserId == "" { + c.SetInvalidParam("user_id") + return + } + + channel, err := c.App.GetChannel(hook.ChannelId) + if err != nil { + c.Err = err + return + } + + if _, err = c.App.GetUser(hook.UserId); err != nil { + c.Err = err + return + } + + auditRec := c.MakeAuditRecord("localCreateIncomingHook", audit.Fail) + defer c.LogAuditRec(auditRec) + auditRec.AddMeta("channel", channel) + c.LogAudit("attempt") + + incomingHook, err := c.App.CreateIncomingWebhookForChannel(hook.UserId, channel, hook) + if err != nil { + c.Err = err + return + } + + auditRec.Success() + auditRec.AddMeta("hook", incomingHook) + c.LogAudit("success") + + w.WriteHeader(http.StatusCreated) + w.Write([]byte(incomingHook.ToJson())) +} + +func localCreateOutgoingHook(c *Context, w http.ResponseWriter, r *http.Request) { + hook := model.OutgoingWebhookFromJson(r.Body) + if hook == nil { + c.SetInvalidParam("outgoing_webhook") + return + } + + auditRec := c.MakeAuditRecord("createOutgoingHook", audit.Fail) + defer c.LogAuditRec(auditRec) + auditRec.AddMeta("hook_id", hook.Id) + c.LogAudit("attempt") + + if hook.CreatorId == "" { + c.SetInvalidParam("user_id") + return + } + + _, err := c.App.GetUser(hook.CreatorId) + if err != nil { + c.Err = err + return + } + + rhook, err := c.App.CreateOutgoingWebhook(hook) + if err != nil { + c.LogAudit("fail") + c.Err = err + return + } + + auditRec.Success() + auditRec.AddMeta("hook_display", rhook.DisplayName) + auditRec.AddMeta("channel_id", rhook.ChannelId) + auditRec.AddMeta("team_id", rhook.TeamId) + c.LogAudit("success") + + w.WriteHeader(http.StatusCreated) + w.Write([]byte(rhook.ToJson())) +} diff --git a/api4/webhook_test.go b/api4/webhook_test.go index 9a85401159..f06c256b30 100644 --- a/api4/webhook_test.go +++ b/api4/webhook_test.go @@ -17,9 +17,11 @@ func TestCreateIncomingWebhook(t *testing.T) { defer th.TearDown() Client := th.Client - th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.EnableIncomingWebhooks = true }) - th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.EnablePostUsernameOverride = true }) - th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.EnablePostIconOverride = true }) + th.App.UpdateConfig(func(cfg *model.Config) { + *cfg.ServiceSettings.EnableIncomingWebhooks = true + *cfg.ServiceSettings.EnablePostUsernameOverride = true + *cfg.ServiceSettings.EnablePostIconOverride = true + }) defaultRolePermissions := th.SaveDefaultRolePermissions() defer func() { @@ -61,6 +63,38 @@ func TestCreateIncomingWebhook(t *testing.T) { _, resp = Client.CreateIncomingWebhook(hook) CheckNoError(t, resp) + th.TestForSystemAdminAndLocal(t, func(t *testing.T, client *model.Client4) { + hook.UserId = th.BasicUser2.Id + defer func() { hook.UserId = "" }() + + newHook, response := client.CreateIncomingWebhook(hook) + CheckNoError(t, response) + require.Equal(t, th.BasicUser2.Id, newHook.UserId) + }, "Create an incoming webhook for a different user") + + th.TestForSystemAdminAndLocal(t, func(t *testing.T, client *model.Client4) { + hook.UserId = "invalid-user" + defer func() { hook.UserId = "" }() + + _, response := client.CreateIncomingWebhook(hook) + CheckNotFoundStatus(t, response) + }, "Create an incoming webhook for an invalid user") + + t.Run("Create an incoming webhook for a different user without permissions", func(t *testing.T) { + hook.UserId = th.BasicUser2.Id + defer func() { hook.UserId = "" }() + + _, response := Client.CreateIncomingWebhook(hook) + CheckForbiddenStatus(t, response) + }) + + t.Run("Create an incoming webhook in local mode without providing user", func(t *testing.T) { + hook.UserId = "" + + _, response := th.LocalClient.CreateIncomingWebhook(hook) + CheckBadRequestStatus(t, response) + }) + th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.EnableIncomingWebhooks = false }) _, resp = Client.CreateIncomingWebhook(hook) CheckNotImplementedStatus(t, resp) @@ -381,6 +415,38 @@ func TestCreateOutgoingWebhook(t *testing.T) { _, resp = Client.CreateOutgoingWebhook(hook) CheckNoError(t, resp) + th.TestForSystemAdminAndLocal(t, func(t *testing.T, client *model.Client4) { + hook.CreatorId = th.BasicUser2.Id + defer func() { hook.CreatorId = "" }() + + newHook, response := client.CreateOutgoingWebhook(hook) + CheckNoError(t, response) + require.Equal(t, th.BasicUser2.Id, newHook.CreatorId) + }, "Create an outgoing webhook for a different user") + + th.TestForSystemAdminAndLocal(t, func(t *testing.T, client *model.Client4) { + hook.CreatorId = "invalid-user" + defer func() { hook.CreatorId = "" }() + + _, response := client.CreateOutgoingWebhook(hook) + CheckNotFoundStatus(t, response) + }, "Create an incoming webhook for an invalid user") + + t.Run("Create an outgoing webhook for a different user without permissions", func(t *testing.T) { + hook.CreatorId = th.BasicUser2.Id + defer func() { hook.CreatorId = "" }() + + _, response := Client.CreateOutgoingWebhook(hook) + CheckForbiddenStatus(t, response) + }) + + t.Run("Create an outgoing webhook in local mode without providing user", func(t *testing.T) { + hook.CreatorId = "" + + _, response := th.LocalClient.CreateOutgoingWebhook(hook) + CheckBadRequestStatus(t, response) + }) + th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.EnableOutgoingWebhooks = false }) _, resp = Client.CreateOutgoingWebhook(hook) CheckNotImplementedStatus(t, resp) @@ -755,11 +821,12 @@ func TestUpdateIncomingHook(t *testing.T) { th.AddPermissionToRole(model.PERMISSION_MANAGE_INCOMING_WEBHOOKS.Id, model.TEAM_USER_ROLE_ID) t.Run("UpdateHookOfSameUser", func(t *testing.T) { - sameUserHook := &model.IncomingWebhook{ChannelId: th.BasicChannel.Id, UserId: th.BasicUser2.Id} + sameUserHook := &model.IncomingWebhook{ChannelId: th.BasicChannel.Id} sameUserHook, resp := th.Client.CreateIncomingWebhook(sameUserHook) CheckNoError(t, resp) + sameUserHook.UserId = th.BasicUser2.Id _, resp = th.Client.UpdateIncomingWebhook(sameUserHook) CheckNoError(t, resp) })