Page MenuHomePhabricator

[CommCoreModule][native_rust_library] update downloading User Keys to support user identifier
ClosedPublic

Authored by kamil on Thu, Oct 31, 8:28 AM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Nov 18, 11:56 AM
Unknown Object (File)
Fri, Nov 15, 8:19 PM
Unknown Object (File)
Fri, Nov 15, 1:14 AM
Unknown Object (File)
Thu, Nov 14, 11:22 PM
Unknown Object (File)
Mon, Nov 11, 6:22 AM
Unknown Object (File)
Mon, Nov 11, 2:06 AM
Unknown Object (File)
Thu, Nov 7, 10:23 AM
Unknown Object (File)
Thu, Nov 7, 7:20 AM

Details

Summary

ENG-6145.

An alternative to this is to pass username and get BackupID from Rust — in terms of network calls, it should be the same. I implemented this because it required a lot fewer changes because it reused existing code.

Alternatively, we can download only backupID and read backupDataKey and backupLogDataKey (it should exist on primary always) to avoid downloading User Keys - but curious about @bartek's perspective as I don't fully remember why it was implemented that way,

  1. Deprecate download_latest_backup_id
  2. Update retrieveBackupKeys to have backupID

Depends on D13847

Test Plan

This is the final diff in the stack so testing this end-to-end:

  1. iOS and Android (I was using physical devices)
  2. username and wallet user
  3. testing restore similarly to link

Testing secondary device auth:

  1. Added log here to make sure keys are retrieved
  2. Added log on the recipient to make sure those are transferred correctly

Tested password/SIWE user on iOS, when secondary was web or Android - works.

I found a couple of issues with this flow but not related to the backup itself - I am going to list them in ENG-9841.

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

kamil held this revision as a draft.
kamil published this revision for review.Thu, Oct 31, 8:55 AM
kamil edited the summary of this revision. (Show Details)
kamil added inline comments.
native/native_rust_library/src/backup.rs
329–354 ↗(On Diff #45516)

deprecated in favour of download_latest_backup_info

varun added 1 blocking reviewer(s): bartek.

LGTM, but setting @bartek as blocking since @kamil has asked for his thoughts above

Alternatively, we can download only backupID and read backupDataKey and backupLogDataKey (it should exist on primary always) to avoid downloading User Keys - but curious about @bartek's perspective as I don't fully remember why it was implemented that way,

I don't remember either. Your changes make sense though

This revision is now accepted and ready to land.Wed, Nov 6, 1:01 AM

Alternatively, we can download only backupID and read backupDataKey and backupLogDataKey (it should exist on primary always) to avoid downloading User Keys - but curious about @bartek's perspective as I don't fully remember why it was implemented that way,

I don't remember either. Your changes make sense though

ENG-9873