refactor: remote claim builder: simplify NewClaimBuilders

- collapse `range o.Metadata` and `range o.Claims` for loops into the func `getStaticValues` since they're duplicates in terms of logic
- moved the `IsStatic()` error check out of `getStaticValues` to `func (v Value) RawMessage() (json.RawMessage, error)` since that's a better home for that particular check, reducing the cognitive load

Author: denopink

token/claimBuilder.go

@@ -289,28 +289,11 @@ func (cb *clientCertificateClaimBuilder) AddClaims(_ context.Context, r *Request
 // The returned builders do not include those claims derived from HTTP requests.  Claims derived from HTTP
 // requests are handled by NewRequestBuilders and DecodeServerRequest.
 func NewClaimBuilders(n random.Noncer, client xhttpclient.Interface, o Options) (ClaimBuilders, error) {
-	var (
-		builders           = ClaimBuilders{requestClaimBuilder{}}
-		staticClaimBuilder = make(staticClaimBuilder)
-	)
-
-	if o.Remote != nil { // scan the metadata looking for static values that should be applied when invoking the remote server
-		metadata := make(map[string]interface{})
-		for _, value := range o.Metadata {
-			switch {
-			case len(value.Key) == 0:
-				return nil, ErrMissingKey
-			case value.IsFromHTTP():
-				continue
-			case !value.IsStatic():
-				return nil, fmt.Errorf("a value is required for the static metadata: %s", value.Key)
-			default:
-				msg, err := value.RawMessage()
-				if err != nil {
-					return nil, err
-				}
-				metadata[value.Key] = msg
-			}
+	builders := ClaimBuilders{requestClaimBuilder{}}
+	if o.Remote != nil {
+		metadata, err := getStaticValues(o.Metadata)
+		if err != nil {
+			return nil, fmt.Errorf("remote claim builder configuration failure: metadata error: %w", err)
 		}
 
 		remoteClaimBuilder, err := newRemoteClaimBuilder(client, metadata, o.Remote)
@@ -321,31 +304,12 @@ func NewClaimBuilders(n random.Noncer, client xhttpclient.Interface, o Options)
 		builders = append(builders, remoteClaimBuilder)
 	}
 
-	for _, value := range o.Claims {
-		switch {
-		case len(value.Key) == 0:
-			return nil, ErrMissingKey
-
-		case value.IsFromHTTP():
-			continue
-
-		case !value.IsStatic():
-			return nil, fmt.Errorf("a value is required for the static claim: %s", value.Key)
-
-		default:
-			msg, err := value.RawMessage()
-			if err != nil {
-				return nil, err
-			}
-
-			staticClaimBuilder[value.Key] = msg
-		}
-	}
-
-	if len(staticClaimBuilder) > 0 {
-		builders = append(builders, staticClaimBuilder)
+	staticClaims, err := getStaticValues(o.Claims)
+	if err != nil {
+		return nil, fmt.Errorf("static claim builder configuration failure: %w", err)
 	}
 
+	builders = append(builders, staticClaimBuilder(staticClaims))
 	if o.Nonce && n != nil {
 		builders = append(builders, nonceClaimBuilder{n: n})
 	}
@@ -371,3 +335,26 @@ func NewClaimBuilders(n random.Noncer, client xhttpclient.Interface, o Options)
 
 	return builders, err
 }
+
+func getStaticValues(vals []Value) (map[string]any, error) {
+	var errs []error
+
+	m := make(map[string]any)
+	for _, value := range vals {
+		if len(value.Key) == 0 {
+			errs = append(errs, fmt.Errorf("valid static field `%s`: %w", value.Key, ErrVariableNotAllowed))
+
+			continue
+		}
+
+		if value.IsFromHTTP() {
+			continue
+		}
+
+		msg, err := value.RawMessage()
+		errs = append(errs, err)
+		m[value.Key] = msg
+	}
+
+	return m, errors.Join(errs...)
+}

token/options.go

@@ -4,6 +4,7 @@ package token
 
 import (
 	"encoding/json"
+	"fmt"
 	"time"
 
 	"github.com/xmidt-org/themis/key"
@@ -77,7 +78,7 @@ func (v Value) RawMessage() (json.RawMessage, error) {
 		return json.RawMessage(raw), err
 
 	default:
-		return json.RawMessage(nil), nil
+		return json.RawMessage(nil), fmt.Errorf("a value is required for the static field: %s", v.Key)
 	}
 }