Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BREAKING: Allow multi-use public invites and public invites with metadata #2034

Merged

Conversation

mepeltier
Copy link
Contributor

This PR adjusts the functionality of invitations created with Public DIDs.
Specifically, it does the following:

  • allows for multi-use invitations to be created with a Public DID
  • allows for an agent to create an invitation with metadata when using a Public DID
  • adds a new flag governing behavior of an agent with a Public DID: --requests-through-public-did
    • This flag handles a slightly odd edge case. Prior to this PR, if Alice has --public-invites and --auto-accept, Bob could make an unsolicited connection request against Alice’s Public DId, which is to say, Bob is not responding to an explicit invitation that Alice has created. In that scenario, Alice would automatically accept that request.
    • This flag separates that behavior from the —public-invites flag. --public-invites still allows for invitations to be created with a Public DID, but unless --requests-through-public-did is set, those unsolicited connection requests will be ignored.

Thanks to @dbluhm for additional conceptual design and help debugging :)

@ianco
Copy link
Contributor

ianco commented Jan 5, 2023

This is a breaking change, right? We currently have some implementations that accept unsolicited connection requests using the public did (however not with --auto-accept) - will we need to add the --requests-through-public-did setting to these deployments for the unsolicited connections to work?

@dbluhm
Copy link
Contributor

dbluhm commented Jan 6, 2023

@ianco I think it is unfortunately a bit of a grey area; the original --public-invites flag enabling any connection or DID exchange request seems more like an unintended side effect to me but this PR does technically change this behavior. If your deployment requires the functionality of accepting requests through a public DID without prior invitation, yes, the --requests-through-public-did would be needed to continue operating as before.

We could invert the flag for backwards compatibility (a flag that disables accepting requests through a DID without an invite). However, I am in favor of keeping this a breaking change. I think this will either cause deployments to work the way the --public-invites flag implies and the owner of the agent intended or else make people consciously choose to explicitly allow unsolicited requests.

Thoughts?

@ianco
Copy link
Contributor

ianco commented Jan 13, 2023

@dbluhm I think this PR is ok as is we just need to make sure we include it in the release notes as a breaking change. (I don't think it will affect very many deployments, but folks need to be aware to add the --requests-through-public-did parameter if they are using unsolicited connections with the public DID.) @swcurran do we track breaking changes somewhere?
@mepeltier the PR is out of date can you catch it up with the main branch?

@swcurran swcurran changed the title Allow multi-use public invites and public invites with metadata BREAKING: Allow multi-use public invites and public invites with metadata Jan 13, 2023
@swcurran
Copy link
Contributor

swcurran commented Jan 13, 2023

Breaking changes are tracked in the PR -- such as by changing the name of the PR as I just did -- and then describing the breaking change in a "Breaking Changes" section of the changelog, based on the details about what is broken in the PR notes.

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@swcurran swcurran merged commit 748ec9c into openwallet-foundation:main Jan 13, 2023
WadeBarnes added a commit to WadeBarnes/trust-over-ip-configurations that referenced this pull request Apr 19, 2023
esune pushed a commit to bcgov/trust-over-ip-configurations that referenced this pull request May 10, 2023
swcurran added a commit to bcgov/traction that referenced this pull request Jun 6, 2023
Adds a setting for the Endorser to allow it to accept "implicit invitation" connection requests -- another agent requesting a connection by looking up the Endorser agent's public DID.

This was a breaking change in ACA-Py as per -- openwallet-foundation/acapy#2034

Signed-off-by: Stephen Curran <[email protected]>

Signed-off-by: Stephen Curran <[email protected]>
@dbluhm dbluhm deleted the feat/public-did-multi-use branch July 7, 2023 13:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants