From edbbf2f01b2fc8421702569032de05d39566d5bb Mon Sep 17 00:00:00 2001 From: Denis Arh Date: Fri, 19 Nov 2021 14:12:58 +0100 Subject: [PATCH] Fix bool/false value sanitization Boolean value sanitization MUST return empty string for false boolean value to ensure consisten behaviour when empty or false value for field is send back and that field is read-only (via RBAC). --- compose/service/values/sanitizer.go | 3 +- compose/service/values/sanitizer_test.go | 4 +-- compose/service/values/shared.go | 5 +-- compose/service/values/validator.go | 2 +- tests/compose/record_test.go | 44 +++++++++++++++++++++--- 5 files changed, 47 insertions(+), 11 deletions(-) diff --git a/compose/service/values/sanitizer.go b/compose/service/values/sanitizer.go index 495ebff31..2dd081e4b 100644 --- a/compose/service/values/sanitizer.go +++ b/compose/service/values/sanitizer.go @@ -188,7 +188,8 @@ func sBool(v interface{}) string { } } - return strBoolFalse + // Returning empty string here to align false value with everything else + return strBoolFalseAlt } func sDatetime(v interface{}, onlyDate, onlyTime bool) string { diff --git a/compose/service/values/sanitizer_test.go b/compose/service/values/sanitizer_test.go index c447b6587..f959b5d90 100644 --- a/compose/service/values/sanitizer_test.go +++ b/compose/service/values/sanitizer_test.go @@ -68,13 +68,13 @@ func Test_sanitizer_Run(t *testing.T) { name: "booleans should be converted (false)", kind: "Bool", input: "false", - output: "0", + output: "", }, { name: "booleans should be converted (garbage)", kind: "Bool", input: "%%#)%)')$)'", - output: "0", + output: "", }, { name: "dates should be converted to ISO", diff --git a/compose/service/values/shared.go b/compose/service/values/shared.go index f045c2e0b..9b57fd58c 100644 --- a/compose/service/values/shared.go +++ b/compose/service/values/shared.go @@ -6,8 +6,9 @@ import ( ) const ( - strBoolTrue = "1" - strBoolFalse = "0" + strBoolTrue = "1" + strBoolFalse = "0" + strBoolFalseAlt = "" datetimeInternalFormatDate = "2006-01-02" datetimeIntenralFormatTime = "15:04:05" diff --git a/compose/service/values/validator.go b/compose/service/values/validator.go index d8efc6469..a88555cc5 100644 --- a/compose/service/values/validator.go +++ b/compose/service/values/validator.go @@ -250,7 +250,7 @@ func (vldtr validator) vBool(v *types.RecordValue, f *types.ModuleField, r *type return nil } - if v.Value != strBoolTrue && v.Value != strBoolFalse { + if v.Value != strBoolTrue && v.Value != strBoolFalse && v.Value != strBoolFalseAlt { return e2s(makeInvalidValueErr(f, v.Value)) } diff --git a/tests/compose/record_test.go b/tests/compose/record_test.go index b34385bfd..639239940 100644 --- a/tests/compose/record_test.go +++ b/tests/compose/record_test.go @@ -13,6 +13,7 @@ import ( "time" "github.com/cortezaproject/corteza-server/compose/service" + "github.com/cortezaproject/corteza-server/compose/service/values" "github.com/cortezaproject/corteza-server/compose/types" "github.com/cortezaproject/corteza-server/pkg/id" "github.com/cortezaproject/corteza-server/store" @@ -149,7 +150,11 @@ func (h helper) makeRecord(module *types.Module, rvs ...*types.RecordValue) *typ CreatedAt: time.Now(), ModuleID: module.ID, NamespaceID: module.NamespaceID, - Values: rvs, + + // We are directly storing the record values here, so ensure + // everything is formatted in the same manner as it would be + // when stored through the service + Values: values.Formatter().Run(module, rvs), } h.noError(store.CreateComposeRecord(context.Background(), service.DefaultStore, module, rec)) @@ -542,24 +547,53 @@ func TestRecordUpdate_forbiddenFields(t *testing.T) { module := h.repoMakeRecordModuleWithFields("record testing module", &types.ModuleField{Name: "f1", Kind: "String"}, &types.ModuleField{Name: "f2", Kind: "String"}, + + // we'll test all kinds of boolean inputs + &types.ModuleField{Name: "f-b-f-n", Kind: "Bool"}, + &types.ModuleField{Name: "f-b-f-m", Kind: "Bool"}, + &types.ModuleField{Name: "f-b-f-e", Kind: "Bool"}, + &types.ModuleField{Name: "f-b-f-z", Kind: "Bool"}, + &types.ModuleField{Name: "f-b-t-n", Kind: "Bool"}, + &types.ModuleField{Name: "f-b-t-m", Kind: "Bool"}, + &types.ModuleField{Name: "f-b-t-v", Kind: "Bool"}, ) record := h.makeRecord(module, &types.RecordValue{Name: "f1", Value: "f1.v1"}, &types.RecordValue{Name: "f2", Value: "f2.v1"}, + &types.RecordValue{Name: "f-b-f-n", Value: "0"}, // no-value + &types.RecordValue{Name: "f-b-f-e", Value: "0"}, // empty + &types.RecordValue{Name: "f-b-f-z", Value: "0"}, // zero + &types.RecordValue{Name: "f-b-t-n", Value: "1"}, // no-value + &types.RecordValue{Name: "f-b-t-v", Value: "1"}, // value ) - helpers.AllowMe(h, types.ModuleRbacResource(0, 0), "record.update") - helpers.DenyMe(h, module.Fields[1].RbacResource(), "record.value.update") + helpers.AllowMe(h, types.RecordRbacResource(0, 0, record.ID), "update") + helpers.AllowMe(h, module.Fields[0].RbacResource(), "record.value.update") + helpers.DenyMe(h, types.ModuleFieldRbacResource(0, record.ModuleID, 0), "record.value.update") h.apiInit(). Post(fmt.Sprintf("/namespace/%d/module/%d/record/%d", module.NamespaceID, module.ID, record.ID)). - JSON(fmt.Sprintf(`{"values": [{"name": "f1", "value": "f1.v1"}, {"name": "f2", "value": "f2.v1"}]}`)). + JSON(fmt.Sprintf(`{"values": [ + {"name": "f1", "value": "f1.v1"}, + {"name": "f2", "value": "f2.v1"}, + {"name": "f-b-f-n"}, + {"name": "f-b-f-e", "value": ""}, + {"name": "f-b-f-z", "value": "0"}, + {"name": "f-b-t-v", "value": "1"} + ]}`)). + Header("Accept", "application/json"). Expect(t). Status(http.StatusOK). + Assert(helpers.AssertNoErrors). End() r := h.lookupRecordByID(module, record.ID) h.a.NotNil(r) h.a.Equal("f2.v1", r.Values.FilterByName("f2")[0].Value) + h.a.Equal("", r.Values.FilterByName("f-b-f-n")[0].Value) + h.a.Equal("", r.Values.FilterByName("f-b-f-e")[0].Value) + h.a.Equal("", r.Values.FilterByName("f-b-f-z")[0].Value) + h.a.Equal("1", r.Values.FilterByName("f-b-t-n")[0].Value) + h.a.Equal("1", r.Values.FilterByName("f-b-t-v")[0].Value) } func TestRecordUpdate_refUnchanged(t *testing.T) { @@ -1063,7 +1097,7 @@ func TestRecordFieldModulePermissionCheck(t *testing.T) { Assert(jsonpath.Present(`$.response.values[? @.name=="email"]`)). End() - // Searching records should work as before but without read-protected fields + // Searching records should work as before but without read-protected fields h.apiInit(). Get(fmt.Sprintf("/namespace/%d/module/%d/record/", module.NamespaceID, module.ID)). Header("Accept", "application/json").