From 718ec8e0618efcb0b5c47e526fc89e11fedbf1dd Mon Sep 17 00:00:00 2001 From: Benedikt Kulmann Date: Wed, 3 Jun 2020 10:50:28 +0200 Subject: [PATCH] Set up input validation for Identifiers on settings bundles Implemented first input validation steps by making static validation on identifiers in settings bundle requests. --- go.mod | 1 + go.sum | 5 ++ pkg/proto/v0/settings.pb.micro_test.go | 52 +++++++++++----- pkg/service/v0/service.go | 31 ++++++++-- pkg/service/v0/validator.go | 86 ++++++++++++++++++++++++++ 5 files changed, 155 insertions(+), 20 deletions(-) create mode 100644 pkg/service/v0/validator.go diff --git a/go.mod b/go.mod index 9303d85b6..c8cabc47a 100644 --- 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 diff --git a/go.sum b/go.sum index 2cdce11fe..0192b3d7a 100644 --- 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= diff --git a/pkg/proto/v0/settings.pb.micro_test.go b/pkg/proto/v0/settings.pb.micro_test.go index cef2b6fd0..913f536ea 100644 --- 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) diff --git a/pkg/service/v0/service.go b/pkg/service/v0/service.go index 06d93150a..631d784fe 100644 --- 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 } diff --git a/pkg/service/v0/validator.go b/pkg/service/v0/validator.go new file mode 100644 index 000000000..5f6b1398b --- /dev/null +++ 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...), + ) +} +