jsonld-signatures-bbs icon indicating copy to clipboard operation
jsonld-signatures-bbs copied to clipboard

TypeError: suite.ensureSuiteContext is not a function

Open OR13 opened this issue 3 years ago • 15 comments

This is caused by recent changes in https://github.com/digitalbazaar/jsonld-signatures

Here is an example implementation that would remove this error:

import sec from '@transmute/security-context';

ensureSuiteContext({ document }: any) {
    const contextUrl = sec.constants.BLS12381_2020_V1_URL;
    if (
      document['@context'] === contextUrl ||
      (Array.isArray(document['@context']) &&
        document['@context'].includes(contextUrl))
    ) {
      // document already includes the required context
      return;
    }
    throw new TypeError(
      `The document to be signed must contain this suite's @context, ` +
        `"${contextUrl}".`
    );
  }

OR13 avatar Jul 10 '21 19:07 OR13

has any progress been made on any of these issues? I am running into the same problem

brianorwhatever avatar Sep 16 '21 19:09 brianorwhatever

https://github.com/transmute-industries/verifiable-data/blob/main/packages/bbs-bls12381-signature-2020/src/BbsBlsSignature2020.ts#L48

^ this implementation supports it... feel free to help port it here.

OR13 avatar Sep 20 '21 15:09 OR13

Hi @OR13 thanks for the link! I'm trying to understand the differences between these two implementations. Is there a specific reason the Transmute implementation requires the https://w3id.org/security/suites/bls12381-2020/v1 context on the input doc and this MATTR one doesn't?

Mainly asking because I don't see any mention of this context in the BBS spec either.

karimStekelenburg avatar Apr 02 '22 19:04 karimStekelenburg

The function is required by digital bazaar, the context url is in the form we agreed to use during the did WG.

On Sat, Apr 2, 2022, 2:11 PM Karim Stekelenburg @.***> wrote:

Hi @OR13 https://github.com/OR13 thanks for the link! I'm trying to understand the differences between these two implementations. Is there a specific reason the Transmute implementation requires the https://w3id.org/security/suites/bls12381-2020/v1 context on the input doc and this MATTR one doesn't?

Mainly asking because I don't see any mention of this context in the BBS spec either.

— Reply to this email directly, view it on GitHub https://github.com/mattrglobal/jsonld-signatures-bbs/issues/142#issuecomment-1086704742, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB7JLMAYMW6NQXLOQMAZKILVDCLXXANCNFSM5AEQHRQA . You are receiving this because you were mentioned.Message ID: @.***>

OR13 avatar Apr 04 '22 13:04 OR13

This seems to be caused by the fact that BbsBlsSignature2020 is extending the wrong base class: it should be extending suites.LinkedDataSignature, but it's instead extending suites.LinkedDataProof.

This is why the ensureContext() function doesn't exist, even though that function has a default implementation in the LinkedDataSignature class.

geel9 avatar Apr 20 '22 21:04 geel9

Once I add the missing ensureContext function, I get into another issue with context collision.

jsonld.SyntaxError: Invalid JSON-LD syntax; tried to redefine a protected term.

The issued is caused by "Ed25519Signature2018".

Any idea/workaround for getting that solved ? Thanks

pcibraro avatar May 11 '22 17:05 pcibraro

probably you have too many contexts in your JSON-LD... and they both define the same term, Ed25519Signature2018

OR13 avatar May 11 '22 18:05 OR13

Yes, but the strange thing is that term is only defined in the "https://www.w3.org/2018/credentials/v1" context for this sample. I don't have them anywhere else. Thanks

pcibraro avatar May 11 '22 18:05 pcibraro

Past the example you are trying to sign.

OR13 avatar May 11 '22 18:05 OR13

Thanks for looking into this. It is the sample in this repo. https://github.com/mattrglobal/jsonld-signatures-bbs/tree/master/sample/ts-node/src/demo_single.ts. I only updated the jsonld and jsonld-signatures packages to be the latest available version, and provided an empty implementation for ensureSuiteContext.

pcibraro avatar May 11 '22 18:05 pcibraro

what is the plan for this? Do I have to downgrade json-ld from 9 to 5 or is there a way to go around this?

cre8 avatar Sep 21 '22 16:09 cre8

@cre8 I am not sure, but I would guess you would need to downgrade jsonld-signatures in order to avoid this issue.

OR13 avatar Sep 27 '22 17:09 OR13

From what I can tell, support for the method ensureSuiteContext is the only thing that's missing for compatibility with jsonld-signatures in its latest form (11.0.0).

@OR13 the context URL in your implementation differs from the one in the Mattr example: https://w3id.org/security/bbs/v1 vs https://w3id.org/security/suites/bls12381-2020/v1, however they are both the same in content. Are both acceptable? Or should one prevail over the other? What is the reason, if you know, why there are 2 URLs? I suppose the ensureSuiteContext should account for both in the end.

My quite hacky solution at this moment is to do something like this:

  const suite = new BbsBlsSignature2020({ key: keyPair });

  (suite as any).ensureSuiteContext = ({ document }) => {
    const contextUrls = [
      'https://w3id.org/security/suites/bls12381-2020/v1',
      'https://w3id.org/security/bbs/v1'
    ];

    if (typeof document['@context'] === 'string' && contextUrls.includes(document['@context'])) {
      return;
    }

    if (Array.isArray(document['@context']) &&
      contextUrls.filter(url => document['@context'].includes(url)).length) {
      return;
    }

    throw new TypeError(
      `The document to be signed must contain one of this suite's @context, ` +
      `"${contextUrls.join(', ')}", got "${document['@context'].join(', ')}".`
    );
  };

But that way I can maintain this package as a dependency and not downgrade jsonld-signatures.

To improve devX we could also just append the missing context URL to the @context list instead of throwing, but one has to be ok with silently modifying the data (console.warn maybe?)

lemoustachiste avatar Nov 14 '22 13:11 lemoustachiste

@lemoustachiste the context IRI should not matter, as long as it produces the same nquads.

My opinion is that an error should be thrown when attempting to sign a document with invalid context, and no "just in time patching / mutation" should be applied.

Also the documentLoader should always be an argument to the functions that require it (any that produce nquads).

OR13 avatar Nov 15 '22 14:11 OR13

From what I can tell, support for the method ensureSuiteContext is the only thing that's missing for compatibility with jsonld-signatures in its latest form (11.0.0).

After more time spent on the matter, verification proved difficult on 2 aspects:

  • no compatibility with security-document-loader, but that was my own personal implementation detail
  • the shape of the verification method as created by this library was not compatible for verification with a later version of jsonld-signature (@graph property):
{
  "@context": "https://w3id.org/security/v2",
  "@graph": [
    {
      "id": "did:key:z5TcCsDp9X8AWTuwTMYiXrk8ukCdgvmbZZmRoZe4cmrhQTabL9kxTuiJYUY4s8zGKcf4Pwxh9b6wPk6Qms8GN933f6a6RXoca7fNCDQAHgHwQ9pX2Jj421XTufDyPTWwVwbYQnUkF5w1K3ja25H7YXam1r1N2apt9K5HK9veWUS7RRqfBmmV4WBEdV7cNPE26cPeFfXUJ#zUC7Gd4cgVrBBCFRRQsC7R1bgZN8rYMcLFca2Mghg5qmynEaXkkdrzAxk3ZnXtwAZkcCCKkN3tok4XMzzzkpRF68ZYBHCVaujP3finwQD4vgRFePq4Godv6ChmPS2uhmosKB3ae",
      "type": "sec:JsonWebKey2020",
      "controller": {
        "id": "did:key:z5TcCsDp9X8AWTuwTMYiXrk8ukCdgvmbZZmRoZe4cmrhQTabL9kxTuiJYUY4s8zGKcf4Pwxh9b6wPk6Qms8GN933f6a6RXoca7fNCDQAHgHwQ9pX2Jj421XTufDyPTWwVwbYQnUkF5w1K3ja25H7YXam1r1N2apt9K5HK9veWUS7RRqfBmmV4WBEdV7cNPE26cPeFfXUJ",
        "assertionMethod": [
          {
            "id": "did:key:z5TcCsDp9X8AWTuwTMYiXrk8ukCdgvmbZZmRoZe4cmrhQTabL9kxTuiJYUY4s8zGKcf4Pwxh9b6wPk6Qms8GN933f6a6RXoca7fNCDQAHgHwQ9pX2Jj421XTufDyPTWwVwbYQnUkF5w1K3ja25H7YXam1r1N2apt9K5HK9veWUS7RRqfBmmV4WBEdV7cNPE26cPeFfXUJ#z3tEF3nW8YQfhah9KxHfg4UjhRF5N3JkGDTfVrpkjpVNknV1mmDkzZjswdJvw67AdNhc85",
            "type": "sec:JsonWebKey2020",
...

lemoustachiste avatar Nov 17 '22 14:11 lemoustachiste