cli did create web #263
cli did create web #263
Conversation
diehuxx
requested review from
KendallWeihe,
nitro-neal,
kirahsapong,
shamilovtim,
decentralgabe and
frankhinek
as code owners
| no_indent, | ||
| json_escape, | ||
| } => { | ||
| let private_jwk = Ed25519Generator::generate(); |
There was a problem hiding this comment.
I know that with did web especially, we should allow people to pass in the own private key / portable did. We can add this as a feature in the future though
There was a problem hiding this comment.
agreed, on the private key input (portable DID slightly different use case I haven't thought through -- creating a new DID with an existing DID), but yeah not a priority at this moment
|
if you have the output handy can you put here? This is amazing! |
|
|
||
| let verification_method = VerificationMethod { | ||
| id: format!("{}#key-0", did), | ||
| r#type: "JsonWebKey2020".to_string(), |
There was a problem hiding this comment.
@nitro-neal didn't we standardize this to just JsonWebKey?
| @@ -18,6 +21,29 @@ pub struct DidWeb { | |||
| } | |||
|
|
|||
| impl DidWeb { | |||
| pub fn new(domain: &str, public_jwk: Jwk) -> Result<Self> { | |||
| let did = format!("did:web:{}", domain); | |||
There was a problem hiding this comment.
This isn't quite right because the domain has to be encoded according to the spec. See here and here. Also web5-go as inspiration.
Probably we should use a strongly typed Url here to ensure the passed domain: &str is a valid domain.
I believe also did:web has a requirement of enforcing https over http, but with the caveat that, if the domain is localhost then http is allowed. We're currently setup to handle this in resolution, but here too, we should use a strongly typed Url to ensure the domain is localhost and not something like localhost-neal-example.com. Feel free to improve the resolution code as well in this PR if you think it's appropriate.
crates/web5_cli/src/dids/create.rs
Outdated
| @@ -17,7 +17,12 @@ pub enum Commands { | |||
| json_escape: bool, | |||
| }, | |||
| Web { | |||
| #[arg(long)] | |||
There was a problem hiding this comment.
| #[arg(long)] |
The domain is a required parameter, so not an arg (you can just delete the line).
Also, thoughts on calling it URL because it could include the HTTP path such as in Example 5 here. I'm thinking, name it url, and then handle the following use cases:
- If they pass in
http://some-domain.comthen thehttpwould throw an error, b/c needs to behttps - With the caveat,
httpis allowed if the domain name islocalhost(or127.0.0.1) - If they pass in
https://some-domain.comthen that's fine, but we have to splice off thehttps:// - If they pass in the
did.jsonfile name (as the trailing path part), that's fine but it must be spliced off before creating the DID URI - If they pass in the
.well-knownuse case in theurlthen it's also spliced off
Basically my thinking is, enable the developer to copy/paste exactly the endpoint they're expecting to host the DID Document at but then ensure the DID URI is spec conformant.
There was a problem hiding this comment.
Added a commit to cover these cases
crates/web5_cli/src/dids/create.rs
Outdated
| let mut public_jwk = private_jwk.clone(); | ||
| public_jwk.d = None; | ||
|
|
||
| let valid_url = if domain.starts_with("http://") || domain.starts_with("https://") { |
There was a problem hiding this comment.
this is really amazing code @diehuxx 👏
so good, in fact, I'm going to move it inside the DidWeb::new() function, so that we enable this DX not just for the CLI experience but also for application-level use cases. I feel confident exposing this code such that we can expose to developers, "send in any endpoint and we'll create a did:web instance out of it"
also I spy a slight bug in the case wherein both a .well-known and did.json are passed in, like in Example 4 here
Add basic CLI to create and print a did web document