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

add data integrity verifiable presentation as proof of possession #66

Merged
merged 23 commits into from
Dec 10, 2023

Conversation

F-Node-Karlsruhe
Copy link
Contributor

Added a linked data proof type for the proof of possession in the OID4VCI flow

Original PR: https://bitbucket.org/openid/connect/pull-requests/487/add-ldp_vp-as-proof-of-possession

Signed-off-by: F-Node-Karlsruhe <christian.fries@eecc.de>
Co-authored-by: Kristina <52878547+Sakurann@users.noreply.github.com>
openid-4-verifiable-credential-issuance-1_0.md Outdated Show resolved Hide resolved
openid-4-verifiable-credential-issuance-1_0.md Outdated Show resolved Hide resolved
openid-4-verifiable-credential-issuance-1_0.md Outdated Show resolved Hide resolved
openid-4-verifiable-credential-issuance-1_0.md Outdated Show resolved Hide resolved
openid-4-verifiable-credential-issuance-1_0.md Outdated Show resolved Hide resolved
F-Node-Karlsruhe and others added 5 commits October 5, 2023 15:13
Co-authored-by: Giuseppe De Marco <giuseppe.demarco@teamdigitale.governo.it>
Co-authored-by: Giuseppe De Marco <giuseppe.demarco@teamdigitale.governo.it>
Co-authored-by: Giuseppe De Marco <giuseppe.demarco@teamdigitale.governo.it>
Co-authored-by: Giuseppe De Marco <giuseppe.demarco@teamdigitale.governo.it>
Signed-off-by: F-Node-Karlsruhe <christian.fries@eecc.de>
Signed-off-by: F-Node-Karlsruhe <christian.fries@eecc.de>
@F-Node-Karlsruhe
Copy link
Contributor Author

F-Node-Karlsruhe commented Oct 17, 2023

For the use case of proof of possession i actually advocate removing the version number 2.0 ldp_vp_2.0-> ldp_vp again as the proof already carries this information itself and it only makes the spec more complicated.

One verifying a ldp_vp should both be capable of verifying a 1.0 as well as a 2.0 depending on the referenced context. Especially bearing in mind that the data model only changed slightly and the general form of the presentation remains the same. Most libraries already are capable of treating both versions at the same time.

This naming is also more consistent with with the rest of the spec, where all DI-Proof credentials are typed as ldp_vc

@awoie

F-Node-Karlsruhe and others added 2 commits October 17, 2023 11:22
Co-authored-by: Gabe <7622243+decentralgabe@users.noreply.github.com>
Co-authored-by: Gabe <7622243+decentralgabe@users.noreply.github.com>
@dlongley
Copy link
Contributor

For the use case of proof of possession i actually advocate removing the version number 2.0 ldp_vp_2.0-> ldp_vp again as the proof already carries this information itself and it only makes the spec more complicated.

One verifying a ldp_vp should both be capable of verifying a 1.0 as well as a 2.0 depending on the referenced context. Especially bearing in mind that the data model only changed slightly and the general form of the presentation remains the same. Most libraries already are capable of treating both versions at the same time.

+1 to keeping ldp_vp for simplicity.

@Sakurann
Copy link
Collaborator

@dlongley does the mechanism described in the PR looks good to you? would appreciate an approval/request for changes.

Signed-off-by: F-Node-Karlsruhe <christian.fries@eecc.de>
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.

Approving with a few comments, one a minor clean up and the other a comment on language used in specs (generally) around "proof of possession" that promises / requires more than is technically possible.

openid-4-verifiable-credential-issuance-1_0.md Outdated Show resolved Hide resolved
@dlongley
Copy link
Contributor

@Sakurann,

does the mechanism described in the PR looks good to you? would appreciate an approval/request for changes.

Yes, approved -- with a couple of minor suggestions. This would be very helpful, thanks!

Co-authored-by: Dave Longley <dlongley@digitalbazaar.com>
Copy link
Collaborator

@Sakurann Sakurann left a comment

Choose a reason for hiding this comment

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

example needs to be moved to an appropriate section

openid-4-verifiable-credential-issuance-1_0.md Outdated Show resolved Hide resolved
Copy link
Collaborator

@Sakurann Sakurann left a comment

Choose a reason for hiding this comment

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

concrete changes suggested

F-Node-Karlsruhe and others added 6 commits November 2, 2023 07:52
Co-authored-by: Kristina <52878547+Sakurann@users.noreply.github.com>
Signed-off-by: F-Node-Karlsruhe <christian.fries@eecc.de>
Co-authored-by: Kristina <52878547+Sakurann@users.noreply.github.com>
Signed-off-by: F-Node-Karlsruhe <christian.fries@eecc.de>
Signed-off-by: F-Node-Karlsruhe <christian.fries@eecc.de>
Co-authored-by: Kristina <52878547+Sakurann@users.noreply.github.com>
Copy link
Collaborator

@Sakurann Sakurann left a comment

Choose a reason for hiding this comment

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

looks good. approving assuming my one suggestion will be accepted

openid-4-verifiable-credential-issuance-1_0.md Outdated Show resolved Hide resolved
F-Node-Karlsruhe and others added 2 commits December 1, 2023 08:44
Co-authored-by: Kristina <52878547+Sakurann@users.noreply.github.com>
Signed-off-by: F-Node-Karlsruhe <christian.fries@eecc.de>
@Sakurann Sakurann added this to the ID-1 milestone Dec 7, 2023
Co-authored-by: Kristina <52878547+Sakurann@users.noreply.github.com>
@Sakurann Sakurann merged commit bc9a5da into openid:main Dec 10, 2023
2 checks passed
@Sakurann
Copy link
Collaborator

@F-Node-Karlsruhe thank you for the PR and working with us. apologies it took the wg a while to merge this!

@Sakurann
Copy link
Collaborator

@F-Node-Karlsruhe what name can I use to add you to the contributors in the acknowledgements section of the specificaiton?

@F-Node-Karlsruhe
Copy link
Contributor Author

@F-Node-Karlsruhe what name can I use to add you to the contributors in the acknowledgements section of the specificaiton?

Christian Fries - European EPC Competence Center

https://www.researchgate.net/profile/Christian_Fries4

thank you for mentioning me :)

Sakurann added a commit that referenced this pull request Jan 18, 2024
Sakurann added a commit that referenced this pull request Jan 22, 2024
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.

6 participants