From 9c65346b78c2fb2de17c1900f101939446174bc3 Mon Sep 17 00:00:00 2001 From: Andrew Whitehead Date: Mon, 14 Mar 2022 15:33:34 -0700 Subject: [PATCH 1/3] fetch stored credentials prior to sending ack response in issue-credential 2.0 Signed-off-by: Andrew Whitehead --- .../protocols/issue_credential/v2_0/routes.py | 43 +++++++++++++------ 1 file changed, 30 insertions(+), 13 deletions(-) diff --git a/aries_cloudagent/protocols/issue_credential/v2_0/routes.py b/aries_cloudagent/protocols/issue_credential/v2_0/routes.py index b7d98758b0..c8e1fea7f7 100644 --- a/aries_cloudagent/protocols/issue_credential/v2_0/routes.py +++ b/aries_cloudagent/protocols/issue_credential/v2_0/routes.py @@ -396,19 +396,31 @@ def _formats_filters(filt_spec: Mapping) -> Mapping: ) -async def _get_result_with_details( +async def _get_attached_credentials( profile: Profile, cred_ex_record: V20CredExRecord ) -> Mapping: - """Get credential exchange result with detail records.""" - result = {"cred_ex_record": cred_ex_record.serialize()} + """Fetch the detail records attached to a credential exchange.""" + result = {} for fmt in V20CredFormat.Format: detail_record = await fmt.handler(profile).get_detail_record( cred_ex_record.cred_ex_id ) + if detail_record: + result[fmt.api] = detail_record + + return result - result[fmt.api] = detail_record.serialize() if detail_record else None +def _format_result_with_details( + cred_ex_record: V20CredExRecord, details: Mapping +) -> Mapping: + """Get credential exchange result with detail records.""" + result = {"cred_ex_record": cred_ex_record.serialize()} + for fmt in V20CredFormat.Format: + ident = fmt.api + detail_record = details.get(ident) + result[ident] = detail_record.serialize() if detail_record else None return result @@ -450,7 +462,8 @@ async def credential_exchange_list(request: web.BaseRequest): results = [] for cxr in cred_ex_records: - result = await _get_result_with_details(profile, cxr) + details = await _get_attached_credentials(profile, cxr) + result = _format_result_with_details(cxr, details) results.append(result) except (StorageError, BaseModelError) as err: @@ -486,8 +499,8 @@ async def credential_exchange_retrieve(request: web.BaseRequest): async with profile.session() as session: cred_ex_record = await V20CredExRecord.retrieve_by_id(session, cred_ex_id) - result = await _get_result_with_details(context.profile, cred_ex_record) - + details = await _get_attached_credentials(profile, cred_ex_record) + result = _format_result_with_details(cred_ex_record, details) except StorageNotFoundError as err: # no such cred ex record: not protocol error, user fat-fingered id raise web.HTTPNotFound(reason=err.roll_up) from err @@ -1321,8 +1334,9 @@ async def credential_exchange_issue(request: web.BaseRequest): connection_id = cred_ex_record.connection_id conn_record = await ConnRecord.retrieve_by_id(session, connection_id) - if not conn_record.is_ready: - raise web.HTTPForbidden(reason=f"Connection {connection_id} not ready") + + if not conn_record.is_ready: + raise web.HTTPForbidden(reason=f"Connection {connection_id} not ready") cred_manager = V20CredManager(profile) (cred_ex_record, cred_issue_message) = await cred_manager.issue_credential( @@ -1330,7 +1344,8 @@ async def credential_exchange_issue(request: web.BaseRequest): comment=comment, ) - result = await _get_result_with_details(profile, cred_ex_record) + details = await _get_attached_credentials(profile, cred_ex_record) + result = _format_result_with_details(cred_ex_record, details) except ( BaseModelError, @@ -1433,14 +1448,16 @@ async def credential_exchange_store(request: web.BaseRequest): ) try: + # fetch these early, before potential removal + details = await _get_attached_credentials(profile, cred_ex_record) + + # the record may be auto-removed here ( cred_ex_record, cred_ack_message, ) = await cred_manager.send_cred_ack(cred_ex_record) - # We first need to retrieve the the cred_ex_record with detail record - # as the record may be auto removed - result = await _get_result_with_details(profile, cred_ex_record) + result = _format_result_with_details(cred_ex_record, details) except ( BaseModelError, From ac3f21c681e7359bbc64322e5805928f8249dbb0 Mon Sep 17 00:00:00 2001 From: Andrew Whitehead Date: Wed, 16 Mar 2022 08:49:07 -0700 Subject: [PATCH 2/3] set revoc_reg_id, revocation_id on issued credential exchange Signed-off-by: Andrew Whitehead --- .../protocols/issue_credential/v1_0/manager.py | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/aries_cloudagent/protocols/issue_credential/v1_0/manager.py b/aries_cloudagent/protocols/issue_credential/v1_0/manager.py index 7934d1725d..74dd313a8d 100644 --- a/aries_cloudagent/protocols/issue_credential/v1_0/manager.py +++ b/aries_cloudagent/protocols/issue_credential/v1_0/manager.py @@ -566,7 +566,9 @@ async def issue_credential( schema_id = cred_ex_record.schema_id rev_reg = None + rev_reg_id = None credential_ser = None + cred_rev_id = None if cred_ex_record.credential: LOGGER.warning( @@ -575,6 +577,8 @@ async def issue_credential( cred_ex_record.credential_exchange_id, ) credential_ser = cred_ex_record._credential.ser + cred_rev_id = cred_ex_record.revocation_id + rev_reg_id = cred_ex_record.revoc_reg_id else: cred_offer_ser = cred_ex_record._credential_offer.ser cred_req_ser = cred_ex_record._credential_request.ser @@ -599,8 +603,7 @@ async def issue_credential( cred_ex_record.credential_definition_id ) rev_reg = await active_rev_reg_rec.get_registry() - cred_ex_record.revoc_reg_id = active_rev_reg_rec.revoc_reg_id - + rev_reg_id = rev_reg.registry_id tails_path = rev_reg.tails_local_path await rev_reg.get_or_fetch_local_tails_path() @@ -661,22 +664,19 @@ async def issue_credential( ) issuer = self._profile.inject(IndyIssuer) try: - ( - credential_json, - cred_ex_record.revocation_id, - ) = await issuer.create_credential( + (credential_json, cred_rev_id) = await issuer.create_credential( schema, cred_offer_ser, cred_req_ser, credential_values, cred_ex_record.credential_exchange_id, - cred_ex_record.revoc_reg_id, + rev_reg_id, tails_path, ) credential_ser = json.loads(credential_json) # If the rev reg is now full - if rev_reg and rev_reg.max_creds == int(cred_ex_record.revocation_id): + if rev_reg and rev_reg.max_creds == int(cred_rev_id): async with self._profile.session() as session: await active_rev_reg_rec.set_state( session, @@ -728,6 +728,8 @@ async def issue_credential( ) cred_ex_record.state = V10CredentialExchange.STATE_ISSUED cred_ex_record.credential = credential_ser + cred_ex_record.revoc_reg_id = rev_reg_id + cred_ex_record.revocation_id = cred_rev_id await cred_ex_record.save(txn, reason="issue credential") await txn.commit() From 86f92dde82c3bd2bc95abc36cdc872ca66b5b871 Mon Sep 17 00:00:00 2001 From: Andrew Whitehead Date: Wed, 16 Mar 2022 12:29:25 -0700 Subject: [PATCH 3/3] tolerate multiple calls to create_request, issue_credential Signed-off-by: Andrew Whitehead --- .../issue_credential/v1_0/manager.py | 114 +++++++++--------- .../v1_0/tests/test_manager.py | 16 +-- 2 files changed, 60 insertions(+), 70 deletions(-) diff --git a/aries_cloudagent/protocols/issue_credential/v1_0/manager.py b/aries_cloudagent/protocols/issue_credential/v1_0/manager.py index 74dd313a8d..b4b9291be4 100644 --- a/aries_cloudagent/protocols/issue_credential/v1_0/manager.py +++ b/aries_cloudagent/protocols/issue_credential/v1_0/manager.py @@ -400,13 +400,6 @@ async def create_request( A tuple (credential exchange record, credential request message) """ - if cred_ex_record.state != V10CredentialExchange.STATE_OFFER_RECEIVED: - raise CredentialManagerError( - f"Credential exchange {cred_ex_record.credential_exchange_id} " - f"in {cred_ex_record.state} state " - f"(must be {V10CredentialExchange.STATE_OFFER_RECEIVED})" - ) - credential_definition_id = cred_ex_record.credential_definition_id cred_offer_ser = cred_ex_record._credential_offer.ser cred_req_ser = None @@ -436,14 +429,14 @@ async def _create(): "metadata": json.loads(metadata_json), } - if cred_ex_record.credential_request: + if cred_ex_record.state == V10CredentialExchange.STATE_REQUEST_SENT: LOGGER.warning( "create_request called multiple times for v1.0 credential exchange: %s", cred_ex_record.credential_exchange_id, ) cred_req_ser = cred_ex_record._credential_request.ser cred_req_meta = cred_ex_record.credential_request_metadata - else: + elif cred_ex_record.state == V10CredentialExchange.STATE_OFFER_RECEIVED: nonce = cred_offer_ser["nonce"] cache_key = ( f"credential_request::{credential_definition_id}::{holder_did}::{nonce}" @@ -462,6 +455,29 @@ async def _create(): cred_req_ser = cred_req_result["request"] cred_req_meta = cred_req_result["metadata"] + async with self._profile.transaction() as txn: + cred_ex_record = await V10CredentialExchange.retrieve_by_id( + txn, cred_ex_record.credential_exchange_id, for_update=True + ) + if cred_ex_record.state != V10CredentialExchange.STATE_OFFER_RECEIVED: + raise CredentialManagerError( + f"Credential exchange {cred_ex_record.credential_exchange_id} " + f"in {cred_ex_record.state} state " + f"(must be {V10CredentialExchange.STATE_OFFER_RECEIVED})" + ) + + cred_ex_record.credential_request = cred_req_ser + cred_ex_record.credential_request_metadata = cred_req_meta + cred_ex_record.state = V10CredentialExchange.STATE_REQUEST_SENT + await cred_ex_record.save(txn, reason="create credential request") + await txn.commit() + else: + raise CredentialManagerError( + f"Credential exchange {cred_ex_record.credential_exchange_id} " + f"in {cred_ex_record.state} state " + f"(must be {V10CredentialExchange.STATE_OFFER_RECEIVED})" + ) + credential_request_message = CredentialRequest( requests_attach=[CredentialRequest.wrap_indy_cred_req(cred_req_ser)] ) @@ -470,23 +486,6 @@ async def _create(): self._profile.settings, cred_ex_record.trace ) - async with self._profile.transaction() as txn: - cred_ex_record = await V10CredentialExchange.retrieve_by_id( - txn, cred_ex_record.credential_exchange_id, for_update=True - ) - if cred_ex_record.state != V10CredentialExchange.STATE_OFFER_RECEIVED: - raise CredentialManagerError( - f"Credential exchange {cred_ex_record.credential_exchange_id} " - f"in {cred_ex_record.state} state " - f"(must be {V10CredentialExchange.STATE_OFFER_RECEIVED})" - ) - - cred_ex_record.credential_request = cred_req_ser - cred_ex_record.credential_request_metadata = cred_req_meta - cred_ex_record.state = V10CredentialExchange.STATE_REQUEST_SENT - await cred_ex_record.save(txn, reason="create credential request") - await txn.commit() - return (cred_ex_record, credential_request_message) async def receive_request(self, message: CredentialRequest, connection_id: str): @@ -557,31 +556,22 @@ async def issue_credential( """ - if cred_ex_record.state != V10CredentialExchange.STATE_REQUEST_RECEIVED: - raise CredentialManagerError( - f"Credential exchange {cred_ex_record.credential_exchange_id} " - f"in {cred_ex_record.state} state " - f"(must be {V10CredentialExchange.STATE_REQUEST_RECEIVED})" - ) - - schema_id = cred_ex_record.schema_id - rev_reg = None - rev_reg_id = None credential_ser = None - cred_rev_id = None if cred_ex_record.credential: LOGGER.warning( - "issue_credential called multiple times for " - + "credential exchange record %s - abstaining", + "issue_credential called multiple times for v1.0 credential exchange %s", cred_ex_record.credential_exchange_id, ) credential_ser = cred_ex_record._credential.ser - cred_rev_id = cred_ex_record.revocation_id - rev_reg_id = cred_ex_record.revoc_reg_id - else: + + elif cred_ex_record.state == V10CredentialExchange.STATE_REQUEST_RECEIVED: + rev_reg = None + rev_reg_id = None + cred_rev_id = None cred_offer_ser = cred_ex_record._credential_offer.ser cred_req_ser = cred_ex_record._credential_request.ser + schema_id = cred_ex_record.schema_id ledger_exec_inst = self._profile.inject(IndyLedgerRequestsExecutor) ledger = ( await ledger_exec_inst.get_ledger_for_identifier( @@ -716,28 +706,32 @@ async def issue_credential( raise - async with self._profile.transaction() as txn: - cred_ex_record = await V10CredentialExchange.retrieve_by_id( - txn, cred_ex_record.credential_exchange_id, for_update=True - ) - if cred_ex_record.state != V10CredentialExchange.STATE_REQUEST_RECEIVED: - raise CredentialManagerError( - f"Credential exchange {cred_ex_record.credential_exchange_id} " - f"in {cred_ex_record.state} state " - f"(must be {V10CredentialExchange.STATE_REQUEST_RECEIVED})" + async with self._profile.transaction() as txn: + cred_ex_record = await V10CredentialExchange.retrieve_by_id( + txn, cred_ex_record.credential_exchange_id, for_update=True ) - cred_ex_record.state = V10CredentialExchange.STATE_ISSUED - cred_ex_record.credential = credential_ser - cred_ex_record.revoc_reg_id = rev_reg_id - cred_ex_record.revocation_id = cred_rev_id - await cred_ex_record.save(txn, reason="issue credential") - await txn.commit() + if cred_ex_record.state != V10CredentialExchange.STATE_REQUEST_RECEIVED: + raise CredentialManagerError( + f"Credential exchange {cred_ex_record.credential_exchange_id} " + f"in {cred_ex_record.state} state " + f"(must be {V10CredentialExchange.STATE_REQUEST_RECEIVED})" + ) + cred_ex_record.state = V10CredentialExchange.STATE_ISSUED + cred_ex_record.credential = credential_ser + cred_ex_record.revoc_reg_id = rev_reg_id + cred_ex_record.revocation_id = cred_rev_id + await cred_ex_record.save(txn, reason="issue credential") + await txn.commit() + else: + raise CredentialManagerError( + f"Credential exchange {cred_ex_record.credential_exchange_id} " + f"in {cred_ex_record.state} state " + f"(must be {V10CredentialExchange.STATE_REQUEST_RECEIVED})" + ) credential_message = CredentialIssue( comment=comment, - credentials_attach=[ - CredentialIssue.wrap_indy_credential(cred_ex_record._credential.ser) - ], + credentials_attach=[CredentialIssue.wrap_indy_credential(credential_ser)], ) credential_message._thread = {"thid": cred_ex_record.thread_id} credential_message.assign_trace_decorator( diff --git a/aries_cloudagent/protocols/issue_credential/v1_0/tests/test_manager.py b/aries_cloudagent/protocols/issue_credential/v1_0/tests/test_manager.py index 6e3df6387e..24287a6523 100644 --- a/aries_cloudagent/protocols/issue_credential/v1_0/tests/test_manager.py +++ b/aries_cloudagent/protocols/issue_credential/v1_0/tests/test_manager.py @@ -661,12 +661,11 @@ async def test_create_request(self): await self.manager.create_request(stored_exchange, holder_did) # cover case with existing cred req - stored_exchange.state = V10CredentialExchange.STATE_OFFER_RECEIVED - stored_exchange.credential_request = INDY_CRED_REQ ( - _ret_existing_exchange, + ret_existing_exchange, ret_existing_request, - ) = await self.manager.create_request(stored_exchange, holder_did) + ) = await self.manager.create_request(ret_exchange, holder_did) + assert ret_existing_exchange == ret_exchange assert ret_existing_request._thread_id == thread_id async def test_create_request_no_cache(self): @@ -895,9 +894,9 @@ async def test_issue_credential(self): ) as save_ex: revoc.return_value.get_active_issuer_rev_reg_record = async_mock.CoroutineMock( return_value=async_mock.MagicMock( # active_rev_reg_rec - revoc_reg_id=REV_REG_ID, get_registry=async_mock.CoroutineMock( return_value=async_mock.MagicMock( # rev_reg + registry_id=REV_REG_ID, tails_local_path="dummy-path", get_or_fetch_local_tails_path=async_mock.CoroutineMock(), ) @@ -926,14 +925,11 @@ async def test_issue_credential(self): assert ret_cred_issue._thread_id == thread_id # cover case with existing cred - stored_exchange.credential = cred - stored_exchange.state = V10CredentialExchange.STATE_REQUEST_RECEIVED - await stored_exchange.save(self.session) ( ret_existing_exchange, ret_existing_cred, ) = await self.manager.issue_credential( - stored_exchange, comment=comment, retries=0 + ret_exchange, comment=comment, retries=0 ) assert ret_existing_exchange == ret_exchange assert ret_existing_cred._thread_id == thread_id @@ -1061,9 +1057,9 @@ async def test_issue_credential_fills_rr(self): get_active_issuer_rev_reg_record=( async_mock.CoroutineMock( return_value=async_mock.MagicMock( # active_rev_reg_rec - revoc_reg_id=REV_REG_ID, get_registry=async_mock.CoroutineMock( return_value=async_mock.MagicMock( # rev_reg + registry_id=REV_REG_ID, tails_local_path="dummy-path", max_creds=1000, get_or_fetch_local_tails_path=(