From 90ad4ee00d35b8bfbfe92cd8691862852cd51765 Mon Sep 17 00:00:00 2001 From: dekiel Date: Thu, 9 Jan 2025 14:19:47 +0100 Subject: [PATCH] Creating a verifier config with client id read from trusted issuer. This allow creating a config with different expected client id values, based on detected issuer in provided oidc token. Each client specific for oidc provider and application must use uniq audience value. --- cmd/oidc-token-verifier/main.go | 40 +++-- pkg/oidc/mocks/mock_ClaimsInterface.go | 74 ++------- pkg/oidc/mocks/mock_ClaimsReader.go | 2 +- pkg/oidc/mocks/mock_ProviderInterface.go | 2 +- pkg/oidc/mocks/mock_TokenInterface.go | 2 +- pkg/oidc/mocks/mock_TokenProcessorOption.go | 2 +- pkg/oidc/mocks/mock_TokenVerifierInterface.go | 6 +- pkg/oidc/mocks/mock_Verifier.go | 6 +- pkg/oidc/mocks/mock_VerifierConfigOption.go | 2 +- pkg/oidc/mocks/mock_VerifierProvider.go | 2 +- pkg/oidc/mocks/mock_loggerInterface.go | 2 +- pkg/oidc/oidc.go | 140 ++++++++++-------- pkg/oidc/oidc_test.go | 123 +++++++-------- pkg/oidc/oidc_unit_test.go | 42 +++++- 14 files changed, 231 insertions(+), 214 deletions(-) diff --git a/cmd/oidc-token-verifier/main.go b/cmd/oidc-token-verifier/main.go index 9f8c9a80917b..c1f750f919f5 100644 --- a/cmd/oidc-token-verifier/main.go +++ b/cmd/oidc-token-verifier/main.go @@ -21,7 +21,6 @@ type Logger interface { type options struct { token string - clientID string trustedWorkflows []string debug bool oidcTokenExpirationTime int // OIDC token expiration time in minutes @@ -47,7 +46,6 @@ func NewRootCmd() *cobra.Command { // if err != nil { // panic(err) // } - rootCmd.PersistentFlags().StringVarP(&opts.clientID, "client-id", "c", "image-builder", "OIDC token client ID, this is used to verify the audience claim in the token. The value should be the same as the audience claim value in the token.") rootCmd.PersistentFlags().BoolVarP(&opts.debug, "debug", "d", false, "Enable debug mode") rootCmd.PersistentFlags().IntVarP(&opts.oidcTokenExpirationTime, "oidc-token-expiration-time", "e", 10, "OIDC token expiration time in minutes") return rootCmd @@ -119,29 +117,32 @@ func (opts *options) verifyToken() error { return err } - // Print used options values. - logger.Infow("Using the following trusted workflows", "trusted-workflows", opts.trustedWorkflows) - logger.Infow("Using the following client ID", "client-id", opts.clientID) - - // Create a new verifier config that will be used to verify the token. - // The clientID is used to verify the audience claim in the token. - verifyConfig, err := tioidc.NewVerifierConfig(logger, opts.clientID, tioidc.SkipExpiryCheck()) - if err != nil { - return err - } - logger.Infow("Verifier config created", "config", verifyConfig) - // Create a new token processor // It reads issuer from the token and verifies if the issuer is trusted. // The tokenProcessor is a main object that is used to verify the token and extract the claim values. // TODO(dekiel): add support for providing trusted issuers instead of using the value from the package. - tokenProcessor, err := tioidc.NewTokenProcessor(logger, tioidc.TrustedOIDCIssuers, opts.token, verifyConfig) + tokenProcessor, err := tioidc.NewTokenProcessor(logger, tioidc.TrustedOIDCIssuers, opts.token) if err != nil { return err } + logger.Infow("Token processor created for trusted issuer", "issuer", tokenProcessor.Issuer()) + + // TODO (dekiel): implement writing output data to the file. This will give us separated clear output for a data and logs. fmt.Printf("GITHUB_URL=%s\n", tokenProcessor.GetIssuer().GetGithubURL()) + // Create a new verifier config that will be used to verify the token. + // The standard expiration check is skipped. + // We use custom expiration time check to allow longer token expiration time than the value in the token. + // The extended expiration time is needed due to Azure DevOps delays in starting the pipeline. + // The delay was causing the token to expire before the pipeline started. + verifierConfig, err := tokenProcessor.NewVerifierConfig(tioidc.SkipExpiryCheck()) + if err != nil { + return err + } + + logger.Infow("Verifier config created") + ctx := context.Background() // Create a new provider using OIDC discovery to get the public keys. // It uses the issuer from the token to get the OIDC discovery endpoint. @@ -153,10 +154,15 @@ func (opts *options) verifyToken() error { // Create a new verifier using the provider and the verifier config. // The verifier is used to verify the token signature, expiration time and execute standard OIDC validation. - verifier, err := provider.NewVerifier(logger, verifyConfig, tioidc.WithExtendedExpiration(opts.oidcTokenExpirationTime)) + // TODO (dekiel): Consider using verifier config as the only way to parametrize the verification process. + // The WithExtendedExpiration could be moved to the verifier config. + // The WithExtendedExpiration could disable the standard expiration check. + // This would allow to have a single place to configure the verification process. + verifier, err := provider.NewVerifier(logger, verifierConfig, tioidc.WithExtendedExpiration(opts.oidcTokenExpirationTime)) if err != nil { return err } + logger.Infow("New verifier created") // Verify the token @@ -164,10 +170,12 @@ func (opts *options) verifyToken() error { if err != nil { return err } + logger.Infow("Token verified successfully") // Create claims claims := tioidc.NewClaims(logger) + logger.Infow("Verifying token claims") // Pass the token to ValidateClaims diff --git a/pkg/oidc/mocks/mock_ClaimsInterface.go b/pkg/oidc/mocks/mock_ClaimsInterface.go index 353b3ad7acdc..119ecc4c05a7 100644 --- a/pkg/oidc/mocks/mock_ClaimsInterface.go +++ b/pkg/oidc/mocks/mock_ClaimsInterface.go @@ -1,12 +1,10 @@ -// Code generated by mockery v2.38.0. DO NOT EDIT. +// Code generated by mockery v2.46.3. DO NOT EDIT. package oidcmocks import ( - jwt "github.com/go-jose/go-jose/v4/jwt" - mock "github.com/stretchr/testify/mock" - oidc "github.com/kyma-project/test-infra/pkg/oidc" + mock "github.com/stretchr/testify/mock" ) // MockClaimsInterface is an autogenerated mock type for the ClaimsInterface type @@ -22,58 +20,12 @@ func (_m *MockClaimsInterface) EXPECT() *MockClaimsInterface_Expecter { return &MockClaimsInterface_Expecter{mock: &_m.Mock} } -// Validate provides a mock function with given fields: _a0 -func (_m *MockClaimsInterface) Validate(_a0 jwt.Expected) error { - ret := _m.Called(_a0) - - if len(ret) == 0 { - panic("no return value specified for Validate") - } - - var r0 error - if rf, ok := ret.Get(0).(func(jwt.Expected) error); ok { - r0 = rf(_a0) - } else { - r0 = ret.Error(0) - } - - return r0 -} - -// MockClaimsInterface_Validate_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'Validate' -type MockClaimsInterface_Validate_Call struct { - *mock.Call -} - -// Validate is a helper method to define mock.On call -// - _a0 jwt.Expected -func (_e *MockClaimsInterface_Expecter) Validate(_a0 interface{}) *MockClaimsInterface_Validate_Call { - return &MockClaimsInterface_Validate_Call{Call: _e.mock.On("Validate", _a0)} -} - -func (_c *MockClaimsInterface_Validate_Call) Run(run func(_a0 jwt.Expected)) *MockClaimsInterface_Validate_Call { - _c.Call.Run(func(args mock.Arguments) { - run(args[0].(jwt.Expected)) - }) - return _c -} - -func (_c *MockClaimsInterface_Validate_Call) Return(_a0 error) *MockClaimsInterface_Validate_Call { - _c.Call.Return(_a0) - return _c -} - -func (_c *MockClaimsInterface_Validate_Call) RunAndReturn(run func(jwt.Expected) error) *MockClaimsInterface_Validate_Call { - _c.Call.Return(run) - return _c -} - -// ValidateExpectations provides a mock function with given fields: _a0 -func (_m *MockClaimsInterface) ValidateExpectations(_a0 oidc.Issuer) error { +// validateExpectations provides a mock function with given fields: _a0 +func (_m *MockClaimsInterface) validateExpectations(_a0 oidc.Issuer) error { ret := _m.Called(_a0) if len(ret) == 0 { - panic("no return value specified for ValidateExpectations") + panic("no return value specified for validateExpectations") } var r0 error @@ -86,30 +38,30 @@ func (_m *MockClaimsInterface) ValidateExpectations(_a0 oidc.Issuer) error { return r0 } -// MockClaimsInterface_ValidateExpectations_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'ValidateExpectations' -type MockClaimsInterface_ValidateExpectations_Call struct { +// MockClaimsInterface_validateExpectations_Call is a *mock.Call that shadows Run/Return methods with type explicit version for method 'validateExpectations' +type MockClaimsInterface_validateExpectations_Call struct { *mock.Call } -// ValidateExpectations is a helper method to define mock.On call +// validateExpectations is a helper method to define mock.On call // - _a0 oidc.Issuer -func (_e *MockClaimsInterface_Expecter) ValidateExpectations(_a0 interface{}) *MockClaimsInterface_ValidateExpectations_Call { - return &MockClaimsInterface_ValidateExpectations_Call{Call: _e.mock.On("ValidateExpectations", _a0)} +func (_e *MockClaimsInterface_Expecter) validateExpectations(_a0 interface{}) *MockClaimsInterface_validateExpectations_Call { + return &MockClaimsInterface_validateExpectations_Call{Call: _e.mock.On("validateExpectations", _a0)} } -func (_c *MockClaimsInterface_ValidateExpectations_Call) Run(run func(_a0 oidc.Issuer)) *MockClaimsInterface_ValidateExpectations_Call { +func (_c *MockClaimsInterface_validateExpectations_Call) Run(run func(_a0 oidc.Issuer)) *MockClaimsInterface_validateExpectations_Call { _c.Call.Run(func(args mock.Arguments) { run(args[0].(oidc.Issuer)) }) return _c } -func (_c *MockClaimsInterface_ValidateExpectations_Call) Return(_a0 error) *MockClaimsInterface_ValidateExpectations_Call { +func (_c *MockClaimsInterface_validateExpectations_Call) Return(_a0 error) *MockClaimsInterface_validateExpectations_Call { _c.Call.Return(_a0) return _c } -func (_c *MockClaimsInterface_ValidateExpectations_Call) RunAndReturn(run func(oidc.Issuer) error) *MockClaimsInterface_ValidateExpectations_Call { +func (_c *MockClaimsInterface_validateExpectations_Call) RunAndReturn(run func(oidc.Issuer) error) *MockClaimsInterface_validateExpectations_Call { _c.Call.Return(run) return _c } diff --git a/pkg/oidc/mocks/mock_ClaimsReader.go b/pkg/oidc/mocks/mock_ClaimsReader.go index 489be1efe73a..c9b948613f2b 100644 --- a/pkg/oidc/mocks/mock_ClaimsReader.go +++ b/pkg/oidc/mocks/mock_ClaimsReader.go @@ -1,4 +1,4 @@ -// Code generated by mockery v2.38.0. DO NOT EDIT. +// Code generated by mockery v2.46.3. DO NOT EDIT. package oidcmocks diff --git a/pkg/oidc/mocks/mock_ProviderInterface.go b/pkg/oidc/mocks/mock_ProviderInterface.go index 61b4618393b1..9e951ee701d3 100644 --- a/pkg/oidc/mocks/mock_ProviderInterface.go +++ b/pkg/oidc/mocks/mock_ProviderInterface.go @@ -1,4 +1,4 @@ -// Code generated by mockery v2.38.0. DO NOT EDIT. +// Code generated by mockery v2.46.3. DO NOT EDIT. package oidcmocks diff --git a/pkg/oidc/mocks/mock_TokenInterface.go b/pkg/oidc/mocks/mock_TokenInterface.go index 9ff95bbaa040..f0239d9e6077 100644 --- a/pkg/oidc/mocks/mock_TokenInterface.go +++ b/pkg/oidc/mocks/mock_TokenInterface.go @@ -1,4 +1,4 @@ -// Code generated by mockery v2.38.0. DO NOT EDIT. +// Code generated by mockery v2.46.3. DO NOT EDIT. package oidcmocks diff --git a/pkg/oidc/mocks/mock_TokenProcessorOption.go b/pkg/oidc/mocks/mock_TokenProcessorOption.go index 284373ec55f6..821b292ee325 100644 --- a/pkg/oidc/mocks/mock_TokenProcessorOption.go +++ b/pkg/oidc/mocks/mock_TokenProcessorOption.go @@ -1,4 +1,4 @@ -// Code generated by mockery v2.38.0. DO NOT EDIT. +// Code generated by mockery v2.46.3. DO NOT EDIT. package oidcmocks diff --git a/pkg/oidc/mocks/mock_TokenVerifierInterface.go b/pkg/oidc/mocks/mock_TokenVerifierInterface.go index 325fe88383f2..5694cb56ddde 100644 --- a/pkg/oidc/mocks/mock_TokenVerifierInterface.go +++ b/pkg/oidc/mocks/mock_TokenVerifierInterface.go @@ -1,12 +1,12 @@ -// Code generated by mockery v2.38.0. DO NOT EDIT. +// Code generated by mockery v2.46.3. DO NOT EDIT. package oidcmocks import ( - context "context" + mock "github.com/stretchr/testify/mock" + context "golang.org/x/net/context" oidc "github.com/kyma-project/test-infra/pkg/oidc" - mock "github.com/stretchr/testify/mock" ) // MockTokenVerifierInterface is an autogenerated mock type for the TokenVerifierInterface type diff --git a/pkg/oidc/mocks/mock_Verifier.go b/pkg/oidc/mocks/mock_Verifier.go index b0e509035ba8..b9a690d4612d 100644 --- a/pkg/oidc/mocks/mock_Verifier.go +++ b/pkg/oidc/mocks/mock_Verifier.go @@ -1,12 +1,12 @@ -// Code generated by mockery v2.38.0. DO NOT EDIT. +// Code generated by mockery v2.46.3. DO NOT EDIT. package oidcmocks import ( - context "context" + mock "github.com/stretchr/testify/mock" + context "golang.org/x/net/context" oidc "github.com/coreos/go-oidc/v3/oidc" - mock "github.com/stretchr/testify/mock" ) // MockVerifier is an autogenerated mock type for the Verifier type diff --git a/pkg/oidc/mocks/mock_VerifierConfigOption.go b/pkg/oidc/mocks/mock_VerifierConfigOption.go index 1cec5a37d1be..76b45dabff0a 100644 --- a/pkg/oidc/mocks/mock_VerifierConfigOption.go +++ b/pkg/oidc/mocks/mock_VerifierConfigOption.go @@ -1,4 +1,4 @@ -// Code generated by mockery v2.38.0. DO NOT EDIT. +// Code generated by mockery v2.46.3. DO NOT EDIT. package oidcmocks diff --git a/pkg/oidc/mocks/mock_VerifierProvider.go b/pkg/oidc/mocks/mock_VerifierProvider.go index 799850cf0f2b..223203525742 100644 --- a/pkg/oidc/mocks/mock_VerifierProvider.go +++ b/pkg/oidc/mocks/mock_VerifierProvider.go @@ -1,4 +1,4 @@ -// Code generated by mockery v2.38.0. DO NOT EDIT. +// Code generated by mockery v2.46.3. DO NOT EDIT. package oidcmocks diff --git a/pkg/oidc/mocks/mock_loggerInterface.go b/pkg/oidc/mocks/mock_loggerInterface.go index 192ad1097b88..750b2b409bbc 100644 --- a/pkg/oidc/mocks/mock_loggerInterface.go +++ b/pkg/oidc/mocks/mock_loggerInterface.go @@ -1,4 +1,4 @@ -// Code generated by mockery v2.38.0. DO NOT EDIT. +// Code generated by mockery v2.46.3. DO NOT EDIT. package oidcmocks diff --git a/pkg/oidc/oidc.go b/pkg/oidc/oidc.go index 409045e1e230..514f0c8b6461 100644 --- a/pkg/oidc/oidc.go +++ b/pkg/oidc/oidc.go @@ -26,6 +26,7 @@ var ( JWKSURL: "https://token.actions.githubusercontent.com/.well-known/jwks", ExpectedJobWorkflowRef: "kyma-project/test-infra/.github/workflows/image-builder.yml@refs/heads/main", GithubURL: "https://github.com", + ClientID: "image-builder", } GithubToolsSAPOIDCIssuer = Issuer{ Name: "github-tools-sap", @@ -33,8 +34,12 @@ var ( JWKSURL: "https://github.tools.sap/_services/token/.well-known/jwks", ExpectedJobWorkflowRef: "kyma/oci-image-builder/.github/workflows/image-builder.yml@refs/heads/main", GithubURL: "https://github.tools.sap", + ClientID: "image-builder", + } + TrustedOIDCIssuers = map[string]Issuer{ + GithubOIDCIssuer.IssuerURL: GithubOIDCIssuer, + GithubToolsSAPOIDCIssuer.IssuerURL: GithubToolsSAPOIDCIssuer, } - TrustedOIDCIssuers = map[string]Issuer{GithubOIDCIssuer.IssuerURL: GithubOIDCIssuer, GithubToolsSAPOIDCIssuer.IssuerURL: GithubToolsSAPOIDCIssuer} ) // TODO(dekiel) interfaces need to be clenup up to remove redundancy. @@ -83,6 +88,8 @@ type Issuer struct { JWKSURL string `json:"jwks_url" yaml:"jwks_url"` ExpectedJobWorkflowRef string `json:"expected_job_workflow_ref" yaml:"expected_job_workflow_ref"` GithubURL string `json:"github_url" yaml:"github_url"` + // The clientID is used to verify the audience claim in the token. + ClientID string `json:"client_id" yaml:"client_id"` } func (i Issuer) GetGithubURL() string { @@ -107,8 +114,8 @@ type TokenProcessor struct { rawToken string trustedIssuers map[string]Issuer issuer Issuer - verifierConfig VerifierConfig - logger LoggerInterface + // verifierConfig VerifierConfig + logger LoggerInterface } func (tokenProcessor *TokenProcessor) GetIssuer() Issuer { @@ -194,47 +201,6 @@ func maskToken(token string) string { return token[:2] + "********" + token[len(token)-2:] } -// NewVerifierConfig creates a new VerifierConfig. -// It verifies the clientID is not empty. -func NewVerifierConfig(logger LoggerInterface, clientID string, options ...VerifierConfigOption) (VerifierConfig, error) { - if clientID == "" { - return VerifierConfig{}, fmt.Errorf("clientID is empty") - } - - verifierConfig := VerifierConfig{} - verifierConfig.ClientID = clientID - verifierConfig.SkipClientIDCheck = false - verifierConfig.SkipExpiryCheck = false - verifierConfig.SkipIssuerCheck = false - verifierConfig.InsecureSkipSignatureCheck = false - verifierConfig.SupportedSigningAlgs = SupportedSigningAlgorithms - - logger.Debugw("Created Verifier config with default values", - "clientID", clientID, - "SkipClientIDCheck", verifierConfig.SkipClientIDCheck, - "SkipExpiryCheck", verifierConfig.SkipExpiryCheck, - "SkipIssuerCheck", verifierConfig.SkipIssuerCheck, - "InsecureSkipSignatureCheck", verifierConfig.InsecureSkipSignatureCheck, - "SupportedSigningAlgs", verifierConfig.SupportedSigningAlgs, - ) - logger.Debugw("Applying VerifierConfigOptions") - for _, option := range options { - err := option(&verifierConfig) - if err != nil { - return VerifierConfig{}, fmt.Errorf("failed to apply VerifierConfigOption: %w", err) - } - } - logger.Debugw("Applied all VerifierConfigOptions", - "clientID", clientID, - "SkipClientIDCheck", verifierConfig.SkipClientIDCheck, - "SkipExpiryCheck", verifierConfig.SkipExpiryCheck, - "SkipIssuerCheck", verifierConfig.SkipIssuerCheck, - "InsecureSkipSignatureCheck", verifierConfig.InsecureSkipSignatureCheck, - "SupportedSigningAlgs", verifierConfig.SupportedSigningAlgs, - ) - return verifierConfig, nil -} - // Verify verifies the raw OIDC token. // It returns a Token struct which contains the verified token if successful. // Verify allow checking extended expiration time for the token. @@ -353,14 +319,13 @@ func (provider *Provider) NewVerifier(logger LoggerInterface, verifierConfig Ver return tokenVerifier, nil } -// NewTokenProcessor creates a new TokenProcessor for trusted issuers. -// It reads the token, gets the issuer from the token, and checks if the issuer is trusted. -// It verifies the VerifierConfig has a clientID. +// NewTokenProcessor creates a new TokenProcessor for a trusted issuer. +// It reads the issuer from the raw token and checks if the issuer is trusted. +// It verifies the trusted issuer has a non-empty clientID. func NewTokenProcessor( logger LoggerInterface, trustedIssuers map[string]Issuer, rawToken string, - config VerifierConfig, options ...TokenProcessorOption, ) (TokenProcessor, error) { logger.Debugw("Creating token processor") @@ -371,30 +336,31 @@ func NewTokenProcessor( tokenProcessor.rawToken = rawToken logger.Debugw("Added raw token to token processor", "rawToken", maskToken(rawToken)) - tokenProcessor.verifierConfig = config - logger.Debugw("Added Verifier config to token processor", - "clientID", config.ClientID, - "SkipClientIDCheck", config.SkipClientIDCheck, - "SkipExpiryCheck", config.SkipExpiryCheck, - "SkipIssuerCheck", config.SkipIssuerCheck, - "InsecureSkipSignatureCheck", config.InsecureSkipSignatureCheck, - "SupportedSigningAlgs", config.SupportedSigningAlgs, - ) - if tokenProcessor.verifierConfig.ClientID == "" { - return TokenProcessor{}, errors.New("verifierConfig clientID is empty") - } - tokenProcessor.trustedIssuers = trustedIssuers + logger.Debugw("Added trusted issuers to token processor", "trustedIssuers", trustedIssuers) + issuer, err := tokenProcessor.tokenIssuer(SupportedSigningAlgorithms) if err != nil { return TokenProcessor{}, fmt.Errorf("failed to get issuer from token: %w", err) } + logger.Debugw("Got issuer from token", "issuer", issuer) + trustedIssuer, err := tokenProcessor.isTrustedIssuer(issuer, tokenProcessor.trustedIssuers) if err != nil { return TokenProcessor{}, err } + + logger.Debugw("Matched issuer with trusted issuer", "trustedIssuer", trustedIssuer) + + if trustedIssuer.ClientID == "" { + return TokenProcessor{}, errors.New("trusted issuer clientID is empty") + } + + logger.Debugw("Verified trusted issuer clientID", "clientID", trustedIssuer.ClientID) + tokenProcessor.issuer = trustedIssuer + logger.Debugw("Added trusted issuer to TokenProcessor", "issuer", tokenProcessor.issuer) if len(options) > 0 { @@ -409,9 +375,59 @@ func NewTokenProcessor( } logger.Debugw("Created token processor", "issuer", tokenProcessor.issuer) + return tokenProcessor, nil } +// NewVerifierConfig creates a new VerifierConfig for trusted issuer. +// It verifies if the clientID in the tokenProcessor is not empty. +func (tokenProcessor *TokenProcessor) NewVerifierConfig(options ...VerifierConfigOption) (VerifierConfig, error) { + logger := tokenProcessor.logger + + if tokenProcessor.issuer.ClientID == "" { + return VerifierConfig{}, fmt.Errorf("clientID is empty") + } + + logger.Debugw("TokenProcessor clientID is not empty", "clientID", tokenProcessor.issuer.ClientID) + + verifierConfig := VerifierConfig{} + verifierConfig.ClientID = tokenProcessor.issuer.ClientID + verifierConfig.SkipClientIDCheck = false + verifierConfig.SkipExpiryCheck = false + verifierConfig.SkipIssuerCheck = false + verifierConfig.InsecureSkipSignatureCheck = false + verifierConfig.SupportedSigningAlgs = SupportedSigningAlgorithms + + logger.Debugw("Created Verifier config with default values", + "clientID", verifierConfig.ClientID, + "SkipClientIDCheck", verifierConfig.SkipClientIDCheck, + "SkipExpiryCheck", verifierConfig.SkipExpiryCheck, + "SkipIssuerCheck", verifierConfig.SkipIssuerCheck, + "InsecureSkipSignatureCheck", verifierConfig.InsecureSkipSignatureCheck, + "SupportedSigningAlgs", verifierConfig.SupportedSigningAlgs, + ) + + logger.Debugw("Applying VerifierConfigOptions") + + for _, option := range options { + err := option(&verifierConfig) + if err != nil { + return VerifierConfig{}, fmt.Errorf("failed to apply VerifierConfigOption: %w", err) + } + } + + logger.Debugw("Applied all VerifierConfigOptions", + "clientID", verifierConfig.ClientID, + "SkipClientIDCheck", verifierConfig.SkipClientIDCheck, + "SkipExpiryCheck", verifierConfig.SkipExpiryCheck, + "SkipIssuerCheck", verifierConfig.SkipIssuerCheck, + "InsecureSkipSignatureCheck", verifierConfig.InsecureSkipSignatureCheck, + "SupportedSigningAlgs", verifierConfig.SupportedSigningAlgs, + ) + + return verifierConfig, nil +} + // tokenIssuer gets the issuer from the token. // It doesn't verify the token, just parses its claims. // It's used to create a new TokenProcessor. @@ -486,4 +502,4 @@ func (tokenProcessor *TokenProcessor) ValidateClaims(claims ClaimsInterface, tok return fmt.Errorf("expecations validation failed: %w", err) } return nil -} +} \ No newline at end of file diff --git a/pkg/oidc/oidc_test.go b/pkg/oidc/oidc_test.go index 70f4d967308d..ff85489667db 100644 --- a/pkg/oidc/oidc_test.go +++ b/pkg/oidc/oidc_test.go @@ -30,7 +30,6 @@ var _ = Describe("OIDC", func() { rawToken []byte verifierConfig tioidc.VerifierConfig tokenProcessor tioidc.TokenProcessor - clientID string ) BeforeEach(func() { @@ -38,7 +37,16 @@ var _ = Describe("OIDC", func() { Expect(err).NotTo(HaveOccurred()) logger = l.Sugar() - clientID = "testClientID" + + trustedIssuers = map[string]tioidc.Issuer{ + "https://fakedings.dev-gcp.nais.io/fake": { + Name: "github", + IssuerURL: "https://fakedings.dev-gcp.nais.io/fake", + JWKSURL: "https://fakedings.dev-gcp.nais.io/fake/jwks", + ExpectedJobWorkflowRef: "kyma-project/test-infra/.github/workflows/verify-oidc-token.yml@refs/heads/main", + ClientID: "testClientID", + }, + } }) Describe("SkipExpiryCheck", func() { @@ -55,26 +63,33 @@ var _ = Describe("OIDC", func() { var ( verifierConfigOption tioidc.VerifierConfigOption ) - It("should return a new oidc.Config", func() { - verifierConfig, err := tioidc.NewVerifierConfig(logger, clientID) + + BeforeEach(func() { + rawToken, err = os.ReadFile("test-fixtures/raw-oidc-token") + Expect(err).NotTo(HaveOccurred()) + + tokenProcessor, err = tioidc.NewTokenProcessor(logger, trustedIssuers, string(rawToken)) + Expect(err).NotTo(HaveOccurred()) + Expect(tokenProcessor).NotTo(BeNil()) + }) + + It("should create a new default VerifierConfig", func() { + verifierConfig, err := tokenProcessor.NewVerifierConfig() Expect(err).NotTo(HaveOccurred()) Expect(verifierConfig).To(BeAssignableToTypeOf(tioidc.VerifierConfig{})) - Expect(verifierConfig.ClientID).To(Equal(clientID)) + Expect(verifierConfig.ClientID).To(Equal(trustedIssuers["https://fakedings.dev-gcp.nais.io/fake"].ClientID)) Expect(verifierConfig.SupportedSigningAlgs).To(Equal(tioidc.SupportedSigningAlgorithms)) + Expect(verifierConfig.SkipExpiryCheck).To(BeFalse()) + Expect(verifierConfig.SkipClientIDCheck).To(BeFalse()) + Expect(verifierConfig.SkipIssuerCheck).To(BeFalse()) + Expect(verifierConfig.InsecureSkipSignatureCheck).To(BeFalse()) }) When("invalid VerifierConfigOption are provided", func() { It("should return an error", func() { verifierConfigOption = func(config *tioidc.VerifierConfig) error { return errors.New("invalid VerifierConfigOption") } - verifierConfig, err := tioidc.NewVerifierConfig(logger, clientID, verifierConfigOption) - Expect(err).To(HaveOccurred()) - Expect(verifierConfig).To(Equal(tioidc.VerifierConfig{})) - }) - }) - When("empty clientID is provided", func() { - It("should return an error", func() { - verifierConfig, err := tioidc.NewVerifierConfig(logger, "") + verifierConfig, err := tokenProcessor.NewVerifierConfig(verifierConfigOption) Expect(err).To(HaveOccurred()) Expect(verifierConfig).To(Equal(tioidc.VerifierConfig{})) }) @@ -90,32 +105,23 @@ var _ = Describe("OIDC", func() { // Read the token from the file in test-fixtures directory. rawToken, err = os.ReadFile("test-fixtures/raw-oidc-token") Expect(err).NotTo(HaveOccurred()) - - verifierConfig, err = tioidc.NewVerifierConfig(logger, clientID) - Expect(err).NotTo(HaveOccurred()) - - trustedIssuers = map[string]tioidc.Issuer{ - "https://fakedings.dev-gcp.nais.io/fake": { - Name: "github", - IssuerURL: "https://fakedings.dev-gcp.nais.io/fake", - JWKSURL: "https://fakedings.dev-gcp.nais.io/fake/jwks", - }, - } }) + When("issuer is trusted", func() { It("should return a new TokenProcessor", func() { - tokenProcessor, err = tioidc.NewTokenProcessor(logger, trustedIssuers, string(rawToken), verifierConfig) + tokenProcessor, err = tioidc.NewTokenProcessor(logger, trustedIssuers, string(rawToken)) Expect(err).NotTo(HaveOccurred()) Expect(tokenProcessor).To(BeAssignableToTypeOf(tioidc.TokenProcessor{})) }) }) - When("empty verifierConfig is provided", func() { + When("issuer with empty clientID is provided", func() { It("should return an error", func() { - // Empty verifierConfig - verifierConfig = tioidc.VerifierConfig{} - tokenProcessor, err := tioidc.NewTokenProcessor(logger, trustedIssuers, string(rawToken), verifierConfig) + // Empty issuer clientID + trustedIssuers["https://fakedings.dev-gcp.nais.io/fake"].ClientID = "" + + tokenProcessor, err := tioidc.NewTokenProcessor(logger, trustedIssuers, string(rawToken)) Expect(err).To(HaveOccurred()) - Expect(err).To(MatchError("verifierConfig clientID is empty")) + Expect(err).To(MatchError("trusted issuer clientID is empty")) Expect(tokenProcessor).To(Equal(tioidc.TokenProcessor{})) }) }) @@ -123,14 +129,15 @@ var _ = Describe("OIDC", func() { It("should return an error", func() { // Untrusted issuer trustedIssuers = map[string]tioidc.Issuer{ - "https://untrusted.fakedings.dev-gcp.nais.io/fake": { + "https://other-trusted.fakedings.dev-gcp.nais.io/fake": { Name: "github", - IssuerURL: "https://untrusted.fakedings.dev-gcp.nais.io/fake", - JWKSURL: "https://untrusted.fakedings.dev-gcp.nais.io/fake/jwks", + IssuerURL: "https://other-trusted.fakedings.dev-gcp.nais.io/fake", + JWKSURL: "https://other-trusted.fakedings.dev-gcp.nais.io/fake/jwks", + ClientID: "testClientID", }, } - tokenProcessor, err = tioidc.NewTokenProcessor(logger, trustedIssuers, string(rawToken), verifierConfig) + tokenProcessor, err = tioidc.NewTokenProcessor(logger, trustedIssuers, string(rawToken)) Expect(err).To(HaveOccurred()) Expect(err).To(MatchError("issuer https://fakedings.dev-gcp.nais.io/fake is not trusted")) Expect(tokenProcessor).To(Equal(tioidc.TokenProcessor{})) @@ -138,7 +145,7 @@ var _ = Describe("OIDC", func() { }) When("no trustedIssuers are provided", func() { It("should return an error", func() { - tokenProcessor, err := tioidc.NewTokenProcessor(logger, nil, string(rawToken), verifierConfig) + tokenProcessor, err := tioidc.NewTokenProcessor(logger, nil, string(rawToken)) Expect(err).To(HaveOccurred()) Expect(err).To(MatchError("issuer https://fakedings.dev-gcp.nais.io/fake is not trusted")) Expect(tokenProcessor).To(Equal(tioidc.TokenProcessor{})) @@ -150,7 +157,7 @@ var _ = Describe("OIDC", func() { return errors.New("invalid TokenProcessorOption") } - tokenProcessor, err := tioidc.NewTokenProcessor(logger, trustedIssuers, string(rawToken), verifierConfig, invalidTokenProcessorOption) + tokenProcessor, err := tioidc.NewTokenProcessor(logger, trustedIssuers, string(rawToken), invalidTokenProcessorOption) Expect(err).To(HaveOccurred()) Expect(err).To(MatchError("failed to apply TokenProcessorOption: invalid TokenProcessorOption")) Expect(tokenProcessor).To(Equal(tioidc.TokenProcessor{})) @@ -158,7 +165,7 @@ var _ = Describe("OIDC", func() { }) When("invalid raw token is provided", func() { It("should return an error", func() { - tokenProcessor, err := tioidc.NewTokenProcessor(logger, trustedIssuers, "invalidToken", verifierConfig) + tokenProcessor, err := tioidc.NewTokenProcessor(logger, trustedIssuers, "invalidToken") Expect(err).To(HaveOccurred()) Expect(err).To(MatchError("failed to get issuer from token: failed to parse oidc token: go-jose/go-jose: compact JWS format must have three parts")) Expect(tokenProcessor).To(Equal(tioidc.TokenProcessor{})) @@ -168,29 +175,16 @@ var _ = Describe("OIDC", func() { Describe("TokenProcessor", func() { var ( - Token oidcmocks.MockTokenInterface - claims tioidc.Claims - token tioidc.Token - mockToken oidcmocks.MockClaimsReader - tokenProcessor tioidc.TokenProcessor + Token oidcmocks.MockTokenInterface + claims tioidc.Claims + token tioidc.Token + mockToken oidcmocks.MockClaimsReader ) BeforeEach(func() { - verifierConfig, err = tioidc.NewVerifierConfig(logger, clientID) - Expect(err).NotTo(HaveOccurred()) - rawToken, err = os.ReadFile("test-fixtures/raw-oidc-token") Expect(err).NotTo(HaveOccurred()) - trustedIssuers = map[string]tioidc.Issuer{ - "https://fakedings.dev-gcp.nais.io/fake": { - Name: "github", - IssuerURL: "https://fakedings.dev-gcp.nais.io/fake", - JWKSURL: "https://fakedings.dev-gcp.nais.io/fake/jwks", - ExpectedJobWorkflowRef: "kyma-project/test-infra/.github/workflows/verify-oidc-token.yml@refs/heads/main", - }, - } - - tokenProcessor, err = tioidc.NewTokenProcessor(logger, trustedIssuers, string(rawToken), verifierConfig) + tokenProcessor, err = tioidc.NewTokenProcessor(logger, trustedIssuers, string(rawToken)) Expect(err).NotTo(HaveOccurred()) Expect(tokenProcessor).NotTo(BeNil()) @@ -280,7 +274,7 @@ var _ = Describe("OIDC", func() { Verifier: verifier, Logger: logger, } - verifierConfig, err = tioidc.NewVerifierConfig(logger, clientID) + // verifierConfig, err = tioidc.NewVerifierConfig(logger, clientID) Expect(err).NotTo(HaveOccurred()) ctx = context.Background() rawToken, err = os.ReadFile("test-fixtures/raw-oidc-token") @@ -443,16 +437,23 @@ var _ = Describe("OIDC", func() { Describe("Provider", func() { var ( - provider tioidc.Provider - oidcProvider *oidcmocks.MockVerifierProvider - verifierConfig tioidc.VerifierConfig + provider tioidc.Provider + oidcProvider *oidcmocks.MockVerifierProvider ) BeforeEach(func() { oidcProvider = &oidcmocks.MockVerifierProvider{} provider = tioidc.Provider{ VerifierProvider: oidcProvider, } - verifierConfig, err = tioidc.NewVerifierConfig(logger, clientID) + + rawToken, err = os.ReadFile("test-fixtures/raw-oidc-token") + Expect(err).NotTo(HaveOccurred()) + + tokenProcessor, err = tioidc.NewTokenProcessor(logger, trustedIssuers, string(rawToken)) + Expect(err).NotTo(HaveOccurred()) + Expect(tokenProcessor).NotTo(BeNil()) + + verifierConfig, err = tokenProcessor.NewVerifierConfig() Expect(err).NotTo(HaveOccurred()) }) Describe("NewVerifier", func() { @@ -465,4 +466,4 @@ var _ = Describe("OIDC", func() { }) }) }) -}) +}) \ No newline at end of file diff --git a/pkg/oidc/oidc_unit_test.go b/pkg/oidc/oidc_unit_test.go index a4b996cc4fca..9060ffb57f18 100644 --- a/pkg/oidc/oidc_unit_test.go +++ b/pkg/oidc/oidc_unit_test.go @@ -3,8 +3,11 @@ package oidc // oidc_unit_test.go contains tests which require access to non-exported functions and variables. import ( + "os" + . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" + "go.uber.org/zap" ) var _ = Describe("OIDC", func() { @@ -34,4 +37,41 @@ var _ = Describe("OIDC", func() { }) }) -}) + Describe("NewVerifierConfig", func() { + var ( + tokenProcessor TokenProcessor + err error + logger *zap.SugaredLogger + trustedIssuers map[string]Issuer + rawToken []byte + ) + + BeforeEach(func() { + rawToken, err = os.ReadFile("test-fixtures/raw-oidc-token") + Expect(err).NotTo(HaveOccurred()) + + trustedIssuers = map[string]Issuer{ + "https://fakedings.dev-gcp.nais.io/fake": { + Name: "github", + IssuerURL: "https://fakedings.dev-gcp.nais.io/fake", + JWKSURL: "https://fakedings.dev-gcp.nais.io/fake/jwks", + ExpectedJobWorkflowRef: "kyma-project/test-infra/.github/workflows/verify-oidc-token.yml@refs/heads/main", + ClientID: "testClientID", + }, + } + + tokenProcessor, err = NewTokenProcessor(logger, trustedIssuers, string(rawToken)) + Expect(err).NotTo(HaveOccurred()) + Expect(tokenProcessor).NotTo(BeNil()) + }) + + When("empty clientID is provided", func() { + It("should return an error", func() { + tokenProcessor.issuer.ClientID = "" + verifierConfig, err := tokenProcessor.NewVerifierConfig() + Expect(err).To(HaveOccurred()) + Expect(verifierConfig).To(Equal(VerifierConfig{})) + }) + }) + }) +}) \ No newline at end of file