search

bcgov / bc-wallet-mobile

BC Wallet to hold Verifiable Credentials
Apache License 2.0
62 stars 49 forks source link

Connection amalgamation (reuse) and management #66

Closed cvarjao closed 1 day ago

cvarjao commented 2 years ago

Problem: Every time a user scans a connection invitation (QR code) a new connection is added which leads to a ever increasing list of connections. This is an issue with AIP 1.0 only.

To resolve this, connections need to be managed appropriately by the wallet back-end and the UX.

Options to resolve this:

  1. (Recommended for v1.0): Number connections automatically, but have them all visible to the user. e.g. "BC Gov issuer", "BC Gov issuer #2", "BC Gov issuer #3", etc.

  2. Amalgamate connections at the UX level but keep them separate in the back-end. So the user sees one connection to "BC Gov issuer", even if the back-end manages 4 separate connections.

  3. When a new connection is made but the wallet app determines there's already an existing connection to that issuer, the wallet app makes the new connection temporary, and automatically closes it when the issuing / presenting is complete.

  4. Do nothing; have multiple connections with the same name.

Acceptance Criteria

Tasks

Notes

jeffaudette commented 11 months ago

Not ready for estimate yet, but team needs to be aware of the impacts

nodlesh commented 11 months ago

@cvarjao It seems that, when I have a bare bones OOB invitation with no credential or proof attachment, with the "use_public_did": True set as required, BC Wallet is not recognizing it as a valid QR Code.

When the QR code is scanned by the wallet, the issuer reports the following. 

aiohttp.access INFO 174.96.0.4 [08/Dec/2023:20:17:26 +0000] "GET /?oob=eyJAdHlwZSI6ICJodHRwczovL2RpZGNvbW0ub3JnL291dC1vZi1iYW5kLzEuMS9pbnZpdGF0aW9uIiwgIkBpZCI6ICI2MTM2OTY5MC0zMTkwLTQ2NWEtYjE3Mi1hYTVlMTVhZWMyY2EiLCAibGFiZWwiOiAiYWNhLXB5LkFjbWUiLCAiaGFuZHNoYWtlX3Byb3RvY29scyI6IFsiaHR0cHM6Ly9kaWRjb21tLm9yZy9kaWRleGNoYW5nZS8xLjAiXSwgInNlcnZpY2VzIjogWyJkaWQ6c292OkhaYTQ0Z0ZrMjNQVWlQWE00ZzJmcXoiXX0= HTTP/1.1" 200 149 "-" "BCWallet/1383 CFNetwork/1335.0.3 Darwin/21.6.0"
cvarjao commented 11 months ago

Hey team! Please add your planning poker estimate with Zenhub @bryce-mcmath @jleach @wadeking98

wadeking98 commented 10 months ago

The aries RFCs allow connection reuse and it is already supported by Agents like ACA-py and AFJ. Thiago got it running on bifold using a slightly modified Indy-VDR proxy to resolve the Public DID (https://github.com/thiagoromanos/aries-javascript-indy-vdr-proxy/tree/main). There seems to be some issues with how the react native indy-vdr library resolves DIDs for connection reuse, hence the use of the vdr proxy.

From what I understand, the way connection reuse works is that the inviter sets their DID as the invitation endpoint. The invitee then checks their list of connections to see if they have one that matches the DID and reuses it if possible, otherwise it will form a new connection. This is a limitation for the showcase because each showcase instance is configured to use a traction tenant, all traction tenants share the same endpoint because they're all kind of the same instance. I checked with Emiliano and there is no way for our traction tenant to configure it's own endpoint.

Stephen said that connection reuse should work with peer, non-public DIDs, however I believe the Indy-VDR proxy can only resolve public DIDs. I think if we decide to implement this then we are better off patching the underlying library if necessary instead of using the VDR proxy like Thiago so that we can reuse connections to peer DIDs.

TLDR:

Connection reuse is supported but we won't be able to make it work with the showcase as is, and we may have to make a patch the indy-vdr-react-native library in bifold.

swcurran commented 10 months ago

If we are using did:peer DIDs, the Indy VDR would not be used as part of this. I’ll see about getting someone that knows ACA-Py to try to a spike on this to determine the best approach. Requirements:

swcurran commented 10 months ago

Investigation task added to ACA-Py:

jleach commented 9 months ago

@swcurran Read through hyperledger/aries-cloudagent-python#2703 and am looking forward to the summary. I don't get all the finer points of that discussion. 🙏