diff --git a/src/outcomes.ts b/src/outcomes.ts index 5989582..36d5b4f 100644 --- a/src/outcomes.ts +++ b/src/outcomes.ts @@ -330,9 +330,14 @@ const believedChecks = (r: VerificationResponse, options: OutcomeOptions): Map): boolean => - hasProblem(checks.get(CHECK.status), PROBLEM.revoked) && !listNotIssuers(checks); + sealHeld(checks) && hasProblem(checks.get(CHECK.status), PROBLEM.revoked) && !listNotIssuers(checks); /** * The issuer has suspended this: set aside, possibly for now. verifier-core @@ -340,15 +345,20 @@ const isWithdrawn = (checks: Map): boolean => * "withdrawn". Withdrawal leads where a credential carries both. */ const isSuspended = (checks: Map): boolean => - hasProblem(checks.get(CHECK.status), PROBLEM.suspended) && !listNotIssuers(checks); + sealHeld(checks) && hasProblem(checks.get(CHECK.status), PROBLEM.suspended) && !listNotIssuers(checks); /** * The withdrawal list loaded, is believed (by default whoever signed it; with * `requireIssuerSignedStatusList`, only the issuer's own), and doesn't list * this. The only grounds for saying the issuer hasn't withdrawn it. + * + * Behind a held seal only, like its siblings above. No test can catch this + * guard today, since everything that reads this already stands behind a + * held seal; it is here because "the issuer hasn't deactivated it" is the + * claim that must never leak past a broken seal. Don't remove it. */ const notWithdrawn = (checks: Map): boolean => - passed(checks.get(CHECK.status)) && !listNotIssuers(checks); + sealHeld(checks) && passed(checks.get(CHECK.status)) && !listNotIssuers(checks); /** The withdrawal check ran, failed, and established nothing either way. */ const withdrawalUnavailable = (check: CheckResult | undefined): boolean => @@ -1244,7 +1254,13 @@ export const listChecks = (r: VerificationResponse, options: OutcomeOptions = {} }); const revocation = checks.get(CHECK.status); - if (!hasStatusList(r)) { + if (!sealHeld(checks)) { + // Behind a seal that didn't hold, whether the credential has a list, and + // which entry of it is its own, are the credential's own claims: removing + // its status entry would otherwise read "the issuer set up no way to + // deactivate this". So the row reads "not checked", as Dates does. + rows.push({ id: CHECK.status, label: 'Status', severity: 'unchecked', value: 'not checked' }); + } else if (!hasStatusList(r)) { // No list at all. Nothing failed, and we cannot say it was not withdrawn // either — there was no way to withdraw it. Saying so is for the issuer // debugging their own setup, who otherwise cannot tell this apart from a diff --git a/test/consistency-combinations.ts b/test/consistency-combinations.ts index a01d22c..d738185 100644 --- a/test/consistency-combinations.ts +++ b/test/consistency-combinations.ts @@ -642,9 +642,9 @@ export const agreementTests = (file: string): void => { stranger ? /^Let .+ know we couldn't check whether this copy is still active\.$/ : /^Try again in a little while\.$/, ); } - // Only behind a seal that held: behind a broken one the Status row - // should be about the seal, not the list. verifier-plugin#32 is that - // fix, making it "not checked" whatever the list said. + // Only behind a seal that held: behind a broken one the Status row is + // about the seal and reads "not checked" whatever the list said + // (verifier-plugin#32), as asserted below with the Dates row. if (row(CHECK.signature)?.severity === 'success') { expect(row(CHECK.status)?.value, `${where}: the wrong Status row`).toBe( stranger ? "couldn't check — the information we found may not be from the issuer" : 'not checked', @@ -772,10 +772,14 @@ export const agreementTests = (file: string): void => { expect(row(CHECK.signature)?.severity, `${where}: a date reported without the seal`).toBe('success'); } - // ...and the Dates row says the same: no date at all, "in date" - // included, behind a seal that didn't hold. + // ...and the Dates and Status rows say the same: no date and no status + // at all behind a seal that didn't hold — not "in date", not "not + // deactivated", and not "no way to deactivate this", which removing a + // credential's status entry would otherwise buy. if (row(CHECK.signature)?.severity !== 'success') { expect(row(DATES_ROW)?.value, `${where}: the Dates row behind an unheld seal`).toBe('not checked'); + expect(row(CHECK.status)?.value, `${where}: the Status row behind an unheld seal`).toBe('not checked'); + expect(row(CHECK.status)?.severity, `${where}: the Status row behind an unheld seal`).toBe('unchecked'); } // Only the library says a credential has run out. An end date in the diff --git a/test/outcomes.test.ts b/test/outcomes.test.ts index eb3ab94..d4f92dc 100644 --- a/test/outcomes.test.ts +++ b/test/outcomes.test.ts @@ -589,12 +589,33 @@ describe('an action names the issuer only when the seal held', () => { }); it('says "the issuer" when the seal never reported', () => { + // "Built wrong" is about the file, so it leads even behind an unchecked + // seal, and its action is the one that would name them. + const r = withSchema(missingProperty); + remove(r, CHECK.signature); + const out = summarise(r); + expect(out.code).toBe('malformed'); + expect(out.action).toBe("The issuer needs to fix how this was issued. There's nothing for you to do."); + }); + + it('believes no withdrawal when the seal never reported', () => { + // Which entry of the list is this credential's is its own claim, so a + // withdrawal behind an unchecked seal is no more believed than an expiry. + // The library halts after a failed seal; the rule must not depend on it. const r = ok(); revoke(r); remove(r, CHECK.signature); - const out = summarise(r); - expect(out.code).toBe('withdrawn'); - expect(out.action).toBe('Ask the issuer for a current copy.'); + expect(summarise(r).code).toBe('signature_unchecked'); + expect(listChecks(r).find((c) => c.id === CHECK.status)).toMatchObject({ severity: 'unchecked', value: 'not checked' }); + }); + + it('believes no hold when the seal never reported', () => { + // As for a withdrawal, the same for a hold. + const r = ok(); + set(r, revokedBy(PROBLEM.suspended, 'Credential Suspended')); + remove(r, CHECK.signature); + expect(summarise(r).code).toBe('signature_unchecked'); + expect(listChecks(r).find((c) => c.id === CHECK.status)).toMatchObject({ severity: 'unchecked', value: 'not checked' }); }); it.each([ @@ -1894,6 +1915,40 @@ describe('an expired credential, now that expiry stops nothing after it', () => }); }); +describe('the Status row behind a seal that did not hold', () => { + // verifier-plugin#32. Whether a credential has a status list, and which + // entry of it is its own, are the credential's claims; behind a broken + // seal they are worth nothing, so the row says "not checked". + it.each([ + ['tampered', () => tamperedSignature()], + ['unreachable', () => fail(CHECK.signature, [{ type: PROBLEM.didWebUnresolved, title: 't', detail: 'd' }], true)], + ])('reads "not checked" when the seal is %s and there is no list', (_, signature) => { + const r = ok(); + r.verifiableCredential = { ...withoutStatusList }; + remove(r, CHECK.status); + set(r, signature()); + expect(row(r, CHECK.status)).toMatchObject({ severity: 'unchecked', value: 'not checked' }); + }); + + it.each([ + ['came back clear', () => pass(CHECK.status, 'Credential status is valid (not revoked or suspended).')], + ['says deactivated', () => revokedBy(PROBLEM.revoked, 'Credential Revoked')], + ['says on hold', () => revokedBy(PROBLEM.suspended, 'Credential Suspended')], + ])('reads "not checked" when the seal is tampered and the list %s', (_, status) => { + // "Not checked" whatever the list said. Either the row's own guard or the + // matching predicate's is enough to pass these: notWithdrawn's for a list + // that came back clear, isWithdrawn's and isSuspended's for the other two. + // The row's guard alone is pinned by the "no list" cases above; + // isWithdrawn's and isSuspended's by "believes no withdrawal" and + // "believes no hold when the seal never reported". notWithdrawn's can't + // be pinned alone (see its docblock). + const r = ok(); + set(r, tamperedSignature()); + set(r, status()); + expect(row(r, CHECK.status)).toMatchObject({ severity: 'unchecked', value: 'not checked' }); + }); +}); + describe('dates nobody checked, though the seal held', () => { // A check that throws comes back as a failure carrying CHECK_ERROR, with no // date named; a run limited to some phases leaves no row at all.