mattrglobal / jsonld-signatures-bbs

A linked data proof suite for BBS+ signatures
Apache License 2.0
141 stars 41 forks source link

TypeError: suite.ensureSuiteContext is not a function #142

Open OR13 opened 3 years ago

OR13 commented 3 years ago

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}".`
    );
  }
brianorwhatever commented 3 years ago

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

OR13 commented 3 years ago

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.

karimStekelenburg commented 2 years ago

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.

OR13 commented 2 years ago

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: @.***>

geel9 commented 2 years ago

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.

pcibraro commented 2 years ago

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

OR13 commented 2 years ago

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

pcibraro commented 2 years ago

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

OR13 commented 2 years ago

Past the example you are trying to sign.

pcibraro commented 2 years ago

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.

cre8 commented 2 years ago

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?

OR13 commented 2 years ago

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

lemoustachiste commented 2 years ago

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?)

OR13 commented 2 years ago

@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).

lemoustachiste commented 2 years ago

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: