diff --git a/server/channels/api4/integration_action.go b/server/channels/api4/integration_action.go index 75b9cc5253..24912b38c6 100644 --- a/server/channels/api4/integration_action.go +++ b/server/channels/api4/integration_action.go @@ -84,7 +84,7 @@ func openDialog(c *Context, w http.ResponseWriter, r *http.Request) { return } - if appErr := c.App.OpenInteractiveDialog(dialog); appErr != nil { + if appErr := c.App.OpenInteractiveDialog(c.AppContext, dialog); appErr != nil { c.Err = appErr return } diff --git a/server/channels/api4/integration_action_test.go b/server/channels/api4/integration_action_test.go index 82e510329d..ae0dbf0062 100644 --- a/server/channels/api4/integration_action_test.go +++ b/server/channels/api4/integration_action_test.go @@ -16,6 +16,8 @@ import ( "github.com/stretchr/testify/require" "github.com/mattermost/mattermost/server/public/model" + "github.com/mattermost/mattermost/server/public/shared/mlog" + "github.com/mattermost/mattermost/server/v8/channels/testlib" ) type testHandler struct { @@ -209,6 +211,53 @@ func TestOpenDialog(t *testing.T) { require.NoError(t, err) }) + t.Run("Should pass with too long display name of elements", func(t *testing.T) { + request.Dialog.Elements = []model.DialogElement{ + { + DisplayName: "Very very long Element Name", + Name: "element_name", + Type: "text", + Placeholder: "Enter a value", + }, + } + + buffer := &mlog.Buffer{} + err := mlog.AddWriterTarget(th.TestLogger, buffer, true, mlog.StdAll...) + require.NoError(t, err) + + _, err = client.OpenInteractiveDialog(context.Background(), request) + require.NoError(t, err) + + require.NoError(t, th.TestLogger.Flush()) + testlib.AssertLog(t, buffer, mlog.LvlWarn.Name, "Interactive dialog is invalid") + }) + + t.Run("Should pass with same elements", func(t *testing.T) { + request.Dialog.Elements = []model.DialogElement{ + { + DisplayName: "Element Name", + Name: "element_name", + Type: "text", + Placeholder: "Enter a value", + }, + { + DisplayName: "Element Name", + Name: "element_name", + Type: "text", + Placeholder: "Enter a value", + }, + } + buffer := &mlog.Buffer{} + err := mlog.AddWriterTarget(th.TestLogger, buffer, true, mlog.StdAll...) + require.NoError(t, err) + + _, err = client.OpenInteractiveDialog(context.Background(), request) + require.NoError(t, err) + + require.NoError(t, th.TestLogger.Flush()) + testlib.AssertLog(t, buffer, mlog.LvlWarn.Name, "Interactive dialog is invalid") + }) + t.Run("Should pass with nil elements slice", func(t *testing.T) { request.Dialog.Elements = nil _, err := client.OpenInteractiveDialog(context.Background(), request) diff --git a/server/channels/app/app_iface.go b/server/channels/app/app_iface.go index cd8bdbdb53..8dcb64b4a2 100644 --- a/server/channels/app/app_iface.go +++ b/server/channels/app/app_iface.go @@ -967,7 +967,7 @@ type AppIface interface { NotificationsLog() *mlog.Logger NotifySelfHostedSignupProgress(progress string, userId string) NotifySharedChannelUserUpdate(user *model.User) - OpenInteractiveDialog(request model.OpenDialogRequest) *model.AppError + OpenInteractiveDialog(c request.CTX, request model.OpenDialogRequest) *model.AppError OriginChecker() func(*http.Request) bool OutgoingOAuthConnections() einterfaces.OutgoingOAuthConnectionInterface PatchChannel(c request.CTX, channel *model.Channel, patch *model.ChannelPatch, userID string) (*model.Channel, *model.AppError) diff --git a/server/channels/app/integration_action.go b/server/channels/app/integration_action.go index f840cd9824..dadfcbb08b 100644 --- a/server/channels/app/integration_action.go +++ b/server/channels/app/integration_action.go @@ -453,13 +453,17 @@ func (a *App) DoLocalRequest(c request.CTX, rawURL string, body []byte) (*http.R return a.doPluginRequest(c, "POST", rawURL, nil, body) } -func (a *App) OpenInteractiveDialog(request model.OpenDialogRequest) *model.AppError { +func (a *App) OpenInteractiveDialog(c request.CTX, request model.OpenDialogRequest) *model.AppError { timeout := time.Duration(*a.Config().ServiceSettings.OutgoingIntegrationRequestsTimeout) * time.Second clientTriggerId, userID, appErr := request.DecodeAndVerifyTriggerId(a.AsymmetricSigningKey(), timeout) if appErr != nil { return appErr } + if dialogErr := request.IsValid(); dialogErr != nil { + c.Logger().Warn("Interactive dialog is invalid", mlog.Err(dialogErr)) + } + request.TriggerId = clientTriggerId jsonRequest, err := json.Marshal(request) diff --git a/server/channels/app/opentracing/opentracing_layer.go b/server/channels/app/opentracing/opentracing_layer.go index ce0a670e27..6499b65ae3 100644 --- a/server/channels/app/opentracing/opentracing_layer.go +++ b/server/channels/app/opentracing/opentracing_layer.go @@ -13181,7 +13181,7 @@ func (a *OpenTracingAppLayer) OnSharedChannelsSyncMsg(msg *model.SyncMsg, rc *mo return resultVar0, resultVar1 } -func (a *OpenTracingAppLayer) OpenInteractiveDialog(request model.OpenDialogRequest) *model.AppError { +func (a *OpenTracingAppLayer) OpenInteractiveDialog(c request.CTX, request model.OpenDialogRequest) *model.AppError { origCtx := a.ctx span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.OpenInteractiveDialog") @@ -13193,7 +13193,7 @@ func (a *OpenTracingAppLayer) OpenInteractiveDialog(request model.OpenDialogRequ }() defer span.Finish() - resultVar0 := a.app.OpenInteractiveDialog(request) + resultVar0 := a.app.OpenInteractiveDialog(c, request) if resultVar0 != nil { span.LogFields(spanlog.Error(resultVar0)) diff --git a/server/channels/app/plugin_api.go b/server/channels/app/plugin_api.go index 457a6d4c3c..4b1cbd0a99 100644 --- a/server/channels/app/plugin_api.go +++ b/server/channels/app/plugin_api.go @@ -849,7 +849,7 @@ func (api *PluginAPI) SetTeamIcon(teamID string, data []byte) *model.AppError { } func (api *PluginAPI) OpenInteractiveDialog(dialog model.OpenDialogRequest) *model.AppError { - return api.app.OpenInteractiveDialog(dialog) + return api.app.OpenInteractiveDialog(api.ctx, dialog) } func (api *PluginAPI) RemoveTeamIcon(teamID string) *model.AppError { diff --git a/server/public/go.mod b/server/public/go.mod index b5d14ebaa5..be5e177d59 100644 --- a/server/public/go.mod +++ b/server/public/go.mod @@ -13,6 +13,7 @@ require ( github.com/gorilla/mux v1.8.1 github.com/gorilla/websocket v1.5.1 github.com/hashicorp/go-hclog v1.6.2 + github.com/hashicorp/go-multierror v1.1.1 github.com/hashicorp/go-plugin v1.6.0 github.com/lib/pq v1.10.9 github.com/mattermost/go-i18n v1.11.1-0.20211013152124-5c415071e404 @@ -40,6 +41,7 @@ require ( github.com/go-asn1-ber/asn1-ber v1.5.5 // indirect github.com/golang/protobuf v1.5.3 // indirect github.com/google/uuid v1.6.0 // indirect + github.com/hashicorp/errwrap v1.0.0 // indirect github.com/hashicorp/yamux v0.1.1 // indirect github.com/kr/pretty v0.3.0 // indirect github.com/mattn/go-colorable v0.1.13 // indirect diff --git a/server/public/go.sum b/server/public/go.sum index edaae5de72..cbb2bcb905 100644 --- a/server/public/go.sum +++ b/server/public/go.sum @@ -77,8 +77,12 @@ github.com/gorilla/websocket v1.5.1 h1:gmztn0JnHVt9JZquRuzLw3g4wouNVzKL15iLr/zn/ github.com/gorilla/websocket v1.5.1/go.mod h1:x3kM2JMyaluk02fnUJpQuwD2dCS5NDG2ZHL0uE0tcaY= github.com/gregjones/httpcache v0.0.0-20180305231024-9cad4c3443a7/go.mod h1:FecbI9+v66THATjSRHfNgh1IVFe/9kFxbXtjV0ctIMA= github.com/grpc-ecosystem/grpc-gateway v1.5.0/go.mod h1:RSKVYQBd5MCa4OVpNdGskqpgL2+G+NZTnrVHpWWfpdw= +github.com/hashicorp/errwrap v1.0.0 h1:hLrqtEDnRye3+sgx6z4qVLNuviH3MR5aQ0ykNJa/UYA= +github.com/hashicorp/errwrap v1.0.0/go.mod h1:YH+1FKiLXxHSkmPseP+kNlulaMuP3n2brvKWEqk/Jc4= github.com/hashicorp/go-hclog v1.6.2 h1:NOtoftovWkDheyUM/8JW3QMiXyxJK3uHRK7wV04nD2I= github.com/hashicorp/go-hclog v1.6.2/go.mod h1:W4Qnvbt70Wk/zYJryRzDRU/4r0kIg0PVHBcfoyhpF5M= +github.com/hashicorp/go-multierror v1.1.1 h1:H5DkEtf6CXdFp0N0Em5UCwQpXMWke8IA0+lD48awMYo= +github.com/hashicorp/go-multierror v1.1.1/go.mod h1:iw975J/qwKPdAO1clOe2L8331t/9/fmwbPZ6JB6eMoM= github.com/hashicorp/go-plugin v1.6.0 h1:wgd4KxHJTVGGqWBq4QPB1i5BZNEx9BR8+OFmHDmTk8A= github.com/hashicorp/go-plugin v1.6.0/go.mod h1:lBS5MtSSBZk0SHc66KACcjjlU6WzEVP/8pwz68aMkCI= github.com/hashicorp/yamux v0.1.1 h1:yrQxtgseBDrq9Y652vSRDvsKCJKOUD+GzTS4Y0Y8pvE= diff --git a/server/public/model/integration_action.go b/server/public/model/integration_action.go index f9a06e80ae..011cc3c522 100644 --- a/server/public/model/integration_action.go +++ b/server/public/model/integration_action.go @@ -20,11 +20,22 @@ import ( "strconv" "strings" "time" + + "github.com/hashicorp/go-multierror" + "github.com/pkg/errors" ) const ( - PostActionTypeButton = "button" - PostActionTypeSelect = "select" + PostActionTypeButton = "button" + PostActionTypeSelect = "select" + DialogTitleMaxLength = 24 + DialogElementDisplayNameMaxLength = 24 + DialogElementNameMaxLength = 300 + DialogElementHelpTextMaxLength = 150 + DialogElementTextMaxLength = 150 + DialogElementTextareaMaxLength = 3000 + DialogElementSelectMaxLength = 3000 + DialogElementBoolMaxLength = 150 ) var PostActionRetainPropKeys = []string{"from_webhook", "override_username", "override_icon_url"} @@ -335,6 +346,148 @@ func (r *OpenDialogRequest) DecodeAndVerifyTriggerId(s *ecdsa.PrivateKey, timeou return DecodeAndVerifyTriggerId(r.TriggerId, s, timeout) } +func (r *OpenDialogRequest) IsValid() error { + var multiErr *multierror.Error + if r.URL == "" { + multiErr = multierror.Append(multiErr, errors.New("empty URL")) + } + + if r.TriggerId == "" { + multiErr = multierror.Append(multiErr, errors.New("empty trigger id")) + } + + err := r.Dialog.IsValid() + if err != nil { + multiErr = multierror.Append(multiErr, err) + } + + return multiErr.ErrorOrNil() +} + +func (d *Dialog) IsValid() error { + var multiErr *multierror.Error + + if d.Title == "" || len(d.Title) > DialogTitleMaxLength { + multiErr = multierror.Append(multiErr, errors.Errorf("invalid dialog title %q", d.Title)) + } + + if d.IconURL != "" && !IsValidHTTPURL(d.IconURL) { + multiErr = multierror.Append(multiErr, errors.New("invalid icon url")) + } + + if len(d.Elements) != 0 { + elementMap := make(map[string]bool) + + for _, element := range d.Elements { + if elementMap[element.Name] { + multiErr = multierror.Append(multiErr, errors.Errorf("duplicate dialog element %q", element.Name)) + } + elementMap[element.Name] = true + + err := element.IsValid() + if err != nil { + multiErr = multierror.Append(multiErr, errors.Wrapf(err, "%q field is not valid", element.Name)) + } + } + } + return multiErr.ErrorOrNil() +} + +func (e *DialogElement) IsValid() error { + var multiErr *multierror.Error + textSubTypes := map[string]bool{ + "": true, + "text": true, + "email": true, + "number": true, + "tel": true, + "url": true, + "password": true, + } + + if e.MinLength < 0 { + multiErr = multierror.Append(multiErr, errors.Errorf("min length cannot be a negative number, got %d", e.MinLength)) + } + if e.MinLength > e.MaxLength { + multiErr = multierror.Append(multiErr, errors.Errorf("min length should be less then max length, got %d > %d", e.MinLength, e.MaxLength)) + } + + multiErr = multierror.Append(multiErr, checkMaxLength("DisplayName", e.DisplayName, DialogElementDisplayNameMaxLength)) + multiErr = multierror.Append(multiErr, checkMaxLength("Name", e.Name, DialogElementNameMaxLength)) + multiErr = multierror.Append(multiErr, checkMaxLength("HelpText", e.HelpText, DialogElementHelpTextMaxLength)) + + switch e.Type { + case "text": + multiErr = multierror.Append(multiErr, checkMaxLength("Default", e.Default, DialogElementTextMaxLength)) + multiErr = multierror.Append(multiErr, checkMaxLength("Placeholder", e.Placeholder, DialogElementTextMaxLength)) + if _, ok := textSubTypes[e.SubType]; !ok { + multiErr = multierror.Append(multiErr, errors.Errorf("invalid subtype %q", e.Type)) + } + + case "textarea": + multiErr = multierror.Append(multiErr, checkMaxLength("Default", e.Default, DialogElementTextareaMaxLength)) + multiErr = multierror.Append(multiErr, checkMaxLength("Placeholder", e.Placeholder, DialogElementTextareaMaxLength)) + + if _, ok := textSubTypes[e.SubType]; !ok { + multiErr = multierror.Append(multiErr, errors.Errorf("invalid subtype %q", e.Type)) + } + + case "select": + multiErr = multierror.Append(multiErr, checkMaxLength("Default", e.Default, DialogElementSelectMaxLength)) + multiErr = multierror.Append(multiErr, checkMaxLength("Placeholder", e.Placeholder, DialogElementSelectMaxLength)) + if e.DataSource != "" && e.DataSource != "users" && e.DataSource != "channels" { + multiErr = multierror.Append(multiErr, errors.Errorf("invalid data source %q, allowed are 'users' or 'channels'", e.DataSource)) + } + if e.DataSource == "" && !isDefaultInOptions(e.Default, e.Options) { + multiErr = multierror.Append(multiErr, errors.Errorf("default value %q doesn't exist in options ", e.Default)) + } + + case "bool": + if e.Default != "" && e.Default != "true" && e.Default != "false" { + multiErr = multierror.Append(multiErr, errors.New("invalid default of bool")) + } + multiErr = multierror.Append(multiErr, checkMaxLength("Placeholder", e.Placeholder, DialogElementBoolMaxLength)) + + case "radio": + if !isDefaultInOptions(e.Default, e.Options) { + multiErr = multierror.Append(multiErr, errors.Errorf("default value %q doesn't exist in options ", e.Default)) + } + + default: + multiErr = multierror.Append(multiErr, errors.Errorf("invalid element type: %q", e.Type)) + } + + return multiErr.ErrorOrNil() +} + +func isDefaultInOptions(defaultValue string, options []*PostActionOptions) bool { + if defaultValue == "" { + return true + } + + for _, option := range options { + if defaultValue == option.Value { + return true + } + } + + return false +} + +func checkMaxLength(fieldName string, field string, length int) error { + var valid bool + // DisplayName and Name are required fields + if fieldName == "DisplayName" || fieldName == "Name" { + valid = len(field) > 0 && len(field) > length + } else { + valid = len(field) > length + } + if valid { + return errors.Errorf("%v cannot be longer than %d characters", fieldName, length) + } + return nil +} + func (o *Post) StripActionIntegrations() { attachments := o.Attachments() if o.GetProp("attachments") != nil { diff --git a/server/public/model/integration_action_test.go b/server/public/model/integration_action_test.go index c7b581d9d1..9bafd0fde2 100644 --- a/server/public/model/integration_action_test.go +++ b/server/public/model/integration_action_test.go @@ -8,6 +8,7 @@ import ( "crypto/elliptic" "crypto/rand" "encoding/base64" + "strings" "testing" "time" @@ -180,3 +181,190 @@ func TestPostActionIntegrationEquals(t *testing.T) { require.True(t, pa1.Equals(pa2)) }) } + +func TestOpenDialogRequestIsValid(t *testing.T) { + getBaseOpenDialogRequest := func() OpenDialogRequest { + return OpenDialogRequest{ + TriggerId: "triggerId", + URL: "http://localhost:8065", + Dialog: Dialog{ + CallbackId: "callbackid", + Title: "Some Title", + Elements: []DialogElement{ + { + DisplayName: "Element Name", + Name: "element_name", + Type: "text", + Placeholder: "Enter a value", + }, + }, + SubmitLabel: "Submit", + NotifyOnCancel: false, + State: "somestate", + }, + } + } + + t.Run("should pass validation", func(t *testing.T) { + request := getBaseOpenDialogRequest() + err := request.IsValid() + assert.NoError(t, err) + }) + + t.Run("should fail on empty url", func(t *testing.T) { + request := getBaseOpenDialogRequest() + request.URL = "" + err := request.IsValid() + assert.ErrorContains(t, err, "empty URL") + }) + + t.Run("should fail on empty trigger", func(t *testing.T) { + request := getBaseOpenDialogRequest() + request.TriggerId = "" + err := request.IsValid() + assert.ErrorContains(t, err, "empty trigger id") + }) + + t.Run("should fail on empty dialog title", func(t *testing.T) { + request := getBaseOpenDialogRequest() + request.Dialog.Title = "" + err := request.IsValid() + assert.ErrorContains(t, err, "invalid dialog title") + }) + + t.Run("should fail on wrong subtype and long dialog title", func(t *testing.T) { + request := getBaseOpenDialogRequest() + request.Dialog.Elements[0].SubType = "wrong SubType" + request.Dialog.Title = "Very very long Dialog Name" + err := request.IsValid() + assert.ErrorContains(t, err, "invalid subtype") + assert.ErrorContains(t, err, "invalid dialog title") + t.Cleanup(func() { + request.Dialog.Elements[0].SubType = "" + request.Dialog.Title = "Some Title" + }) + }) + + t.Run("should fail on wrong dialog icon url", func(t *testing.T) { + request := getBaseOpenDialogRequest() + request.Dialog.IconURL = "wrong url" + err := request.IsValid() + assert.ErrorContains(t, err, "invalid icon url") + }) + + t.Run("should pass on empty dialog icon url", func(t *testing.T) { + request := getBaseOpenDialogRequest() + request.Dialog.IconURL = "" + err := request.IsValid() + assert.NoError(t, err) + }) + + t.Run("should fail on wrong minimal and maximal element length", func(t *testing.T) { + request := getBaseOpenDialogRequest() + request.Dialog.Elements[0].MinLength = 10 + request.Dialog.Elements[0].MaxLength = 9 + err := request.IsValid() + assert.ErrorContains(t, err, "field is not valid") + assert.ErrorContains(t, err, "min length should be less then max length") + }) + + t.Run("should fail on wrong element type", func(t *testing.T) { + request := getBaseOpenDialogRequest() + request.Dialog.Elements[0].Type = "wrong type" + err := request.IsValid() + assert.ErrorContains(t, err, "invalid element type") + }) + + t.Run("should fail on duplicate element name", func(t *testing.T) { + request := getBaseOpenDialogRequest() + request.Dialog.Elements = append(request.Dialog.Elements, DialogElement{ + DisplayName: "Radio element name", + Name: "element_name", + Type: "radio", + }) + err := request.IsValid() + assert.ErrorContains(t, err, "duplicate dialog element") + }) + + t.Run("should fail on wrong bool default value", func(t *testing.T) { + request := getBaseOpenDialogRequest() + request.Dialog.Elements = append(request.Dialog.Elements, DialogElement{ + DisplayName: "Bool element name", + Name: "bool_element_name", + Type: "bool", + Default: "wrong default", + }) + err := request.IsValid() + assert.ErrorContains(t, err, "invalid default of bool") + }) + + t.Run("should pass on bool default value", func(t *testing.T) { + request := getBaseOpenDialogRequest() + request.Dialog.Elements = append(request.Dialog.Elements, DialogElement{ + DisplayName: "Bool element name", + Name: "bool_element_name", + Type: "bool", + Default: "true", + }) + err := request.IsValid() + assert.NoError(t, err) + }) + + t.Run("should fail on wrong select datasource value", func(t *testing.T) { + request := getBaseOpenDialogRequest() + request.Dialog.Elements = append(request.Dialog.Elements, DialogElement{ + DisplayName: "Select element name", + Name: "select_element_name", + Type: "select", + DataSource: "wrong DataSource", + }) + err := request.IsValid() + assert.ErrorContains(t, err, "invalid data source") + }) + + t.Run("should fail on wrong radio default value", func(t *testing.T) { + request := getBaseOpenDialogRequest() + request.Dialog.Elements = append(request.Dialog.Elements, DialogElement{ + DisplayName: "Radio element name", + Name: "radio_element_name", + Type: "radio", + Default: "default", + Options: []*PostActionOptions{ + { + Text: "Text 1", + Value: "value 1", + }, + }, + }) + err := request.IsValid() + assert.ErrorContains(t, err, "default value \"default\" doesn't exist in options") + }) + t.Run("should pass radio default value", func(t *testing.T) { + request := getBaseOpenDialogRequest() + request.Dialog.Elements = append(request.Dialog.Elements, DialogElement{ + DisplayName: "Radio element name", + Name: "radio_element_name", + Type: "radio", + Default: "default", + Options: []*PostActionOptions{ + { + Text: "Text 1", + Value: "value 1", + }, + { + Text: "Text 2", + Value: "default", + }, + }, + }) + err := request.IsValid() + assert.NoError(t, err) + }) + + t.Run("should fail on too long text placeholder", func(t *testing.T) { + request := getBaseOpenDialogRequest() + request.Dialog.Elements[0].Placeholder = strings.Repeat("x", 151) + err := request.IsValid() + assert.ErrorContains(t, err, "Placeholder cannot be longer than 150 characters") + }) +}