Skip to content

Add guidance for credential+ld+json for externalproof #1033

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

Merged
merged 9 commits into from
Mar 6, 2023

Conversation

OR13
Copy link
Contributor

@OR13 OR13 commented Feb 10, 2023

Remove proposed guidance that lacked consensus per @dlongley on: #1014


Preview | Diff

@mprorock
Copy link
Contributor

added some suggestions to make it very clear what can and can't be done to make things very simple and hopefully avoid 1) confusion, 2) resulting type confusion issues (i.e. like what we keep seeing in x509 and related openssl CVEs)

Co-authored-by: Dave Longley <[email protected]>
@dlongley
Copy link
Contributor

dlongley commented Feb 10, 2023

added some suggestions to make it very clear what can and can't be done to make things very simple and hopefully avoid 1) confusion, 2) resulting type confusion issues (i.e. like what we keep seeing in x509 and related openssl CVEs)

Of course, adding restrictions do define what can and can't be done. However, the specific restrictions added should have a clear rationale and be technologically defensible.

So, the specific suggestion that you cannot use this media type with a verifiable credential that has an embedded proof ... serves what purpose? Where is that useful and what are the pros / cons?

EDIT: Perhaps it's best to keep this thread of the conversation in #1014.

@mprorock
Copy link
Contributor

added some suggestions to make it very clear what can and can't be done to make things very simple and hopefully avoid 1) confusion, 2) resulting type confusion issues (i.e. like what we keep seeing in x509 and related openssl CVEs)

Of course, adding restrictions do define what can and can't be done. However, the specific restrictions added should have a clear rationale and be technologically defensible.

So, the specific suggestion that you cannot use this media type with a verifiable credential that has an embedded proof ... serves what purpose? Where is that useful and what are the pros / cons?

converting to approve - did not mean to take discussion out of #1014

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.

LGTM. Don't know about @mprorock's addition... on the fence about that one (but wouldn't object if it was merged w/ the corrections).

Copy link
Member

@iherman iherman left a comment

Choose a reason for hiding this comment

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

This PR seems to be unfinished. It refers to the term of "external proof", but it does not define what this means. Until that is properly defined, this is incomplete and should not be merged imho.

Co-authored-by: Ted Thibodeau Jr <[email protected]>
@OR13
Copy link
Contributor Author

OR13 commented Feb 24, 2023

@iherman external proof is defined here:

https://w3c.github.io/vc-data-model/#proofs-signatures

However I agree that proof is in general not defined sufficiently in the core data model.

Which is why people can't understand its relationship to credential vs verifiable credential.

@iherman
Copy link
Member

iherman commented Feb 25, 2023

@iherman external proof is defined here:

https://w3c.github.io/vc-data-model/#proofs-signatures

I stand corrected. But #1033 (comment) explains why I went wrong: in the new section the term "external proof" appeared as a definition and not as a reference.

Co-authored-by: Ivan Herman <[email protected]>
@OR13
Copy link
Contributor Author

OR13 commented Mar 1, 2023

@iherman accepted your change suggestion, can you please re-review

@iherman iherman self-requested a review March 2, 2023 11:53
Co-authored-by: Dave Longley <[email protected]>
@msporny
Copy link
Member

msporny commented Mar 6, 2023

Editorial, multiple reviews, changes requested and made, no objections, merging.

@msporny msporny merged commit f12c4a1 into main Mar 6, 2023
@msporny msporny deleted the feat/remove-guidance-for-embedded-proof branch March 6, 2023 23:26
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.

7 participants