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.

Sign up for GitHub

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

Add DataIntegrityProof to credentials/v2 JSON-LD Context. #943

Merged
merged 1 commit into from
Oct 23, 2022
Merged

Add DataIntegrityProof to credentials/v2 JSON-LD Context. #943

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 64 additions & 1 deletion contexts/credentials/v2
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@
},
"proof": {
"@id": "https://w3id.org/security#proof",
"@type": "@id",
"@type": "@id",
"@container": "@graph"
},
"verifiableCredential": {
Expand All @@ -87,6 +87,69 @@
"@container": "@graph"
Copy link
Contributor

Choose a reason for hiding this comment

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

sigh.... this impacts a lot of how the data will be processed.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, it's important to separate the proof metadata from the VC -- putting each in their own isolated graphs does this (and is what "@container": "@graph" does).

}
}
},

"DataIntegrityProof": {
"@id": "https://w3id.org/security#DataIntegrityProof",
Copy link
Contributor

Choose a reason for hiding this comment

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

Link is dead: https://w3id.org/security#DataIntegrityProof

Copy link
Contributor

Choose a reason for hiding this comment

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

is there a corresponding PR on security context?

Copy link
Member Author

Choose a reason for hiding this comment

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

Not yet, no. I was going to do that if/when the Data Integrity JSON-LD Context was merged in the Data Integrity spec repo.

Copy link
Member Author

Choose a reason for hiding this comment

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

There is now: w3c/vc-data-integrity#65

"@context": {
"@protected": true,
"id": "@id",
"type": "@type",
"challenge": "https://w3id.org/security#challenge",
Copy link
Contributor

Choose a reason for hiding this comment

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

example of a good term definition: https://w3id.org/security#challenge

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, the security vocab is in serious need of some love right now. I expect we'll clean it up before or shortly after FPWD. We should certainly make sure that all the term entries exist in the security vocab before FPWD.

Copy link
Member Author

Choose a reason for hiding this comment

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

There is a PR to address this now: w3c/vc-data-integrity#65

"created": {
"@id": "http://purl.org/dc/terms/created",
"@type": "http://www.w3.org/2001/XMLSchema#dateTime"
},
"domain": "https://w3id.org/security#domain",
"expires": {
"@id": "https://w3id.org/security#expiration",
"@type": "http://www.w3.org/2001/XMLSchema#dateTime"
},
"nonce": "https://w3id.org/security#nonce",
Copy link
Contributor

Choose a reason for hiding this comment

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

why we have this and challenge?... prefer 1.

Copy link
Contributor

Choose a reason for hiding this comment

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

They have different semantics (challenge is never chosen by the signer and nonce usually is). That being said, I prefer just having challenge and moving any use of nonces that is specific to a particular type of crypto (e.g., BBS) into the proofValue via some internal encoding, just like any other crypto / proof-specific parameters or values should be.

In other words, proofValue should be self-contained and opaque to applications and only proof verifiers would decode it and break apart its constituent parts and do whatever is needed. This is comparable to a jws (that internally has parts like header and payload, etc.). That is where a signer-provided nonce belongs, IMO.

The only reason to expose any values via directly accessible proof properties (i.e., not encoded within proofValue) is if applications clearly have a need for them. Otherwise it just generates more churn for people writing generalized / layered code. If such a use can be argued for nonce then there's a reason to have it -- otherwise I don't think it's necessary.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I almost removed nonce but then thought that folks might complain. nonce is currently on the chopping block, we need to see if people see a need to keep it around.

Copy link
Contributor

Choose a reason for hiding this comment

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

As a developer, I prefer not to start a new software project with a bunch of unused variables.

Copy link
Member Author

Choose a reason for hiding this comment

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

@tplooker had some concerns around removing nonce -- thoughts, @tplooker ?

"proofPurpose": {
"@id": "https://w3id.org/security#proofPurpose",
"@type": "@vocab",
"@context": {
"@protected": true,
"id": "@id",
"type": "@type",
"assertionMethod": {
"@id": "https://w3id.org/security#assertionMethod",
Copy link
Contributor

Choose a reason for hiding this comment

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

These are for VCs

"@type": "@id",
"@container": "@set"
},
"authentication": {
"@id": "https://w3id.org/security#authenticationMethod",
Copy link
Contributor

Choose a reason for hiding this comment

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

These are for VPs

"@type": "@id",
"@container": "@set"
},
"capabilityInvocation": {
"@id": "https://w3id.org/security#capabilityInvocationMethod",
"@type": "@id",
"@container": "@set"
},
"capabilityDelegation": {
Copy link
Contributor

Choose a reason for hiding this comment

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

What does this have to do with credentials ?

Copy link
Contributor

Choose a reason for hiding this comment

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

The term definitions here are identical to the ones used in the data integrity context because they are intentionally @protected to simplify processing. Using different ones would be confusing and would raise a redefinition error if anyone combined the contexts. This is better for developers.

Copy link
Contributor

@OR13 OR13 Oct 14, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

To be clear, this has to do with using this context for things that are not verifiable credentials correct?

Can we at least provide an informative use case for how this might be related to verifiable credentials?

Some thing like, capabilityDelegation can be used to grant access to verifiable credentials stored on remote services?

Copy link
Member Author

Choose a reason for hiding this comment

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

Explanation and plan to move forward is here: #943 (comment)

"@id": "https://w3id.org/security#capabilityDelegationMethod",
"@type": "@id",
"@container": "@set"
},
Comment on lines +131 to +135
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"capabilityDelegation": {
"@id": "https://w3id.org/security#capabilityDelegationMethod",
"@type": "@id",
"@container": "@set"
},

Copy link
Contributor

Choose a reason for hiding this comment

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

let's keep this out for now - might make sense, but is a dedicated topic on its own

Copy link
Contributor

Choose a reason for hiding this comment

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

See: #943 (comment)

Copy link
Member Author

Choose a reason for hiding this comment

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

If we remove this, we deviate from what the Data Integrity spec's main context. We should have this discussion over there instead of over here.

In the meantime, we might want to think about what it means to not include the non-VC-ecosystem related things in the vc/v2 context. These are values that we expect to exist in proofPurpose, which is a Data Integrity thing... I'm not sure mixing and matching is the best approach, so we'd need to explore what happens if we decide to start mixing and matching (and the ways that could all go horribly wrong).

Copy link
Contributor

Choose a reason for hiding this comment

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

Same comment as above, if we keep it in, we need to do a better job of explaining it.... that means adding informative text.

Copy link
Member Author

Choose a reason for hiding this comment

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

Explanation and plan to move forward is here: #943 (comment)

"keyAgreement": {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is encryption in scope for VPs?

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think that's relevant. This PR is about copying a protected term definition from the data integrity context into the credentials context for the convenience of developers. That means copying it so it is exactly the same. What that definition is / should / shouldn't contain is not relevant in this PR.

Copy link
Member Author

Choose a reason for hiding this comment

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

Explanation and plan to move forward is here: #943 (comment)

"@id": "https://w3id.org/security#keyAgreementMethod",
"@type": "@id",
"@container": "@set"
}
}
},
"cryptosuite": "https://w3id.org/security#cryptosuite",
"proofValue": {
"@id": "https://w3id.org/security#proofValue",
"@type": "https://w3id.org/security#multibase"
},
"verificationMethod": {
"@id": "https://w3id.org/security#verificationMethod",
"@type": "@id"
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably worth several calls on this topic.... what this means is likely not well understood, by the group.

Copy link
Contributor

Choose a reason for hiding this comment

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

+1 - i would like clarity on this

Copy link
Contributor

Choose a reason for hiding this comment

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

It means that the verificationMethod value will be interpreted as a URL.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can be clearer... it means the verificationMethod will be an IRI which can be dereferenced... does it mean it's allowed to be relative or absolute?

Copy link
Member Author

Choose a reason for hiding this comment

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

It means it is a URL (relative or absolute). Relative URLs will be resolved relative to the document base, which can be set to any URL via @base. This is described in: https://www.w3.org/TR/json-ld11/#base-iri

}
}
}
}
}