Resolve did:web #24
Resolve did:web #24
Conversation
amika-sq
requested review from
andresuribe87,
mistermoe,
decentralgabe,
KendallWeihe,
phoebe-lew and
diehuxx
| } | ||
|
|
||
| #[tokio::test] | ||
| async fn resolution_success() { |
There was a problem hiding this comment.
is this doing a real resolution? should it be mocked?
There was a problem hiding this comment.
Yes, it's a real resolution. I can add a mock if we want to.
There was a problem hiding this comment.
For reference, Spruce is testing mocked responses here: https://github.com/spruceid/ssi/blob/main/did-web/src/lib.rs#L197-L218
There was a problem hiding this comment.
I actually prefer not mocking but I believe we're mocking elsewhere in our libs. Just flagging because the test could fail even if it's correct, since it relies on an external resource. if there's a way to distinguish network tests somehow that would be useful.
There was a problem hiding this comment.
I second the intuition of finding a way to distinguish an integration test vs a unit test, though maybe unnecessary at this stage and we could stub in a GitHub Issue on a backlog to address later
|
|
||
| #[tokio::test] | ||
| async fn resolution_success() { | ||
| let did_uri = "did:web:tbd.website"; |
There was a problem hiding this comment.
Would be nice to add a test like did:web:users.tbddev.org:demo which should resolve to https://users.tbddev.org/demo/did.json
There was a problem hiding this comment.
Agreed 👍
It's strange to me that this test actually passes, though I see the result.did_document.expect("did_document not found") below. Does the did:web spec allow non-error resolution wherein the no-results case is indicated by an empty/null document? Seems weird to me
There was a problem hiding this comment.
Is this valid? I'm trying to use https://dev.uniresolver.io/ and it's not able to resolve this 🤔
There was a problem hiding this comment.
Hmm, maybe uniresolver doesn't support DID web!
There was a problem hiding this comment.
Did a little digging. Resolving this DID currently fails because it's using an unknown VerificationMethod property publicKeyHex. This doesn't appear in the spec: https://www.w3.org/TR/did-core/#verification-method-properties
Is there another spec that does include this? If so, why have these diverged?
@decentralgabe maybe you have some extra insight here?
There was a problem hiding this comment.
@amika-sq it's not included in the default context for did-core, but it is included in the security vocab here -- https://github.com/w3c-ccg/security-vocab/pull/40/files
this means to make that did web valid it would have to add the context like this
{
"@context": ["https://w3id.org/did/v1", "https://w3id.org/security/v3-unstable"],
"id": "did:web:users.tbddev.org:demo",
"verificationMethod": [
{
"id": "did:web:users.tbddev.org:demo#key-0",
"type": "EcdsaSecp256k1VerificationKey2019",
"publicKeyHex": "04e8e82698e64db79e13dc6e7478f47a5fe867c4acd6fa8928b0c93c763a02fd6c54a622de247cc04449531429dbb6a2373b6c685a8e1326913e0531e5f649208d"
}
]
}or not use hex
There was a problem hiding this comment.
really we should be using JWKs everywhere.
There was a problem hiding this comment.
nice this lgtm, but I think we should make the change @andresuribe87 suggested around the non-empty resolution
Resolve
did:webDIDs using Spruce's did-web crate. Uses ourDidResolutionResultimplementation so that the resolved DIDDocument & all respective metadata is in the expected format according to the community draft resolution spec here.Resolves #12