VC 2.0 Add validUntil & validFrom / make credentialStatus.id checks optional #168

aljones15 · 2023-12-08T19:20:59Z

Features:

changes the DateRegexp to an XML schema DateTime and expands tests on it
move expirationDate into the VC 1.0 validators and only checks date when mode is verify
adds validUntil and associated tests
adds validFrom and associated tests
make some minor corrections with test data for better consistency
This pr now contains credentialStatus check changes from here
Makes credentialStatus.id optional
Checks to ensure if credentialStatus.id is present it must be a URL.

JSAssassin

LGTM, just some minor comments. TY!

Edit: In the PR title and the description it says validBefore, but maybe you intended to refer to validAfter instead?

lib/index.js

test/10-verify.spec.js

test/helpers.js

codecov-commenter · 2024-01-02T17:57:01Z

Codecov Report

❗ No coverage uploaded for pull request base (update-vc-2.0@9ff44fe). Click here to learn what that means.
The diff coverage is n/a.

Additional details and impacted files

@@               Coverage Diff                @@
##             update-vc-2.0     #168   +/-   ##
================================================
  Coverage                 ?   90.54%           
================================================
  Files                    ?        5           
  Lines                    ?     1058           
  Branches                 ?        0           
================================================
  Hits                     ?      958           
  Misses                   ?      100           
  Partials                 ?        0

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9ff44fe...0bb92ad. Read the comment docs.

BigBlueHat · 2024-01-11T15:33:47Z

test/10-verify.spec.js

+          error = e;
+        }
+        should.exist(error,
+          'Should throw error when "evidence" is not a URI');


What's "evidence" here? Looks like a copy/paste error.

I actually was assuming as evidence is moving out the spec that credentialStatus was now a piece of evidence? @davidlehn is this a mistake or should the credentialStatus.id not being a URI be taken as an evidence error?

Looks like typo. Fixed. Do these id changes need a changelog entry? Not sure what's going in this for this PR.

BigBlueHat · 2024-01-11T15:38:29Z

@aljones15 it looks like this PR has moved (or is moving) well beyond validUntil and validBefore. Is that correct? If so, we should relabel the PR at least.

I'm also a bit concerned about us ending up with one massive PR to review. Can we do more, smaller PRs against a non-main branch (as this one is against update-vc-2.0)? That would make reviews much faster, safer, and more sane. 😃

aljones15 · 2024-01-11T15:47:20Z

I will refrain from merging any further PRs into this PR, but as this PR and the other PR: #170 had both been approved it seemed safe to merge the two.

As for this PR moving beyond validUntil & validBefore that was not intentional, but the feature making credentialStatus.id optional branched from the validUntil branch so it couldn't be aimed at the core VC 2.0 branch/PR.

aljones15 · 2024-01-11T20:57:10Z

test/10-verify.spec.js

@@ -808,7 +808,7 @@ for(const [version, mockCredential] of versionedCredentials) {
          .contain('"issuer" must be a URI');
      });

-      it('should reject credentialStatus that is not a URI', () => {
+      it('should reject credentialStatus id that is not a URI', () => {


the spec says URL actually. Also, only one id is allowed.

Can you match the spec text where possible? Makes it much easier to match tests to the spec. Thanks!

I don't know what needs changing here. Please do whatever is needed. If only one status is allowed, the check code should check that, and tests should be written to fail on multiple statuses.

@BigBlueHat the tests inside of the libraries usually don't match the spec text so what you're asking might be better as a separate issue. If you want it done here I could get a start on it in this pr.

BigBlueHat · 2024-01-11T21:03:35Z

I will refrain from merging any further PRs into this PR, but as this PR and the other PR: #170 had both been approved it seemed safe to merge the two.

As for this PR moving beyond validUntil & validBefore that was not intentional, but the feature making credentialStatus.id optional branched from the validUntil branch so it couldn't be aimed at the core VC 2.0 branch/PR.

Mostly, my aim is to keep topics clear...and now it's pretty muddied what's going on.

Also, as @JSAssassin mentioned earlier, the fields are validFrom and validUntil--and we should change this PR to match.

Thanks!

CHANGELOG.md

test/10-verify.spec.js

…C 1.0 checks.

Co-authored-by: Tashi D. Gyeltshen <[email protected]>

…stamp. Co-authored-by: Tashi D. Gyeltshen <[email protected]>

- Make "id" optional. - If "id" present, check it's a URL. - Add tests.

Co-authored-by: BigBlueHat <[email protected]>

aljones15 · 2024-04-29T15:58:19Z

@BigBlueHat @davidlehn please double check this as this is your last chance before merge into update-vc-2.0.

aljones15 changed the title ~~Move expirationDate test to v1 tests & use mockCredential in all tests.~~ VC 2.0 add validUntil and validBefore Dec 8, 2023

aljones15 force-pushed the update-vc-2.0 branch from dbaf8c8 to fdc98f6 Compare December 8, 2023 19:24

aljones15 requested review from JSAssassin and BigBlueHat December 8, 2023 19:25

aljones15 marked this pull request as ready for review December 8, 2023 19:25

JSAssassin approved these changes Dec 8, 2023

View reviewed changes

lib/index.js Outdated Show resolved Hide resolved

test/10-verify.spec.js Outdated Show resolved Hide resolved

test/helpers.js Outdated Show resolved Hide resolved

aljones15 mentioned this pull request Dec 8, 2023

VC 2.0 Release Canidate #148

Merged

12 tasks

aljones15 mentioned this pull request Jan 10, 2024

Update credentialStatus checks. #170

Merged

BigBlueHat requested changes Jan 11, 2024

View reviewed changes

aljones15 added the enhancement label Jan 11, 2024

aljones15 commented Jan 11, 2024

View reviewed changes

BigBlueHat requested changes Jan 11, 2024

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

test/10-verify.spec.js Outdated Show resolved Hide resolved

aljones15 changed the title ~~VC 2.0 add validUntil and validBefore~~ VC 2.0 Add validUntil & validBefore / make credentialStatus.id checks optional Jan 13, 2024

aljones15 changed the title ~~VC 2.0 Add validUntil & validBefore / make credentialStatus.id checks optional~~ VC 2.0 Add validUntil & validFrom / make credentialStatus.id checks optional Jan 15, 2024

aljones15 added the VC 2.0 label Feb 5, 2024

aljones15 force-pushed the vc-2.0-time-props branch from 000dfb7 to 0bb92ad Compare February 5, 2024 18:23

aljones15 force-pushed the update-vc-2.0 branch from 9ff44fe to 4f59e13 Compare April 19, 2024 21:07

aljones15 added 9 commits April 29, 2024 15:51

Add initial validUntil and validFrom tests.

9d57469

Clarify and expand VC 2.0 validUntil & validFrom tests.

9d518b8

Move 2 validFrom & 2 validUntil issuer tests to issuer section.

c5b79bc

Add suite & documentLoader & await issue.

997ea76

Move issue both validFrom and validUntil to issue section of test suite.

ad84534

Add checkContextVersion & replace hasV1CredentialContext.

4590dfd

Move dateRegex to helpers & add assertion for date times.

6a61b28

Fix faulty assertion in vc 2.0 time sensitive test.

37b4426

Fix test data for expired validUntil.

6f26a8d

aljones15 and others added 20 commits April 29, 2024 15:51

Make test fail description for validFrom consistent with others.

12221e5

Use dynamic dates for issue tests.

5194cfb

Use live dates to better integration test expiration logic.

616cab4

Use full ISOString for dynamic dates.

aa6b7e9

Drop the from validBefore and validAfter error messages.

a25906c

Assert on error messages for validUntil and validBefore.

115fc39

Date checks return datetime verbatim & use seconds in test data.

122045d

Use mockCredential() in validFrom & validUntil same time test.

acfda2c

Move expirationDate test to v1 tests & use mockCredential in all tests.

aeefdcb

Add issuanceDate format check under issuanceDate existence check in V…

0e113e6

…C 1.0 checks.

Check validUntil & validFrom format on issue.

4236613

Update lib/index.js clarify jsdoc comments about issuanceDate.

6ab781b

Co-authored-by: Tashi D. Gyeltshen <[email protected]>

Update test/10-verify.spec.js remove extra blank line.

989a1e7

Co-authored-by: Tashi D. Gyeltshen <[email protected]>

Update test/helpers.js use substring & template string for date time …

da83d8d

…stamp. Co-authored-by: Tashi D. Gyeltshen <[email protected]>

Remove extra semicolon from inline github code edit.

067bd63

Update credentialStatus checks.

c9110e5

- Make "id" optional. - If "id" present, check it's a URL. - Add tests.

Fix credentialStatus id test typos.

dd4dfd8

Update test/10-verify.spec.js remove ref to validAfter in jsdoc comment.

f39d44f

Co-authored-by: BigBlueHat <[email protected]>

Update CHANGELOG.md remove validAfter from CHANGELOG.

a5fd168

Co-authored-by: BigBlueHat <[email protected]>

Import mock.data in verify & export credentials from mock.data.

be2533e

aljones15 force-pushed the vc-2.0-time-props branch from 0bb92ad to be2533e Compare April 29, 2024 15:57

aljones15 requested review from davidlehn, JSAssassin and BigBlueHat April 29, 2024 15:57

aljones15 removed the request for review from JSAssassin April 29, 2024 15:58

dlongley approved these changes Apr 29, 2024

View reviewed changes

Attempt to fix codecov upload.

b78162e

dlongley merged commit d3abfab into update-vc-2.0 Apr 29, 2024
5 checks passed

dlongley deleted the vc-2.0-time-props branch July 25, 2024 22:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

VC 2.0 Add validUntil & validFrom / make credentialStatus.id checks optional #168

VC 2.0 Add validUntil & validFrom / make credentialStatus.id checks optional #168

aljones15 commented Dec 8, 2023 •

edited

Loading

JSAssassin left a comment •

edited

Loading

codecov-commenter commented Jan 2, 2024 •

edited

Loading

BigBlueHat Jan 11, 2024

aljones15 Jan 11, 2024

davidlehn Jan 11, 2024

BigBlueHat commented Jan 11, 2024

aljones15 commented Jan 11, 2024 •

edited

Loading

aljones15 Jan 11, 2024

BigBlueHat Jan 11, 2024

davidlehn Jan 11, 2024

aljones15 Jan 14, 2024

BigBlueHat commented Jan 11, 2024

aljones15 commented Apr 29, 2024

VC 2.0 Add validUntil & validFrom / make credentialStatus.id checks optional #168

VC 2.0 Add validUntil & validFrom / make credentialStatus.id checks optional #168

Conversation

aljones15 commented Dec 8, 2023 • edited Loading

JSAssassin left a comment • edited Loading

Choose a reason for hiding this comment

codecov-commenter commented Jan 2, 2024 • edited Loading

Codecov Report

BigBlueHat Jan 11, 2024

Choose a reason for hiding this comment

aljones15 Jan 11, 2024

Choose a reason for hiding this comment

davidlehn Jan 11, 2024

Choose a reason for hiding this comment

BigBlueHat commented Jan 11, 2024

aljones15 commented Jan 11, 2024 • edited Loading

aljones15 Jan 11, 2024

Choose a reason for hiding this comment

BigBlueHat Jan 11, 2024

Choose a reason for hiding this comment

davidlehn Jan 11, 2024

Choose a reason for hiding this comment

aljones15 Jan 14, 2024

Choose a reason for hiding this comment

BigBlueHat commented Jan 11, 2024

aljones15 commented Apr 29, 2024

aljones15 commented Dec 8, 2023 •

edited

Loading

JSAssassin left a comment •

edited

Loading

codecov-commenter commented Jan 2, 2024 •

edited

Loading

aljones15 commented Jan 11, 2024 •

edited

Loading