Implement Revoked check for a key when signing manifest, schema, presentation request. #579
Conversation
presentation request, etc
vlad-tim
requested review from
decentralgabe,
nitro-neal and
andresuribe87
as code owners
pkg/service/schema/service.go
Outdated
There was a problem hiding this comment.
Would it make sense to add a test for this changes as well?
There was a problem hiding this comment.
I believe that this change is already tested here https://github.com/TBD54566975/ssi-service/pull/579/files#diff-c16fc760aec028bd13fafd51b015b31db88fbd79a1e130f3097fc3010b838ac7R264-R268
pkg/server/router/credential_test.go
Outdated
| @@ -261,6 +261,11 @@ func TestCredentialRouter(t *testing.T) { | |||
| assert.Empty(tt, createdCred) | |||
| assert.Error(tt, err) | |||
| assert.ErrorContains(tt, err, "cannot use revoked key") | |||
|
|
|||
| // create a schema with the revoked key, it fails | |||
| _, err = schemaService.CreateSchema(context.Background(), schema.CreateSchemaRequest{Issuer: controllerDID.DID.ID, Name: "schema (revoked key)", Schema: getEmailSchema(), FullyQualifiedVerificationMethodID: keyID}) | |||
There was a problem hiding this comment.
Seems like the wrong file to test this in. Can you move to router/schema_test.go?
There was a problem hiding this comment.
Yes, I can. I put the test in this file because it seemed like a simplest way to test the change with minimal number of new lines (since the key revocation logic is already in place), and because schema creation was already being tested in this file anyways.
Codecov Report
@@ Coverage Diff @@
## main #579 +/- ##
==========================================
+ Coverage 24.00% 24.08% +0.08%
==========================================
Files 46 46
Lines 5312 5315 +3
==========================================
+ Hits 1275 1280 +5
+ Misses 3799 3796 -3
- Partials 238 239 +1
|
Overview
This PR adds
Revokedchecks for a key when signing manifest, schema, presentation request. This is a follow-up to #546, in particular addresses this comment #546 (comment).Description
It is related to #451. Previously in #546 revocation check was implemented for credential creation. This PR adds the check to all other relevant places. Please help verify that all applicable scenarios where revocation check is required are covered.
How Has This Been Tested?
Extended the existing auto-tests to cover the new code.
Checklist
Before submitting this PR, please make sure:
References
See above.