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

proofPurpose range must be a Class containing verification relationships (was: Discrepancies between the spec and the vocabulary) #248

Closed
iherman opened this issue Feb 19, 2024 · 7 comments
Assignees
Labels
bug Something isn't working CR1 This item was processed during the first Candidate Recommendation phase. editorial This issue or PR constitutes an editorial change. pr exists A pull request exists to address this issue.

Comments

@iherman
Copy link
Member

iherman commented Feb 19, 2024

The spec says for the proofPurpose property:

proofPurpose
The reason the proof was created MUST be specified as a string that maps to a URL [URL]. […]

The vocabulary specification claims that the range is an xsd:string, i.e., it is a Literal; but the text above says, in a somewhat convoluted way, that the value, in RDF terminology, is not a literal but a "real" resource.

Looking at the usage of the terms, Example 2 says:

{
  "proof": {
	
	"proofPurpose": "assertionMethod",
    
  }
}

The base context does some tricks using "@type":"@vocab", and the result of these lines result, in Turtle, in the following quads:

_:b0 <https://w3id.org/security#proof> _:b1 .
_:b2 <https://w3id.org/security#proofPurpose> <https://w3id.org/security#assertionMethod> _:b1 .

which shows that the range of proofPurpose is an RDF Resource with, in this case, the URL https://w3id.org/security#assertionMethod.

But then it goes further. The vocabulary specification for assertionMethod (and for authentication, etc...) defines this term as a property with a range VerificationMethod. That does not coincide with the usage in the example. Instead, assertionMethod is used as an individual and not as a property. Also, these individuals have nothing to do, relationship-wise, with the Verification Method. Instead, they could be characterized as individuals, all of the type ProofPurpose (this class is not part of the security vocabulary).

Based on all these, I think the following changes must be done on the di vocabulary:

  • Introduce a new class, called ProofPurpose (formally defined in §2.2)
  • the range of proofPurpose is the (new) ProofPurpose class (and not a string literal, as it says now)
  • the following terms: authentication, assertionMethod, capabilityDelegation, capabilityInvocation, and keyAgreement are all to be defined as individuals, all of the type ProofPurpose (and not as properties as they are now).

cc @dlongley @msporny

(Obviously, if we have an agreement, the creation of the corresponding PR will be on me.)

@iherman iherman added the bug Something isn't working label Feb 19, 2024
@iherman iherman self-assigned this Feb 19, 2024
@msporny
Copy link
Member

msporny commented Feb 19, 2024

I think the following changes must be done on the di vocabulary

If we were only considering DI, you'd be correct. The one thing that I haven't thought too deeply about is how this will affect DID Documents, which do use the authentication, assertionMethod, capabilityDelegation, and capabilityInvocation terms as verification relationships:

https://www.w3.org/TR/did-core/#verification-relationships

Same for DI:

https://w3c.github.io/vc-data-integrity/#verification-relationships

How do we align both needs, @iherman?

@iherman
Copy link
Member Author

iherman commented Feb 19, 2024

@msporny

first of all, ouch! 😀 The DI usage is indeed fundamentally different from the DID case. (The section on verification relationships in DI is an almost verbatim copy of the DID spec, I do not think it adds anything to it.) In an ideal situation, I would characterize that as a bug in modeling, but I presume it is too late for that.

But that is my personal reaction, because... in RDF it is allowed to use the same term (ie, the same URI) both for a predicate and (as here) an individual. Ie, my proposal would have to change to say:

  • the following terms: authentication, assertionMethod, capabilityDelegation, capabilityInvocation, and keyAgreement are all to be defined as individuals, all of the type ProofPurpose as well.

The practical problem may be that the yml2vocab tool will (probably) fall on its face if we do that: the tool relies on the separation of individuals from properties (and classes).

Alternatively, we may also play down all this, keep these properties as they are, ie, not adding an explicit type relationship to ProofPurpose into the vocabulary. If we do introduce the range property for proofPurpose (that change proposal still stands) then by virtue of the RDF Semantics that type relationship will be automatically inferred from the real-life proofs by any tool that cares.

So the situation may be salvaged, but I really do believe this modeling, well, "trick" must be documented somewhere to make it explicit. I would note that the current text on verification relationships in the DI spec sounds so much "isolated", without real reference to the rest of the DI spec, that this may be a good opportunity to add such a description in there.

cc this to @pchampin. Maybe he has some additional trick up his sleeve...

@msporny msporny added CR1 This item was processed during the first Candidate Recommendation phase. editorial This issue or PR constitutes an editorial change. labels Feb 25, 2024
@iherman
Copy link
Member Author

iherman commented Mar 15, 2024

Coming back to this issue, here is, I believe what we have to change. The good news is that it is only a change in the vocabulary. There may be an editorial change in the spec if we really want to make it nice.

Here are some excerpts from the vocabulary (I only do this for authentication, the rest are similar). The following statements are in the vocab, and they should't be changed:

sec:verificationMethods rdf:range sec:VerificationMethod.
sec:JsonWebKey rdfs:subclassOf sec:VerificationMethods .
sec:Multikey rdfs:subClassOf sec:VerificationMethod .
sec:authentication rdf:type rdf:Property ;
    rdfs:range VerificationMethod .

There is also this which, I think, should be changed:

sec:proofPurpose rdfs:range xsd:string; 

And the followings should be added:

sec:VerificationRelationship rdf:type rdf:Class ;
	rdfs:isDefinedBy <https://www.w3.org/TR/vc-data-integrity/#verification-relationships> .
	
sec:authentication rdf:type VerificationRelationship .

sec:proofPurpose rdfs:range sec:VerificationRelationship .

I.e., we define a new top level class to model the collection of verification relationships; the current ones are part of the class, and the proof purpose refers to individuals in that class.

I am not yet sure whether that will go through yml2vocab without problems, but I may be able to adapt it. The first question is whether this change is appropriate or not. If so, I can work on it and, eventually, raise a PR.

cc @dlongley @msporny @TallTed

@TallTed
Copy link
Member

TallTed commented Mar 15, 2024

Overall, looks right. One thing —

If I understand correctly, you mean to delete —

sec:proofPurpose rdfs:range xsd:string;

— which will be replaced by this (last of the "to be added" block) —

sec:proofPurpose rdfs:range sec:VerificationRelationship .

— but since the former ends with ;, there must be other predicates applicable to the subject sec:proofPurpose that immediately follow it; therefore, the latter must not end with . but with ; and not be added at some arbitrary location, but replace the former at its existing location.

(In other words, change sec:proofPurpose rdfs:range xsd:string; to sec:proofPurpose rdfs:range sec:VerificationRelationship;.)

@iherman
Copy link
Member Author

iherman commented Mar 15, 2024

@TallTed,

these are copy-pastes from the turtle version of the vocabulary, and I did not pay too much attention to the ';' vs '.'. Sorry.

My intention is to replace the range for proofPurpose from xsd:string to sec:VerificationRelationship. All other statements are unchanged.

@iherman iherman changed the title Discrepancies between the spec and the vocabulary (with bugs in the vocabulary) proofPurpose range must be a Class containing verification relationships (was: Discrepancies between the spec and the vocabulary) Mar 18, 2024
@iherman
Copy link
Member Author

iherman commented Mar 19, 2024

PR #258 has been raised; if that PR is merged, this issue can be closed.

@iherman iherman added the pr exists A pull request exists to address this issue. label Mar 19, 2024
@msporny
Copy link
Member

msporny commented Apr 28, 2024

PR #258 has been merged, closing.

@msporny msporny closed this as completed Apr 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working CR1 This item was processed during the first Candidate Recommendation phase. editorial This issue or PR constitutes an editorial change. pr exists A pull request exists to address this issue.
Projects
None yet
Development

No branches or pull requests

3 participants