From 96a66631b712eb17bf822496a7c13709222de318 Mon Sep 17 00:00:00 2001 From: Javan lacerda Date: Sun, 30 Jun 2024 16:33:06 +0000 Subject: [PATCH] several fixes Signed-off-by: Javan lacerda --- pkg/config/config.go | 28 ++++++++++------------- pkg/config/config_network_test.go | 10 ++++---- pkg/identity/ciprovider/principal.go | 4 ++-- pkg/identity/ciprovider/principal_test.go | 4 ++-- 4 files changed, 21 insertions(+), 25 deletions(-) diff --git a/pkg/config/config.go b/pkg/config/config.go index 3181fdfbf..c2d2c36cd 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -76,13 +76,15 @@ type FulcioConfig struct { } type IssuerMetadata struct { - // Default key and values that can be used for filling the templates + // Defaults contains key-value pairs that can be used for filling the templates from ExtensionTemplates // If a key cannot be found on the token claims, the template will use the defaults DefaultTemplateValues map[string]string - // It is a Extensions version which the values are template strigs. - // It expects strings with templates syntax https://pkg.go.dev/text/template - // or raw strings with claims keys to be replaced - ClaimsTemplates certificate.Extensions + // ExtensionTemplates contains a mapping between certificate extension and token claim + // Provide either strings following https://pkg.go.dev/text/template syntax, + // e.g "{{ .url }}/{{ .repository }}" + // or non-templated strings with token claim keys to be replaced, + // e.g "job_workflow_sha" + ExtensionTemplates certificate.Extensions // Template for the Subject Alternative Name extension // It's typically the same value as Build Signer URI SubjectAlternativeNameTemplate string @@ -96,7 +98,7 @@ type OIDCIssuer struct { // Used to determine the subject of the certificate and if additional // certificate values are needed Type IssuerType `json:"Type" yaml:"type,omitempty"` - // Issuers CiProvider type + // CIProvider is an optional configuration to map token claims to extensions for CI workflows CIProvider string `json:"CIProvider,omitempty" yaml:"ci-provider,omitempty"` // Optional, if the issuer is in a different claim in the OIDC token IssuerClaim string `json:"IssuerClaim,omitempty" yaml:"issuer-claim,omitempty"` @@ -416,7 +418,7 @@ func validateConfig(conf *FulcioConfig) error { } } - return nil + return validateCIIssuerMetadata(conf) } var DefaultConfig = &FulcioConfig{ @@ -459,7 +461,7 @@ func FromContext(ctx context.Context) *FulcioConfig { // It checks that the templates defined are parseable // We should check it during the service bootstrap to avoid errors further -func CheckParseTemplates(fulcioConfig *FulcioConfig) error { +func validateCIIssuerMetadata(fulcioConfig *FulcioConfig) error { checkParse := func(temp interface{}) error { t := template.New("").Option("missingkey=error") @@ -469,7 +471,7 @@ func CheckParseTemplates(fulcioConfig *FulcioConfig) error { for _, ciIssuerMetadata := range fulcioConfig.CIIssuerMetadata { claimsTemplates := make(map[string]interface{}) - err := mapstructure.Decode(ciIssuerMetadata.ClaimsTemplates, &claimsTemplates) + err := mapstructure.Decode(ciIssuerMetadata.ExtensionTemplates, &claimsTemplates) if err != nil { return err } @@ -501,13 +503,7 @@ func Load(configPath string) (*FulcioConfig, error) { if err != nil { return nil, fmt.Errorf("read file: %w", err) } - - fulcioConfig, err := Read(b) - if err != nil { - return fulcioConfig, err - } - err = CheckParseTemplates(fulcioConfig) - return fulcioConfig, err + return Read(b) } // Read parses the bytes of a config diff --git a/pkg/config/config_network_test.go b/pkg/config/config_network_test.go index 54a67dc63..093b33df9 100644 --- a/pkg/config/config_network_test.go +++ b/pkg/config/config_network_test.go @@ -75,26 +75,26 @@ func TestParseTemplate(t *testing.T) { invalidTemplate := "{{.foobar}" ciissuerMetadata := make(map[string]IssuerMetadata) ciissuerMetadata["github"] = IssuerMetadata{ - ClaimsTemplates: certificate.Extensions{ + ExtensionTemplates: certificate.Extensions{ BuildTrigger: invalidTemplate, }, } fulcioConfig := &FulcioConfig{ CIIssuerMetadata: ciissuerMetadata, } - err := CheckParseTemplates(fulcioConfig) + err := validateCIIssuerMetadata(fulcioConfig) if err == nil { t.Error("It should raise an error") } ciissuerMetadata["github"] = IssuerMetadata{ - ClaimsTemplates: certificate.Extensions{ + ExtensionTemplates: certificate.Extensions{ BuildTrigger: validTemplate, }, } fulcioConfig = &FulcioConfig{ CIIssuerMetadata: ciissuerMetadata, } - err = CheckParseTemplates(fulcioConfig) + err = validateCIIssuerMetadata(fulcioConfig) if err != nil { t.Error("It shouldn't raise an error") } @@ -104,7 +104,7 @@ func TestParseTemplate(t *testing.T) { fulcioConfig = &FulcioConfig{ CIIssuerMetadata: ciissuerMetadata, } - err = CheckParseTemplates(fulcioConfig) + err = validateCIIssuerMetadata(fulcioConfig) if err == nil { t.Error("It should raise an error") } diff --git a/pkg/identity/ciprovider/principal.go b/pkg/identity/ciprovider/principal.go index ac50c6514..f45068e42 100644 --- a/pkg/identity/ciprovider/principal.go +++ b/pkg/identity/ciprovider/principal.go @@ -69,7 +69,7 @@ func applyTemplateOrReplace(extValueTemplate string, tokenClaims map[string]stri // for the template t := template.New("").Option("missingkey=error") // It shouldn't raise error since we already checked all - // templates in CheckParseTemplates functions in config.go + // templates in validateCIIssuerMetadata functions in config.go p, err := t.Parse(extValueTemplate) if err != nil { return "", err @@ -111,7 +111,7 @@ func (principal ciPrincipal) Name(_ context.Context) string { func (principal ciPrincipal) Embed(_ context.Context, cert *x509.Certificate) error { - claimsTemplates := principal.ClaimsMetadata.ClaimsTemplates + claimsTemplates := principal.ClaimsMetadata.ExtensionTemplates defaults := principal.ClaimsMetadata.DefaultTemplateValues claims, err := getTokenClaims(principal.Token) if err != nil { diff --git a/pkg/identity/ciprovider/principal_test.go b/pkg/identity/ciprovider/principal_test.go index 2d70e5959..4e407d82b 100644 --- a/pkg/identity/ciprovider/principal_test.go +++ b/pkg/identity/ciprovider/principal_test.go @@ -38,7 +38,7 @@ func TestWorkflowPrincipalFromIDToken(t *testing.T) { `Github workflow challenge should have all Github workflow extensions and issuer set`: { ExpectedPrincipal: ciPrincipal{ ClaimsMetadata: config.IssuerMetadata{ - ClaimsTemplates: certificate.Extensions{ + ExtensionTemplates: certificate.Extensions{ Issuer: "issuer", GithubWorkflowTrigger: "event_name", GithubWorkflowSHA: "sha", @@ -237,7 +237,7 @@ func TestEmbed(t *testing.T) { }, Principal: ciPrincipal{ ClaimsMetadata: config.IssuerMetadata{ - ClaimsTemplates: certificate.Extensions{ + ExtensionTemplates: certificate.Extensions{ GithubWorkflowTrigger: "event_name", GithubWorkflowSHA: "sha", GithubWorkflowName: "workflow",