Skip to content

Commit e4973b4

Browse files
issue #3895 Show all fingerprints for recipient (#4003)
* issue 3895 Show all fingerprints for recipient * move to flaky tests * Fix filtering expired keys * separate valid, revoked and expired in one pass * Move common methods to the class ComposePageRecipe Co-authored-by: Ivan Pizhenko <IvanPizhenko@users.noreply.github.com> Co-authored-by: Roman Shevchenko <rrrooommmaaa@mail.ru>
1 parent 07b2221 commit e4973b4

File tree

10 files changed

+11415
-257
lines changed

10 files changed

+11415
-257
lines changed

.gitignore

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,3 +18,5 @@
1818
release/log.txt
1919
release/*
2020
npm-debug.log
21+
/rt.sh
22+
/grt.sh

extension/chrome/elements/compose-modules/compose-recipients-module.ts

Lines changed: 91 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
'use strict';
44

55
import { ChunkedCb, EmailProviderContact, RecipientType } from '../../../js/common/api/shared/api.js';
6-
import { Contact } from '../../../js/common/core/crypto/key.js';
6+
import { Contact, KeyUtil } from '../../../js/common/core/crypto/key.js';
77
import { PUBKEY_LOOKUP_RESULT_FAIL, PUBKEY_LOOKUP_RESULT_WRONG } from './compose-err-module.js';
88
import { ProviderContactsQuery, Recipients } from '../../../js/common/api/email-provider/email-provider-api.js';
99
import { RecipientElement, RecipientStatus } from './compose-types.js';
@@ -20,7 +20,7 @@ import { moveElementInArray } from '../../../js/common/platform/util.js';
2020
import { ViewModule } from '../../../js/common/view-module.js';
2121
import { ComposeView } from '../compose.js';
2222
import { AcctStore } from '../../../js/common/platform/store/acct-store.js';
23-
import { ContactPreview, ContactStore, ContactUpdate } from '../../../js/common/platform/store/contact-store.js';
23+
import { ContactPreview, ContactStore, ContactUpdate, PubkeyInfo } from '../../../js/common/platform/store/contact-store.js';
2424

2525
/**
2626
* todo - this class is getting too big
@@ -84,7 +84,9 @@ export class ComposeRecipientsModule extends ViewModule<ComposeView> {
8484
this.view.S.cached('compose_table').click(this.view.setHandler(() => this.hideContacts(), this.view.errModule.handle(`hide contact box`)));
8585
this.view.S.cached('add_their_pubkey').click(this.view.setHandler(() => this.addTheirPubkeyClickHandler(), this.view.errModule.handle('add pubkey')));
8686
BrowserMsg.addListener('addToContacts', this.checkReciepientsKeys);
87-
BrowserMsg.addListener('reRenderRecipient', async ({ contact }: Bm.ReRenderRecipient) => await this.reRenderRecipientFor(contact));
87+
BrowserMsg.addListener('reRenderRecipient', async ({ contact }: Bm.ReRenderRecipient) => {
88+
await this.reRenderRecipientFor(contact.email);
89+
});
8890
BrowserMsg.listen(this.view.parentTabId);
8991
}
9092

@@ -196,13 +198,18 @@ export class ComposeRecipientsModule extends ViewModule<ComposeView> {
196198
this.view.S.now('send_btn_text').text(this.BTN_LOADING);
197199
this.view.sizeModule.setInputTextHeightManuallyIfNeeded();
198200
recipient.evaluating = (async () => {
199-
let pubkeyLookupRes: Contact | 'fail' | 'wrong';
201+
let pubkeyLookupRes: PubkeyInfo[] | 'fail' | 'wrong' = 'wrong';
202+
// console.log(`>>>> evaluateRecipients: ${JSON.stringify(recipient)}`);
200203
if (recipient.status !== RecipientStatus.WRONG) {
201-
pubkeyLookupRes = await this.view.storageModule.lookupPubkeyFromKeyserversThenOptionallyFetchExpiredByFingerprintAndUpsertDb(recipient.email, undefined);
204+
pubkeyLookupRes = await this.view.storageModule.
205+
lookupPubkeyFromKeyserversThenOptionallyFetchExpiredByFingerprintAndUpsertDb(
206+
recipient.email, undefined);
207+
}
208+
if (pubkeyLookupRes === 'fail' || pubkeyLookupRes === 'wrong') {
209+
await this.renderPubkeyResult(recipient, pubkeyLookupRes);
202210
} else {
203-
pubkeyLookupRes = 'wrong';
211+
await this.renderPubkeyResult(recipient, pubkeyLookupRes);
204212
}
205-
await this.renderPubkeyResult(recipient, pubkeyLookupRes);
206213
recipient.evaluating = undefined; // Clear promise when it finished
207214
})();
208215
}
@@ -336,10 +343,14 @@ export class ComposeRecipientsModule extends ViewModule<ComposeView> {
336343
}
337344
}
338345

339-
public reRenderRecipientFor = async (contact: Contact): Promise<void> => {
340-
for (const recipient of this.addedRecipients.filter(r => r.email === contact.email)) {
341-
this.view.errModule.debug(`re-rendering recipient: ${contact.email}`);
342-
await this.renderPubkeyResult(recipient, contact);
346+
public reRenderRecipientFor = async (email: string): Promise<void> => {
347+
if (this.addedRecipients.every(r => r.email !== email)) {
348+
return;
349+
}
350+
const emailAndPubkeys = await ContactStore.getOneWithAllPubkeys(undefined, email);
351+
for (const recipient of this.addedRecipients.filter(r => r.email === email)) {
352+
this.view.errModule.debug(`re-rendering recipient: ${email}`);
353+
await this.renderPubkeyResult(recipient, emailAndPubkeys ? emailAndPubkeys.sortedPubkeys : []);
343354
this.view.recipientsModule.showHideCcAndBccInputsIfNeeded();
344355
await this.view.recipientsModule.setEmailsPreview(this.getRecipients());
345356
}
@@ -753,11 +764,8 @@ export class ComposeRecipientsModule extends ViewModule<ComposeView> {
753764
toLookup.push(contact);
754765
}
755766
}
756-
await Promise.all(toLookup.map(c => this.view.storageModule.lookupPubkeyFromKeyserversAndUpsertDb(c.email, c.name || undefined, undefined).then(lookupRes => {
757-
if (lookupRes === 'fail') {
758-
this.failedLookupEmails.push(c.email);
759-
}
760-
})));
767+
await Promise.all(toLookup.map(c => this.view.storageModule.lookupPubkeyFromKeyserversAndUpsertDb(
768+
c.email, c.name || undefined).catch(() => this.failedLookupEmails.push(c.email))));
761769
}
762770

763771
private renderSearchResultsLoadingDone = () => {
@@ -798,58 +806,74 @@ export class ComposeRecipientsModule extends ViewModule<ComposeView> {
798806
}
799807

800808
private checkReciepientsKeys = async () => {
801-
for (const recipientEl of this.addedRecipients.filter(r => r.element.className.includes('no_pgp'))) {
809+
for (const recipientEl of this.addedRecipients.filter(
810+
r => r.element.className.includes('no_pgp'))) {
802811
const email = $(recipientEl).text().trim();
803-
const [dbContact] = await ContactStore.get(undefined, [email]);
804-
if (dbContact) {
812+
const dbContacts = await ContactStore.getOneWithAllPubkeys(undefined, email);
813+
if (dbContacts && dbContacts.sortedPubkeys && dbContacts.sortedPubkeys.length) {
805814
recipientEl.element.classList.remove('no_pgp');
806-
await this.renderPubkeyResult(recipientEl, dbContact);
815+
await this.renderPubkeyResult(recipientEl, dbContacts.sortedPubkeys);
807816
}
808817
}
809818
}
810819

811-
private renderPubkeyResult = async (recipient: RecipientElement, contact: Contact | 'fail' | 'wrong') => {
820+
private renderPubkeyResult = async (
821+
recipient: RecipientElement, sortedPubkeyInfos: PubkeyInfo[] | 'fail' | 'wrong'
822+
) => {
823+
// console.log(`>>>> renderPubkeyResult: ${JSON.stringify(sortedPubkeyInfos)}`);
812824
const el = recipient.element;
813825
this.view.errModule.debug(`renderPubkeyResult.emailEl(${String(recipient.email)})`);
814826
this.view.errModule.debug(`renderPubkeyResult.email(${recipient.email})`);
815-
this.view.errModule.debug(`renderPubkeyResult.contact(${JSON.stringify(contact)})`);
827+
this.view.errModule.debug(`renderPubkeyResult.contact(${JSON.stringify(sortedPubkeyInfos)})`);
816828
$(el).children('img, i').remove();
817829
const contentHtml = '<img src="/img/svgs/close-icon.svg" alt="close" class="close-icon svg" />' +
818830
'<img src="/img/svgs/close-icon-black.svg" alt="close" class="close-icon svg display_when_sign" />';
819831
Xss.sanitizeAppend(el, contentHtml)
820832
.find('img.close-icon')
821833
.click(this.view.setHandler(target => this.removeRecipient(target.parentElement!), this.view.errModule.handle('remove recipient')));
822834
$(el).removeClass(['failed', 'wrong', 'has_pgp', 'no_pgp', 'expired']);
823-
if (contact === PUBKEY_LOOKUP_RESULT_FAIL) {
835+
if (sortedPubkeyInfos === PUBKEY_LOOKUP_RESULT_FAIL) {
824836
recipient.status = RecipientStatus.FAILED;
825837
$(el).attr('title', 'Failed to load, click to retry');
826838
$(el).addClass("failed");
827839
Xss.sanitizeReplace($(el).children('img:visible'), '<img src="/img/svgs/repeat-icon.svg" class="repeat-icon action_retry_pubkey_fetch">' +
828840
'<img src="/img/svgs/close-icon-black.svg" class="close-icon-black svg remove-reciepient">');
829841
$(el).find('.action_retry_pubkey_fetch').click(this.view.setHandler(async () => await this.refreshRecipients(), this.view.errModule.handle('refresh recipient')));
830842
$(el).find('.remove-reciepient').click(this.view.setHandler(element => this.removeRecipient(element.parentElement!), this.view.errModule.handle('remove recipient')));
831-
} else if (contact === PUBKEY_LOOKUP_RESULT_WRONG) {
843+
} else if (sortedPubkeyInfos === PUBKEY_LOOKUP_RESULT_WRONG) {
832844
recipient.status = RecipientStatus.WRONG;
833845
this.view.errModule.debug(`renderPubkeyResult: Setting email to wrong / misspelled in harsh mode: ${recipient.email}`);
834846
$(el).attr('title', 'This email address looks misspelled. Please try again.');
835847
$(el).addClass("wrong");
836-
} else if (contact.pubkey && ((contact.expiresOn || Infinity) <= Date.now() || contact.pubkey.usableForEncryptionButExpired)) {
837-
recipient.status = RecipientStatus.EXPIRED;
838-
$(el).addClass("expired");
839-
Xss.sanitizePrepend(el, '<img src="/img/svgs/expired-timer.svg" class="revoked-or-expired">');
840-
$(el).attr('title', 'Does use encryption but their public key is expired. You should ask them to send ' +
841-
'you an updated public key.' + this.recipientKeyIdText(contact));
842-
} else if (contact.revoked) {
843-
recipient.status = RecipientStatus.REVOKED;
844-
$(el).addClass("revoked");
845-
Xss.sanitizePrepend(el, '<img src="/img/svgs/revoked.svg" class="revoked-or-expired">');
846-
$(el).attr('title', 'Does use encryption but their public key is revoked. You should ask them to send ' +
847-
'you an updated public key.' + this.recipientKeyIdText(contact));
848-
} else if (contact.pubkey) {
849-
recipient.status = RecipientStatus.HAS_PGP;
850-
$(el).addClass('has_pgp');
851-
Xss.sanitizePrepend(el, '<img class="lock-icon" src="/img/svgs/locked-icon.svg" />');
852-
$(el).attr('title', 'Does use encryption' + this.recipientKeyIdText(contact));
848+
} else if (sortedPubkeyInfos.length) {
849+
// New logic:
850+
// 1. Keys are sorted in a special way.
851+
// 2. If there is at least one key:
852+
// - if first key is valid (non-expired, non-revoked) public key, then it's HAS_PGP.
853+
// - else if first key is revoked, then REVOKED.
854+
// - else EXPIRED.
855+
// 3. Otherwise NO_PGP.
856+
const firstKeyInfo = sortedPubkeyInfos[0];
857+
if (!firstKeyInfo.revoked && !KeyUtil.expired(firstKeyInfo.pubkey)) {
858+
recipient.status = RecipientStatus.HAS_PGP;
859+
$(el).addClass('has_pgp');
860+
Xss.sanitizePrepend(el, '<img class="lock-icon" src="/img/svgs/locked-icon.svg" />');
861+
$(el).attr('title', 'Does use encryption\n\n' + this.formatPubkeysHintText(sortedPubkeyInfos));
862+
} else if (firstKeyInfo.revoked) {
863+
recipient.status = RecipientStatus.REVOKED;
864+
$(el).addClass("revoked");
865+
Xss.sanitizePrepend(el, '<img src="/img/svgs/revoked.svg" class="revoked-or-expired">');
866+
$(el).attr('title', 'Does use encryption but their public key is revoked. ' +
867+
'You should ask them to send you an updated public key.\n\n' +
868+
this.formatPubkeysHintText(sortedPubkeyInfos));
869+
} else {
870+
recipient.status = RecipientStatus.EXPIRED;
871+
$(el).addClass("expired");
872+
Xss.sanitizePrepend(el, '<img src="/img/svgs/expired-timer.svg" class="revoked-or-expired">');
873+
$(el).attr('title', 'Does use encryption but their public key is expired. ' +
874+
'You should ask them to send you an updated public key.\n\n' +
875+
this.formatPubkeysHintText(sortedPubkeyInfos));
876+
}
853877
} else {
854878
recipient.status = RecipientStatus.NO_PGP;
855879
$(el).addClass("no_pgp");
@@ -860,6 +884,30 @@ export class ComposeRecipientsModule extends ViewModule<ComposeView> {
860884
this.view.myPubkeyModule.reevaluateShouldAttachOrNot();
861885
}
862886

887+
private formatPubkeysHintText = (pubkeyInfos: PubkeyInfo[]): string => {
888+
const valid: PubkeyInfo[] = [];
889+
const expired: PubkeyInfo[] = [];
890+
const revoked: PubkeyInfo[] = [];
891+
for (const pubkeyInfo of pubkeyInfos) {
892+
if (pubkeyInfo.revoked) {
893+
revoked.push(pubkeyInfo);
894+
} else if (KeyUtil.expired(pubkeyInfo.pubkey)) {
895+
expired.push(pubkeyInfo);
896+
} else {
897+
valid.push(pubkeyInfo);
898+
}
899+
}
900+
return [
901+
{ groupName: 'Valid public key fingerprints:', pubkeyInfos: valid },
902+
{ groupName: 'Expired public key fingerprints:', pubkeyInfos: expired },
903+
{ groupName: 'Revoked public key fingerprints:', pubkeyInfos: revoked }
904+
].filter(g => g.pubkeyInfos.length).map(g => this.formatKeyGroup(g.groupName, g.pubkeyInfos)).join('\n\n');
905+
}
906+
907+
private formatKeyGroup = (groupName: string, pubkeyInfos: PubkeyInfo[]): string => {
908+
return [groupName, ...pubkeyInfos.map(info => this.formatPubkeyId(info))].join('\n');
909+
}
910+
863911
private removeRecipient = (element: HTMLElement) => {
864912
const index = this.addedRecipients.findIndex(r => r.element.isEqualNode(element));
865913
this.addedRecipients[index].element.remove();
@@ -878,12 +926,8 @@ export class ComposeRecipientsModule extends ViewModule<ComposeView> {
878926
await this.reEvaluateRecipients(failedRecipients);
879927
}
880928

881-
private recipientKeyIdText = (contact: Contact) => {
882-
if (contact.fingerprint) {
883-
return `\n\nRecipient public key fingerprint:\n${Str.spaced(contact.fingerprint)}`;
884-
} else {
885-
return '';
886-
}
929+
private formatPubkeyId = (pubkeyInfo: PubkeyInfo): string => {
930+
return `${Str.spaced(pubkeyInfo.pubkey.id)} (${pubkeyInfo.pubkey.type})`;
887931
}
888932

889933
private generateRecipientId = (): string => {

0 commit comments

Comments
 (0)