openwallet-foundation / credo-ts

Typescript framework for building decentralized identity and verifiable credential solutions
https://credo.js.org
Apache License 2.0
276 stars 202 forks source link

Cannot process RequestCredentialMessage for the OOB attached credential-offer #1129

Closed conanoc closed 1 year ago

conanoc commented 2 years ago

Test scenario:

  1. Faber creates an credential-offer-message and creates an oob with it.
    const issueCredentialConfig = {
    label: 'Faber College',
    handshake: true,
    }
    const { message } = await faber.credentials.createOffer({...})
    const { outOfBandInvitation } = await faber.oob.createInvitation({
    ...issueCredentialConfig,
    messages: [message],
    })
  2. Alice receives the invitation. 2.1 Alice makes a connection with Faber 2.2 After the connection, Alice sends request-credential-message to Faber
  3. Faber fails to process the request-credential-message because:
    • Faber tries to find credentialRecord with connection id here
    • But, Faber created the credentialRecord without connection id here in Step1. So, Faber cannot find the credentialRecord and throws an error.

Solution: In processRequest(), find the credentialRecord without connection id like this:

    const credentialRecord = await this.getByThreadAndConnectionId(requestMessage.threadId, null)
conanoc commented 2 years ago

This is the error message from Faber.

ERROR: Error handling message with type https://didcomm.org/issue-credential/1.0/request-credential {
  "message": {
    "@type": "https://didcomm.org/issue-credential/1.0/request-credential",
    "~transport": {
      "return_route": "all"
    },
    "requests~attach": [
      {
        "@id": "libindy-cred-request-0",
        "data": {
          "base64": "eyJwcm92ZXJfZGlkIjoiM2d3eTJHQ1dCR3pwM3o4SzZBdzh4RCIsImNyZWRfZGVmX2lkIjoiN0t1RFRwUWgzR0o3R3A2a0VycFd2TTozOkNMOjU5NjkyMTpkZWZhdWx0IiwiYmxpbmRlZF9tcyI6eyJ1IjoiNTU0NTUxMzM1NTY4MzU1MDI2Mjg2MjM4NDY0OTcyNTM4MzE4Nzk0NzI2MTM1Nzg1MjI5NTQwMDExMDAyOTAxNjI1MjYzNzI4MDA5NDYzNDA2MDUxOTIxODUzMzgzMjk4NTAyNzEyMTcwOTU0NTUwMjc1OTI0NTExNTQ4NDI5OTUxNjI4NDM1NzU0NDI2OTEwODY0MjEyMjQwODkyNTk5MDIzNTI3NjUyOTAwNTIwNTY4NjY2MjMzMDEyOTU2MTY3NDk0NzE3MTI4MDU3NTAzMTMwNjk1NDE4MzIyOTkwOTQwMjg0OTg2OTEwNzY1OTkyNzM4OTAyNTY2Mzc4NjY0MjY0MjYwODAwMDIxNjgyOTM5NzQyNjEyMTc1NjA4MzE1Nzg4NDY4MDI1MjEwMDU0ODczMDc2MzI4MDg5Mzk3NDc4MDQyNzU0NjY0MDUxNDI1OTM5MzIzNzE4OTA4ODA5MzY5MzY2MDU3MzUyNjk1MTMyMDc2MjM3OTE5MTUxMjA0MTkyNjI5MTA4OTk2NTQ1NjYyNTYzMTc1MDQ3MzMzMjAxNjI1MDY2NTQ4MDY0MzQ2MDY2Mzk5MjE0NTMyNTcwNDA5NzMxMDM2NDQyMjExMzE1NzU5NjgxMzA0MDE5NzAyMzAzOTI5MzQxNDY3MDUwNTk0NTYxOTY5NzIyMDk5Mzg5NDcxNTc5ODc0NjUxNjY0NDUxNjAzOTA4MDMxMDYyNDk3NzU2Nzc2MDE3NTIxMDY4NDIwNzAyNTAwNzM4OTk3MjA1NjM2MjI4OTg0OTI5MTQ2NTU0MzkyMDk3MDAzMzQzNDc3NjY4OTMiLCJ1ciI6bnVsbCwiaGlkZGVuX2F0dHJpYnV0ZXMiOlsibWFzdGVyX3NlY3JldCJdLCJjb21taXR0ZWRfYXR0cmlidXRlcyI6e319LCJibGluZGVkX21zX2NvcnJlY3RuZXNzX3Byb29mIjp7ImMiOiI0MDQ5ODE0NzEzNzI1NzY0MjkwNzU0MzUxNDcyNjQyMTE1ODg2MzQwMjg5NjAzODY3ODY3MzU0NzYyNDQ3ODAyMzQxNDI1ODAyMzM5OSIsInZfZGFzaF9jYXAiOiI4MTgxMDk0NDQ0MTU4MTM3OTE4MDk0MzI4MTUyMjg0NDQxNDgyMDg1MzI3NTQ2NDQwOTU4MzQxNTc3MDk0MzIzMjcxODcxNjMwMjI0NzgzMDE1NTAzMTI4MjkyMTQ0MjA1OTY5NjAzMzE5MDY0NTU3ODg3MzQ5NzkxODIwNjg2NzI3NjEwNzI3NTM5MDQ4NDU0MzQxMDEzNzQ4NzI2OTc4NDk5NjAwMTA1MTIyNTMzMDk5NzY1MjQwNjE2MjcxNjYzMzY5MzUyNjU2NDQyNzgzMDQ2MTI1OTAwNjg1NzQ5MjQ1MjI5NzY3Njk2MDM2NzM5ODcyNjY0MjkzMzU3NDc0ODg2NjE5NDA5Nzc5MzAxNzY4OTU0OTAwODIyNjA1MDk3NTc4Mjk4ODMyNjY0ODkwNjM0ODE3Nzc0MTM3MzA5OTAyOTQxMzg5MDU1NzA4MTE1NjY0ODIzODk4ODkwODA5OTM0MzEyMzY0NDU4NTg3NTAyNTIwNzQ2MTc2Nzk4NDE4NzIyNDUzOTQ1NTcxNjI5Mzc2MTI3MzI3MjIzNTUxMTI2NzEwMDE2NDMzNDA4MTUxMjY3NjgxNzg2MjA0ODAxOTkwNjE1Nzg3NDA2Nzc1NTQ3OTAyMjY1NDYzMzU3MTk3NzI4NzE4Mjg4Njk2MDAzNzg4MjE3MzY2NDQ3ODkyODU1MTIyNzE1MDEzMjU4NTQ2NTA4MTE3NzY2MTkwMTIxMjIzOTc4NjkwMzQ5Njc5MzIwMjI0NTQwNDYwNjgzNjg2NzcyMjIxMjE4MTA1MTc4MjMzMDM0NTE0MTA3MTgxMDkyNTU4NDExMzkwODQ5ODQ0MTAwMjc5NjI1NDIzMzU5NTI4NzMwNTc1NjU5MTI5NDgwNDE3MTM3NzY0MzEzMTI0ODQzNjAwOTQwODUwODczNTM0NjExNDU4OTcyMzgyNjI3NTk1NTgxMzI1NjIiLCJtX2NhcHMiOnsibWFzdGVyX3NlY3JldCI6IjIwNTgwNzEzMTAxMTk3NzEyNDgxMTY3NjI5NDk2NTI1OTYzOTMxMDUyNjk1OTA4MDY3NTUwMTQyNzE5NjQ4NDg4MDc3NDUyMTk0NDg3OTk2MjEwMTAwNTkzNjQ4NzAwNDE3NTc3OTAzMTMxODg5NjQ1ODc4Mzk0NTUzMjkxMTMyOTU0Njk3OTU5NjA3MTUwMDIzMTA4MjY1NjY0MDgxNDk1NTQyODI0MDQ2NjA5MzA1NjYwIn0sInJfY2FwcyI6e319LCJub25jZSI6IjU0ODQwOTk1NDkxNTQ3NjQzNjE4MjIyMSJ9"
        },
        "mime-type": "application/json"
      }
    ],
    "~thread": {
      "thid": "1c00dc55-37ad-4b4e-83a1-2ca022eebba7"
    },
    "@id": "235E523F-E4D6-4E8D-A884-CAE4DDAD48D7"
  },
  "error": {
    "name": "RecordNotFoundError",
    "message": "CredentialRecord: No record found for given query '{\"connectionId\":\"6ba54e40-405a-4276-a743-d72206e337fb\",\"threadId\":\"1c00dc55-37ad-4b4e-83a1-2ca022eebba7\"}'",
    "stack": "RecordNotFoundError: CredentialRecord: No record found for given query '{\"connectionId\":\"6ba54e40-405a-4276-a743-d72206e337fb\",\"threadId\":\"1c00dc55-37ad-4b4e-83a1-2ca022eebba7\"}'\n    at CredentialRepository.getSingleByQuery (/Users/conanoc/work/contrib/aries-framework-swift/AriesFramework/AriesFrameworkTests/javascript/node_modules/@aries-framework/core/src/storage/Repository.ts:141:13)\n    at async V1CredentialService.processRequest (/Users/conanoc/work/contrib/aries-framework-swift/AriesFramework/AriesFrameworkTests/javascript/node_modules/@aries-framework/core/src/modules/credentials/protocol/v1/V1CredentialService.ts:691:30)\n    at async V1RequestCredentialHandler.handle (/Users/conanoc/work/contrib/aries-framework-swift/AriesFramework/AriesFrameworkTests/javascript/node_modules/@aries-framework/core/src/modules/credentials/protocol/v1/handlers/V1RequestCredentialHandler.ts:28:30)\n    at async Dispatcher.dispatch (/Users/conanoc/work/contrib/aries-framework-swift/AriesFramework/AriesFrameworkTests/javascript/node_modules/@aries-framework/core/src/agent/Dispatcher.ts:47:25)\n    at async MessageReceiver.receiveEncryptedMessage (/Users/conanoc/work/contrib/aries-framework-swift/AriesFramework/AriesFrameworkTests/javascript/node_modules/@aries-framework/core/src/agent/MessageReceiver.ts:128:5)\n    at async MessageReceiver.receiveMessage (/Users/conanoc/work/contrib/aries-framework-swift/AriesFramework/AriesFrameworkTests/javascript/node_modules/@aries-framework/core/src/agent/MessageReceiver.ts:69:7)\n    at async Agent.receiveMessage (/Users/conanoc/work/contrib/aries-framework-swift/AriesFramework/AriesFrameworkTests/javascript/node_modules/@aries-framework/core/src/agent/Agent.ts:273:12)\n    at async /Users/conanoc/work/contrib/aries-framework-swift/AriesFramework/AriesFrameworkTests/javascript/node_modules/@aries-framework/node/src/transport/HttpInboundTransport.ts:44:9"
  },
  "senderKey": "z6MkfvFro25uLLuDiWoZRzUWtdJGQaf9Lu46Yjvjddn291VH",
  "recipientKey": "z6MkesZcnGL9EL2KqJPfmettheTgh695HqVNXF4n6uUr1smB",
  "connectionId": "6ba54e40-405a-4276-a743-d72206e337fb"
}
ERROR: Error processing inbound message: CredentialRecord: No record found for given query '{"connectionId":"6ba54e40-405a-4276-a743-d72206e337fb","threadId":"1c00dc55-37ad-4b4e-83a1-2ca022eebba7"}' {
  "name": "RecordNotFoundError",
  "message": "CredentialRecord: No record found for given query '{\"connectionId\":\"6ba54e40-405a-4276-a743-d72206e337fb\",\"threadId\":\"1c00dc55-37ad-4b4e-83a1-2ca022eebba7\"}'",
  "stack": "RecordNotFoundError: CredentialRecord: No record found for given query '{\"connectionId\":\"6ba54e40-405a-4276-a743-d72206e337fb\",\"threadId\":\"1c00dc55-37ad-4b4e-83a1-2ca022eebba7\"}'\n    at CredentialRepository.getSingleByQuery (/Users/conanoc/work/contrib/aries-framework-swift/AriesFramework/AriesFrameworkTests/javascript/node_modules/@aries-framework/core/src/storage/Repository.ts:141:13)\n    at async V1CredentialService.processRequest (/Users/conanoc/work/contrib/aries-framework-swift/AriesFramework/AriesFrameworkTests/javascript/node_modules/@aries-framework/core/src/modules/credentials/protocol/v1/V1CredentialService.ts:691:30)\n    at async V1RequestCredentialHandler.handle (/Users/conanoc/work/contrib/aries-framework-swift/AriesFramework/AriesFrameworkTests/javascript/node_modules/@aries-framework/core/src/modules/credentials/protocol/v1/handlers/V1RequestCredentialHandler.ts:28:30)\n    at async Dispatcher.dispatch (/Users/conanoc/work/contrib/aries-framework-swift/AriesFramework/AriesFrameworkTests/javascript/node_modules/@aries-framework/core/src/agent/Dispatcher.ts:47:25)\n    at async MessageReceiver.receiveEncryptedMessage (/Users/conanoc/work/contrib/aries-framework-swift/AriesFramework/AriesFrameworkTests/javascript/node_modules/@aries-framework/core/src/agent/MessageReceiver.ts:128:5)\n    at async MessageReceiver.receiveMessage (/Users/conanoc/work/contrib/aries-framework-swift/AriesFramework/AriesFrameworkTests/javascript/node_modules/@aries-framework/core/src/agent/MessageReceiver.ts:69:7)\n    at async Agent.receiveMessage (/Users/conanoc/work/contrib/aries-framework-swift/AriesFramework/AriesFrameworkTests/javascript/node_modules/@aries-framework/core/src/agent/Agent.ts:273:12)\n    at async /Users/conanoc/work/contrib/aries-framework-swift/AriesFramework/AriesFrameworkTests/javascript/node_modules/@aries-framework/node/src/transport/HttpInboundTransport.ts:44:9"
}
TimoGlastra commented 2 years ago

Yep seems like a bug. If we query without the connectionId we should then afterwards make sense there is no connectionId OR the connectionId matches (to prevent hijacking a credential exchange based on threadId).

Even beter would probalbly be to also look at the outOfBand record that is present in the inbound message context and see if the thread id matches one of the message in the oob invitation.

Thanks for reporting!

dinbtechit commented 1 year ago

@TimoGlastra - Curious do we have an ETA for this bug? This is preventing us from accepting OOB credentials in our bifold based wallet (NS Wallet). Or is there any workaround that we can do in the meantime?

Thanks in advance!

TimoGlastra commented 1 year ago

I don't think our team will have time soon to work on this issue.

I'm happy to give some pointers if you want to pick it up!

dinbtechit commented 1 year ago

@TimoGlastra Absolutely! We have some bandwidth to look into this. And yes, some pointers to get started would be very helpful.

Based on this comment https://github.com/hyperledger/aries-cloudagent-python/issues/2069#issuecomment-1582480274 it appeared like the AJF was missing to include pthid in ~thread but we are not sure where to go from there.

conanoc commented 1 year ago

Let me add some comments about this issue. There are two related issues here. One occurs between bifold(holder) and AFJ(issuer), and another occurs between bifold(holder) and ACA-Py(issuer). bifold<->AFJ issue could be fixed simply by finding the credentialRecord only by thid as I mentioned above. This fix is on the issuer side of AFJ.

bifold<->ACA-Py issue is related with pthid because ACA-Py lookup pthid to find related message records for an OOB connection. This fix should be on the holder side of AFJ. When you search the setThread() function in V1ProofProtocol.ts, you can find the proof messages containing pthid like this: https://github.com/hyperledger/aries-framework-javascript/blob/v0.4.0/packages/anoncreds/src/protocols/proofs/v1/V1ProofProtocol.ts#L132 But, in the case of V1CredentialProtocol.ts, only thids are set for credential messages like this: https://github.com/hyperledger/aries-framework-javascript/blob/v0.4.0/packages/anoncreds/src/protocols/credentials/v1/V1CredentialProtocol.ts#L330

In the Swift framework, I addressed the second issue by ensuring that the pthid is assigned to all messages exchanged over the OOB connection. https://github.com/hyperledger/aries-framework-swift/pull/19/files#diff-1efb4f8e3f552182ef2c41ef023e6017fdc42f81562a15e0a90157abe91b5108R51

dinbtechit commented 1 year ago

@conanoc I appreciate the detailed explanation. Our issuer side utilizes ACA-PY, while the holder end utilizes AFJ (bifold). So... would it be correct to assume that the necessary adjustments need to be made only on the holder side (AFJ) in my case?

conanoc commented 1 year ago

@dinbtechit Yeah, that's right.

nbAmit commented 1 year ago

@TimoGlastra I can work on this issue, Can you please assign to me and share pointers?

TimoGlastra commented 1 year ago

1558 should fix the issue