OpenID4VCI icon indicating copy to clipboard operation
OpenID4VCI copied to clipboard

Asymmetry between credential and deferred credential endpoints

Open nklomp opened this issue 1 year ago • 7 comments

Currently the credentials and deferred credentials endpoint are having a bit of an asymmetrical implementation.

The distinction between whether it is deferred or not is based on the HTTP response code 202 vs 200 and the response body of the credential endpoint. When deferred the transaction_id from the response needs to be used to post that value with the access token to the deferred endpoint, which in turn needs to be looked up from the metadata.

Then the deferred endpoint is returning the same response as the credential endpoint when the credential is available. If not available a 400 bad request with error issuance_pending. Given it is pretty normal to query the deferred endpoint whilst the credential is not available yet, it is odd to return an error for that. It also is not symmetrical with how the normal endpoint indicates that it will be a deferred flow by returning a 202. Returning a 202 when the issuance is still pending in the deferred endpoint makes sense. If you would go down that route the handling from a wallets perspective is the same for both endpoints.

However then it would also raise the question mentioned in #6 and #18 about the need for a separate endpoint for deferred credentials to begin with.

I see benefits from just having one endpoint for both direct and deferred issuance.

  • No need to signal deferred issuance support in the server metadata perse, as the endpoint is the same and it will be the issuer anyway that returns the 200 or 202 in current spec. A wallet currently does not know up front anyway whether the issuer will return a 202 or 200 and/or whether that is for all credentials or for some, so the only need for the deferred endpoint is to look it up when a 202 is returned (it would have made sense to simply return that URL in the 202 response, allowing for more flexibility). An issuer does not need to implement deferred, by simply making sure it only directly returns credentials with a 200 response.
  • Simplified logic for the wallet as it simply keeps posting to the same endpoint as long as a 202 is being returned. If a 200 is returned you are getting the credential.
  • The issuer could update the c_nonce, making sure the wallet updates its proof
  • Instead of having the interval value in the 400 error response for issuance_pending, simply return that in the 202, allowing for the issuer to increase this value over time (could also be done in current spec with issuance_pending to be clear)

The drawback of course is that you actually have 2 different requests for the same endpoint. One being the initial request, the other being the deferred request with the transaction_id. Given the endpoints are not following REST conventions anyway this drawback is a non-issue if you ask me.

nklomp avatar Dec 21 '23 16:12 nklomp

BTW if regular and deferred issuance would be unified into one endpoint a similar argument could be made to remove the batch endpoint altogether. Simply always have an array in the request and responses. Only need one credential, simply have one element in the array. One of the distinctions of course is that you would not know up front whether the issuer supports batched credentials (more than one VC), but that could easily be remedied with a metadata option replacing current batch_endpoint url, or simply requiring that issuers should support issuing multiple credentials.

I do see some arguments against this as it is a bit more muddy, so not feeling strongly about merging batched credentials as well, but just putting it out here that it could all be unified into one endpoint

nklomp avatar Dec 21 '23 16:12 nklomp

Then the deferred endpoint is returning the same response as the credential endpoint when the credential is available. If not available a 400 bad request with error issuance_pending. Given it is pretty normal to query the deferred endpoint whilst the credential is not available yet, it is odd to return an error for that.

Agree 100%. Related to #226

I am also in favor of the proposal to condense the two endpoints into one, I believe it will simplify implementations and lighten the burden on both servers and clients alike.

decentralgabe avatar Jan 24 '24 17:01 decentralgabe

I do see the benefit of merging deferred capability into credential endpoint.

Regarding the batch endpoint, deferred issuance capability should be merged into batch credential endpoint, as opposed to merging all three capabilities into one single endpoint. Currently batch endpoint uses the same syntax as the credential endpoint, but I would not expect it to stay this way because as proposed in issuance #93, there are some breaking changes needed for the batch issuance to add missing capabilities. so I would expect Credential Endpoint to stay simple and as-is for the interoperability purposes. (for the context, when taking VCI to the implementer's draft, WG agreed to there is a high chance breaking changes will be made to the batch endpoint but it is compensated by credential endpoint remaining unchanged.)

We should also introduce an issuer metadata parameter deferred_credential_issuance_supported (and deferred_batch_issuance_supported) or something for the issuers to indicate whether they support deferred capability or not, because I do not think all the issuers should be required to support deferred capability.

Sakurann avatar Feb 16 '24 21:02 Sakurann

https://github.com/openid/OpenID4VCI/pull/364 has removed the batch credential endpoint (now that the normal credential endpoint can issue batches of a single credential there was no clear need for an endpoint that issued different datasets in a single request, and there were lots of unsolved problems with doing so so the working group agreed removing was the best way forward).

Hence closing this issue. Feel free to comment/reopen if I missed some aspect that's still applicable.

jogu avatar Jul 25 '24 10:07 jogu

I guess the title of this ticket was wrong. It mainly talks about the asymmetry between deferred endpoint and regular credential endpoint and then also discusses the batch endpoint to some extent. I will have a look whether the asymmetry is gone in the latest commits

nklomp avatar Jul 25 '24 10:07 nklomp

Darn, I missed that, thanks - let's fix the title & reopen.

I think there are definitely some issues with deferred endpoint still.

jogu avatar Jul 25 '24 10:07 jogu

https://github.com/openid/OpenID4VCI/pull/391 I think fixes many of the issues with the deferred endpoint currently - if you could review it that would be great @nklomp !

We did discuss at one of the WG meetings or perhaps on another issue about merging the deferred endpoint into other endpoints, but there was a general feeling the current split actually works well and keeps things clear.

On this point:

The issuer could update the c_nonce, making sure the wallet updates its proof

People are clear that the wallet should not be required to update it's proof whilst a deferred issuance is ongoing - the credential should/must be issued with the proof initially passed - and the fact that you don't want to provide another proof when asking if a deferred issuance is complete yet is why having a separate endpoint and having transaction_id are good things.

jogu avatar Sep 24 '24 15:09 jogu

I believe what Joseph said above captures the current feeling in the WG. based that, can this issue be closed, @nklomp ? (putting pending close as a placeholder)

Sakurann avatar Jan 22 '25 13:01 Sakurann

@Sakurann Yes ticket can be closed now with changes in #391

nklomp avatar Jan 22 '25 14:01 nklomp