Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions extension/chrome/elements/add_pubkey.htm
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@

<div id="content" class="add_pubkey dialog">
<div class="line">
<h1>Add a pubkey to email address</h1>
<h1>Add a public key to email address</h1>
</div>
<div class="separator"></div>
<div class="line">
Expand All @@ -37,7 +37,7 @@ <h1>Add a pubkey to email address</h1>
<div class="line">
<textarea class="pubkey" placeholder="ASCII Armored Public Key" spellcheck="false" data-test="input-pubkey"></textarea>
</div>
<div class="line orange">
<div class="line orange" id="manual-import-warning" data-test="manual-import-warning">
Manually importing Public Keys can be dangerous
</div>
<div class="line">
Expand All @@ -57,4 +57,4 @@ <h1>Add a pubkey to email address</h1>
<script src="add_pubkey.js" type="module"></script>
</body>

</html>
</html>
34 changes: 27 additions & 7 deletions extension/chrome/elements/add_pubkey.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,16 @@ View.run(class AddPubkeyView extends View {
public render = async () => {
Ui.event.protect();
for (const missingPubkeyEmail of this.missingPubkeyEmails) {
Xss.sanitizeAppend('select.email', `<option value="${Xss.escape(missingPubkeyEmail)}">${Xss.escape(missingPubkeyEmail)}</option>`);
const escapedMissingPubkeyEmail = Xss.escape(missingPubkeyEmail);
Xss.sanitizeAppend('select.email', `<option value="${escapedMissingPubkeyEmail}">${escapedMissingPubkeyEmail}</option>`);
}
const uniqueEmails = new Set<string>();
for (const contact of await ContactStore.search(undefined, { hasPgp: true })) {
Xss.sanitizeAppend('select.copy_from_email', `<option value="${Xss.escape(contact.email)}">${Xss.escape(contact.email)}</option>`);
uniqueEmails.add(contact.email);
}
for (const email of Array.from(uniqueEmails).sort()) {
const escapedEmail = Xss.escape(email);
Xss.sanitizeAppend('select.copy_from_email', `<option value="${escapedEmail}">${escapedEmail}</option>`);
Copy link
Contributor

Choose a reason for hiding this comment

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

That's the first part, all right.
And where is the second part, as specified by Tom's comment?

Then all public keys of that recipient should be copied over for the other recipient.

I suggest to not copy revoked keys, ContactStore.getEncryptionKeys would be enough.
Also, <textarea class="pubkey" should be invisible when an email from "Copy From Contact" is selected, as it isn't appropriate for multiple pubkeys (we discussed that we'll no longer use ContactStore.get here)

Copy link
Contributor Author

@IvanPizhenko IvanPizhenko Feb 13, 2022

Choose a reason for hiding this comment

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

@rrrooommmaaa Please recheck now

}
this.fetchKeyUi.handleOnPaste($('.pubkey'));
$('.action_settings').click(this.setHandler(async () => await Browser.openSettingsPage('index.htm', this.acctEmail, '/chrome/settings/modules/contacts.htm')));
Expand All @@ -53,6 +59,7 @@ View.run(class AddPubkeyView extends View {
if (errs.length) {
await Ui.modal.warning(`some keys could not be processed due to errors:\n${errs.map(e => `-> ${e.message}\n`).join('')}`);
}
$('.copy_from_email').val('');
$('.pubkey').val(String(KeyUtil.armor(keys[0])));
$('.action_ok').trigger('click');
} else if (errs.length) {
Expand All @@ -73,21 +80,34 @@ View.run(class AddPubkeyView extends View {
if ($(fromSelect).val()) {
const [contact] = await ContactStore.get(undefined, [String($(fromSelect).val())]);
if (contact?.pubkey) {
$('.pubkey').val(KeyUtil.armor(contact.pubkey)).prop('disabled', true);
$('.pubkey').val('').prop('disabled', true).prop('style', 'display: none;');
$('#manual-import-warning').prop('style', 'display: none;');
} else {
Catch.report('Contact unexpectedly not found when copying pubkey by email in add_pubkey.htm');
await Ui.modal.error('Contact not found.');
}
} else {
$('.pubkey').val('').prop('disabled', false);
$('.pubkey').val('').prop('disabled', false).prop('style', 'display: inline;');
$('#manual-import-warning').prop('style', 'display: inline;');
}
};

private submitHandler = async () => {
try {
const keyImportUi = new KeyImportUi({ checkEncryption: true });
const normalized = await keyImportUi.checkPub(String($('.pubkey').val()));
await ContactStore.update(undefined, String($('select.email').val()), { pubkey: normalized });
const email = String($('select.email').val());
if ($('.copy_from_email').val()) {
const fromEmail = String($('.copy_from_email').val());
const keys = await ContactStore.getEncryptionKeys(undefined, [fromEmail]);
for (const keyArray of keys) {
for (const key of keyArray.keys) {
await ContactStore.update(undefined, email, { pubkey: key });
}
}
} else {
const keyImportUi = new KeyImportUi({ checkEncryption: true });
const normalized = await keyImportUi.checkPub(String($('.pubkey').val()));
await ContactStore.update(undefined, email, { pubkey: normalized });
}
this.closeDialog();
} catch (e) {
if (e instanceof UserAlert) {
Expand Down
5 changes: 3 additions & 2 deletions extension/js/common/ui/key-import-ui.ts
Original file line number Diff line number Diff line change
Expand Up @@ -246,10 +246,11 @@ export class KeyImportUi {
if (KeyUtil.getKeyType(armored) !== 'openpgp') {
return { normalized: armored };
}
const headers = PgpArmor.headers(type);
const normalized = await KeyUtil.normalize(armored);
if (!normalized) {
throw new UserAlert('There was an error processing this key, possibly due to bad formatting.\nPlease insert complete key, including "' + headers.begin + '" and "' + headers.end + '"');
const headers = PgpArmor.headers(type);
throw new UserAlert('There was an error processing this key, possibly due to bad formatting.\n' +
`Please insert complete key, including "${headers.begin}" and "${headers.end}".`);
}
return normalized;
};
Expand Down
2 changes: 1 addition & 1 deletion test/source/mock/google/google-endpoints.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import { oauth } from '../lib/oauth';

type DraftSaveModel = { message: { raw: string, threadId: string } };

const allowedRecipients: Array<string> = ['flowcrypt.compatibility@gmail.com', 'human+manualcopypgp@flowcrypt.com',
const allowedRecipients: Array<string> = ['flowcrypt.compatibility@gmail.com', 'manualcopypgp@flowcrypt.com',
'censored@email.com', 'test@email.com', 'human@flowcrypt.com', 'human+nopgp@flowcrypt.com', 'expired.on.attester@domain.com',
'ci.tests.gmail@flowcrypt.test', 'smime1@recipient.com', 'smime2@recipient.com', 'smime@recipient.com',
'smime.attachment@recipient.com', 'auto.refresh.expired.key@recipient.com', 'to@example.com', 'cc@example.com', 'bcc@example.com',
Expand Down
35 changes: 26 additions & 9 deletions test/source/tests/compose.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import { SetupPageRecipe } from './page-recipe/setup-page-recipe';
import { testConstants } from './tooling/consts';
import { MsgUtil } from '../core/crypto/pgp/msg-util';
import { Buf } from '../core/buf';
import { PubkeyInfoWithLastCheck } from '../core/crypto/key';

// tslint:disable:no-blank-lines-func
// tslint:disable:no-unused-expression
Expand Down Expand Up @@ -269,25 +270,41 @@ export const defineComposeTests = (testVariant: TestVariant, testWithBrowser: Te
}));

ava.default('compose - settings - manually copied pubkey', testWithBrowser('ci.tests.gmail', async (t, browser) => {
const dbPage = await browser.newPage(t, TestUrls.extension('chrome/dev/ci_unit_test.htm'));
// add a contact containing 2 pubkeys to the storage
await dbPage.page.evaluate(async (pubkeys: string[]) => {
for (const pubkey of pubkeys) {
const key = await (window as any).KeyUtil.parse(pubkey);
await (window as any).ContactStore.update(undefined, 'tocopyfrom@example.test', { pubkey: key });
}
}, [testConstants.abcddfTestComPubkey, testConstants.abcdefTestComPubkey]);
const inboxPage = await browser.newPage(t, TestUrls.extensionInbox('ci.tests.gmail@flowcrypt.test'));
let composeFrame = await InboxPageRecipe.openAndGetComposeFrame(inboxPage);
await ComposePageRecipe.fillMsg(composeFrame, { to: 'human@flowcrypt.com' }, 'just to load - will close this page');
await Util.sleep(2); // todo: should wait until actually loaded
await composeFrame.waitAndClick('@action-close-new-message');
await inboxPage.waitTillGone('@container-new-message');
composeFrame = await InboxPageRecipe.openAndGetComposeFrame(inboxPage);
await ComposePageRecipe.fillMsg(composeFrame, { to: 'human+manualcopypgp@flowcrypt.com' }, 'manual copied key');
const composeFrame = await InboxPageRecipe.openAndGetComposeFrame(inboxPage);
await ComposePageRecipe.fillMsg(composeFrame, { to: 'manualcopypgp@flowcrypt.com' }, 'manual copied key');
await composeFrame.waitAndClick('@action-open-add-pubkey-dialog', { delay: 1 });
await inboxPage.waitAll('@dialog-add-pubkey');
const addPubkeyDialog = await inboxPage.getFrame(['add_pubkey.htm']);
await addPubkeyDialog.waitAll('@input-select-copy-from');
await Util.sleep(1);
await addPubkeyDialog.selectOption('@input-select-copy-from', 'human@flowcrypt.com');
await Util.sleep(1);
await addPubkeyDialog.selectOption('@input-select-copy-from', 'tocopyfrom@example.test');
Copy link
Contributor

Choose a reason for hiding this comment

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

@IvanPizhenko I added filling the contact store source contact with pubkeys
and testing the resulting contact in the contact store via dbPage.
It would also be good to test that @input-pubkey textarea is visible before selecting the option and invisible right after.
And I guess "Manually importing Public Keys can be dangerous" only applies by pasting a pubkey text, but that's more a question for @tomholub

Copy link
Contributor Author

@IvanPizhenko IvanPizhenko Feb 17, 2022

Choose a reason for hiding this comment

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

@rrrooommmaaa I've updated test to check appearance/disapperance of the @input-pubkey and show/hide warning message "Manually importing...".

await addPubkeyDialog.waitTillGone(['@input-pubkey', '@manual-import-warning']);
await addPubkeyDialog.selectOption('@input-select-copy-from', 'Copy from Contact');
await addPubkeyDialog.waitAll(['@input-pubkey', '@manual-import-warning']);
await addPubkeyDialog.selectOption('@input-select-copy-from', 'tocopyfrom@example.test');
await addPubkeyDialog.waitTillGone(['@input-pubkey', '@manual-import-warning']);
await addPubkeyDialog.waitAndClick('@action-add-pubkey');
await inboxPage.waitTillGone('@dialog-add-pubkey');
await composeFrame.waitAndClick('@action-send', { delay: 2 });
await inboxPage.waitTillGone('@container-new-message');
await inboxPage.close();
// test the pubkeys we copied
const contact = await dbPage.page.evaluate(async () => {
return await (window as any).ContactStore.getOneWithAllPubkeys(undefined, 'manualcopypgp@flowcrypt.com');
});
expect(contact.sortedPubkeys.length).to.equal(2);
expect((contact.sortedPubkeys as PubkeyInfoWithLastCheck[]).map(pub => pub.pubkey.id)).to.include.members(
['6CF53D2329C2A80828F499D375AA44AB8930F7E9', '3155F118B6E732B3638A1CE1608BCD797A23FB91']);
await dbPage.close();
}));

ava.default('compose - keyboard - Ctrl+Enter sends message', testWithBrowser('ci.tests.gmail', async (t, browser) => {
Expand Down