search

openid / OpenID4VCI

68 stars 20 forks source link

Enable non-breaking extensibility #382

Closed selfissued closed 1 month ago

selfissued commented 2 months ago

Fixes #375

selfissued commented 2 months ago

I enabled additional Deferred Credential Response parameters in my latest commit.

I didn't add Notification Response parameters because the defined response is 204 No Content. But we could allow for Notification Response parameters if we choose. What do people think?

jogu commented 2 months ago

I would really like us to discuss the "both wallet and the issuer must understand the proof type" requirement. it we keep this requirement, i really don't understand why we don't have similar requirements for other issuer metadata that the wallet has to pre-discover from the issuer metadata we are not enabling extensibility within credential format profiles?

Discussed on today's WG call, there was consensus on keeping the requirement.

Sakurann commented 2 months ago

I would really like us to discuss the "both wallet and the issuer must understand the proof type" requirement. it we keep this requirement, i really don't understand why we don't have similar requirements for other issuer metadata that the wallet has to pre-discover from the issuer metadata we are not enabling extensibility within credential format profiles?

Discussed on today's WG call, there was consensus on keeping the requirement.

Could you please elaborate why? could not find in wg call minutes

selfissued commented 2 months ago

Could you please elaborate why? could not find in wg call minutes

As discussed in the call, the reason that both parties must understand the proof is that understanding it is integral to the security of the interaction. It's not like most extension points where not-understood values can be ignored. That's why people on the call were good with documenting that this particular extension point doesn't fit the normal must-ignore-if-not-understood pattern.

Sakurann commented 2 months ago

Could you please elaborate why? could not find in wg call minutes

As discussed in the call, the reason that both parties must understand the proof is that understanding it is integral to the security of the interaction. It's not like most extension points where not-understood values can be ignored. That's why people on the call were good with documenting that this particular extension point doesn't fit the normal must-ignore-if-not-understood pattern.

I understand the thinking, but I still believe this is not enforceable. You can't stop the wallet from sending the proof type that the issuer does not support (just like for any other issuer metadata). it would be much better to paraphrase this to be an error condition - "issuer MUST return an error if the wallet uses a proof type not supported by the issuer.

I am sorry, but I am not comfortable merging this PR with this text. please separate this proof related language into an another PR, so that we could merge everything else meanwhile, thank you.

selfissued commented 2 months ago

I am sorry, but I am not comfortable merging this PR with this text. please separate this proof related language into an another PR, so that we could merge everything else meanwhile, thank you.

I deleted the text you're not comfortable with so that we can merge the rest.

Sakurann commented 2 months ago

@selfissued thank you! let us know when all other comments are addressed so that we can merge this PR.

jogu commented 1 month ago

I believe all comments have been responded to and/or resolved now (if I missed any feel free to open issues) and we have 8 approvals so merging!