fix: return noTarget error for replace with unmatched value expression

Author: q-uint

patch_apply.go

@@ -3,15 +3,17 @@ package scim
 import (
 	"fmt"
 
+	scimErrors "github.com/elimity-com/scim/errors"
 	internal "github.com/elimity-com/scim/filter"
 	"github.com/elimity-com/scim/schema"
 	filter "github.com/scim2/filter-parser/v2"
 )
 
-func applyToMatching(list []interface{}, t *valueExprTarget, value interface{}, replace bool) ([]interface{}, error) {
+func applyToMatching(list []interface{}, t *valueExprTarget, value interface{}, replace bool) ([]interface{}, bool, error) {
 	result := make([]interface{}, len(list))
 	copy(result, list)
 
+	matched := false
 	for i, elem := range result {
 		m, ok := elem.(map[string]interface{})
 		if !ok {
@@ -20,6 +22,7 @@ func applyToMatching(list []interface{}, t *valueExprTarget, value interface{},
 		if !matchesExpression(m, t.expr, t.attr, t.refSchema) {
 			continue
 		}
+		matched = true
 		if t.subAttrName != "" {
 			updated := copyMap(m)
 			updated[t.subAttrName] = value
@@ -49,7 +52,7 @@ func applyToMatching(list []interface{}, t *valueExprTarget, value interface{},
 			}
 		}
 	}
-	return result, nil
+	return result, matched, nil
 }
 
 func copyMap(m map[string]interface{}) map[string]interface{} {
@@ -263,7 +266,7 @@ func applyAdd(attrs ResourceAttributes, op PatchOperation, s schema.Schema, exte
 		if !ok {
 			return nil, fmt.Errorf("expected multi-valued attribute for %s", attrName)
 		}
-		list, err := applyToMatching(list, t, op.Value, false)
+		list, _, err := applyToMatching(list, t, op.Value, false)
 		if err != nil {
 			return nil, err
 		}
@@ -405,18 +408,25 @@ func applyReplace(attrs ResourceAttributes, op PatchOperation, s schema.Schema,
 		m[t.subAttrName] = op.Value
 		attrs[attrName] = m
 	case *valueExprTarget:
+		// RFC 7644 Section 3.5.2.3: if the target location is a multi-valued
+		// attribute for which a value selection filter ("valuePath") has been
+		// supplied and no record match was made, the service provider SHALL
+		// return a 400 error with a scimType of noTarget.
 		existing, exists := attrs[attrName]
 		if !exists {
-			return attrs, nil
+			return nil, scimErrors.ScimErrorNoTarget
 		}
 		list, ok := existing.([]interface{})
 		if !ok {
 			return nil, fmt.Errorf("expected multi-valued attribute for %s", attrName)
 		}
-		list, err := applyToMatching(list, t, op.Value, true)
+		list, matched, err := applyToMatching(list, t, op.Value, true)
 		if err != nil {
 			return nil, err
 		}
+		if !matched {
+			return nil, scimErrors.ScimErrorNoTarget
+		}
 		attrs[attrName] = list
 	}
 	return attrs, nil

patch_apply_test.go

@@ -3,8 +3,9 @@ package scim
 import (
 	"testing"
 
+	scimErrors "github.com/elimity-com/scim/errors"
 	"github.com/elimity-com/scim/schema"
-	filterlib "github.com/scim2/filter-parser/v2"
+	filter "github.com/scim2/filter-parser/v2"
 )
 
 func TestApplyPatch_AddSimpleAttribute(t *testing.T) {
@@ -306,6 +307,47 @@ func TestApplyPatch_ReplaceSimpleAttribute(t *testing.T) {
 	}
 }
 
+// RFC 7644 Section 3.5.2.3: "If the target location is a multi-valued
+// attribute for which a value selection filter ("valuePath") has been
+// supplied and no record match was made, the service provider SHALL
+// return a 400 error with a scimType of noTarget".
+func TestApplyPatch_ReplaceValueExprNoTarget_AttributeMissing(t *testing.T) {
+	s := testUserSchema()
+	attrs := ResourceAttributes{
+		"userName": "john",
+	}
+
+	_, err := ApplyPatch(attrs, []PatchOperation{
+		{
+			Op:    PatchOperationReplace,
+			Path:  mustParsePath(`emails[type eq "work"].value`),
+			Value: "new@work.com",
+		},
+	}, s)
+	assertScimError(t, err, scimErrors.ScimTypeNoTarget)
+}
+
+func TestApplyPatch_ReplaceValueExprNoTarget_NoMatch(t *testing.T) {
+	s := testUserSchema()
+	attrs := ResourceAttributes{
+		"emails": []interface{}{
+			map[string]interface{}{
+				"value": "john@home.com",
+				"type":  "home",
+			},
+		},
+	}
+
+	_, err := ApplyPatch(attrs, []PatchOperation{
+		{
+			Op:    PatchOperationReplace,
+			Path:  mustParsePath(`emails[type eq "work"].value`),
+			Value: "new@work.com",
+		},
+	}, s)
+	assertScimError(t, err, scimErrors.ScimTypeNoTarget)
+}
+
 func TestApplyPatch_ReplaceWithNoPath(t *testing.T) {
 	s := testUserSchema()
 	attrs := ResourceAttributes{
@@ -374,8 +416,8 @@ func TestApplyPatch_ReplaceWithValueExpression(t *testing.T) {
 	}
 }
 
-func mustParsePath(s string) *filterlib.Path {
-	p, err := filterlib.ParsePath([]byte(s))
+func mustParsePath(s string) *filter.Path {
+	p, err := filter.ParsePath([]byte(s))
 	if err != nil {
 		panic(err)
 	}

utils_test.go

@@ -69,6 +69,20 @@ func assertNotNil(t *testing.T, object interface{}, name string) {
 	}
 }
 
+func assertScimError(t *testing.T, err error, expectedType errors.ScimType) {
+	t.Helper()
+	if err == nil {
+		t.Fatalf("expected scimType %q error, got nil", expectedType)
+	}
+	scimErr, ok := err.(errors.ScimError)
+	if !ok {
+		t.Fatalf("expected ScimError, got %T: %v", err, err)
+	}
+	if scimErr.ScimType != expectedType {
+		t.Errorf("expected scimType %q, got %q", expectedType, scimErr.ScimType)
+	}
+}
+
 func assertTrue(t *testing.T, ok bool) {
 	if !ok {
 		t.Error("value should be true")