diff --git a/e2e-tests/playwright/support/server/default_config.ts b/e2e-tests/playwright/support/server/default_config.ts index 2bcfdbee8f..0ee535ae33 100644 --- a/e2e-tests/playwright/support/server/default_config.ts +++ b/e2e-tests/playwright/support/server/default_config.ts @@ -96,6 +96,7 @@ const defaultServerConfig: AdminConfig = { EnableIncomingWebhooks: true, EnableOutgoingWebhooks: true, EnableCommands: true, + OutgoingIntegrationRequestsTimeout: 30, EnablePostUsernameOverride: false, EnablePostIconOverride: false, GoogleDeveloperKey: '', diff --git a/server/channels/api4/integration_action_test.go b/server/channels/api4/integration_action_test.go index 343e8bb159..a7dd9c3408 100644 --- a/server/channels/api4/integration_action_test.go +++ b/server/channels/api4/integration_action_test.go @@ -10,6 +10,7 @@ import ( "net/http" "net/http/httptest" "testing" + "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -175,41 +176,62 @@ func TestOpenDialog(t *testing.T) { }, } - _, err := client.OpenInteractiveDialog(context.Background(), request) - require.NoError(t, err) + t.Run("Should pass with valid request", func(t *testing.T) { + _, err := client.OpenInteractiveDialog(context.Background(), request) + require.NoError(t, err) + }) - // Should fail on bad trigger ID - request.TriggerId = "junk" - resp, err := client.OpenInteractiveDialog(context.Background(), request) - require.Error(t, err) - CheckBadRequestStatus(t, resp) + t.Run("Should fail on bad trigger ID", func(t *testing.T) { + request.TriggerId = "junk" + resp, err := client.OpenInteractiveDialog(context.Background(), request) + require.Error(t, err) + CheckBadRequestStatus(t, resp) + }) - // URL is required - request.TriggerId = triggerId - request.URL = "" - resp, err = client.OpenInteractiveDialog(context.Background(), request) - require.Error(t, err) - CheckBadRequestStatus(t, resp) + t.Run("URL is required", func(t *testing.T) { + request.TriggerId = triggerId + request.URL = "" + resp, err := client.OpenInteractiveDialog(context.Background(), request) + require.Error(t, err) + CheckBadRequestStatus(t, resp) + }) - // Should pass with markdown formatted introduction text - request.URL = "http://localhost:8065" - request.Dialog.IntroductionText = "**Some** _introduction text" - _, err = client.OpenInteractiveDialog(context.Background(), request) - require.NoError(t, err) + t.Run("Should pass with markdown formatted introduction text", func(t *testing.T) { + request.URL = "http://localhost:8065" + request.Dialog.IntroductionText = "**Some** _introduction text" + _, err := client.OpenInteractiveDialog(context.Background(), request) + require.NoError(t, err) + }) - // Should pass with empty introduction text - request.Dialog.IntroductionText = "" - _, err = client.OpenInteractiveDialog(context.Background(), request) - require.NoError(t, err) + t.Run("Should pass with empty introduction text", func(t *testing.T) { + request.Dialog.IntroductionText = "" + _, err := client.OpenInteractiveDialog(context.Background(), request) + require.NoError(t, err) + }) - // Should pass with no elements - request.Dialog.Elements = nil - _, err = client.OpenInteractiveDialog(context.Background(), request) - require.NoError(t, err) + t.Run("Should pass with nil elements slice", func(t *testing.T) { + request.Dialog.Elements = nil + _, err := client.OpenInteractiveDialog(context.Background(), request) + require.NoError(t, err) + }) - request.Dialog.Elements = []model.DialogElement{} - _, err = client.OpenInteractiveDialog(context.Background(), request) - require.NoError(t, err) + t.Run("Should pass with empty elements slice", func(t *testing.T) { + request.Dialog.Elements = []model.DialogElement{} + _, err := client.OpenInteractiveDialog(context.Background(), request) + require.NoError(t, err) + }) + + t.Run("Should fail if trigger timeout is extended", func(t *testing.T) { + th.App.UpdateConfig(func(cfg *model.Config) { + cfg.ServiceSettings.OutgoingIntegrationRequestsTimeout = model.NewInt64(1) + }) + + time.Sleep(1 * time.Second) + + _, err := client.OpenInteractiveDialog(context.Background(), request) + require.Error(t, err) + assert.Contains(t, err.Error(), "Trigger ID for interactive dialog is expired.") + }) } func TestSubmitDialog(t *testing.T) { diff --git a/server/channels/app/app_iface.go b/server/channels/app/app_iface.go index 4fa762e298..cb945dcb4a 100644 --- a/server/channels/app/app_iface.go +++ b/server/channels/app/app_iface.go @@ -554,12 +554,11 @@ type AppIface interface { DisableUserAccessToken(c request.CTX, token *model.UserAccessToken) *model.AppError DoAppMigrations() DoCheckForAdminNotifications(trial bool) *model.AppError - DoCommandRequest(cmd *model.Command, p url.Values) (*model.Command, *model.CommandResponse, *model.AppError) + DoCommandRequest(rctx request.CTX, cmd *model.Command, p url.Values) (*model.Command, *model.CommandResponse, *model.AppError) DoEmojisPermissionsMigration() DoGuestRolesCreationMigration() DoLocalRequest(c request.CTX, rawURL string, body []byte) (*http.Response, *model.AppError) DoLogin(c request.CTX, w http.ResponseWriter, r *http.Request, user *model.User, deviceID string, isMobile, isOAuthUser, isSaml bool) (*model.Session, *model.AppError) - DoPostAction(c request.CTX, postID, actionId, userID, selectedOption string) (string, *model.AppError) DoPostActionWithCookie(c request.CTX, postID, actionId, userID, selectedOption string, cookie *model.PostActionCookie) (string, *model.AppError) DoSystemConsoleRolesCreationMigration() DoUploadFile(c request.CTX, now time.Time, rawTeamId string, rawChannelId string, rawUserId string, rawFilename string, data []byte) (*model.FileInfo, *model.AppError) diff --git a/server/channels/app/command.go b/server/channels/app/command.go index da327c8aa4..0342dd1c2d 100644 --- a/server/channels/app/command.go +++ b/server/channels/app/command.go @@ -12,6 +12,7 @@ import ( "regexp" "strings" "sync" + "time" "unicode" "github.com/mattermost/mattermost/server/public/model" @@ -477,17 +478,20 @@ func (a *App) tryExecuteCustomCommand(c request.CTX, args *model.CommandArgs, tr } p.Set("response_url", args.SiteURL+"/hooks/commands/"+hook.Id) - return a.DoCommandRequest(cmd, p) + return a.DoCommandRequest(c, cmd, p) } -func (a *App) DoCommandRequest(cmd *model.Command, p url.Values) (*model.Command, *model.CommandResponse, *model.AppError) { +func (a *App) DoCommandRequest(rctx request.CTX, cmd *model.Command, p url.Values) (*model.Command, *model.CommandResponse, *model.AppError) { + ctx, cancel := context.WithTimeout(context.Background(), time.Duration(*a.Config().ServiceSettings.OutgoingIntegrationRequestsTimeout)*time.Second) + defer cancel() + // Prepare the request var req *http.Request var err error if cmd.Method == model.CommandMethodGet { - req, err = http.NewRequest(http.MethodGet, cmd.URL, nil) + req, err = http.NewRequestWithContext(ctx, http.MethodGet, cmd.URL, nil) } else { - req, err = http.NewRequest(http.MethodPost, cmd.URL, strings.NewReader(p.Encode())) + req, err = http.NewRequestWithContext(ctx, http.MethodPost, cmd.URL, strings.NewReader(p.Encode())) } if err != nil { @@ -507,9 +511,11 @@ func (a *App) DoCommandRequest(cmd *model.Command, p url.Values) (*model.Command req.Header.Set("Content-Type", "application/x-www-form-urlencoded") } - // Send the request resp, err := a.Srv().outgoingWebhookClient.Do(req) if err != nil { + if errors.Is(err, context.DeadlineExceeded) { + rctx.Logger().Info("Outgoing Command request timed out. Consider increasing ServiceSettings.OutgoingIntegrationRequestsTimeout.") + } return cmd, nil, model.NewAppError("command", "api.command.execute_command.failed.app_error", map[string]any{"Trigger": cmd.Trigger}, "", http.StatusInternalServerError).Wrap(err) } diff --git a/server/channels/app/integration_action.go b/server/channels/app/integration_action.go index a2da5df738..42ad92a477 100644 --- a/server/channels/app/integration_action.go +++ b/server/channels/app/integration_action.go @@ -5,8 +5,8 @@ // // 1. An integration creates an interactive message button or menu. // 2. A user clicks on a button or selects an option from the menu. -// 3. The client sends a request to server to complete the post action, calling DoPostAction below. -// 4. DoPostAction will send an HTTP POST request to the integration containing contextual data, including +// 3. The client sends a request to server to complete the post action, calling DoPostActionWithCookie below. +// 4. DoPostActionWithCookie will send an HTTP POST request to the integration containing contextual data, including // an encoded and signed trigger ID. Slash commands also include trigger IDs in their payloads. // 5. The integration performs any actions it needs to and optionally makes a request back to the MM server // using the trigger ID to open an interactive dialog. @@ -29,6 +29,7 @@ import ( "path" "path/filepath" "strings" + "time" "github.com/gorilla/mux" @@ -40,10 +41,6 @@ import ( "github.com/mattermost/mattermost/server/v8/channels/utils" ) -func (a *App) DoPostAction(c request.CTX, postID, actionId, userID, selectedOption string) (string, *model.AppError) { - return a.DoPostActionWithCookie(c, postID, actionId, userID, selectedOption, nil) -} - func (a *App) DoPostActionWithCookie(c request.CTX, postID, actionId, userID, selectedOption string, cookie *model.PostActionCookie) (string, *model.AppError) { // PostAction may result in the original post being updated. For the // updated post, we need to unconditionally preserve the original @@ -320,8 +317,14 @@ func (a *App) DoActionRequest(c request.CTX, rawURL string, body []byte) (*http. return a.DoLocalRequest(c, rawURLPath, body) } - req, err := http.NewRequest("POST", rawURL, bytes.NewReader(body)) + ctx, cancel := context.WithTimeout(context.Background(), time.Duration(*a.Config().ServiceSettings.OutgoingIntegrationRequestsTimeout)*time.Second) + defer cancel() + + req, err := http.NewRequestWithContext(ctx, "POST", rawURL, bytes.NewReader(body)) if err != nil { + if errors.Is(err, context.DeadlineExceeded) { + c.Logger().Info("Outgoing Integration Action request timed out. Consider increasing ServiceSettings.OutgoingIntegrationRequestsTimeout.") + } return nil, model.NewAppError("DoActionRequest", "api.post.do_action.action_integration.app_error", nil, "", http.StatusBadRequest).Wrap(err) } req.Header.Set("Content-Type", "application/json") @@ -584,7 +587,8 @@ func (a *App) DoLocalRequest(c request.CTX, rawURL string, body []byte) (*http.R } func (a *App) OpenInteractiveDialog(request model.OpenDialogRequest) *model.AppError { - clientTriggerId, userID, appErr := request.DecodeAndVerifyTriggerId(a.AsymmetricSigningKey()) + timeout := time.Duration(*a.Config().ServiceSettings.OutgoingIntegrationRequestsTimeout) * time.Second + clientTriggerId, userID, appErr := request.DecodeAndVerifyTriggerId(a.AsymmetricSigningKey(), timeout) if appErr != nil { return appErr } diff --git a/server/channels/app/integration_action_test.go b/server/channels/app/integration_action_test.go index 08330f01b0..6d730599af 100644 --- a/server/channels/app/integration_action_test.go +++ b/server/channels/app/integration_action_test.go @@ -12,6 +12,7 @@ import ( "net/url" "strings" "testing" + "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -65,22 +66,21 @@ func TestPostActionInvalidURL(t *testing.T) { require.NotEmpty(t, attachments[0].Actions) require.NotEmpty(t, attachments[0].Actions[0].Id) - _, err = th.App.DoPostAction(th.Context, post.Id, attachments[0].Actions[0].Id, th.BasicUser.Id, "") + _, err = th.App.DoPostActionWithCookie(th.Context, post.Id, attachments[0].Actions[0].Id, th.BasicUser.Id, "", nil) require.NotNil(t, err) require.True(t, strings.Contains(err.Error(), "missing protocol scheme")) } func TestPostActionEmptyResponse(t *testing.T) { + th := Setup(t).InitBasic() + defer th.TearDown() + + channel := th.BasicChannel + th.App.UpdateConfig(func(cfg *model.Config) { + *cfg.ServiceSettings.AllowedUntrustedInternalConnections = "localhost,127.0.0.1" + }) + t.Run("Empty response on post action", func(t *testing.T) { - th := Setup(t).InitBasic() - defer th.TearDown() - - channel := th.BasicChannel - - th.App.UpdateConfig(func(cfg *model.Config) { - *cfg.ServiceSettings.AllowedUntrustedInternalConnections = "localhost,127.0.0.1" - }) - ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {})) defer ts.Close() @@ -118,9 +118,58 @@ func TestPostActionEmptyResponse(t *testing.T) { attachments, ok := post.GetProp("attachments").([]*model.SlackAttachment) require.True(t, ok) - _, err = th.App.DoPostAction(th.Context, post.Id, attachments[0].Actions[0].Id, th.BasicUser.Id, "") + _, err = th.App.DoPostActionWithCookie(th.Context, post.Id, attachments[0].Actions[0].Id, th.BasicUser.Id, "", nil) require.Nil(t, err) }) + + t.Run("Empty response on post action, timeout", func(t *testing.T) { + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + time.Sleep(2 * time.Second) + })) + defer ts.Close() + + interactivePost := model.Post{ + Message: "Interactive post", + ChannelId: channel.Id, + PendingPostId: model.NewId() + ":" + fmt.Sprint(model.GetMillis()), + UserId: th.BasicUser.Id, + Props: model.StringInterface{ + "attachments": []*model.SlackAttachment{ + { + Text: "hello", + Actions: []*model.PostAction{ + { + Integration: &model.PostActionIntegration{ + Context: model.StringInterface{ + "s": "foo", + "n": 3, + }, + URL: ts.URL, + }, + Name: "action", + Type: "some_type", + DataSource: "some_source", + }, + }, + }, + }, + }, + } + + post, err := th.App.CreatePostAsUser(th.Context, &interactivePost, "", true) + require.Nil(t, err) + + attachments, ok := post.GetProp("attachments").([]*model.SlackAttachment) + require.True(t, ok) + + th.App.UpdateConfig(func(cfg *model.Config) { + cfg.ServiceSettings.OutgoingIntegrationRequestsTimeout = model.NewInt64(1) + }) + + _, err = th.App.DoPostActionWithCookie(th.Context, post.Id, attachments[0].Actions[0].Id, th.BasicUser.Id, "", nil) + require.Error(t, err) + assert.Contains(t, err.DetailedError, "context deadline exceeded") + }) } func TestPostAction(t *testing.T) { @@ -258,16 +307,16 @@ func TestPostAction(t *testing.T) { require.NotEmpty(t, attachments2[0].Actions) require.NotEmpty(t, attachments2[0].Actions[0].Id) - clientTriggerId, err := th.App.DoPostAction(th.Context, post.Id, "notavalidid", th.BasicUser.Id, "") + clientTriggerId, err := th.App.DoPostActionWithCookie(th.Context, post.Id, "notavalidid", th.BasicUser.Id, "", nil) require.NotNil(t, err) assert.Equal(t, http.StatusNotFound, err.StatusCode) assert.True(t, clientTriggerId == "") - clientTriggerId, err = th.App.DoPostAction(th.Context, post.Id, attachments[0].Actions[0].Id, th.BasicUser.Id, "") + clientTriggerId, err = th.App.DoPostActionWithCookie(th.Context, post.Id, attachments[0].Actions[0].Id, th.BasicUser.Id, "", nil) require.Nil(t, err) assert.True(t, len(clientTriggerId) == 26) - clientTriggerId, err = th.App.DoPostAction(th.Context, post2.Id, attachments2[0].Actions[0].Id, th.BasicUser.Id, "selected") + clientTriggerId, err = th.App.DoPostActionWithCookie(th.Context, post2.Id, attachments2[0].Actions[0].Id, th.BasicUser.Id, "selected", nil) require.Nil(t, err) assert.True(t, len(clientTriggerId) == 26) @@ -275,7 +324,7 @@ func TestPostAction(t *testing.T) { *cfg.ServiceSettings.AllowedUntrustedInternalConnections = "" }) - _, err = th.App.DoPostAction(th.Context, post.Id, attachments[0].Actions[0].Id, th.BasicUser.Id, "") + _, err = th.App.DoPostActionWithCookie(th.Context, post.Id, attachments[0].Actions[0].Id, th.BasicUser.Id, "", nil) require.NotNil(t, err) require.True(t, strings.Contains(err.Error(), "address forbidden")) @@ -313,14 +362,14 @@ func TestPostAction(t *testing.T) { attachmentsPlugin, ok := postplugin.GetProp("attachments").([]*model.SlackAttachment) require.True(t, ok) - _, err = th.App.DoPostAction(th.Context, postplugin.Id, attachmentsPlugin[0].Actions[0].Id, th.BasicUser.Id, "") + _, err = th.App.DoPostActionWithCookie(th.Context, postplugin.Id, attachmentsPlugin[0].Actions[0].Id, th.BasicUser.Id, "", nil) require.Equal(t, "api.post.do_action.action_integration.app_error", err.Id) th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.AllowedUntrustedInternalConnections = "localhost,127.0.0.1" }) - _, err = th.App.DoPostAction(th.Context, postplugin.Id, attachmentsPlugin[0].Actions[0].Id, th.BasicUser.Id, "") + _, err = th.App.DoPostActionWithCookie(th.Context, postplugin.Id, attachmentsPlugin[0].Actions[0].Id, th.BasicUser.Id, "", nil) require.Nil(t, err) th.App.UpdateConfig(func(cfg *model.Config) { @@ -361,7 +410,7 @@ func TestPostAction(t *testing.T) { attachmentsSiteURL, ok := postSiteURL.GetProp("attachments").([]*model.SlackAttachment) require.True(t, ok) - _, err = th.App.DoPostAction(th.Context, postSiteURL.Id, attachmentsSiteURL[0].Actions[0].Id, th.BasicUser.Id, "") + _, err = th.App.DoPostActionWithCookie(th.Context, postSiteURL.Id, attachmentsSiteURL[0].Actions[0].Id, th.BasicUser.Id, "", nil) require.NotNil(t, err) require.False(t, strings.Contains(err.Error(), "address forbidden")) @@ -403,7 +452,7 @@ func TestPostAction(t *testing.T) { attachmentsSubpath, ok := postSubpath.GetProp("attachments").([]*model.SlackAttachment) require.True(t, ok) - _, err = th.App.DoPostAction(th.Context, postSubpath.Id, attachmentsSubpath[0].Actions[0].Id, th.BasicUser.Id, "") + _, err = th.App.DoPostActionWithCookie(th.Context, postSubpath.Id, attachmentsSubpath[0].Actions[0].Id, th.BasicUser.Id, "", nil) require.Nil(t, err) }) } @@ -477,7 +526,7 @@ func TestPostActionProps(t *testing.T) { attachments, ok := post.GetProp("attachments").([]*model.SlackAttachment) require.True(t, ok) - clientTriggerId, err := th.App.DoPostAction(th.Context, post.Id, attachments[0].Actions[0].Id, th.BasicUser.Id, "") + clientTriggerId, err := th.App.DoPostActionWithCookie(th.Context, post.Id, attachments[0].Actions[0].Id, th.BasicUser.Id, "", nil) require.Nil(t, err) assert.True(t, len(clientTriggerId) == 26) @@ -661,7 +710,7 @@ func TestPostActionRelativeURL(t *testing.T) { require.NotEmpty(t, attachments[0].Actions) require.NotEmpty(t, attachments[0].Actions[0].Id) - _, err = th.App.DoPostAction(th.Context, post.Id, attachments[0].Actions[0].Id, th.BasicUser.Id, "") + _, err = th.App.DoPostActionWithCookie(th.Context, post.Id, attachments[0].Actions[0].Id, th.BasicUser.Id, "", nil) require.NotNil(t, err) }) @@ -701,7 +750,7 @@ func TestPostActionRelativeURL(t *testing.T) { require.NotEmpty(t, attachments[0].Actions) require.NotEmpty(t, attachments[0].Actions[0].Id) - _, err = th.App.DoPostAction(th.Context, post.Id, attachments[0].Actions[0].Id, th.BasicUser.Id, "") + _, err = th.App.DoPostActionWithCookie(th.Context, post.Id, attachments[0].Actions[0].Id, th.BasicUser.Id, "", nil) require.NotNil(t, err) }) @@ -741,7 +790,7 @@ func TestPostActionRelativeURL(t *testing.T) { require.NotEmpty(t, attachments[0].Actions) require.NotEmpty(t, attachments[0].Actions[0].Id) - _, err = th.App.DoPostAction(th.Context, post.Id, attachments[0].Actions[0].Id, th.BasicUser.Id, "") + _, err = th.App.DoPostActionWithCookie(th.Context, post.Id, attachments[0].Actions[0].Id, th.BasicUser.Id, "", nil) require.NotNil(t, err) }) @@ -781,7 +830,7 @@ func TestPostActionRelativeURL(t *testing.T) { require.NotEmpty(t, attachments[0].Actions) require.NotEmpty(t, attachments[0].Actions[0].Id) - _, err = th.App.DoPostAction(th.Context, post.Id, attachments[0].Actions[0].Id, th.BasicUser.Id, "") + _, err = th.App.DoPostActionWithCookie(th.Context, post.Id, attachments[0].Actions[0].Id, th.BasicUser.Id, "", nil) require.NotNil(t, err) }) @@ -821,7 +870,7 @@ func TestPostActionRelativeURL(t *testing.T) { require.NotEmpty(t, attachments[0].Actions) require.NotEmpty(t, attachments[0].Actions[0].Id) - _, err = th.App.DoPostAction(th.Context, post.Id, attachments[0].Actions[0].Id, th.BasicUser.Id, "") + _, err = th.App.DoPostActionWithCookie(th.Context, post.Id, attachments[0].Actions[0].Id, th.BasicUser.Id, "", nil) require.NotNil(t, err) }) } @@ -898,7 +947,7 @@ func TestPostActionRelativePluginURL(t *testing.T) { require.NotEmpty(t, attachments[0].Actions) require.NotEmpty(t, attachments[0].Actions[0].Id) - _, err = th.App.DoPostAction(th.Context, post.Id, attachments[0].Actions[0].Id, th.BasicUser.Id, "") + _, err = th.App.DoPostActionWithCookie(th.Context, post.Id, attachments[0].Actions[0].Id, th.BasicUser.Id, "", nil) require.NotNil(t, err) }) @@ -938,7 +987,7 @@ func TestPostActionRelativePluginURL(t *testing.T) { require.NotEmpty(t, attachments[0].Actions) require.NotEmpty(t, attachments[0].Actions[0].Id) - _, err = th.App.DoPostAction(th.Context, post.Id, attachments[0].Actions[0].Id, th.BasicUser.Id, "") + _, err = th.App.DoPostActionWithCookie(th.Context, post.Id, attachments[0].Actions[0].Id, th.BasicUser.Id, "", nil) require.Nil(t, err) }) @@ -978,7 +1027,7 @@ func TestPostActionRelativePluginURL(t *testing.T) { require.NotEmpty(t, attachments[0].Actions) require.NotEmpty(t, attachments[0].Actions[0].Id) - _, err = th.App.DoPostAction(th.Context, post.Id, attachments[0].Actions[0].Id, th.BasicUser.Id, "") + _, err = th.App.DoPostActionWithCookie(th.Context, post.Id, attachments[0].Actions[0].Id, th.BasicUser.Id, "", nil) require.Nil(t, err) }) @@ -1018,7 +1067,7 @@ func TestPostActionRelativePluginURL(t *testing.T) { require.NotEmpty(t, attachments[0].Actions) require.NotEmpty(t, attachments[0].Actions[0].Id) - _, err = th.App.DoPostAction(th.Context, post.Id, attachments[0].Actions[0].Id, th.BasicUser.Id, "") + _, err = th.App.DoPostActionWithCookie(th.Context, post.Id, attachments[0].Actions[0].Id, th.BasicUser.Id, "", nil) require.Nil(t, err) }) } diff --git a/server/channels/app/opentracing/opentracing_layer.go b/server/channels/app/opentracing/opentracing_layer.go index e014b1f121..2f5f88efcb 100644 --- a/server/channels/app/opentracing/opentracing_layer.go +++ b/server/channels/app/opentracing/opentracing_layer.go @@ -3776,7 +3776,7 @@ func (a *OpenTracingAppLayer) DoCheckForAdminNotifications(trial bool) *model.Ap return resultVar0 } -func (a *OpenTracingAppLayer) DoCommandRequest(cmd *model.Command, p url.Values) (*model.Command, *model.CommandResponse, *model.AppError) { +func (a *OpenTracingAppLayer) DoCommandRequest(rctx request.CTX, cmd *model.Command, p url.Values) (*model.Command, *model.CommandResponse, *model.AppError) { origCtx := a.ctx span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.DoCommandRequest") @@ -3788,7 +3788,7 @@ func (a *OpenTracingAppLayer) DoCommandRequest(cmd *model.Command, p url.Values) }() defer span.Finish() - resultVar0, resultVar1, resultVar2 := a.app.DoCommandRequest(cmd, p) + resultVar0, resultVar1, resultVar2 := a.app.DoCommandRequest(rctx, cmd, p) if resultVar2 != nil { span.LogFields(spanlog.Error(resultVar2)) @@ -3894,28 +3894,6 @@ func (a *OpenTracingAppLayer) DoPermissionsMigrations() error { return resultVar0 } -func (a *OpenTracingAppLayer) DoPostAction(c request.CTX, postID string, actionId string, userID string, selectedOption string) (string, *model.AppError) { - origCtx := a.ctx - span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.DoPostAction") - - a.ctx = newCtx - a.app.Srv().Store().SetContext(newCtx) - defer func() { - a.app.Srv().Store().SetContext(origCtx) - a.ctx = origCtx - }() - - defer span.Finish() - resultVar0, resultVar1 := a.app.DoPostAction(c, postID, actionId, userID, selectedOption) - - if resultVar1 != nil { - span.LogFields(spanlog.Error(resultVar1)) - ext.Error.Set(span, true) - } - - return resultVar0, resultVar1 -} - func (a *OpenTracingAppLayer) DoPostActionWithCookie(c request.CTX, postID string, actionId string, userID string, selectedOption string, cookie *model.PostActionCookie) (string, *model.AppError) { origCtx := a.ctx span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.DoPostActionWithCookie") diff --git a/server/channels/app/slashcommands/command_test.go b/server/channels/app/slashcommands/command_test.go index a61a102016..9f13ac98fc 100644 --- a/server/channels/app/slashcommands/command_test.go +++ b/server/channels/app/slashcommands/command_test.go @@ -17,7 +17,6 @@ import ( "github.com/stretchr/testify/require" "github.com/mattermost/mattermost/server/public/model" - "github.com/mattermost/mattermost/server/v8/platform/services/httpservice" ) type InfiniteReader struct { @@ -363,7 +362,7 @@ func TestDoCommandRequest(t *testing.T) { })) defer server.Close() - _, resp, err := th.App.DoCommandRequest(&model.Command{URL: server.URL}, url.Values{}) + _, resp, err := th.App.DoCommandRequest(th.Context, &model.Command{URL: server.URL}, url.Values{}) require.Nil(t, err) assert.NotNil(t, resp) @@ -378,7 +377,7 @@ func TestDoCommandRequest(t *testing.T) { })) defer server.Close() - _, resp, err := th.App.DoCommandRequest(&model.Command{URL: server.URL}, url.Values{}) + _, resp, err := th.App.DoCommandRequest(th.Context, &model.Command{URL: server.URL}, url.Values{}) require.Nil(t, err) assert.NotNil(t, resp) @@ -393,7 +392,7 @@ func TestDoCommandRequest(t *testing.T) { // Since we limit the length of the response, no error will be returned and resp.Text will be a finite string - _, resp, err := th.App.DoCommandRequest(&model.Command{URL: server.URL}, url.Values{}) + _, resp, err := th.App.DoCommandRequest(th.Context, &model.Command{URL: server.URL}, url.Values{}) require.Nil(t, err) require.NotNil(t, resp) }) @@ -406,7 +405,7 @@ func TestDoCommandRequest(t *testing.T) { })) defer server.Close() - _, _, err := th.App.DoCommandRequest(&model.Command{URL: server.URL}, url.Values{}) + _, _, err := th.App.DoCommandRequest(th.Context, &model.Command{URL: server.URL}, url.Values{}) require.NotNil(t, err) require.Equal(t, "api.command.execute_command.failed.app_error", err.Id) }) @@ -419,29 +418,47 @@ func TestDoCommandRequest(t *testing.T) { })) defer server.Close() - _, _, err := th.App.DoCommandRequest(&model.Command{URL: server.URL}, url.Values{}) + _, _, err := th.App.DoCommandRequest(th.Context, &model.Command{URL: server.URL}, url.Values{}) require.NotNil(t, err) require.Equal(t, "api.command.execute_command.failed.app_error", err.Id) }) - t.Run("with a slow response", func(t *testing.T) { + t.Run("with a too slow response", func(t *testing.T) { done := make(chan bool) server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { <-done - io.Copy(w, strings.NewReader(`{"text": "Hello, World!"}`)) + io.Copy(w, strings.NewReader("Hello, World!")) })) defer server.Close() - th.App.HTTPService().(*httpservice.HTTPServiceImpl).RequestTimeout = 100 * time.Millisecond - defer func() { - th.App.HTTPService().(*httpservice.HTTPServiceImpl).RequestTimeout = httpservice.RequestTimeout - }() + th.App.UpdateConfig(func(cfg *model.Config) { + cfg.ServiceSettings.OutgoingIntegrationRequestsTimeout = model.NewInt64(1) + }) - _, _, err := th.App.DoCommandRequest(&model.Command{URL: server.URL}, url.Values{}) + _, _, err := th.App.DoCommandRequest(th.Context, &model.Command{URL: server.URL}, url.Values{}) require.NotNil(t, err) require.Equal(t, "api.command.execute_command.failed.app_error", err.Id) close(done) }) + + t.Run("with a too slow response, long timeout configured", func(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + time.Sleep(1 * time.Second) + + io.Copy(w, strings.NewReader("Hello, World!")) + })) + defer server.Close() + + th.App.UpdateConfig(func(cfg *model.Config) { + cfg.ServiceSettings.OutgoingIntegrationRequestsTimeout = model.NewInt64(2) + }) + + _, resp, err := th.App.DoCommandRequest(th.Context, &model.Command{URL: server.URL}, url.Values{}) + require.Nil(t, err) + + require.NotNil(t, resp) + assert.Equal(t, "Hello, World!", resp.Text) + }) } func TestMentionsToTeamMembers(t *testing.T) { diff --git a/server/channels/app/webhook.go b/server/channels/app/webhook.go index e2cadc260a..f721fd3d7f 100644 --- a/server/channels/app/webhook.go +++ b/server/channels/app/webhook.go @@ -13,6 +13,7 @@ import ( "regexp" "strings" "sync" + "time" "unicode/utf8" "github.com/mattermost/mattermost/server/public/model" @@ -118,7 +119,11 @@ func (a *App) TriggerWebhook(c request.CTX, payload *model.OutgoingWebhookPayloa defer wg.Done() webhookResp, err := a.doOutgoingWebhookRequest(url, body, contentType) if err != nil { - c.Logger().Error("Event POST failed.", mlog.Err(err)) + if errors.Is(err, context.DeadlineExceeded) { + c.Logger().Error("Outgoing Webhook POST timed out. Consider increasing ServiceSettings.OutgoingIntegrationRequestsTimeout.", mlog.Err(err)) + } else { + c.Logger().Error("Outgoing Webhook POST failed", mlog.Err(err)) + } return } @@ -158,7 +163,10 @@ func (a *App) TriggerWebhook(c request.CTX, payload *model.OutgoingWebhookPayloa } func (a *App) doOutgoingWebhookRequest(url string, body io.Reader, contentType string) (*model.OutgoingWebhookResponse, error) { - req, err := http.NewRequest("POST", url, body) + ctx, cancel := context.WithTimeout(context.Background(), time.Duration(*a.Config().ServiceSettings.OutgoingIntegrationRequestsTimeout)*time.Second) + defer cancel() + + req, err := http.NewRequestWithContext(ctx, "POST", url, body) if err != nil { return nil, err } diff --git a/server/channels/app/webhook_test.go b/server/channels/app/webhook_test.go index 0bca4d2042..9e3283e0f8 100644 --- a/server/channels/app/webhook_test.go +++ b/server/channels/app/webhook_test.go @@ -19,7 +19,6 @@ import ( "github.com/mattermost/mattermost/server/public/model" "github.com/mattermost/mattermost/server/v8/channels/testlib" - "github.com/mattermost/mattermost/server/v8/platform/services/httpservice" ) func TestCreateIncomingWebhookForChannel(t *testing.T) { @@ -787,7 +786,7 @@ func TestDoOutgoingWebhookRequest(t *testing.T) { resp, err := th.App.doOutgoingWebhookRequest(server.URL, strings.NewReader(""), "application/json") require.NoError(t, err) - assert.NotNil(t, resp) + require.NotNil(t, resp) assert.NotNil(t, resp.Text) assert.Equal(t, "Hello, World!", *resp.Text) }) @@ -835,16 +834,34 @@ func TestDoOutgoingWebhookRequest(t *testing.T) { defer server.Close() defer close(releaseHandler) - th.App.HTTPService().(*httpservice.HTTPServiceImpl).RequestTimeout = 500 * time.Millisecond - defer func() { - th.App.HTTPService().(*httpservice.HTTPServiceImpl).RequestTimeout = httpservice.RequestTimeout - }() + th.App.UpdateConfig(func(cfg *model.Config) { + cfg.ServiceSettings.OutgoingIntegrationRequestsTimeout = model.NewInt64(1) + }) _, err := th.App.doOutgoingWebhookRequest(server.URL, strings.NewReader(""), "application/json") require.Error(t, err) require.IsType(t, &url.Error{}, err) }) + t.Run("with a slow response, long timeout configured", func(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + time.Sleep(1 * time.Second) + + io.Copy(w, strings.NewReader(`{"text": "Hello, World!"}`)) + })) + defer server.Close() + + th.App.UpdateConfig(func(cfg *model.Config) { + cfg.ServiceSettings.OutgoingIntegrationRequestsTimeout = model.NewInt64(2) + }) + + resp, err := th.App.doOutgoingWebhookRequest(server.URL, strings.NewReader(""), "application/json") + require.NoError(t, err) + require.NotNil(t, resp) + assert.NotNil(t, resp.Text) + assert.Equal(t, "Hello, World!", *resp.Text) + }) + t.Run("without response", func(t *testing.T) { server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { })) diff --git a/server/i18n/en.json b/server/i18n/en.json index 24f7e87680..19255506c4 100644 --- a/server/i18n/en.json +++ b/server/i18n/en.json @@ -8264,7 +8264,7 @@ }, { "id": "interactive_message.decode_trigger_id.expired", - "translation": "Trigger ID for interactive dialog is expired. Trigger IDs live for a maximum of {{.Seconds}} seconds." + "translation": "Trigger ID for interactive dialog is expired. Trigger IDs live for a maximum of {{.Duration}}." }, { "id": "interactive_message.decode_trigger_id.missing_data", @@ -8922,6 +8922,10 @@ "id": "model.config.is_valid.move_thread.domain_invalid.app_error", "translation": "Invalid domain for move thread settings" }, + { + "id": "model.config.is_valid.outgoing_integrations_request_timeout.app_error", + "translation": "Invalid Outgoing Integrations Request Timeout for service settings. Must be a positive number." + }, { "id": "model.config.is_valid.password_length.app_error", "translation": "Minimum password length must be a whole number greater than or equal to {{.MinLength}} and less than or equal to {{.MaxLength}}." diff --git a/server/platform/services/telemetry/telemetry.go b/server/platform/services/telemetry/telemetry.go index fac03bf47c..6effefc22b 100644 --- a/server/platform/services/telemetry/telemetry.go +++ b/server/platform/services/telemetry/telemetry.go @@ -411,6 +411,7 @@ func (ts *TelemetryService) trackConfig() { "enable_incoming_webhooks": cfg.ServiceSettings.EnableIncomingWebhooks, "enable_outgoing_webhooks": cfg.ServiceSettings.EnableOutgoingWebhooks, "enable_commands": *cfg.ServiceSettings.EnableCommands, + "outgoing_integrations_requests_timeout": cfg.ServiceSettings.OutgoingIntegrationRequestsTimeout, "enable_post_username_override": cfg.ServiceSettings.EnablePostUsernameOverride, "enable_post_icon_override": cfg.ServiceSettings.EnablePostIconOverride, "enable_user_access_tokens": *cfg.ServiceSettings.EnableUserAccessTokens, diff --git a/server/public/model/config.go b/server/public/model/config.go index 2bc83f7551..06226ccb25 100644 --- a/server/public/model/config.go +++ b/server/public/model/config.go @@ -214,6 +214,8 @@ const ( DataRetentionSettingsDefaultTimeBetweenBatchesMilliseconds = 100 DataRetentionSettingsDefaultRetentionIdsBatchSize = 100 + OutgoingIntegrationRequestsDefaultTimeout = 30 + PluginSettingsDefaultDirectory = "./plugins" PluginSettingsDefaultClientDirectory = "./client/plugins" PluginSettingsDefaultEnableMarketplace = true @@ -309,6 +311,7 @@ type ServiceSettings struct { EnableIncomingWebhooks *bool `access:"integrations_integration_management"` EnableOutgoingWebhooks *bool `access:"integrations_integration_management"` EnableCommands *bool `access:"integrations_integration_management"` + OutgoingIntegrationRequestsTimeout *int64 `access:"integrations_integration_management"` // In seconds. EnablePostUsernameOverride *bool `access:"integrations_integration_management"` EnablePostIconOverride *bool `access:"integrations_integration_management"` GoogleDeveloperKey *string `access:"site_posts,write_restrictable,cloud_restrictable"` @@ -509,6 +512,10 @@ func (s *ServiceSettings) SetDefaults(isUpdate bool) { s.EnableOutgoingWebhooks = NewBool(true) } + if s.OutgoingIntegrationRequestsTimeout == nil { + s.OutgoingIntegrationRequestsTimeout = NewInt64(OutgoingIntegrationRequestsDefaultTimeout) + } + if s.ConnectionSecurity == nil { s.ConnectionSecurity = NewString("") } @@ -4011,6 +4018,10 @@ func (s *ServiceSettings) isValid() *AppError { return NewAppError("Config.IsValid", "model.config.is_valid.listen_address.app_error", nil, "", http.StatusBadRequest) } + if *s.OutgoingIntegrationRequestsTimeout <= 0 { + return NewAppError("Config.IsValid", "model.config.is_valid.outgoing_integrations_request_timeout.app_error", nil, "", http.StatusBadRequest) + } + if *s.ExperimentalGroupUnreadChannels != GroupUnreadChannelsDisabled && *s.ExperimentalGroupUnreadChannels != GroupUnreadChannelsDefaultOn && *s.ExperimentalGroupUnreadChannels != GroupUnreadChannelsDefaultOff { diff --git a/server/public/model/config_test.go b/server/public/model/config_test.go index f590bc0bf8..78a3ee0832 100644 --- a/server/public/model/config_test.go +++ b/server/public/model/config_test.go @@ -74,6 +74,47 @@ func TestConfigEmptySiteName(t *testing.T) { require.Equal(t, *c1.TeamSettings.SiteName, TeamSettingsDefaultSiteName) } +func TestServiceSettingsIsValid(t *testing.T) { + for name, test := range map[string]struct { + ServiceSettings ServiceSettings + ExpectError bool + }{ + "empty": { + ServiceSettings: ServiceSettings{}, + ExpectError: false, + }, + "OutgoingIntegrationRequestsTimeout is negative": { + ServiceSettings: ServiceSettings{ + OutgoingIntegrationRequestsTimeout: NewInt64(-1), + }, + ExpectError: true, + }, + "OutgoingIntegrationRequestsTimeout is zero": { + ServiceSettings: ServiceSettings{ + OutgoingIntegrationRequestsTimeout: NewInt64(0), + }, + ExpectError: true, + }, + "OutgoingIntegrationRequestsTimeout is positiv": { + ServiceSettings: ServiceSettings{ + OutgoingIntegrationRequestsTimeout: NewInt64(1), + }, + ExpectError: false, + }, + } { + t.Run(name, func(t *testing.T) { + test.ServiceSettings.SetDefaults(false) + + appErr := test.ServiceSettings.isValid() + if test.ExpectError { + assert.NotNil(t, appErr) + } else { + assert.Nil(t, appErr) + } + }) + } +} + func TestConfigEnableDeveloper(t *testing.T) { testCases := []struct { Description string diff --git a/server/public/model/integration_action.go b/server/public/model/integration_action.go index e215bdca2d..009995dcba 100644 --- a/server/public/model/integration_action.go +++ b/server/public/model/integration_action.go @@ -19,12 +19,12 @@ import ( "reflect" "strconv" "strings" + "time" ) const ( - PostActionTypeButton = "button" - PostActionTypeSelect = "select" - InteractiveDialogTriggerTimeoutMilliseconds = 3000 + PostActionTypeButton = "button" + PostActionTypeSelect = "select" ) var PostActionRetainPropKeys = []string{"from_webhook", "override_username", "override_icon_url"} @@ -280,7 +280,7 @@ func (r *PostActionIntegrationRequest) GenerateTriggerId(s crypto.Signer) (strin return clientTriggerId, triggerId, nil } -func DecodeAndVerifyTriggerId(triggerId string, s *ecdsa.PrivateKey) (string, string, *AppError) { +func DecodeAndVerifyTriggerId(triggerId string, s *ecdsa.PrivateKey, timeout time.Duration) (string, string, *AppError) { triggerIdBytes, err := base64.StdEncoding.DecodeString(triggerId) if err != nil { return "", "", NewAppError("DecodeAndVerifyTriggerId", "interactive_message.decode_trigger_id.base64_decode_failed", nil, "", http.StatusBadRequest).Wrap(err) @@ -296,9 +296,8 @@ func DecodeAndVerifyTriggerId(triggerId string, s *ecdsa.PrivateKey) (string, st timestampStr := split[2] timestamp, _ := strconv.ParseInt(timestampStr, 10, 64) - now := GetMillis() - if now-timestamp > InteractiveDialogTriggerTimeoutMilliseconds { - return "", "", NewAppError("DecodeAndVerifyTriggerId", "interactive_message.decode_trigger_id.expired", map[string]any{"Seconds": InteractiveDialogTriggerTimeoutMilliseconds / 1000}, "", http.StatusBadRequest) + if time.Since(time.UnixMilli(timestamp)) > timeout { + return "", "", NewAppError("DecodeAndVerifyTriggerId", "interactive_message.decode_trigger_id.expired", map[string]any{"Duration": timeout.String()}, "", http.StatusBadRequest) } signature, err := base64.StdEncoding.DecodeString(split[3]) @@ -327,8 +326,8 @@ func DecodeAndVerifyTriggerId(triggerId string, s *ecdsa.PrivateKey) (string, st return clientTriggerId, userId, nil } -func (r *OpenDialogRequest) DecodeAndVerifyTriggerId(s *ecdsa.PrivateKey) (string, string, *AppError) { - return DecodeAndVerifyTriggerId(r.TriggerId, s) +func (r *OpenDialogRequest) DecodeAndVerifyTriggerId(s *ecdsa.PrivateKey, timeout time.Duration) (string, string, *AppError) { + return DecodeAndVerifyTriggerId(r.TriggerId, s, timeout) } func (o *Post) StripActionIntegrations() { diff --git a/server/public/model/integration_action_test.go b/server/public/model/integration_action_test.go index 4b550c38e0..1bf2a3ce23 100644 --- a/server/public/model/integration_action_test.go +++ b/server/public/model/integration_action_test.go @@ -9,6 +9,7 @@ import ( "crypto/rand" "encoding/base64" "testing" + "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -22,7 +23,7 @@ func TestTriggerIdDecodeAndVerification(t *testing.T) { userId := NewId() clientTriggerId, triggerId, appErr := GenerateTriggerId(userId, key) require.Nil(t, appErr) - decodedClientTriggerId, decodedUserId, appErr := DecodeAndVerifyTriggerId(triggerId, key) + decodedClientTriggerId, decodedUserId, appErr := DecodeAndVerifyTriggerId(triggerId, key, OutgoingIntegrationRequestsDefaultTimeout*time.Second) assert.Nil(t, appErr) assert.Equal(t, clientTriggerId, decodedClientTriggerId) assert.Equal(t, userId, decodedUserId) @@ -35,38 +36,38 @@ func TestTriggerIdDecodeAndVerification(t *testing.T) { clientTriggerId, triggerId, appErr := actionReq.GenerateTriggerId(key) require.Nil(t, appErr) dialogReq := &OpenDialogRequest{TriggerId: triggerId} - decodedClientTriggerId, decodedUserId, appErr := dialogReq.DecodeAndVerifyTriggerId(key) + decodedClientTriggerId, decodedUserId, appErr := dialogReq.DecodeAndVerifyTriggerId(key, OutgoingIntegrationRequestsDefaultTimeout*time.Second) assert.Nil(t, appErr) assert.Equal(t, clientTriggerId, decodedClientTriggerId) assert.Equal(t, actionReq.UserId, decodedUserId) }) t.Run("should fail on base64 decode", func(t *testing.T) { - _, _, appErr := DecodeAndVerifyTriggerId("junk!", key) + _, _, appErr := DecodeAndVerifyTriggerId("junk!", key, OutgoingIntegrationRequestsDefaultTimeout*time.Second) require.NotNil(t, appErr) assert.Equal(t, "interactive_message.decode_trigger_id.base64_decode_failed", appErr.Id) }) t.Run("should fail on trigger parsing", func(t *testing.T) { - _, _, appErr := DecodeAndVerifyTriggerId(base64.StdEncoding.EncodeToString([]byte("junk!")), key) + _, _, appErr := DecodeAndVerifyTriggerId(base64.StdEncoding.EncodeToString([]byte("junk!")), key, OutgoingIntegrationRequestsDefaultTimeout*time.Second) require.NotNil(t, appErr) assert.Equal(t, "interactive_message.decode_trigger_id.missing_data", appErr.Id) }) t.Run("should fail on expired timestamp", func(t *testing.T) { - _, _, appErr := DecodeAndVerifyTriggerId(base64.StdEncoding.EncodeToString([]byte("some-trigger-id:some-user-id:1234567890:junksignature")), key) + _, _, appErr := DecodeAndVerifyTriggerId(base64.StdEncoding.EncodeToString([]byte("some-trigger-id:some-user-id:1234567890:junksignature")), key, OutgoingIntegrationRequestsDefaultTimeout*time.Second) require.NotNil(t, appErr) assert.Equal(t, "interactive_message.decode_trigger_id.expired", appErr.Id) }) t.Run("should fail on base64 decoding signature", func(t *testing.T) { - _, _, appErr := DecodeAndVerifyTriggerId(base64.StdEncoding.EncodeToString([]byte("some-trigger-id:some-user-id:12345678900000:junk!")), key) + _, _, appErr := DecodeAndVerifyTriggerId(base64.StdEncoding.EncodeToString([]byte("some-trigger-id:some-user-id:12345678900000:junk!")), key, OutgoingIntegrationRequestsDefaultTimeout*time.Second) require.NotNil(t, appErr) assert.Equal(t, "interactive_message.decode_trigger_id.base64_decode_failed_signature", appErr.Id) }) t.Run("should fail on bad signature", func(t *testing.T) { - _, _, appErr := DecodeAndVerifyTriggerId(base64.StdEncoding.EncodeToString([]byte("some-trigger-id:some-user-id:12345678900000:junk")), key) + _, _, appErr := DecodeAndVerifyTriggerId(base64.StdEncoding.EncodeToString([]byte("some-trigger-id:some-user-id:12345678900000:junk")), key, OutgoingIntegrationRequestsDefaultTimeout*time.Second) require.NotNil(t, appErr) assert.Equal(t, "interactive_message.decode_trigger_id.signature_decode_failed", appErr.Id) }) @@ -76,7 +77,7 @@ func TestTriggerIdDecodeAndVerification(t *testing.T) { require.Nil(t, appErr) newKey, keyErr := ecdsa.GenerateKey(elliptic.P256(), rand.Reader) require.NoError(t, keyErr) - _, _, appErr = DecodeAndVerifyTriggerId(triggerId, newKey) + _, _, appErr = DecodeAndVerifyTriggerId(triggerId, newKey, OutgoingIntegrationRequestsDefaultTimeout*time.Second) require.NotNil(t, appErr) assert.Equal(t, "interactive_message.decode_trigger_id.verify_signature_failed", appErr.Id) }) diff --git a/webapp/channels/src/components/admin_console/admin_definition.tsx b/webapp/channels/src/components/admin_console/admin_definition.tsx index 348b6f68c1..4d7b3f5ca5 100644 --- a/webapp/channels/src/components/admin_console/admin_definition.tsx +++ b/webapp/channels/src/components/admin_console/admin_definition.tsx @@ -6204,6 +6204,50 @@ const AdminDefinition: AdminDefinitionType = { isDisabled: it.not(it.userHasWritePermissionOnResource(RESOURCE_KEYS.INTEGRATIONS.INTEGRATION_MANAGEMENT)), isHidden: it.licensedForFeature('Cloud'), }, + { + type: 'number', + key: 'ServiceSettings.OutgoingIntegrationRequestsTimeout', + label: t('admin.service.integrationRequestTitle'), + label_default: 'Integration request timeout: ', + help_text: t('admin.service.integrationRequestDesc'), + help_text_default: 'The number of seconds to wait for Integration requests. That includes Slash Commands, Outgoing Webhooks, Interactive Messages and Interactive Dialogs.', + help_text_values: { + slashCommands: (msg: string) => ( + + {msg} + + ), + outgoingWebhooks: (msg: string) => ( + + {msg} + + ), + interactiveMessages: (msg: string) => ( + + {msg} + + ), + interactiveDialogs: (msg: string) => ( + + {msg} + + ), + }, + help_text_markdown: false, + isDisabled: it.not(it.userHasWritePermissionOnResource(RESOURCE_KEYS.INTEGRATIONS.INTEGRATION_MANAGEMENT)), + }, { type: 'bool', key: 'ServiceSettings.EnablePostUsernameOverride', diff --git a/webapp/channels/src/i18n/en.json b/webapp/channels/src/i18n/en.json index e3020b6db3..866ac9b182 100644 --- a/webapp/channels/src/i18n/en.json +++ b/webapp/channels/src/i18n/en.json @@ -2244,6 +2244,8 @@ "admin.service.iconTitle": "Enable integrations to override profile picture icons:", "admin.service.insecureTlsDesc": "When true, any outgoing HTTPS requests will accept unverified, self-signed certificates. For example, outgoing webhooks to a server with a self-signed TLS certificate, using any domain, will be allowed. Note that this makes these connections susceptible to man-in-the-middle attacks.", "admin.service.insecureTlsTitle": "Enable Insecure Outgoing Connections: ", + "admin.service.integrationRequestDesc": "The number of seconds to wait for Integration requests. That includes Slash Commands, Outgoing Webhooks, Interactive Messages and Interactive Dialogs.", + "admin.service.integrationRequestTitle": "Integration request timeout: ", "admin.service.internalConnectionsDesc": "A whitelist of local network addresses that can be requested by the Mattermost server on behalf of a client. Care should be used when configuring this setting to prevent unintended access to your local network. See documentation to learn more. Changing this requires a server restart before taking effect.", "admin.service.internalConnectionsEx": "webhooks.internal.example.com 127.0.0.1 10.0.16.0/28", "admin.service.internalConnectionsTitle": "Allow untrusted internal connections to: ", diff --git a/webapp/channels/src/utils/constants.tsx b/webapp/channels/src/utils/constants.tsx index a4465c8d49..8cc842c1fc 100644 --- a/webapp/channels/src/utils/constants.tsx +++ b/webapp/channels/src/utils/constants.tsx @@ -1139,6 +1139,8 @@ export const DeveloperLinks = { ENABLE_OAUTH2: 'https://mattermost.com/pl/enable-oauth', INCOMING_WEBHOOKS: 'https://mattermost.com/pl/incoming-webhooks', OUTGOING_WEBHOOKS: 'https://mattermost.com/pl/outgoing-webhooks', + INTERACTIVE_MESSAGES: 'https://mattermost.com/pl/interactive-messages', + INTERACTIVE_DIALOGS: 'https://mattermost.com/pl/interactive-dialogs', PERSONAL_ACCESS_TOKENS: 'https://mattermost.com/pl/personal-access-tokens', PLUGIN_SIGNING: 'https://mattermost.com/pl/sign-plugins', PLUGINS: 'https://mattermost.com/pl/plugins', diff --git a/webapp/platform/types/src/config.ts b/webapp/platform/types/src/config.ts index 7afbac8bf9..8112d2233d 100644 --- a/webapp/platform/types/src/config.ts +++ b/webapp/platform/types/src/config.ts @@ -303,6 +303,7 @@ export type ServiceSettings = { EnableIncomingWebhooks: boolean; EnableOutgoingWebhooks: boolean; EnableCommands: boolean; + OutgoingIntegrationRequestsTimeout: number; EnablePostUsernameOverride: boolean; EnablePostIconOverride: boolean; EnableLinkPreviews: boolean;