openid / OpenID4VCI

64 stars 18 forks source link

merge credential endpoint with the batch credential endpoint #18

Closed OIDF-automation closed 1 month ago

OIDF-automation commented 1 year ago

Imported from AB/Connect bitbucket: https://bitbucket.org/openid/connect/issues/1759

Original Reporter: danielfyes

The normal credential endpoint is really a specialized instance of the batch endpoint. I think the spec and implementations can be simplified by removing what today is the regular credential endpoint and only keeping the batch endpoint. As far as I can see, the overhead is very small and the functionality of the batch endpoint is a superset of the one of the regular credential endpoint.

OIDF-automation commented 1 year ago

Imported from AB/Connect bitbucket - Original Commenter: KristinaYasuda

I think it is the other way around: The batch endpoint is a specialized, extended instance of the normal credential endpoint.

Most of the implementations only need normal credential endpoint for their use cases. So for the interoperability, the spec mandates the support for the normal credential endpoint for the supporting implementations, while if the architecture requires, batch endpoint can also be supported.

We need more implementation experience, but batch endpoint has been seen as adding a certain amount of complexity (more than “small”) to the normal credential endpoint.

OIDF-automation commented 1 year ago

Imported from AB/Connect bitbucket - Original Commenter: mbj

I agree that requiring the credential endpoint is appropriate, since it’s the normal simple case, whereas having an optional batch credential endpoint is also appropriate.

OIDF-automation commented 1 year ago

Imported from AB/Connect bitbucket - Original Commenter: KristinaYasuda

SIOP 2023-04-07 call current direction is to keep both endpoints and wait for implementation experience if we want to remove either. Here is the original issue that led to a batch credential endpoint (I think) https://bitbucket.org/openid/connect/issues/1544/has-pr-requesting-multiple-credentials

OIDF-automation commented 1 year ago

Imported from AB/Connect bitbucket - Original Commenter: KristinaYasuda

pending close. @{557058:cd7888c3-ecf3-47c0-92ec-168b48519672}

OIDF-automation commented 1 year ago

Imported from AB/Connect bitbucket - Original Commenter: danielfett

Following offline conversations, I’d like to re-open this issue.

Besides the points raised above - one endpoint less to protect and maintain - there is another issue in the spec right now.

How is deferred batch issuance supposed to work?

As far as I can tell, the idea is that in case the batch endpoint cannot issue all credentials immediately, it will issue a transaction_id (or many?) and the wallet is supposed to hit the deferred endpoint with that transaction_id. The deferred credential endpoint is defined as returning one credential only. So is the wallet supposed to hit the endpoint 100 times for 100 credentials? It doesn’t seem that this is the intended solution.

So my proposal is to do away with the specialized one-credential case alltogether, for all endpoints. It introduces complexity and corner cases that are just not needed.

The credential endpoint should be removed, the batch credential endpoint becomes the new default, and the deferred endpoint is enabled for batch deferred issuance.

OIDF-automation commented 1 year ago

Imported from AB/Connect bitbucket - Original Commenter: danielfett

See comment.

OIDF-automation commented 1 year ago

Imported from AB/Connect bitbucket - Original Commenter: danielfett

In the SIOP call, a case was discussed where the Wallet might not know how many credentials are to be issued. The Wallet would not know which endpoint to hit. This issue would go away with this proposal.

paulbastian commented 10 months ago

I agree with @danielfett that it should be possible to merge credential and batch credential endpoint, see https://github.com/openid/OpenID4VCI/issues/93#issuecomment-1791187949

Sakurann commented 10 months ago

I think requirements are:

I think we could achieve this by having one endpoint that is mandatory to support issuance of one credential per request, but can also be used with more than one credential per request (though the same endpoint will handle differing error conditions) and issuer metadata will include a parameter that indicates whether this endpoint supports batch issuance capability or not.

tlodderstedt commented 10 months ago

I think we need a understanding of what is implemented today. My assumption would be most implementations support "normal" issuance but not batch issuance so removing the "normal" issuance from the spec would be a big breaking change. Also I would like to learn from implementers whether a removal of the "normal" issuance endpoint would make their life easier or harder. I also think we need to come to a conclusion around support for issuing multiple instances of the same credential before deciding on that matter.

paulbastian commented 3 months ago

I support removing batch credential endpoint given that we merge #293 and credential endpoint has possiblities to issue multiple Credential instances for the same Credential Dataset.

c2bo commented 3 months ago

I support removing batch credential endpoint given that we merge #293 and credential endpoint has possiblities to issue multiple Credential instances for the same Credential Dataset.

+1 from me

Sakurann commented 2 months ago

I support removing batch credential endpoint given that we merge https://github.com/openid/OpenID4VCI/pull/293 and credential endpoint has possiblities to issue multiple Credential instances for the same Credential Dataset.

+1 from me

+1 from me too

bc-pi commented 2 months ago

I support removing batch credential endpoint given that we merge #293 and credential endpoint has possiblities [sic] to issue multiple Credential instances for the same Credential Dataset.

I support Paul's support for removing the batch credential endpoint.

jogu commented 2 months ago

I agree with removing the batch credential endpoint, given that the normal credential endpoint can now issue multiple instances of a credential.

David-Chadwick commented 2 months ago

I also agree with removing it

stefan-kauhaus commented 2 months ago

In the EWC consortium (one of the eIDAS 2.0 LSPs), we are currently working on a use case where a bank customer will receive several credentials into their wallet in a single occasion -- one per account/card the customer has with the bank. This is to support the eIDAS 2.0 requirement that EU DI wallets can be used to confirm payments. These credentials will have different payloads. It would be very unfortunate if the user would have to go through multiple issuance flows for this.

paulbastian commented 2 months ago

@stefan-kauhaus to clarify: you are still able to issue multiple credentials within one process, but instead of using batch credential request, the wallet sends multiple credentials requests.

pmhsfelix commented 2 months ago

I agree with the batch credential endpoint removal, given the newly added capability on the regular credential endpoint.

babisRoutis commented 2 months ago

Also agree.

Removal of batch endpoint simplifies somehow VCI for both issuers & wallets and the only price to pay is the loss of the ability to issue different credentials type with a single http request.

Since there is a workaround for this (by calling the credential endpoint with the same access_token multiple times) I don't consider this loss of flexibility significant.

tplooker commented 2 months ago

I'm also in favour of removing the batch credential endpoint for all the reasons already highlighted.

bc-pi commented 2 months ago

I'm also in favour of removing the batch credential endpoint for all the reasons already highlighted.

I'm also in favor of removing the batch credential endpoint for all the reasons already highlighted.

manually translated by an American

Sakurann commented 2 months ago

discussed in the WG: