verida / data-connector-server

1 stars 2 forks source link

API Feedback from Aurel #114

Closed tahpot closed 1 month ago

tahpot commented 1 month ago

Closes #113 #108 #105 #111 #112 #106 #107

aurelticot commented 1 month ago

@tahpot let me know when the PR is ready for review. I see you're adding a few more things

tahpot commented 1 month ago

Note: This refactor has made a lot of breaking changes.

You will need to disconnect any existing connections you have and then re-create them before using this branch.

tahpot commented 1 month ago

See inline comments

Additionally, I was unable to connect to Google while testing with the embedded web dev UI.

Got the following error:

Screenshot 2024-09-18 at 12 47 40 PM

It refers to:

https://github.com/verida/data-connector-server/blob/16b742a388bc438952c66ba30e8fbdafb2898551/src/utils.ts#L59-L64

The network and DID client config come from my serverconfig.local.json whic seem correct.

The contextSignature is passed when calling Utils.getNetwork in the `callback controller function.

https://github.com/verida/data-connector-server/blob/16b742a388bc438952c66ba30e8fbdafb2898551/src/providers/controller.ts#L88-L100

The key comes from the session, this may be the missing element

I had this issue as well. There's a bug in the UI where entering the private key for the first time in the connection screen doesn't work. Enter the private key, then reload the page, then try to connect.

aurelticot commented 1 month ago

I had this issue as well. There's a bug in the UI where entering the private key for the first time in the connection screen doesn't work. Enter the private key, then reload the page, then try to connect.

Ok. I was about to comment tohat I don't have this issue in the Vault web when using this branch for the server, so yes it comes from the embedded dev web UI.

tahpot commented 1 month ago

Back to Google: data.profile.email doesn't exist in the Passport profile, it's data.profile.emails, hence the importance of typing passport properly, it would have been caught.

Should be fixed with: https://github.com/verida/data-connector-server/commit/218a7d8aab6d891f2319635a546785cdb4e0017c

tahpot commented 1 month ago

Back in the callback controller, it eventually saveProvider (should actually be called saveNewConnection to be more relevant)

Fixed.

It can be easily fixed with the following (but also requires fixing the types of the profile)

Should also be fixed.

See https://github.com/verida/data-connector-server/commit/3007d70338a882a2d952d87334029c41c2c98dca