3
0

Sanitization & validation refactoring (dates, refs)

This commit is contained in:
Denis Arh 2020-03-11 18:52:52 +01:00
parent 4ab0b61daa
commit ed445d120c
7 changed files with 281 additions and 124 deletions

View File

@ -337,41 +337,6 @@ func (svc record) Export(filter types.RecordFilter, enc Encoder) error {
func (svc record) Create(new *types.Record) (rec *types.Record, err error) {
var invokerID = auth.GetIdentityFromContext(svc.ctx).Identity()
// Runs value sanitization, sets values that should be used
// and validates the final result
//
// This logic is kept in a utility function - it's used in the beginning
// of the creation procedure and after results are back from the automation scripts
//
// Both these points introduce external data that need to be checked fully in the same manner
procChanges := func(m *types.Module, new *types.Record) *types.RecordValueErrorSet {
// Before values are processed further and
// sent to automation scripts (if any)
// we need to make sure it does not get sanitized data
new.Values = svc.sanitizer.Run(m, new.Values)
// Reset values to new record
// to make sure nobody slips in something we do not want
new.CreatedBy = invokerID
new.CreatedAt = *nowPtr()
new.UpdatedAt = nil
new.UpdatedBy = 0
new.DeletedAt = nil
new.DeletedBy = 0
// Mark all values as updated (new)
new.Values.SetUpdatedFlag(true)
if new.OwnedBy == 0 {
// If od owner is not set, make current user
// the owner of the record
new.OwnedBy = invokerID
}
// Run validation of the updated records
return svc.validator.Run(m, new)
}
return rec, svc.db.Transaction(func() (err error) {
var (
ns *types.Namespace
@ -396,7 +361,7 @@ func (svc record) Create(new *types.Record) (rec *types.Record, err error) {
)
// Handle input payload
if rve = procChanges(m, new); !rve.IsValid() {
if rve = svc.procCreate(invokerID, m, new); !rve.IsValid() {
return rve
}
@ -410,7 +375,7 @@ func (svc record) Create(new *types.Record) (rec *types.Record, err error) {
new.Values = svc.setDefaultValues(m, new.Values)
// Handle payload from automation scripts
if rve = procChanges(m, new); !rve.IsValid() {
if rve = svc.procCreate(invokerID, m, new); !rve.IsValid() {
return rve
}
@ -430,53 +395,44 @@ func (svc record) Create(new *types.Record) (rec *types.Record, err error) {
})
}
// Runs value sanitization, sets values that should be used
// and validates the final result
//
// This logic is kept in a utility function - it's used in the beginning
// of the creation procedure and after results are back from the automation scripts
//
// Both these points introduce external data that need to be checked fully in the same manner
func (svc record) procCreate(invokerID uint64, m *types.Module, new *types.Record) *types.RecordValueErrorSet {
// Mark all values as updated (new)
new.Values.SetUpdatedFlag(true)
// Before values are processed further and
// sent to automation scripts (if any)
// we need to make sure it does not get un-sanitized data
new.Values = svc.sanitizer.Run(m, new.Values)
// Reset values to new record
// to make sure nobody slips in something we do not want
new.CreatedBy = invokerID
new.CreatedAt = *nowPtr()
new.UpdatedAt = nil
new.UpdatedBy = 0
new.DeletedAt = nil
new.DeletedBy = 0
if new.OwnedBy == 0 {
// If od owner is not set, make current user
// the owner of the record
new.OwnedBy = invokerID
}
// Run validation of the updated records
return svc.validator.Run(m, new)
}
func (svc record) Update(upd *types.Record) (rec *types.Record, err error) {
var invokerID = auth.GetIdentityFromContext(svc.ctx).Identity()
// Runs value sanitization, copies values that should updated
// and validates the final result
//
// This logic is kept in a utility function - it's used in the beginning
// of the update procedure and after results are back from the automation scripts
//
// Both these points introduce external data that need to be checked fully in the same maner
procChanges := func(m *types.Module, upd *types.Record, old *types.Record) *types.RecordValueErrorSet {
// 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.Values = svc.sanitizer.Run(m, upd.Values)
// Copy values to updated record
// to make sure nobody slips in something we do not want
upd.CreatedAt = old.CreatedAt
upd.CreatedBy = old.CreatedBy
upd.UpdatedAt = nowPtr()
upd.UpdatedBy = invokerID
upd.DeletedAt = old.DeletedAt
upd.DeletedBy = old.DeletedBy
// Merge new (updated) values with old ones
// This way we get list of updated, stale and deleted values
// that we can selectively update in the repository
upd.Values = old.Values.Merge(upd.Values)
if upd.OwnedBy == 0 && old.OwnedBy > 0 {
// Owner not set/send in the payload
//
// Fallback to old owner (if set)
upd.OwnedBy = old.OwnedBy
} else {
// If od owner is not set, make current user
// the owner of the record
upd.OwnedBy = invokerID
}
// Run validation of the updated records
return svc.validator.Run(m, upd)
}
return rec, svc.db.Transaction(func() (err error) {
if upd.ID == 0 {
return ErrInvalidID.withStack()
@ -510,7 +466,7 @@ func (svc record) Update(upd *types.Record) (rec *types.Record, err error) {
)
// Handle input payload
if rve = procChanges(m, upd, old); !rve.IsValid() {
if rve = svc.procUpdate(invokerID, m, upd, old); !rve.IsValid() {
return rve
}
@ -526,7 +482,7 @@ func (svc record) Update(upd *types.Record) (rec *types.Record, err error) {
}
// Handle payload from automation scripts
if rve = procChanges(m, upd, old); !rve.IsValid() {
if rve = svc.procUpdate(invokerID, m, upd, old); !rve.IsValid() {
return rve
}
@ -548,6 +504,53 @@ func (svc record) Update(upd *types.Record) (rec *types.Record, err error) {
})
}
// Runs value sanitization, copies values that should updated
// and validates the final result
//
// This logic is kept in a utility function - it's used in the beginning
// of the update procedure and after results are back from the automation scripts
//
// Both these points introduce external data that need to be checked fully in the same maner
func (svc record) procUpdate(invokerID uint64, m *types.Module, upd *types.Record, old *types.Record) *types.RecordValueErrorSet {
// Mark all values as updated (new)
upd.Values.SetUpdatedFlag(true)
// 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.Values = svc.sanitizer.Run(m, upd.Values)
// Copy values to updated record
// to make sure nobody slips in something we do not want
upd.CreatedAt = old.CreatedAt
upd.CreatedBy = old.CreatedBy
upd.UpdatedAt = nowPtr()
upd.UpdatedBy = invokerID
upd.DeletedAt = old.DeletedAt
upd.DeletedBy = old.DeletedBy
// Merge new (updated) values with old ones
// This way we get list of updated, stale and deleted values
// that we can selectively update in the repository
upd.Values = old.Values.Merge(upd.Values)
if upd.OwnedBy == 0 && old.OwnedBy > 0 {
// Owner not set/send in the payload
//
// Fallback to old owner (if set)
upd.OwnedBy = old.OwnedBy
} else {
// If od owner is not set, make current user
// the owner of the record
upd.OwnedBy = invokerID
}
// Run validation of the updated records
return svc.validator.Run(m, upd)
}
func (svc record) recordInfoUpdate(r *types.Record) {
now := time.Now()
r.UpdatedAt = &now

View File

@ -88,5 +88,4 @@ func TestDefaultValueSetting(t *testing.T) {
chk(out, "single", 0, "s")
chk(out, "multi", 0, "m1")
chk(out, "multi", 1, "m2")
}

View File

@ -39,13 +39,10 @@ func (s sanitizer) Run(m *types.Module, vv types.RecordValueSet) (out types.Reco
continue
}
c := v.Clone()
c.Place = uint(i)
out = append(out, c)
i++
out = append(out, &types.RecordValue{
Name: f.Name,
Value: v.Value,
Ref: v.Ref,
Place: uint(i),
})
}
}
@ -63,7 +60,7 @@ func (s sanitizer) Run(m *types.Module, vv types.RecordValueSet) (out types.Reco
continue
}
if v.IsDeleted() || !v.IsUpdated() {
if v.IsDeleted() || !v.Updated {
// Ignore unchanged and deleted
continue
}
@ -75,8 +72,14 @@ func (s sanitizer) Run(m *types.Module, vv types.RecordValueSet) (out types.Reco
v.Value = strings.TrimSpace(v.Value)
}
if f.IsRef() && refy.MatchString(v.Value) {
v.Ref, _ = strconv.ParseUint(v.Value, 10, 64)
if f.IsRef() {
if refy.MatchString(v.Value) {
v.Ref, _ = strconv.ParseUint(v.Value, 10, 64)
}
if v.Ref == 0 {
v.Value = ""
}
}
// Per field type validators
@ -166,17 +169,26 @@ func (sanitizer) sDatetime(v *types.RecordValue, f *types.ModuleField, m *types.
"2019/_1/_2 15:04:05",
"2019/_1/_2 15:04",
}
// if string looks like a RFC 3330 (ISO 8601), see if we need to suffix it with Z
if isoDaty.MatchString(v.Value) && !hasTimezone.MatchString(v.Value) {
// No timezone, add Z to satisfy parser
v.Value = v.Value + "Z"
// Simplifiy list of rules
inputFormats = []string{time.RFC3339}
}
}
for _, format := range inputFormats {
parsed, err := time.Parse(format, v.Value)
if err == nil {
v.Value = parsed.UTC().Format(internalFormat)
break
return v
}
}
v.Value = ""
return v
}

View File

@ -0,0 +1,107 @@
package values
import (
"github.com/cortezaproject/corteza-server/compose/types"
"reflect"
"testing"
)
func Test_sanitizer_Run(t *testing.T) {
tests := []struct {
name string
kind string
input string
output string
outref uint64
}{
{
name: "numbers should be trimmed",
kind: "Number",
input: " 42 ",
output: "42",
},
{
name: "object reference should be processed",
kind: "Record",
input: " 133569629112020995 ",
output: "133569629112020995",
outref: 133569629112020995,
},
{
name: "object reference should be numeric",
kind: "Record",
input: " foo ",
output: "",
},
{
name: "user reference should be processed",
kind: "User",
input: " 133569629112020995 ",
output: "133569629112020995",
outref: 133569629112020995,
},
{
name: "user reference should be numeric",
kind: "User",
input: " foo ",
output: "",
},
{
name: "strings should be kept intact",
kind: "String",
input: " The answer ",
output: " The answer ",
},
{
name: "booleans should be converted (t)",
kind: "Bool",
input: "t",
output: "1",
},
{
name: "booleans should be converted (false)",
kind: "Bool",
input: "false",
output: "0",
},
{
name: "booleans should be converted (garbage)",
kind: "Bool",
input: "%%#)%)')$)'",
output: "0",
},
{
name: "dates should be converted to ISO",
kind: "DateTime",
input: "Mon Jan 2 15:04:05 2006",
output: "2006-01-02T15:04:05Z",
},
{
name: "dates should be converted to UTC",
kind: "DateTime",
input: "2020-03-02T20:20:20+05:00",
output: "2020-03-02T15:20:20Z",
},
{
name: "micro/mili seconds should be cut off",
kind: "DateTime",
input: "2020-03-11T11:20:08.471Z",
output: "2020-03-11T11:20:08Z",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
s := sanitizer{}
m := &types.Module{Fields: types.ModuleFieldSet{&types.ModuleField{Name: "testField", Kind: tt.kind}}}
v := types.RecordValueSet{&types.RecordValue{Name: "testField", Value: tt.input}}
o := types.RecordValueSet{&types.RecordValue{Name: "testField", Value: tt.output, Ref: tt.outref}}
// Need to mark values as updated to trigger sanitization.
v.SetUpdatedFlag(true)
o.SetUpdatedFlag(true)
if sanitized := s.Run(m, v); !reflect.DeepEqual(sanitized, o) {
t.Errorf("\ninput value:\n%v\n\nresult of sanitization:\n%v\n\nexpected:\n%v\n", tt.input, sanitized, o)
}
})
}
}

View File

@ -29,6 +29,10 @@ var (
// value resembles something that can be a reference
refy = regexp.MustCompile(`^[1-9](\d*)$`)
// value resembels something that can be parsed as ISO 8601
isoDaty = regexp.MustCompile(`^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}`)
hasTimezone = regexp.MustCompile(`(Z|\+\d{2}:\d{2})$`)
)
func nowPtr() *time.Time {

View File

@ -19,19 +19,32 @@ type (
Place uint `db:"place" json:"-"`
DeletedAt *time.Time `db:"deleted_at" json:"deletedAt,omitempty"`
updated bool
oldValue string
Updated bool `db:"-" json:"-"`
OldValue string `db:"-" json:"-"`
}
)
func (v RecordValue) IsUpdated() bool {
return v.updated
return v.Updated
}
func (v RecordValue) IsDeleted() bool {
return v.DeletedAt != nil
}
func (v RecordValue) Clone() *RecordValue {
return &RecordValue{
RecordID: v.RecordID,
Name: v.Name,
Value: v.Value,
Ref: v.Ref,
Place: v.Place,
DeletedAt: v.DeletedAt,
Updated: v.Updated,
OldValue: v.OldValue,
}
}
func (set RecordValueSet) FilterByName(name string) (vv RecordValueSet) {
for i := range set {
if set[i].Name == name {
@ -108,14 +121,14 @@ func (set RecordValueSet) Has(name string, place uint) bool {
func (set RecordValueSet) SetUpdatedFlag(updated bool) {
for i := range set {
set[i].updated = updated
set[i].Updated = updated
}
}
func (set RecordValueSet) GetUpdated() (out RecordValueSet) {
out = make([]*RecordValue, 0, len(set))
for i := range set {
if !set[i].updated {
if !set[i].Updated {
continue
}
@ -153,7 +166,7 @@ func (set RecordValueSet) Merge(new RecordValueSet) (out RecordValueSet) {
if len(set) == 0 {
// Empty set, copy all new values and return them
for i := range new {
new[i].updated = true
new[i].Updated = true
}
return new
@ -168,8 +181,8 @@ func (set RecordValueSet) Merge(new RecordValueSet) (out RecordValueSet) {
Ref: set[s].Ref,
Place: set[s].Place,
DeletedAt: &time.Time{},
updated: true,
oldValue: set[s].Value,
Updated: true,
OldValue: set[s].Value,
})
}
@ -178,12 +191,12 @@ func (set RecordValueSet) Merge(new RecordValueSet) (out RecordValueSet) {
// Reset deleted flag
ex.DeletedAt = new[n].DeletedAt
if ex.oldValue == new[n].Value {
ex.updated = false
} else if !ex.updated {
if ex.OldValue == new[n].Value {
ex.Updated = false
} else if !ex.Updated {
// Did value change?
ex.updated = ex.Value != new[n].Value
ex.oldValue = ex.Value
ex.Updated = ex.Value != new[n].Value
ex.OldValue = ex.Value
}
ex.Value = new[n].Value
@ -195,10 +208,10 @@ func (set RecordValueSet) Merge(new RecordValueSet) (out RecordValueSet) {
Value: new[n].Value,
Ref: new[n].Ref,
Place: new[n].Place,
updated: true,
Updated: true,
// verbose & explicit for clarity
oldValue: "",
OldValue: "",
DeletedAt: nil,
})
}
@ -232,10 +245,28 @@ func (set RecordValueSet) String() (o string) {
return "<RecordValueSet = nil>"
}
const tpl = "%-10s %2d %-10s %-20d %-10s %v %v\n"
is := func(in interface{}) string {
switch in := in.(type) {
case bool:
if in {
return "✔"
}
case *time.Time:
if in != nil {
return "✔"
}
}
return "x"
}
o += "━━━━━━━━━━━┳━━━━┳━━━┳━━━┳━━━━━━━━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━━━━━━━━━━━━━━━━━━\n"
o += "name ┃ ## ┃ u ┃ d ┃ value ┃ ref ┃ old value \n"
o += "━━━━━━━━━━━╋━━━━╋━━━╋━━━╋━━━━━━━━━━━━━━━━━━━━━━━━━━━╋━━━━━━━━━━━━━━━━━━━━━━╋━━━━━━━━━━━━━━━━━━━━━━━━━━━\n"
const tpl = "%-10s ┃ %2d ┃ %s ┃ %s ┃ %-25s ┃ %-20d ┃ %-25s\n"
for _, v := range set {
if v == nil {
o += "<RecordValue = nil>\n"
o += "<--------> ┃ -- ┃ - ┃ - ┃ <------------------------> ┃ <------------------> ┃ <------------------> \n"
continue
}
@ -243,13 +274,14 @@ func (set RecordValueSet) String() (o string) {
tpl,
v.Name,
v.Place,
is(v.Updated),
is(v.DeletedAt),
v.Value,
v.Ref,
v.oldValue,
v.updated,
v.DeletedAt,
v.OldValue,
)
}
o += "━━━━━━━━━━━┻━━━━┻━━━┻━━━┻━━━━━━━━━━━━━━━━━━━━━━━━━━━┻━━━━━━━━━━━━━━━━━━━━━━┻━━━━━━━━━━━━━━━━━━━━━━━━━━━\n"
return o
}

View File

@ -53,41 +53,41 @@ func TestRecordValueSet_Merge(t *testing.T) {
name: "simple update of an empty set",
set: RecordValueSet{},
new: RecordValueSet{{Name: "n", Value: "v"}},
want: RecordValueSet{{Name: "n", Value: "v", updated: true}},
want: RecordValueSet{{Name: "n", Value: "v", Updated: true}},
},
{
name: "update nil",
set: nil,
new: RecordValueSet{{Name: "n", Value: "v"}},
want: RecordValueSet{{Name: "n", Value: "v", oldValue: "", updated: true}},
want: RecordValueSet{{Name: "n", Value: "v", OldValue: "", Updated: true}},
},
{
name: "update with nil",
set: RecordValueSet{{Name: "n", Value: "v"}},
new: nil,
want: RecordValueSet{{Name: "n", Value: "v", oldValue: "v", DeletedAt: &time.Time{}, updated: true}},
want: RecordValueSet{{Name: "n", Value: "v", OldValue: "v", DeletedAt: &time.Time{}, Updated: true}},
},
{
name: "update with new value",
set: RecordValueSet{{Name: "n", Value: "1"}},
new: RecordValueSet{{Name: "n", Value: "2"}},
want: RecordValueSet{{Name: "n", Value: "2", oldValue: "1", updated: true}},
want: RecordValueSet{{Name: "n", Value: "2", OldValue: "1", 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", oldValue: "1", updated: true}, {Name: "deleted", Value: "d", oldValue: "d", updated: true, DeletedAt: &time.Time{}}},
want: RecordValueSet{{Name: "n", Value: "2", OldValue: "1", Updated: true}, {Name: "deleted", Value: "d", OldValue: "d", Updated: true, DeletedAt: &time.Time{}}},
},
{
name: "update multi value",
set: RecordValueSet{{Name: "c", Value: "1st", Place: 1}, {Name: "c", Value: "2nd", Place: 2}, {Name: "c", Value: "3rd", Place: 3}, {Name: "c", Value: "4th", Place: 4}},
new: RecordValueSet{{Name: "c", Value: "1st", Place: 1}, {Name: "c", Value: "2nd", Place: 2}, {Name: "c", Value: "4th", Place: 3}},
want: RecordValueSet{
{Name: "c", Value: "1st", Place: 1, oldValue: "1st"},
{Name: "c", Value: "2nd", Place: 2, oldValue: "2nd"},
{Name: "c", Value: "4th", Place: 3, oldValue: "3rd", updated: true},
{Name: "c", Value: "4th", Place: 4, oldValue: "4th", updated: true, DeletedAt: &time.Time{}},
{Name: "c", Value: "1st", Place: 1, OldValue: "1st"},
{Name: "c", Value: "2nd", Place: 2, OldValue: "2nd"},
{Name: "c", Value: "4th", Place: 3, OldValue: "3rd", Updated: true},
{Name: "c", Value: "4th", Place: 4, OldValue: "4th", Updated: true, DeletedAt: &time.Time{}},
},
},
}