w3c-ccg / vc-issuer-http-api

A specification for an HTTP API used to issue Verifiable Credentials.
https://w3c-ccg.github.io/vc-http-api/
Other
7 stars 5 forks source link

Make API compositional and use style guide naming convention. #43

Closed dlongley closed 4 years ago

dlongley commented 4 years ago

This PR changes the name for the issuing endpoint and the compose endpoint to fall in line with the proposed style guide naming conventions. It also changes the compose API to only compose a credential -- the output of which can then be sent to the issue API. IMO, this gives us a cleaner separation of concerns and better composition API/reuse.

dlongley commented 4 years ago

@troyronda,

I’m also concerned with the idea of "already composed" mode is being at the root of /credentials path while "compose" is in a subpath.

The reason for this is to follow restful naming convention per the proposed style guide. The /credentials/compose API proposed here does not create or store a resource on the server. It is a controller that performs a function. The /credentials API creates a credential reference resource on the server after issuing the credential.

@peacekeeper,

It makes no sense to me to have an issuing API that only composes but doesn't sign a credential. When would you use that?

You'd use it to compose the credential right before you submit it to the /credentials endpoint that issues it. That allows users to use the template function, if desired and it is supported. Just like you would call two different local functions, where the first function is optional. This matches the requirements (and optionality) of the API.

troyronda commented 4 years ago

It also changes the compose API to only compose a credential

@dlongley I see. I didn't understand from the API description that you were constructing the flow this way.

Composes a credential and returns it in the response body. Support of this part of the API is OPTIONAL for implementations.

troyronda commented 4 years ago

The /credentials API creates a credential reference resource on the server after issuing the credential.

So if I understand the proposal from @dlongley properly:

Due to this last assumption in the proposal, /credentials is being used for both issuance and storage. (rather than having /credentials only for storage along with /credentials/issue and /credentials/compose for their operations).

dlongley commented 4 years ago

@troyronda,

That's right. Nothing is stored on the server without it also having been issued and there's no option to only issue a credential without storing a reference to it. If you want or need to use another service to compose the credential via a template/other options before submitting it to be issued, the /credentials/compose method can be used if made available by the implementer.

troyronda commented 4 years ago

@dlongley Ok that makes sense under those assumptions.

Are there alternative situations where:

(those scenarios could impact the consistency of a combined issue + store operation on /credentials)

llorllale commented 4 years ago

I don't like the direction this PR takes. Composability should be carefully weighed against performance with these network calls.

The overall picture isn't clear to me yet, but I'm on the same boat as @peacekeeper for now - I don't see the usefulness of this particular decomposition ("issuance" + "saving"). And there will be functional overlaps (like "linting").

OR13 commented 4 years ago

I'm in favor of closing this PR, getting better alignment on the API style guide and merging the specs so we don't have as many places to look, split focus on these API design issues.

dlongley commented 4 years ago

This is on hold until after some interop has occurred on the current version. This can be picked up later as more is learned from implementation experience on the current version.

OR13 commented 4 years ago

I suggest closing this PR, and waiting till we are in 1 repo before attempting any further changes.

OR13 commented 4 years ago

I am closing this PR, we need to merge the specs.