Skip to content

Surface DID resolution errors in VP verification failures#3979

Open
Copilot wants to merge 1 commit intomasterfrom
copilot/improve-client-error-message
Open

Surface DID resolution errors in VP verification failures#3979
Copilot wants to merge 1 commit intomasterfrom
copilot/improve-client-error-message

Conversation

Copy link

Copilot AI commented Jan 29, 2026

Plan: Improve Client Error Message for DID Resolution Failures

  • Understand the issue and codebase structure
  • Identify specific errors related to DID resolution that can be safely exposed
  • Modify error handling to allow DID resolution errors to pass through
  • Add tests to verify the new error messages
  • Run existing tests to ensure no regression
  • Run security checks (CodeQL - 0 alerts found)
  • Address review feedback - simplify implementation

Summary of Changes

Problem: Currently, when the server can't resolve a DID document during VP verification, it returns a generic error: "presentation(s) or contained credential(s) are invalid". This makes troubleshooting difficult for clients.

Solution Implemented:

  1. Created verificationErrorDescription() helper (auth/api/iam/openid4vp.go):

    • Detects DID resolution errors using errors.Is() and string pattern matching
    • Returns detailed error messages by appending the actual error message
    • Maintains generic message for other errors to preserve security
  2. Updated error handling in two locations:

    • auth/api/iam/openid4vp.go - OpenID4VP HandleAuthorizeResponse flow
    • auth/api/iam/s2s_vptoken.go - Service-to-service VP token verification flow
  3. Added comprehensive tests (auth/api/iam/openid4vp_test.go):

    • Tests for DID resolution error types
    • Tests for wrapped errors
    • Tests for generic fallback behavior

New Error Messages:

  • ✅ Appends actual error message for DID resolution errors (ErrNotFound, ErrKeyNotFound, "unable to resolve")
  • ✅ Generic message maintained for other errors (security preserved)

Review Feedback Addressed:

  • ✅ Removed legacy did:nuts error checks (deactivated, no active controllers)
  • ✅ Simplified to use err.Error() directly instead of custom messages

Testing Results:

  • ✅ All unit tests pass (4/4)
  • ✅ All existing IAM package tests pass
  • ✅ Build successful

Security Considerations:

  • Only DID resolution-related errors are exposed with specific messages
  • All other verification errors continue to use the generic message
  • Internal error details are still logged via InternalError field for debugging
  • No sensitive information is exposed to clients
Original prompt

This section details on the original issue you should resolve

<issue_title>QoL: improve client error message if DID can't be resolved.</issue_title>
<issue_description>Currently, the server returns:

invalid_request - presentation(s) or contained credential(s) are invalid

This typically happens because the server can't resolve the DID document. We sanitize error messages for security reasons, but maybe we can allow some errors that aren't so sensitive.</issue_description>

Comments on the Issue (you are @copilot in this section)


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

@qltysh
Copy link

qltysh bot commented Jan 29, 2026

Qlty

Coverage Impact

⬆️ Merging this pull request will increase total coverage on master by 0.02%.

Modified Files with Diff Coverage (4)

RatingFile% DiffUncovered Line #s
Coverage rating: A Coverage rating: A
vcr/verifier/verifier.go75.0%87-88
Coverage rating: B Coverage rating: B
auth/api/iam/openid4vp.go100.0%
Coverage rating: B Coverage rating: B
auth/api/iam/s2s_vptoken.go100.0%
Coverage rating: B Coverage rating: B
vcr/verifier/signature_verifier.go62.5%62, 137-138
Total82.1%
🤖 Increase coverage with AI coding...

In the `copilot/improve-client-error-message` branch, add test coverage for this new code:

- `vcr/verifier/signature_verifier.go` -- Lines 62 and 137-138
- `vcr/verifier/verifier.go` -- Line 87-88

🚦 See full report on Qlty Cloud »

🛟 Help
  • Diff Coverage: Coverage for added or modified lines of code (excludes deleted files). Learn more.

  • Total Coverage: Coverage for the whole repository, calculated as the sum of all File Coverage. Learn more.

  • File Coverage: Covered Lines divided by Covered Lines plus Missed Lines. (Excludes non-executable lines including blank lines and comments.)

    • Indirect Changes: Changes to File Coverage for files that were not modified in this PR. Learn more.

Copilot AI changed the title [WIP] Update client error message for unresolved DID Surface DID resolution errors in VP verification failures Jan 29, 2026
Copilot AI requested a review from reinkrul January 29, 2026 10:27
@qltysh
Copy link

qltysh bot commented Feb 4, 2026

1 new issue

Tool Category Rule Count
qlty Structure Function with many returns (count = 8): jwtSignature 1

@reinkrul reinkrul force-pushed the copilot/improve-client-error-message branch from 8aa2b43 to 3e58304 Compare February 5, 2026 07:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

QoL: improve client error message if DID can't be resolved.

2 participants