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

Add revoked and expires properties to JsonWebKey context. #184

Merged
merged 5 commits into from
Sep 2, 2023

Conversation

msporny
Copy link
Member

@msporny msporny commented Aug 26, 2023

This PR attempts to address issue #182 by adds the revoked and expires properties to JsonWebKey context. It also differentiates the anchors for expires on proof and verificationMethod.


Preview | Diff

SHOULD cease to be used. Once the value is set, it is not expected to be updated, and
systems depending on the value are expected to not verify any proofs associated
with the <a>verification method</a> at or after the time of revocation.
SHOULD cease to be used. Once the value is set, it is not expected to be
Copy link
Contributor

Choose a reason for hiding this comment

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

@msporny this seems like a great places to remove confusion over "verification method revocation" and "credential revocation"... we get this question a LOT... and it would nice to be able to share a note to a comment regarding this. something like:

verification method revocation time information can reveal information about a controller. credential status do not include timing information, because it can degrade the privacy characteristics of some status lists... there is not formal relationship between these concepts, etc....

Copy link
Member Author

@msporny msporny Aug 28, 2023

Choose a reason for hiding this comment

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

Agreed, I'll add that text to that effect in a future update to this PR.

Copy link
Member Author

Choose a reason for hiding this comment

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

Added in c4aede2.

Copy link
Member

@TallTed TallTed left a comment

Choose a reason for hiding this comment

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

The language we developed later is better, and should be applied throughout, as here.

index.html Outdated Show resolved Hide resolved
Comment on lines +212 to +214
domain:
- sec:Proof
- sec:VerificationMethod
Copy link
Member

@TallTed TallTed Sep 1, 2023

Choose a reason for hiding this comment

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

According to @iherman in w3c/vc-data-model#1262 (comment)

the domain expired is set to both (i.e., the union) of Proof and VerificationMethod.

Note that expires rdfs:domain Proof, VerificationMethod does not set rdfs:domain to the union of Proof and VerificationMethod, but rather to their intersection; that is, any entity with the property expires must be both a Proof and a VerificationMethod.

For the apparently intended union, where an entity with the property expires could be either a Proof or a VerificationMethod, rdfs:domain must be changed to schema:domainIncludes.

Suggested change
domain:
- sec:Proof
- sec:VerificationMethod
domainIncludes:
- sec:Proof
- sec:VerificationMethod

Copy link
Member Author

Choose a reason for hiding this comment

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

@iherman would have to add this to the yml2vocab tool, as I don't think it has a domainIncludes property? Thoughts, @iherman?

@msporny
Copy link
Member Author

msporny commented Sep 2, 2023

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

@msporny msporny merged commit 041bcf5 into main Sep 2, 2023
1 check passed
@msporny msporny deleted the msporny-jwk-rev-exp branch September 2, 2023 14:58
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.

7 participants