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

Address kid #163

Merged
merged 4 commits into from
Oct 10, 2023
Merged

Address kid #163

merged 4 commits into from
Oct 10, 2023

Conversation

OR13
Copy link
Contributor

@OR13 OR13 commented Oct 2, 2023

Attempt to close remaining kid issues: #30 and #117


Preview | Diff

@OR13 OR13 changed the title Add examples, remove closed issues Address kid Oct 2, 2023
@vdods
Copy link

vdods commented Oct 3, 2023

Looks great!

Though I think there might be a typo in the links to "DID URL" -- they point to "relative DID URL":

When the <a data-cite="VC-DATA-MODEL#dfn-issuers">issuer</a> is identified as a <a data-cite="DID-CORE#did-subject">DID Subject</a>, 
        the <a href="#kid">kid</a> MUST be an absolute <a data-cite="DID-CORE#relative-did-urls">DID URL</a>.

I believe there are a few places in the changes that have this typo.

If this is intentional, then please ignore this comment.

@OR13
Copy link
Contributor Author

OR13 commented Oct 4, 2023

It is intentional, I found that section most helpful in explaining the difference.

<!-- REGULAR URLS via "issuer" and "holder" -->
<p>
When the <a data-cite="VC-DATA-MODEL#dfn-issuers">issuer</a> is identified as a [[URL]],
the <a href="#kid">kid</a> MUST be an absolute [[URL]] to a verification method listed in a controller document.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can "controller document" be a reference to a defined term?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sadly not yet, this is due to the dialog regarding data integrity defining controller document.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See: #160

Copy link
Collaborator

@selfissued selfissued left a comment

Choose a reason for hiding this comment

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

LGTM, modulo my non-blocking comment about what "controller document" should refer to.

// ...
}
</pre>
<pre class="example" title="An kid as an absolute DID URL">
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
<pre class="example" title="An kid as an absolute DID URL">
<pre class="example" title="A kid as an absolute DID URL">

<!-- REGULAR URLS via "iss" -->

<p>
When <a href="iss">iss</a> is a present, and is a [[URL]],
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
When <a href="iss">iss</a> is a present, and is a [[URL]],
When <a href="iss">iss</a> is present, and is a [[URL]],

</p>

<p class="issue" title="(AT RISK) Feature depends on demonstration of independent implementations">
This normative statement depends on a -00 IETF OAUTH WG Adopted draft.
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
This normative statement depends on a -00 IETF OAUTH WG Adopted draft.
This normative statement depends on a
<a href="https://datatracker.ietf.org/doc/html/draft-ietf-oauth-sd-jwt-vc-00#name-jwt-issuer-metadata">-00 IETF OAUTH WG Adopted draft</a>

@TallTed
Copy link
Member

TallTed commented Oct 24, 2023

@OR13 -- Please let me know if you need a PR from me for the above minor tweaks to this. I would have preferred to submit them as change requests before this PR was merged, but time travel remains tragically unavailable.

@OR13
Copy link
Contributor Author

OR13 commented Oct 24, 2023

@TallTed please raise a PR to make these suggestions.

@TallTed
Copy link
Member

TallTed commented Oct 24, 2023

Done. See #171.

@decentralgabe decentralgabe deleted the better-kid-guidance branch February 26, 2024 20:06
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.

5 participants