w3c / did-core

W3C Decentralized Identifier Specification v1.0
https://www.w3.org/TR/did-core/
Other
407 stars 96 forks source link

Restrictions on `@context` property values in DID Documents #432

Closed OR13 closed 3 years ago

OR13 commented 4 years ago

Per https://github.com/w3c/did-core/pull/408#pullrequestreview-504771602 Pulling this into an issue.

I will summarize here, but essentially, we are asking:

What additional restrictions on @context should DID Core introduce, beyond what JSON-LD allows.

OR13 commented 4 years ago

My perspective is that the default should be "No additional restrictions" if we can't reach consensus on them.

I would agree with requiring the @context to always being an array.

and always having a first member https://www.w3.org/ns/did/v1.

OR13 commented 4 years ago

ping @msporny @dlongley @rhiaro @peacekeeper @tplooker :)

msporny commented 4 years ago

I'm fine w/ what you said above... but I'm guessing that's because it's not the point of contention that created this issue. :)

The real question is whether or not we should be suggesting that people utilize @base in JSON-LD in DID Core. I'd prefer that we, at a minimum, stay silent on it. I suggest further that using @base in @context is an anti-pattern for DID Documents because it won't map cleanly to JSON-only documents, or vanilla CBOR documents. It doesn't work the same way across representations. We also have rules in DID Core that obviate the need for @base (because we didn't want people to depend on it -- @dhh1128 requested the relative URI resolution stuff, IIRC -- my memory may be off on that one, though).

Here are a couple of proposals folks could weigh in on to start to determine consensus:

PROPOSAL: Suggest that DID Documents use @base to set the Base URI in DID Core for JSON-LD documents.

-1

PROPOSAL: Stay silent on the use of @base to set the Base URI in DID Core for JSON-LD documents.

+0.5

PROPOSAL: Advise against the use of @base to set the Base URI in DID Core.

+1

dhh1128 commented 4 years ago

I would vote exactly as Manu did on the 3 proposals above.

OR13 commented 4 years ago

@dhh1128 have you used JSON-LD with relative URIs before? do you understand how @base is used?

Its absence requires you to configure your JSON-LD processor explicitly, in way that is only required when using relative URIs.

I believe peer did intend to use relative refs... if peer did also intends to support Linked Data Proofs you will need to either use @base... or add this code everywhere (including libraries you are not the author of / written in other languages)....

const expanded = await jsonld.expand(didDocument, { documentLoader, base: didDocument.id });

with @base you can just follow the default documentation for expand and do this:

const expanded = await jsonld.expand(didDocument, { documentLoader });

You will note that base is not documented here: https://github.com/digitalbazaar/jsonld.js/#expand

All Sidetree did methods currently use @base today, because we use relative refs, and we intend to support JSON-LD and JSON.... I suggest writing tests ASAP if you think you are going to use relative refs, and also think you don't need @base.

here are some tests, you can start with: https://github.com/OR13/jsonld-did-production-with-base/blob/main/produce-without-base.test.js#L23

These show that omitting @base causes relative URI processing to fail in absence of the suggestion to change the code above.

Since I am not sure that everyone will be able to implement the code changes above (in python, go, rust, java, .net and javascript).... I suggest that we allow for @base.... I am fine with not recommending it generally, but I think its a mistake not to explain it in did core, especially if we are going to allow relative refs....

I propose adding a section that is specific to JSON-LD that describes this sufficiently so that developers know that IF they don't include @base they MUST ensure it is set EVERYWHERE to the didDocument.id.

I imagine this would blow up when folks who are just trying to do "JSON-LD" happen to stumble on a did document... and so I would never return JSON-LD with relative refs that does not contain @base.... because of the exact behavior documented in the tests above.

msporny commented 4 years ago

Its absence requires you to configure your JSON-LD processor explicitly, in way that is only required when using relative URIs.

Yes, you have to configure your software correctly for it to work. :)

This is how URL resolvers have worked for decades:

https://nodejs.org/api/url.html#url_new_url_input_base https://metacpan.org/pod/URI#$uri-=-URI-%3Enew_abs(-$str,-$base_uri-) https://docs.rs/url/1.5.1/url/struct.Url.html#examples-3 https://docs.python.org/3/library/urllib.parse.html#urllib.parse.urljoin

You give them the Base URI to resolve against.

I propose adding a section that is specific to JSON-LD that describes this sufficiently so that developers know that IF they don't include @base they MUST ensure it is set EVERYWHERE to the didDocument.id.

We have this section in the spec already:

https://www.w3.org/TR/did-core/#relative-did-urls

We should have some tests that do relative URI resolution, so +1 to that, but I believe that was already going to happen anyway.

I'm fine if we add text to the JSON-LD section pointing out that implementers should pay particular attention to setting the base URI to didDocument.id in their JSON-LD processors.

@OR13 are you aware of any JSON-LD processors that don't allow you to set this value?

OR13 commented 4 years ago

https://github.com/emersion/go-jsonld/blob/401cba701428835b62b3c8a8ea83fff1207f73dd/encoder.go#L66

I don't have time to search github for all of them, but this was one of the first results I found.

@msporny I don't agree that the section you reference that describes base URI with no link to defintion or example is sufficient.

although this is clearly the section where commentary on this belongs.

We need some language like:

If relative URIs are used in JSON-LD, either the baseURI must be set to the DID Document id or an @base MUST be present in the @context and its value must be the DID Document id.

And base URI needs to be defined in a way that explains how JSON-LD uses it, or provides a link to such a definition.

If we don't say something like this, folks will continuously discover the error documented in the tests I have provided... we cannot assume JSON-LD knowledge, we must provide guidance.

dhh1128 commented 4 years ago

Responding to @OR13 's comments:

Yes, I'm pretty aware of what it means to processing code to not include an explicit @base. As Manu said, specifying the base in code is the price you pay, and all JSON-LD libraries should allow that parameter. I still would +1 and -1 stuff the way Manu did above.

I don't mind adding extra guidance about this topic in DID core, however, to improve clarity.

dlongley commented 4 years ago

are you aware of any JSON-LD processors that don't allow you to set this value?

They aren't compliant with the spec if they don't: https://www.w3.org/TR/json-ld11-api/#dom-jsonldoptions

msporny commented 4 years ago

@OR13 wrote:

If we don't say something like this, folks will continuously discover the error documented in the tests I have provided... we cannot assume JSON-LD knowledge, we must provide guidance.

I'd be supportive of a PR that adds more descriptive language in the spec along the lines of what you suggested.

To be clear, @dlongley is correct, looks like the go-jsonld implementation is non-conformant. Taking a look at their implementation, I believe you could inject @base as didDocument.id right before you begin processing and still get the right outcome. It's the notion that a DID Method would define @base because of a broken JSON-LD implementation that is of concern. That's the tail wagging the dog -- the implementation should be fixed to be conformant... not a serialization should be hacked to work with a non-conformant JSON-LD implementation.

dlongley commented 4 years ago

I would also vote similarly to Manu on those proposals. My opinion is that any guidance we give on expressing DID Documents in JSON-LD should be such that they look as similar as possible to JSON documents because it makes things easier on implementers. There has been lots of discussion in the past in various groups around using absolute URLs when possible (as opposed to relative ones) as well, again, because it's easier for end users. There's no issue with using something like @base in an underlying implementation to save space or whatever -- but when exposing DID Documents to end users, it's better to remove as many additional complexities as possible to reduce cognitive burden and simplify code.

OR13 commented 4 years ago

It's the notion that a DID Method would define @base because of a broken JSON-LD implementation that is of concern.

People write software that works with other software... at some point, OpenSSL bugs become the spec.

I think its best not to force forever reliance on broken implementations... but its also good to not prevent interoperability with broken implementations.

I agree that @base should not be necessary, IF one understands and implements JSON-LD correctly... The problem is that I also believe that most people won't do this, and so I would rather them copy paste something a little more verbose, that will "always work".... after all, in JSON, if you don't understand it, just ignore it :)

I am also a strong supporter of explicit over implicit, and in this case, I feel explicitly setting @base is better than "reading the spec to see how it should be set".

peacekeeper commented 4 years ago

From a resolution/dereferencing standpoint, I also agree with @msporny and others that the Base IRI should be implicitly understood to be the DID that is being resolved/dereferenced. Therefore it should not be necessary (and could be problematic) to include @base in a DID document.

The Java JSON-LD library I am using supports this: https://github.com/filip26/titanium-json-ld/blob/master/src/main/java/com/apicatalog/jsonld/processor/ExpansionProcessor.java#L65

OR13 commented 4 years ago

hmm, I wrote some tests for this... and it seems like you guys are right.... https://github.com/transmute-industries/vc.js/pull/21

I think my confusion for this was entirely manufactured by https://json-ld.org/playground/

Try using: https://github.com/transmute-industries/vc.js/blob/master/packages/vc.js/src/__fixtures__/shortFormDidDoc.json

If you look at the N-Quads it produces... it assumes an incorrect baseURI, when @base is not present...... although the vc test I just wrote suggests @base is not required.... by Linked Data VCs when using fragment URIs.

I assume that every JSON-LD implementation that sets the documentUrl correctly, as I have here:

https://github.com/transmute-industries/vc.js/pull/21/files#diff-bcd54a11ff3c0f175b7166c240d62bb905eb40445f6b58486949f9eedd29007bR97

will work...

I assume its the fact that the JSON-LD Document Loader has this specific signature that makes this work:

          return {
            contextUrl: null,
            document: didDocument,
            documentUrl: didDocument.id,
          };

The only remaining question I have is, have we tested fragment URIs without @base enough to be certain that it should be forbidden?

I am in favor of forbidding it if it's possible to, I agree it could be confusing... I was clearly confused by it being required to make JSON-LD playground render n-quads properly.

msporny commented 4 years ago

@OR13 wrote:

hmm, I wrote some tests for this... and it seems like you guys are right.

Huzzah! Progress!

I assume that every JSON-LD implementation that sets the documentUrl correctly, as I have here ... will work...

Yes, and if they don't allow it, they're a non-conformant JSON-LD processor. If they do not have the capability to set the base IRI, you can still hack around it by injecting @base right before processing -- that is clearly a hack, but one that you can use while the JSON-LD processor fixes their mistake.

The only remaining question I have is, have we tested fragment URIs without @base enough to be certain that it should be forbidden?

Fragment URIs w/o @base have been extensively tested in the JSON-LD days/test suite. That said, happy to have lots and lots of corner-case tests in the DID test suite to see if we missed anything. I don't think we've done enough testing for relative frag ids in DID Documents.

I am in favor of forbidding it if it's possible to, I agree it could be confusing... I was clearly confused by it being required to make JSON-LD playground render n-quads properly.

I don't think we need to forbid it... our options include:

  1. Forbid use of @base (and @vocab, and maybe a few others).
  2. State that extra JSON-LD processing directives like @base and @vocab that result in funky interpretation by JSON-only processors SHOULD NOT be used. Try to keep it so that if the @context is chucked out of the window, that the JSON-only interpretation doesn't change.
  3. Stay silent on use of @base and @vocab.

I prefer 3... I expect @OR13 might prefer 1 or 2.

OR13 commented 4 years ago

I prefer 3, and suggest we close this issue with the comment in the spec saying "we're debating @base is removed".

OR13 commented 4 years ago

I suggest we see a PR which removes that comment as the next step to close this issue.

peacekeeper commented 4 years ago

I also prefer 3.

OR13 commented 4 years ago

Cross linking:

msporny commented 3 years ago

This issue will be closed once PR #488 is merged.

msporny commented 3 years ago

PR #488 has been merged. Closing.