bcgov / vc-authn-oidc

Apache License 2.0
143 stars 74 forks source link

OOB format problem #545

Closed loneil closed 3 months ago

loneil commented 3 months ago

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 commented 3 months ago

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

loneil commented 3 months ago

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:

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.

esune commented 3 months ago

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 commented 3 months ago

https://github.com/bcgov/vc-authn-oidc/pull/556