openid / OpenID4VP

57 stars 20 forks source link

add transaction_data #197

Closed Sakurann closed 1 month ago

Sakurann commented 5 months ago

resolves #174 resolves #173 resolves #172

Sakurann commented 2 months ago
paulbastian commented 2 months ago

In LSP POTENTIAL there are two designs for QES with Transaction data:

Are these flows applicable for both?

Sakurann commented 2 months ago

In LSP POTENTIAL there are two designs for QES with Transaction data:

the Wallet-centric model the QTSP-centric model

Are these flows applicable for both?

@paulbastian I would say so, because in both flows, presenting a credential from the wallet is used as a way to authenticate the user, right? do you have a hypothesis why this would not work for the wallet-centric model?

paulbastian commented 2 months ago

In LSP POTENTIAL there are two designs for QES with Transaction data:

the Wallet-centric model the QTSP-centric model

Are these flows applicable for both?

@paulbastian I would say so, because in both flows, presenting a credential from the wallet is used as a way to authenticate the user, right? do you have a hypothesis why this would not work for the wallet-centric model?

I would say the workings of the QTSP centric model does not align with the general model of OpenID4VP and what most readers here would assume. For example, it wouldn't allow the real relying party to request a credential and QES in the same request, as the actual request is from QTSP embedded into relying party website.

paulbastian commented 2 months ago

@Sakurann I would like to better understand the QTSP centric model for QES. I imagine a scenario where I am on a website (OpenID4VP Relying Party) and it wants me to sign a contract. How does an Authorization Request look like in this case, especially what is the client_id and request_uri?

paulbastian commented 2 months ago

This PR adds a significant feature that should be described in a use case appendix, similar to what OpenID4VCI does in my opinion.

paulbastian commented 2 months ago

I am looking at this from the perspective using this within eIDAS QES use case. Illustrating with this picture, there are two possible flows:

The QTSP centric flow and how transaction data is used is very clear to me.

The Wallet centric flow is less clear to me. As I imagine this, the communication between Wallet and QTSP look quite similar with the Wallet having a fixed relation with a particular QTSP. The question arises how a Verifier would send a request for a QES (either including the hash or the document itself) to the Wallet, marked with "?" in the diagrams. Questions:

Sakurann commented 1 month ago

@paulbastian regarding wallet-centric flow... I re-read LSP POTENTIAL UC5 QES diagrams and if I understand correctly, the difference between QTSP centric and Wallet centric is where the Signature Creation Application is located ouside or inside the EUDIW and whether the user chooses QTSP to use from the RP or from the Wallet. Which means that the part where QTSP authenticates the user using the wallet is the same between both flows, hence how transaction data is used is also the same. Am I missing something?

paulbastian commented 1 month ago

If my question wasn't clear enough: what do we intend to use for "?" in my figure for the wallet centric flow?

alenhorvat commented 1 month ago

If my question wasn't clear enough: what do we intend to use for "?" in my figure for the wallet centric flow?

For ? the wallet should receive a request to share data/VC/... signed with one of the signature profiles requested or supported by the RP. So OID4VP would be a good fit. If wallet has a web interface, RP -> QTSP and RP-> (Web) Wallet could use the same protocol/interfaces.

I'm not sure the QTSP-specific API will be symmetric as above. At least today we see the following patterns

QTSP --> Wallet (1st flow) (today this is implemented in a closed setting where QTSPs are using push notifications) Wallet --> QTSP (2nd flow) (today oauth or other client-secret based approaches are used, again, having in mind a closed system)

If this needs to be opened up, QTSPs would need to align on the interfaces (I guess this is what CSC is trying to achieve?)

Sakurann commented 1 month ago

If my question wasn't clear enough: what do we intend to use for "?" in my figure for the wallet centric flow?

probably CSC specification etc. but I really don't think that's in scope for this PR and need to hold it up.

jogu commented 1 month ago

@paulbastian

I'm not objecting

Paul, did you mean to clear your 'requested changes' then? It's still showing currently.

paulbastian commented 1 month ago

@paulbastian

I'm not objecting

Paul, did you mean to clear your 'requested changes' then? It's still showing currently.

Yes, it seems there is no way to clear "request changes" other than to approve. But I didn't have enough time yet to read the PR in all its details to feel confident to approve.

stefan-kauhaus commented 1 month ago

At the EU Digital Wallet Consortium (EWC) we are about to start piloting a use case that includes using the wallet to confirm payment details (Dynamic Linking requirement from PSD2). Therefore we would be happy and explicitly support transaction_data to go into the next Implementer's Draft so that we can implement and test it and report our findings back to the community.

c2bo commented 1 month ago

Looks good to me with the changes! I guess we should not merge this until the Query Language PR is merged so it doesn't break the github action (vp_query reference)?

jogu commented 1 month ago

4 approvals, discussed in multiple WG calls, open for over 4 months, all comments addressed - merging! Thank you everyone!

jogu commented 1 month ago

4 approvals, discussed in multiple WG calls, open for over 4 months, all comments addressed - merging! Thank you everyone!

Well, merging after making a trivial tweak so the spec compiles (removing the #vp_query reference) - we should probably add back the two VP query section links once query language is merged.