Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update did-v0.11.jsonld to match the current spec #156

Closed
wants to merge 1 commit into from

Conversation

OR13
Copy link
Contributor

@OR13 OR13 commented Jan 25, 2020

Add missing properties to context.

Absence of these properties severely impacts interoperability when relying on https://www.w3.org/ns/did/v1

And per the current spec: https://w3c.github.io/did-core/#public-keys

Public keys of all types MUST be expressed in either JSON Web Key (JWK) format using the publicKeyJwk property or one of the formats listed in the table below.

Add missing properties to context. Absence of these properties severly impacts interoperability when relying on https://www.w3.org/ns/did/v1

And per the current spec: https://w3c.github.io/did-core/#public-keys

`Public keys of all types MUST be expressed in either JSON Web Key (JWK) format using the publicKeyJwk property or one of the formats listed in the table below.`
@OR13 OR13 changed the title Update did-v0.11.jsonld Update did-v0.11.jsonld to match the current spec Jan 25, 2020
@OR13
Copy link
Contributor Author

OR13 commented Jan 25, 2020

IPR integration is bugged, my accounts are linked, it will not re-authorize.

@OR13 OR13 requested a review from dlongley January 25, 2020 18:23
@OR13
Copy link
Contributor Author

OR13 commented Jan 25, 2020

Absence of these property definitions currently prevents adoption of https://www.w3.org/ns/did/v1 ... which cannot be relied on until they are added.

@OR13
Copy link
Contributor Author

OR13 commented Jan 25, 2020

In the meantime, if you are blocked by this, feel free to add the WIP sidetree extension context, which defines these properties with proper documentation: https://docs.element-did.com/contexts/sidetree/sidetree-v0.1.jsonld

OR13 added a commit to OR13/self that referenced this pull request Jan 25, 2020
add sidetree context because of w3c/did-core#156
Copy link
Contributor

@dlongley dlongley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just realized that publicKeyJwk is misspecified here. And we should perhaps move to a different version to make new changes anyway as that has been the practice to date. publicKeyJwk needs to specified like this:

"publicKeyJwk": {"@id": "sec:publicKeyJwk", "@type": "@json"}

In order to be express that its value is a JSON literal.

Copy link
Member

@msporny msporny left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Major issues with this PR: 1) People have shipped did-v0.11.jsonld in software so we shouldn't change it, we should bump the version to v0.12.jsonld and make changes there, 2) publicKeyJwk is specified without specifying all of the possible values as terms or making it a JSON Literal, and we need to discuss what the right thing to do is before making a change to the context, and 3) publicKeyHex was specifically called out as something that the group didn't want to support IIRC... only publicKeyPem, publicKeyJwk, and publicKeyBase58 are allowed.

Here's what we need to make progress on this PR:

  1. Someone needs to do an analysis on the entire JWK registry and ensure that there are no canonization issues with values associated with items in the registry. If there are no JCS canonization issues, then we should use "@type": "@json". If there are JCS canonization issues, then we will need to express all possible JWK values in the context as Linked Data.
  2. The DID Core Registry needs to be put in place, and we may be able to add publicKeyHex there, but expect that there may be objections based on the previous discussion that rejected publicKeyHex as a valid interoperability option.

@@ -50,6 +50,8 @@
"publicKey": {"@id": "sec:publicKey", "@type": "@id", "@container": "@set"},
"publicKeyBase58": "sec:publicKeyBase58",
"publicKeyPem": "sec:publicKeyPem",
"publicKeyJwk": "sec:publicKeyJwk",
"publicKeyHex": "sec:publicKeyHex",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"publicKeyHex": "sec:publicKeyHex",

publicKeyHex was not agreed to in the public key discussions... only publicKeyBase58 and publicKeyPem were allowed. publicKeyHex may have to use the extension registry.

@OR13 OR13 closed this Feb 6, 2020
@OR13
Copy link
Contributor Author

OR13 commented Feb 6, 2020

I'll wait for a way to fix this in did core.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants