From f8fc02791b8232baec5e32aec921d068bca141f7 Mon Sep 17 00:00:00 2001
From: Sundaran Kumar <daran9@gmail.com>
Date: Mon, 8 Apr 2024 11:25:03 +0200
Subject: [PATCH] [MM-47953] Outgoing webhook does not trigger when using
 multiple callback URLs (#22394)

Co-authored-by: Michael Kochell <6913320+mickmister@users.noreply.github.com>
---
 server/channels/app/webhook.go      | 23 ++++---
 server/channels/app/webhook_test.go | 95 +++++++++++++++++++++++++++++
 2 files changed, 110 insertions(+), 8 deletions(-)

diff --git a/server/channels/app/webhook.go b/server/channels/app/webhook.go
index a4d005dd52..4679063423 100644
--- a/server/channels/app/webhook.go
+++ b/server/channels/app/webhook.go
@@ -95,23 +95,30 @@ func (a *App) handleWebhookEvents(c request.CTX, post *model.Post, team *model.T
 }
 
 func (a *App) TriggerWebhook(c request.CTX, payload *model.OutgoingWebhookPayload, hook *model.OutgoingWebhook, post *model.Post, channel *model.Channel) {
-	var body io.Reader
-	var contentType string
+	var jsonBytes []byte
+	var err error
+
+	contentType := "application/x-www-form-urlencoded"
 	if hook.ContentType == "application/json" {
-		js, err := json.Marshal(payload)
+		contentType = "application/json"
+		jsonBytes, err = json.Marshal(payload)
 		if err != nil {
 			c.Logger().Warn("Failed to encode to JSON", mlog.Err(err))
+			return
 		}
-		body = bytes.NewReader(js)
-		contentType = "application/json"
-	} else {
-		body = strings.NewReader(payload.ToFormValues())
-		contentType = "application/x-www-form-urlencoded"
 	}
 
 	var wg sync.WaitGroup
+
 	for i := range hook.CallbackURLs {
+		var body io.Reader
+		if hook.ContentType == "application/json" {
+			body = bytes.NewReader(jsonBytes)
+		} else {
+			body = strings.NewReader(payload.ToFormValues())
+		}
 		wg.Add(1)
+
 		// Get the callback URL by index to properly capture it for the go func
 		url := hook.CallbackURLs[i]
 
diff --git a/server/channels/app/webhook_test.go b/server/channels/app/webhook_test.go
index fc596501a6..0b490f4d13 100644
--- a/server/channels/app/webhook_test.go
+++ b/server/channels/app/webhook_test.go
@@ -756,6 +756,101 @@ func TestTriggerOutGoingWebhookWithUsernameAndIconURL(t *testing.T) {
 	}
 }
 
+func TestTriggerOutGoingWebhookWithMultipleURLs(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, ","),
+		}
+	}
+
+	createOutgoingWebhook := func(channel *model.Channel, testCallBackURLs []string, th *TestHelper) (*model.OutgoingWebhook, *model.AppError) {
+		outgoingWebhook := model.OutgoingWebhook{
+			ChannelId:    channel.Id,
+			TeamId:       channel.TeamId,
+			CallbackURLs: testCallBackURLs,
+			Username:     "some-user-name",
+			IconURL:      "http://some-website.com/assets/some-icon.png",
+			DisplayName:  "some-display-name",
+			Description:  "some-description",
+			CreatorId:    th.BasicUser.Id,
+			TriggerWords: []string{"Abracadabra"},
+			ContentType:  "application/json",
+		}
+
+		return th.App.CreateOutgoingWebhook(&outgoingWebhook)
+	}
+
+	chanTs1 := make(chan string, 1)
+	ts1 := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		chanTs1 <- "webhook received!"
+	}))
+	defer ts1.Close()
+
+	chanTs2 := make(chan string, 1)
+	ts2 := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		chanTs2 <- "webhook received!"
+	}))
+	defer ts2.Close()
+
+	th := Setup(t).InitBasic()
+	defer th.TearDown()
+
+	th.App.UpdateConfig(func(cfg *model.Config) {
+		*cfg.ServiceSettings.AllowedUntrustedInternalConnections = "localhost,127.0.0.1"
+	})
+
+	for name, testCase := range map[string]struct {
+		CallBackURLs []string
+	}{
+		"One WebhookURL": {
+			CallBackURLs: []string{ts1.URL},
+		},
+		"Two WebhookURLs": {
+			CallBackURLs: []string{ts1.URL, ts2.URL},
+		},
+	} {
+		t.Run(name, func(t *testing.T) {
+			th.App.UpdateConfig(func(cfg *model.Config) {
+				*cfg.ServiceSettings.EnableOutgoingWebhooks = true
+			})
+			channel := th.CreateChannel(th.Context, th.BasicTeam)
+			hook, _ := createOutgoingWebhook(channel, testCase.CallBackURLs, th)
+			payload := getPayload(hook, th, channel)
+
+			th.App.TriggerWebhook(th.Context, payload, hook, th.BasicPost, channel)
+
+			select {
+			case webhookResponse := <-chanTs1:
+				require.Equal(t, "webhook received!", webhookResponse)
+
+			case <-time.After(5 * time.Second):
+				require.Fail(t, "Timeout, webhook URL 1 response not received")
+			}
+
+			if len(testCase.CallBackURLs) > 1 {
+				select {
+				case webhookResponse := <-chanTs2:
+					require.Equal(t, "webhook received!", webhookResponse)
+
+				case <-time.After(5 * time.Second):
+					require.Fail(t, "Timeout, webhook URL 2 response not received")
+				}
+			}
+		})
+	}
+}
+
 type InfiniteReader struct {
 	Prefix string
 }