Hiya, Here's my review of https://datatracker.ietf.org/doc/draft-ietf-oauth-sd-jwt-vc/
In general/substantive comments: * Need to update references from ietf-oauth-selective-disclosure-jwt to the RFC https://datatracker.ietf.org/doc/rfc9901/ * Since one explicit aim of the type metadata is versioning and you don't have content type for versioning, does it make sense to put a v1 somewhere in the URI, or whatever else might be used to version the type metadata? (I think Tim mentioned this too.) * You define Consumer in section 1.4, but later use consuming application instead of Consumer. (Section 8, section 10.5). Suggest standardizing on Consumer by searching/for "consuming application" and replacing it with Consumer. * Had questions about the fact a new JSON path parsing ruleset is defined, more below. * Had questions about the placement of the well-known path in relation to the tenant in multi-tenant scenarios. Specific sections: --- Section 1.1 This early paragraph is long and a bit confusing: Verifiers can check the authenticity of the data in the Verifiable Credentials and optionally enforce Key Binding, i.e., ask the Holder to prove that they are the intended Holder of the Verifiable Credential, for example, by proving possession of a cryptographic key referenced in the credential. Would suggest breaking into two sentences: Verifiers can check the authenticity of the data in the Verifiable Credentials and optionally enforce Key Binding. Key Binding asks the Holder to prove that they are the intended Holder of the Verifiable Credential, for example, by proving possession of a cryptographic key referenced in the credential. The note about not using W3C Verifiable Credentials Data Model seemed a bit out of place. Is it normal to reference unused, related standards? Searched the mailing list and didn't see a reason for this. Maybe better to put it into section 12? Section 1.4 The definition of Consumer had a weird singular/plural disconnect "Consumer" vs "Applications". Suggest: Consumer: An Application using the Type Metadata specified in Section 6 is called a Consumer Section 3.4 The check in point 2.3 of Section 7.1 ... -> The check in point 2.a of Section 7.1 ... I don't see a section 2.3 in the referenced doc. Sections 4.1 and 3.2 seem to be duplicative. Can we remove one? Section 5.1 Is there a reason to not follow the OIDC pattern of having the tenant or any other path before the .well-known/jwt-vc-issuer string? https://openid.net/specs/openid-connect-discovery-1_0.html#ProviderConfigurationRequest I didn't see anything in the mailing list when I looked for why that decision was made. Section 5.2 Missing the word 'status': the 200 OK HTTP and -> the 200 OK HTTP status and Make it more clear about what the correct status code for errors are: An error response uses the applicable HTTP status code value. -> An error response MUST use the applicable HTTP status code value. This section was confusing: --- It is RECOMMENDED that the JWT contains a kid JWT header parameter that can be used to look up the public key in the JWK Set included by value or referenced in the JWT VC Issuer Metadata --- I think the word JWT references the SD-JWT VC for which this is the issuer metadata, but wasn't sure. maybe change it to It is RECOMMENDED that the SD-JWT VC contains a kid JWT header parameter if my reading is correct. Considering the sample "JWT VC Issuer Metadata" doc from { "issuer":"https://example.com";, "jwks_uri":"https://jwt-vc-issuer.example.com/my_public_keys.jwks"; } to { "issuer":"https://example.com/issuer";, "jwks_uri":"https://jwt-vc-issuer.example.com/my_public_keys.jwks"; } Because in section 5.3 you specify that "The issuer value returned MUST be identical to the iss value of the JWT." and the issuer value of the sample JWT elsewhere has the /issuer path. Section 6.2 Do you need extends#integrity in the claims list (it's in the example in 6.1)? or is that covered by the "A Type Metadata document MAY contain additional top level" sentence after the list? Section 6.3 A consumer retrieving Type Metadata MUST ensure that the vct value in the SD-JWT VC payload is identical to the vct value in the reference to the Type Metadata assume we can trust the implementor knows how to compare urls and there's no need to reference RFC 5890, 3987, etc? Section 6.3.1 Small typo: "Section Section 7." Section 7 You state: ...MUST verify the integrity of the retrieved document as defined in Section 3.3.5 of... But I think you mean section 3.5 as there is no section 3.3.5 in https://www.w3.org/TR/sri/ You say: "A Consumer of the respective documents MUST verify the integrity ... but the required claim is marked as optional in 3.2.2.2. Found that confusing. Assume that means that if the X#integrity claim is present you must verify the integrity? Maybe change to When the corresponding integrity claim is present, a Consumer of the respective documents MUST verify the integrity of the retrieved document as defined ... Section 8.2 Why is the display claim treated differently in terms of overriding an extension than any other metadata? any reason to call that out? The general overriding process is mentioned in section 9.5, maybe remove 8.2. Section 9 why would you have a claim defined without an svg_id? what is the use case? if you are using a simple rendering method, no claims are displayed. Is this future proofing, in case there's another render method added that isn't an svg? in that case, maybe change it to claim_id so a future method could reference the claim for display? then make it required? Or am I missing some other reason for the claims to be defined in the type metadata? Section 9.1 why not use jsonpath or jsonpointer (rfc6901) instead of defining a new JSON path processing framework? I guess I worry whenever I see another path processing algorithm defined. But maybe I'm missing the reason not to use a more general purpose solution? Section 10.6 Having the prefix *Recommendation:* is kinda weird and not common. maybe strike, not sure it adds anything. Section 10.7 Introduces the term "publisher" which is not used anywhere else in the doc. Maybe use issuer? ---
_______________________________________________ OAuth mailing list -- [email protected] To unsubscribe send an email to [email protected]
