Skip to content

Commit

Permalink
Merge pull request #1827 from andrewwhitehead/fix/upd-crd-rev
Browse files Browse the repository at this point in the history 
Fix IssuerCredRevRecord state update on revocation publish
  • Loading branch information
@swcurran
swcurran authored Jun 21, 2022
2 parents 4240fa9 + 213d2fa commit 4d29a2f
Show file tree
Hide file tree
Showing 5 changed files with 125 additions and 60 deletions.
2 changes: 1 addition & 1 deletion aries_cloudagent/indy/sdk/issuer.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,7 @@ async def revoke_credentials(
tails_reader_handle = await create_tails_reader(tails_file_path)

result_json = None
for cred_rev_id in cred_rev_ids:
for cred_rev_id in set(cred_rev_ids):
with IndyErrorHandler(
"Exception when revoking credential", IndyIssuerError
):
Expand Down
21 changes: 12 additions & 9 deletions aries_cloudagent/indy/sdk/tests/test_issuer.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -311,17 +311,20 @@ async def test_create_revoke_credentials_x(
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(
def mock_revoke(_h, _t, _r, cred_rev_id):
if cred_rev_id == "42":
return json.dumps(TEST_RR_DELTA)
if cred_rev_id == "54":
raise IndyError(
error_code=ErrorCode.AnoncredsInvalidUserRevocId,
error_details={"message": "already revoked"},
)
raise IndyError(
error_code=ErrorCode.UnknownCryptoTypeError,
error_details={"message": "truly an outlier"},
),
]
)

mock_indy_revoke_credential.side_effect = mock_revoke
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_rev_ids=test_cred_rev_ids
Expand Down
62 changes: 34 additions & 28 deletions aries_cloudagent/revocation/manager.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ async def revoke_credential(
rev_reg = await revoc.get_ledger_registry(rev_reg_id)
await rev_reg.get_or_fetch_local_tails_path()
# pick up pending revocations on input revocation registry
crids = list(set(issuer_rr_rec.pending_pub + [cred_rev_id]))
crids = (issuer_rr_rec.pending_pub or []) + [cred_rev_id]
(delta_json, _) = await issuer.revoke_credentials(
issuer_rr_rec.revoc_reg_id, issuer_rr_rec.tails_local_path, crids
)
Expand All @@ -145,9 +145,9 @@ async def revoke_credential(
issuer_rr_upd.revoc_reg_entry = json.loads(delta_json)
await issuer_rr_upd.clear_pending(txn, crids)
await txn.commit()
await self.set_cred_revoked_state(rev_reg_id, crids)
if delta_json:
await issuer_rr_upd.send_entry(self._profile)
await self.set_cred_revoked_state(rev_reg_id, [cred_rev_id])
await notify_revocation_published_event(
self._profile, rev_reg_id, [cred_rev_id]
)
Expand Down Expand Up @@ -215,11 +215,11 @@ async def publish_pending_revocations(
issuer_rr_upd.revoc_reg_entry = json.loads(delta_json)
await issuer_rr_upd.clear_pending(txn, crids)
await txn.commit()
await self.set_cred_revoked_state(issuer_rr_rec.revoc_reg_id, crids)
if delta_json:
await issuer_rr_upd.send_entry(self._profile)
published = sorted(crid for crid in crids if crid not in failed_crids)
result[issuer_rr_rec.revoc_reg_id] = published
await self.set_cred_revoked_state(issuer_rr_rec.revoc_reg_id, crids)
await notify_revocation_published_event(
self._profile, issuer_rr_rec.revoc_reg_id, crids
)
Expand Down Expand Up @@ -289,34 +289,40 @@ async def set_cred_revoked_state(
"""
for cred_rev_id in cred_rev_ids:
cred_ex_id = None

try:
async with self._profile.transaction() as txn:
rev_rec = await IssuerCredRevRecord.retrieve_by_ids(
txn, rev_reg_id, cred_rev_id, for_update=True
)
cred_ex_id = rev_rec.cred_ex_id
rev_rec.state = IssuerCredRevRecord.STATE_REVOKED
await rev_rec.save(txn, reason="revoke credential")
await txn.commit()
except StorageNotFoundError:
continue

async with self._profile.transaction() as txn:
try:
rev_rec = await IssuerCredRevRecord.retrieve_by_ids(
txn, rev_reg_id, cred_rev_id
cred_ex_record = await V10CredentialExchange.retrieve_by_id(
txn, cred_ex_id, for_update=True
)
cred_ex_record.state = (
V10CredentialExchange.STATE_CREDENTIAL_REVOKED
)
try:
cred_ex_record = await V10CredentialExchange.retrieve_by_id(
txn, rev_rec.cred_ex_id, for_update=True
)
cred_ex_record.state = (
V10CredentialExchange.STATE_CREDENTIAL_REVOKED
)
await cred_ex_record.save(txn, reason="revoke credential")
await txn.commit()

except StorageNotFoundError:
try:
cred_ex_record = await V20CredExRecord.retrieve_by_id(
txn, rev_rec.cred_ex_id, for_update=True
)
cred_ex_record.state = (
V20CredExRecord.STATE_CREDENTIAL_REVOKED
)
await cred_ex_record.save(txn, reason="revoke credential")
await txn.commit()

except StorageNotFoundError:
pass
await cred_ex_record.save(txn, reason="revoke credential")
await txn.commit()
continue # skip 2.0 record check
except StorageNotFoundError:
pass

try:
cred_ex_record = await V20CredExRecord.retrieve_by_id(
txn, cred_ex_id, for_update=True
)
cred_ex_record.state = V20CredExRecord.STATE_CREDENTIAL_REVOKED
await cred_ex_record.save(txn, reason="revoke credential")
await txn.commit()
except StorageNotFoundError:
pass
7 changes: 6 additions & 1 deletion aries_cloudagent/revocation/models/issuer_cred_rev_record.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,10 +90,15 @@ async def retrieve_by_ids(
session: ProfileSession,
rev_reg_id: str,
cred_rev_id: str,
*,
for_update: bool = False,
) -> "IssuerCredRevRecord":
"""Retrieve an issuer cred rev record by rev reg id and cred rev id."""
return await cls.retrieve_by_tag_filter(
session, {"rev_reg_id": rev_reg_id}, {"cred_rev_id": cred_rev_id}
session,
{"rev_reg_id": rev_reg_id},
{"cred_rev_id": cred_rev_id},
for_update=for_update,
)

@classmethod
Expand Down
93 changes: 72 additions & 21 deletions aries_cloudagent/revocation/tests/test_manager.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@
from asynctest import TestCase as AsyncTestCase
from more_itertools import side_effect

from aries_cloudagent.revocation.models.issuer_cred_rev_record import (
IssuerCredRevRecord,
)

from ...core.in_memory import InMemoryProfile
from ...indy.issuer import IndyIssuer
from ...protocols.issue_credential.v1_0.models.credential_exchange import (
Expand Down Expand Up @@ -38,7 +42,25 @@ async def test_revoke_credential_publish(self):
tails_local_path=TAILS_LOCAL,
send_entry=async_mock.CoroutineMock(),
clear_pending=async_mock.CoroutineMock(),
pending_pub=["2"],
)
issuer = async_mock.MagicMock(IndyIssuer, autospec=True)
issuer.revoke_credentials = async_mock.CoroutineMock(
return_value=(
json.dumps(
{
"ver": "1.0",
"value": {
"prevAccum": "1 ...",
"accum": "21 ...",
"issued": [1],
},
}
),
[],
)
)
self.profile.context.injector.bind_instance(IndyIssuer, issuer)

with async_mock.patch.object(
test_module.IssuerCredRevRecord,
Expand All @@ -64,26 +86,14 @@ async def test_revoke_credential_publish(self):
return_value=mock_rev_reg
)

issuer = async_mock.MagicMock(IndyIssuer, autospec=True)
issuer.revoke_credentials = async_mock.CoroutineMock(
return_value=(
json.dumps(
{
"ver": "1.0",
"value": {
"prevAccum": "1 ...",
"accum": "21 ...",
"issued": [1],
},
}
),
[],
)
)
self.profile.context.injector.bind_instance(IndyIssuer, issuer)

await self.manager.revoke_credential_by_cred_ex_id(CRED_EX_ID, publish=True)

issuer.revoke_credentials.assert_awaited_once_with(
mock_issuer_rev_reg_record.revoc_reg_id,
mock_issuer_rev_reg_record.tails_local_path,
["2", "1"],
)

async def test_revoke_cred_by_cxid_not_found(self):
CRED_EX_ID = "dummy-cxid"

Expand Down Expand Up @@ -128,6 +138,8 @@ async def test_revoke_credential_pend(self):
mock_issuer_rev_reg_record = async_mock.MagicMock(
mark_pending=async_mock.CoroutineMock()
)
issuer = async_mock.MagicMock(IndyIssuer, autospec=True)
self.profile.context.injector.bind_instance(IndyIssuer, issuer)

with async_mock.patch.object(
test_module, "IndyRevocation", autospec=True
Expand All @@ -148,14 +160,13 @@ async def test_revoke_credential_pend(self):
return_value=mock_issuer_rev_reg_record
)

issuer = async_mock.MagicMock(IndyIssuer, autospec=True)
self.profile.context.injector.bind_instance(IndyIssuer, issuer)

await self.manager.revoke_credential(REV_REG_ID, CRED_REV_ID, False)
mock_issuer_rev_reg_record.mark_pending.assert_called_once_with(
session.return_value, CRED_REV_ID
)

issuer.revoke_credentials.assert_not_awaited()

async def test_publish_pending_revocations_basic(self):
deltas = [
{
Expand Down Expand Up @@ -415,3 +426,43 @@ async def test_retrieve_records(self):
)
assert ret_ex.connection_id == str(index)
assert ret_ex.thread_id == str(1000 + index)

async def test_set_revoked_state(self):
CRED_REV_ID = "1"

async with self.profile.session() as session:
exchange_record = V10CredentialExchange(
connection_id="mark-revoked-cid",
thread_id="mark-revoked-tid",
initiator=V10CredentialExchange.INITIATOR_SELF,
revoc_reg_id=REV_REG_ID,
revocation_id=CRED_REV_ID,
role=V10CredentialExchange.ROLE_ISSUER,
state=V10CredentialExchange.STATE_ISSUED,
)
await exchange_record.save(session)

crev_record = IssuerCredRevRecord(
cred_ex_id=exchange_record.credential_exchange_id,
cred_def_id=CRED_DEF_ID,
rev_reg_id=REV_REG_ID,
cred_rev_id=CRED_REV_ID,
state=IssuerCredRevRecord.STATE_ISSUED,
)
await crev_record.save(session)

await self.manager.set_cred_revoked_state(REV_REG_ID, [CRED_REV_ID])

async with self.profile.session() as session:
check_exchange_record = await V10CredentialExchange.retrieve_by_id(
session, exchange_record.credential_exchange_id
)
assert (
check_exchange_record.state
== V10CredentialExchange.STATE_CREDENTIAL_REVOKED
)

check_crev_record = await IssuerCredRevRecord.retrieve_by_id(
session, crev_record.record_id
)
assert check_crev_record.state == IssuerCredRevRecord.STATE_REVOKED

0 comments on commit 4d29a2f

Please sign in to comment.