digitalbazaar / jsonld-signatures

An implementation of the Linked Data Signatures specification for JSON-LD. Works in the browser and Node.js.
BSD 3-Clause "New" or "Revised" License
138 stars 41 forks source link

Implement better errors when docs/contexts are missing from documentLoader #63

Open dmitrizagidulin opened 5 years ago

dmitrizagidulin commented 5 years ago

In implementing vc-js tests, I've ran into multiple instances where if a context or any other document (like the key controller, or the key doc itself) was not handled by the documentLoader, it would result in an obscure failure message, something like 'invalid @id'.

So the action item is: track down where this is happening, and issue a better error message, something closer to "Context not able to be loaded, make sure the documentLoader supports it."

Note: this is somewhat related to issue #61, but that one just has to do with adding more discussion to the README, whereas this one is about improving the error messages.

dmitrizagidulin commented 5 years ago

@aljones15 When you have a chance and don't have other items, take a swing at this.

aljones15 commented 5 years ago

@dmitrizagidulin started this today hoping to have it done by the end of the week. thanks for the issue.

aljones15 commented 5 years ago

@dmitrizagidulin can you link me to a test that causes any of these issues?

anyways looks like this is pretty simple

let context = contexts[url]
if(context !=== undefined) {
  return {
    ...
  }
}
context = documentLoader(url); // note: error might be here
if(!context) {
  throw new Error(`Context ${url} not able to be loaded, make sure the documentLoader supports it.`);
}
return context;

also it looks like the issue is right now if the context is not one of the contexts in contexts.js then the documentLoader tries to load it. so maybe we should not throw there. yeah the error should be coming from documentLoader trying to load a url not in contexts.js which I guess could be an invalid file.

only ProofSet add and verify use documentLoader and both require a documentLoader via strictDocumentLoader.

oh yeah and also jsonld.js uses documentLoader all over the place.

aljones15 commented 5 years ago

Do you mean these errors?

     Error: The property "credentialSubject" in the input was not defined in the context.

     jsonld.SyntaxError: Invalid JSON-LD syntax; @context term values must be strings or objects.

     jsonld.SyntaxError: Invalid JSON-LD syntax; a term definition must not contain id

    Invalid JSON-LD syntax; a @context @id value must be an absolute IRI, a blank node identifier, or a keyword.

or is it something else?

also when a context is loaded via a url and it not found the error is kind of weird:

URL could not be dereferenced: Not Found
dlongley commented 5 years ago

I'm not sure what's needed to repeat the errors @dmitrizagidulin was seeing, but it would be something along these lines:

dlongley commented 5 years ago

A verification method needs to be provided as a full document or as a URL (i.e. as specified in the proof) where a document loader can load it. Also, the verification method document (in the case of a "ControllerProofPurpose" type or one that extends that class) needs to have a controller property. This library needs to do a better job of validating the above and throwing errors that make sense to the dev so they can fix what parameters they are passing to verify.

aljones15 commented 5 years ago

from op: """ right, so, I don't have an exact failing test. but what was failing before (with cryptic errors) is if a document being verified had a context, and that context had another one which was failing to resolve has something to do with documentLoader not being passed in somewhere deep in jsonld-signatures """

aljones15 commented 5 years ago

this is what an invalid url in a child context is currently returning from verify:

{ verified: false,
  error:
   { jsonld.InvalidUrl: Dereferencing a URL did not result in a valid JSON-LD object. Possible causes are an inaccessible URL perhaps due to a same-origin policy (ensure the server uses CORS if you are using client-side JavaScript), too many redirects, a non-JSON response, or more than one HTTP Link Header was provided for a remote context.
       at Promise.all.queue.map (/Users/andrewjones/Programs/javascript/digital_bazaar/jsonld-signatures/node_modules/jsonld/lib/context.js:1000:15)
     name: 'jsonld.InvalidUrl',
     details:
      { code: 'loading remote context failed',
        url: 'https://do-not-resolve-dsfsdfs.org',
        cause:
         Error: Document "https://do-not-resolve-dsfsdfs.org" not found.
             at module.exports (/Users/andrewjones/Programs/javascript/digital_bazaar/jsonld-signatures/tests/mock/test-loader.js:29:9)
             at /Users/andrewjones/Programs/javascript/digital_bazaar/jsonld-signatures/lib/documentLoader.js:23:12
             at /Users/andrewjones/Programs/javascript/digital_bazaar/jsonld-signatures/node_modules/jsonld/lib/util.js:443:25
             at Promise.all.queue.map (/Users/andrewjones/Programs/javascript/digital_bazaar/jsonld-signatures/node_modules/jsonld/lib/context.js:993:27)
             at Array.map (<anonymous>)
             at retrieve (/Users/andrewjones/Programs/javascript/digital_bazaar/jsonld-signatures/node_modules/jsonld/lib/context.js:978:30)
             at Promise.all.queue.map (/Users/andrewjones/Programs/javascript/digital_bazaar/jsonld-signatures/node_modules/jsonld/lib/context.js:1036:13) } } }

which actually is pretty informative however this is not being thrown. it's returned as part of the verify process.

btw the above comes from testLoader this is from node documentLoader:

{ verified: false,
  error:
   { jsonld.InvalidUrl: Dereferencing a URL did not result in a JSON object. The response was valid JSON, but it was not a JSON object.
       at Promise.all.queue.map (/Users/andrewjones/Programs/javascript/digital_bazaar/jsonld-signatures/node_modules/jsonld/lib/context.js:1013:15)
     name: 'jsonld.InvalidUrl',
     details:
      { code: 'invalid remote context',
        url: 'http://localhost:3424/' } } }

and with strictDocumentLoader as the loader:

strictDocumentLoader { verified: false,
  error:
   { Error: A URL "https://invalid-context-url" could not be fetched; you need to pass "documentLoader" or resolve the URL before calling "verify".
       at Object.verify (/Users/andrewjones/Programs/javascript/digital_bazaar/jsonld-signatures/lib/jsonld-signatures.js:43:19)
       at processTicksAndRejections (internal/process/next_tick.js:81:5)
     cause:
      { jsonld.InvalidUrl: Dereferencing a URL did not result in a valid JSON-LD object. Possible causes are an inaccessible URL perhaps due to a same-origin policy (ensure the server uses CORS if you are using client-side JavaScript), too many redirects, a non-JSON response, or more than one HTTP Link Header was provided for a remote context.
          at Promise.all.queue.map (/Users/andrewjones/Programs/javascript/digital_bazaar/jsonld-signatures/node_modules/jsonld/lib/context.js:1000:15)
          at processTicksAndRejections (internal/process/next_tick.js:81:5) name: 'jsonld.InvalidUrl', details: [Object] } } }

this error is a bit vague actually as it does not specify a documentLoader capable of loading remote documents.