diff --git a/auth/api/iam/openid4vp.go b/auth/api/iam/openid4vp.go index 405a52e7b..53868ed60 100644 --- a/auth/api/iam/openid4vp.go +++ b/auth/api/iam/openid4vp.go @@ -23,7 +23,11 @@ import ( "encoding/json" "errors" "fmt" + "github.com/nuts-foundation/nuts-node/http/user" + "github.com/nuts-foundation/nuts-node/vcr/types" + "github.com/nuts-foundation/nuts-node/vcr/verifier" + "net/http" "net/url" "slices" @@ -536,7 +540,7 @@ func (r Wrapper) handleAuthorizeResponseSubmission(ctx context.Context, request if err != nil { return nil, oauth.OAuth2Error{ Code: oauth.InvalidRequest, - Description: "presentation(s) or contained credential(s) are invalid", + Description: verificationErrorDescription(err), InternalError: err, RedirectURI: callbackURI, } @@ -764,3 +768,20 @@ func oauthError(code oauth.ErrorCode, description string, internalError ...error InternalError: errors.Join(internalError...), } } + +// verificationErrorDescription returns a more specific error description when DID resolution fails, +// otherwise returns the generic error message. This improves user experience by providing actionable +// error information for common DID resolution issues while maintaining security for other errors. +func verificationErrorDescription(err error) string { + // Check for DID resolution errors + if errors.Is(err, verifier.VerificationError{}) { + return err.Error() + } + if errors.Is(err, resolver.ErrNotFound) || errors.Is(err, resolver.ErrKeyNotFound) || strings.Contains(err.Error(), "unable to resolve") || + errors.Is(err, types.ErrStatusNotFound) || errors.Is(err, types.ErrRevoked) || errors.Is(err, types.ErrCredentialNotValidAtTime) || errors.Is(err, types.ErrPresentationNotValidAtTime) { + return verifier.ToVerificationError(err).Error() + } + + // Default generic message for other errors + return "presentation(s) or credential(s) verification failed" +} diff --git a/auth/api/iam/openid4vp_test.go b/auth/api/iam/openid4vp_test.go index f57ff74a7..bcc4137bd 100644 --- a/auth/api/iam/openid4vp_test.go +++ b/auth/api/iam/openid4vp_test.go @@ -21,12 +21,16 @@ package iam import ( "context" "encoding/json" - "github.com/nuts-foundation/nuts-node/http/user" + "errors" + "fmt" "net/http" "net/url" "strings" "testing" + "github.com/nuts-foundation/nuts-node/http/user" + "github.com/nuts-foundation/nuts-node/vcr/verifier" + "github.com/lestrrat-go/jwx/v2/jwt" "github.com/nuts-foundation/go-did/did" "github.com/nuts-foundation/go-did/vc" @@ -35,6 +39,7 @@ import ( "github.com/nuts-foundation/nuts-node/storage" "github.com/nuts-foundation/nuts-node/test" "github.com/nuts-foundation/nuts-node/vcr/pe" + "github.com/nuts-foundation/nuts-node/vdr/resolver" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "go.uber.org/mock/gomock" @@ -461,7 +466,7 @@ func TestWrapper_HandleAuthorizeResponse(t *testing.T) { _, err := ctx.client.HandleAuthorizeResponse(context.Background(), baseRequest()) - oauthErr := assertOAuthError(t, err, "presentation(s) or contained credential(s) are invalid") + oauthErr := assertOAuthError(t, err, "presentation(s) or credential(s) verification failed") assert.Equal(t, "https://example.com/iam/holder/cb", oauthErr.RedirectURI.String()) }) t.Run("expired session", func(t *testing.T) { @@ -964,5 +969,38 @@ func putNonce(ctx *testCtx, nonce string) { func putCodeSession(ctx *testCtx, code string, oauthSession OAuthSession) { _ = ctx.client.oauthCodeStore().Put(code, oauthSession) +} + +func Test_verificationErrorDescription(t *testing.T) { + t.Run("DID not found error", func(t *testing.T) { + err := resolver.ErrNotFound + result := verificationErrorDescription(err) + assert.Equal(t, "presentation(s) or credential(s) verification failed: unable to find the DID document", result) + }) + + t.Run("Key not found error", func(t *testing.T) { + err := resolver.ErrKeyNotFound + result := verificationErrorDescription(err) + assert.Equal(t, "presentation(s) or credential(s) verification failed: key not found in DID document", result) + }) + + t.Run("Wrapped DID resolution error", func(t *testing.T) { + err := fmt.Errorf("unable to resolve valid signing key: %w", resolver.ErrNotFound) + result := verificationErrorDescription(err) + assert.Equal(t, "presentation(s) or credential(s) verification failed: unable to resolve valid signing key: unable to find the DID document", result) + }) + t.Run("Generic error returns default message", func(t *testing.T) { + err := errors.New("some other error") + result := verificationErrorDescription(err) + assert.Equal(t, "presentation(s) or credential(s) verification failed", result) + }) + + t.Run("VerificationError", func(t *testing.T) { + err := verifier.VerificationError{ + Msg: "some verification error", + } + result := verificationErrorDescription(err) + assert.Equal(t, "presentation(s) or credential(s) verification failed: some verification error", result) + }) } diff --git a/auth/api/iam/s2s_vptoken.go b/auth/api/iam/s2s_vptoken.go index 16018dbc3..c215ea426 100644 --- a/auth/api/iam/s2s_vptoken.go +++ b/auth/api/iam/s2s_vptoken.go @@ -101,7 +101,7 @@ func (r Wrapper) handleS2SAccessTokenRequest(ctx context.Context, clientID strin if err != nil { return nil, oauth.OAuth2Error{ Code: oauth.InvalidRequest, - Description: "presentation(s) or contained credential(s) are invalid", + Description: verificationErrorDescription(err), InternalError: err, } } diff --git a/auth/api/iam/s2s_vptoken_test.go b/auth/api/iam/s2s_vptoken_test.go index f0b3f177e..7cc4504b7 100644 --- a/auth/api/iam/s2s_vptoken_test.go +++ b/auth/api/iam/s2s_vptoken_test.go @@ -301,7 +301,7 @@ func TestWrapper_handleS2SAccessTokenRequest(t *testing.T) { resp, err := ctx.client.handleS2SAccessTokenRequest(contextWithValue, clientID, issuerSubjectID, requestedScope, submissionJSON, presentation.Raw()) - assert.EqualError(t, err, "invalid_request - invalid - presentation(s) or contained credential(s) are invalid") + assert.EqualError(t, err, "invalid_request - invalid - presentation(s) or credential(s) verification failed") assert.Nil(t, resp) }) t.Run("proof of ownership", func(t *testing.T) { diff --git a/auth/services/selfsigned/validator_test.go b/auth/services/selfsigned/validator_test.go index 26ac1862e..2a4995706 100644 --- a/auth/services/selfsigned/validator_test.go +++ b/auth/services/selfsigned/validator_test.go @@ -22,6 +22,10 @@ import ( "context" "encoding/json" "errors" + "os" + "testing" + "time" + "github.com/nuts-foundation/nuts-node/audit" "github.com/nuts-foundation/nuts-node/auth/services" "github.com/nuts-foundation/nuts-node/auth/services/selfsigned/types" @@ -33,9 +37,6 @@ import ( "github.com/nuts-foundation/nuts-node/vcr/verifier" "github.com/nuts-foundation/nuts-node/vdr/didnuts/didstore" "go.uber.org/mock/gomock" - "os" - "testing" - "time" ssi "github.com/nuts-foundation/go-did" "github.com/nuts-foundation/go-did/did" @@ -176,7 +177,7 @@ func TestValidator_VerifyVP(t *testing.T) { require.NoError(t, err) assert.Equal(t, contract.Invalid, result.Validity()) - assert.Equal(t, "verification error: ", result.Reason()) + assert.Equal(t, "presentation(s) or credential(s) verification failed", result.Reason()) }) t.Run("ok using in-memory DBs", func(t *testing.T) { diff --git a/vcr/api/vcr/v2/api_test.go b/vcr/api/vcr/v2/api_test.go index 510978cae..b3e8b0285 100644 --- a/vcr/api/vcr/v2/api_test.go +++ b/vcr/api/vcr/v2/api_test.go @@ -1160,15 +1160,15 @@ func TestWrapper_VerifyVP(t *testing.T) { testContext := newMockContext(t) request := VPVerificationRequest{VerifiablePresentation: vp} testContext.mockVerifier.EXPECT().VerifyVP(vp, true, false, nil).Return(nil, verifier.VerificationError{}) - errMsg := "verification error: " - expectedRepsonse := VerifyVP200JSONResponse(VPVerificationResult{ + errMsg := "presentation(s) or credential(s) verification failed" + expectedResponse := VerifyVP200JSONResponse(VPVerificationResult{ Message: &errMsg, Validity: false, }) response, err := testContext.client.VerifyVP(testContext.requestCtx, VerifyVPRequestObject{Body: &request}) - assert.Equal(t, expectedRepsonse, response) + assert.Equal(t, expectedResponse, response) assert.NoError(t, err) }) } diff --git a/vcr/verifier/signature_verifier.go b/vcr/verifier/signature_verifier.go index 75ff0334f..3ff787656 100644 --- a/vcr/verifier/signature_verifier.go +++ b/vcr/verifier/signature_verifier.go @@ -59,7 +59,7 @@ func (sv *signatureVerifier) VerifySignature(credentialToVerify vc.VerifiableCre func (sv *signatureVerifier) VerifyVPSignature(presentation vc.VerifiablePresentation, validateAt *time.Time) error { signerDID, err := credential.PresentationSigner(presentation) if err != nil { - return toVerificationError(err) + return ToVerificationError(err) } switch presentation.Format() { @@ -100,7 +100,7 @@ func (sv *signatureVerifier) jsonldProof(documentToVerify any, issuer string, at validAt = *at } if !ldProof.ValidAt(validAt, maxSkew) { - return toVerificationError(types.ErrPresentationNotValidAtTime) + return ToVerificationError(types.ErrPresentationNotValidAtTime) } // find key @@ -132,7 +132,11 @@ func (sv *signatureVerifier) jwtSignature(jwtDocumentToVerify string, issuer str return nil, err } metadata.JwtProtectedHeaders = headers - return sv.resolveSigningKey(kid, issuer, metadata) + key, err := sv.resolveSigningKey(kid, issuer, metadata) + if err != nil { + return nil, fmt.Errorf("unable to resolve signing key: %w", err) + } + return key, err }, jwt.WithClock(jwt.ClockFunc(func() time.Time { if at == nil { return time.Now() @@ -140,7 +144,7 @@ func (sv *signatureVerifier) jwtSignature(jwtDocumentToVerify string, issuer str return *at }))) if err != nil { - return fmt.Errorf("unable to validate JWT signature: %w", err) + return newVerificationError("unable to validate JWT signature: %w", err) } if keyID != "" && strings.Split(keyID, "#")[0] != issuer { return errVerificationMethodNotOfIssuer diff --git a/vcr/verifier/signature_verifier_test.go b/vcr/verifier/signature_verifier_test.go index 3342a6519..38805b38a 100644 --- a/vcr/verifier/signature_verifier_test.go +++ b/vcr/verifier/signature_verifier_test.go @@ -31,14 +31,15 @@ import ( "encoding/base64" "encoding/json" "errors" + "os" + "testing" + "time" + "github.com/google/uuid" "github.com/lestrrat-go/jwx/v2/cert" "github.com/lestrrat-go/jwx/v2/jwa" testpki "github.com/nuts-foundation/nuts-node/test/pki" "github.com/nuts-foundation/nuts-node/vdr/didx509" - "os" - "testing" - "time" "github.com/lestrrat-go/jwx/v2/jwk" ssi "github.com/nuts-foundation/go-did" @@ -111,7 +112,7 @@ func TestSignatureVerifier_VerifySignature(t *testing.T) { } sv := x509VerifierTestSetup(t) err = sv.VerifySignature(*cred, nil) - assert.ErrorIs(t, err, expectedError) + assert.EqualError(t, err, "presentation(s) or credential(s) verification failed: unable to validate JWT signature: failing ExtractProtectedHeaders") }) }) t.Run("JWT", func(t *testing.T) { @@ -160,7 +161,7 @@ func TestSignatureVerifier_VerifySignature(t *testing.T) { err = sv.VerifySignature(*cred, nil) - assert.EqualError(t, err, "unable to validate JWT signature: could not verify message using any of the signatures or keys") + assert.EqualError(t, err, "presentation(s) or credential(s) verification failed: unable to validate JWT signature: could not verify message using any of the signatures or keys") }) t.Run("expired token", func(t *testing.T) { // Credential taken from Sphereon Wallet, expires on Tue Oct 03 2023 @@ -174,7 +175,7 @@ func TestSignatureVerifier_VerifySignature(t *testing.T) { } err := sv.VerifySignature(*cred, nil) - assert.EqualError(t, err, "unable to validate JWT signature: \"exp\" not satisfied") + assert.EqualError(t, err, "presentation(s) or credential(s) verification failed: unable to validate JWT signature: \"exp\" not satisfied") }) t.Run("without kid header, derived from issuer", func(t *testing.T) { // Credential taken from Sphereon Wallet @@ -203,7 +204,7 @@ func TestSignatureVerifier_VerifySignature(t *testing.T) { } err := sv.VerifySignature(*cred, nil) - assert.EqualError(t, err, "unable to validate JWT signature: could not verify message using any of the signatures or keys") + assert.EqualError(t, err, "presentation(s) or credential(s) verification failed: unable to validate JWT signature: could not verify message using any of the signatures or keys") }) }) @@ -257,7 +258,7 @@ func TestSignatureVerifier_VerifySignature(t *testing.T) { err := sv.VerifySignature(vc2, nil) - assert.EqualError(t, err, "verification error: missing proof") + assert.EqualError(t, err, "presentation(s) or credential(s) verification failed: missing proof") }) t.Run("error - wrong jws in proof", func(t *testing.T) { diff --git a/vcr/verifier/verifier.go b/vcr/verifier/verifier.go index 8cf4ba732..8164b373f 100644 --- a/vcr/verifier/verifier.go +++ b/vcr/verifier/verifier.go @@ -22,11 +22,12 @@ import ( "encoding/json" "errors" "fmt" - "github.com/nuts-foundation/nuts-node/pki" - "github.com/nuts-foundation/nuts-node/vcr/revocation" "strings" "time" + "github.com/nuts-foundation/nuts-node/pki" + "github.com/nuts-foundation/nuts-node/vcr/revocation" + ssi "github.com/nuts-foundation/go-did" "github.com/nuts-foundation/go-did/did" "github.com/nuts-foundation/go-did/vc" @@ -44,7 +45,7 @@ const ( maxSkew = 5 * time.Second ) -var errVerificationMethodNotOfIssuer = errors.New("verification method is not of issuer") +var errVerificationMethodNotOfIssuer = newVerificationError("verification method is not of issuer") // verifier implements the Verifier interface. // It implements the generic methods for verifying verifiable credentials and verifiable presentations. @@ -62,8 +63,8 @@ type verifier struct { // VerificationError is used to describe a VC/VP verification failure. type VerificationError struct { - msg string - args []interface{} + Msg string + Args []interface{} } // Is checks whether the given error is a VerificationError as well. @@ -73,15 +74,19 @@ func (e VerificationError) Is(other error) bool { } func newVerificationError(msg string, args ...interface{}) error { - return VerificationError{msg: msg, args: args} + return VerificationError{Msg: msg, Args: args} } -func toVerificationError(cause error) error { - return VerificationError{msg: cause.Error()} +func ToVerificationError(cause error) error { + return VerificationError{Msg: cause.Error()} } func (e VerificationError) Error() string { - return fmt.Errorf("verification error: "+e.msg, e.args...).Error() + const msg = "presentation(s) or credential(s) verification failed" + if e.Msg == "" { + return msg + } + return fmt.Errorf(msg+": "+e.Msg, e.Args...).Error() } // NewVerifier creates a new instance of the verifier. It needs a key resolver for validating signatures. diff --git a/vcr/verifier/verifier_test.go b/vcr/verifier/verifier_test.go index 8c60c4b75..ab9d36520 100644 --- a/vcr/verifier/verifier_test.go +++ b/vcr/verifier/verifier_test.go @@ -23,8 +23,6 @@ import ( "crypto" "encoding/json" "errors" - "github.com/nuts-foundation/nuts-node/storage/orm" - "github.com/nuts-foundation/nuts-node/test/pki" "net/http" "net/http/httptest" "os" @@ -33,6 +31,9 @@ import ( "testing" "time" + "github.com/nuts-foundation/nuts-node/storage/orm" + "github.com/nuts-foundation/nuts-node/test/pki" + "github.com/lestrrat-go/jwx/v2/jwk" "github.com/lestrrat-go/jwx/v2/jwt" ssi "github.com/nuts-foundation/go-did" @@ -547,7 +548,7 @@ func TestVerifier_VerifyVP(t *testing.T) { vcs, err := ctx.verifier.doVerifyVP(mockVerifier, presentationWithHolder, true, true, nil) - assert.EqualError(t, err, "verification error: presentation holder must equal credential subject") + assert.EqualError(t, err, "presentation(s) or credential(s) verification failed: presentation holder must equal credential subject") assert.Empty(t, vcs) }) t.Run("JWT expired", func(t *testing.T) { @@ -557,7 +558,7 @@ func TestVerifier_VerifyVP(t *testing.T) { validAt := time.Date(2023, 10, 21, 12, 0, 0, 0, time.UTC) vcs, err := ctx.verifier.VerifyVP(*presentation, false, false, &validAt) - assert.EqualError(t, err, "unable to validate JWT signature: \"exp\" not satisfied") + assert.EqualError(t, err, "presentation(s) or credential(s) verification failed: unable to validate JWT signature: \"exp\" not satisfied") assert.Empty(t, vcs) }) t.Run("VP signer != VC credentialSubject.id", func(t *testing.T) { @@ -579,7 +580,7 @@ func TestVerifier_VerifyVP(t *testing.T) { vcs, err := ctx.verifier.VerifyVP(*presentation, false, false, nil) - assert.EqualError(t, err, "verification error: credential(s) must be presented by subject") + assert.EqualError(t, err, "presentation(s) or credential(s) verification failed: credential(s) must be presented by subject") assert.Empty(t, vcs) }) }) @@ -688,7 +689,7 @@ func TestVerifier_VerifyVP(t *testing.T) { vcs, err := ctx.verifier.doVerifyVP(mockVerifier, vp, true, true, &validAt) - assert.EqualError(t, err, "verification error: presentation not valid at given time") + assert.EqualError(t, err, "presentation(s) or credential(s) verification failed: presentation not valid at given time") assert.Empty(t, vcs) }) t.Run("error - VC verification fails", func(t *testing.T) { @@ -720,7 +721,7 @@ func TestVerifier_VerifyVP(t *testing.T) { vcs, err := ctx.verifier.VerifyVP(vp, false, false, validAt) - assert.EqualError(t, err, "verification error: invalid signature: invalid proof signature: failed to verify signature using ecdsa") + assert.EqualError(t, err, "presentation(s) or credential(s) verification failed: invalid signature: invalid proof signature: failed to verify signature using ecdsa") assert.Empty(t, vcs) }) t.Run("error - signing key unknown", func(t *testing.T) { @@ -749,7 +750,7 @@ func TestVerifier_VerifyVP(t *testing.T) { vcs, err := ctx.verifier.VerifyVP(vp, false, false, validAt) - assert.EqualError(t, err, "verification error: presenter is credential subject: invalid LD-proof for presentation: json: cannot unmarshal string into Go value of type proof.LDProof") + assert.EqualError(t, err, "presentation(s) or credential(s) verification failed: presenter is credential subject: invalid LD-proof for presentation: json: cannot unmarshal string into Go value of type proof.LDProof") assert.Empty(t, vcs) }) t.Run("error - no proof", func(t *testing.T) { @@ -763,7 +764,7 @@ func TestVerifier_VerifyVP(t *testing.T) { vcs, err := ctx.verifier.VerifyVP(vp, false, false, validAt) - assert.EqualError(t, err, "verification error: presenter is credential subject: presentation should have exactly 1 proof, got 0") + assert.EqualError(t, err, "presentation(s) or credential(s) verification failed: presenter is credential subject: presentation should have exactly 1 proof, got 0") assert.Empty(t, vcs) }) })