Add IsValid method to *Manifest struct (#13609)

app/plugin_install.go

@@ -50,7 +50,6 @@ import (
 
 	"github.com/mattermost/mattermost-server/v5/mlog"
 	"github.com/mattermost/mattermost-server/v5/model"
-	"github.com/mattermost/mattermost-server/v5/plugin"
 	"github.com/mattermost/mattermost-server/v5/services/filesstore"
 	"github.com/mattermost/mattermost-server/v5/utils"
 )
@@ -290,8 +289,8 @@ func extractPlugin(pluginFile io.ReadSeeker, extractDir string) (*model.Manifest
 		return nil, "", model.NewAppError("extractPlugin", "app.plugin.manifest.app_error", nil, err.Error(), http.StatusBadRequest)
 	}
 
-	if !plugin.IsValidId(manifest.Id) {
-		return nil, "", model.NewAppError("extractPlugin", "app.plugin.invalid_id.app_error", map[string]interface{}{"Min": plugin.MinIdLength, "Max": plugin.MaxIdLength, "Regex": plugin.ValidIdRegex}, "", http.StatusBadRequest)
+	if !model.IsValidPluginId(manifest.Id) {
+		return nil, "", model.NewAppError("installPluginLocally", "app.plugin.invalid_id.app_error", map[string]interface{}{"Min": model.MinIdLength, "Max": model.MaxIdLength, "Regex": model.ValidIdRegex}, "", http.StatusBadRequest)
 	}
 
 	return manifest, extractDir, nil

model/manifest.go

@@ -5,7 +5,6 @@ package model
 
 import (
 	"encoding/json"
-	"errors"
 	"fmt"
 	"io"
 	"io/ioutil"
@@ -14,6 +13,7 @@ import (
 	"strings"
 
 	"github.com/blang/semver"
+	"github.com/pkg/errors"
 	"gopkg.in/yaml.v2"
 )
 
@@ -25,6 +25,19 @@ type PluginOption struct {
 	Value string `json:"value" yaml:"value"`
 }
 
+type PluginSettingType int
+
+const (
+	Bool PluginSettingType = iota
+	Dropdown
+	Generated
+	Radio
+	Text
+	LongText
+	Username
+	Custom
+)
+
 type PluginSetting struct {
 	// The key that the setting will be assigned to in the configuration file.
 	Key string `json:"key" yaml:"key"`
@@ -304,6 +317,103 @@ func (m *Manifest) MeetMinServerVersion(serverVersion string) (bool, error) {
 	return true, nil
 }
 
+func (m *Manifest) IsValid() error {
+	if !IsValidPluginId(m.Id) {
+		return errors.New("invalid plugin ID")
+	}
+
+	if m.HomepageURL == "" || !IsValidHttpUrl(m.HomepageURL) {
+		return errors.New("invalid HomepageURL")
+	}
+
+	if m.SupportURL == "" || !IsValidHttpUrl(m.SupportURL) {
+		return errors.New("invalid SupportURL")
+	}
+
+	_, err := semver.Parse(m.Version)
+	if err != nil {
+		return errors.Wrap(err, "failed to parse Version")
+	}
+
+	_, err2 := semver.Parse(m.MinServerVersion)
+	if err2 != nil {
+		return errors.Wrap(err2, "failed to parse MinServerVersion")
+	}
+
+	if m.SettingsSchema != nil {
+		err3 := m.SettingsSchema.isValidSchema()
+		if err3 != nil {
+			return errors.Wrap(err3, "invalid settings schema")
+		}
+	}
+
+	return nil
+}
+
+func (s *PluginSettingsSchema) isValidSchema() error {
+	for _, setting := range s.Settings {
+		err := setting.isValid()
+		if err != nil {
+			return err
+		}
+	}
+
+	return nil
+}
+
+func (s *PluginSetting) isValid() error {
+	pluginSettingType, err := convertTypeToPluginSettingType(s.Type)
+	if err != nil {
+		return err
+	}
+
+	if s.RegenerateHelpText != "" && pluginSettingType != Generated {
+		return errors.New("should not set RegenerateHelpText for setting type that is not generated")
+	}
+
+	if s.Placeholder != "" && !(pluginSettingType == Text || pluginSettingType == Generated || pluginSettingType == Username) {
+		return errors.New("should not set Placeholder for setting type not in text, generated or username")
+	}
+
+	if s.Options != nil {
+		if pluginSettingType != Radio && pluginSettingType != Dropdown {
+			return errors.New("should not set Options for setting type not in radio or dropdown")
+		}
+
+		for _, option := range s.Options {
+			if option.DisplayName == "" || option.Value == "" {
+				return errors.New("should not have empty Displayname or Value for any option")
+			}
+		}
+	}
+
+	return nil
+}
+
+func convertTypeToPluginSettingType(t string) (PluginSettingType, error) {
+	var settingType PluginSettingType
+	switch t {
+	case "bool":
+		return Bool, nil
+	case "dropdown":
+		return Dropdown, nil
+	case "generated":
+		return Generated, nil
+	case "radio":
+		return Radio, nil
+	case "text":
+		return Text, nil
+	case "longtext":
+		return LongText, nil
+	case "username":
+		return Username, nil
+	case "custom":
+		return Custom, nil
+	default:
+		return settingType, errors.New("invalid setting type: " + t)
+	}
+}
+
 // FindManifest will find and parse the manifest in a given directory.
 //
 // In all cases other than a does-not-exist error, path is set to the path of the manifest file that was

model/manifest_test.go

@@ -17,6 +17,162 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
+func TestIsValid(t *testing.T) {
+	testCases := []struct {
+		Title       string
+		manifest    *Manifest
+		ExpectError bool
+	}{
+		{"Invalid Id", &Manifest{Id: "some id"}, true},
+		{"Invalid homePageURL", &Manifest{Id: "com.company.test", HomepageURL: "some url"}, true},
+		{"Invalid supportURL", &Manifest{Id: "com.company.test", HomepageURL: "http://someurl.com", SupportURL: "some url"}, true},
+		{"Invalid version", &Manifest{Id: "com.company.test", HomepageURL: "http://someurl.com", SupportURL: "http://someotherurl.com", Version: "version"}, true},
+		{"Invalid min version", &Manifest{Id: "com.company.test", HomepageURL: "http://someurl.com", SupportURL: "http://someotherurl.com", Version: "5.10.0", MinServerVersion: "version"}, true},
+		{"SettingSchema error", &Manifest{Id: "com.company.test", HomepageURL: "http://someurl.com", SupportURL: "http://someotherurl.com", Version: "5.10.0", MinServerVersion: "5.10.8", SettingsSchema: &PluginSettingsSchema{
+			Settings: []*PluginSetting{{Type: "Invalid"}},
+		}}, true},
+		{"Happy case", &Manifest{
+			Id:               "com.company.test",
+			Name:             "thename",
+			Description:      "thedescription",
+			HomepageURL:      "http://someurl.com",
+			SupportURL:       "http://someotherurl.com",
+			Version:          "0.0.1",
+			MinServerVersion: "5.6.0",
+			Server: &ManifestServer{
+				Executable: "theexecutable",
+			},
+			Webapp: &ManifestWebapp{
+				BundlePath: "thebundlepath",
+				BundleHash: []byte{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15},
+			},
+			SettingsSchema: &PluginSettingsSchema{
+				Header: "theheadertext",
+				Footer: "thefootertext",
+				Settings: []*PluginSetting{
+					{
+						Key:         "thesetting",
+						DisplayName: "thedisplayname",
+						Type:        "dropdown",
+						HelpText:    "thehelptext",
+						Options: []*PluginOption{
+							{
+								DisplayName: "theoptiondisplayname",
+								Value:       "thevalue",
+							},
+						},
+						Default: "thedefault",
+					},
+				},
+			},
+		}, false},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.Title, func(t *testing.T) {
+			err := tc.manifest.IsValid()
+			if tc.ExpectError {
+				assert.Error(t, err)
+			} else {
+				assert.NoError(t, err)
+			}
+		})
+	}
+}
+
+func TestIsValidSettingsSchema(t *testing.T) {
+	testCases := []struct {
+		Title          string
+		settingsSchema *PluginSettingsSchema
+		ExpectError    bool
+	}{
+		{"Invalid Setting", &PluginSettingsSchema{Settings: []*PluginSetting{{Type: "invalid"}}}, true},
+		{"Happy case", &PluginSettingsSchema{Settings: []*PluginSetting{{Type: "text"}}}, false},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.Title, func(t *testing.T) {
+			err := tc.settingsSchema.isValidSchema()
+			if tc.ExpectError {
+				assert.Error(t, err)
+			} else {
+				assert.NoError(t, err)
+			}
+		})
+	}
+}
+
+func TestSettingIsValid(t *testing.T) {
+	testCases := []struct {
+		Title       string
+		setting     *PluginSetting
+		ExpectError bool
+	}{
+		{"Invalid setting type", &PluginSetting{Type: "invalid"}, true},
+		{"RegenerateHelpText error", &PluginSetting{Type: "text", RegenerateHelpText: "some text"}, true},
+		{"Placeholder error", &PluginSetting{Type: "bool", Placeholder: "some text"}, true},
+		{"Nil Options", &PluginSetting{Type: "bool"}, false},
+		{"Options error", &PluginSetting{Type: "generated", Options: []*PluginOption{}}, true},
+		{"Options displayName error", &PluginSetting{Type: "radio", Options: []*PluginOption{
+			{
+				Value: "some value",
+			},
+		}}, true},
+		{"Options value error", &PluginSetting{Type: "radio", Options: []*PluginOption{
+			{
+				DisplayName: "some name",
+			},
+		}}, true},
+		{"Happy case", &PluginSetting{Type: "radio", Options: []*PluginOption{
+			{
+				DisplayName: "Name",
+				Value:       "value",
+			},
+		}}, false},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.Title, func(t *testing.T) {
+			err := tc.setting.isValid()
+			if tc.ExpectError {
+				assert.Error(t, err)
+			} else {
+				assert.NoError(t, err)
+			}
+		})
+	}
+}
+
+func TestConvertTypeToPluginSettingType(t *testing.T) {
+	testCases := []struct {
+		Title               string
+		Type                string
+		ExpectedSettingType PluginSettingType
+		ExpectError         bool
+	}{
+		{"bool", "bool", Bool, false},
+		{"dropdown", "dropdown", Dropdown, false},
+		{"generated", "generated", Generated, false},
+		{"radio", "radio", Radio, false},
+		{"text", "text", Text, false},
+		{"longtext", "longtext", LongText, false},
+		{"username", "username", Username, false},
+		{"custom", "custom", Custom, false},
+		{"invalid", "invalid", Bool, true},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.Title, func(t *testing.T) {
+			settingType, err := convertTypeToPluginSettingType(tc.Type)
+			if !tc.ExpectError {
+				assert.Equal(t, settingType, tc.ExpectedSettingType)
+			} else {
+				assert.Error(t, err)
+			}
+		})
+	}
+}
+
 func TestFindManifest(t *testing.T) {
 	for _, tc := range []struct {
 		Filename       string

model/plugin_valid.go

@@ -1,7 +1,7 @@
 // Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
 // See LICENSE.txt for license information.
 
-package plugin
+package model
 
 import (
 	"regexp"
@@ -22,11 +22,11 @@ func init() {
 	validId = regexp.MustCompile(ValidIdRegex)
 }
 
-// IsValidId verifies that the plugin id has a minimum length of 3, maximum length of 190, and
+// IsValidPluginId verifies that the plugin id has a minimum length of 3, maximum length of 190, and
 // contains only alphanumeric characters, dashes, underscores and periods.
 //
 // These constraints are necessary since the plugin id is used as part of a filesystem path.
-func IsValidId(id string) bool {
+func IsValidPluginId(id string) bool {
 	if utf8.RuneCountInString(id) < MinIdLength {
 		return false
 	}

model/plugin_valid_test.go

@@ -1,17 +1,15 @@
 // Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved.
 // See LICENSE.txt for license information.
 
-package plugin_test
+package model
 
 import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
-
-	"github.com/mattermost/mattermost-server/v5/plugin"
 )
 
-func TestIsValid(t *testing.T) {
+func TestIsValidPluginId(t *testing.T) {
 	t.Parallel()
 
 	testCases := map[string]bool{
@@ -29,7 +27,7 @@ func TestIsValid(t *testing.T) {
 
 	for id, valid := range testCases {
 		t.Run(id, func(t *testing.T) {
-			assert.Equal(t, valid, plugin.IsValidId(id))
+			assert.Equal(t, valid, IsValidPluginId(id))
 		})
 	}
 }