Skip to content

#6199 Improve user backup detection#6202

Open
martgil wants to merge 8 commits intomasterfrom
issue-6199-fix-improper-backup-detection
Open

#6199 Improve user backup detection#6202
martgil wants to merge 8 commits intomasterfrom
issue-6199-fix-improper-backup-detection

Conversation

@martgil
Copy link
Copy Markdown
Collaborator

@martgil martgil commented Mar 27, 2026

This PR improves user's backup detection rendered in Gmail.

Closes #6199


Tests (delete all except exactly one):

  • Tests added or updated

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

@martgil martgil requested a review from sosnovsky as a code owner March 27, 2026 10:48
@martgil martgil marked this pull request as draft March 27, 2026 10:51
@martgil
Copy link
Copy Markdown
Collaborator Author

martgil commented Mar 27, 2026

wip: adding a test

@martgil martgil marked this pull request as ready for review April 1, 2026 06:47
@martgil
Copy link
Copy Markdown
Collaborator Author

martgil commented Apr 1, 2026

Hi @sosnovsky - This one is ready for a review. Thank you!

@martgil martgil marked this pull request as draft April 3, 2026 08:25
@martgil martgil requested a review from sosnovsky April 9, 2026 09:24
@martgil martgil marked this pull request as ready for review April 9, 2026 09:24
Comment on lines 60 to +61
if (notUserOwnedPrvKey) {
$('.backup_message_text').text(
'This message contains a private key received from ' +
this.fromEmail +
'. Import only if you intentionally sent this to yourself or received it from your administrator.'
);
if (notUserOwnedPrvKey) {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

duplicated if (notUserOwnedPrvKey) { :)

`This private key with fingerprint <span class="green">${Xss.escape(Str.spaced(fingerprint))}</span> has already been imported.`
);
} else {
const notUserOwnedPrvKey = this.fromEmail !== this.acctEmail;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

what about user email aliases? here we check only main user email, but email aliases will probably show warning message. in MessageRenderer we have isOutgoing method which checks array of sendAs aliases, I think we should use similar check here too:

return Boolean(senderEmail && this.sendAsAliases.has(senderEmail));

public srcBackupIframe = (armoredPrvBackup: string) => {
return this.frameSrc(this.extUrl('chrome/elements/backup.htm'), { frameId: this.newId(), armoredPrvBackup });
public srcBackupIframe = (armoredPrvBackup: string, fromEmail?: string) => {
return this.frameSrc(this.extUrl('chrome/elements/backup.htm'), { frameId: this.newId(), armoredPrvBackup, fromEmail });
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

fromEmail is required property in backup.ts (https://github.com/FlowCrypt/flowcrypt-browser/blob/issue-6199-fix-improper-backup-detection/extension/chrome/elements/backup.ts#L22), but here optional value fromEmail?: string is passed.
what happens if undefined value is passed? backup.htm will probably fail to render, could you please check?

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.

Improper recognition of user backup keys

2 participants