Basic support for credential revocation and revocation registry handling #306

andrewwhitehead · 2019-12-13T00:09:48Z

Adds RevocationRegistry class for interfacing with local and external revocation registries
Adds IssuerRevocationRecord for tracking revocation registry generation and issuance
Adds an admin route for generating a new registry against a credential definition
Adds parameters to credential-definition and issue-credential routes for supporting revocation

lgtm-com · 2019-12-13T00:23:06Z

This pull request introduces 1 alert when merging c3c1010 into 6d42d1e - view on LGTM.com

new alerts:

1 for __eq__ not overridden when adding attributes

codecov-io · 2019-12-13T00:26:36Z

Codecov Report

❗ No coverage uploaded for pull request base (master@89b350d). Click here to learn what that means.
The diff coverage is 45.45%.

@@            Coverage Diff            @@
##             master     #306   +/-   ##
=========================================
  Coverage          ?   86.85%           
=========================================
  Files             ?      243           
  Lines             ?    11874           
  Branches          ?        0           
=========================================
  Hits              ?    10313           
  Misses            ?     1561           
  Partials          ?        0

lgtm-com · 2019-12-13T00:37:53Z

This pull request introduces 1 alert when merging a5bfd36 into 6d42d1e - view on LGTM.com

new alerts:

1 for __eq__ not overridden when adding attributes

andrewwhitehead · 2019-12-14T04:02:57Z

Remaining work:

When a credential is revoked, the delta that is produced should likely be combined with the revocation entry on the associated IssuerRevocationRecord if there is a previous entry. This can be done with indy.anoncreds.issuer_merge_revocation_registry_deltas.
There is likely a need for a separate Admin API call in the revocation package to publish the current revocation entry on an IssuerRevocationRecord (referenced by revocation registry ID) to the ledger. At the moment only the initial entry is published, when revocation/create-registry is called.
The number of credentials issued against a revocation registry is not currently tracked (except internally by Indy, but that value is not exposed). Either the controller could track this itself, or it could be stored on the IssuerRevocationRecord. It might also be useful to track the date of the earliest unpublished delta per IssuerRevocationRecord and use this when determining whether to publish changes to the ledger.
Changes are required to catch the exception thrown when trying to issue a credential against a full revocation registry and set an error state on the credential exchange. The controller might use this as a signal to create and publish a new registry.

sklump · 2019-12-17T12:54:30Z

aries_cloudagent/messaging/credential_definitions/routes.py

-        **INDY_SCHEMA_ID
+    schema_id = fields.Str(description="Schema identifier", **INDY_SCHEMA_ID)
+    support_revocation = fields.Boolean(
+        required=False, description="Revocation supported flag"


default=False is implicit here by the way the code gets the value, but setting it explicitly could be useful here for clarity of intent?

sklump · 2019-12-17T13:00:26Z

aries_cloudagent/messaging/schemas/routes.py

        required=True,
-        description="List of schema attributes"
+        description="List of schema attributes",


comma consistency

That's an annoying issue with the Black autoformatter, it will insert the trailing comma when a call spans multiple lines, but doesn't remove it when it later shortens it to one line.

sklump · 2019-12-17T13:00:37Z

aries_cloudagent/messaging/schemas/routes.py

-            description="attribute name",
-            example="score",
-        ),
+        fields.Str(description="attribute name", example="score",),


comma consistency

sklump · 2019-12-17T13:01:00Z

aries_cloudagent/messaging/schemas/routes.py

-        description="Schema name",
-        example="prefs",
-    )
+    schema_name = fields.Str(required=True, description="Schema name", example="prefs",)


comma consistency

sklump · 2019-12-17T13:01:44Z

aries_cloudagent/messaging/schemas/routes.py

-            description="Attribute name",
-            example="score",
-        ),
+        fields.Str(description="Attribute name", example="score",),


comma consistency

sklump · 2019-12-17T13:03:03Z

aries_cloudagent/messaging/schemas/routes.py

-            "schema": {"type": "string"},
-            "required": False,
-        } for p in SCHEMA_TAGS
+        {"name": p, "in": "query", "schema": {"type": "string"}, "required": False}


To me this looks way less clear than the one on the left. My impression only.

sklump · 2019-12-17T13:06:29Z

aries_cloudagent/protocols/issue_credential/v1_0/manager.py

@@ -142,7 +154,7 @@ def context(self) -> InjectionContext:
            schema_name=schema_name,
            schema_version=schema_version,
            cred_def_id=cred_def_id,
-            issuer_did=issuer_did
+            issuer_did=issuer_did,


I hate to harp on comma consistency so I'll stop here.

lgtm-com · 2020-01-31T03:00:04Z

This pull request introduces 1 alert when merging 3f63113 into 9e30014 - view on LGTM.com

new alerts:

1 for __eq__ not overridden when adding attributes

lgtm-com · 2020-02-11T23:23:17Z

This pull request introduces 2 alerts when merging 26c8228 into 0b3cdc0 - view on LGTM.com

new alerts:

1 for __eq__ not overridden when adding attributes
1 for Unused local variable

lgtm-com · 2020-02-12T01:00:39Z

This pull request introduces 1 alert when merging 705c463 into 0b3cdc0 - view on LGTM.com

new alerts:

1 for __eq__ not overridden when adding attributes

aries_cloudagent/revocation/routes.py

sklump · 2020-02-12T12:36:09Z

demo/runners/support/agent.py

+        log_msg(f"Revocation Registry ID: {revocation_registry_id}")
+        assert tails_hash == my_tails_hash
+
+        # Real app should publish tail file somewhere and update the revocation registry with the URI.


demo/runners/support/agent.py

aries_cloudagent/revocation/models/issuer_revocation_record.py

aries_cloudagent/revocation/routes.py

demo/runners/support/agent.py

sklump · 2020-02-12T12:45:35Z

aries_cloudagent/revocation/routes.py

+    except RevocationNotSupportedError as e:
+        raise web.HTTPBadRequest(reason=e.message) from e
+    await shield(
+        registry_record.generate_registry(context, RevocationRegistry.get_temp_dir())


This operation can take minutes - consider a background process that generates the revocation registry ahead of time, with tag increment.

Alternatively, the non-secrets API could hold a work queue.

…n-revoked proof Signed-off-by: Pengyu Chen <[email protected]>

…f is not valid. Signed-off-by: Pengyu Chen <[email protected]>

Signed-off-by: Pengyu Chen <[email protected]>

The revocation part in performance demo is disabled because the revocation registry won't be found immediately after they are registered. Signed-off-by: Pengyu Chen <[email protected]>

Signed-off-by: Pengyu Chen <[email protected]>

Changed the faber demo to add another revocation registry when it is full. Signed-off-by: Pengyu Chen <[email protected]>

- Select latest credential when making proof - Add an option to add revocation registry - Catch error when issuing credential Signed-off-by: Pengyu Chen <[email protected]>

Signed-off-by: Pengyu Chen <[email protected]>

Signed-off-by: Andrew Whitehead <[email protected]>

lgtm-com · 2020-03-04T17:21:59Z

This pull request introduces 1 alert when merging 75fd1be into 26f7107 - view on LGTM.com

new alerts:

1 for __eq__ not overridden when adding attributes

lgtm-com · 2020-03-05T07:06:08Z

This pull request introduces 1 alert when merging 4522cee into 3c719eb - view on LGTM.com

new alerts:

1 for __eq__ not overridden when adding attributes

aries_cloudagent/revocation/indy.py

aries_cloudagent/revocation/routes.py

Signed-off-by: Andrew Whitehead <[email protected]>

lgtm-com · 2020-03-05T14:52:11Z

This pull request introduces 1 alert when merging 3b95740 into 3c719eb - view on LGTM.com

new alerts:

1 for __eq__ not overridden when adding attributes

sklump

Looks great!

demo/runners/faber.py

andrewwhitehead added the incomplete Needs more work label Dec 13, 2019

swcurran mentioned this pull request Dec 16, 2019

Implement revocation registry support for credential issuance #146

Closed

3 tasks

sklump reviewed Dec 17, 2019

View reviewed changes

andrewwhitehead force-pushed the feature/rev-reg branch from 9302af9 to 3f63113 Compare January 31, 2020 02:49