Fix the presentation submission in the oid4vp-rs e2e test. #8

Ryanmtate · 2024-08-08T20:46:15Z

As part of this fix, upgrading ssi dependency to 0.8.1;

Upgrade ssi version to 0.8.1; (use ssi sub-crates)
Construct a jwt_vp token;
Return a jwt_vp token instead of the jtw_vc token in the Presentation Submission;
Validate the Presentation Submission against the Presentation Definition;
Validate the VP Token flow, see: https://openid.net/specs/openid-4-verifiable-presentations-1_0.html#section-6.5 and https://identity.foundation/presentation-exchange/spec/v2.0.0/#input-evaluation;

NOTE: Saving the following for another PR:

Handle JWT encoded authorization request.

Signed-off-by: Ryan Tate <[email protected]>

src/core/authorization_request/verification/did.rs

src/presentation_exchange.rs

src/verifier/client.rs

Signed-off-by: Ryan Tate <[email protected]>

tests/jwt_vc.rs

Signed-off-by: Ryan Tate <[email protected]>

add implementation methods for Presentation Definition. WIP: Need to continue work for Presentation Submission and the rest of the structs used in the presentation exchange flow. Signed-off-by: Ryan Tate <[email protected]> Co-authored-by: Todd Showalter <[email protected]>

Signed-off-by: Ryan Tate <[email protected]>

Ryanmtate · 2024-08-09T23:50:22Z

Merge #9 before this.

Signed-off-by: Ryan Tate <[email protected]>

…e for presentation exchange Signed-off-by: Ryan Tate <[email protected]>

Signed-off-by: Ryan Tate <[email protected]>

Cargo.toml

src/core/authorization_request/verification/did.rs

cobward

Great progress so far!

src/core/authorization_request/verification/did.rs

src/presentation_exchange.rs

src/verifier/client.rs

Signed-off-by: Ryan Tate <[email protected]>

src/json_schema_validation.rs

Signed-off-by: Ryan Tate <[email protected]>

src/core/credential_format/mod.rs

src/core/input_descriptor.rs

src/core/presentation_definition.rs

cobward · 2024-08-27T11:10:52Z

src/holder/verifiable_presentation_builder.rs

Yep let's yank then.

src/lib.rs

Co-authored-by: Jacob <[email protected]>

Signed-off-by: Ryan Tate <[email protected]>

…ion impl Signed-off-by: Ryan Tate <[email protected]>

Signed-off-by: Ryan Tate <[email protected]>

cobward · 2024-08-27T11:27:27Z

tests/e2e.rs

Just to point out: OID4VP is a framework specification. The semantics are not well defined, and it's down to the "profile" to define how a request is interpreted, and what kind of response to be built.

src/core/metadata/mod.rs

src/tests.rs

…ferencing Signed-off-by: Ryan Tate <[email protected]>

Signed-off-by: Ryan Tate <[email protected]>

tests/e2e.rs

tests/jwt_vc.rs

src/core/presentation_submission.rs

…le is enforced. Signed-off-by: Ryan Tate <[email protected]>

Signed-off-by: Ryan Tate <[email protected]>

Cargo.toml

cobward · 2024-09-09T10:51:34Z

src/core/credential_format/mod.rs

+    },
+    #[serde(rename = "mso_mdoc")]
+    MsoMDoc(serde_json::Value),
+    Other(serde_json::Value),


I think this Other case needs #[serde(untagged)] otherwise it will only capture:

{ ... "Other": { ... } ... }

Could you add a little unit test for deserializing and the designation fn for this case?

cobward · 2024-09-09T10:52:57Z

src/core/credential_format/mod.rs

+    /// Other claim format designations not covered by the above.
+    ///
+    /// The value of this variant is the name of the claim format designation.
+    Other(String),


As above I think this needs a #[serde(untagged)]. Please add a test.

cobward · 2024-09-09T10:53:45Z

src/core/credential_format/mod.rs

+pub enum CredentialType {
+    /// an ISO 18013-5:2021 mobile driving license (mDL) Credential
+    #[serde(rename = "org.iso.18013.5.1.mDL")]
+    Iso18013_5_1mDl,
+    /// A vehicle title credential
+    ///
+    /// Given there is no universal standard for how to present a vehicle title credential,
+    /// the inner String provides a dynamic way to represent a vehicle title credential.
+    // TODO: is there a standard identifier for a vehicle title credential instead of `vehicle_title`?
+    #[serde(rename = "vehicle_title")]
+    VehicleTitle(String),
+    // TODO: Add additional credential types here. Fallback to a string for any other credential type.
+    /// Other credential types not covered by the above.
+    Other(String),
+}


We will never be able to enumerate all credential types, so I think a string is the most reasonable representation.

cobward · 2024-09-09T11:14:48Z

src/core/credential_format/mod.rs

+/// A Json object of claim formats.
+pub type ClaimFormatMap = HashMap<ClaimFormatDesignation, ClaimFormatPayload>;


Why is this needed in addition to ClaimFormat? Isn't ClaimFormatMap equivalent to Vec<ClaimFormat>?

The presentation exchange presentation definition test suite test case format_example shows format property as a map. I tried with the Vec<ClaimFormat> but encountered errors in the test case.

{ "presentation_definition": { "id": "32f54163-7166-48f1-93d8-ff217bdb0653", "input_descriptors": [], "format": { "jwt": { "alg": ["EdDSA", "ES256K", "ES384"] }, "jwt_vc": { "alg": ["ES256K", "ES384"] }, "jwt_vp": { "alg": ["EdDSA", "ES256K"] }, "ldp_vc": { "proof_type": [ "JsonWebSignature2020", "Ed25519Signature2018", "EcdsaSecp256k1Signature2019", "RsaSignature2018" ] }, "ldp_vp": { "proof_type": ["Ed25519Signature2018"] }, "ldp": { "proof_type": ["RsaSignature2018"] } } } }

src/core/input_descriptor.rs

cobward · 2024-09-09T11:19:33Z

src/core/input_descriptor.rs

+    #[serde(skip_serializing_if = "Option::is_none")]
+    fields: Option<Vec<ConstraintsField>>,


As above, may not be important to distinguish between None and an empty vec.

src/core/input_descriptor.rs

src/core/presentation_definition.rs

src/core/response/mod.rs

cobward · 2024-09-09T13:33:03Z

src/core/response/parameters.rs

+    /// Parse the VP Token as a JSON object.
+    ///
+    /// This will attempt to decode the token as base64, and if that fails, it
+    /// will attempt to parse the token as a JSON object.
+    ///
+    /// See: https://openid.net/specs/openid-4-verifiable-presentations-1_0.html#section-6.1-2.2
+    ///
+    /// If you want to check for decode errors, use [VpToken::decode_base64].
+    pub fn parse(&self) -> Result<Json, Error> {
+        match self.decode_base64() {
+            Ok(decoded) => Ok(decoded),
+            Err(_) => Ok(serde_json::from_str(&self.0)?),
+        }
+    }
+
+    /// Return the Verifiable Presentation Token as a JSON object from a base64
+    /// encoded string.
+    pub fn decode_base64(&self) -> Result<Json, Error> {
+        let decoded = BASE64_STANDARD.decode(&self.0)?;
+        Ok(serde_json::from_slice(&decoded)?)
+    }


These methods are inaccurate:

JSON String or JSON object that MUST contain a single Verifiable Presentation or an array of JSON Strings and JSON objects each of them containing a Verifiable Presentations. Each Verifiable Presentation MUST be represented as a JSON string (that is a Base64url encoded value) or a JSON object depending on a format as defined in Annex E of [OpenID.VCI]

We have a bug in our definition of VpToken. It should be something like:

enum VpToken { Single(String), SingleAsMap(Map<String, Value>), Many(Vec<VpToken>), }

I have a TODO in the code to update the VpToken. Maybe we can do this in another PR.

cobward · 2024-09-09T13:38:28Z

src/core/response/parameters.rs

+    /// the requirements of the presentation definition.
+    ///
+    ///
+    pub fn validate_unencoded(


As above, this is making an assumption that the VpToken is a VCDM VP. There's also some confusion here about encoded as a JWT vs unencoded. Encoded as a JWT refers to the authorization response (see direct_post.jwt), not the VP token. The VP token is never encoded directly as a JWT. It's either a String, a JSON object, or an array of VP tokens.

This method has been removed.

cobward · 2024-09-09T13:39:30Z

src/holder/verifiable_presentation_builder.rs

I think we said this should be removed from this library and added to SSI?

Okay to remove in another PR once added to SSI?

Co-authored-by: Jacob <[email protected]>

Signed-off-by: Ryan Tate <[email protected]>

…elds This commit also removes validation logic from vp token response struct. Signed-off-by: Ryan Tate <[email protected]>

…uite Signed-off-by: Ryan Tate <[email protected]>

timothee-haudebourg

Adding a late review of this to clarify some stuff about ssi.

timothee-haudebourg · 2024-09-24T11:47:49Z

src/holder/verifiable_presentation_builder.rs

+use std::time::{SystemTime, UNIX_EPOCH};
+
+use serde::{Deserialize, Serialize};
+use ssi_claims::jwt::{VerifiableCredential, VerifiablePresentation};


I think there is a confusion here, probably because of the lack of documentation in ssi I'll admit. Those two types are wrappers around the JWT registered claims vc and vp to be used with ssi_claims::jwt::ClaimSet.

If you want to represent VCs and VPs, the appropriate types are defined in ssi_claims::vc.

timothee-haudebourg · 2024-09-24T11:48:46Z

src/holder/verifiable_presentation_builder.rs

+
+#[derive(Debug, Clone)]
+pub struct VerifiablePresentationBuilderOptions {
+    pub issuer: DIDURLBuf,


Is it a requirement of OID4VP that we use DIDs? I know this is not a requirement of VPs.

timothee-haudebourg · 2024-09-24T11:55:00Z

src/holder/verifiable_presentation_builder.rs

+    ///
+    /// This will set the issuance date to the current time and the expiration
+    /// date to the expiration secs from the issuance date.
+    pub fn from_options(options: VerifiablePresentationBuilderOptions) -> VerifiablePresentation {


This sounds like a re-implementation of VCDM1.0 JWT VP encoding. This is already implemented in ssi here.

// Create the presentation. let presentation: ssi::claims::vc::v1::JsonPresentation = ...; // Encode the JWT claims according to VCDM1.0 JWT VP format. let jwt_claims = presentation.to_jwt_claims(); // Sign the claims into a JWT. let jwt = jwt_claims.sign(signer).await.unwrap();

timothee-haudebourg · 2024-09-24T11:57:23Z

src/core/credential_format/mod.rs

+    #[serde(rename = "jwt_vp")]
+    JwtVp,
+    #[serde(rename = "jwt_vc_json")]
+    JwtVcJson,


What is this format?

wip: use ssi 0.8.1, debugging e2e test using did resolver

10611a8

Signed-off-by: Ryan Tate <[email protected]>