GoodDollar / GoodServer

Backend to support the GoodDAPP
MIT License
13 stars 13 forks source link

Server should have cron task to delete FV records #288

Closed sirpy closed 3 years ago

sirpy commented 3 years ago

To keep users privacy we dont keep a connection between their FV record and their database record If a user loses his account we have no way to delete his FV record, and so he will never be able to open another account. After a user passes FV we whitelist him in the identity smart contract. the user will be whitelisted for the "authenticationPeriod" specified in days in the smart contract. The server should periodically delete FV records of users that did their last FV > authenticationPeriod

omerzam commented 3 years ago

authenticationPeriod should be read from the contract @sirpy which contract do I read this from?

serdiukov-o-nordwhale commented 3 years ago

Short pre-amble:

The feature you developed could use that queue as FV snapshots storage. Without adding an separate model and cron job

To do that we just need to extend a bit queue options:

With that option we just need then:

serdiukov-o-nordwhale commented 3 years ago

@sirpy @omerzam all that stuff isn't needed (except getAuthenticationPeriod() of course) because actually we already have the tasks queue to dispose enrollments.

How i see the solution:

  1. subject field by design could have any value depending of task type. for dispose enrollments it's just a string with enrollmend id now. Need to change it to the object
    { enrollmentIdentifier: string, executeAt: ‘auth-period’ | ‘account-removal’ }
  2. add cancelTasksQueued method to the UserPrivateDB with the interface similar to hasTasksQueued (but returning Promise<void>). It should remove tasks of taskName and filters specified
  3. add taskUtil.js file, move DISPOSE_ENROLLMENTS_TASK constant definition and export function scheduleDisposalTask(storage, enrollmentIdentifier, executeAt) which calls
    await storage.cancelTasksQueued(DISPOSE_ENROLLMENTS_TASK, { 'subject.enrollmentIdentifier': enrollmentIdentifier })
    await storage.enqueueTask(DISPOSE_ENROLLMENTS_TASK, { enrollmentIdentifier, executeAt })

    this is neet to avoid passing processor to session and to avoid circular imports processor -> session -> processor (i'll refactor that file to the TaskHelper class later)

  4. in EnrollmentSession / onEnrollmentCompleted add
    scheduleDisposalTask(storage, enrollmentIdentifier, 'auth-period');  

    to the Promise.all(...)

  5. in enqueueDisposal update enqueueTask call to
    scheduleDisposalTask(storage, enrollmentIdentifier, 'account-removal');  
  6. update hasTasksQueued call in isEnqueuedForDisposal to
    hasTasksQueued(DISPOSE_ENROLLMENTS_TASK, { subject: { enrollmentIdentifier, executeAt: 'account-removal' })
  7. update disposeEnqueuedEnrollments. Call getAuthenticationPeriod(). Update the following code:

    const enqueuedAtFilters = {}
    
    if (keepEnrollments > 0) {
      enqueuedAtFilters.createdAt = {
        $lte: moment()
          .subtract(keepEnrollments, 'hours')
          .toDate()
      }
    }

to

const deletedAccountFilters = {  'subject.executeAt': 'account-removal' }

if (keepEnrollments > 0) {
  deletedAccountFilters.createdAt = {
    $lte: moment()
      .subtract(keepEnrollments, 'hours')
      .toDate()
  }
}

const enqueuedAtFilters = { 
  $or: [deletedAccountFilters, {
     'subject.executeAt': 'auth-period',
    createdAt: moment()
      .subtract(authPeriod, 'days')
      .toDate()
  }]

That's all. no need to introduce new mongo model, collection cron task etc. it re-uses everything was before, just:

iLystopad commented 3 years ago

The FV record is deleted after the long-term disuse of the account. The last claim was 26.02.21 and after that, the account hasn't been using for more than 14 days.

Checked on the QA 1.24.1-6 Device: Windows 10 // Google Chrome 89.0.4389.90

Additional info: Email of the checked account - 1tester8913+obivan@gmail.com

Attachment: https://www.screencast.com/t/nxWNtWySqRe

sirpy commented 3 years ago

@iLystopad i'm not sure from the screencast how this was tested. i think this needs to be tested on backend @serdiukov-o-nordwhale please provide explanation how to test, or perform test together with @iLystopad

serdiukov-o-nordwhale commented 3 years ago

@iLystopad for now authentication period is set to half of year, not 14 days. It reads from the blockchain But we still have 24hrs period after removal. Doesn't matters which case to test (14d or 24h) - it uses the same queue, the same tasks etc

How to test:

Also, you could test 'auth period' using mongo client (like Robo3T). I could share connection string via Slack.

How to test:

iLystopad commented 3 years ago

@sirpy @serdiukov-o-nordwhale The FV record is deleted after 24 hours from the deleting of the account and the user can to pass FV again

How it was tested: 1) 29.03.21 at 16.00 the Account with passed FV was deleted and recreated with the same Google account https://www.dropbox.com/s/43s4vzx0rlrzwlk/Deleting%20of%20the%20account%20with%20passed%20FV%20At%2016-00%20%202021-03-29_16-14-32.mp4?dl=0

2) 30.03.21 at 15.00 Tried to pass FV on the recreated account - The "New wallet? you need to wait 24 hours" message is displayed https://www.dropbox.com/s/k344r7ukps0pim1/Trying%20to%20pass%20the%20FV%20on%20the%20recreated%20Account%20at%2015-00%2030-03-21%20.mp4?dl=0

3) At 15.15 tried to pass FV on another account - The "Unfortunately we found your tween" message is displayed https://www.dropbox.com/s/aoerqp1l1967mas/Trying%20to%20pass%20the%20FVon%20the%20Another%20account%202021-03-30_15-14-24.mp4?dl=0

4) 30.03.21 At 17.00 Passed the FV on the recreated account after 24 hours from the deleting of the account https://www.dropbox.com/s/81l1ds5yk4sgitc/Passing%20the%20FV%20after%20deleting%20at%2017.05%202021-03-30_17-06-06.mp4?dl=0

Checked on the Prod 1.25.0 Device: Windows 10 // Google Chrome v86

Additional info: Tested on the account - 5tester8913@gmail.com Wallet address - 0x29cD00f8d3811af7a8987a0b7945B6b8d59800fe UserID - 0x951D25409a73ACE0EFeE9b8747952f29ba75AfA7 GD_masterSeed - "f0557b770bc32efaa4faffd8bd219b0ecb5e9bfc40877dc24855f78e373924ef"