Update API Component Overview to include workflows.

[msporny]

This PR is an attempt to address issue #513 by adding the Workflow Service to callers of existing issuing/verifying APIs.

---

Preview | Diff

[eric-schuh]

My only question is how we want to go about updating the tables themselves, the function in the respec-oas that creates the tables should be able to handle an array of endpoints fine, but we will also will need to check index.html for each of the components that have changes, here:

vc-api/index.html

Line 838 in 1575102

<h3>API Component Overview</h3>

I think the best way to go about this may be to push the updates to the x-expectedCaller so that the yaml is accurate, and then make any changes needed to the index.html/respec-oas as it should be easy to spot errors in the main github.io page here:

https://w3c-ccg.github.io/vc-api/#api-component-overview

For instance right now DELETE /credential/{id} on the Issuer and Holder Services should probably not be listing both Issuer Coordinator AND Holder Coordinator as expected callers.

[TallTed]

lgtm

[kezike]

@msporny

Since we are looking at the API Component Overview table in this PR, can I ask about the GET /workflows/{localWorkflowId} endpoint in exchanges.yml?

The expected callers is currently listed as Administrators, which I originally agreed with, but I have found that it could be useful for coordinators to have access to this info too. Specifically, knowing the credentialTemplates data is useful for coordinators to know in order to check that the credential data they are managing maps to templates that actually exist in the workflow.

As I am writing this, I also acknowledge that those workflow instances are created by admin clients and hence should be available mostly (arguably exclusively) to those clients. I am just noting that the template quality/consistency check seems like a useful operation for coordinators to perform before creating exchanges. Perhaps, there is another way to do this without exposing this endpoint to them, so I'm open to hearing thoughts on this.

[msporny]

@dlongley would have to weigh in there... I don't have much of a working knowledge of how that endpoint is used outside of administrative interfaces. My only concern here is accidentally leaking non-critical, but still important, secrets to issuer coordinators. If this level of access is needed by issuer coordinators, perhaps we're missing an endpoint (or return value) that would solve this problem for issuer coordinators w/o giving them administrative read access?

[dlongley]

Hmm, I don't think it should ever be necessary for a coordinator to read the workflow configuration.

I would say that if a coordinator has to "discover" what variables to include by querying a workflow, then they also won't know what those variables mean. I think enabling a coordinator to read the workflow configuration would therefore be insufficient for this use case. I suspect that there would have to be a fundamental breakdown in the setup/configuration of the coordinator if it doesn't know exactly what variables to send and what those variables mean when it uses a particular workflow endpoint to create an exchange.

[kezike]

@dlongley To be clear, I am not describing the discovery use case. I agree that coordinators should know the structure of templates. What I am saying is that it would help to verify that the unsigned credential records that the coordinator has created in the past still match against a valid template in the workflow (by validating that it matches an existing template ID or schema in the workflow). This could be useful to ensure that changes in a workflow are captured in unsigned credential records.

[dlongley]

@kezike,

What I am saying is that it would help to verify that the unsigned credential records that the coordinator has created in the past still match against a valid template in the workflow (by validating that it matches an existing template ID or schema in the workflow).

Well, I would say that in any production system, changing an existing workflow that might cause this kind of disruption or a "need for confirmation" by running coordinator processes is an anti-pattern. What would happen if that confirmation failed? Existing service interruption until the workflow was restored or another one was created to allow the (at least two) different processes to continue?

A new workflow should just be created instead -- if there are any existing coordinator processes that could possibly be disrupted by a particular change (i.e., a non-backwards compatible change). IMO, this approach is less complex to implement and less error prone. I would expect it to be much a more manageable approach for decentralized updates -- and it will be what has to happen anyway if the "confirmation failure" risk above is realized.

[kezike]

@kezike

Well, I would say that in any production system, changing an existing workflow that might cause this kind of disruption or a "need for confirmation" by running coordinator processes is an anti-pattern. What would happen if that confirmation failed? Existing service interruption until the workflow was restored or another one was created to allow the (at least two) different processes to continue?

Agreed! Creating a new workflow or being extra careful to only make backwards compatible changes that are tested prior to release is ideal. I saw it more like an extra sanity check in case someone has tampered with the data (perhaps via direct database modification) or accidentally introduced a breaking change to the workflow. It is admittedly an unlikely outcome, so maybe not worth accommodating via exposure of this admin endpoint. I yield my concern.

[kezike]

I was looking for the GET /credentials/{id} and DELETE /credentials/{id} endpoints in issuer.yml because the endpoints in the issuer service table here need to be updated to include workflow service as an expected caller. However, I found that the /credentials/{id} path only exists on holder.yml, which is presumably where these paths are being sourced for this table, per this line in index.html. So, it seems that we need a few things:

1. GET /credentials/{id} endpoint in issuer.yml with the workflow service added as an expected caller
2. DELETE /credentials/{id} endpoint in issuer.yml with the workflow service added as an expected caller
3. [OPTIONAL] Disambiguation between the /credentials/{id} definitions in index.html, such that the proper one is pulled in for different tables (issuer vs. holder)

[kezike]

I noticed that the API component section (currently Section 3.7) is a bit out of sync with the API definition sections (currently Sections 3.8 to 3.12) with respect to the distinction between issuer responsibilities and status responsibilities. In the API component section, the POST /credentials/status endpoint is listed under the status service. Meanwhile, in the API definition sections, this endpoint is listed under the issuer service.

It feels to me like this endpoint should be listed in a new status.yml file that would map to a new API definition section for the status service, which would also bring the API component section and the API definition sections into alignment. I know we've discussed this a bit in the past, but I could not find an open issue for it. My questions:

1. Do we agree that there should be a distinction between issuer and status services in the API definition sections?
2. If yes, should I create an issue for this to avoid bloating the scope of this PR? If no, how should we address the misalignment between the API component section and the API definition sections?

cc @dlongley