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.

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

OOB format problem #545

Closed
loneil opened this issue Jun 12, 2024 · 4 comments · Fixed by #556
Closed

OOB format problem #545

loneil opened this issue Jun 12, 2024 · 4 comments · Fixed by #556
Assignees
Labels
bug Something isn't working

Comments

@loneil
Copy link
Contributor

loneil commented Jun 12, 2024

When using VCAuth-N in OOB mode it fails to scan in BC Wallet. After some debugging in Credo with Bryce we've determined there are a few validation errors in the payload that comes back from the QR Code scan redirect

A sample OOB message (what the wallet gets from the QR code scan redirect response) body is something like this:

{
  "@id": "2700f47e-0430-4f45-b34e-5c4057c6265a",
  "@type": "https://didcomm.org/out-of-band/1.1/invitation",
  "goal_code": "request-proof",
  "label": "vc-authn Out-of-Band present-proof authorization request",
  "requests~attach": [
    {
      "@id": "request-0",
      "mime-type": "application/json",
      "data": {
        "json": {
          "@id": "2700f47e-0430-4f45-b34e-5c4057c6265a",
          "@type": "did:sov:BzCbsNYhMrjHiqZDTUASHg;spec/present-proof/1.0/request-presentation",
          "request_presentations~attach": [
            {
              "@id": "libindy-request-presentation-0",
              "mime-type": "application/json",
              "data": {
                "base64": "eyJub25jZSI6ICIyMDg2Mjk2OTc3ODY1OTA2MDk5MTAwNjMiLCAibmFtZSI6ICJwcm9vZl9yZXF1ZXN0ZWQiLCAidmVyc2lvbiI6ICIwLjAuMSIsICJyZXF1ZXN0ZWRfYXR0cmlidXRlcyI6IHsicmVxX2F0dHJfMCI6IHsibmFtZXMiOiBbImdpdmVuX25hbWVzIiwgImZhbWlseV9uYW1lIiwgImNvdW50cnkiXSwgInJlc3RyaWN0aW9ucyI6IFt7InNjaGVtYV9uYW1lIjogIlBlcnNvbiIsICJpc3N1ZXJfZGlkIjogIkw2QVNqbUREYkRIN3lQTDF0MnlGajkifSwgeyJzY2hlbWFfbmFtZSI6ICJQZXJzb24iLCAiaXNzdWVyX2RpZCI6ICJRRXF1QUhrTTM1dzRYVlQzS3U1eWF0In0sIHsic2NoZW1hX25hbWUiOiAiUGVyc29uIiwgImlzc3Vlcl9kaWQiOiAiTTZkaHVGajVVd2JoV2tTTG12WVNQYyJ9XSwgIm5vbl9yZXZva2VkIjogeyJmcm9tIjogMTcxODA0MTcwMywgInRvIjogMTcxODA0MTcwM319fSwgInJlcXVlc3RlZF9wcmVkaWNhdGVzIjoge319"
              }
            }
          ],
          "comment": null,
          "~service": null
        }
      }
    }
  ],
  "services": [
    {
      "recipient_keys": [
        "2gMSwKkZe1oA8H7jFCHSsGGeCoSH489N6PTc2Qtg8jKJ"
      ],
      "routing_keys": [],
      "service_endpoint": "https://0c25-75-156-98-192.ngrok-free.app",
      "id": "did:vc-authn-oidc:123456789zyxwvutsr#did-communication",
      "type": "did-communication",
      "priority": 0
    }
  ]
}

There are a couple different issues in the service object

image

  1. The keys should be in camelCase (this is an easy alias fix in VCAuth-N model)

  2. The recipient key is not right. It's just putting the wallet verkey in there. I don't know if this is valid in some cases, or an error in the vcauthn code(?) but Credo rejects it with each value in recipientKeys must be a did:key string
    Adding did:key to a prefix then rejects with [Error: No decoder found for multibase prefix '2']

So this needs to be a valid recipient key.

@loneil
Copy link
Contributor Author

loneil commented Jun 12, 2024

Some relevant discussion on Credo Discord: https://discord.com/channels/1022962884864643214/1249796862379294820

@loneil
Copy link
Contributor Author

loneil commented Jun 12, 2024

I think what we should be doing here is actually calling ACA-Py to get an Out Of Band message. This would include the recipient key and proper service object naming. This would also future-proof things better as an ACA-Py OOB should be interoperable with Credo more than creating it manually in VCAuth would be.

The current VCAuthN controller pattern instead:

  • Calls ACA-Py for the Pres Req
  • Calls ACA-Py for wallet DID info
  • Builds an OOB message up in code instead.

I tried getting a OOB message from ACA-Py and hardcoding things (recipient key) into VCAuthN instead and this worked end-to-end with the BC Wallet.

@loneil loneil self-assigned this Jun 13, 2024
@loneil loneil moved this to In Progress in CDT Enterprise Apps Jun 13, 2024
@esune
Copy link
Member

esune commented Jun 14, 2024

Agreed on using the ACA-Py generated message. The controller processing is likely a leftover from conenction-less handling, since there is no endpoint in ACA-Py to generate a conenction-less proof-request with the service endpoint as far as I know.

We can implement the change and plan to phase-out connection-less in favour of always using OOB/did-exchange.

@loneil
Copy link
Contributor Author

loneil commented Jun 18, 2024

#556

@loneil loneil added the bug Something isn't working label Jun 18, 2024
@loneil loneil moved this from In Progress to In Review in CDT Enterprise Apps Jun 18, 2024
@github-project-automation github-project-automation bot moved this from In Review to Complete in CDT Enterprise Apps Jun 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants