openid / OpenID4VCI

68 stars 20 forks source link

Credential Encryption Parameter Name Discrepancy #113

Closed peppelinux closed 11 months ago

peppelinux commented 1 year ago

In the credential request we have the parameter names for the credential response encryption, defined in the text as follows:

credential_encryption_jwk does not include response_ in it. Me and other implementers are wondering if this is intentional and why.

If this is not intentional, WDYT if we change the name to credential_response_encryption_jwk, for naming normalization?

immagine

paulbastian commented 1 year ago

Does it make sense to group them together into a separate JSON object?

Sakurann commented 11 months ago

I agree it is cleaner to rename to credential_response_encryption_jwk unless @awoie there was a reason why you chose to omit _response here? less sure about the grouping. it does not seem to be a convention in OAuth/OIDC, so we can probably leave as-is

awoie commented 11 months ago

No specific reason. I think it makes sense to rename it.

awoie commented 11 months ago

Does it make sense to group them together into a separate JSON object?

I used the same pattern that you can find here: https://openid.net/specs/openid-connect-discovery-1_0.html

paulbastian commented 11 months ago

Does it make sense to group them into a single object?

awoie commented 11 months ago

Does it make sense to group them into a single object?

I would support that since the names will get shorter. OAuth2 and OP Metadata took a different path. If there was no technical reason why those aren't Objects, then I'd be in favor of changing this.

paulbastian commented 11 months ago

Proposal:

"credential_response_encryption" : {
    "jwk" : {},
    "alg" : "...",
    "enc" : "..."
}
Sakurann commented 11 months ago

@cobward is Spruce using these in production? Would you be ok with this breaking change? if so, I think we can try get this in before ID-1

cobward commented 11 months ago

We're fine with breaking changes at this point.