Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: failure to verify jsonld on non-conformant doc but vaild vmethod #1301

Conversation

dbluhm
Copy link
Contributor

@dbluhm dbluhm commented Jul 7, 2021

Minor fix for jsonld verify route.

@dbluhm dbluhm marked this pull request as draft July 7, 2021 18:23
@dbluhm
Copy link
Contributor Author

dbluhm commented Jul 7, 2021

Tests to hit the error path pending, hoping to get this ready for merge ASAP

@codecov-commenter
Copy link

codecov-commenter commented Jul 7, 2021

Codecov Report

Merging #1301 (2887a53) into main (1f8eeb0) will decrease coverage by 0.00%.
The diff coverage is 94.11%.

@@            Coverage Diff             @@
##             main    #1301      +/-   ##
==========================================
- Coverage   95.43%   95.42%   -0.01%     
==========================================
  Files         476      476              
  Lines       28977    28980       +3     
==========================================
+ Hits        27653    27655       +2     
- Misses       1324     1325       +1     

)
try:
ver_meth_expanded = VerificationMethod.deserialize(
ver_meth_expanded.serialize()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is it if not a VerificationMethod?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The dereference method can also be used to dereference a Service. The method itself has a return value of Resource which is the base VerificationMethod and Service build from. It's a little on the complicated side due to attempts to work with documents that don't strictly conform to the schema outlined in the DID spec. In that case, a plain Resource object is returned since the more feature rich parsing failed. We give that parsing another try for just the identified resource here.

Alternatively, I could see this being simpler: we default to dereferencing resources from a DID document as if the doc itself was non-conforming but modify the dereference method to also take a parameter asserting that the dereferenced resource is a VerificationMethod or Service and "casts" the resource to one of these objects (or whatever type that was passed in) or fails.

@dbluhm dbluhm marked this pull request as ready for review July 22, 2021 00:37
@andrewwhitehead andrewwhitehead merged commit c09ed19 into openwallet-foundation:main Aug 16, 2021
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.

3 participants