Skip to content

Conversation

msporny
Copy link
Member

@msporny msporny commented Jun 14, 2024

This PR is an attempt to address issue #31 by defining the properties that are required and optional in controller documents. This text is ported from the DID Core specification (it was deleted, by mistake, when the original document was ported over).


Preview | Diff

@iherman
Copy link
Member

iherman commented Jun 15, 2024

The new text includes this:

<p class="note" title="Property names used in map of different types">
The property names `id`, `type`, and `controller` can be present in map of
different types with possible differences in constraints.
 </p>

Unless I completely misunderstand, I believe this note contains essential information that should be part of the core text and elaborated upon. This note means that different other specification can use the controller document "structure" in their own, security related specification. I.e., a Verifiable Credential, a Verifiable Presentation, or a DID document are all examples of controller documents (and this is how this specification relates to the VCDM or DID).

If this is all true, it should be made much more clear and explicit somewhere in the document, and shown also with examples.

From an ontology/vocabulary point of view, the clean way would be to define a class for ControllerDocument on which some properties are defined, and then a VerifiableCredential, a VerifiablePresentation ProofGraph or a DIDDocument is defined as a subclass. (It may be too late to do that.)

(Edited 12 hours later, realizing that a VC or a VP is probably not a Controller Document. My mistake.)

@iherman
Copy link
Member

iherman commented Jun 15, 2024

Also related to #32 (comment): the terminology on controller document is, sort of, circular:

controller
An entity that has the capability to make changes to a controller document.
controller document
A set of data that specifies one or more relationships between a controller and a set of data, such as a set of public cryptographic keys.

Nowhere is the term "controller document" properly defined; the definition above fits any kind of resource under the Sun ("such as" means that any property can be present...). The Data model section, which includes the aforementioned note, is also void of real information.

We should come up with a more specific definition in the terminology section to make the term more "restrictive", i.e., to really separate Controller Documents from other resources.

(I must admit that I do not understand how this term "Controller" fits here. But it may be too late to come up with a better name. All the more important to provide a better descriptive text.)

index.html Outdated
Comment on lines 550 to 567
<td>`assertionMethod`</td>
<td>no</td>

</tr>
<tr>
<td>`keyAgreement`</td>
<td>no</td>

</tr>
<tr>
<td>`capabilityInvocation`</td>
<td>no</td>

</tr>
<tr>
<td>`capabilityDelegation`</td>
<td>no</td>

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
<td>`assertionMethod`</td>
<td>no</td>
</tr>
<tr>
<td>`keyAgreement`</td>
<td>no</td>
</tr>
<tr>
<td>`capabilityInvocation`</td>
<td>no</td>
</tr>
<tr>
<td>`capabilityDelegation`</td>
<td>no</td>
<td>`assertionMethod`</td>
<td>no</td>
<td>none</td>
</tr>
<tr>
<td>`keyAgreement`</td>
<td>no</td>
<td>none</td>
</tr>
<tr>
<td>`capabilityInvocation`</td>
<td>no</td>
<td>none</td>
</tr>
<tr>
<td>`capabilityDelegation`</td>
<td>no</td>
<td>none</td>

index.html Outdated
<tr>
<td>`authentication`</td>
<td>no</td>
<td rowspan="5">
Copy link
Member

@TallTed TallTed Jun 19, 2024

Choose a reason for hiding this comment

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

If the rowspan works, then the following suggestion with the none cells can be rejected. It might be good to list the properties to which this cell applies, within this cell.

Suggested change
<td rowspan="5">
<td rowspan="5" style="vertical-align: middle;">

Copy link
Member Author

Choose a reason for hiding this comment

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

I ended up implementing this instead: 345e8e4 ... which builds off of one of your corrections that helped me see a way forward w/o using "rowspan".

@iherman
Copy link
Member

iherman commented Jun 19, 2024

The issue was discussed in a meeting on 2024-06-19

  • no resolutions were taken
View the transcript

3.1. Add section on Controller Document data model. (pr controller-document#32)

See github pull request controller-document#32.

Manu Sporny: This replaces a section that was accidentally removed a while ago. We never actually said what we would find in the controller document. Lifted text from DIDCORE with modifications.
… Talks about what are the mandatory and optional properties beyond just verification methods and relationships.
… Some feedback on text from Ivan, some good points but the further we get from what DIDCORE says the harder it is to maintain alignment.
… TallTed's changes are also good.
… Need guidance from the group. Are we going to address Ivan's more fundamental concerns about the PR?

Brent Zundel: Not seeing anyone on the queue. It seems like Ivan's comments might be better addressed as a separate issue rather than as part of this PR.

Ted Thibodeau Jr.: Table changes are also editorial.

David Chadwick: Question to Manu. If the controller property is present and says "Fred and Mary can do this", what does it say about the subject?
… Does the subject still have all the properties as if the controller is absent?

Manu Sporny: If there is no controller field, then the subject is the controller. If there is a controller, I believe that most implementations allow the subject to have the same power.

David Chadwick: Will raise an issue.

Manu Sporny: TallTed, trying to bunch the same description in the fields, will try to fix the table.

KevinDean: Supports the need for clarification of controller. Parents may be controllers of a child's VC without wanting the child to have the same capability.

@iherman
Copy link
Member

iherman commented Jun 26, 2024

The issue was discussed in a meeting on 2024-06-26

  • no resolutions were taken
View the transcript

3.1. Add section on Controller Document data model. (pr controller-document#32)

See github pull request controller-document#32.

Manu Sporny: we have a number of approvals already on these additions to the data model.
… the text is mainly from DID core so I prefer not to change it in the Controller document, but rather raise issues on DID core.
… specifically to ivan's comments.

Ivan Herman: I could not properly explain what a controller document is from the existing text.
… need a higher level section to explain what the controller document is actually about.

@msporny msporny force-pushed the msporny-required-properties branch from 345e8e4 to b891f34 Compare June 30, 2024 17:22
@msporny
Copy link
Member Author

msporny commented Jun 30, 2024

Normative, multiple reviews, changes requested and made, no objections, merging.

@msporny msporny merged commit d356d77 into main Jun 30, 2024
@msporny msporny deleted the msporny-required-properties branch June 30, 2024 17:24
TallTed added a commit that referenced this pull request Jul 1, 2024
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.

6 participants