-
Notifications
You must be signed in to change notification settings - Fork 110
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 the formal vocabulary files #920
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The way the documentations of terms are written is ambiguous. Some are focused on the conceptual level (e.g. "The value of the credentialSchema property MUST be one or more data schemas") while other are focused on the JSON-LD syntactic level (e.g. "The value of the credentialStatus property MUST include the id property, which MUST be a URL").
This should be harmonized, all focusing on the conceptual level, if indeed the JSON-LD context is maintained elsewhere.
In the same line, the Term Definitions
section in the HTML document seems to be mostly focused on JSON-LD, not RDFS, so I would simply remove it.
I would agree with this. I note that the definitions are usually come as a copy of the data model spec. I still have to familiarize myself more with the model before I could provide a set of alternative texts, and the spec may also change, so I would keep that on a todo list and do that at some point. Again, point well taken.
Yes, I was wondering about that myself. This section does not bring anything new and valuable to the reader, and I was tempted to remove it altogether, but I shied away from it for now. @msporny is it o.k. to remove it? |
@pchampin just to be on the safe side: is the text used for |
They look good to me, but in the class description, I would replace "Instances of this class identify RDF graphs" with "Instances of this class are RDF graphs".
The change above keeps us on the safe side, I think. It does not imply any means by which the object of the |
Well... a class is a collection of resources, more exactly URI-s to resources, right? Whether these URI-s are graphs or whether they identify graphs... we can get into a long discussion, see also https://www.w3.org/TR/rdf11-datasets/. That is why I tried to be as neutral as possible.
|
No, I wouldn't put it that way. Quoting [RDF11-MT]:
"A set of things in the universe" (a.k.a. resources), not a set of IRIs. The fact that those resources may be identified by IRIs is irrelevant to the class itself. So, to develop my argument above:
To fill that gap, the common practice with VC is to put the graph identified by |
Ok, I stand corrected...
Based on what you said above, I am fine with this formulation.
Right. But I will not put this into the text itself:-) Just remaining terse. |
@msporny @dlongley I have added a description (after having re-read the spec) for the |
(maybe that deserves a separate issue, you tell me.) The specification makes a difference between a "credential" and a "verifiable credential", as well as a "presentation" and a "verifiable presentation", that could be roughly characterized by saying that a "verifiable credential" (resp. "verifiable presentation") is a temper evident "credential" (resp. "presentation"). If this distinction is important, then it would deserve a separation in the ontology, by defining a "Credential" class and a "Verifiable Credential" subclass. (resp. "Presentation" and "Verifiable Presentation"). This is easy to add to the ontology. If we do so, the question is whether the domain of the various properties should be verifiable credentials or just credentials. It strikes that almost none of the properties in this vocabulary depend on the "verifiable" aspect, maybe except for "verifiableCredential" which, at least with its name, is bound to "verifiable". The only property that MUST have a domain of "verifiable ..." would be WDYT? |
Co-authored-by: Ted Thibodeau Jr <tthibodeau@openlinksw.com>
IIRC, adding types in the ontology for this was avoided in the 1.0 work to keep down the complexity. I imagine we'll want to continue doing so as a trade off (simplicity over theoretrical purity), but we'll have to see. For this reason, I don't think we should add it in this PR. |
Personally, I would still be in favour of having this (or, alternatively, the spec text should not make a difference between "Credential" and "Verifiable Credential") but I am o.k. postponing this discussion. If we close this PR I may raise an issue just to keep a trace for ourselves. |
@@ -0,0 +1,382 @@ | |||
{ | |||
"@context": [ | |||
"https://w3id.org/security/v2", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm... prefer to not do this... this will trigger an entire 2 other network requests, since v2 relies on v1... thats 3 network requests to load the complete v2 context...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can be avoided by simply copying the terms required by the vc data model... instead of copying everything...for example:
{
"@context": [{
"@version": 1.1
}, "https://w3id.org/security/v1", {
"AesKeyWrappingKey2019": "sec:AesKeyWrappingKey2019",
"DeleteKeyOperation": "sec:DeleteKeyOperation",
"DeriveSecretOperation": "sec:DeriveSecretOperation",
"EcdsaSecp256k1Signature2019": "sec:EcdsaSecp256k1Signature2019",
"EcdsaSecp256r1Signature2019": "sec:EcdsaSecp256r1Signature2019",
"EcdsaSecp256k1VerificationKey2019": "sec:EcdsaSecp256k1VerificationKey2019",
"EcdsaSecp256r1VerificationKey2019": "sec:EcdsaSecp256r1VerificationKey2019",
"Ed25519Signature2018": "sec:Ed25519Signature2018",
"Ed25519VerificationKey2018": "sec:Ed25519VerificationKey2018",
"EquihashProof2018": "sec:EquihashProof2018",
"ExportKeyOperation": "sec:ExportKeyOperation",
"GenerateKeyOperation": "sec:GenerateKeyOperation",
"KmsOperation": "sec:KmsOperation",
"RevokeKeyOperation": "sec:RevokeKeyOperation",
"RsaSignature2018": "sec:RsaSignature2018",
"RsaVerificationKey2018": "sec:RsaVerificationKey2018",
"Sha256HmacKey2019": "sec:Sha256HmacKey2019",
"SignOperation": "sec:SignOperation",
"UnwrapKeyOperation": "sec:UnwrapKeyOperation",
"VerifyOperation": "sec:VerifyOperation",
"WrapKeyOperation": "sec:WrapKeyOperation",
"X25519KeyAgreementKey2019": "sec:X25519KeyAgreementKey2019",
"allowedAction": "sec:allowedAction",
"assertionMethod": {"@id": "sec:assertionMethod", "@type": "@id", "@container": "@set"},
"authentication": {"@id": "sec:authenticationMethod", "@type": "@id", "@container": "@set"},
"capability": {"@id": "sec:capability", "@type": "@id"},
"capabilityAction": "sec:capabilityAction",
"capabilityChain": {"@id": "sec:capabilityChain", "@type": "@id", "@container": "@list"},
"capabilityDelegation": {"@id": "sec:capabilityDelegationMethod", "@type": "@id", "@container": "@set"},
"capabilityInvocation": {"@id": "sec:capabilityInvocationMethod", "@type": "@id", "@container": "@set"},
"caveat": {"@id": "sec:caveat", "@type": "@id", "@container": "@set"},
"challenge": "sec:challenge",
"ciphertext": "sec:ciphertext",
"controller": {"@id": "sec:controller", "@type": "@id"},
"delegator": {"@id": "sec:delegator", "@type": "@id"},
"equihashParameterK": {"@id": "sec:equihashParameterK", "@type": "xsd:integer"},
"equihashParameterN": {"@id": "sec:equihashParameterN", "@type": "xsd:integer"},
"invocationTarget": {"@id": "sec:invocationTarget", "@type": "@id"},
"invoker": {"@id": "sec:invoker", "@type": "@id"},
"jws": "sec:jws",
"keyAgreement": {"@id": "sec:keyAgreementMethod", "@type": "@id", "@container": "@set"},
"kmsModule": {"@id": "sec:kmsModule"},
"parentCapability": {"@id": "sec:parentCapability", "@type": "@id"},
"plaintext": "sec:plaintext",
"proof": {"@id": "sec:proof", "@type": "@id", "@container": "@graph"},
"proofPurpose": {"@id": "sec:proofPurpose", "@type": "@vocab"},
"proofValue": "sec:proofValue",
"referenceId": "sec:referenceId",
"unwrappedKey": "sec:unwrappedKey",
"verificationMethod": {"@id": "sec:verificationMethod", "@type": "@id"},
"verifyData": "sec:verifyData",
"wrappedKey": "sec:wrappedKey"
}]
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Which then loads v1:
{
"@context": {
"id": "@id",
"type": "@type",
"dc": "http://purl.org/dc/terms/",
"sec": "https://w3id.org/security#",
"xsd": "http://www.w3.org/2001/XMLSchema#",
"EcdsaKoblitzSignature2016": "sec:EcdsaKoblitzSignature2016",
"Ed25519Signature2018": "sec:Ed25519Signature2018",
"EncryptedMessage": "sec:EncryptedMessage",
"GraphSignature2012": "sec:GraphSignature2012",
"LinkedDataSignature2015": "sec:LinkedDataSignature2015",
"LinkedDataSignature2016": "sec:LinkedDataSignature2016",
"CryptographicKey": "sec:Key",
"authenticationTag": "sec:authenticationTag",
"canonicalizationAlgorithm": "sec:canonicalizationAlgorithm",
"cipherAlgorithm": "sec:cipherAlgorithm",
"cipherData": "sec:cipherData",
"cipherKey": "sec:cipherKey",
"created": {"@id": "dc:created", "@type": "xsd:dateTime"},
"creator": {"@id": "dc:creator", "@type": "@id"},
"digestAlgorithm": "sec:digestAlgorithm",
"digestValue": "sec:digestValue",
"domain": "sec:domain",
"encryptionKey": "sec:encryptionKey",
"expiration": {"@id": "sec:expiration", "@type": "xsd:dateTime"},
"expires": {"@id": "sec:expiration", "@type": "xsd:dateTime"},
"initializationVector": "sec:initializationVector",
"iterationCount": "sec:iterationCount",
"nonce": "sec:nonce",
"normalizationAlgorithm": "sec:normalizationAlgorithm",
"owner": {"@id": "sec:owner", "@type": "@id"},
"password": "sec:password",
"privateKey": {"@id": "sec:privateKey", "@type": "@id"},
"privateKeyPem": "sec:privateKeyPem",
"publicKey": {"@id": "sec:publicKey", "@type": "@id"},
"publicKeyBase58": "sec:publicKeyBase58",
"publicKeyPem": "sec:publicKeyPem",
"publicKeyWif": "sec:publicKeyWif",
"publicKeyService": {"@id": "sec:publicKeyService", "@type": "@id"},
"revoked": {"@id": "sec:revoked", "@type": "xsd:dateTime"},
"salt": "sec:salt",
"signature": "sec:signature",
"signatureAlgorithm": "sec:signingAlgorithm",
"signatureValue": "sec:signatureValue"
}
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A better starting point would be:
{
"@context": {
"@version": 1.1,
"@vocab": "https://w3id.org/security#"
}
}
Since you can see that all the security concerns are actually mapped to w3id.org... not w3c.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-1 for cascaded loading of security/v2 and security/v1 -- we want to get away from that in this iteration of the work.
-1 for use of @vocab
for any JSON-LD context that touches security, we want to be specific about the terms we use, especially when those things are going to be digitally signed. Using @vocab
causes the new "undefined terms detected" safety features of jsonld.js to not fire, and we definitely don't want to silence those safeties.
We should try to start from here since its much simpler (though, I'm fine w/ doing all of this in another PR):
https://github.com/w3c/vc-data-model/blob/main/contexts/credentials/v2
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think people are reacting to the fact that this PR re-defines the v2 JSON-LD Context...
Does it? I thought that was a totally different file that only the vocab used.
Re: throwing errors when using @vocab
...
Using @vocab
is an explicit signal that the author does not want errors thrown for undefined terms, but instead wants them folded into some vocabulary without knowing what they are. So I don't expect safe
mode to ever throw for those cases. Using @vocab
is a sort of middle ground in the security space -- where unknown terms will be signed, but there will be no globally unambiguous definition for their meaning. The introduction of a new context could also "define them later" -- in ways that may contradict their previous (locally understood) meaning. This is the danger of using @vocab
, but if that danger is understood, i.e., there is zero reliability with those terms at all, perhaps that's an acceptable compromise for allowing their use.
We should get a clear understanding for the potential advantage sought by using @vocab
so it can be weighed transparently and appropriately against the disadvantages. Of course, if @vocab
is only being used as a bad substitute for safe
mode -- then it seems to me that safe
mode is clearly better. I think there's more people are looking for though.
All that being said, the implementation of the new safe
mode feature in jsonld.js (at least) actually uses a more comprehensive internal event system that is able to emit all kinds of events for debugging purposes (including ones around the use of @vocab
). That feature hasn't been externalized yet with a clean API. But that feature will be a development / debugging tool as opposed to a data validation / protection feature like safe
mode is.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think people are reacting to the fact that this PR re-defines the v2 JSON-LD Context in a way that 1) we don't want, and 2) is in conflict with what's already agreed to as a baseline by the group:
Everything else you said sounds fine, Ivan. :)
This PR does not re-defines the JSON-LD context (and it should not), it uses a (in my view, bloated) context within the vocabulary definition in JSON-LD.
See my comment elsewhere: #920 (comment). Should I take that branch and merge with this one, to get this issue completely out of the way?
Of course, once this PR is settled, we have to look at the differences between the vocabulary and the v2 context to bring them in line. That is the next step, for sure.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think people are reacting to the fact that this PR re-defines the v2 JSON-LD Context...
Does it? I thought that was a totally different file that only the vocab used.
Exactly. And it uses it "internally", not for external usage.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dlongley wrote:
I thought that was a totally different file that only the vocab used.
Yes, I see that now... but it's not something the vocab uses, it's just a different representation of the human-readable vocabulary document. It took me a bit to remember what Gregg's ruby tool actually does (generates a human-readable and machine-readable vocabulary definition -- NOT a JSON-LD Context).
@iherman wrote:
And it uses it "internally", not for external usage.
Ah, I see now that the "JSON-LD Context" is just "the JSON-LD representation of the Vocabulary". I think both @OR13 and @mprorock were confused in the same way that I was.
Perhaps if we rename these files to vocabulary.(html|ttl|jsonld) it would lead to less confusion? Granted, actually reading the entirety of the file might have done that, but I do remember scanning down quite a ways... but not far enough to get to the vocabulary definitions at the bottom.
You can ignore all of my suggestions (except the one to rename the files) at this point. :P
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps if we rename these files to vocabulary.(html|ttl|jsonld) it would lead to less confusion? Granted, actually reading the entirety of the file might have done that, but I do remember scanning down quite a ways... but not far enough to get to the vocabulary definitions at the bottom.
You can ignore all of my suggestions (except the one to rename the files) at this point. :P
I am fine renaming the files; it is probably a good idea. I think I will also go ahead and simplify the json-ld file to the strict minimum to avoid any misunderstandings.
But... I have to call it a day, this will be done tomorrow...
vocab/credentials/v2/credentials.ttl
Outdated
@prefix owl: <http://www.w3.org/2002/07/owl#> . | ||
@prefix rdf: <http://www.w3.org/1999/02/22-rdf-syntax-ns#> . | ||
@prefix rdfs: <http://www.w3.org/2000/01/rdf-schema#> . | ||
@prefix cred: <https://w3.org/2018/credentials#> . |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how come we don't see
https://w3id.org/security#
In this list....
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because the vocabulary does not refer/use this namespace.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(see also my comment in #920 (comment))
<section> | ||
<h2>Introduction</h2> | ||
<p property="dc:description"><%= ont["dc:description"]["en"] %></p> | ||
<p>This specification makes use of the following namespaces:</p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I really hope we can simplify this list in v2.... especially removing odrl.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that this PR has not yet looked at the v2 version of the @context
. In fact, it is only an attempt to clean up the 1.1 vocabulary.
I agree that the ODRL reference may be an overkill.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe the only blocking issue in this PR is the definition of vocab/credentials/v2/credentials.jsonld
-- ideally, we should leave that (and all generated files), out of being checked in and perhaps generate them during the build process?
I am not sure what you say, @msporny. The As I said, I have created a separate branch ( |
What I'm saying is that if we have auto-generated files, we should use a Github process to auto-generate them instead of manually committing the files into Github. When we manually commit auto-generated files, there is a good chance that they will diverge from the underlying source files. It also unnecessarily pollutes the Github history with a bunch of check ins for auto-generated files. This doesn't have to be fixed in this PR, can be fixed in a future PR, and so we don't need to hold up the merge here to fix the longer-term issue. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only one minor nit for a file that was probably checked in by mistake. Otherwise, LGTM.
.vscode/bookmarks.json
Outdated
{ | ||
"files": [ | ||
{ | ||
"path": "vocab/credentials/v2/mk_vocab.rb", | ||
"bookmarks": [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We shouldn't commit .vscode files for personal developer bookmarks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops... I usually have this on .gitignore, but I missed it this time. Sorry about that.
Substantive, multiple reviews, changes requested and made, no objections, merging. |
This PR creates a v2 version of the RDFS vocabulary definition of the data model. Eventually, just as the context v2 will supersede the current v1, this vocabulary definition will supersede the current one.
The plan is to catch up with the evolution of v2 context and keep the two in sync.
Because there is no diff (and there were lots of small changes that would make a diff unusable anyway, here are the main changes on the file(s):
Editorial changes:
owl:versionInfo
is not o.k., it would have to take an IRI as an object)id
attribute to all terms, so that something like.../#evidence
would properly dereference.Handling bugs in the previous version:
odrl
namespace (which is used) to the list of used namespaces in the HTML document , and removedsec
(which is not used)VerifiableCredentialGraph
class and changed the domain of theverifiableCredential
property, to fix Bug in the credential vocabulary specification #770dc:imports
triple that had no objectNote that this PR does not yet sync up with the latest version of the context file, as merged in #905. That should come in a new PR if and when this one is accepted and merged.
To make the review easier, here are the previews of the three generated file:
Fix #916