feat(core): support image url in invitations

[TimoGlastra]

non-standard approach for showing images with connections. Almost all wallets and implementations support this (ACA-Py, AF.NET, Connect.Me, Lissi, Trinsic, etc...)

Signed-off-by: Timo Glastra [email protected]

[TimoGlastra]

@janrtvld ^^

[JamesKEbert]

I am somewhat concerned about moving our support away from the Aries RFCs standards, including #456.

If this image url is something that all agents are supporting, perhaps it's pertinent to update the protocol? Also, is this something present in OOB? If not, that'd definitely be a place to consider too.

[TimoGlastra]

I understand the concern. There was a brief discussion about this at the Aries WG. It's not in the protocol I think because of privacy reasons (so that should maybe also be a reason to not include it in the mobile wallet)

Re #456, d_m is the oob approach but it seems Trinsic uses this for their invitations. I'd vote for having interop at the moment and keep good track of the non-standard features we introduce. But maybe we should have a discussion about it at the WG call.

E.g. if you create an invitation in trinsic studio it shows a shortened url which resolved to: https://trinsic.studio/link/?d_m=eyJsYWJlbCxxxxx

[JamesKEbert]

Thanks for the feedback!

It's not in the protocol I think because of privacy reasons (so that should maybe also be a reason to not include it in the mobile wallet)

I must have missed that discussion, but I'd be interested in exploring what we can do to either include in the protocol, or identify a better methodology for doing it given that everyone is doing it but not having it documented in the protocol.

Re #456, d_m is the oob approach but it seems Trinsic uses this for their invitations.

Perhaps I'm missing something but shouldn't the query be for OOB: ?oob=? I think we need to add URL shortening checking, but after Trinsic's is resolved I don't think it's compliant with OOB or connections v1?

I'd vote for having interop at the moment and keep good track of the non-standard features we introduce.

I am okay for including for now, but I think it'd be wise to add a MD file or list in the repo of deviations made, IMO. Happy to talk about it more in the WG call.

[TimoGlastra]

Perhaps I'm missing something but shouldn't the query be for OOB: ?oob=? I think we need to add URL shortening checking, but after Trinsic's is resolved I don't think it's compliant with OOB or connections v1?

No you're right, I think it is often used for connnectionless exchanges. So if I want to issue you a credential without connection, pre-oob d_m (from didcomm_message) is used. Just found a reference to it in RFC 0268: https://github.com/hyperledger/aries-rfcs/blob/08653f21a489bf4717b54e4d7fd2d0bdfe6b4d1a/concepts/0268-unified-didcomm-agent-deeplinking/README.md#message-requirements

[TimoGlastra]

I must have missed that discussion, but I'd be interested in exploring what we can do to either include in the protocol, or identify a better methodology for doing it given that everyone is doing it but not having it documented in the protocol.

Let's see if we can discuss it at the next Aries WG

[JamesKEbert]

So if I want to issue you a credential without connection, pre-oob d_m (from didcomm_message) is used. Just found a reference to it in RFC 0268

Ah yeah--maybe followup dumb question, but given that it's in a state of proposed this isn't something broadly accepted yet and would warrant consideration before adding to the framework? But it does make me a little bit more comfortable with the previous PR #456 though. Hopefully we'll be able to better align around OOB going forward.

So yeah, I'm not opposed here then, but I think a followup discussion in the Aries WG call would be good. Thanks for the discussion!

[TelegramSam]

Will discuss in Aries call.

The primary problem with this is the ease of impersonation and phishing. I believe the desire to include more information in an invitation is the result of a bad mental model (likely contributed to by me) that a user must approve the creation of a connection and exchange of verified information.

[codecov-commenter]

Codecov Report

Merging #463 (4e68363) into main (336baa0) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #463      +/-   ##
==========================================
- Coverage   86.42%   86.42%   -0.01%     
==========================================
  Files         266      266              
  Lines        5733     5739       +6     
  Branches      923      923              
==========================================
+ Hits         4955     4960       +5     
- Misses        777      778       +1     
  Partials        1        1              

Impacted Files	Coverage Δ
packages/core/src/types.ts	100.00% <ø> (ø)
packages/core/src/agent/AgentConfig.ts	94.54% <100.00%> (+0.20%)	⬆️
...onnections/messages/ConnectionInvitationMessage.ts	98.07% <100.00%> (ø)
...s/connections/messages/ConnectionRequestMessage.ts	100.00% <100.00%> (ø)
...modules/connections/repository/ConnectionRecord.ts	98.41% <100.00%> (+0.02%)	⬆️
.../modules/connections/services/ConnectionService.ts	95.78% <100.00%> (+0.02%)	⬆️
packages/core/src/wallet/IndyWallet.ts	68.42% <0.00%> (-0.76%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 336baa0...4e68363. Read the comment docs.

[JamesKEbert]

The code looks great here @TimoGlastra! Happy to approve and get merged as soon as we add some short text to the README detailing our divergence from the RFC as discussed on the AFJ Call on 10-8-21.

[TimoGlastra]

@JamesKEbert I've added a brief explanation of the divergence from the Aries RFCs. Could you take a look?

[JamesKEbert]

Yes! Your docs additions here are wonderful, thanks @TimoGlastra!