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

Fix IssuerCredRevRecord state update on revocation publish #1827

Merged
merged 1 commit into from
Jun 21, 2022
Merged

Fix IssuerCredRevRecord state update on revocation publish #1827

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion aries_cloudagent/indy/sdk/issuer.py
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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