From 7ce5a13bca41bc837e38fb61ecaea7fbd853850b Mon Sep 17 00:00:00 2001 From: Denis Arh Date: Fri, 21 Feb 2020 19:26:39 +0100 Subject: [PATCH] Refactor and improve record value sanitization & validation --- compose/repository/record.go | 13 + compose/rest/record.go | 35 ++- compose/service/event/events.yaml | 2 + compose/service/event/record.gen.go | 105 +++++--- compose/service/record.go | 358 +++++++++++++++++----------- compose/service/record_test.go | 50 ++-- compose/service/values/sanitizer.go | 209 ++++++++++++++++ compose/service/values/validator.go | 207 ++++++++++++++++ compose/types/module_field.go | 11 +- compose/types/record_value.go | 84 +++++++ compose/types/record_value_test.go | 43 ++++ compose/types/validated.go | 25 ++ system/service/sink.go | 2 - 13 files changed, 936 insertions(+), 208 deletions(-) create mode 100644 compose/service/values/sanitizer.go create mode 100644 compose/service/values/validator.go create mode 100644 compose/types/validated.go diff --git a/compose/repository/record.go b/compose/repository/record.go index 55934d4de..14d004b1f 100644 --- a/compose/repository/record.go +++ b/compose/repository/record.go @@ -29,6 +29,7 @@ type ( Update(record *types.Record) (*types.Record, error) Delete(record *types.Record) error + RefValueLookup(moduleID uint64, field string, ref uint64) (recordID uint64, err error) LoadValues(fieldNames []string, IDs []uint64) (rvs types.RecordValueSet, err error) DeleteValues(record *types.Record) error UpdateValues(recordID uint64, rvs types.RecordValueSet) (err error) @@ -330,7 +331,19 @@ func (r record) PartialUpdateValues(rvs ...*types.RecordValue) (err error) { }) return errors.Wrap(err, "could not replace record values") +} +func (r record) RefValueLookup(moduleID uint64, field string, ref uint64) (recordID uint64, err error) { + var sql = "SELECT record_id" + + " FROM compose_record AS r INNER JOIN compose_record_value AS v " + + " WHERE rel_module = ? " + + " AND v.name = ? " + + " AND v.ref = ? " + + " AND r.deleted_at IS NULL " + + " AND v.deleted_at IS NULL " + + " LIMIT 1" + + return recordID, r.db().Get(recordID, sql, moduleID, field, ref) } func (r record) LoadValues(fieldNames []string, IDs []uint64) (rvs types.RecordValueSet, err error) { diff --git a/compose/rest/record.go b/compose/rest/record.go index 4a049aeb8..7dde0a08b 100644 --- a/compose/rest/record.go +++ b/compose/rest/record.go @@ -5,6 +5,7 @@ import ( "encoding/csv" "encoding/json" "fmt" + "github.com/cortezaproject/corteza-server/compose/service/values" "github.com/cortezaproject/corteza-server/pkg/payload" "net/http" "strings" @@ -128,6 +129,10 @@ func (ctrl *Record) Create(ctx context.Context, r *request.RecordCreate) (interf Values: r.Values, }) + if rve, is := err.(*types.RecordValueErrorSet); is && !rve.IsValid() { + return ctrl.handleValidationError(rve), nil + } + return ctrl.makePayload(ctx, m, record, err) } @@ -148,6 +153,10 @@ func (ctrl *Record) Update(ctx context.Context, r *request.RecordUpdate) (interf Values: r.Values, }) + if rve, is := err.(*types.RecordValueErrorSet); is && !rve.IsValid() { + return ctrl.handleValidationError(rve), nil + } + return ctrl.makePayload(ctx, m, record, err) } @@ -410,12 +419,13 @@ func (ctrl *Record) TriggerScript(ctx context.Context, r *request.RecordTriggerS } record = oldRecord - record.Values = r.Values + record.Values = values.Sanitizer().Run(module, r.Values) + validated := values.Validator().Run(module, record) err = corredor.Service().ExecOnManual( ctx, r.Script, - event.RecordOnManual(record, oldRecord, module, namespace), + event.RecordOnManual(record, oldRecord, module, namespace, validated), ) // Script can return modified record and we'll pass it on to the caller @@ -448,3 +458,24 @@ func (ctrl Record) makeFilterPayload(ctx context.Context, m *types.Module, rr ty return modp, nil } + +// Special care for record validation errors +// +// We need to return a bit different format of response +// with all details that were collected through validation +func (ctrl Record) handleValidationError(rve *types.RecordValueErrorSet) interface{} { + return func(w http.ResponseWriter, _ *http.Request) { + rval := struct { + Error struct { + Message string `json:"message"` + Details []types.RecordValueError `json:"details,omitempty"` + } `json:"error"` + }{} + + rval.Error.Message = rve.Error() + rval.Error.Details = rve.Set + + w.Header().Set("Content-Type", "application/json") + json.NewEncoder(w).Encode(rval) + } +} diff --git a/compose/service/event/events.yaml b/compose/service/event/events.yaml index ea6ba42ea..1dc6c87f3 100644 --- a/compose/service/event/events.yaml +++ b/compose/service/event/events.yaml @@ -52,3 +52,5 @@ compose:record: - name: 'namespace' type: '*types.Namespace' immutable: true + - name: 'recordValueErrors' + type: '*types.RecordValueErrorSet' diff --git a/compose/service/event/record.gen.go b/compose/service/event/record.gen.go index e20fb1114..50f976002 100644 --- a/compose/service/event/record.gen.go +++ b/compose/service/event/record.gen.go @@ -22,11 +22,12 @@ type ( // // This type is auto-generated. recordBase struct { - record *types.Record - oldRecord *types.Record - module *types.Module - namespace *types.Namespace - invoker auth.Identifiable + record *types.Record + oldRecord *types.Record + module *types.Module + namespace *types.Namespace + recordValueErrors *types.RecordValueErrorSet + invoker auth.Identifiable } // recordOnManual @@ -143,13 +144,15 @@ func RecordOnManual( argOldRecord *types.Record, argModule *types.Module, argNamespace *types.Namespace, + argRecordValueErrors *types.RecordValueErrorSet, ) *recordOnManual { return &recordOnManual{ recordBase: &recordBase{ - record: argRecord, - oldRecord: argOldRecord, - module: argModule, - namespace: argNamespace, + record: argRecord, + oldRecord: argOldRecord, + module: argModule, + namespace: argNamespace, + recordValueErrors: argRecordValueErrors, }, } } @@ -162,13 +165,15 @@ func RecordBeforeCreate( argOldRecord *types.Record, argModule *types.Module, argNamespace *types.Namespace, + argRecordValueErrors *types.RecordValueErrorSet, ) *recordBeforeCreate { return &recordBeforeCreate{ recordBase: &recordBase{ - record: argRecord, - oldRecord: argOldRecord, - module: argModule, - namespace: argNamespace, + record: argRecord, + oldRecord: argOldRecord, + module: argModule, + namespace: argNamespace, + recordValueErrors: argRecordValueErrors, }, } } @@ -181,13 +186,15 @@ func RecordBeforeUpdate( argOldRecord *types.Record, argModule *types.Module, argNamespace *types.Namespace, + argRecordValueErrors *types.RecordValueErrorSet, ) *recordBeforeUpdate { return &recordBeforeUpdate{ recordBase: &recordBase{ - record: argRecord, - oldRecord: argOldRecord, - module: argModule, - namespace: argNamespace, + record: argRecord, + oldRecord: argOldRecord, + module: argModule, + namespace: argNamespace, + recordValueErrors: argRecordValueErrors, }, } } @@ -200,13 +207,15 @@ func RecordBeforeDelete( argOldRecord *types.Record, argModule *types.Module, argNamespace *types.Namespace, + argRecordValueErrors *types.RecordValueErrorSet, ) *recordBeforeDelete { return &recordBeforeDelete{ recordBase: &recordBase{ - record: argRecord, - oldRecord: argOldRecord, - module: argModule, - namespace: argNamespace, + record: argRecord, + oldRecord: argOldRecord, + module: argModule, + namespace: argNamespace, + recordValueErrors: argRecordValueErrors, }, } } @@ -219,13 +228,15 @@ func RecordAfterCreate( argOldRecord *types.Record, argModule *types.Module, argNamespace *types.Namespace, + argRecordValueErrors *types.RecordValueErrorSet, ) *recordAfterCreate { return &recordAfterCreate{ recordBase: &recordBase{ - record: argRecord, - oldRecord: argOldRecord, - module: argModule, - namespace: argNamespace, + record: argRecord, + oldRecord: argOldRecord, + module: argModule, + namespace: argNamespace, + recordValueErrors: argRecordValueErrors, }, } } @@ -238,13 +249,15 @@ func RecordAfterUpdate( argOldRecord *types.Record, argModule *types.Module, argNamespace *types.Namespace, + argRecordValueErrors *types.RecordValueErrorSet, ) *recordAfterUpdate { return &recordAfterUpdate{ recordBase: &recordBase{ - record: argRecord, - oldRecord: argOldRecord, - module: argModule, - namespace: argNamespace, + record: argRecord, + oldRecord: argOldRecord, + module: argModule, + namespace: argNamespace, + recordValueErrors: argRecordValueErrors, }, } } @@ -257,13 +270,15 @@ func RecordAfterDelete( argOldRecord *types.Record, argModule *types.Module, argNamespace *types.Namespace, + argRecordValueErrors *types.RecordValueErrorSet, ) *recordAfterDelete { return &recordAfterDelete{ recordBase: &recordBase{ - record: argRecord, - oldRecord: argOldRecord, - module: argModule, - namespace: argNamespace, + record: argRecord, + oldRecord: argOldRecord, + module: argModule, + namespace: argNamespace, + recordValueErrors: argRecordValueErrors, }, } } @@ -303,6 +318,20 @@ func (res recordBase) Namespace() *types.Namespace { return res.namespace } +// SetRecordValueErrors sets new recordValueErrors value +// +// This function is auto-generated. +func (res *recordBase) SetRecordValueErrors(argRecordValueErrors *types.RecordValueErrorSet) { + res.recordValueErrors = argRecordValueErrors +} + +// RecordValueErrors returns recordValueErrors +// +// This function is auto-generated. +func (res recordBase) RecordValueErrors() *types.RecordValueErrorSet { + return res.recordValueErrors +} + // SetInvoker sets new invoker value // // This function is auto-generated. @@ -337,6 +366,10 @@ func (res recordBase) Encode() (args map[string][]byte, err error) { return nil, err } + if args["recordValueErrors"], err = json.Marshal(res.recordValueErrors); err != nil { + return nil, err + } + if args["invoker"], err = json.Marshal(res.invoker); err != nil { return nil, err } @@ -358,6 +391,12 @@ func (res *recordBase) Decode(results map[string][]byte) (err error) { } } + if r, ok := results["recordValueErrors"]; ok && len(results) == 1 { + if err = json.Unmarshal(r, res.recordValueErrors); err != nil { + return + } + } + if r, ok := results["invoker"]; ok && len(results) == 1 { if err = json.Unmarshal(r, res.invoker); err != nil { return diff --git a/compose/service/record.go b/compose/service/record.go index 6cbf9056c..2d392cdda 100644 --- a/compose/service/record.go +++ b/compose/service/record.go @@ -3,6 +3,7 @@ package service import ( "context" "fmt" + "github.com/cortezaproject/corteza-server/compose/service/values" "regexp" "strconv" "time" @@ -38,6 +39,20 @@ type ( recordRepo repository.RecordRepository moduleRepo repository.ModuleRepository nsRepo repository.NamespaceRepository + + sanitizer recordValuesSanitizer + validator recordValuesValidator + } + + recordValuesSanitizer interface { + Run(*types.Module, types.RecordValueSet) types.RecordValueSet + } + + recordValuesValidator interface { + Run(*types.Module, *types.Record) *types.RecordValueErrorSet + UniqueChecker(fn values.UniqueChecker) + RecordRefChecker(fn values.ReferenceChecker) + UserRefChecker(fn values.ReferenceChecker) } recordAccessController interface { @@ -113,6 +128,31 @@ func Record() RecordService { func (svc record) With(ctx context.Context) RecordService { db := repository.DB(ctx) + // Initialize validator and setup all checkers it needs + validator := values.Validator() + + validator.UniqueChecker(func(m *types.Module, f *types.ModuleField, v *types.RecordValue) (uint64, error) { + if v.Ref == 0 { + return 0, nil + } + + return repository.Record(ctx, db).RefValueLookup(m.ID, f.Name, v.Ref) + }) + + validator.RecordRefChecker(func(m *types.Module, f *types.ModuleField, v *types.RecordValue) (bool, error) { + if v.Ref == 0 { + return false, nil + } + + r, err := repository.Record(ctx, db).FindByID(m.NamespaceID, v.Ref) + return r != nil, err + }) + + validator.UserRefChecker(func(m *types.Module, f *types.ModuleField, v *types.RecordValue) (bool, error) { + // @todo cross service check + return true, nil + }) + return &record{ db: db, ctx: ctx, @@ -124,6 +164,9 @@ func (svc record) With(ctx context.Context) RecordService { recordRepo: repository.Record(ctx, db), moduleRepo: repository.Module(ctx, db), nsRepo: repository.Namespace(ctx, db), + + sanitizer: values.Sanitizer(), + validator: validator, } } @@ -282,46 +325,77 @@ func (svc record) Export(filter types.RecordFilter, enc Encoder) error { return set.Walk(enc.Record) } -func (svc record) Create(new *types.Record) (r *types.Record, err error) { - ns, m, r, err := svc.loadCombo(new.NamespaceID, new.ModuleID, 0) - if err != nil { - return - } +func (svc record) Create(new *types.Record) (rec *types.Record, err error) { + return rec, svc.db.Transaction(func() (err error) { + var ( + ns *types.Namespace + m *types.Module + ) - if !svc.ac.CanCreateRecord(svc.ctx, m) { - return nil, ErrNoCreatePermissions.withStack() - } + ns, m, _, err = svc.loadCombo(new.NamespaceID, new.ModuleID, 0) + if err != nil { + return + } - creatorID := auth.GetIdentityFromContext(svc.ctx).Identity() - r = &types.Record{ - ModuleID: new.ModuleID, - NamespaceID: new.NamespaceID, + if !svc.ac.CanCreateRecord(svc.ctx, m) { + return ErrNoCreatePermissions.withStack() + } - CreatedBy: creatorID, - OwnedBy: creatorID, + if err = svc.generalValueSetValidation(m, new.Values); err != nil { + return + } - CreatedAt: time.Now(), - } + // First sanitization + // + // Before values are sent to automation scripts (if any) + // we need to make sure it does not get sanitized data + creatorID := auth.GetIdentityFromContext(svc.ctx).Identity() - if err = svc.eventbus.WaitFor(svc.ctx, event.RecordBeforeCreate(new, nil, m, ns)); err != nil { - return - } + new.OwnedBy = creatorID + new.CreatedBy = creatorID + new.CreatedAt = time.Now() + new.UpdatedAt = nil + new.UpdatedBy = 0 + new.DeletedAt = nil + new.UpdatedBy = 0 + new.Values = svc.sanitizer.Run(m, new.Values) - if err = svc.setDefaultValues(m, new); err != nil { - return - } + // Before values are stored, we have to validate them + rve := values.Validator().Run(m, new) + if !rve.IsValid() { + return rve + } - if err = svc.copyChanges(m, new, r); err != nil { - return - } + if err = svc.eventbus.WaitFor(svc.ctx, event.RecordBeforeCreate(new, nil, m, ns, rve)); err != nil { + return + } else if !rve.IsValid() { + return rve + } - // We do not know what happened in the before-create script, - // so we must sanitize values again before we store it - if r.Values, err = svc.sanitizeValues(m, r.Values); err != nil { - return - } + // Assign defaults (only on missing values) + new.Values = svc.setDefaultValues(m, new.Values) + + if new.OwnedBy == 0 { + // Allow ownership change + new.OwnedBy = creatorID + } + + // Reconstruct the final record, and re-sanitize everything + // we could use new, but we do not trust before-create scripts + r := &types.Record{ + ModuleID: new.ModuleID, + NamespaceID: new.NamespaceID, + CreatedBy: creatorID, + OwnedBy: new.OwnedBy, + CreatedAt: time.Now(), + Values: svc.sanitizer.Run(m, new.Values), + } + + // Before values are stored, we have to validate them + if rve = svc.validator.Run(m, r); !rve.IsValid() { + return rve + } - return r, svc.db.Transaction(func() (err error) { if r, err = svc.recordRepo.Create(r); err != nil { return } @@ -330,52 +404,84 @@ func (svc record) Create(new *types.Record) (r *types.Record, err error) { return } - defer svc.eventbus.Dispatch(svc.ctx, event.RecordAfterCreate(r, nil, m, ns)) + // At this point we can return the value + rec = r + defer svc.eventbus.Dispatch(svc.ctx, event.RecordAfterCreate(r, nil, m, ns, nil)) return }) } -func (svc record) Update(upd *types.Record) (r *types.Record, err error) { - if upd.ID == 0 { - return nil, ErrInvalidID.withStack() - } +func (svc record) Update(upd *types.Record) (rec *types.Record, err error) { + return rec, svc.db.Transaction(func() (err error) { + if upd.ID == 0 { + return ErrInvalidID.withStack() + } - ns, m, r, err := svc.loadCombo(upd.NamespaceID, upd.ModuleID, upd.ID) - if err != nil { - return - } + ns, m, r, err := svc.loadCombo(upd.NamespaceID, upd.ModuleID, upd.ID) + if err != nil { + return + } - if !svc.ac.CanUpdateRecord(svc.ctx, m) { - return nil, ErrNoUpdatePermissions.withStack() - } + if err = svc.generalValueSetValidation(m, upd.Values); err != nil { + return + } - // Test if stale (update has an older copy) - if isStale(upd.UpdatedAt, r.UpdatedAt, r.CreatedAt) { - return nil, ErrStaleData.withStack() - } + if !svc.ac.CanUpdateRecord(svc.ctx, m) { + return ErrNoUpdatePermissions.withStack() + } - // Preload old record values so we can send it together with event - if err = svc.preloadValues(m, r); err != nil { - return nil, err - } + // Test if stale (update has an older version of data) + if isStale(upd.UpdatedAt, r.UpdatedAt, r.CreatedAt) { + return ErrStaleData.withStack() + } - if err = svc.eventbus.WaitFor(svc.ctx, event.RecordBeforeUpdate(upd, r, m, ns)); err != nil { - return - } + // First sanitization + // + // Before values are merged with existing data and + // sent to automation scripts (if any) + // we need to make sure it does not get sanitized data + upd.CreatedAt = r.CreatedAt + upd.CreatedBy = r.CreatedBy + upd.UpdatedAt = r.UpdatedAt + upd.UpdatedBy = r.UpdatedBy + upd.DeletedAt = r.DeletedAt + upd.UpdatedBy = r.UpdatedBy + upd.Values = svc.sanitizer.Run(m, upd.Values) + if upd.OwnedBy == 0 { + upd.OwnedBy = r.OwnedBy + } - svc.recordInfoUpdate(r) + // Before values are stored, we have to validate them + rve := svc.validator.Run(m, upd) + if !rve.IsValid() { + return rve + } - if err = svc.copyChanges(m, upd, r); err != nil { - return - } + // Preload old record values so we can send it together with event + if err = svc.preloadValues(m, r); err != nil { + return + } - // We do not know what happened in the before-update script, - // so we must sanitize values again before we store it - if r.Values, err = svc.sanitizeValues(m, r.Values); err != nil { - return - } + if err = svc.eventbus.WaitFor(svc.ctx, event.RecordBeforeUpdate(upd, r, m, ns, rve)); err != nil { + return + } else if !rve.IsValid() { + return rve + } + + svc.recordInfoUpdate(r) + + // Sanitize values we got from before-update automation scripts + r.Values = svc.sanitizer.Run(m, upd.Values) + if upd.OwnedBy > 0 { + // Allow ownership change: + r.OwnedBy = upd.OwnedBy + } + + // Before values are stored, we have to validate them + if rve = svc.validator.Run(m, r); !rve.IsValid() { + return rve + } - return r, svc.db.Transaction(func() (err error) { if r, err = svc.recordRepo.Update(r); err != nil { return } @@ -384,7 +490,9 @@ func (svc record) Update(upd *types.Record) (r *types.Record, err error) { return } - defer svc.eventbus.Dispatch(svc.ctx, event.RecordAfterUpdate(upd, r, m, ns)) + // At this point we can return the value + rec = r + defer svc.eventbus.Dispatch(svc.ctx, event.RecordAfterUpdate(upd, r, m, ns, nil)) return }) } @@ -449,7 +557,7 @@ func (svc record) DeleteByID(namespaceID, moduleID uint64, recordIDs ...uint64) } // Calling before-record-delete scripts - if err = svc.eventbus.WaitFor(svc.ctx, event.RecordBeforeDelete(nil, del, m, ns)); err != nil { + if err = svc.eventbus.WaitFor(svc.ctx, event.RecordBeforeDelete(nil, del, m, ns, nil)); err != nil { if isBulkDelete { // Not considered fatal, // continue with next record @@ -470,7 +578,7 @@ func (svc record) DeleteByID(namespaceID, moduleID uint64, recordIDs ...uint64) return err } - defer svc.eventbus.Dispatch(svc.ctx, event.RecordAfterDelete(nil, del, m, ns)) + defer svc.eventbus.Dispatch(svc.ctx, event.RecordAfterDelete(nil, del, m, ns, nil)) return err }) @@ -664,98 +772,78 @@ func (svc record) loadCombo(namespaceID, moduleID, recordID uint64) (ns *types.N return } -// Copies changes from mod to r(ecord) -func (svc record) copyChanges(m *types.Module, mod, r *types.Record) (err error) { - r.OwnedBy = mod.OwnedBy - r.Values, err = svc.sanitizeValues(m, mod.Values) - return err -} +func (svc record) setDefaultValues(m *types.Module, vv types.RecordValueSet) (out types.RecordValueSet) { + out = vv -func (svc record) setDefaultValues(module *types.Module, rec *types.Record) (err error) { - err = module.Fields.Walk(func(field *types.ModuleField) error { - if field.DefaultValue == nil { + for _, f := range m.Fields { + if f.DefaultValue == nil { return nil } - place := uint(0) - return field.DefaultValue.Walk(func(value *types.RecordValue) error { + for i, dv := range f.DefaultValue { // Default values on field are (might be) without field name and place - value.Name = field.Name - value.Place = place - place++ - if !rec.Values.Has(value.Name, value.Place) { - rec.Values = rec.Values.Set(value) + if !out.Has(f.Name, uint(i)) { + out = append(out, &types.RecordValue{ + Name: f.Name, + Value: dv.Value, + Ref: 0, + Place: uint(i), + DeletedAt: nil, + }) } + } + } - return nil - }) - - return nil - }) - - rec.Values, err = svc.sanitizeValues(module, rec.Values) - return err + return } -// Validates and filters record values -func (svc record) sanitizeValues(module *types.Module, values types.RecordValueSet) (out types.RecordValueSet, err error) { - // Make sure there are no multi values in a non-multi value fields - err = module.Fields.Walk(func(field *types.ModuleField) error { - if !field.Multi && len(values.FilterByName(field.Name)) > 1 { - return errors.Errorf("more than one value for a single-value field %q", field.Name) - } - return nil - }) - - if err != nil { - return - } - - // Remove all values on un-updatable fields - out, err = values.Filter(func(v *types.RecordValue) (bool, error) { - var field = module.Fields.FindByName(v.Name) - if field == nil { - return false, errors.Errorf("no such field %q", v.Name) - } - - if field.IsRef() && v.Value == "" { - // Skip empty values on ref fields - return false, nil - } - - return svc.ac.CanUpdateRecordValue(svc.ctx, field), nil - }) - - if err != nil { - return - } - +// Does basic field and format validation +// +// Received values must fit the data model: on unknown fields +// or multi/single value mismatch we return an error +// +func (svc record) generalValueSetValidation(m *types.Module, vv types.RecordValueSet) (err error) { var ( - places = map[string]uint{} - numeric = regexp.MustCompile(`^(\d+)$`) + numeric = regexp.MustCompile(`^[1-9](\d+)$`) ) - return out, out.Walk(func(v *types.RecordValue) (err error) { - var field = module.Fields.FindByName(v.Name) + err = vv.Walk(func(v *types.RecordValue) error { + var field = m.Fields.FindByName(v.Name) if field == nil { return errors.Errorf("no such field %q", v.Name) } if field.IsRef() { + if v.Value == "" { + return nil + } + if !numeric.MatchString(v.Value) { return errors.Errorf("invalid reference format") } - - if v.Ref, err = strconv.ParseUint(v.Value, 10, 64); err != nil { - return err - } } - v.Place = places[field.Name] - places[field.Name]++ - return nil }) + + if err != nil { + return + } + + // Make sure there are no multi values in a non-multi value fields + err = m.Fields.Walk(func(field *types.ModuleField) error { + if !field.Multi && len(vv.FilterByName(field.Name)) > 1 { + return errors.Errorf("more than one value for a single-value field %q", field.Name) + } + + return nil + }) + + if err != nil { + return + } + + return } func (svc record) preloadValues(m *types.Module, rr ...*types.Record) error { diff --git a/compose/service/record_test.go b/compose/service/record_test.go index ae7d7c207..d662cfb28 100644 --- a/compose/service/record_test.go +++ b/compose/service/record_test.go @@ -10,7 +10,7 @@ import ( "github.com/cortezaproject/corteza-server/pkg/permissions" ) -func TestValueSanitizer(t *testing.T) { +func TestGeneralValueSetValidation(t *testing.T) { var ( req = require.New(t) @@ -26,51 +26,42 @@ func TestValueSanitizer(t *testing.T) { }, } - rvs, out types.RecordValueSet - err error + rvs types.RecordValueSet + err error ) rvs = types.RecordValueSet{{Name: "single1", Value: "single"}} - out, err = svc.sanitizeValues(module, rvs) + err = svc.generalValueSetValidation(module, rvs) req.NoError(err) - req.Len(out, 1, "expecting 1 record value after sanitization, got %d", len(rvs)) rvs = types.RecordValueSet{{Name: "unknown", Value: "single"}} - out, err = svc.sanitizeValues(module, rvs) - req.True(err != nil, "expecting sanitizeValues() to return an error, got nil") + err = svc.generalValueSetValidation(module, rvs) + req.True(err != nil, "expecting generalValueSetValidation() to return an error, got nil") rvs = types.RecordValueSet{{Name: "single1", Value: "single"}, {Name: "single1", Value: "single2"}} - out, err = svc.sanitizeValues(module, rvs) - req.True(err != nil, "expecting sanitizeValues() to return an error, got nil") + err = svc.generalValueSetValidation(module, rvs) + req.Error(err) rvs = types.RecordValueSet{{Name: "multi1", Value: "multi1"}, {Name: "multi1", Value: "multi1"}} - out, err = svc.sanitizeValues(module, rvs) + err = svc.generalValueSetValidation(module, rvs) req.NoError(err) - req.Len(out, 2, "expecting 2 record values after sanitization, got %d", len(rvs)) - req.Equal(uint(0), out[0].Place, "expecting first value to have place value 0, got %d", out[0].Place) - req.Equal(uint(1), out[1].Place, "expecting second value to have place value 1, got %d", out[1].Place) - rvs = types.RecordValueSet{{Name: "ref1", Value: "multi1"}} - out, err = svc.sanitizeValues(module, rvs) - req.True(err != nil, "expecting sanitizeValues() to return an error, got nil") + rvs = types.RecordValueSet{{Name: "ref1", Value: "non numeric value"}} + err = svc.generalValueSetValidation(module, rvs) + req.Error(err) rvs = types.RecordValueSet{{Name: "ref1", Value: "12345"}} - out, err = svc.sanitizeValues(module, rvs) + err = svc.generalValueSetValidation(module, rvs) req.NoError(err) - req.Len(out, 1, "expecting 1 record values after sanitization, got %d", len(rvs)) - req.Equal(uint64(12345), out[0].Ref, "expecting parsed ref value to match, got %d", rvs[0].Ref) rvs = types.RecordValueSet{{Name: "multiRef1", Value: "12345"}, {Name: "multiRef1", Value: "67890"}} - out, err = svc.sanitizeValues(module, rvs) + err = svc.generalValueSetValidation(module, rvs) req.NoError(err) req.Len(rvs, 2, "expecting 2 record values after sanitization, got %d", len(rvs)) - req.Equal(uint64(12345), out[0].Ref, "expecting parsed ref value to match, got %d", out[0].Ref) - req.Equal(uint64(67890), out[1].Ref, "expecting parsed ref value to match, got %d", out[1].Ref) rvs = types.RecordValueSet{{Name: "ref1", Value: ""}} - out, err = svc.sanitizeValues(module, rvs) + err = svc.generalValueSetValidation(module, rvs) req.NoError(err) - req.Len(out, 0, "expecting 0 record values after sanitization, got %d", len(rvs)) } func TestDefaultValueSetting(t *testing.T) { @@ -87,18 +78,15 @@ func TestDefaultValueSetting(t *testing.T) { }, } - rec *types.Record - chk = func(vv types.RecordValueSet, f string, p uint, v string) { a.True(vv.Has("multi", p)) a.Equal(v, vv.Get(f, p).Value) } ) - rec = &types.Record{} - a.NoError(svc.setDefaultValues(mod, rec)) - chk(rec.Values, "single", 0, "s") - chk(rec.Values, "multi", 0, "m1") - chk(rec.Values, "multi", 1, "m2") + out := svc.setDefaultValues(mod, nil) + chk(out, "single", 0, "s") + chk(out, "multi", 0, "m1") + chk(out, "multi", 1, "m2") } diff --git a/compose/service/values/sanitizer.go b/compose/service/values/sanitizer.go new file mode 100644 index 000000000..26eebb542 --- /dev/null +++ b/compose/service/values/sanitizer.go @@ -0,0 +1,209 @@ +package values + +import ( + "github.com/cortezaproject/corteza-server/compose/types" + "regexp" + "strconv" + "strings" + "time" +) + +const ( + boolTrue = "1" + boolFalse = "0" +) + +type ( + sanitizer struct { + } +) + +var ( + // value resembles something that can be true + truthy = regexp.MustCompile(`^(t(rue)?|y(es)?|1)$`) + + // valeu resembles something that can be a reference + refy = regexp.MustCompile(`^[1-9](\d*)$`) +) + +// Sanitizer initializes sanitizer +// +// Not really needed, following pattern in the package +func Sanitizer() *sanitizer { + return &sanitizer{} +} + +// Run cleans up input data +// - fix multi-value order/place index +// - trim all the strings! +// - parse & format input values to match field specific -- nullify/falsify invalid +// - field kind specific, no errors raised, data is modified +// +// Existing data (when updating record) is not yet loaded at this point +func (s sanitizer) Run(m *types.Module, vv types.RecordValueSet) (out types.RecordValueSet) { + out = make([]*types.RecordValue, 0, len(vv)) + + for _, f := range m.Fields { + // Reorder and sanitize place value (no gaps) + // + // Values are ordered when received so we treat them like it + // and assign the appropriate place no. + for i, v := range vv.FilterByName(f.Name) { + out = append(out, &types.RecordValue{ + Name: f.Name, + Value: v.Value, + Ref: v.Ref, + Place: uint(i), + }) + } + } + + var ( + f *types.ModuleField + kind string + ) + + for _, v := range out { + f = m.Fields.FindByName(v.Name) + if f == nil { + // Unknown field, + // if it is not handled before, + // sanitizer does not care about it + continue + } + + kind = strings.ToLower(f.Kind) + + if kind != "string" { + // Trim all but string + v.Value = strings.TrimSpace(v.Value) + } + + if f.IsRef() && refy.MatchString(v.Value) { + v.Ref, _ = strconv.ParseUint(v.Value, 10, 64) + } + + // Per field type validators + switch strings.ToLower(f.Kind) { + case "bool": + v = s.sanitizeBool(v, f, m) + case "datetime": + v = s.sanitizeDatetime(v, f, m) + case "email": + v = s.sanitizeEmail(v, f, m) + case "file": + v = s.sanitizeFile(v, f, m) + case "number": + v = s.sanitizeNumber(v, f, m) + case "record": + v = s.sanitizeRecord(v, f, m) + case "select": + v = s.sanitizeSelect(v, f, m) + case "string": + v = s.sanitizeString(v, f, m) + case "url": + v = s.sanitizeUrl(v, f, m) + case "user": + v = s.sanitizeUser(v, f, m) + } + } + + return +} + +func (sanitizer) sanitizeBool(v *types.RecordValue, f *types.ModuleField, m *types.Module) *types.RecordValue { + if truthy.MatchString(strings.ToLower(v.Value)) { + v.Value = boolTrue + } else { + v.Value = boolFalse + } + + return v +} + +func (sanitizer) sanitizeDatetime(v *types.RecordValue, f *types.ModuleField, m *types.Module) *types.RecordValue { + var ( + // input format set + inputFormats []string + + // output format + of string + ) + + if f.Options.Bool("onlyDate") { + of = "2006-01-02" + inputFormats = []string{ + "2006-01-02", + "02 Jan 06", + "Monday, 02-Jan-06", + "Mon, 02 Jan 2006", + "2019/_1/_2", + } + } else if f.Options.Bool("onlyTime") { + of = "15:04:05" + inputFormats = []string{ + "15:04:05", + "15:04", + time.Kitchen, + } + } else { + of = time.RFC3339 + // date & time + inputFormats = []string{ + time.RFC3339, + time.RFC1123Z, + time.RFC1123, + time.RFC850, + time.RFC822Z, + time.RFC822, + time.RubyDate, + time.UnixDate, + time.ANSIC, + "2019/_1/_2 15:04:05", + "2019/_1/_2 15:04", + } + } + + for _, format := range inputFormats { + parsed, err := time.Parse(format, v.Value) + + if err == nil { + v.Value = parsed.UTC().Format(of) + break + } + } + + return v +} + +func (sanitizer) sanitizeEmail(v *types.RecordValue, f *types.ModuleField, m *types.Module) *types.RecordValue { + return v +} + +func (sanitizer) sanitizeFile(v *types.RecordValue, f *types.ModuleField, m *types.Module) *types.RecordValue { + return v +} + +func (sanitizer) sanitizeNumber(v *types.RecordValue, f *types.ModuleField, m *types.Module) *types.RecordValue { + return v +} + +func (sanitizer) sanitizeRecord(v *types.RecordValue, f *types.ModuleField, m *types.Module) *types.RecordValue { + return v +} + +func (sanitizer) sanitizeSelect(v *types.RecordValue, f *types.ModuleField, m *types.Module) *types.RecordValue { + return v +} + +func (sanitizer) sanitizeString(v *types.RecordValue, f *types.ModuleField, m *types.Module) *types.RecordValue { + return v +} + +func (sanitizer) sanitizeUrl(v *types.RecordValue, f *types.ModuleField, m *types.Module) *types.RecordValue { + return v +} + +func (sanitizer) sanitizeUser(v *types.RecordValue, f *types.ModuleField, m *types.Module) *types.RecordValue { + return v +} diff --git a/compose/service/values/validator.go b/compose/service/values/validator.go new file mode 100644 index 000000000..7d715673e --- /dev/null +++ b/compose/service/values/validator.go @@ -0,0 +1,207 @@ +package values + +import ( + "github.com/cortezaproject/corteza-server/compose/types" + "strings" +) + +// Validator package provides tooling to validate +// record and it's values against field constraints +// +// Structures and basic logic is similar to what we offer on the frontend +// (see corteza-js/validator package) but with less features as there +// is no need for such level of interaction and dynamic we require on the frontend + +type ( + UniqueChecker func(*types.Module, *types.ModuleField, *types.RecordValue) (uint64, error) + ReferenceChecker func(*types.Module, *types.ModuleField, *types.RecordValue) (bool, error) + + validator struct { + uniqueCheckerFn UniqueChecker + recordRefCheckerFn ReferenceChecker + userRefCheckerFn ReferenceChecker + } +) + +func makeInternalErr(field string, err error) types.RecordValueError { + return types.RecordValueError{Kind: "internal", Message: err.Error(), Meta: map[string]interface{}{"field": field}} +} +func makeEmptyErr(field string) types.RecordValueError { + return types.RecordValueError{Kind: "empty", Meta: map[string]interface{}{"field": field}} +} +func makeDuplicateValueInSetErr(field string) types.RecordValueError { + return types.RecordValueError{Kind: "duplicateValueInSet", Meta: map[string]interface{}{"field": field}} +} +func makeDuplicateValueErr(field string, recordID uint64) types.RecordValueError { + return types.RecordValueError{Kind: "duplicateValue", Meta: map[string]interface{}{"recordID": recordID, "field": field}} +} + +func Validator() *validator { + return &validator{} +} + +func (vldtr validator) UniqueChecker(fn UniqueChecker) { + vldtr.uniqueCheckerFn = fn +} + +func (vldtr validator) RecordRefChecker(fn ReferenceChecker) { + vldtr.recordRefCheckerFn = fn +} + +func (vldtr validator) UserRefChecker(fn ReferenceChecker) { + vldtr.userRefCheckerFn = fn +} + +// Run validates record and it's values against module & module fields options +// +// +// Validation is done in phases for optimal resource usage: +// - check if required values are present +// - check for unique-multi-value in multi value fields +// - field-kind specific validation on all values +// - unique check on all all values +func (vldtr validator) Run(m *types.Module, r *types.Record) (out *types.RecordValueErrorSet) { + var ( + f *types.ModuleField + ) + + out = &types.RecordValueErrorSet{} + +fields: + for _, f := range m.Fields { + vv := r.Values.FilterByName(f.Name) + + if f.Required { + if len(vv) == 0 { + out.Push(makeEmptyErr(f.Name)) + continue fields + } + + for _, v := range vv { + if len(v.Value) == 0 || (f.IsRef() && v.Ref == 0) { + out.Push(makeEmptyErr(f.Name)) + continue fields + } + } + } + + if f.Multi && f.Options.IsUniqueMultiValue() { + flipped := make(map[string]bool) + for _, v := range vv { + if flipped[v.Value] { + out.Push(makeDuplicateValueInSetErr(f.Name)) + continue fields + } + + flipped[v.Value] = true + } + } + } + + for _, v := range r.Values { + if f = m.Fields.FindByName(v.Name); f == nil { + continue + } + + // Per field type validators + switch strings.ToLower(f.Kind) { + case "bool": + out.Push(vldtr.validateBoolFieldKind(v, f, r, m)...) + case "datetime": + out.Push(vldtr.validateDatetimeFieldKind(v, f, r, m)...) + case "email": + out.Push(vldtr.validateEmailFieldKind(v, f, r, m)...) + case "file": + out.Push(vldtr.validateFileFieldKind(v, f, r, m)...) + case "number": + out.Push(vldtr.validateNumberFieldKind(v, f, r, m)...) + case "record": + out.Push(vldtr.validateRecordFieldKind(v, f, r, m)...) + case "select": + out.Push(vldtr.validateSelectFieldKind(v, f, r, m)...) + case "string": + out.Push(vldtr.validateStringFieldKind(v, f, r, m)...) + case "url": + out.Push(vldtr.validateUrlFieldKind(v, f, r, m)...) + case "user": + out.Push(vldtr.validateUserFieldKind(v, f, r, m)...) + } + } + + // This is the most resource-heavy operation + // we'll do in at the end + for _, v := range r.Values { + if f = m.Fields.FindByName(v.Name); f == nil { + continue + } + + if !f.Options.IsUnique() { + // Only interested in unique fields + continue + } + + duplicateRecordID, err := vldtr.uniqueCheckerFn(m, f, v) + if err != nil { + out.Push(makeInternalErr(f.Name, err)) + } else if duplicateRecordID > 0 && duplicateRecordID != r.ID { + out.Push(makeDuplicateValueErr(f.Name, duplicateRecordID)) + } + } + + if out.IsValid() { + return nil + } + + return out +} + +func (vldtr validator) validateBoolFieldKind(v *types.RecordValue, f *types.ModuleField, r *types.Record, m *types.Module) []types.RecordValueError { + // @todo must be 1 or 0 + return nil +} + +func (vldtr validator) validateDatetimeFieldKind(v *types.RecordValue, f *types.ModuleField, r *types.Record, m *types.Module) []types.RecordValueError { + // @todo must be datetime, UTC! + // @todo how do we check past/future (no info about previous value) + return nil +} + +func (vldtr validator) validateEmailFieldKind(v *types.RecordValue, f *types.ModuleField, r *types.Record, m *types.Module) []types.RecordValueError { + // @todo must be an email + return nil +} + +func (vldtr validator) validateFileFieldKind(v *types.RecordValue, f *types.ModuleField, r *types.Record, m *types.Module) []types.RecordValueError { + // @todo file is a Ref! + // @todo we can check for uniquenes (as we do for other refs) + return nil +} + +func (vldtr validator) validateNumberFieldKind(v *types.RecordValue, f *types.ModuleField, r *types.Record, m *types.Module) []types.RecordValueError { + // @todo make sure it is a number and cut decimals (precision) + return nil +} + +func (vldtr validator) validateRecordFieldKind(v *types.RecordValue, f *types.ModuleField, r *types.Record, m *types.Module) []types.RecordValueError { + // @todo record is a ref + return nil +} + +func (vldtr validator) validateSelectFieldKind(v *types.RecordValue, f *types.ModuleField, r *types.Record, m *types.Module) []types.RecordValueError { + // @todo validate v.Value against f.Options.Strings("options") + return nil +} + +func (vldtr validator) validateStringFieldKind(v *types.RecordValue, f *types.ModuleField, r *types.Record, m *types.Module) []types.RecordValueError { + return nil +} + +func (vldtr validator) validateUrlFieldKind(v *types.RecordValue, f *types.ModuleField, r *types.Record, m *types.Module) []types.RecordValueError { + // @todo must be URL + return nil +} + +func (vldtr validator) validateUserFieldKind(v *types.RecordValue, f *types.ModuleField, r *types.Record, m *types.Module) []types.RecordValueError { + // @todo user is a ref + return nil +} diff --git a/compose/types/module_field.go b/compose/types/module_field.go index abe29259e..3014efcc0 100644 --- a/compose/types/module_field.go +++ b/compose/types/module_field.go @@ -106,11 +106,6 @@ func (set ModuleFieldSet) Swap(i, j int) { set[i], set[j] = set[j], set[i] } -// IsRef tells us if value of this field be a reference to something (another record, user)? -func (f ModuleField) IsRef() bool { - return f.Kind == "Record" || f.Kind == "Owner" || f.Kind == "File" -} - func (f ModuleField) IsNumeric() bool { return f.Kind == "Number" } @@ -118,3 +113,9 @@ func (f ModuleField) IsNumeric() bool { func (f ModuleField) IsDateTime() bool { return f.Kind == "DateTime" } + +// IsRef tells us if value of this field be a reference to something +// (another record, file , user)? +func (f ModuleField) IsRef() bool { + return f.Kind == "Record" || f.Kind == "User" || f.Kind == "File" +} diff --git a/compose/types/record_value.go b/compose/types/record_value.go index 960af44c9..ebf02712b 100644 --- a/compose/types/record_value.go +++ b/compose/types/record_value.go @@ -17,9 +17,19 @@ type ( Ref uint64 `db:"ref" json:"-"` Place uint `db:"place" json:"-"` DeletedAt *time.Time `db:"deleted_at" json:"deletedAt,omitempty"` + + updated bool } ) +func (v RecordValue) IsUpdated() bool { + return v.updated +} + +func (v RecordValue) IsDeleted() bool { + return v.DeletedAt != nil +} + func (set RecordValueSet) FilterByName(name string) (vv RecordValueSet) { for i := range set { if set[i].Name == name { @@ -91,6 +101,76 @@ func (set RecordValueSet) Has(name string, place uint) bool { return false } +func (set RecordValueSet) SetUpdatedFlag(updated bool) { +} + +func (set RecordValueSet) GetUpdated() (out RecordValueSet) { + out = make([]*RecordValue, 0, len(set)) + for i := range set { + if !set[i].updated { + continue + } + + out = append(out, set[i]) + } + + // Append new value + return out +} + +// Replace old value set with new one +// +// Will remove all values that are not set in new set +// +// This satisfies current requirements where record is always +// manipulated as a whole +func (set RecordValueSet) Replace(new RecordValueSet) (out RecordValueSet) { + if len(new) == 0 { + return nil + } + + if len(set) == 0 { + for i := range new { + new[i].updated = true + } + + return new + } + + out = make([]*RecordValue, 0, len(set)+len(new)) + for s := range set { + // Mark all old as deleted + out = append(out, &RecordValue{ + Name: set[s].Name, + Value: set[s].Value, + Place: set[s].Place, + DeletedAt: &time.Time{}, + updated: true, + }) + } + + for n := range new { + if ex := out.Get(new[n].Name, new[n].Place); ex != nil { + // Reset deleted flag + ex.DeletedAt = nil + + // Did value change? + ex.updated = ex.updated || ex.Value != new[n].Value + + ex.Value = new[n].Value + } else { + out = append(out, &RecordValue{ + Name: new[n].Name, + Value: new[n].Value, + Place: new[n].Place, + updated: true, + }) + } + } + + return out +} + func (set *RecordValueSet) Scan(value interface{}) error { //lint:ignore S1034 This typecast is intentional, we need to get []byte out of a []uint8 switch value.(type) { @@ -108,3 +188,7 @@ func (set *RecordValueSet) Scan(value interface{}) error { func (set RecordValueSet) Value() (driver.Value, error) { return json.Marshal(set) } + +func (set RecordValueSet) Len() int { return len(set) } +func (set RecordValueSet) Swap(i, j int) { set[i], set[j] = set[j], set[i] } +func (set RecordValueSet) Less(i, j int) bool { return set[i].Place < set[j].Place } diff --git a/compose/types/record_value_test.go b/compose/types/record_value_test.go index f26c9f6e9..cbf85f512 100644 --- a/compose/types/record_value_test.go +++ b/compose/types/record_value_test.go @@ -3,6 +3,7 @@ package types import ( "reflect" "testing" + "time" ) func TestRecordValueSet_Set(t *testing.T) { @@ -40,3 +41,45 @@ func TestRecordValueSet_Set(t *testing.T) { }) } } + +func TestRecordValueSet_Replace(t *testing.T) { + tests := []struct { + name string + set RecordValueSet + new RecordValueSet + want RecordValueSet + }{ + { + name: "simple update of an empty set", + set: RecordValueSet{}, + new: RecordValueSet{{Name: "n", Value: "v"}}, + want: RecordValueSet{{Name: "n", Value: "v", updated: true}}, + }, + { + name: "update with nil", + set: RecordValueSet{{Name: "n", Value: "v"}}, + new: nil, + want: nil, + }, + { + name: "update with new value", + set: RecordValueSet{{Name: "n", Value: "1"}}, + new: RecordValueSet{{Name: "n", Value: "2"}}, + want: RecordValueSet{{Name: "n", Value: "2", updated: true}}, + }, + { + name: "update with less values", + set: RecordValueSet{{Name: "n", Value: "1"}, {Name: "deleted", Value: "d"}}, + new: RecordValueSet{{Name: "n", Value: "2"}}, + want: RecordValueSet{{Name: "n", Value: "2", updated: true}, {Name: "deleted", Value: "d", updated: true, DeletedAt: &time.Time{}}}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := tt.set.Replace(tt.new); !reflect.DeepEqual(got, tt.want) { + t.Errorf("Update() = %+v, want %+v", got, tt.want) + } + }) + } +} diff --git a/compose/types/validated.go b/compose/types/validated.go new file mode 100644 index 000000000..84fe7b4e6 --- /dev/null +++ b/compose/types/validated.go @@ -0,0 +1,25 @@ +package types + +type ( + RecordValueError struct { + Kind string `json:"kind"` + Message string `json:"message"` + Meta map[string]interface{} `json:"meta"` + } + + RecordValueErrorSet struct { + Set []RecordValueError `json:"set"` + } +) + +func (v *RecordValueErrorSet) Push(err ...RecordValueError) { + v.Set = append(v.Set, err...) +} + +func (v *RecordValueErrorSet) IsValid() bool { + return v == nil || len(v.Set) == 0 +} + +func (v *RecordValueErrorSet) Error() string { + return "invalid values input" +} diff --git a/system/service/sink.go b/system/service/sink.go index 2cff640a2..621bbcf9c 100644 --- a/system/service/sink.go +++ b/system/service/sink.go @@ -4,7 +4,6 @@ import ( "context" "encoding/base64" "encoding/json" - "github.com/davecgh/go-spew/spew" "io" "net/http" "net/url" @@ -236,7 +235,6 @@ func (svc *sink) process(contentType string, w http.ResponseWriter, r *http.Requ return } - spew.Dump(rsp) // Now write everything we've received from the script //if err = rsp.Header.Write(w); err != nil { // return