search

blockmason / lndr

Lending on the Blockchain
https://lndr.io/
9 stars 6 forks source link

Added sig authentication to PayPalRequest and added delete push channel method #190

Closed willbach closed 6 years ago

willbach commented 6 years ago
  1. Fixes an issue where the user's pushId was not removed from out DB when the user removes an account https://blockmason.atlassian.net/secure/RapidBoard.jspa?rapidView=9&projectKey=ENG&modal=detail&selectedIssue=ENG-133
  2. Approach: add a route for '/register_push' DELETE method to remove entries from the push_data table by user address
  3. No concerns
  4. No blockers
canterberry commented 6 years ago

The semantics of the push registration creation endpoint and the push registration removal endpoint are not quite symmetric. The creation endpoint also takes a provider and a channel ID, whereas the removal endpoint removes the registrations across all providers and channel IDs.

Here is a philosophical question, then. Whichever the answer, some change would be warranted:

The corresponding PR in the LNDR mobile app is to deliver an account removal feature, of which the removal of push registrations is one piece. In that context, it is easier not to iterate over push registration providers and channel IDs (and may not even be possible to do so, if, for example, a user has loaded the same LNDR account to multiple devices). So:

  1. Should the LNDR mobile app, as part of the account removal process, be modified to only remove its own push registrations, making sure to leave the registrations for other devices alone? If so, this endpoint needs to accept a provider and channelID as well as the address.
  2. Should the LNDR mobile app, as part of the account removal process, continue to remove all push registrations as it is currently designed to do in the corresponding PR? If so, this endpoint should be renamed to indicate that it removes all push registrations for the address (e.g: POST /unregister_push_all).
    1. Should the LNDR mobile app, as part of the account removal process, be concerned with what exactly is involved with account removal on the server? (i.e: call something like POST /remove_account to indicate to the server that it should do whatever it needs to do to delete that account, which right now would happen to just be removing all push registrations)
willbach commented 6 years ago
  1. Is probably the correct way to handle this. It will require updating the mobile app to store the user's channelID so that it can be removed
  2. Is overkill as the user could have multiple devices registered
  3. The user is removing their account from the device, not deleting all of their account data. If the user ever wants to restart their account they can use their mnemonic.