fix(extension): refuse RAR members that differ only in case at any depth

extension/errors.go

@@ -21,10 +21,11 @@ var (
 	// with a slice T, or vice versa).
 	ErrCardinalityMismatch = errors.New("extension: cardinality does not match the definition's type")
 
-	// ErrDuplicateMember indicates a RAR detail object had the same
-	// top-level JSON member name more than once, compared
-	// case-insensitively as encoding/json matches names, or spelled its
-	// "type" member other than exactly "type".
+	// ErrDuplicateMember indicates a RAR detail object, or an object
+	// nested at any depth in one, had the same JSON member name more than
+	// once, compared case-insensitively as encoding/json matches names,
+	// or that the detail spelled its "type" member other than exactly
+	// "type".
 	ErrDuplicateMember = errors.New("extension: duplicate JSON member")
 
 	// ErrRARTooLarge indicates an authorization_details array exceeded

extension/rar.go

@@ -8,7 +8,8 @@ import (
 	"reflect"
 	"sort"
 	"strings"
-	"unicode"
+
+	"github.com/idfoundry/fapigo/internal/strictjson"
 )
 
 // RARDefinition captures the wire contract for one Rich Authorization
@@ -75,9 +76,15 @@ func (d RARDefinition[T]) decodeCheck(raw json.RawMessage) error {
 			return fmt.Errorf("extension: authorization_details type %q: malformed value: %w", d.Type, err)
 		}
 	}
+	// A member spelled other than its field's json tag — "ACTIONS" for
+	// "actions" — is one encoding/json reads and a case-sensitive reader
+	// of the issued token doesn't.
+	var v T
+	if err := strictjson.CheckTaggedFieldCase(raw, &v); err != nil {
+		return fmt.Errorf("extension: authorization_details type %q: malformed value: %w", d.Type, err)
+	}
 	dec := json.NewDecoder(bytes.NewReader(raw))
 	dec.DisallowUnknownFields()
-	var v T
 	if err := dec.Decode(&v); err != nil {
 		return fmt.Errorf("extension: authorization_details type %q: malformed value: %w", d.Type, err)
 	}
@@ -134,6 +141,9 @@ func RARGet[T any](values RARValues, def RARDefinition[T]) ([]RARDetail[T], erro
 	out := make([]RARDetail[T], 0, len(raws))
 	for _, raw := range raws {
 		var v T
+		if err := strictjson.CheckTaggedFieldCase(raw, &v); err != nil {
+			return nil, fmt.Errorf("extension: authorization_details type %q: %w", def.Type, err)
+		}
 		if err := json.Unmarshal(raw, &v); err != nil {
 			return nil, fmt.Errorf("extension: authorization_details type %q: %w", def.Type, err)
 		}
@@ -265,7 +275,7 @@ func (r *RARRegistry) Parse(raw json.RawMessage) (RARValues, error) {
 	values := RARValues{byType: make(map[string][]json.RawMessage)}
 	counts := make(map[string]int, len(r.byType))
 	for _, objRaw := range objects {
-		if err := checkNoDuplicateTopLevelKeys(objRaw); err != nil {
+		if err := checkMembers(objRaw); err != nil {
 			return RARValues{}, err
 		}
 
@@ -374,47 +384,34 @@ func checkJSONDepth(raw []byte, maxDepth int) error {
 	}
 }
 
-// checkNoDuplicateTopLevelKeys reports whether raw — expected to be a
-// JSON object — repeats a top-level member name, or spells "type" other
-// than exactly so. Names that differ only in case count as repeats:
-// encoding/json matches a member to a field case-insensitively, so of
-// {"amount":"1","AMOUNT":"1000"} it reads the last, where a
-// case-sensitive reader of the same issued token reads the first, and it
-// reads {"TYPE":"b"} as type "b" where that reader finds no type at all.
-// It does not recurse
-// into nested objects/arrays; encoding/json's own decode already applies
-// DisallowUnknownFields for whatever shape a RARDefinition's T declares,
-// so a duplicate nested member can only smuggle in a value the target
-// struct doesn't expose to begin with.
-// foldKey maps name to one spelling shared by every name it equals under
-// Unicode simple case folding (strings.EqualFold), as encoding/json
-// compares member names: each rune becomes the least of its fold orbit,
-// so "K", "k" and the Kelvin sign all map to "K".
-func foldKey(name string) string {
-	var b strings.Builder
-	for _, r := range name {
-		least := r
-		for f := unicode.SimpleFold(r); f != r; f = unicode.SimpleFold(f) {
-			least = min(least, f)
-		}
-		b.WriteRune(least)
-	}
-	return b.String()
-}
-
-func checkNoDuplicateTopLevelKeys(raw json.RawMessage) error {
+// checkMembers reports whether raw — expected to be a JSON object —
+// repeats a member name in any object at any depth, or spells its "type"
+// other than exactly so. Names that differ only in case count as
+// repeats: encoding/json matches a member to a field
+// case-insensitively, so of {"amount":"1","AMOUNT":"1000"} it reads the
+// last, where a case-sensitive reader of the same issued token reads the
+// first — nested as much as at the top, since DisallowUnknownFields
+// matches names the same way — and it reads {"TYPE":"b"} as type "b"
+// where that reader finds no type at all. A lone member spelled other
+// than its field's tag is RARDefinition.decodeCheck's to refuse, since
+// only the detail type knows its fields.
+func checkMembers(raw json.RawMessage) error {
 	dec := json.NewDecoder(bytes.NewReader(raw))
 	tok, err := dec.Token()
 	if err != nil {
 		return fmt.Errorf("extension: %w", err)
 	}
-	delim, ok := tok.(json.Delim)
-	if !ok || delim != '{' {
+	if delim, ok := tok.(json.Delim); !ok || delim != '{' {
 		return fmt.Errorf("extension: authorization_details object must be a JSON object")
 	}
+	return checkObjectMembers(dec, true)
+}
 
-	seen := make(map[string]string) // by foldKey, the name as spelled
-	typeKey := foldKey("type")
+// checkObjectMembers checks the members of the object whose "{" dec has
+// just read, and everything inside them, through its "}".
+func checkObjectMembers(dec *json.Decoder, top bool) error {
+	seen := make(map[string]string) // by strictjson.FoldKey, the name as spelled
+	typeKey := strictjson.FoldKey("type")
 	for dec.More() {
 		keyTok, err := dec.Token()
 		if err != nil {
@@ -424,19 +421,40 @@ func checkNoDuplicateTopLevelKeys(raw json.RawMessage) error {
 		if !ok {
 			return fmt.Errorf("extension: malformed object key")
 		}
-		folded := foldKey(key)
+		folded := strictjson.FoldKey(key)
 		if first, dup := seen[folded]; dup {
 			return fmt.Errorf("%w: %q and %q", ErrDuplicateMember, first, key)
 		}
-		if folded == typeKey && key != "type" {
+		if top && folded == typeKey && key != "type" {
 			return fmt.Errorf("%w: %q is not \"type\"", ErrDuplicateMember, key)
 		}
 		seen[folded] = key
+		if err := checkValueMembers(dec); err != nil {
+			return err
+		}
+	}
+	_, err := dec.Token() // "}"
+	return err
+}
 
-		var skip json.RawMessage
-		if err := dec.Decode(&skip); err != nil {
-			return fmt.Errorf("extension: %w", err)
+// checkValueMembers checks the next value dec reads: any object in it,
+// at any depth.
+func checkValueMembers(dec *json.Decoder) error {
+	tok, err := dec.Token()
+	if err != nil {
+		return fmt.Errorf("extension: %w", err)
+	}
+	switch tok {
+	case json.Delim('{'):
+		return checkObjectMembers(dec, false)
+	case json.Delim('['):
+		for dec.More() {
+			if err := checkValueMembers(dec); err != nil {
+				return err
+			}
 		}
+		_, err := dec.Token() // "]"
+		return err
 	}
 	return nil
 }

extension/rar_granted.go

@@ -21,11 +21,13 @@ const AuthorizationDetailsClaim = "authorization_details"
 //
 // A token without the claim was granted none: the result is empty. A
 // claim that isn't an array of objects each with a string "type" is an
-// error, never an empty grant read as "nothing to check". Each object's
-// members are checked as RARRegistry.Parse checks a request's (no member
-// twice, compared as encoding/json compares names, and "type" spelled
-// exactly), so this reads the claim as any case-sensitive reader of the
-// same token would. It doesn't otherwise validate the details: the
+// error, never an empty grant read as "nothing to check". Every object
+// in it, at any depth, is checked as RARRegistry.Parse checks a
+// request's: no member twice, compared as encoding/json compares names,
+// and "type" spelled exactly. RARGet then refuses a member spelled other
+// than its field's json tag. So the claim reads as any case-sensitive
+// reader of the same token would read it. It doesn't otherwise validate
+// the details: the
 // authorization server did that when it granted them, and RARGet decodes
 // only the types asked for, so a type this resource server doesn't know
 // is left alone.
@@ -39,7 +41,7 @@ func ParseGrantedRAR(claim json.RawMessage) (RARValues, error) {
 	}
 	byType := make(map[string][]json.RawMessage, len(objects))
 	for i, obj := range objects {
-		if err := checkNoDuplicateTopLevelKeys(obj); err != nil {
+		if err := checkMembers(obj); err != nil {
 			return RARValues{}, fmt.Errorf("extension: granted authorization_details object %d: %w", i, err)
 		}
 		var head rarObjectHead

extension/rar_nested_member_test.go

@@ -0,0 +1,78 @@
+package extension_test
+
+import (
+	"encoding/json"
+	"errors"
+	"testing"
+
+	"github.com/idfoundry/fapigo/extension"
+)
+
+type nestedAmount struct {
+	Value    string `json:"value"`
+	Currency string `json:"currency"`
+}
+
+type nestedPayment struct {
+	InstructedAmount nestedAmount   `json:"instructedAmount"`
+	Actions          []string       `json:"actions,omitempty"`
+	Creditors        []nestedAmount `json:"creditors,omitempty"`
+	Reference        untaggedNested `json:"reference,omitempty"`
+}
+
+// untaggedNested has no json tags: encoding/json matches its members to
+// the Go names in any case, as it always has.
+type untaggedNested struct {
+	Text string
+}
+
+var nestedPaymentType = extension.RARDefinition[nestedPayment]{Type: "payment", MaxObjects: 1, MaxBytesPerObject: 512}
+
+// TestRARParseRejectsNestedCaseVariants covers members below the top
+// level that encoding/json reads differently from a case-sensitive
+// reader of the issued token: a second "value" spelled "VALUE" makes the
+// consent page show 1.00 where that reader sees 1000.00, and a lone
+// "ACTIONS" is actions to one and no actions at all to the other.
+func TestRARParseRejectsNestedCaseVariants(t *testing.T) {
+	reg, err := extension.NewRARRegistry(4096, 5, nestedPaymentType)
+	if err != nil {
+		t.Fatal(err)
+	}
+	for name, detail := range map[string]string{
+		"value and VALUE":          `{"type":"payment","instructedAmount":{"value":"1000.00","currency":"EUR","VALUE":"1.00"}}`,
+		"value twice":              `{"type":"payment","instructedAmount":{"value":"1000.00","currency":"EUR","value":"1.00"}}`,
+		"in an array":              `{"type":"payment","instructedAmount":{"value":"1.00","currency":"EUR"},"creditors":[{"value":"1.00","Value":"9.00","currency":"EUR"}]}`,
+		"a lone ACTIONS":           `{"type":"payment","instructedAmount":{"value":"1.00","currency":"EUR"},"ACTIONS":["read"]}`,
+		"a lone nested VALUE":      `{"type":"payment","instructedAmount":{"VALUE":"1.00","currency":"EUR"}}`,
+		"long s for s in currency": "{\"type\":\"payment\",\"instructedAmount\":{\"value\":\"1.00\",\"currenſy\":\"EUR\"}}",
+	} {
+		if _, err := reg.Parse(json.RawMessage(`[` + detail + `]`)); err == nil {
+			t.Errorf("%s: Parse = nil error, want refusal", name)
+		}
+	}
+	for name, detail := range map[string]string{
+		"exact names":                  `{"type":"payment","instructedAmount":{"value":"1.00","currency":"EUR"},"actions":["read"]}`,
+		"an untagged field, as before": `{"type":"payment","instructedAmount":{"value":"1.00","currency":"EUR"},"reference":{"text":"inv-1"}}`,
+	} {
+		if _, err := reg.Parse(json.RawMessage(`[` + detail + `]`)); err != nil {
+			t.Errorf("%s: Parse = %v, want nil", name, err)
+		}
+	}
+}
+
+// TestGrantedRARRejectsNestedCaseVariants covers the resource side: a
+// claim with a nested repeat is refused by ParseGrantedRAR, and a lone
+// variant by RARGet, which knows the detail type.
+func TestGrantedRARRejectsNestedCaseVariants(t *testing.T) {
+	_, err := extension.ParseGrantedRAR(json.RawMessage(`[{"type":"payment","instructedAmount":{"value":"1000.00","VALUE":"1.00","currency":"EUR"}}]`))
+	if !errors.Is(err, extension.ErrDuplicateMember) {
+		t.Errorf("ParseGrantedRAR(nested repeat) = %v, want ErrDuplicateMember", err)
+	}
+	granted, err := extension.ParseGrantedRAR(json.RawMessage(`[{"type":"payment","instructedAmount":{"value":"1.00","currency":"EUR"},"ACTIONS":["read"]}]`))
+	if err != nil {
+		t.Fatalf("ParseGrantedRAR: %v", err)
+	}
+	if _, err := extension.RARGet(granted, nestedPaymentType); err == nil {
+		t.Error("RARGet(a lone ACTIONS) = nil error, want refusal")
+	}
+}