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.

Sign up for GitHub

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

Revocation hotfixes #579

Merged
merged 4 commits into from
Jun 26, 2020
Merged

Revocation hotfixes #579

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
fd60b81
work in progress
sklump Jun 26, 2020
22679bd
revocation updates
sklump Jun 26, 2020
556da57
revocation updates; master merge; black tweaks
sklump Jun 26, 2020
5f6c280
fix error when ctrl-D is pressed on option menu
andrewwhitehead Jun 26, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions aries_cloudagent/issuer/base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ async def create_credential(
@abstractmethod
async def revoke_credentials(
self, revoc_reg_id: str, tails_file_path: str, cred_revoc_ids: Sequence[str]
) -> str:
) -> (str, Sequence[str]):
"""
Revoke a set of credentials in a revocation registry.

Expand All @@ -153,7 +153,7 @@ async def revoke_credentials(
cred_revoc_ids: sequences of credential indexes in the revocation registry

Returns:
the combined revocation delta
Tuple with the combined revocation delta, list of cred rev ids not revoked

"""

Expand Down
36 changes: 28 additions & 8 deletions aries_cloudagent/issuer/indy.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ async def create_credential(
(
credential_json,
credential_revocation_id,
revoc_reg_delta_json,
_, # rev_reg_delta_json only figures if rev reg is ISSUANCE_ON_DEMAND
) = await indy.anoncreds.issuer_create_credential(
self.wallet.handle,
json.dumps(credential_offer),
Expand All @@ -236,7 +236,7 @@ async def create_credential(

async def revoke_credentials(
self, revoc_reg_id: str, tails_file_path: str, cred_revoc_ids: Sequence[str]
) -> str:
) -> (str, Sequence[str]):
"""
Revoke a set of credentials in a revocation registry.

Expand All @@ -246,26 +246,46 @@ async def revoke_credentials(
cred_revoc_ids: sequences of credential indexes in the revocation registry

Returns:
the combined revocation delta
Tuple with the combined revocation delta, list of cred rev ids not revoked

"""
failed_crids = []
tails_reader_handle = await create_tails_reader(tails_file_path)

result_json = None
for cred_revoc_id in cred_revoc_ids:
with IndyErrorHandler("Exception when revoking credential", IssuerError):
# may throw AnoncredsInvalidUserRevocId if using ISSUANCE_ON_DEMAND
delta_json = await indy.anoncreds.issuer_revoke_credential(
self.wallet.handle, tails_reader_handle, revoc_reg_id, cred_revoc_id
)
try:
delta_json = await indy.anoncreds.issuer_revoke_credential(
self.wallet.handle,
tails_reader_handle,
revoc_reg_id,
cred_revoc_id,
)
except IndyError as error:
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice, this is great.

Also note @esune I believe this should cure the problem you were seeing without any changes on your end.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

... then take the troublesome cred_rev_ids on the rev_reg_id and purge them via clear_pending_revocations API as per issue_credentials/v1_0/routes.py, submitting body

{
    "rrid2crid": {
        "<rev-reg-id>": ["<cred-rev-id>", "<cred-rev-id>", ...]
    }
}

if error.error_code == ErrorCode.AnoncredsInvalidUserRevocId:
self.logger.error(
"Abstaining from revoking credential on "
f"rev reg id {revoc_reg_id}, cred rev id={cred_revoc_id}: "
"already revoked or not yet issued"
)
else:
self.logger.error(
IndyErrorHandler.wrap_error(
error, "Revocation error", IssuerError
).roll_up
)
failed_crids.append(cred_revoc_id)
continue

if result_json:
result_json = await self.merge_revocation_registry_deltas(
result_json, delta_json
)
else:
result_json = delta_json

return result_json
return (result_json, failed_crids)

async def merge_revocation_registry_deltas(
self, fro_delta: str, to_delta: str
Expand Down
88 changes: 87 additions & 1 deletion aries_cloudagent/issuer/tests/test_indy.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,13 +191,99 @@ async def test_create_revoke_credentials(

mock_indy_revoke_credential.return_value = json.dumps(TEST_RR_DELTA)
mock_indy_merge_rr_deltas.return_value = json.dumps(TEST_RR_DELTA)
result = await self.issuer.revoke_credentials(
(result, failed) = await self.issuer.revoke_credentials(
REV_REG_ID, tails_file_path="dummy", cred_revoc_ids=test_cred_rev_ids
)
assert json.loads(result) == TEST_RR_DELTA
assert not failed
assert mock_indy_revoke_credential.call_count == 2
mock_indy_merge_rr_deltas.assert_called_once()

@async_mock.patch("indy.anoncreds.issuer_create_credential")
@async_mock.patch("aries_cloudagent.issuer.indy.create_tails_reader")
@async_mock.patch("indy.anoncreds.issuer_revoke_credential")
@async_mock.patch("indy.anoncreds.issuer_merge_revocation_registry_deltas")
async def test_create_revoke_credentials_x(
self,
mock_indy_merge_rr_deltas,
mock_indy_revoke_credential,
mock_tails_reader,
mock_indy_create_credential,
):
test_schema = {"attrNames": ["attr1"]}
test_offer = {
"schema_id": SCHEMA_ID,
"cred_def_id": CRED_DEF_ID,
"key_correctness_proof": {"c": "...", "xz_cap": "...", "xr_cap": ["..."]},
"nonce": "...",
}
test_request = {"test": "request"}
test_values = {"attr1": "value1"}
test_cred = {
"schema_id": SCHEMA_ID,
"cred_def_id": CRED_DEF_ID,
"rev_reg_id": REV_REG_ID,
"values": {"attr1": {"raw": "value1", "encoded": "123456123899216581404"}},
"signature": {"...": "..."},
"signature_correctness_proof": {"...": "..."},
"rev_reg": {"accum": "21 12E8..."},
"witness": {"omega": "21 1369..."},
}
test_cred_rev_ids = ["42", "54", "103"]
test_rr_delta = TEST_RR_DELTA
mock_indy_create_credential.side_effect = [
(json.dumps(test_cred), cr_id, test_rr_delta,)
for cr_id in test_cred_rev_ids
]

with self.assertRaises(IssuerError): # missing attribute
cred_json, revoc_id = await self.issuer.create_credential(
test_schema, test_offer, test_request, {}
)

(cred_json, cred_rev_id) = await self.issuer.create_credential( # main line
test_schema,
test_offer,
test_request,
test_values,
REV_REG_ID,
"/tmp/tails/path/dummy",
)
mock_indy_create_credential.assert_called_once()
(
call_wallet,
call_offer,
call_request,
call_values,
call_etc1,
call_etc2,
) = mock_indy_create_credential.call_args[0]
assert call_wallet is self.wallet.handle
assert json.loads(call_offer) == test_offer
assert json.loads(call_request) == test_request
values = json.loads(call_values)
assert "attr1" in values

mock_indy_revoke_credential.side_effect = [
json.dumps(TEST_RR_DELTA),
IndyError(
error_code=ErrorCode.AnoncredsInvalidUserRevocId,
error_details={"message": "already revoked"},
),
IndyError(
error_code=ErrorCode.UnknownCryptoTypeError,
error_details={"message": "truly an outlier"},
),
]
mock_indy_merge_rr_deltas.return_value = json.dumps(TEST_RR_DELTA)
(result, failed) = await self.issuer.revoke_credentials(
REV_REG_ID, tails_file_path="dummy", cred_revoc_ids=test_cred_rev_ids
)
assert json.loads(result) == TEST_RR_DELTA
assert failed == ["54", "103"]
assert mock_indy_revoke_credential.call_count == 3
mock_indy_merge_rr_deltas.assert_not_called()

@async_mock.patch("indy.anoncreds.issuer_create_credential")
@async_mock.patch("aries_cloudagent.issuer.indy.create_tails_reader")
async def test_create_credential_rr_full(
Expand Down
12 changes: 11 additions & 1 deletion aries_cloudagent/messaging/tests/test_valid.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@
DID_KEY,
ENDPOINT,
INDY_CRED_DEF_ID,
INDY_CRED_REV_ID,
INDY_DID,
INT_EPOCH,
INDY_EXTRA_WQL,
INDY_ISO8601_DATETIME,
INDY_PREDICATE,
Expand All @@ -23,6 +23,7 @@
INDY_SCHEMA_ID,
INDY_VERSION,
INDY_WQL,
INT_EPOCH,
NATURAL_NUM,
JWS_HEADER_KID,
JWT,
Expand Down Expand Up @@ -200,6 +201,15 @@ def test_rev_reg_id(self):
"Q4zqM7aXqm7gDQkUVLng9h:2:bc-reg:1.0:tag:CL_ACCUM:0"
) # long

def test_cred_rev_id(self):
non_cred_rev_ids = ["Wg", "0", "-5", "3.14"]
for non_cred_rev_id in non_cred_rev_ids:
with self.assertRaises(ValidationError):
INDY_CRED_REV_ID["validate"](non_cred_rev_id)

INDY_CRED_REV_ID["validate"]("1")
INDY_CRED_REV_ID["validate"]("99999999")

def test_version(self):
non_versions = ["-1", "", "3_5", "3.5a"]
for non_version in non_versions:
Expand Down
16 changes: 16 additions & 0 deletions aries_cloudagent/messaging/valid.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,21 @@ def __init__(self):
)


class IndyCredRevId(Regexp):
"""Validate value against indy credential revocation identifier specification."""

EXAMPLE = "12345"
PATTERN = rf"^[1-9][0-9]*$"

def __init__(self):
"""Initializer."""

super().__init__(
IndyCredRevId.PATTERN,
error="Value {input} is not an indy credential revocation identifier",
)


class IndyPredicate(OneOf):
"""Validate value against indy predicate."""

Expand Down Expand Up @@ -426,6 +441,7 @@ def __init__(self):
INDY_SCHEMA_ID = {"validate": IndySchemaId(), "example": IndySchemaId.EXAMPLE}
INDY_CRED_DEF_ID = {"validate": IndyCredDefId(), "example": IndyCredDefId.EXAMPLE}
INDY_REV_REG_ID = {"validate": IndyRevRegId(), "example": IndyRevRegId.EXAMPLE}
INDY_CRED_REV_ID = {"validate": IndyCredRevId(), "example": IndyCredRevId.EXAMPLE}
INDY_VERSION = {"validate": IndyVersion(), "example": IndyVersion.EXAMPLE}
INDY_PREDICATE = {"validate": IndyPredicate(), "example": IndyPredicate.EXAMPLE}
INDY_ISO8601_DATETIME = {
Expand Down
Loading