Fixes for credential details in issue-credential webhook responses

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,27 +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
         credential_ser = 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
-        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(
@@ -599,8 +593,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 +654,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,
@@ -716,26 +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
-            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(

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=(

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,16 +1334,18 @@ 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(
             cred_ex_record,
             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,