Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add APID VC implementations for sign & verify, add JSON serialization for VC at UniFFI layer #251

Merged
merged 9 commits into from
Jun 26, 2024

Conversation

KendallWeihe
Copy link
Contributor

  • Implement VC sign & verify within apid module
  • Change VC datetime types from String to SystemTime
  • Add JSON serialization for VC issuer and credentialSubject at the UniFFI layer

}
}

// TODO prioritize JWT claims -- hit up Neal
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💪

pub context: Vec<String>,
pub id: String,
pub r#type: Vec<String>,
pub issuer: String, // JSON serialized
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This could cause confusion on if its an issuer string "did:dht:123" or an json serialized json string.

Should we rename json_serialized_issuer ?

I think sticking with the official name has value too though

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh yeah wow I really appreciate this idea... I think yes we should follow that practice everywhere wherein we're json serializing over the FFI (including over in tbdex, going to open a PR for that now...)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice, yeah I really like this idea TBD54566975/tbdex-rs#63

self.sign_with_signer(&key_id, signer)
}

pub fn sign_with_signer(&self, key_id: &str, signer: Arc<dyn Signer>) -> Result<String> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

header.set_token_type("JWT");
let vc_jwt = josekit::jwt::encode_with_signer(&payload, &header, &jose_signer)?;

Ok(vc_jwt)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we should validate/verify before we give the Ok, I wonder if invalid values can sneak through to this point and we give a VC that is signed with invalid properties

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmmmmmm maybe, but I'm not convinced yet

}

fn signature_len(&self) -> usize {
64
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there was a JoseKit Enum for this

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there is EdCurve::Ed25519 but it doesn't have the usize value associated to it

Copy link
Contributor

@nitro-neal nitro-neal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🥇 awesome!

@KendallWeihe KendallWeihe merged commit cfe6f16 into main Jun 26, 2024
6 checks passed
@KendallWeihe KendallWeihe deleted the kendall/vc-apid branch June 26, 2024 19:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants