Page MenuHomePhabricator

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

Authored by kamil on Oct 31 2024, 8:28 AM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Dec 16, 2:44 PM
Unknown Object (File)
Thu, Dec 5, 1:42 PM
Unknown Object (File)
Wed, Dec 4, 5:14 AM
Unknown Object (File)
Tue, Dec 3, 12:00 AM
Unknown Object (File)
Thu, Nov 28, 7:25 AM
Unknown Object (File)
Tue, Nov 26, 8:01 PM
Unknown Object (File)
Tue, Nov 26, 7:06 AM
Unknown Object (File)
Sat, Nov 23, 2:27 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.Oct 31 2024, 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.Nov 6 2024, 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