Skip to content

Commit

Permalink
feat: Check issuer policies regardless of attestation config (#1598)
Browse files Browse the repository at this point in the history
Signed-off-by: Bob Stasyszyn <[email protected]>
  • Loading branch information
bstasyszyn authored Feb 15, 2024
1 parent fbd86f6 commit 600ed42
Show file tree
Hide file tree
Showing 4 changed files with 52 additions and 13 deletions.
2 changes: 1 addition & 1 deletion pkg/service/oidc4ci/oidc4ci_service.go
Original file line number Diff line number Diff line change
Expand Up @@ -439,7 +439,7 @@ func (s *Service) ValidatePreAuthorizedCodeRequest( //nolint:gocognit,nolintlint
}
}

if err = s.AuthenticateClient(ctx, profile, clientAssertionType, clientAssertion); err != nil {
if err = s.CheckPolicies(ctx, profile, clientAssertionType, clientAssertion); err != nil {
return nil, resterr.NewCustomError(resterr.OIDCClientAuthenticationFailed, err)
}

Expand Down
34 changes: 25 additions & 9 deletions pkg/service/oidc4ci/oidc4ci_service_authenticate_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,25 @@ import (

const attestJWTClientAuthType = "attest_jwt_client_auth"

func (s *Service) AuthenticateClient(
func (s *Service) CheckPolicies(
ctx context.Context,
profile *profile.Issuer,
clientAssertionType,
clientAssertion string) error {
if err := s.validateClientAssertionConfig(profile, clientAssertionType, clientAssertion); err != nil {
return err
}

if profile.Checks.Policy.PolicyURL != "" {
if err := s.trustRegistryService.ValidateIssuance(ctx, profile, clientAssertion); err != nil {
return resterr.NewCustomError(resterr.OIDCClientAuthenticationFailed, err)
}
}

return nil
}

func (s *Service) validateClientAssertionConfig(
profile *profile.Issuer,
clientAssertionType,
clientAssertion string) error {
Expand All @@ -28,12 +45,15 @@ func (s *Service) AuthenticateClient(
return nil
}

if !profile.Checks.ClientAttestationCheck.Enabled {
return errors.New("client attestation not set for profile") // this is profile configuration error
if profile.Checks.Policy.PolicyURL == "" {
// This is a profile configuration error
return errors.New("client attestation is required but policy url not set for profile")
}

if profile.Checks.Policy.PolicyURL == "" {
return errors.New("policy url not set for profile") // this is profile configuration error
// TODO: This check should be removed.
if !profile.Checks.ClientAttestationCheck.Enabled {
// This is a profile configuration error
return errors.New("client attestation check not set for profile")
}

if clientAssertionType == "" {
Expand All @@ -51,9 +71,5 @@ func (s *Service) AuthenticateClient(
errors.New("client_assertion is required"))
}

if err := s.trustRegistryService.ValidateIssuance(ctx, profile, clientAssertion); err != nil {
return resterr.NewCustomError(resterr.OIDCClientAuthenticationFailed, err)
}

return nil
}
27 changes: 25 additions & 2 deletions pkg/service/oidc4ci/oidc4ci_service_authenticate_client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ func TestService_AuthenticateClient(t *testing.T) {
trustRegistryService = NewMockTrustRegistryService(gomock.NewController(t))
},
check: func(t *testing.T, err error) {
require.ErrorContains(t, err, "client attestation not set for profile")
require.ErrorContains(t, err, "client attestation is required but policy url not set for profile")
},
},
{
Expand Down Expand Up @@ -240,6 +240,29 @@ func TestService_AuthenticateClient(t *testing.T) {
require.ErrorContains(t, err, "validate error")
},
},
{
name: "client attestation check not set",
setup: func() {
profile = &profileapi.Issuer{
OIDCConfig: &profileapi.OIDCConfig{
TokenEndpointAuthMethodsSupported: []string{"attest_jwt_client_auth"},
},
Checks: profileapi.IssuanceChecks{
Policy: profileapi.PolicyCheck{
PolicyURL: "https://policy.example.com",
},
},
}

clientAssertionType = "attest_jwt_client_auth"
clientAssertion = ""

trustRegistryService = NewMockTrustRegistryService(gomock.NewController(t))
},
check: func(t *testing.T, err error) {
require.ErrorContains(t, err, "client attestation check not set for profile")
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
Expand All @@ -250,7 +273,7 @@ func TestService_AuthenticateClient(t *testing.T) {
})
require.NoError(t, err)

err = svc.AuthenticateClient(context.Background(), profile, clientAssertionType, clientAssertion)
err = svc.CheckPolicies(context.Background(), profile, clientAssertionType, clientAssertion)
tt.check(t, err)
})
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/service/oidc4ci/oidc4ci_service_exchange_code.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ func (s *Service) ExchangeAuthorizationCode(
return "", e
}

if err = s.AuthenticateClient(ctx, profile, clientAssertionType, clientAssertion); err != nil {
if err = s.CheckPolicies(ctx, profile, clientAssertionType, clientAssertion); err != nil {
s.sendFailedTransactionEvent(ctx, tx, err)
return "", resterr.NewCustomError(resterr.OIDCClientAuthenticationFailed, err)
}
Expand Down

0 comments on commit 600ed42

Please sign in to comment.