atsign-foundation / noports

Connect to any device with no external listening ports open
https://noports.com
BSD 3-Clause "New" or "Revised" License
267 stars 15 forks source link

Sshnp to yourself i.e single atSign #219

Open XavierChanth opened 1 year ago

XavierChanth commented 1 year ago
### Tasks
- [ ] Minor bug: Cannot share a key to your self (i.e. with a shared key)
- [x] Put tests in place
cconstab commented 1 year ago

This is required for the use of a single atSign and using PKAM for additional atKeys. Also enables things like atMosphere to send files to yourself.

gkc commented 1 year ago

Setting to P0 as this is part of the solution one large customer requires and testing may throw up some issues

gkc commented 1 year ago

We didn't get to this during PR 66 but is top of my list for PR 67

gkc commented 1 year ago

Other even more important things took priority during PR67 😎

@VJag would you be willing to pick this up during this sprint? Specifically we'd be looking to ensure that (a) we have tests which will ensure that sending notifications to oneself continues to work (b) see if we can optimize how that actually works (I think right now it ends up with the atServer pol-ing to itself)

VJag commented 1 year ago

Sure. I will work on this in this sprint.

VJag commented 1 year ago

Notes on self-notification (notify:text & notify:update)

  1. A secondary server allows self-notifications (The sender is same as the recipient)

  2. SDK does not impose any artificial restriction on self notifications

  3. On the server side, NotificationVerbHandler correctly marks the notification as a "received" notification. An incorrect way of handling would be to mark it as a sent (to_be_sent) notification, in which case it will be polling itself.

  4. This Notification will be written back to the Monitor connections, just like any other received notifications

  5. A self Notification is marked as "Delivered" when we persist it to the Notification store

  6. There is a functional test that tests if a self-notification is delivered. However, we can extend this test or add a few more tests for self notifications (Test name = A test to verify notification shared to current atSign is fetched)

These tests could:

  1. Verify that a self-notification is delivered to self
  2. Verify that the self-notification is written to monitors (Need to write)
gkc commented 1 year ago

Thanks @VJag

What about notify:update? That's what we are using in sshnoports

VJag commented 1 year ago

notify:update works too using the same underlying mechanism. Tested with below code snippet.

var notificationResult = await atClientManager.atClient.notificationService.notify( NotificationParams.forUpdate( (AtKey.shared('phone', namespace: 'wavi', sharedBy: '@ sitaram') ..sharedWith('@ sitaram')) .build()));

We need to add tests for this scenario too.

purnimavenkatasubbu commented 1 year ago

Added tests for self-notifications. Tests are available in the branch