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

IssueCredentialResponse does not allow for metadata #291

Closed
msporny opened this issue May 9, 2022 · 4 comments
Closed

IssueCredentialResponse does not allow for metadata #291

msporny opened this issue May 9, 2022 · 4 comments
Assignees
Labels
ready for PR Issue ready to be resolved via a Pull Request

Comments

@msporny
Copy link
Contributor

msporny commented May 9, 2022

When issuing a Verifiable Credential, the response object is the VC itself. This means that the API is not capable of returning any metadata that might be important to the issuance request (such as URLs that can be used to further manage the VC, profiling/debugging information on how long it took to create the VC, and other metadata that would be useful when issuing.

https://github.com/w3c-ccg/vc-api/blob/main/issuer.yml#L68-L69

The suggested fix to this is to add verifiableCredential as an encapsulating object on the return value so that we can provide "other data" in the future. The rest of the API calls should probably follow this pattern as well.

@msporny msporny self-assigned this May 9, 2022
@peacekeeper
Copy link
Member

+1 we should do this.

In our Issuer implementation (https://uniissuer.io/), we've had this evil vendor-lock-in-construct uh I mean "very useful extension point" for a while already:

Screenshot from 2022-05-09 22-28-58

In our design, we modeled it to be roughly similar to the DID Resolution outputs:

  • DID Resolution outputs: didDocument, didResolutionMetadata, didDocumentMetadata
  • VC Issuing outputs: verificationCredential, issuerMetadata, documentMetadata

And we invented an input option returnMetadata=true as another "very useful extension point", to instruct the issuer to return the above structure instead of the credential only. This is again similar to DID Resolution where you can also control whether you just want the DID document, or the DID document plus metadata.

@msporny
Copy link
Contributor Author

msporny commented May 10, 2022

The group discussed this on the 2022-05-10 call. Manu and Markus provided some background. Danube Tech has already implemented this in their API and it's aligned with the way the DID Resolution API operates. They have two buckets of data about issuing/verifying process, then something about the document itself. There was discussion about the number of metadata buckets. @dlongley noted that we might want to do this in a two-step process... support verificationCredential first, then decide which metadata buckets should be supported. @mavarley wondered if there should be a versioned tag -- @msporny noted that it could be done, but might slow us down -- @mavarley was concerned about changes to API vs. test suite -- how are changes to test suite coordinated? As PRs get accepted, previous version/3 months changes. @dlongley noted that versioning might be difficult at this point -- any version might be ok vs. what we want to become official standard... don't want to bake different versions -- test suite should address latest version. @msporny proposed when PRs get merged, we can change test suite. @dmitrizagidulin noted that not versioning yet is good, but we do need to add some text about breaking changes to a specifications.

The group did not object to the following VC API change process for breaking changes: Raise a PR that makes a breaking change to the VC API, ensure there are no objections to the breaking change and then merge, if the change breaks implementations, warn in the test suite output that implementations are catching up. Do not version... yet.

Next step: Create a PR that adds the verifiableCredential property to the top level JSON object when issuing a VC.

/cc @OR13 @mprorock @mkhraisha @nissimsan

@msporny msporny added the ready for PR Issue ready to be resolved via a Pull Request label May 10, 2022
@mavarley
Copy link
Contributor

I think there will be an obvious lag in the test suite to the spec at any rate - writing a test to match the spec will always take time. If the test suite has a "last updated" field to indicate its status vs. the spec this will provide good information on the status of the test and integrated vendor implementations. I wouldn't let the test suite stop us from moving the spec forward.

And +1 to this change; allowing for more meta-data to be returned with the credential data will be very useful.

@msporny
Copy link
Contributor Author

msporny commented Jan 7, 2024

PR #313 addressed this issue. Closing.

@msporny msporny closed this as completed Jan 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready for PR Issue ready to be resolved via a Pull Request
Projects
None yet
Development

No branches or pull requests

3 participants