Sphereon-Opensource / OID4VC

OpenID for Verifiable Credentials - modules for issuers, holders and RPs
Apache License 2.0
62 stars 19 forks source link

fix: type for cred request ldp #96

Closed TimoGlastra closed 6 months ago

TimoGlastra commented 6 months ago

The type was incorrectly set, as according to draft 11 of OID4VCI the LDP credential request MUST have a credential_definition object.

The same is true for draft 12.

The code still works with top-level types as well, (as currently both credential_definition.types and types already worked), but the type is corrected now.

nklomp commented 6 months ago

Not sure that is correct. V11 had types and credentialSubject as a toplevel property. That moved in v12 to credential_definition. For types it works indeed, but I don't know whether that also is the case for credentialSubject

We actually have a v12 request type but that is not used it seems. IMO the solution should be to add the v12 request type to union, instead of removing v11

nklomp commented 6 months ago

Can't wait to remove all the draft version support

TimoGlastra commented 6 months ago

The type I modified in this PR is only for the Credential Request. The issuer metadata indeed has them top-level, but that is a separate type is declared here: https://github.com/Sphereon-Opensource/OID4VCI/blob/5218c5c59a1d02f2d2b3d323c6b791aa1788b85b/packages/common/lib/types/Generic.types.ts#L86

See the relevant section in the draft 11 spec here: https://openid.net/specs/openid-4-verifiable-credential-issuance-1_0-11.html#section-e.1.3.5

nklomp commented 6 months ago

Oh yeah, you are correct. Wow, managed to get it wrong twice :P