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 readme section about the EnvelopingProof tag usage. #128

Merged
merged 1 commit into from
Oct 17, 2024

Conversation

PatStLouis
Copy link
Collaborator

This is to partially address the concerns raised in #127.

It's not a requirement to use data integrity to be conformant with this test suite. This test suite doesn't test the securing mechanism, only the data model.

Copy link

@decentralgabe decentralgabe left a comment

Choose a reason for hiding this comment

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

Thanks for documenting this.

Copy link
Contributor

@aljones15 aljones15 left a comment

Choose a reason for hiding this comment

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

approving, but do remember the issuer.id becomes issuer in the actual VC which is relevant as an issuer should not being issuing a VC from another issuer.

@BigBlueHat
Copy link
Member

approving, but do remember the issuer.id becomes issuer in the actual VC which is relevant as an issuer should not being issuing a VC from another issuer.

@PatStLouis @decentralgabe any thoughts on what you'd expect to see set as issuer.id in an enveloping proof scenario? We typically recommend/support did:key or HTTP URL values there for the test suites. Thoughts?

@aljones15
Copy link
Contributor

approving, but do remember the issuer.id becomes issuer in the actual VC which is relevant as an issuer should not being issuing a VC from another issuer.

@PatStLouis @decentralgabe any thoughts on what you'd expect to see set as issuer.id in an enveloping proof scenario? We typically recommend/support did:key or HTTP URL values there for the test suites. Thoughts?

I believe kid is the issuer: https://w3c.github.io/vc-jose-cose/#jwt-issuer

it should be a did:key for the sake of simplicity and being in line with this test suite.
Not sure how you can get around the alg though which even in the spec is set to eddsa.

@decentralgabe
Copy link

decentralgabe commented Oct 7, 2024

the semantic layer and securing layer are distinct. the issuer property will still be present after you decode the vc jose cose payload.

@PatStLouis
Copy link
Collaborator Author

PatStLouis commented Oct 7, 2024

I would say for envelopes it would be a did:jwk:?

edit: I also want to suggest that for this specific test-suite, it doesn't really matter. As long as it matches the vcdm 2.0 spec of being a valid URI. It would matter more in something like a VC-JOSE test suite, which is another reason why I would like to keep this type of testing separate and focus on the data model here, not the semantics of the securing mechanism.

@PatStLouis
Copy link
Collaborator Author

I will also open a PR on the implementations repo to add the same information.

@decentralgabe
Copy link

There are no requirements for it to be a did:jwk, did:key, or any other DID method (or controller document URI) would work too.

@aljones15
Copy link
Contributor

@decentralgabe all other test suites use did:key at the moment so that would ensure other implementers at least support the did method used for issuance. We do actually specify only did:key for issuer/verificationMethod in some of the other suites although I believe the VC 2.0 suite does not have such a condition set down in it.

@decentralgabe
Copy link

thanks @aljones15 that makes sense. we can stick to did:jwk in our test suite to make things easier.

@PatStLouis PatStLouis merged commit 202d028 into main Oct 17, 2024
2 checks passed
@PatStLouis PatStLouis deleted the enveloping-proof-documentation branch October 17, 2024 18:59
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.

5 participants