-
Notifications
You must be signed in to change notification settings - Fork 57
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
add prerequisites for expansion of connect flow #964
base: main
Are you sure you want to change the base?
Conversation
🦋 Changeset detectedLatest commit: 1b396bd The changes in this PR will be included in the next version bump. This PR includes changesets to release 5 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
TBDocs Report ✅ No errors or warnings @web5/api
@web5/crypto
@web5/crypto-aws-kms
@web5/dids
@web5/credentials
TBDocs Report Updated at 2024-10-24T17:04:00Z |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #964 +/- ##
==========================================
- Coverage 93.37% 93.33% -0.04%
==========================================
Files 118 118
Lines 33806 33913 +107
Branches 2755 2765 +10
==========================================
+ Hits 31565 31653 +88
- Misses 2200 2217 +17
- Partials 41 43 +2
|
* 5. `export`: (if `exported` is true) client will POST a {@link PortableDid} | ||
* 6. `retrieve`: (if `exported` is true) wallet will GET the {@link PortableDid} | ||
* 7. `export-token`: (if `exported` is true) wallet will POST the grants in order to finalize the flow. | ||
* 8. `retrieve-token`: (if `exported` is true) client will GET the grants in order to finalize the flow. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
these are all renamed in the next PR to be more clear, so just ignore this part
Context
The web5 connect flow previously relied on the delegateDid in order to encrypt, sign + verify and move the workflow from step to step.
While this worked for an MVP implementation to get the release out the door, it had a number of issues. The same DID should not have been used for sign/verify as is used for encryption and that same DID should also not be used for persistent delegation.
The code was implemented with TODOs knowing that this would need to be enhanced but the enhancements were slated as security enhancements only.
When the "export" flow was proposed it turned out that this code would need to be fixed immediately. In the export flow the delegation is only transferred at the end of the connect flow. This means that you therefore cannot rely on the delegation for the creation of shared encryption keys.
Therefore fixing this security issue became a priority and a prerequisite for the "export" flow.
Implementation
permissions
helper into theinitClient
method to reduce the amount of boilerplate going on in theWeb5
classBreaking changes for wallet authors.
web5 connect's
getAuthRequest()
now returns an object which include both the authRequest and a DID:web5 connect's
submitAuthResponse()
now requires that the did received fromgetAuthRequest()
is passed in to the method at position 4: