Skip to content

Commit

Permalink
feat: Dissociate policy check from attestation check for VP (#1597)
Browse files Browse the repository at this point in the history 
Policy checks during presentation are now performed regardless of whether attestation checks are enabled, if the policy URL is configured. The policy URL is now configured against the policy check type.

Signed-off-by: Bob Stasyszyn <[email protected]>
  • Loading branch information
@bstasyszyn
bstasyszyn authored Feb 12, 2024
1 parent 8a09d1e commit fbd86f6
Show file tree
Hide file tree
Showing 14 changed files with 377 additions and 174 deletions.
16 changes: 8 additions & 8 deletions cmd/vc-rest/startcmd/start.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,6 @@ import (
oidc4vpv1 "github.com/trustbloc/vcs/pkg/restapi/v1/oidc4vp"
verifierv1 "github.com/trustbloc/vcs/pkg/restapi/v1/verifier"
"github.com/trustbloc/vcs/pkg/restapi/v1/version"
"github.com/trustbloc/vcs/pkg/service/clientattestation"
"github.com/trustbloc/vcs/pkg/service/clientidscheme"
clientmanagersvc "github.com/trustbloc/vcs/pkg/service/clientmanager"
credentialstatustypes "github.com/trustbloc/vcs/pkg/service/credentialstatus"
Expand All @@ -92,6 +91,7 @@ import (
"github.com/trustbloc/vcs/pkg/service/oidc4ci"
"github.com/trustbloc/vcs/pkg/service/oidc4vp"
"github.com/trustbloc/vcs/pkg/service/requestobject"
"github.com/trustbloc/vcs/pkg/service/trustregistry"
"github.com/trustbloc/vcs/pkg/service/verifycredential"
"github.com/trustbloc/vcs/pkg/service/verifypresentation"
wellknownfetcher "github.com/trustbloc/vcs/pkg/service/wellknown/fetcher"
Expand Down Expand Up @@ -687,8 +687,8 @@ func buildEchoHandler(

proofChecker := defaults.NewDefaultProofChecker(vermethod.NewVDRResolver(conf.VDR))

clientAttestationService := clientattestation.NewService(
&clientattestation.Config{
trustRegistryService := trustregistry.NewService(
&trustregistry.Config{
HTTPClient: getHTTPClient(metricsProvider.ClientAttestationService),
DocumentLoader: documentLoader,
ProofChecker: proofChecker,
Expand Down Expand Up @@ -718,7 +718,7 @@ func buildEchoHandler(
KMSRegistry: kmsRegistry,
CryptoJWTSigner: vcCrypto,
JSONSchemaValidator: jsonSchemaValidator,
ClientAttestationService: clientAttestationService,
TrustRegistryService: trustRegistryService,
AckService: ackService,
})
if err != nil {
Expand Down Expand Up @@ -862,10 +862,10 @@ func buildEchoHandler(
var verifyPresentationSvc verifypresentation.ServiceInterface

verifyPresentationSvc = verifypresentation.New(&verifypresentation.Config{
VcVerifier: verifyCredentialSvc,
DocumentLoader: documentLoader,
VDR: conf.VDR,
ClientAttestationService: clientAttestationService,
VcVerifier: verifyCredentialSvc,
DocumentLoader: documentLoader,
VDR: conf.VDR,
TrustRegistryService: trustRegistryService,
})

if conf.IsTraceEnabled {
Expand Down
9 changes: 8 additions & 1 deletion pkg/profile/api.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -160,17 +160,24 @@ type OIDC4VPConfig struct {
type VerificationChecks struct {
Credential CredentialChecks `json:"credential,omitempty"`
Presentation *PresentationChecks `json:"presentation,omitempty"`
Policy PolicyCheck `json:"policy,omitempty"`
ClientAttestationCheck ClientAttestationCheck `json:"clientAttestationCheck,omitempty"`
}

// IssuanceChecks are checks to be performed for issuance credentials and presentations.
type IssuanceChecks struct {
Policy PolicyCheck `json:"policy,omitempty"`
ClientAttestationCheck ClientAttestationCheck `json:"clientAttestationCheck,omitempty"`
}

// PolicyCheck stores policy check configuration.
type PolicyCheck struct {
PolicyURL string `json:"policyUrl"`
}

// ClientAttestationCheck stores Client Attestation check configuration.
type ClientAttestationCheck struct {
PolicyURL string `json:"policyUrl"`
Enabled bool `json:"enabled"`
}

// PresentationChecks are checks to be performed during presentation verification.
Expand Down
10 changes: 5 additions & 5 deletions pkg/service/oidc4ci/oidc4ci_service.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ Copyright SecureKey Technologies Inc. All Rights Reserved.
SPDX-License-Identifier: Apache-2.0
*/

//go:generate mockgen -destination oidc4ci_service_mocks_test.go -self_package mocks -package oidc4ci_test -source=oidc4ci_service.go -mock_names transactionStore=MockTransactionStore,wellKnownService=MockWellKnownService,eventService=MockEventService,pinGenerator=MockPinGenerator,credentialOfferReferenceStore=MockCredentialOfferReferenceStore,claimDataStore=MockClaimDataStore,profileService=MockProfileService,dataProtector=MockDataProtector,kmsRegistry=MockKMSRegistry,cryptoJWTSigner=MockCryptoJWTSigner,jsonSchemaValidator=MockJSONSchemaValidator,clientAttestationService=MockClientAttestationService,ackStore=MockAckStore,ackService=MockAckService
//go:generate mockgen -destination oidc4ci_service_mocks_test.go -self_package mocks -package oidc4ci_test -source=oidc4ci_service.go -mock_names transactionStore=MockTransactionStore,wellKnownService=MockWellKnownService,eventService=MockEventService,pinGenerator=MockPinGenerator,credentialOfferReferenceStore=MockCredentialOfferReferenceStore,claimDataStore=MockClaimDataStore,profileService=MockProfileService,dataProtector=MockDataProtector,kmsRegistry=MockKMSRegistry,cryptoJWTSigner=MockCryptoJWTSigner,jsonSchemaValidator=MockJSONSchemaValidator,trustRegistryService=MockTrustRegistryService,ackStore=MockAckStore,ackService=MockAckService

package oidc4ci

Expand Down Expand Up @@ -118,7 +118,7 @@ type jsonSchemaValidator interface {
Validate(data interface{}, schemaID string, schema []byte) error
}

type clientAttestationService interface {
type trustRegistryService interface {
ValidateIssuance(ctx context.Context, profile *profileapi.Issuer, jwtVP string) error
}

Expand Down Expand Up @@ -156,7 +156,7 @@ type Config struct {
KMSRegistry kmsRegistry
CryptoJWTSigner cryptoJWTSigner
JSONSchemaValidator jsonSchemaValidator
ClientAttestationService clientAttestationService
TrustRegistryService trustRegistryService
AckService ackService
}

Expand All @@ -177,7 +177,7 @@ type Service struct {
kmsRegistry kmsRegistry
cryptoJWTSigner cryptoJWTSigner
schemaValidator jsonSchemaValidator
clientAttestationService clientAttestationService
trustRegistryService trustRegistryService
ackService ackService
}

Expand All @@ -199,7 +199,7 @@ func NewService(config *Config) (*Service, error) {
kmsRegistry: config.KMSRegistry,
cryptoJWTSigner: config.CryptoJWTSigner,
schemaValidator: config.JSONSchemaValidator,
clientAttestationService: config.ClientAttestationService,
trustRegistryService: config.TrustRegistryService,
ackService: config.AckService,
}, nil
}
Expand Down
8 changes: 6 additions & 2 deletions pkg/service/oidc4ci/oidc4ci_service_authenticate_client.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,11 @@ func (s *Service) AuthenticateClient(
return nil
}

if profile.Checks.ClientAttestationCheck.PolicyURL == "" {
if !profile.Checks.ClientAttestationCheck.Enabled {
return errors.New("client attestation not set for profile") // this is profile configuration error
}

if profile.Checks.Policy.PolicyURL == "" {
return errors.New("policy url not set for profile") // this is profile configuration error
}

Expand All @@ -47,7 +51,7 @@ func (s *Service) AuthenticateClient(
errors.New("client_assertion is required"))
}

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

Expand Down
76 changes: 56 additions & 20 deletions pkg/service/oidc4ci/oidc4ci_service_authenticate_client_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,10 @@ const (

func TestService_AuthenticateClient(t *testing.T) {
var (
clientAttestationService *MockClientAttestationService
profile *profileapi.Issuer
clientAssertionType string
clientAssertion string
trustRegistryService *MockTrustRegistryService
profile *profileapi.Issuer
clientAssertionType string
clientAssertion string
)

tests := []struct {
Expand All @@ -44,18 +44,21 @@ func TestService_AuthenticateClient(t *testing.T) {
},
SigningDID: &profileapi.SigningDID{DID: issuerDID},
Checks: profileapi.IssuanceChecks{
ClientAttestationCheck: profileapi.ClientAttestationCheck{
Policy: profileapi.PolicyCheck{
PolicyURL: "https://policy.example.com",
},
ClientAttestationCheck: profileapi.ClientAttestationCheck{
Enabled: true,
},
},
}

clientAssertionType = "attest_jwt_client_auth"
clientAssertion = "client-attestation-jwt-vp"

clientAttestationService = NewMockClientAttestationService(gomock.NewController(t))
trustRegistryService = NewMockTrustRegistryService(gomock.NewController(t))

clientAttestationService.EXPECT().ValidateIssuance(
trustRegistryService.EXPECT().ValidateIssuance(
context.Background(),
profile,
clientAssertion,
Expand All @@ -77,7 +80,7 @@ func TestService_AuthenticateClient(t *testing.T) {
clientAssertionType = "attest_jwt_client_auth"
clientAssertion = "client-attestation-jwt-vp"

clientAttestationService = NewMockClientAttestationService(gomock.NewController(t))
trustRegistryService = NewMockTrustRegistryService(gomock.NewController(t))
},
check: func(t *testing.T, err error) {
require.NoError(t, err)
Expand All @@ -91,19 +94,40 @@ func TestService_AuthenticateClient(t *testing.T) {
TokenEndpointAuthMethodsSupported: []string{"attest_jwt_client_auth"},
},
Checks: profileapi.IssuanceChecks{
ClientAttestationCheck: profileapi.ClientAttestationCheck{},
ClientAttestationCheck: profileapi.ClientAttestationCheck{
Enabled: true,
},
},
}

clientAssertionType = "attest_jwt_client_auth"
clientAssertion = "client-attestation-jwt-vp"

clientAttestationService = NewMockClientAttestationService(gomock.NewController(t))
trustRegistryService = NewMockTrustRegistryService(gomock.NewController(t))
},
check: func(t *testing.T, err error) {
require.ErrorContains(t, err, "policy url not set for profile")
},
},
{
name: "client attestation not set for profile",
setup: func() {
profile = &profileapi.Issuer{
OIDCConfig: &profileapi.OIDCConfig{
TokenEndpointAuthMethodsSupported: []string{"attest_jwt_client_auth"},
},
Checks: profileapi.IssuanceChecks{},
}

clientAssertionType = "attest_jwt_client_auth"
clientAssertion = "client-attestation-jwt-vp"

trustRegistryService = NewMockTrustRegistryService(gomock.NewController(t))
},
check: func(t *testing.T, err error) {
require.ErrorContains(t, err, "client attestation not set for profile")
},
},
{
name: "invalid client assertion type",
setup: func() {
Expand All @@ -113,16 +137,19 @@ func TestService_AuthenticateClient(t *testing.T) {
},

Checks: profileapi.IssuanceChecks{
ClientAttestationCheck: profileapi.ClientAttestationCheck{
Policy: profileapi.PolicyCheck{
PolicyURL: "https://policy.example.com",
},
ClientAttestationCheck: profileapi.ClientAttestationCheck{
Enabled: true,
},
},
}

clientAssertionType = "not_supported_client_assertion_type"
clientAssertion = "client-attestation-jwt-vp"

clientAttestationService = NewMockClientAttestationService(gomock.NewController(t))
trustRegistryService = NewMockTrustRegistryService(gomock.NewController(t))
},
check: func(t *testing.T, err error) {
require.ErrorContains(t, err, "only supported client assertion type is attest_jwt_client_auth")
Expand All @@ -136,16 +163,19 @@ func TestService_AuthenticateClient(t *testing.T) {
TokenEndpointAuthMethodsSupported: []string{"attest_jwt_client_auth"},
},
Checks: profileapi.IssuanceChecks{
ClientAttestationCheck: profileapi.ClientAttestationCheck{
Policy: profileapi.PolicyCheck{
PolicyURL: "https://policy.example.com",
},
ClientAttestationCheck: profileapi.ClientAttestationCheck{
Enabled: true,
},
},
}

clientAssertionType = ""
clientAssertion = "client-attestation-jwt-vp"

clientAttestationService = NewMockClientAttestationService(gomock.NewController(t))
trustRegistryService = NewMockTrustRegistryService(gomock.NewController(t))
},
check: func(t *testing.T, err error) {
require.ErrorContains(t, err, "no client assertion type specified")
Expand All @@ -159,16 +189,19 @@ func TestService_AuthenticateClient(t *testing.T) {
TokenEndpointAuthMethodsSupported: []string{"attest_jwt_client_auth"},
},
Checks: profileapi.IssuanceChecks{
ClientAttestationCheck: profileapi.ClientAttestationCheck{
Policy: profileapi.PolicyCheck{
PolicyURL: "https://policy.example.com",
},
ClientAttestationCheck: profileapi.ClientAttestationCheck{
Enabled: true,
},
},
}

clientAssertionType = "attest_jwt_client_auth"
clientAssertion = ""

clientAttestationService = NewMockClientAttestationService(gomock.NewController(t))
trustRegistryService = NewMockTrustRegistryService(gomock.NewController(t))
},
check: func(t *testing.T, err error) {
require.ErrorContains(t, err, "client_assertion is required")
Expand All @@ -183,18 +216,21 @@ func TestService_AuthenticateClient(t *testing.T) {
TokenEndpointAuthMethodsSupported: []string{"attest_jwt_client_auth"},
},
Checks: profileapi.IssuanceChecks{
ClientAttestationCheck: profileapi.ClientAttestationCheck{
Policy: profileapi.PolicyCheck{
PolicyURL: "https://policy.example.com",
},
ClientAttestationCheck: profileapi.ClientAttestationCheck{
Enabled: true,
},
},
}

clientAssertionType = "attest_jwt_client_auth"
clientAssertion = "client-attestation-jwt-vp"

clientAttestationService = NewMockClientAttestationService(gomock.NewController(t))
trustRegistryService = NewMockTrustRegistryService(gomock.NewController(t))

clientAttestationService.EXPECT().ValidateIssuance(
trustRegistryService.EXPECT().ValidateIssuance(
context.Background(),
profile,
clientAssertion,
Expand All @@ -210,7 +246,7 @@ func TestService_AuthenticateClient(t *testing.T) {
tt.setup()

svc, err := oidc4ci.NewService(&oidc4ci.Config{
ClientAttestationService: clientAttestationService,
TrustRegistryService: trustRegistryService,
})
require.NoError(t, err)

Expand Down
17 changes: 10 additions & 7 deletions pkg/service/oidc4ci/oidc4ci_service_exchange_code_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -142,14 +142,14 @@ func TestExchangeCodeAuthenticateClientError(t *testing.T) {
store := NewMockTransactionStore(gomock.NewController(t))
eventMock := NewMockEventService(gomock.NewController(t))
profileService := NewMockProfileService(gomock.NewController(t))
clientAttestationService := NewMockClientAttestationService(gomock.NewController(t))
trustRegistryService := NewMockTrustRegistryService(gomock.NewController(t))

svc, err := oidc4ci.NewService(&oidc4ci.Config{
TransactionStore: store,
ProfileService: profileService,
ClientAttestationService: clientAttestationService,
EventService: eventMock,
EventTopic: spi.IssuerEventTopic,
TransactionStore: store,
ProfileService: profileService,
TrustRegistryService: trustRegistryService,
EventService: eventMock,
EventTopic: spi.IssuerEventTopic,
})
assert.NoError(t, err)

Expand All @@ -165,9 +165,12 @@ func TestExchangeCodeAuthenticateClientError(t *testing.T) {
TokenEndpointAuthMethodsSupported: []string{"attest_jwt_client_auth"},
},
Checks: profile.IssuanceChecks{
ClientAttestationCheck: profile.ClientAttestationCheck{
Policy: profile.PolicyCheck{
PolicyURL: "https://localhost/policy",
},
ClientAttestationCheck: profile.ClientAttestationCheck{
Enabled: true,
},
},
}, nil)

Expand Down
Loading

0 comments on commit fbd86f6

Please sign in to comment.