Refactor FXIOS-8297 [v124] Logins Key Retrieval Logic

[lougeniaC64]

📜 Tickets

Jira ticket
Github issue

💡 Description

The logic being re-introduced was backed out in PR #17954 after the logins incident and was not the logic that caused the incident. The problematic code, the verification logic introduced in #17355, is being re-worked and will be added in a later release.

Local Test Steps

1. Create an FxA account.

2. Log in to sync on Firefox desktop with the account created in step 1.

3. Add logins and sync.

4. Open Xcode.

5. Checkout the code in code in PR #18401.

6. Remove any pre-existing device data from the simulator and run the code.

7. Sign in to sync with the account created in step 1.

8. Sync logins via the settings menu and ensure that the logins from desktop appear in the Firefox iOS Passwords menu.

9. Stop the simulator.

10. Make the changes below in the getKeychainData function in RustLogins.swift:
    a. To test when the key is present but not the encrypted canary phrase, replace the completion call with the following:

    	completion(key, nil)
    

    b. To test when the key is missing but the encrypted canary phrase is present, replace the completion call with the following:

    	completion(nil, encryptedCanaryPhrase)
    

    c. To test when both the key and the encrypted canary phrase are missing, replace the completion call with the following:

    	completion(nil, nil)
    

11. Run the iOS code locally again, this time without removing the device data.
    Note: You should add a breakpoint within the switch statement of the getStoredKey function for the case you're testing to ensure it's being hit. You may also want to add a breakpoint in the resetLoginsAndKey function to ensure it is successfully executed.

12. Sync logins in Firefox iOS.

13. The expected result is that, despite the database being wiped, all of the logins created in desktop should still be visible in the Firefox iOS Passwords menu.
    a. You can ensure this is the case by checking the sync logs for the results of the passwords engine sync. The applied record count should equal the number of records you created in Firefox desktop and there should be no outgoing records.

📝 Checklist

You have to check all boxes before merging

• Filled in the above information (tickets numbers and description of your work)
• Updated the PR name to follow our PR naming guidelines
• Wrote unit tests and/or ensured the tests suite is passing
• When working on UI, I checked and implemented accessibility (minimum Dynamic Text and VoiceOver)
• If needed I updated documentation / comments for complex code and public methods

[lougeniaC64]

Please add @mhammond for sync review.

[OrlaM]

LGTM

[lmarceau]

Build failed due to:

❌  /Users/vagrant/git/firefox-ios/Providers/RustSyncManager.swift:384:23: parameter 'completion' expects a single parameter of type '([String], [String : String])'
            completion(rustEngines, localEncryptionKeys)
                                 ^~~~~~~~~~~~~~~~~~~~~

[lougeniaC64]

Build failed due to:

❌  /Users/vagrant/git/firefox-ios/Providers/RustSyncManager.swift:384:23: parameter 'completion' expects a single parameter of type '([String], [String : String])'
            completion(rustEngines, localEncryptionKeys)
                                 ^~~~~~~~~~~~~~~~~~~~~

Sorry about that, @lmarceau! I need to test this and fix and then I'll run again--thanks!

[mobiletest-ci-bot]

	Messages
📖	Project coverage: 33.34%
📖	Edited 5 files
📖	Created 0 files

Client.app: Coverage: 32.11

File	Coverage
RustSyncManager.swift	16.1%	⚠️

CredentialProvider.appex: Coverage: 19.25

File	Coverage
RustSyncManager.swift	16.1%	⚠️

NotificationService.appex: Coverage: 22.83

File	Coverage
RustSyncManager.swift	16.1%	⚠️

ShareTo.appex: Coverage: 32.66

File	Coverage
RustSyncManager.swift	16.1%	⚠️

libStorage.a: Coverage: 59.55

File	Coverage
RustLogins.swift	49.38%	⚠️

Generated by 🚫 Danger Swift against d12483b

[lougeniaC64]

FYI to either @lmarceau or @OrlaM, I made a few commits after the initial approval: one to refactor RustSyncManager.getEnginesAndKeys for clarity, another to remove unused code from the RustLoginsTests.swift file, and the last to change the dispatch queues I added for the keychain calls from async to sync.