Merge pull request #66 from owncloud/fix-fetching-value-by-identifier

Fix fetching value by identifier and system scope values
owncloud · Aug 28, 2020 · 0df7512 · 0df7512
2 parents 8caf098 + b28f86f
commit 0df7512
Show file tree

Hide file tree

Showing 5 changed files with 40 additions and 3 deletions.
diff --git a/changelog/unreleased/fix-system-scope-values.md b/changelog/unreleased/fix-system-scope-values.md
@@ -0,0 +1,7 @@
+Bugfix: Fix loading and saving system scoped values
+
+We fixed loading and saving system scoped values. Those are now saved without an account uuid, so that the value
+can be loaded by other accounts as well.
+
+https://github.com/owncloud/ocis-settings/pull/66
+
diff --git a/changelog/unreleased/input-validation.md b/changelog/unreleased/input-validation.md
@@ -0,0 +1,6 @@
+Bugfix: Complete input validation
+
+There was one handler function without input validation. We implemented the input validation for `ValueService.ReadValueByUniqueIdentifiers`.
+
+https://github.com/owncloud/ocis-settings/pull/66
+
diff --git a/pkg/service/v0/service.go b/pkg/service/v0/service.go
@@ -216,8 +216,11 @@ func (g Service) GetValue(c context.Context, req *proto.GetValueRequest, res *pr
 }
 
 // GetValueByUniqueIdentifiers implements the ValueService interface
-func (g Service) GetValueByUniqueIdentifiers(ctx context.Context, in *proto.GetValueByUniqueIdentifiersRequest, res *proto.GetValueResponse) error {
-	v, err := g.manager.ReadValueByUniqueIdentifiers(in.AccountUuid, in.SettingId)
+func (g Service) GetValueByUniqueIdentifiers(ctx context.Context, req *proto.GetValueByUniqueIdentifiersRequest, res *proto.GetValueResponse) error {
+	if validationError := validateGetValueByUniqueIdentifiers(req); validationError != nil {
+		return merrors.BadRequest(g.id, "%s", validationError)
+	}
+	v, err := g.manager.ReadValueByUniqueIdentifiers(req.AccountUuid, req.SettingId)
 	if err != nil {
 		return merrors.NotFound(g.id, "%s", err)
 	}

diff --git a/pkg/service/v0/validator.go b/pkg/service/v0/validator.go
@@ -91,6 +91,14 @@ func validateGetValue(req *proto.GetValueRequest) error {
 	return validation.Validate(req.Id, is.UUID)
 }
 
+func validateGetValueByUniqueIdentifiers(req *proto.GetValueByUniqueIdentifiersRequest) error {
+	return validation.ValidateStruct(
+		req,
+		validation.Field(&req.SettingId, is.UUID),
+		validation.Field(&req.AccountUuid, requireAccountID...),
+	)
+}
+
 func validateListValues(req *proto.ListValuesRequest) error {
 	return validation.ValidateStruct(
 		req,

diff --git a/pkg/store/filesystem/values.go b/pkg/store/filesystem/values.go
@@ -2,6 +2,7 @@
 package store
 
 import (
+	"fmt"
 	"io/ioutil"
 	"path/filepath"
 
@@ -73,13 +74,18 @@ func (s Store) ReadValueByUniqueIdentifiers(accountUUID, settingID string) (*pro
 				return &proto.Value{}, nil
 			}
 
+			// if value saved without accountUUID, then it's a global value
+			if r.AccountUuid == "" && r.SettingId == settingID {
+				return &r, nil
+			}
+			// if value saved with accountUUID, then it's a user specific value
 			if r.AccountUuid == accountUUID && r.SettingId == settingID {
 				return &r, nil
 			}
 		}
 	}
 
-	return &proto.Value{}, nil
+	return nil, fmt.Errorf("could not read value by settingID=%v and accountID=%v", settingID, accountUUID)
 }
 
 // WriteValue writes the given value into a file within the dataPath
@@ -88,6 +94,13 @@ func (s Store) WriteValue(value *proto.Value) (*proto.Value, error) {
 	if value.Id == "" {
 		value.Id = uuid.Must(uuid.NewV4()).String()
 	}
+
+	// modify value depending on associated resource
+	if value.Resource.Type == proto.Resource_TYPE_SYSTEM {
+		value.AccountUuid = ""
+	}
+
+	// write the value
 	filePath := s.buildFilePathForValue(value.Id, true)
 	if err := s.writeRecordToFile(value, filePath); err != nil {
 		return nil, err