Address traceability context bug #2743

[PatStLouis]

This PR is to address #2743

There is an odd bug that's been ongoing for some time where when trying to expand a did document with the pyld library it fails.

Since this happen in a very specific condition, this PR looks to implement a workaround.

When we need to verify a json-ld VC where the issuer/verificationMethod is a did:web: scheme, we first manually resolve the did document with the requests module, then extract the traceability context form the document if present, then frame the document directly....

Its not pretty, but its the best solution I could come up with that has minimal impact on the existing code. If anyone has a better solution, I'm happy to change it!

@swcurran @dbluhm this is the last critical PR I would like to see in the 0.12 release

[dbluhm]

Not pretty indeed 😅 I'm quite hesitant on this one. Could you provide some more details on how pyld fails when resolving the traceability context? Has the issue been raised with the authors of that library?

[PatStLouis]

So I've raised an issue on the pyld repo and the traceability interop repo. The pyld maintainers never acknowledged the issue and the traceability folks claimed the issue was most likely in the traceability context. There was a fix suggested which didn't address the issue. The traceability folks are willing to help however its been difficult to get hold of them.

For a minimal example, here's a code snippet. Its the specific combination of the traceability context, the traceability service endpoint and type. Remove any one of those 3 and the error isn't raised.

from pyld import jsonld

document = {
  "@context": [
    "https://www.w3.org/ns/did/v1",
    "https://w3id.org/traceability/v1"
  ],
  "service": [
    {
      "id": "did:web:example.com#traceability-api",
      "type": ["TraceabilityAPI"],
      "serviceEndpoint": "https://example.com"
    }
  ]
}

jsonld.expand(document)

The issue is with the expanding part, which is a substep of the framing. The error returned is:

pyld.jsonld.JsonLdError: ('Tried to nullify a context with protected terms outside of a term definition.',)
Type: jsonld.SyntaxError
Code: invalid context nullification

I'm really trying to find a solution here since aca-py currently won't be able to verify TraceableCredentials because of this bug.

There's a traceability meeting tomorrow I will try to get more info but it's been over a month I noticed this issue. It bothers me that nobody else encountered it because as far as I can tell acapy is doing things the right way and not being able to expand a linked data document sounds like a significant issue.

[dbluhm]

If I stumbled on the pyld repo and saw that it hadn't seen a commit in 4 years and had 20 open PRs and 50+ issues, I would assume the project was dead. That doesn't seem far from the truth, frankly. A quick scan for an alternative does not yield much in python. So that's too bad lol... Even if we did poke at the library and figure out what needs to be fixed, it seems like it would not help since PRs aren't getting merged.

[dbluhm]

I did a bit of pdbing on the example you provided and from a very uninformed perspective, I think it has to do with the empty @context in the TraceabilityAPI section of the context

[PatStLouis]

@dbluhm thanks for having a look into this. I also suspect the empty context has something to do with it and I have brought this up to the group. They didn't seem to share that opinion. Would you be able to tell what should the content of the TraceabilityAPI @context field should be? I could run some tests with a mock server.

The other option is to prettify this solution. We could leverage the resolver module directly, I ran into issues with injecting the profile and async calls, hence why I went with the requests module. The traceability context endpoint can be added to the constants.

[dbluhm]

Can we call step 5.1.1 to the attention of the traceability folks in this section of the json-ld-api spec? https://w3c.github.io/json-ld-api/#algorithm

[dbluhm]

Ah, you know what, I think I see the issue:

https://github.com/digitalbazaar/pyld/blob/316fbc2c9e25b3cf718b4ee189012a64b91f17e7/lib/pyld/jsonld.py#L3073

if not ctx: should be if ctx is None:. ctx will be falsey when given an empty dictionary. The algorithm states that the error condition should only be met when the ctx is explicitly a null or, in python, a None.

[dbluhm]

I still don't understand the function of the empty context, though. If that's truly necessary according to the traceability context authors then we could try patching the library or putting a PR out for it on the off chance it gets merged (and released....). But if it's not strictly necessary, maybe it could be dropped in a revision of the context.

[PatStLouis]

@dbluhm I will try the proposed edit for the pyld library. The maintainers are in the vc-api call I will point this out to them this afternoon and we can get a PR in if it solves the issue. Do you want to author it since you found the solution?

I will also touch on this at the traceability call this afternoon and bring up the point you mentioned.

[PatStLouis]

@dbluhm so I think there was both an error in the pyld and the traceability context, your suggestion did get pass the nullification step, however it now raises an error at empty context:

pyld.jsonld.JsonLdError: ('Invalid JSON-LD syntax; invalid scoped context.',)
Type: jsonld.SyntaxError
Code: invalid scoped context
Details: {'context': {}, 'term': 'Event'}

Now it seems step 21.1 is where the empty context fails.

There are actually many terms with empty contexts. What a messy situation.

I think bringing the PR in the pyld library is important since this is actually a python related bug. For the empty context I will get more information from the group. Fantastic find, we are definitely on the right track!

[dbluhm]

@dbluhm I will try the proposed edit for the pyld library. The maintainers are in the vc-api call I will point this out to them this afternoon and we can get a PR in if it solves the issue. Do you want to author it since you found the solution?

I will also touch on this at the traceability call this afternoon and bring up the point you mentioned.

Feel free to open the PR! I have no preference to being the author of the solution 🙂

[PatStLouis]

@dbluhm see this PR for the solution

[PatStLouis]

@dbluhm while we have the open pr in the pyld repo I still provided a prettier solution here. Let's keep this open in case the publication of a new pyld package becomes an issue.

I added a non async version of the resolve function for the did:web: resolver module. It's a copy paste of the existing function but I used the requests package instead of aiohttp for the get request on the did document url.

Since the _get_verification_method function was non async, I wasn't able to call the existing did web resolve function without introducing too many changes. Let me know what you think!

[sonarqubecloud]

Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarCloud

[swcurran]

@PatStLouis — can we close this with the new #2795 change?

[swcurran]

Sorry for the extra question — I had been looking at the open PRs in prep for the release candidate and didn’t see your comment on #2795. Doh!

[PatStLouis]

@swcurran Yes when #2795 is merged we won't need this PR as this was a less desirable workaround. The other PR will address the core issue directly.