Skip to content

fix wasteful backup search in inbox on Gmail API#355

Merged
tomholub merged 20 commits intomasterfrom
feature/issue-309-backups
Jul 8, 2021
Merged

fix wasteful backup search in inbox on Gmail API#355
tomholub merged 20 commits intomasterfrom
feature/issue-309-backups

Conversation

@Kharchevskyi
Copy link
Contributor

@Kharchevskyi Kharchevskyi commented Jun 17, 2021

This PR fix wasteful backup search in inbox on Gmail API

close #309

Tests:

  • Start working on GmailService tests
  • Added mocks implementations for GoogleUserServiceType and GmailBackupSearchQueryProviderType

Screenshot 2021-06-17 at 23 25 53


To be filled by reviewers

I have reviewed that this PR... (tick whichever items you personally focused on during this review):

  • addresses the issue it closes (if any)
  • code is readable and understandable
  • is accompanied with tests, or tests are not needed
  • is free of vulnerabilities
  • is documented clearly and usefully, or doesn't need documentation

@Kharchevskyi Kharchevskyi requested a review from tomholub June 17, 2021 20:31
Copy link
Collaborator

@tomholub tomholub left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks good, will test functionality manually now

Copy link
Collaborator

@tomholub tomholub left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This still does not address the issue. When I try it on my personal account, it shows:

"ℹ️[App Start][GlobalRouter] proceed for session nil"
"ℹ️[Keychain][KeyChainService] LocalStorage.secureKeychainPrefix generating new: wFjxOWGVu77zZW4o"
"ℹ️[Keychain][KeyChainService] generateAndSaveStorageEncryptionKey"
"ℹ️[Js][Core] JsContext took 0.532 ms to start"
"ℹ️[App Start][GlobalRouter] Sign in with gmailLogin(<FlowCrypt.SignInViewController: 0x108059c00>)"
"ℹ️[App Start][GlobalRouter] proceed for session Optional(FlowCrypt.SessionType.google(...)"
"ℹ️[App Start][GoogleUserService] Renew session for google user"
"ℹ️ [Setup] searching for backups in inbox"
"🏷[GmailService] will begin searching for backups"
"🏷[GmailService] searching done, found 24 backup messages"
"🏷[GmailService] processing backup \'Your FlowCrypt Backup\' with 1 attachments"
"🏷[GmailService] processing backup \'Fwd: Your FlowCrypt Backup\' with 1 attachments"
"🏷[GmailService] processing backup \'Fwd: Your FlowCrypt Backup\' with 1 attachments"
"🏷[GmailService] processing backup \'Re: Problems with my Flowcrypt account\' with 1 attachments"
"🏷[GmailService] processing backup \'Your FlowCrypt Backup\' with 1 attachments"
"🏷[GmailService] processing backup \'FlowCrypt Feedback from ...@gmail.com\' with 1 attachments"
"🏷[GmailService] processing backup \'Re: Your FlowCrypt Backup\' with 2 attachments"
"🏷[GmailService] processing backup \'Fwd: Your FlowCrypt Backup\' with 1 attachments"
"🏷[GmailService] processing backup \'Please Provide Assistance\' with 1 attachments"
"🏷[GmailService] processing backup \'Re: Your FlowCrypt Backup\' with 2 attachments"
"🏷[GmailService] processing backup \'FlowCrypt Feedback from ...@gmail.com\' with 1 attachments"
"🏷[GmailService] processing backup \'Fwd: Your FlowCrypt Backup\' with 1 attachments"
"🏷[GmailService] processing backup \'Re: Passphrase lost\' with 1 attachments"
"🏷[GmailService] processing backup \'Re: FlowCrypt Feedback from ...@gmail.com\' with 3 attachments"
"🏷[GmailService] processing backup \'Pass phrase\' with 1 attachments"
"🏷[GmailService] processing backup \'Re: Problems with my Flowcrypt account\' with 1 attachments"
"🏷[GmailService] processing backup \'Re: Your FlowCrypt Backup\' with 1 attachments"
"🏷[GmailService] processing backup \'Re: Passphrase lost\' with 1 attachments"
"🏷[GmailService] processing backup \'Re: Your FlowCrypt Backup\' with 2 attachments"
"🏷[GmailService] processing backup \'Fwd: Your FlowCrypt Backup\' with 1 attachments"
"🏷[GmailService] processing backup \'FlowCrypt Feedback from ...@gmail.com\' with 1 attachments"
"🏷[GmailService] processing backup \'Re: Your FlowCrypt Backup\' with 2 attachments"
"🏷[GmailService] processing backup \'FlowCrypt Feedback from ...@gmail.com\' with 1 attachments"
"🏷[GmailService] processing backup \'Re: Passphrase lost\' with 1 attachments"
"🏷[GmailService] downloading 30 attachments with possible backups in them"
"🏷[GmailService] downloaded 30 attachments that contain 2171kB of data"
"❗️[Js][Core] ------ js err -------\nCore parseKeys:\nTypeError: coreHost.log is not a function. (In \'coreHost.log(String(x))\', \'coreHost.log\' is undefined)\n_log\n\n\nasyncFunctionResume@[native code]\n[native code]\npromiseReactionJobWithoutPromise@[native code]\n------- end js err -----"

@tomholub
Copy link
Collaborator

tomholub commented Jun 21, 2021

Problems:

  1. it's supposed to search for the text only in subject, but it actually searches the text anywhere in the email
  2. it's supposed to search for backups from:<my primary email>, to:<my primary email> but these filters are missing, so it's picking up on emails that I've sent to others, or that others sent to me, which is not valid

Also it should NOT be searching for items that are in trash. I think in:anywhere may be also searching in Trash wrongly? (unsure)

@tomholub
Copy link
Collaborator

Btw you don't have to construct the search query manually. If you can supply a textual query, then use endpoint called gmailBackupSearch - just add a Core.gmailBackupSearch method. Input is acctEmail where you put the account email address, and back you get the constructed search query that you will know is correct.

https://github.com/FlowCrypt/flowcrypt-mobile-core/blob/b7d8d10bdd2b5c17e17ec03505801e8bf7e89073/source/mobile-interface/endpoints.ts#L224

@tomholub
Copy link
Collaborator

If you do prefer to construct this query in Swift anyway for some reason, then here is an exact test case to copy, the test case you wrote is too vague:

https://github.com/FlowCrypt/flowcrypt-mobile-core/blob/b7d8d10bdd2b5c17e17ec03505801e8bf7e89073/source/test.ts#L280

@tomholub tomholub marked this pull request as draft June 21, 2021 14:33
@Kharchevskyi Kharchevskyi marked this pull request as ready for review June 22, 2021 16:31
@Kharchevskyi
Copy link
Contributor Author

@tomholub can you please check with query generated with Core

Copy link
Collaborator

@tomholub tomholub left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like it will work - will test

tomholub
tomholub previously approved these changes Jun 23, 2021
Copy link
Collaborator

@tomholub tomholub left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks good. Will test

@tomholub
Copy link
Collaborator

This worked for the search, but after entering pass phrase, it sent me back to splash screen. Log:

"ℹ️[Keychain][KeyChainService] LocalStorage.secureKeychainPrefix generating new: TyvDJ6VPktNBrGQO"
"ℹ️[Keychain][KeyChainService] generateAndSaveStorageEncryptionKey"
"ℹ️[App Start][GlobalRouter] proceed for session nil"
"ℹ️[Js][Core] JsContext took 0.288 ms to start"
"ℹ️[App Start][GlobalRouter] Sign in with gmailLogin(<FlowCrypt.SignInViewController: 0x15802b400>)"
"ℹ️[App Start][GlobalRouter] proceed for session Optional(Google tom@flowcrypt.com Tom J. H.)"
"ℹ️[App Start][GoogleUserService] Renew session for google user"
"ℹ️[Setup][SetupInitialViewController] Changed to new state searching"
"ℹ️[Setup][SetupInitialViewController] Searching for backups in inbox"
"🏷[GmailService] will begin searching for backups"
"🏷[GmailService] searching done, found 2 backup messages"
"🏷[GmailService] processing backup \'Your FlowCrypt Backup\' with 1 attachments"
"🏷[GmailService] processing backup \'Your FlowCrypt Backup\' with 1 attachments"
"🏷[GmailService] downloading 2 attachments with possible backups in them"
"🏷[GmailService] downloaded 2 attachments that contain 7kB of data"
"ℹ️[Setup][SetupInitialViewController] Finish searching for backups in inbox"
"ℹ️[Setup][SetupInitialViewController] 2 key backups found in inbox"
"ℹ️[Core][KeyMethods] Filtered. decryptedKey = nil"
"ℹ️[PassPhraseStorage] Save to storage 0C9C2E6A4D273C6F"
"ℹ️[App Start][GlobalRouter] proceed for session nil"

@tomholub tomholub marked this pull request as draft June 23, 2021 21:34
@Kharchevskyi
Copy link
Contributor Author

I have added some logs. Can you please try to reproduce this issue once again?

@Kharchevskyi Kharchevskyi marked this pull request as ready for review June 24, 2021 17:03
@Kharchevskyi Kharchevskyi marked this pull request as draft June 24, 2021 17:08
@tomholub
Copy link
Collaborator

"ℹ️[Keychain][KeyChainService] LocalStorage.secureKeychainPrefix generating new: 6D1zqclvuM3iBVUz"
"ℹ️[Keychain][KeyChainService] generateAndSaveStorageEncryptionKey"
"ℹ️[App Start][GlobalRouter] proceed for session nil"
"ℹ️[AppStart] Initialize application with session nil"
"ℹ️[AppStart] Setup Core"
"ℹ️[Js][Core] JsContext took 0.271 ms to start"
"ℹ️[AppStart] Setup Migration"
"ℹ️[AppStart] Setup Session"
"ℹ️[AppStart] User is not logged in -> signIn"
"ℹ️[App Start][GlobalRouter] Sign in with gmailLogin(<FlowCrypt.SignInViewController: 0x105069400>)"
"ℹ️[App Start][GlobalRouter] proceed for session Optional(Google ... Tom J. H.)"
"ℹ️[AppStart] Initialize application with session Optional(Google ... Tom J. H.)"
"ℹ️[AppStart] Setup Core"
"ℹ️[AppStart] Setup Migration"
"ℹ️[AppStart] Setup Session"
"ℹ️[App Start][GoogleUserService] Renew session for google user"
"ℹ️ UserId = current user id"
"ℹ️[AppStart] User with session Google ... Tom J. H. -> setupFlow"
"ℹ️[Setup][SetupInitialViewController] Changed to new state searching"
"ℹ️[Setup][SetupInitialViewController] Searching for backups in inbox"
"🏷[GmailService] will begin searching for backups"
"🏷[GmailService] searching done, found 2 backup messages"
"🏷[GmailService] processing backup 'Your FlowCrypt Backup' with 1 attachments"
"🏷[GmailService] processing backup 'Your FlowCrypt Backup' with 1 attachments"
"🏷[GmailService] downloading 2 attachments with possible backups in them"
"🏷[GmailService] downloaded 2 attachments that contain 7kB of data"
"ℹ️[Setup][SetupInitialViewController] Finish searching for backups in inbox"
"ℹ️[Setup][SetupInitialViewController] 2 key backups found in inbox"
"ℹ️[Setup][SetupBackupsViewController] Start recoverAccount with 2"
"ℹ️[Core][KeyMethods] Filtered. decryptedKey = nil for key 84AE4C8A1923A36A"
"ℹ️[Setup][SetupBackupsViewController] matchingKeyBackups = 1"
"ℹ️[PassPhraseService] Save to storage 0C9C2E6A4D273C6F"
"ℹ️[App Start][GlobalRouter] proceed for session nil"
"ℹ️[AppStart] Initialize application with session nil"
"ℹ️[AppStart] Setup Core"
"ℹ️[AppStart] Setup Migration"
"ℹ️[AppStart] Setup Session"
"ℹ️[AppStart] User is not logged in -> signIn"

@Kharchevskyi
Copy link
Contributor Author

thanks for reporting, will investigate this

@tomholub
Copy link
Collaborator

@Kharchevskyi I'll take a look. Please edit your comment per the above bot. Eg you can say cryptup.tester account.

@tomholub
Copy link
Collaborator

tomholub commented Jul 1, 2021

That's not what I meant. The comment was asking you to redact email address from your earlier comment here #355 (comment)

@tomholub
Copy link
Collaborator

tomholub commented Jul 1, 2021

This is a public repo, we don't want email addresses mentioned publicly here.

@tomholub
Copy link
Collaborator

tomholub commented Jul 1, 2021

my own account:

"ℹ️[UserAccountService] Clean up sessions"
"ℹ️[Keychain][KeyChainService] LocalStorage.secureKeychainPrefix generating new: ueAHpE1Bu1opbPk9"
"ℹ️[Keychain][KeyChainService] generateAndSaveStorageEncryptionKey"
"ℹ️[App Start][GlobalRouter] proceed for session nil"
"ℹ️[AppStart] Initialize application with session nil"
"ℹ️[AppStart] Setup Core"
"ℹ️[Js][Core] JsContext took 0.265 ms to start"
"ℹ️[AppStart] Setup Migration"
"ℹ️[AppStart] Setup Session"
"ℹ️[AppStart] User is not logged in -> signIn"
"ℹ️[App Start][GlobalRouter] Sign in with gmailLogin(<FlowCrypt.SignInViewController: 0x15c065200>)"
"ℹ️[App Start][GlobalRouter] proceed for session Optional(Google ... Tom J. H.)"
"ℹ️[AppStart] Initialize application with session Optional(Google ... Tom J. H.)"
"ℹ️[AppStart] Setup Core"
"ℹ️[AppStart] Setup Migration"
"ℹ️[AppStart] Setup Session"
"ℹ️[App Start][GoogleUserService] Renew session for google user"
"ℹ️ UserId = current user id"
"ℹ️[AppStart] User with session Google ... Tom J. H. -> setupFlow"
"ℹ️[Setup][SetupInitialViewController] Changed to new state searching"
"ℹ️[Setup][SetupInitialViewController] Searching for backups in inbox"
"🏷[GmailService] will begin searching for backups"
"🏷[GmailService] searching done, found 2 backup messages"
"🏷[GmailService] processing backup \'Your FlowCrypt Backup\' with 1 attachments"
"🏷[GmailService] processing backup \'Your FlowCrypt Backup\' with 1 attachments"
"🏷[GmailService] downloading 2 attachments with possible backups in them"
"🏷[GmailService] downloaded 2 attachments that contain 7kB of data"
"ℹ️[Setup][SetupInitialViewController] Finish searching for backups in inbox"
"ℹ️[Setup][SetupInitialViewController] 2 key backups found in inbox"
"ℹ️[Setup][SetupBackupsViewController] Start recoverAccount with 2"
"ℹ️[Core][KeyMethods] Filtered. decryptedKey = nil for key 84AE4C8A1923A36A"
"ℹ️[Setup][SetupBackupsViewController] matchingKeyBackups = 1"
"ℹ️[PassPhraseService] Save to storage 0C9C2E6A4D273C6F"
"ℹ️[KeyDataStorage] Add keys for [\"Human at FlowCrypt <...>\"]"
"ℹ️[UserAccountService] Clean up sessions"
"ℹ️[UserAccountService] User session to clean up ..."
"ℹ️[UserAccountService] Log out user with ..."
"ℹ️[App Start][GlobalRouter] proceed for session nil"
"ℹ️[AppStart] Initialize application with session nil"
"ℹ️[AppStart] Setup Core"
"ℹ️[AppStart] Setup Migration"
"ℹ️[AppStart] Setup Session"
"ℹ️[AppStart] User is not logged in -> signIn"

@tomholub
Copy link
Collaborator

tomholub commented Jul 1, 2021

compatibility account - worked normally:


"ℹ️[App Start][GlobalRouter] Sign in with gmailLogin(<FlowCrypt.SignInViewController: 0x15c0dfc00>)"
"ℹ️[App Start][GlobalRouter] proceed for session Optional(Google ... FlowCrypt Compatibility)"
"ℹ️[AppStart] Initialize application with session Optional(Google ... FlowCrypt Compatibility)"
"ℹ️[AppStart] Setup Core"
"ℹ️[AppStart] Setup Migration"
"ℹ️[AppStart] Setup Session"
"ℹ️[App Start][GoogleUserService] Renew session for google user"
"ℹ️ UserId = current user id"
"ℹ️[AppStart] User with session Google ... FlowCrypt Compatibility -> setupFlow"
"ℹ️[Setup][SetupInitialViewController] Changed to new state searching"
"ℹ️[Setup][SetupInitialViewController] Searching for backups in inbox"
"🏷[GmailService] will begin searching for backups"
"🏷[GmailService] searching done, found 3 backup messages"
"🏷[GmailService] processing backup \'Your FlowCrypt Backup\' with 1 attachments"
"🏷[GmailService] processing backup \'Your FlowCrypt Backup\' with 1 attachments"
"🏷[GmailService] processing backup \'Your FlowCrypt Backup\' with 1 attachments"
"🏷[GmailService] downloading 3 attachments with possible backups in them"
"🏷[GmailService] downloaded 3 attachments that contain 20kB of data"
"ℹ️[Setup][SetupInitialViewController] Finish searching for backups in inbox"
"ℹ️[Setup][SetupInitialViewController] 3 key backups found in inbox"
"ℹ️[Setup][SetupBackupsViewController] Start recoverAccount with 3"
"ℹ️[Core][KeyMethods] Filtered. decryptedKey = nil for key 7FDE685548AEA788"
"ℹ️[Core][KeyMethods] Filtered. decryptedKey = nil for key ADAC279C95093207"
"ℹ️[Setup][SetupBackupsViewController] matchingKeyBackups = 1"
"ℹ️[PassPhraseService] Save to storage ADAC279C95093207"
"ℹ️[KeyDataStorage] Add keys for [\"FlowCrypt Compatibility <...>\"]"
"ℹ️[UserAccountService] Clean up sessions"
"ℹ️[App Start][GlobalRouter] proceed for session nil"
"ℹ️[AppStart] Initialize application with session nil"
"ℹ️[AppStart] Setup Core"
"ℹ️[AppStart] Setup Migration"
"ℹ️[AppStart] Setup Session"
"ℹ️[App Start][GoogleUserService] Renew session for google user"
"ℹ️[AppStart] Setup finished -> mainFlow"
"ℹ️[GmailService] Skip category label with CATEGORY_FORUMS"
"ℹ️[GmailService] Skip category label with CATEGORY_UPDATES"
"ℹ️[GmailService] Skip category label with CATEGORY_PERSONAL"
"ℹ️[GmailService] Skip category label with CATEGORY_PROMOTIONS"
"ℹ️[GmailService] Skip category label with CATEGORY_SOCIAL"

@Kharchevskyi
Copy link
Contributor Author

my own account:

probably issue is here "ℹ️[UserAccountService] Clean up sessions". Thank's for the logs

@tomholub
Copy link
Collaborator

tomholub commented Jul 5, 2021

Sent to your email

@tomholub
Copy link
Collaborator

tomholub commented Jul 5, 2021

Copying from email:

You may completely ignore the email addresses listed inside of the PGP keys. They don't have to match the account they are set up for.

@Kharchevskyi Kharchevskyi marked this pull request as ready for review July 5, 2021 17:48
Copy link
Collaborator

@tomholub tomholub left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code looks ok - I'm not at my mac until wednesday to test it. I'll merge it and create another issue in case it misbehaves.

@tomholub
Copy link
Collaborator

tomholub commented Jul 5, 2021

However tests are failing - can you take a look?

@tomholub
Copy link
Collaborator

tomholub commented Jul 7, 2021

This fixed the issue, thank you

@tomholub tomholub enabled auto-merge (squash) July 7, 2021 21:37
@tomholub
Copy link
Collaborator

tomholub commented Jul 7, 2021

@Kharchevskyi the CI still cannot build it, I've updated from master

@tomholub
Copy link
Collaborator

tomholub commented Jul 8, 2021

also #381 was observed on this PR

@Kharchevskyi
Copy link
Contributor Author

Last commits should also fix #381

@tomholub tomholub merged commit 1b46e17 into master Jul 8, 2021
@tomholub tomholub deleted the feature/issue-309-backups branch July 8, 2021 10:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix wasteful backup search in inbox on Gmail API

2 participants