feat(oob): implicit invitations

[genaris]

Add support for Out-of-Band protocol 'implicit invitations', meaning by that to connect to a public DID given it has a DID Communication service in its DidDocument.

The API consists on the method oob.receiveImplicitInvitation, similar to regular oob.receiveInvitation but it of course receiving a resolvable did instead of a Connection Invitation.

It can use both Connection and DID Exchange protocols and it's possible to create multiple connections from the same public DID. By querying connections.findByInvitationDid(publicDid) it's possible for an application to know if it's already connected to a public entity.

Using AFJ agents it's working, but didn't test it against ACA-Py yet. Hopefully with these changes we can pass some DID Exchange tests in the AATH.

[codecov-commenter]

Codecov Report

Merging #1348 (440914e) into main (18abb18) will increase coverage by 0.11%.
The diff coverage is 96.66%.

@@            Coverage Diff             @@
##             main    #1348      +/-   ##
==========================================
+ Coverage   86.54%   86.66%   +0.11%     
==========================================
  Files         778      778              
  Lines       18432    18473      +41     
  Branches     3143     3161      +18     
==========================================
+ Hits        15952    16009      +57     
+ Misses       2473     2457      -16     
  Partials        7        7              

Impacted Files	Coverage Δ
.../modules/connections/services/ConnectionService.ts	88.23% <ø> (ø)
...core/src/modules/oob/repository/OutOfBandRecord.ts	100.00% <ø> (ø)
.../connections/handlers/DidExchangeRequestHandler.ts	82.50% <88.88%> (+2.50%)	⬆️
packages/core/src/modules/oob/OutOfBandService.ts	98.95% <94.73%> (-1.05%)	⬇️
...ore/src/modules/connections/DidExchangeProtocol.ts	87.14% <100.00%> (+0.31%)	⬆️
...s/connections/handlers/ConnectionRequestHandler.ts	88.57% <100.00%> (+1.90%)	⬆️
...connections/handlers/DidExchangeCompleteHandler.ts	85.71% <100.00%> (+1.09%)	⬆️
packages/core/src/modules/oob/OutOfBandApi.ts	88.44% <100.00%> (+2.00%)	⬆️
packages/core/src/agent/MessageSender.ts	87.37% <0.00%> (+0.97%)	⬆️
...re/src/modules/oob/messages/OutOfBandInvitation.ts	95.06% <0.00%> (+1.23%)	⬆️
... and 4 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[TimoGlastra]

This is really great @genaris!!!

I have some code in a local branch somewhere that has most of the work done for creating invitations using public dids, if we can add that soon as well we fully support public dids!

One thing I'm curious about, when someone connects to our public did, will we always rotate to a new did? Or will it use the keys from the invitation to create a peer did (in that case we should be careful as it'll use the same key but a diferent did). If i remember correctly a key will be created when an invitation is created, and then later a peer did will be created from that key. Not 100% if this is working as intended, but as it's so little code I'm curious whether it jus works (I know creating invitations with public dids required quite some chagnes...)

[TimoGlastra]

With the new import method in the dids module it should now be possible to host a mediator, create a did:web document (creating keys with AFJ, but publishing yourself outside of afj), importing the did document, and then you could use the did:web as a mediatorInvitationUrl.

[genaris]

With the new import method in the dids module it should now be possible to host a mediator, create a did:web document (creating keys with AFJ, but publishing yourself outside of afj), importing the did document, and then you could use the did:web as a mediatorInvitationUrl.

This is exactly one of the main goals I want to achieve with this and the import! Using did:web will simplify a lot the deployments and discovery of public agents.

[genaris]

This is really great @genaris!!!

I have some code in a local branch somewhere that has most of the work done for creating invitations using public dids, if we can add that soon as well we fully support public dids!

Thanks for the fast but quite in-depth review! It was actually meant to be marked as 'draft' but fortunately I put it for review and you gave some good pointers about the approach to take.

One thing I'm curious about, when someone connects to our public did, will we always rotate to a new did? Or will it use the keys from the invitation to create a peer did (in that case we should be careful as it'll use the same key but a diferent did). If i remember correctly a key will be created when an invitation is created, and then later a peer did will be created from that key. Not 100% if this is working as intended, but as it's so little code I'm curious whether it jus works (I know creating invitations with public dids required quite some chagnes...)

I'm not completely sure if I understood your question but the idea is that for both DID Exchange Request and Connection Request a new "reusable" Out of Band record will be created, so it will rotate the keys as it would be managing the requests for a reusable OOB Invitation (in the code, it will call getRouting() and will effectively create a new Ed25519 key pair). Of course all DID Exchange and Connection Responses will be signed by using the same key (the one published in the public DidDoc) before rotation, which could maybe lead to some security issues (replay attacks or things like that) but I guess it's the way it was designed.

[TimoGlastra]

I'm not completely sure if I understood your question but the idea is that for both DID Exchange Request and Connection Request a new "reusable" Out of Band record will be created, so it will rotate the keys as it would be managing the requests for a reusable OOB Invitation (in the code, it will call getRouting() and will effectively create a new Ed25519 key pair). Of course all DID Exchange and Connection Responses will be signed by using the same key (the one published in the public DidDoc) before rotation, which could maybe lead to some security issues (replay attacks or things like that) but I guess it's the way it was designed.

This answers my question. I was mostly curious whether we were always rotating to a new did/key in the response message, but if it works the same as reusable out of band invitations then I understand. 👍

[TimoGlastra]

I'm okay with merging, but left some additional comments/questions

[TimoGlastra]

I'm not 100% on the naming of this method. We've just received a request based on an implicit invitation (the service in the did document is the implicit invitation). It feels like createFromImplicitInvitation would be more suitable? Also not 100% happy with that, but it should denote that we're creating an out of band record based on a request that was sent based on an implicit invitation.

[genaris]

Well, at least I removed that isImplicitInvitation that didn't convince you neither 😆. The naming was somehow borrowed from the OutOfBandApi method called createInvitation that returned an OutOfBandRecord. But you are completely right: createFromImplicitInvitation is a bit more accurate. Could be also createFromConnectionRequest or createFromImplicitInvitationResponse? I still prefer your suggestion.

[TimoGlastra]

Why don't we allow reuseConnection? If I'm already connected I may or may not reuse the connection right?

[genaris]

I set this before moving to the idea of using id = did for all OOBInvitations related to the same did. I think it can be re-added, but it can add some complexity: suppose you have a connection related to a public did and call this method with reuseConnection = false, meaning that it will create a new connection related to the same public did. As a result, we will have two connections related to the same did. If you call this method again with reuseConnection = true, which of those two connections should it reuse?

I think it´s valuable to have this feature enabled in an automatic way (otherwise it's the consumer app who should check if there is an existing connection before calling this method). Maybe we can support it and, if two or more connections are found, we throw an error?

[genaris]

The problem I see here is that reuseConnection parameter is related to the reuse messages as specified in RFC 434 (handshake-reuse and handshake-reuse-accepted). This one uses OOB Invitation pthid as the way of identifying the invitee... but in an implicit invitation, the pthid will be the public did of the inviter 🤔 .

This makes me think that this flow is not possible or intended to work with implicit invitations. Any clarification will be certainly welcome (my brains are as fried as yours nowadays 😛)

[TimoGlastra]

Yes makes sense. I think we may also be able to just return an existing connection based on reuseConnection. if you pass reuseConnection=true it will just look if we already have a connection with this did, but if you pass false it will always create a new connection. With an OOB invitation you MUST use the handshake-reuse. But this approach is perfect for now

[TimoGlastra]

LGTM