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 notes for validFrom and validUntil. #646

Merged
merged 5 commits into from Jun 18, 2019
Merged

Conversation

msporny
Copy link
Member

@msporny msporny commented May 26, 2019

Reserve validFrom and validUntil, note expected changes in next version of spec.

Related to #584.


Preview | Diff

index.html Outdated
that is backwards-compatible. The range of acceptable values will remain the
same as will the semantic meaning of the <a>property</a>. Implementers are
advised that the <code>validFrom</code> <a>property</a> is reserved and its use
for any other purpose is discouraged.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think there's a slight problem here for JWT representation. We current express issuanceDate as the "iat" claim in a JWT. The validFrom property should instead map to the "nbf" claim in a JWT.

It is true that we intended "issuanceDate" to mean validFrom in the absence of that property. However, I think both properties could be present in a VC with different values and, under that circumstance, validFrom should take precedence for determining validity. So issuanceDate means the earliest point at which a VC should be considered valid, unless validFrom is present. Of course, we can't say that normatively now and we're just reserving this for the future.

So I think it's still true that the range of acceptable values will remain the same and the semantic meaning of the property will be the same but this will not be a "renaming", rather, the presence of validFrom would take precedence over issuanceDate when determining the VC validity period. Also, as mentioned above, mapping these properties to a JWT would result in using different JWT claims.

cc: @TallTed

Copy link
Member

Choose a reason for hiding this comment

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

Good catch. I think that in the current iteration, we must express issuanceDate as nbf (as this is and was the intended semantic), and not as iat (which was never the intended semantic). Future versions of the VCDM can change things however is appropriate at that point, including how precedence lays out if/when both are present.

Copy link
Member Author

@msporny msporny Jun 8, 2019

Choose a reason for hiding this comment

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

Ok, updating the PR w/ these changes in mind.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed in afaedca

@msporny
Copy link
Member Author

msporny commented Jun 8, 2019

Need a re-review on this one as it too complicated to just merge on its own.

@msporny
Copy link
Member Author

msporny commented Jun 9, 2019

@dlongley and @TallTed, can both of you provide a re-review please?

@TallTed
Copy link
Member

TallTed commented Jun 10, 2019

@msporny, @dlongley -- I think associated changes (which I think would be best made in the same merge) are also needed to the JWT mapping section, particularly (in the msporny-issue-584 branch) the following, which are now focused on JWT exp and iat and do not mention nbf --

@TallTed TallTed mentioned this pull request Jun 11, 2019
@TallTed
Copy link
Member

TallTed commented Jun 18, 2019

As it seems my review is the blocker here...

I cannot approve this without the changes to the associated areas that I highlighted previously.

I think these would count as bugfix, though they would change normative text, and should therefore not trigger a new CR.

@dlongley dlongley merged commit 5a1f617 into gh-pages Jun 18, 2019
@stonematt
Copy link
Contributor

PR was merged to make way for the resolution from WG call on 18 June 19:
RESOLVED: Change spec text mapping of issuanceDate to use JWT nbf claim (with matching semantics) instead of JWT iat claim (with conflicting semantics) (#646), in parallel to testsuite change to match (w3c/vc-test-suite#50)

@msporny msporny deleted the msporny-issue-584 branch June 30, 2019 18:10
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.

None yet

7 participants