Fix record update with update-denied fields

Scenario:
 - values already exist on the record
 - record is updated with users that can not update all fields
 - only values for updatable fields are provided

Access control was moved from generalValueSetValidation to procCreate/procUpdate
to allow return of record value error set.

Existing values from fields with update-deny set are copied to updating set
to ensure proper handling by RVS's Merge fn().

compose/service/record.go

@@ -779,6 +779,19 @@ func (svc record) procCreate(invokerID uint64, m *types.Module, new *types.Recor
 		new.OwnedBy = invokerID
 	}
 
+	rve := &types.RecordValueErrorSet{}
+	_ = new.Values.Walk(func(v *types.RecordValue) error {
+		if v.IsUpdated() && !svc.ac.CanUpdateRecordValue(svc.ctx, m.Fields.FindByName(v.Name)) {
+			rve.Push(types.RecordValueError{Kind: "updateDenied", Meta: map[string]interface{}{"field": v.Name, "value": v.Value}})
+		}
+
+		return nil
+	})
+
+	if !rve.IsValid() {
+		return rve
+	}
+
 	// Run validation of the updated records
 	return svc.validator.Run(m, new)
 }
@@ -824,11 +837,6 @@ func (svc record) procUpdate(invokerID uint64, m *types.Module, upd *types.Recor
 	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 {
 		if old.OwnedBy > 0 {
 			// Owner not set/send in the payload
@@ -842,6 +850,34 @@ func (svc record) procUpdate(invokerID uint64, m *types.Module, upd *types.Recor
 		}
 	}
 
+	// Value merge process does not know anything about permissions so
+	// in case when new values are missing but do exist in the old set and their update/read is denied
+	// we need to copy them to ensure value merge process them correctly
+	for _, f := range m.Fields {
+		if len(upd.Values.FilterByName(f.Name)) == 0 && !svc.ac.CanUpdateRecordValue(svc.ctx, m.Fields.FindByName(f.Name)) {
+			// copy all fields from old to new
+			upd.Values = append(upd.Values, old.Values.FilterByName(f.Name).GetClean()...)
+		}
+	}
+
+	// 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)
+
+	rve := &types.RecordValueErrorSet{}
+	_ = upd.Values.Walk(func(v *types.RecordValue) error {
+		if v.IsUpdated() && !svc.ac.CanUpdateRecordValue(svc.ctx, m.Fields.FindByName(v.Name)) {
+			rve.Push(types.RecordValueError{Kind: "updateDenied", Meta: map[string]interface{}{"field": v.Name, "value": v.Value}})
+		}
+
+		return nil
+	})
+
+	if !rve.IsValid() {
+		return rve
+	}
+
 	// Run validation of the updated records
 	return svc.validator.Run(m, upd)
 }
@@ -1372,10 +1408,6 @@ func (svc record) generalValueSetValidation(m *types.Module, vv types.RecordValu
 			return RecordErrFieldNotFound(aProps.setField(v.Name))
 		}
 
-		if !svc.ac.CanUpdateRecordValue(svc.ctx, field) {
-			return RecordErrNotAllowedToChangeFieldValue(aProps.setField(v.Name))
-		}
-
 		if field.IsRef() {
 			if v.Value == "" {
 				return nil

compose/types/record_value.go

@@ -174,7 +174,7 @@ func (set RecordValueSet) Merge(new RecordValueSet) (out RecordValueSet) {
 
 	out = make([]*RecordValue, 0)
 	for s := range set {
-		// Mark all old as deleted
+		// Mark all old as updated
 		out = append(out, &RecordValue{
 			Name:      set[s].Name,
 			Value:     set[s].Value,
@@ -192,9 +192,11 @@ func (set RecordValueSet) Merge(new RecordValueSet) (out RecordValueSet) {
 			ex.DeletedAt = new[n].DeletedAt
 
 			if ex.OldValue == new[n].Value {
+				// Value is the same
 				ex.Updated = false
 			} else if !ex.Updated {
-				// Did value change?
+				// Value changed and old one was not marked as updated before
+				// See if values really changed and update old value on existing value
 				ex.Updated = ex.Value != new[n].Value
 				ex.OldValue = ex.Value
 			}

tests/compose/record_test.go

@@ -450,7 +450,7 @@ func TestRecordImportImportProgress_sessionNotFound(t *testing.T) {
 func TestRecordFieldModulePermissionCheck(t *testing.T) {
 	h := newHelper(t)
 
-	// make a standad module, and prevent current user to
+	// make a standard module, and prevent (DENY) current user to
 	// read from "name" and update "email" fields
 	module := h.repoMakeRecordModuleWithFields("record testing module")
 	h.deny(module.Fields.FindByName("name").PermissionResource(), "record.value.read")
@@ -495,7 +495,7 @@ func TestRecordFieldModulePermissionCheck(t *testing.T) {
 				b().JSON(fmt.Sprintf(`{"values": [{"name": "email", "value": "changed-email"}]}`)).
 					Expect(t).
 					Status(http.StatusOK).
-					Assert(helpers.AssertError("not allowed to change value of field email")).
+					Assert(helpers.AssertError("1 issue(s) found")).
 					End()
 			})