From 213d2fa07aba49e6d67ce580921e9786e8a526eb Mon Sep 17 00:00:00 2001 From: Andrew Whitehead Date: Mon, 20 Jun 2022 15:15:38 -0700 Subject: [PATCH] fix IssuerCredRevRecord state update on revocation publish; add unit test Signed-off-by: Andrew Whitehead --- aries_cloudagent/indy/sdk/issuer.py | 2 +- .../indy/sdk/tests/test_issuer.py | 21 +++-- aries_cloudagent/revocation/manager.py | 62 +++++++------ .../models/issuer_cred_rev_record.py | 7 +- .../revocation/tests/test_manager.py | 93 ++++++++++++++----- 5 files changed, 125 insertions(+), 60 deletions(-) diff --git a/aries_cloudagent/indy/sdk/issuer.py b/aries_cloudagent/indy/sdk/issuer.py index 8fe7d23777..72f569f2ee 100644 --- a/aries_cloudagent/indy/sdk/issuer.py +++ b/aries_cloudagent/indy/sdk/issuer.py @@ -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 ): diff --git a/aries_cloudagent/indy/sdk/tests/test_issuer.py b/aries_cloudagent/indy/sdk/tests/test_issuer.py index 5981233abc..0378044314 100644 --- a/aries_cloudagent/indy/sdk/tests/test_issuer.py +++ b/aries_cloudagent/indy/sdk/tests/test_issuer.py @@ -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 diff --git a/aries_cloudagent/revocation/manager.py b/aries_cloudagent/revocation/manager.py index 85729ac9f2..4057d7a799 100644 --- a/aries_cloudagent/revocation/manager.py +++ b/aries_cloudagent/revocation/manager.py @@ -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 ) @@ -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] ) @@ -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 ) @@ -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 diff --git a/aries_cloudagent/revocation/models/issuer_cred_rev_record.py b/aries_cloudagent/revocation/models/issuer_cred_rev_record.py index cb87c070b9..5ab8d3ba09 100644 --- a/aries_cloudagent/revocation/models/issuer_cred_rev_record.py +++ b/aries_cloudagent/revocation/models/issuer_cred_rev_record.py @@ -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 diff --git a/aries_cloudagent/revocation/tests/test_manager.py b/aries_cloudagent/revocation/tests/test_manager.py index 470107b6ab..113c4850d1 100644 --- a/aries_cloudagent/revocation/tests/test_manager.py +++ b/aries_cloudagent/revocation/tests/test_manager.py @@ -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 ( @@ -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, @@ -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" @@ -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 @@ -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 = [ { @@ -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