Update BBS+ context to bypass redirections

[swcurran]

Signed-off-by: Stephen Curran [email protected]

[swcurran]

Addresses #2738. A reasonable change to make, @andrewwhitehead ?

Dang...lint issues.

[andrewwhitehead]

I don't see anywhere that we reject a BBS credential if it doesn't have the expected context, so it looks like no change is needed there. Interestingly it looks like we only use the JSON-LD document loader with cached results in tests, not in production – I find that a bit surprising. One minor issue I see is that the BBS context is auto-added to issued credentials with the BBS signature type. If the provided JSON includes the legacy context, then the new context will be added as well, unnecessarily.

[swcurran]

@andrewwhitehead — those issues seem independent of this change, right? All this does is bypass the current redirection that is happening from the old BBS+ context URL to the new one. So I think this one is safe enough. Correct?

The rest of the things you mention existing issues that should be at least recorded and fixed.

Of particular concern to me is the document loader not being used in production. That seems like a problem. The auto-adding is a also a bit of an issue. What do JSON-LD processors do when a context is referenced twice in the same document?

Could you be more precise on exactly what is needed?

[andrewwhitehead]

I think the only immediate change is that if a provided credential contains the 'legacy' BBS context, then we should not be adding the new one as well, just to avoid breaking things for anybody using that functionality. I'm not certain if it would break the JSON-LD processing, we could test that in the playground, but it wouldn't be ideal.

[andrewwhitehead]

Looks like it doesn't break the processing: https://json-ld.org/playground/#startTab=tab-expanded&json-ld=%7B%22%40context%22%3A%5B%22https%3A%2F%2Fwww.w3.org%2F2018%2Fcredentials%2Fv1%22%2C%22https%3A%2F%2Fwww.w3.org%2F2018%2Fcredentials%2Fexamples%2Fv1%22%2C%22https%3A%2F%2Fw3id.org%2Fsecurity%2Fbbs%2Fv1%22%2C%22https%3A%2F%2Fw3c.github.io%2Fvc-di-bbs%2Fcontexts%2Fv1%22%5D%2C%22id%22%3A%22https%3A%2F%2Fexample.gov%2Fcredentials%2F3732%22%2C%22type%22%3A%5B%22VerifiableCredential%22%2C%22UniversityDegreeCredential%22%5D%2C%22issuer%22%3A%22did%3Akey%3AzUC72Q7XD4PE4CrMiDVXuvZng3sBvMmaGgNeTUJuzavH2BS7ThbHL9FhsZM9QYY5fqAQ4MB8M9oudz3tfuaX36Ajr97QRW7LBt6WWmrtESe6Bs5NYzFtLWEmeVtvRYVAgjFcJSa%22%2C%22issuanceDate%22%3A%222020-03-10T04%3A24%3A12.164Z%22%2C%22credentialSubject%22%3A%7B%22degree%22%3A%7B%22type%22%3A%22BachelorDegree%22%2C%22name%22%3A%22Bachelor%20of%20Science%20and%20Arts%22%2C%22degreeType%22%3A%22Underwater%20Basket%20Weaving%22%7D%2C%22college%22%3A%22Contoso%20University%22%7D%2C%22proof%22%3A%7B%22type%22%3A%22BbsBlsSignature2020%22%2C%22verificationMethod%22%3A%22did%3Akey%3AzUC72Q7XD4PE4CrMiDVXuvZng3sBvMmaGgNeTUJuzavH2BS7ThbHL9FhsZM9QYY5fqAQ4MB8M9oudz3tfuaX36Ajr97QRW7LBt6WWmrtESe6Bs5NYzFtLWEmeVtvRYVAgjFcJSa%23zUC72Q7XD4PE4CrMiDVXuvZng3sBvMmaGgNeTUJuzavH2BS7ThbHL9FhsZM9QYY5fqAQ4MB8M9oudz3tfuaX36Ajr97QRW7LBt6WWmrtESe6Bs5NYzFtLWEmeVtvRYVAgjFcJSa%22%2C%22created%22%3A%222019-12-11T03%3A50%3A55%22%2C%22proofPurpose%22%3A%22assertionMethod%22%2C%22proofValue%22%3A%22iRArJRSvmIwx5YH2HXg5OJD%2B0v5sD1HoqhBsiJiw59t3Eb6nSntyOnENEnqnpzQwCjtbvOsU18eBlVi2%2Fign1u1ysz0iOLxSRHvIKtDDpr1dTDwQCbuZo2gUnY%2B8Dy%2BxEst8MDtcXwzNQW8Y3l1XzA%3D%3D%22%7D%7D

[swcurran]

So we are good to go with this one? Assuming the tests pass?

[truonghaha]

Hi @swcurran @andrewwhitehead thank you so much for jumping into this issue quickly, I tried replacing https://w3id.org/security/bbs/v1 with https://w3c.github.io/vc-di-bbs/contexts/v1 on my local but it did work as well. Seems like the pyld could not load document from https://w3c.github.io/vc-di-bbs/contexts/v1.

[truonghaha]

Would it be an issue with the response header from W3C that caused the document loader to not work? When I tried to host the BBS context locally (e.g. http://localhost:2004/vc/context/bbs/v1) with the same content as W3C, and it worked normally, not sure why this issue happened.

If this is the case, I wonder if we have any option to not load BBS context from a remote source? Or the ability to change it to another URL?

[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

[andrewwhitehead]

It looks like that URL also returns a 301 redirect, and it would need an extra trailing slash to avoid that. But the resolver needs to be able to handle redirects anyway.

[swcurran]

So to fully deal with the issue, I should do another PR in to add a “/“ on the end?

[andrewwhitehead]

I feel like it might be better to keep the original URL and make sure that redirects are supported (with a unit test), given that the w3 URL can be more easily redirected in case the hosting URL changes. We also need to make sure that the base credential context is always precached.

[andrewwhitehead]

Actually it does appear that the base context is precached, but the BBS context wasn't added here which might be a faster fix: https://github.com/hyperledger/aries-cloudagent-python/blob/main/aries_cloudagent/vc/ld_proofs/document_downloader.py#L39-L45

[swcurran]

I’ll add a task for you to fix this. I thought this would be a trivial change. I don’t understand redirects enough to understand why continuing to use an outdated URL is the right way to go. That said, I agree, it will be moved again in the future, so whatever we can do to prevent future issues is the right thing.