fix: removing surplus slash, making logs richer (#1762)

* fix: removing surplus slash, making logs richer

Signed-off-by: Javan lacerda <javanlacerda@google.com>

* move trim to trimspace

Signed-off-by: Javan lacerda <javanlacerda@google.com>

* improving logs

Signed-off-by: Javan lacerda <javanlacerda@google.com>

---------

Signed-off-by: Javan lacerda <javanlacerda@google.com>

config/identity/config.yaml

@@ -178,7 +178,7 @@ ci-issuer-metadata:
       # ref_type: The type of the ref
       # E.g. "branch", "tag"
       # ref: Git ref being built
-      source-repository-ref: refs/{{if eq .ref_type "branch"}}heads/{{ else }}tags/{{end}}/{{ .ref }}
+      source-repository-ref: refs/{{if eq .ref_type "branch"}}heads/{{ else }}tags/{{end}}{{ .ref }}
       # project_id: ID to the source repo
       source-repository-identifier: "project_id"
       # namespace_path: Owner of the source repo (mutable)

pkg/identity/ciprovider/issuer_test.go

@@ -20,6 +20,7 @@ import (
 	"testing"
 
 	"github.com/coreos/go-oidc/v3/oidc"
+	"github.com/sigstore/fulcio/pkg/certificate"
 	"github.com/sigstore/fulcio/pkg/config"
 	"github.com/sigstore/fulcio/pkg/identity"
 )
@@ -81,8 +82,16 @@ func TestIssuer(t *testing.T) {
 					ClientID:   "sigstore",
 				},
 			}
+		template := "{{.foobar}}"
+		ciissuerMetadata := make(map[string]config.IssuerMetadata)
+		ciissuerMetadata["github-workflow"] = config.IssuerMetadata{
+			ExtensionTemplates: certificate.Extensions{
+				BuildTrigger: template,
+			},
+		}
 		cfg := &config.FulcioConfig{
-			OIDCIssuers: OIDCIssuers,
+			OIDCIssuers:      OIDCIssuers,
+			CIIssuerMetadata: ciissuerMetadata,
 		}
 		ctx = config.With(ctx, cfg)
 		identity.Authorize = func(_ context.Context, _ string, _ ...config.InsecureOIDCConfigOption) (*oidc.IDToken, error) {

pkg/identity/ciprovider/principal.go

@@ -18,6 +18,7 @@ import (
 	"bytes"
 	"context"
 	"crypto/x509"
+	"encoding/json"
 	"fmt"
 	"html/template"
 	"net/url"
@@ -47,7 +48,10 @@ func getTokenClaims(token *oidc.IDToken) (map[string]string, error) {
 
 // It makes string interpolation for a given string by using the
 // templates syntax https://pkg.go.dev/text/template
-func applyTemplateOrReplace(extValueTemplate string, tokenClaims map[string]string, issuerMetadata map[string]string) (string, error) {
+// logMetadata added as a parameter for having a richer log
+func applyTemplateOrReplace(
+	extValueTemplate string, tokenClaims map[string]string,
+	issuerMetadata map[string]string, logMetadata map[string]string) (string, error) {
 
 	// Here we merge the data from was claimed by the id token with the
 	// default data provided by the yaml file.
@@ -81,7 +85,10 @@ func applyTemplateOrReplace(extValueTemplate string, tokenClaims map[string]stri
 	}
 	claimValue, ok := mergedData[extValueTemplate]
 	if !ok {
-		return "", fmt.Errorf("value <%s> not present in either claims or defaults", extValueTemplate)
+		var jsonMetadata bytes.Buffer
+		inrec, _ := json.Marshal(logMetadata)
+		_ = json.Indent(&jsonMetadata, inrec, "", "\t")
+		return "", fmt.Errorf("value <%s> not present in either claims or defaults. %s", extValueTemplate, jsonMetadata.String())
 	}
 	return claimValue, nil
 }
@@ -97,9 +104,13 @@ func WorkflowPrincipalFromIDToken(ctx context.Context, token *oidc.IDToken) (ide
 	if !ok {
 		return nil, fmt.Errorf("configuration can not be loaded for issuer %v", token.Issuer)
 	}
+	metadata, ok := cfg.CIIssuerMetadata[issuerCfg.CIProvider]
+	if !ok {
+		return nil, fmt.Errorf("metadata not found for ci provider %s", issuerCfg.CIProvider)
+	}
 	return ciPrincipal{
 		token,
-		cfg.CIIssuerMetadata[issuerCfg.CIProvider],
+		metadata,
 	}, nil
 }
 
@@ -115,7 +126,15 @@ func (principal ciPrincipal) Embed(_ context.Context, cert *x509.Certificate) er
 	if err != nil {
 		return err
 	}
-	subjectAlternativeName, err := applyTemplateOrReplace(principal.ClaimsMetadata.SubjectAlternativeNameTemplate, claims, defaults)
+	if strings.TrimSpace(principal.ClaimsMetadata.SubjectAlternativeNameTemplate) == "" {
+		return fmt.Errorf("SubjectAlternativeNameTemplate should not be empty. Issuer: %s", principal.Token.Issuer)
+	}
+	subjectAlternativeName, err := applyTemplateOrReplace(
+		principal.ClaimsMetadata.SubjectAlternativeNameTemplate, claims, defaults,
+		map[string]string{
+			"Issuer":        principal.Token.Issuer,
+			"ExtensionName": "SubjectAlternativeName",
+		})
 	if err != nil {
 		return err
 	}
@@ -135,10 +154,14 @@ func (principal ciPrincipal) Embed(_ context.Context, cert *x509.Certificate) er
 		s := v.Field(i).String() // value of each field, e.g the template string
 		// We check the field name to avoid to apply the template for the Issuer
 		// Issuer field should always come from the token issuer
-		if s == "" || vType.Field(i).Name == "Issuer" {
+		if strings.TrimSpace(s) == "" || vType.Field(i).Name == "Issuer" {
 			continue
 		}
-		extValue, err := applyTemplateOrReplace(s, claims, defaults)
+		extValue, err := applyTemplateOrReplace(s, claims, defaults,
+			map[string]string{
+				"Issuer":        principal.Token.Issuer,
+				"ExtensionName": vType.Field(i).Name,
+			})
 		if err != nil {
 			return err
 		}

pkg/identity/ciprovider/principal_test.go

@@ -180,17 +180,25 @@ func TestName(t *testing.T) {
 			}
 			withClaims(token, claims)
 			ctx := context.TODO()
+			template := "{{.foobar}}"
+			ciissuerMetadata := make(map[string]config.IssuerMetadata)
+			ciissuerMetadata["github-workflow"] = config.IssuerMetadata{
+				ExtensionTemplates: certificate.Extensions{
+					BuildTrigger: template,
+				},
+			}
 			OIDCIssuers :=
 				map[string]config.OIDCIssuer{
 					token.Issuer: {
 						IssuerURL:  token.Issuer,
 						Type:       config.IssuerTypeCIProvider,
-						CIProvider: "ci-provider",
+						CIProvider: "github-workflow",
 						ClientID:   "sigstore",
 					},
 				}
 			cfg := &config.FulcioConfig{
-				OIDCIssuers: OIDCIssuers,
+				OIDCIssuers:      OIDCIssuers,
+				CIIssuerMetadata: ciissuerMetadata,
 			}
 			ctx = config.With(ctx, cfg)
 			principal, err := WorkflowPrincipalFromIDToken(ctx, token)
@@ -305,7 +313,10 @@ func TestApplyTemplateOrReplace(t *testing.T) {
 
 	for name, test := range tests {
 		t.Run(name, func(t *testing.T) {
-			res, err := applyTemplateOrReplace(test.Template, tokenClaims, issuerMetadata)
+			res, err := applyTemplateOrReplace(test.Template, tokenClaims, issuerMetadata,
+				map[string]string{
+					"Issuer": "https://token.actions.githubusercontent.com",
+				})
 			if res != test.ExpectedResult {
 				t.Errorf("expected result don't matches: Expected %s, received: %s, error: %v",
 					test.ExpectedResult, res, err)