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

Wrong ID Property in VerificationMethod specification #74

Open
elribonazo opened this issue Dec 12, 2024 · 6 comments
Open

Wrong ID Property in VerificationMethod specification #74

elribonazo opened this issue Dec 12, 2024 · 6 comments

Comments

@elribonazo
Copy link

Hey hello, would like to report an issue in the specification and align between the did-core specification and the latest didpeer4.

Issue: One specification states that verificationMethod id and any other KeyId must conform with the DIDUrl specification, most DIDComm implementations also stick to that same fact and ask for a keyId (expected as DIDUrl) VS didpeer 4 specification which comes only with the fragment.

Correct

  • did + #key-0

Incorrect

  • #key-0

My proposal would be to use the correct version and stick to the correct version, the docs are literally stating that, despite the requirement being a MUST, the implementations can choose not to follow it, and that doesn't seem correct.

DIDCore specification

Specification https://www.w3.org/TR/did-core/#authentication

The value of the id property for a verification method MUST be a string that conforms to the rules in Section 3.2 DID URL Syntax.

DIDUrl syntax is:
did-url = did path-abempty [ "?" query ] [ "#" fragment ]
Which includes the fragment.

DID Peer 4 specification

Specification https://identity.foundation/peer-did-method-spec/#resolving-a-did

On resolution docs we find a note:
Note: Implementations may turn relative references in the document into absolute references by prepending the reference with the DID. This is not recommended due to length but this is an implementation detail that should not affect usage of the resolved document. Both relative and absolute references are valid within DID Documents.

@elribonazo
Copy link
Author

@FabioPinheiro

@TheTechmage
Copy link

Bringing this to your attention @dbluhm

@dbluhm
Copy link
Member

dbluhm commented Dec 12, 2024

Hm. That makes sense. I seem to have interpreted the Relative DID URLs section as meaning that a Relative DID URL could be used anywhere an absolute DID URL is used but it only says that they may be used for referencing another point in the document. In my defense, I think it is a bit underspecified; it could be more specific about where DID URLs are permitted -- but I agree that the verificationMethod[].id property is sufficiently clear. Given that, I agree with you that it is probably inappropriate for the note to recommend not transforming the relative URLs to absolute on resolution.

For the sake of clarity, when constructing the Input Document, it would still be required to use just a fragment as the identifier for verification methods since the DID is not yet known.

To bring the did:peer:4 spec in line with the DID Core spec, the required change would be to drop the note and add a step about turning the ID of each resource from fragments into absolute URLs during resolution in the "contextualize" step.

@elribonazo are you up to submitting a PR with that change?

@swcurran
Copy link
Collaborator

@peacekeeper — could you please take a look at this from the DID Core / DID URL perspective? I’d like to make sure we do have to change the Peer DID Spec to align it with the DID Core. The approach of just using a fragment on an id does make sense (such as is used in markdown, if nothing else), so wondered if this should be considered in a future DID Core spec.

Thanks

@dhh1128
Copy link
Collaborator

dhh1128 commented Dec 12, 2024

A historical note: when we first added relative URIs into the peer DID method spec, DID core used the word "SHOULD" to describe the requirement to use absolute URIs inside a DID doc, Manu was pushing for "MUST", and I was pushing the other direction; I saw no particular benefit to using absolute URIs for references that took a DID as its base URL, particularly for intra-document references. However, I am not sure that I fully understood Manu's reasoning; it is possible that there is some compelling argument that escaped me.

Anyway, if the spec now says that intra-doc references MUST be absolute URIs, then perhaps Manu convinced the larger community and that divergence came into being when the change was adopted.

I don't have a strong opinion about what to do now; I just added this info FWIW.

@FabioPinheiro
Copy link
Contributor

FabioPinheiro commented Dec 12, 2024

If we consider this I would also like to consider making the controller in the verification missing when this is the same as the id of the DID Document. https://www.w3.org/TR/did-core/#verification-method-properties

I would going either further specifying that in a DID Document all reference must be either relative or absolute to another DID that is not the id of the DID Document.

As a library implementer I would like the interface (in this case the DID Document) converge instead of having many ways to define the same thing.
That being said having everything as absolute ref (as it is specified now in the DID core specs) is good IMO, and is the more simple case. As a developer, I don't need to care about special cases of each property in the DID Document.
The DID document is not support to be send/received many times from the network. And if tomorrow someone like the universal resolver want to minimize the size of DIDDocument they could come up with a 1 to 1 conversion into another format.

In my opinion, we should only fix the did:peer specification.

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

No branches or pull requests

6 participants