refactor: remote claim builder: simplify NewClaimBuilders

token/claimBuilder.go

@@ -117,8 +117,7 @@ func (nc nonceClaimBuilder) AddClaims(_ context.Context, r *Request, target map[
 	return nil
 }
 
-// remoteClaimBuilder invokes a remote system to obtain claims.  The metadata from a token request
-// is passed as the payload.
+// remoteClaimBuilder invokes a remote system to obtain claims.
 type remoteClaimBuilder struct {
 	endpoint endpoint.Endpoint
 	url      string
@@ -289,28 +288,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 +303,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 +334,21 @@ 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 _, v := range vals {
+		errs = append(errs, v.Validate())
+		if !v.IsStatic() {
+			continue
+		}
+
+		msg, err := v.RawMessage()
+		errs = append(errs, err)
+		m[v.Key] = msg
+	}
+
+	return m, errors.Join(errs...)
+}

token/claimBuilder_test.go

@@ -620,6 +620,46 @@ func (suite *NewClaimBuildersTestSuite) testMetadataMissingValue() {
 	suite.Error(err)
 }
 
+func (suite *NewClaimBuildersTestSuite) TestInvalidValueType() {
+	suite.Run("Claims", suite.testClaimsInvalidValueType)
+	suite.Run("Metadata", suite.testMetadataInvalidValueType)
+}
+
+func (suite *NewClaimBuildersTestSuite) testClaimsInvalidValueType() {
+	builder, err := NewClaimBuilders(suite.noncer, nil, Options{
+		Nonce:       false,
+		DisableTime: true,
+		Claims: []Value{
+			{
+				Key:    "test",
+				Header: "header1",
+				Value:  "value1",
+			},
+		},
+	})
+
+	suite.Nil(builder)
+	suite.Error(err)
+}
+
+func (suite *NewClaimBuildersTestSuite) testMetadataInvalidValueType() {
+	builder, err := NewClaimBuilders(suite.noncer, nil, Options{
+		Nonce:       false,
+		DisableTime: true,
+		Metadata: []Value{
+			{
+				Key:    "test",
+				Header: "header1",
+				Value:  "value1",
+			},
+		},
+		Remote: &RemoteClaims{},
+	})
+
+	suite.Nil(builder)
+	suite.Error(err)
+}
+
 func (suite *NewClaimBuildersTestSuite) TestBadJSONValue() {
 	suite.Run("Claims", suite.testClaimsBadJSONValue)
 	suite.Run("Metadata", suite.testMetadataBadJSONValue)

token/factory.go

@@ -30,16 +30,13 @@ type Request struct {
 	// but will not override time-based claims such as nbf or exp.
 	Claims map[string]interface{}
 
-	// Metadata holds non-claim information about the request, usually garnered from the original HTTP request (for remote claim requests).
-	Metadata map[string]interface{}
-
-	// (Optional) PathWildCards is a map listing the values specified for each url path wildcard (for remote claim requests).
-	// used to build request URL.
-	PathWildCards map[string]any
-
 	// TLS represents the state of any underlying TLS connection.
 	// For non-tls connections, this field is unset.
 	TLS *tls.ConnectionState
+
+	// The following fields are for remote claims' requests.
+	Metadata      map[string]any // Metadata is the request payload.
+	PathWildCards map[string]any // PathWildCards are the request path wildcards.
 }
 
 // NewRequest returns an empty, fully initialized token Request

token/options.go

@@ -4,6 +4,7 @@ package token
 
 import (
 	"encoding/json"
+	"fmt"
 	"time"
 
 	"github.com/xmidt-org/themis/key"
@@ -62,6 +63,28 @@ func (v Value) IsStatic() bool {
 	return len(v.JSON) > 0 || v.Value != nil
 }
 
+func (v Value) Validate() error {
+	if len(v.Key) == 0 {
+		return ErrMissingKey
+	}
+
+	var types []string
+	if v.IsFromHTTP() {
+		types = append(types, "http")
+	}
+	if v.IsStatic() {
+		types = append(types, "static")
+	}
+
+	if len(types) == 0 {
+		return fmt.Errorf("value `%s` must be 1 of the following: http, static", v.Key)
+	} else if len(types) > 1 {
+		return fmt.Errorf("value `%s` can't have multiple types: %s", v.Key, types)
+	}
+
+	return nil
+}
+
 // RawMessage precomputes the JSON  for this value.  If the JSON field is set,
 // it is verified by unmarshaling.  Otherwise, the Value field is marshaled.
 func (v Value) RawMessage() (json.RawMessage, error) {
@@ -201,12 +224,6 @@ type Options struct {
 	// or statically from configuration.  For special processing around the partner id, set the PartnerID field.
 	Claims []Value
 
-	// Metadata describes non-claim data, which can be statically configured or supplied via a request
-	Metadata []Value
-
-	// (Optional) PathWildCards are the URL's named path wildcards, which can be statically configured or supplied via a HTTP request
-	PathWildCards []Value
-
 	// PartnerID is the optional partner id configuration.  If unset, no partner id processing is
 	// performed, though a partner id may still be configured as part of the claims.
 	PartnerID *PartnerID
@@ -237,4 +254,8 @@ type Options struct {
 	// and returns a set of claims to be merged into tokens returned by the Factory.  Returned
 	// claims from the remote system do not override claims configured on the Factory.
 	Remote *RemoteClaims
+
+	// The following options are for remote claims' requests.
+	Metadata      []Value // Metadata describes the non-claim request payload, which can be statically configured or supplied via a request.
+	PathWildCards []Value // PathWildCards are the request path wildcards, which can be statically configured or supplied via a HTTP request.
 }