Set up input validation for Identifiers on settings bundles

Implemented first input validation steps by making static validation on identifiers in settings bundle requests.
2026-01-24 22:08:58 -05:00 · 2020-06-03 10:50:28 +02:00
parent d10179c1aa
commit 718ec8e061
5 changed files with 155 additions and 20 deletions
--- a/go.mod
+++ b/go.mod
@@ -9,6 +9,7 @@ require (
 	github.com/UnnoTed/fileb0x v1.1.4
 	github.com/go-chi/chi v4.1.0+incompatible
 	github.com/go-chi/render v1.0.1
+	github.com/go-ozzo/ozzo-validation/v4 v4.2.1
 	github.com/golang/protobuf v1.4.0
 	github.com/grpc-ecosystem/grpc-gateway v1.14.4
 	github.com/haya14busa/goverage v0.0.0-20180129164344-eec3514a20b5 // indirect
--- a/go.sum
+++ b/go.sum
@@ -74,6 +74,8 @@ github.com/armon/go-socks5 v0.0.0-20160902184237-e75332964ef5 h1:0CwZNZbxp69SHPd
 github.com/armon/go-socks5 v0.0.0-20160902184237-e75332964ef5/go.mod h1:wHh0iHkYZB8zMSxRWpUBQtwG5a7fFgvEO+odwuTv2gs=
 github.com/asaskevich/govalidator v0.0.0-20180720115003-f9ffefc3facf/go.mod h1:lB+ZfQJz7igIIfQNfa7Ml4HSf2uFQQRzpGGRXenZAgY=
 github.com/asaskevich/govalidator v0.0.0-20190424111038-f61b66f89f4a/go.mod h1:lB+ZfQJz7igIIfQNfa7Ml4HSf2uFQQRzpGGRXenZAgY=
+github.com/asaskevich/govalidator v0.0.0-20200108200545-475eaeb16496 h1:zV3ejI06GQ59hwDQAvmK1qxOQGB3WuVTRoY0okPTAv0=
+github.com/asaskevich/govalidator v0.0.0-20200108200545-475eaeb16496/go.mod h1:oGkLhpf+kjZl6xBf758TQhh5XrAeiJv/7FRz/2spLIg=
 github.com/ascarter/requestid v0.0.0-20170313220838-5b76ab3d4aee h1:3T/l+vMotQ7cDSLWNAn2Vg1SAQ3mdyLgBWWBitSS3uU=
 github.com/ascarter/requestid v0.0.0-20170313220838-5b76ab3d4aee/go.mod h1:u7Wtt4WATGGgae9mURNGQQqxAudPKrxfsbSDSGOso+g=
 github.com/aws/aws-sdk-go v1.23.0/go.mod h1:KmX6BPdI08NWTb3/sm4ZGu5ShLoqVDhKgpiN924inxo=
@@ -212,6 +214,9 @@ github.com/go-logfmt/logfmt v0.3.0/go.mod h1:Qt1PoO58o5twSAckw1HlFXLmHsOX5/0LbT9
 github.com/go-logfmt/logfmt v0.4.0/go.mod h1:3RMwSq7FuexP4Kalkev3ejPJsZTpXXBr9+V4qmtdjCk=
 github.com/go-openapi/errors v0.19.2/go.mod h1:qX0BLWsyaKfvhluLejVpVNwNRdXZhEbTA4kxxpKBC94=
 github.com/go-openapi/strfmt v0.19.2/go.mod h1:0yX7dbo8mKIvc3XSKp7MNfxw4JytCfCD6+bY1AVL9LU=
+github.com/go-ozzo/ozzo-validation v3.6.0+incompatible h1:msy24VGS42fKO9K1vLz82/GeYW1cILu7Nuuj1N3BBkE=
+github.com/go-ozzo/ozzo-validation/v4 v4.2.1 h1:XALUNshPYumA7UShB7iM3ZVlqIBn0jfwjqAMIoyE1N0=
+github.com/go-ozzo/ozzo-validation/v4 v4.2.1/go.mod h1:2NKgrcHl3z6cJs+3Oo940FPRiTzuqKbvfrL2RxCj6Ew=
 github.com/go-playground/locales v0.12.1/go.mod h1:IUMDtCfWo/w/mtMfIE/IG2K+Ey3ygWanZIBtBW0W2TM=
 github.com/go-playground/locales v0.13.0/go.mod h1:taPMhCMXrRLJO55olJkUXHZBHCxTMfnGwq/HNwmWNS8=
 github.com/go-playground/universal-translator v0.16.0/go.mod h1:1AnU7NaIRDWWzGEKwgtJRd2xk99HeFyHw3yid4rvQIY=
--- a/pkg/proto/v0/settings.pb.micro_test.go
+++ b/pkg/proto/v0/settings.pb.micro_test.go
@@ -3,6 +3,7 @@ package proto_test
 import (
 	"context"
 	"encoding/json"
+	"fmt"
 	"log"
 	"os"
 	"testing"
@@ -74,9 +75,13 @@ func TestSaveGetSettingsBundleWithNoSettings(t *testing.T) {
 			"सिम्प्ले-display-name",
 			"सिम्प्ले-extension-name",
 			"सिम्प्ले",
-			CustomError{},
+			CustomError{
+				ID:     "go.micro.client",
+				Code:   500,
+				Detail: "bundle_key: must be in a valid format; extension: must be in a valid format.",
+				Status: "Internal Server Error",
+			},
 		},
-		//https://github.com/owncloud/ocis-settings/issues/15
 		{
 			"bundle key with ../ in the name",
 			"../file-a-level-higher-up",
@@ -84,9 +89,13 @@ func TestSaveGetSettingsBundleWithNoSettings(t *testing.T) {
 			"simple-display-name",
 			"simple-extension-name",
 			"123e4567-e89b-12d3-a456-426652340000",
-			CustomError{},
+			CustomError{
+				ID:     "go.micro.client",
+				Code:   500,
+				Detail: "bundle_key: must be in a valid format.",
+				Status: "Internal Server Error",
+			},
 		},
-		//https://github.com/owncloud/ocis-settings/issues/15
 		{
 			"bundle key in the root directory",
 			"/tmp/file",
@@ -97,11 +106,10 @@ func TestSaveGetSettingsBundleWithNoSettings(t *testing.T) {
 			CustomError{
 				ID:     "go.micro.client",
 				Code:   500,
-				Detail: "open ocis-settings-store/bundles/simple-extension-name/tmp/file.json: no such file or directory",
+				Detail: "bundle_key: must be in a valid format.",
 				Status: "Internal Server Error",
 			},
 		},
-		//https://github.com/owncloud/ocis-settings/issues/15
 		{
 			"extension name with ../ in the name",
 			"simple-bundle-key",
@@ -109,9 +117,13 @@ func TestSaveGetSettingsBundleWithNoSettings(t *testing.T) {
 			"simple-display-name",
 			"../folder-a-level-higher-up",
 			"123e4567-e89b-12d3-a456-426652340000",
-			CustomError{},
+			CustomError{
+				ID:     "go.micro.client",
+				Code:   500,
+				Detail: "extension: must be in a valid format.",
+				Status: "Internal Server Error",
+			},
 		},
-		//https://github.com/owncloud/ocis-settings/issues/16
 		{
 			"extension name with \\ in the name",
 			"simple-bundle-key",
@@ -119,9 +131,13 @@ func TestSaveGetSettingsBundleWithNoSettings(t *testing.T) {
 			"simple-display-name",
 			"\\",
 			"123e4567-e89b-12d3-a456-426652340000",
-			CustomError{},
+			CustomError{
+				ID:     "go.micro.client",
+				Code:   500,
+				Detail: "extension: must be in a valid format.",
+				Status: "Internal Server Error",
+			},
 		},
-		//https://github.com/owncloud/ocis-settings/issues/16
 		{
 			"bundle key with \\ as the name",
 			"\\",
@@ -129,7 +145,12 @@ func TestSaveGetSettingsBundleWithNoSettings(t *testing.T) {
 			"simple-display-name",
 			"simple-extension-name",
 			"123e4567-e89b-12d3-a456-426652340000",
-			CustomError{},
+			CustomError{
+				ID:     "go.micro.client",
+				Code:   500,
+				Detail: "bundle_key: must be in a valid format.",
+				Status: "Internal Server Error",
+			},
 		},
 		{
 			"spaces in values",
@@ -150,7 +171,7 @@ func TestSaveGetSettingsBundleWithNoSettings(t *testing.T) {
 			CustomError{
 				ID:     "go.micro.client",
 				Code:   500,
-				Detail: "rpc error: code = InvalidArgument desc = Missing a required identifier attribute",
+				Detail: "bundle_key: cannot be blank.",
 				Status: "Internal Server Error",
 			},
 		},
@@ -164,12 +185,12 @@ func TestSaveGetSettingsBundleWithNoSettings(t *testing.T) {
 			CustomError{
 				ID:     "go.micro.client",
 				Code:   500,
-				Detail: "rpc error: code = InvalidArgument desc = Missing a required identifier attribute",
+				Detail: "extension: cannot be blank.",
 				Status: "Internal Server Error",
 			},
 		},
 		{
-			"setting key missing",
+			"setting key missing (omitted on bundles)",
 			"simple-bundle-key",
 			"",
 			"simple-display-name",
@@ -187,7 +208,7 @@ func TestSaveGetSettingsBundleWithNoSettings(t *testing.T) {
 			CustomError{},
 		},
 		{
-			"UUID missing",
+			"UUID missing (omitted on bundles)",
 			"simple-bundle-key",
 			"simple-key",
 			"simple-display-name",
@@ -218,6 +239,7 @@ func TestSaveGetSettingsBundleWithNoSettings(t *testing.T) {
 			cl := proto.NewBundleService("com.owncloud.api.settings", client)

 			cresponse, err := cl.SaveSettingsBundle(context.Background(), &createRequest)
+			fmt.Println(err)
 			if err != nil || (CustomError{} != testCase.expectedError) {
 				var errorData CustomError
 				_ = json.Unmarshal([]byte(err.Error()), &errorData)
--- a/pkg/service/v0/service.go
+++ b/pkg/service/v0/service.go
@@ -2,7 +2,6 @@ package svc

 import (
 	"context"
-
 	"github.com/owncloud/ocis-pkg/v2/middleware"
 	"github.com/owncloud/ocis-settings/pkg/config"
 	"github.com/owncloud/ocis-settings/pkg/proto/v0"
@@ -27,6 +26,9 @@ func NewService(cfg *config.Config) Service {
 // SaveSettingsBundle implements the BundleServiceHandler interface
 func (g Service) SaveSettingsBundle(c context.Context, req *proto.SaveSettingsBundleRequest, res *proto.SaveSettingsBundleResponse) error {
 	req.SettingsBundle.Identifier = getFailsafeIdentifier(c, req.SettingsBundle.Identifier)
+	if validationError := validateSaveSettingsBundle(req); validationError != nil {
+		return validationError
+	}
 	r, err := g.manager.WriteBundle(req.SettingsBundle)
 	if err != nil {
 		return err
@@ -37,7 +39,11 @@ func (g Service) SaveSettingsBundle(c context.Context, req *proto.SaveSettingsBu

 // GetSettingsBundle implements the BundleServiceHandler interface
 func (g Service) GetSettingsBundle(c context.Context, req *proto.GetSettingsBundleRequest, res *proto.GetSettingsBundleResponse) error {
-	r, err := g.manager.ReadBundle(getFailsafeIdentifier(c, req.Identifier))
+	req.Identifier = getFailsafeIdentifier(c, req.Identifier)
+	if validationError := validateGetSettingsBundle(req); validationError != nil {
+		return validationError
+	}
+	r, err := g.manager.ReadBundle(req.Identifier)
 	if err != nil {
 		return err
 	}
@@ -47,7 +53,11 @@ func (g Service) GetSettingsBundle(c context.Context, req *proto.GetSettingsBund

 // ListSettingsBundles implements the BundleServiceHandler interface
 func (g Service) ListSettingsBundles(c context.Context, req *proto.ListSettingsBundlesRequest, res *proto.ListSettingsBundlesResponse) error {
-	r, err := g.manager.ListBundles(getFailsafeIdentifier(c, req.Identifier))
+	req.Identifier = getFailsafeIdentifier(c, req.Identifier)
+	if validationError := validateListSettingsBundles(req); validationError != nil {
+		return validationError
+	}
+	r, err := g.manager.ListBundles(req.Identifier)
 	if err != nil {
 		return err
 	}
@@ -58,6 +68,9 @@ func (g Service) ListSettingsBundles(c context.Context, req *proto.ListSettingsB
 // SaveSettingsValue implements the ValueServiceHandler interface
 func (g Service) SaveSettingsValue(c context.Context, req *proto.SaveSettingsValueRequest, res *proto.SaveSettingsValueResponse) error {
 	req.SettingsValue.Identifier = getFailsafeIdentifier(c, req.SettingsValue.Identifier)
+	if validationError := validateSaveSettingsValue(req); validationError != nil {
+		return validationError
+	}
 	r, err := g.manager.WriteValue(req.SettingsValue)
 	if err != nil {
 		return err
@@ -68,7 +81,11 @@ func (g Service) SaveSettingsValue(c context.Context, req *proto.SaveSettingsVal

 // GetSettingsValue implements the ValueServiceHandler interface
 func (g Service) GetSettingsValue(c context.Context, req *proto.GetSettingsValueRequest, res *proto.GetSettingsValueResponse) error {
-	r, err := g.manager.ReadValue(getFailsafeIdentifier(c, req.Identifier))
+	req.Identifier = getFailsafeIdentifier(c, req.Identifier)
+	if validationError := validateGetSettingsValue(req); validationError != nil {
+		return validationError
+	}
+	r, err := g.manager.ReadValue(req.Identifier)
 	if err != nil {
 		return err
 	}
@@ -78,7 +95,11 @@ func (g Service) GetSettingsValue(c context.Context, req *proto.GetSettingsValue

 // ListSettingsValues implements the ValueServiceHandler interface
 func (g Service) ListSettingsValues(c context.Context, req *proto.ListSettingsValuesRequest, res *proto.ListSettingsValuesResponse) error {
-	r, err := g.manager.ListValues(getFailsafeIdentifier(c, req.Identifier))
+	req.Identifier = getFailsafeIdentifier(c, req.Identifier)
+	if validationError := validateListSettingsValues(req); validationError != nil {
+		return validationError
+	}
+	r, err := g.manager.ListValues(req.Identifier)
 	if err != nil {
 		return err
 	}
--- a/pkg/service/v0/validator.go
+++ b/pkg/service/v0/validator.go
@@ -0,0 +1,86 @@
+package svc
+
+import (
+	"regexp"
+
+	validation "github.com/go-ozzo/ozzo-validation/v4"
+	"github.com/go-ozzo/ozzo-validation/v4/is"
+	"github.com/owncloud/ocis-settings/pkg/proto/v0"
+)
+
+var (
+	regexForKeys = regexp.MustCompile("^[A-Za-z0-9\\-_]*$")
+	keyRule = []validation.Rule{
+		validation.Required,
+		validation.Match(regexForKeys),
+	}
+	settingKeyRule = []validation.Rule{
+		validation.Required,
+		validation.Match(regexForKeys),
+	}
+	accountUuidRule = []validation.Rule{
+		validation.Required,
+		is.UUID,
+	}
+)
+
+func validateSaveSettingsBundle(req *proto.SaveSettingsBundleRequest) error {
+	if err := validateBundleIdentifier(req.SettingsBundle.Identifier); err != nil {
+		return err
+	}
+	return nil
+}
+
+func validateGetSettingsBundle(req *proto.GetSettingsBundleRequest) error {
+	if err := validateBundleIdentifier(req.Identifier); err != nil {
+		return err
+	}
+	return nil
+}
+
+func validateListSettingsBundles(req *proto.ListSettingsBundlesRequest) error {
+	if err := validateBundleIdentifier(req.Identifier); err != nil {
+		return err
+	}
+	return nil
+}
+
+func validateSaveSettingsValue(req *proto.SaveSettingsValueRequest) error {
+	if err := validateValueIdentifier(req.SettingsValue.Identifier); err != nil {
+		return err
+	}
+	return nil
+}
+
+func validateGetSettingsValue(req *proto.GetSettingsValueRequest) error {
+	if err := validateValueIdentifier(req.Identifier); err != nil {
+		return err
+	}
+	return nil
+}
+
+func validateListSettingsValues(req *proto.ListSettingsValuesRequest) error {
+	if err := validateValueIdentifier(req.Identifier); err != nil {
+		return err
+	}
+	return nil
+}
+
+func validateBundleIdentifier(identifier *proto.Identifier) error {
+	return validation.ValidateStruct(
+		identifier,
+		validation.Field(&identifier.Extension, keyRule...),
+		validation.Field(&identifier.BundleKey, keyRule...),
+	)
+}
+
+func validateValueIdentifier(identifier *proto.Identifier) error {
+	return validation.ValidateStruct(
+		identifier,
+		validation.Field(&identifier.Extension, keyRule...),
+		validation.Field(&identifier.BundleKey, keyRule...),
+		validation.Field(&identifier.SettingKey, settingKeyRule...),
+		validation.Field(&identifier.AccountUuid, accountUuidRule...),
+	)
+}
+