veramo icon indicating copy to clipboard operation
veramo copied to clipboard

feat: ION DID Provider implementation

Open nklomp opened this issue 3 years ago • 1 comments

What issue is this PR fixing

closes #336 and closes #440

What is being changed

Implement ION DID Provider. Seeking initial feedback before rebasing to Veramo and opening a PR there.

Changes:

  • Do not depend on ion-tools
  • Depend on external ION POW SDK.
  • Update and recovery keys, including key rotations
  • Full implementation of all Identifier methods, with the exception of the recent updateIdentifier method

Known issues:

  • The ION PoW lib on which this PR depends works on Node. It is not tested yet on React-native, but we do have a React-Native version of the dependency. I will ensure it will work on both, since we need it @Sphereon anyway
  • Adding/Updating services/keys does not get propagated on chain. Needs more investigation. Microsoft gives back a 200 response for them, but it never ends up in the DID Document.

Quality

Check all that apply:

  • [X] I want these changes to be integrated
  • [X] I successfully ran yarn, yarn build, yarn test, yarn test:browser locally.
  • [X] I allow my PR to be updated by the reviewers (to speed up the review process).
  • [X] I added unit tests.
  • [X] I added integration tests.
  • [ ] I did not add automated tests because _________, and I am aware that a PR without tests will likely get rejected.

Details

If applicable, add screen captures, error messages or stack traces to help explain your problem.

nklomp avatar Aug 10 '22 20:08 nklomp

Codecov Report

Merging #987 (a41b321) into next (125bf42) will increase coverage by 0.65%. The diff coverage is 83.19%.

Additional details and impacted files
@@            Coverage Diff             @@
##             next     #987      +/-   ##
==========================================
+ Coverage   80.25%   80.90%   +0.65%     
==========================================
  Files         118      124       +6     
  Lines        4056     4451     +395     
  Branches      875      950      +75     
==========================================
+ Hits         3255     3601     +346     
- Misses        798      847      +49     
  Partials        3        3              

codecov[bot] avatar Aug 15 '22 08:08 codecov[bot]

Updated our ION POW package to now depend on a new isomorphic Argon2 hash package created by us. Verified to be working in browser, node and react-native. For react-native a peer dependency is put in the package.json. This dep needs to be installed, since native components are needed for Argon2 in RN. Except for installing the peer dep, which is mentioned in the Readme, a user does not have to change anything else on RN.

Still looking into the update issue. As far as we can tell right now this only is happening on the public Microsoft hosted ION node. I will get in contact with my contacts within the Microsoft team to see if this is a known issue or not. In the mean time we are setting up our own ION node, but that takes a bit of time, given the components involved.

nklomp avatar Aug 23 '22 17:08 nklomp

Can we flag the update issue as technical debt and merge the PR? It's holding up any app from being able to support MS Entra.

rkreutzer avatar Sep 30 '22 20:09 rkreutzer

Thank you for the contribution!

There is a tiny nit-pick about the import paths in one of the tests, but otherwise I wish all PRs were this beautiful :)

Thanks. Updated the PR, and created tickets for the TD: https://github.com/uport-project/veramo/issues/1019 and suggestion: https://github.com/uport-project/veramo/issues/1021

nklomp avatar Oct 04 '22 13:10 nklomp

Updated Veramo dep versions as it gave an error on the latest CI run

nklomp avatar Oct 04 '22 13:10 nklomp

Updated Veramo dep versions as it gave an error on the latest CI run

Thanks, I was trying to do that on my end but I didn't know if I could push to your PR branch. If this builds fine, we'll be merging it soon

mirceanis avatar Oct 04 '22 13:10 mirceanis