BuildWorkloadJWTSVIDClaims fix for real credential composer implement…

…ation (#4489) BuildWorkloadJWTSVIDClaims fix for real credential composer implementation Signed-off-by: Monis Khan <i@monis.app>
spiffe · Sep 22, 2023 · dcd1c90 · dcd1c90
1 parent 54fc60c
commit dcd1c90
Show file tree

Hide file tree

Showing 2 changed files with 58 additions and 1 deletion.
diff --git a/pkg/server/credtemplate/builder_test.go b/pkg/server/credtemplate/builder_test.go
@@ -14,12 +14,14 @@ import (
 	"time"
 
 	"github.com/spiffe/go-spiffe/v2/spiffeid"
+	credentialcomposerv1 "github.com/spiffe/spire-plugin-sdk/proto/spire/plugin/server/credentialcomposer/v1"
 	"github.com/spiffe/spire/pkg/common/catalog"
 	"github.com/spiffe/spire/pkg/common/x509svid"
 	"github.com/spiffe/spire/pkg/common/x509util"
 	"github.com/spiffe/spire/pkg/server/credtemplate"
 	"github.com/spiffe/spire/pkg/server/plugin/credentialcomposer"
 	"github.com/spiffe/spire/test/clock"
+	"github.com/spiffe/spire/test/plugintest"
 	"github.com/spiffe/spire/test/testkey"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -185,6 +187,12 @@ func TestBuildSelfSignedX509CATemplate(t *testing.T) {
 			},
 			expectErr: "oh no",
 		},
+		{
+			desc: "real no-op composer",
+			overrideConfig: func(config *credtemplate.Config) {
+				config.CredentialComposers = []credentialcomposer.CredentialComposer{loadNoopV1Plugin(t)}
+			},
+		},
 	} {
 		t.Run(tc.desc, func(t *testing.T) {
 			testBuilder(t, tc.overrideConfig, func(t *testing.T, credBuilder *credtemplate.Builder) {
@@ -283,6 +291,12 @@ func TestBuildUpstreamSignedX509CACSR(t *testing.T) {
 			},
 			expectErr: "oh no",
 		},
+		{
+			desc: "real no-op composer",
+			overrideConfig: func(config *credtemplate.Config) {
+				config.CredentialComposers = []credentialcomposer.CredentialComposer{loadNoopV1Plugin(t)}
+			},
+		},
 	} {
 		t.Run(tc.desc, func(t *testing.T) {
 			testBuilder(t, tc.overrideConfig, func(t *testing.T, credBuilder *credtemplate.Builder) {
@@ -410,6 +424,12 @@ func TestBuildDownstreamX509CATemplate(t *testing.T) {
 			},
 			expectErr: "oh no",
 		},
+		{
+			desc: "real no-op composer",
+			overrideConfig: func(config *credtemplate.Config) {
+				config.CredentialComposers = []credentialcomposer.CredentialComposer{loadNoopV1Plugin(t)}
+			},
+		},
 	} {
 		t.Run(tc.desc, func(t *testing.T) {
 			testBuilder(t, tc.overrideConfig, func(t *testing.T, credBuilder *credtemplate.Builder) {
@@ -541,6 +561,12 @@ func TestBuildServerX509SVIDTemplate(t *testing.T) {
 			},
 			expectErr: "oh no",
 		},
+		{
+			desc: "real no-op composer",
+			overrideConfig: func(config *credtemplate.Config) {
+				config.CredentialComposers = []credentialcomposer.CredentialComposer{loadNoopV1Plugin(t)}
+			},
+		},
 	} {
 		t.Run(tc.desc, func(t *testing.T) {
 			testBuilder(t, tc.overrideConfig, func(t *testing.T, credBuilder *credtemplate.Builder) {
@@ -700,6 +726,12 @@ func TestBuildAgentX509SVIDTemplate(t *testing.T) {
 			},
 			expectErr: "oh no",
 		},
+		{
+			desc: "real no-op composer",
+			overrideConfig: func(config *credtemplate.Config) {
+				config.CredentialComposers = []credentialcomposer.CredentialComposer{loadNoopV1Plugin(t)}
+			},
+		},
 	} {
 		t.Run(tc.desc, func(t *testing.T) {
 			testBuilder(t, tc.overrideConfig, func(t *testing.T, credBuilder *credtemplate.Builder) {
@@ -923,6 +955,12 @@ func TestBuildWorkloadX509SVIDTemplate(t *testing.T) {
 			},
 			expectErr: "oh no",
 		},
+		{
+			desc: "real no-op composer",
+			overrideConfig: func(config *credtemplate.Config) {
+				config.CredentialComposers = []credentialcomposer.CredentialComposer{loadNoopV1Plugin(t)}
+			},
+		},
 	} {
 		t.Run(tc.desc, func(t *testing.T) {
 			testBuilder(t, tc.overrideConfig, func(t *testing.T, credBuilder *credtemplate.Builder) {
@@ -1089,6 +1127,12 @@ func TestBuildWorkloadJWTSVIDClaims(t *testing.T) {
 			},
 			expectErr: "oh no",
 		},
+		{
+			desc: "real no-op composer",
+			overrideConfig: func(config *credtemplate.Config) {
+				config.CredentialComposers = []credentialcomposer.CredentialComposer{loadNoopV1Plugin(t)}
+			},
+		},
 	} {
 		t.Run(tc.desc, func(t *testing.T) {
 			testBuilder(t, tc.overrideConfig, func(t *testing.T, credBuilder *credtemplate.Builder) {
@@ -1218,3 +1262,10 @@ func makeOID(id byte) []int {
 func idURIs(id spiffeid.ID) []*url.URL {
 	return []*url.URL{id.URL()}
 }
+
+func loadNoopV1Plugin(t *testing.T) credentialcomposer.CredentialComposer {
+	server := credentialcomposerv1.CredentialComposerPluginServer(credentialcomposerv1.UnimplementedCredentialComposerServer{})
+	cc := new(credentialcomposer.V1)
+	plugintest.Load(t, catalog.MakeBuiltIn("noop", server), cc)
+	return cc
+}
diff --git a/pkg/server/plugin/credentialcomposer/v1.go b/pkg/server/plugin/credentialcomposer/v1.go
@@ -6,6 +6,7 @@ import (
 	"crypto/x509"
 	"crypto/x509/pkix"
 	"encoding/asn1"
+	"encoding/json"
 	"errors"
 	"fmt"
 	"strconv"
@@ -221,8 +222,13 @@ func jwtSVIDAttributesToV1(attributes JWTSVIDAttributes) (*credentialcomposerv1.
 	if len(attributes.Claims) == 0 {
 		return nil, errors.New("invalid claims: cannot be empty")
 	}
-	claims, err := structpb.NewStruct(attributes.Claims)
+	// structpb.NewValue cannot handle Go types such as jwt.NumericDate so we marshal them into their JSON representation first
+	jsonClaims, err := json.Marshal(attributes.Claims)
 	if err != nil {
+		return nil, fmt.Errorf("failed to marshal claims: %w", err)
+	}
+	claims := &structpb.Struct{}
+	if err := claims.UnmarshalJSON(jsonClaims); err != nil {
 		return nil, fmt.Errorf("failed to encode claims: %w", err)
 	}
 	return &credentialcomposerv1.JWTSVIDAttributes{