fix: allow reconnections to a multi-use invitation.

[TheTreek]

We were not able to connect to a multi-use invitation more than once. Removing this check will allow us to do so.

[genaris]

AFAIK the purpose of multi-use invitations is for an inviter to publish a single invitation that can be received by multiple invitees. This does not mean that an invitee (a.k.a. Receiver) can process the same invitation multiple times. So in that sense I think the check in current code is correct.

Is that the use case you are referring to?

[TimoGlastra]

What's the specific use case here? In general we need the invitation id to be unique so we can use that for querying the records. So even if this may work in this case, it's not designed for it.

I think you could either remove the old connection, or query for a connection using the existing invitation id. Would that work?

[TimoGlastra]

@genaris do you know if it's safe to remove this check?

[TimoGlastra]

Ah you already commented, my GH was outdated

[genaris]

@genaris do you know if it's safe to remove this check?

OutOfBandService has a findByReceivedInvitationId method where it expects to find a single OutOfBandRecord related to a given invitation id. As far as I can see, this is only used at agent initialization when using mediatorInvitationUrl: it's used to find out if we are already connected to the mediator.

I could not find a limitation from RFCs/protocol point of view to process the same invitation more than once, so probably it can be safe to remove this check and refactor findByReceivedInvitationId to findAllByReceivedInvitationId. Also it will be needed a bit of rework in getMediationConnection to accept an array of OutOfBandRecord instead of a single one.

[JamesKEbert]

What's the specific use case here? In general we need the invitation id to be unique so we can use that for querying the records. So even if this may work in this case, it's not designed for it.

The #1 best use case that comes to mind is if an invitation is printed on poster or sign for a physical event or where you provide a single invitation to the end user. Additionally, if that issuer does not support connection reuse then there may need to be multiple connections created for workflows that get repeated multiple times.

Indicio's immediate-term use case requires us to hard-code the invitation with the application at build time, so it can't be dynamic/single-use and the user needs to be able to run through the workflow multiple times.

[NimeshNB]

@TheTreek Did you found any solution for this?
I'm facing similar type of issue,
i.e: if user accidentally delete connection and wanted to reconnect then can use multi-use invitation

[TimoGlastra]

I think with some work we can fix the issue that this PR is trying to solve. But I do think it needs to be a bit more complex. ids for messages are meant to be unique, and thus I would rather say we reuse the same out of band record (becuase it's the same oob invitation). We should check though if the invitation from the oob record is the same as the invitation that is parsed the second time (so we don't have two different invitations that have the same id).

[NimeshNB]

@TimoGlastra we are able to solve this by adding some conditions
if outOfBandRecord does not exist then only it will create new outOfBandRecord else it will use existing one for multi use invite so that id will remain same

if(!outOfBandRecord){
           const recipientKeyFingerprints = [];
           for (const service of outOfBandInvitation.getServices()) {
               // Resolve dids to DIDDocs to retrieve services
               if (typeof service === 'string') {
                   this.logger.debug(`Resolving services for did ${service}.`);
                   const resolvedDidCommServices = await this.didCommDocumentService.resolveServicesFromDid(this.agentContext, service);
                   recipientKeyFingerprints.push(...resolvedDidCommServices
                       .reduce((aggr, { recipientKeys }) => [...aggr, ...recipientKeys], [])
                       .map((key) => key.fingerprint));
               }
               else {
                   recipientKeyFingerprints.push(...service.recipientKeys.map((didKey) => dids_1.DidKey.fromDid(didKey).key.fingerprint));
               }
           }
           outOfBandRecord = new OutOfBandRecord_1.OutOfBandRecord({
               role: OutOfBandRole_1.OutOfBandRole.Receiver,
               state: OutOfBandState_1.OutOfBandState.Initial,
               outOfBandInvitation: outOfBandInvitation,
               autoAcceptConnection,
               tags: { recipientKeyFingerprints },
           });
           await this.outOfBandService.save(this.agentContext, outOfBandRecord);
           this.outOfBandService.emitStateChangedEvent(this.agentContext, outOfBandRecord, null);
       }

[TimoGlastra]

Discussed in AFJ WG call. The change to remove the check is a good one, but there may be some issues as @genaris pointed out. @TheTreek do you have time to make the changes as @genaris suggested? Otherwise @genaris can pick this up