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
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,17 @@
'use strict';

import { ApiErr } from '../../../js/common/api/shared/api-error.js';
import { KeyInfo } from '../../../js/common/core/crypto/key.js';
import { KeyInfo, KeyUtil } from '../../../js/common/core/crypto/key.js';
import { Lang } from '../../../js/common/lang.js';
import { Ui } from '../../../js/common/browser/ui.js';
import { ViewModule } from '../../../js/common/view-module.js';
import { ComposeView } from '../compose.js';
import { ContactStore } from '../../../js/common/platform/store/contact-store.js';
import { Str } from '../../../js/common/core/common.js';

export class ComposeMyPubkeyModule extends ViewModule<ComposeView> {

private toggledManually = false;
private wkdFingerprints: { [acctEmail: string]: string[] } = {};

public setHandlers = () => {
this.view.S.cached('icon_pubkey').attr('title', Lang.compose.includePubkeyIconTitle);
Expand Down Expand Up @@ -44,18 +45,40 @@ export class ComposeMyPubkeyModule extends ViewModule<ComposeView> {
return;
}
(async () => {
const contacts = await ContactStore.get(undefined, this.view.recipientsModule.getRecipients().map(r => r.email));
for (const contact of contacts) {
if (contact?.has_pgp && contact.client !== 'cryptup') {
const senderEmail = this.view.senderModule.getSender();
const senderKi = await this.view.storageModule.getKey(senderEmail);
const primaryFingerprint = (await KeyUtil.parse(senderKi.private)).id;
// if we have cashed this fingerprint, setAttachPreference(false) rightaway and return
const cached = this.wkdFingerprints[senderEmail];
if (Array.isArray(cached) && cached.includes(primaryFingerprint)) {
this.setAttachPreference(false);
return;
}
const myDomain = Str.getDomainFromEmailAddress(senderEmail);
const foreignRecipients = this.view.recipientsModule.getRecipients().map(r => r.email)
.filter(Boolean)
.filter(email => myDomain !== Str.getDomainFromEmailAddress(email));
if (foreignRecipients.length > 0) {
if (!Array.isArray(cached)) {
// slow operation -- test WKD for our own key and cache the result
const { keys } = await this.view.pubLookup.wkd.rawLookupEmail(senderEmail);
const fingerprints = keys.map(key => key.id);
this.wkdFingerprints[senderEmail] = fingerprints;
if (fingerprints.includes(primaryFingerprint)) {
this.setAttachPreference(false);
return;
}
}
for (const recipient of foreignRecipients) {
// new message, and my key is not uploaded where the recipient would look for it
if (! await this.view.recipientsModule.doesRecipientHaveMyPubkey(contact.email)) {
// either don't know if they need pubkey (can_read_emails false), or they do need pubkey
if (! await this.view.recipientsModule.doesRecipientHaveMyPubkey(recipient)) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

doesRecipientHaveMyPubkey seems like it would be better to place it in this file and not in recipients module, but hard to say for sure without opening the IDE, and not overly important

// they do need pubkey
this.setAttachPreference(true);
return;
}
}
this.setAttachPreference(false);
}
this.setAttachPreference(false);
})().catch(ApiErr.reportIfSignificant);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { Contact } from '../../../js/common/core/crypto/key.js';
import { PUBKEY_LOOKUP_RESULT_FAIL, PUBKEY_LOOKUP_RESULT_WRONG } from './compose-err-module.js';
import { ProviderContactsQuery, Recipients } from '../../../js/common/api/email-provider/email-provider-api.js';
import { RecipientElement, RecipientStatus } from './compose-types.js';
import { Str, Value } from '../../../js/common/core/common.js';
import { Str } from '../../../js/common/core/common.js';
import { ApiErr } from '../../../js/common/api/shared/api-error.js';
import { Bm, BrowserMsg } from '../../../js/common/browser/browser-msg.js';
import { Catch } from '../../../js/common/platform/catch.js';
Expand Down Expand Up @@ -38,8 +38,6 @@ export class ComposeRecipientsModule extends ViewModule<ComposeView> {
private contactSearchInProgress = false;
private addedPubkeyDbLookupInterval?: number;

private recipientsMissingMyKey: string[] = [];

private onRecipientAddedCallbacks: ((rec: RecipientElement[]) => void)[] = [];

private dragged: Element | undefined = undefined;
Expand Down Expand Up @@ -300,6 +298,7 @@ export class ComposeRecipientsModule extends ViewModule<ComposeView> {
this.onRecipientAddedCallbacks.push(callback);
}

// todo: shouldn't we check longid?
public doesRecipientHaveMyPubkey = async (theirEmailUnchecked: string): Promise<boolean | undefined> => {
const theirEmail = Str.parseEmail(theirEmailUnchecked).email;
if (!theirEmail) {
Expand All @@ -316,7 +315,7 @@ export class ComposeRecipientsModule extends ViewModule<ComposeView> {
const qReceivedMsg = `from:${theirEmail} "BEGIN PGP MESSAGE" "END PGP MESSAGE"`;
try {
const response = await this.view.emailProvider.msgList(`(${qSentPubkey}) OR (${qReceivedMsg})`, true);
if (response.messages) {
if (response.messages && response.messages.length > 0) {
await AcctStore.set(this.view.acctEmail, { pubkey_sent_to: (storage.pubkey_sent_to || []).concat(theirEmail) });
return true;
} else {
Expand Down Expand Up @@ -857,7 +856,6 @@ export class ComposeRecipientsModule extends ViewModule<ComposeView> {
}

private removeRecipient = (element: HTMLElement) => {
this.recipientsMissingMyKey = Value.arr.withoutVal(this.recipientsMissingMyKey, $(element).parent().text());
const index = this.addedRecipients.findIndex(r => r.element.isEqualNode(element));
this.addedRecipients[index].element.remove();
const container = element.parentElement?.parentElement; // Get Container, e.g. '.input-container-cc'
Expand Down
2 changes: 1 addition & 1 deletion extension/chrome/elements/compose.htm
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ <h1 id="header_title" data-test="header-title">New Secure Message</h1>
<div id="send_btn_note" data-test="send-btn-note"></div>
<button class="icon right delete_draft" tabindex="11" title="Delete draft"><img src="/img/svgs/trash-can-icon.svg"
class="delete-draft" alt="Delete Email"></button>
<button class="icon right action_include_pubkey" tabindex="10" title="Include your Public Key"><i
<button class="icon right action_include_pubkey" data-test="action-include-pubkey" tabindex="10" title="Include your Public Key"><i
alt="pubkey"></i></button>
</td>
</tr>
Expand Down
4 changes: 2 additions & 2 deletions extension/js/common/api/email-provider/gmail/google-auth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
// tslint:disable:oneliner-object-literal

import { GOOGLE_API_HOST, GOOGLE_OAUTH_SCREEN_HOST, FLAVOR } from '../../../core/const.js';
import { Url, Value } from '../../../core/common.js';
import { Str, Url, Value } from '../../../core/common.js';
import { tabsQuery, windowsCreate } from '../../../browser/chrome.js';
import { Api } from './../../shared/api.js';
import { ApiErr } from '../../shared/api-error.js';
Expand Down Expand Up @@ -158,7 +158,7 @@ export class GoogleAuth {
* Happens on enterprise builds
*/
public static isFesUnreachableErr = (e: any, email: string): boolean => {
const domain = email.split('@')[1].toLowerCase();
const domain = Str.getDomainFromEmailAddress(email);
const errString = String(e);
if (errString.includes(`-1 when GET-ing https://${domain}/.well-known/host-meta.json`)) {
return true; // err trying to get FES url from .well-known
Expand Down
25 changes: 16 additions & 9 deletions extension/js/common/api/key-server/wkd.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@ import { ApiErr } from '../shared/api-error.js';
import { opgp } from '../../core/crypto/pgp/openpgpjs-custom.js';
import { Buf } from '../../core/buf.js';
import { PubkeySearchResult } from './../pub-lookup.js';
import { KeyUtil } from '../../core/crypto/key.js';
import { Key, KeyUtil } from '../../core/crypto/key.js';
import { Str } from '../../core/common.js';

// tslint:disable:no-null-keyword
// tslint:disable:no-direct-ajax
Expand All @@ -24,21 +25,23 @@ export class Wkd extends Api {
super();
}

public lookupEmail = async (email: string): Promise<PubkeySearchResult> => {
// returns all the received keys
public rawLookupEmail = async (email: string): Promise<{ keys: Key[], errs: Error[] }> => {
// todo: should we return errs on network failures etc.?
const parts = email.split('@');
if (parts.length !== 2) {
return { pubkey: null, pgpClient: null };
return { keys: [], errs: [] };
}
const [user, recipientDomain] = parts;
if (!user || !recipientDomain) {
return { pubkey: null, pgpClient: null };
return { keys: [], errs: [] };
}
if (!opgp) {
// pgp_block.htm does not have openpgp loaded
// the particular usecase (auto-loading pubkeys to verify signatures) is not that important,
// the user typically gets the key loaded from composing anyway
// the proper fix would be to run encodeZBase32 through background scripts
return { pubkey: null, pgpClient: null };
return { keys: [], errs: [] };
}
const directDomain = recipientDomain.toLowerCase();
const advancedDomainPrefix = (directDomain === 'localhost') ? '' : 'openpgpkey.';
Expand All @@ -50,15 +53,19 @@ export class Wkd extends Api {
const directUrl = `https://${directHost}/.well-known/openpgpkey`;
let response = await this.urlLookup(advancedUrl, userPart);
if (!response.buf && response.hasPolicy) {
return { pubkey: null, pgpClient: null }; // do not retry direct if advanced had a policy file
return { keys: [], errs: [] }; // do not retry direct if advanced had a policy file
}
if (!response.buf) {
response = await this.urlLookup(directUrl, userPart);
}
if (!response.buf) {
return { pubkey: null, pgpClient: null }; // do not retry direct if advanced had a policy file
return { keys: [], errs: [] }; // do not retry direct if advanced had a policy file
}
const { keys, errs } = await KeyUtil.readMany(response.buf);
return await KeyUtil.readMany(response.buf);
}

public lookupEmail = async (email: string): Promise<PubkeySearchResult> => {
const { keys, errs } = await this.rawLookupEmail(email);
if (errs.length) {
return { pubkey: null, pgpClient: null };
}
Expand All @@ -67,7 +74,7 @@ export class Wkd extends Api {
return { pubkey: null, pgpClient: null };
}
// if recipient uses same domain, we assume they use flowcrypt
const pgpClient = this.myOwnDomain === recipientDomain ? 'flowcrypt' : 'pgp-other';
const pgpClient = this.myOwnDomain === Str.getDomainFromEmailAddress(email) ? 'flowcrypt' : 'pgp-other';
Copy link
Collaborator

Choose a reason for hiding this comment

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

I suppose we'll be dropping pgpClient from everywhere. If you want to chunk up the upcoming PR, this could also be done in a separate PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let's drop it in the PR #3445 that actually "deletes" the client field from the database. This branch still attempts to store it.

try {
const pubkey = KeyUtil.armor(key);
return { pubkey, pgpClient };
Expand Down
5 changes: 5 additions & 0 deletions extension/js/common/core/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,11 @@ export class Str {
return { email, name, full };
}

public static getDomainFromEmailAddress = (emailAddr: string) => {
// todo: parseEmail()?
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think parseEmail would be sensible here, and throw if it returns "undefined"

return emailAddr.toLowerCase().split('@')[1];
}

public static rmSpecialCharsKeepUtf = (str: string, mode: 'ALLOW-SOME' | 'ALLOW-NONE'): string => {
// not a whitelist because we still want utf chars
str = str.replace(/[@&#`();:'",<>\{\}\[\]\\\/\n\t\r]/gi, '');
Expand Down
4 changes: 2 additions & 2 deletions extension/js/common/org-rules.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ export class OrgRules {
throw new Error(`Not a valid email`);
}
const storage = await AcctStore.get(email, ['rules']);
return new OrgRules(storage.rules || OrgRules.default, acctEmail.split('@')[1]);
return new OrgRules(storage.rules || OrgRules.default, Str.getDomainFromEmailAddress(acctEmail));
}

public static isPublicEmailProviderDomain = (emailAddrOrDomain: string) => {
Expand Down Expand Up @@ -173,7 +173,7 @@ export class OrgRules {
* This is because they already have other means to obtain public keys for these domains, such as from their own internal keyserver
*/
public canLookupThisRecipientOnAttester = (emailAddr: string): boolean => {
return !(this.domainRules.disallow_attester_search_for_domains || []).includes(emailAddr.split('@')[1] || 'NONE');
return !(this.domainRules.disallow_attester_search_for_domains || []).includes(Str.getDomainFromEmailAddress(emailAddr) || 'NONE');
}

/**
Expand Down
3 changes: 3 additions & 0 deletions test/source/mock/backend/backend-data.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,9 @@ export class BackendData {
"enforce_keygen_algo": "rsa2048",
"disallow_attester_search_for_domains": []
};
if (domain === 'google.mock.flowcryptlocal.com:8001') {
return { ...keyManagerAutogenRules, flags: [...keyManagerAutogenRules.flags, 'NO_ATTESTER_SUBMIT'] };
}
if (domain === 'key-manager-autogen.flowcrypt.com') {
return keyManagerAutogenRules;
}
Expand Down
4 changes: 4 additions & 0 deletions test/source/mock/key-manager/key-manager-endpoints.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { oauth } from '../lib/oauth';
import { Dict } from '../../core/common';
import { expect } from 'chai';
import { KeyUtil } from '../../core/crypto/key';
import { wkdAtgooglemockflowcryptlocalcom8001Private } from '../../tests/tooling/consts';

// tslint:disable:max-line-length
/* eslint-disable max-len */
Expand Down Expand Up @@ -193,6 +194,9 @@ export const mockKeyManagerEndpoints: HandlersDefinition = {
'/flowcrypt-email-key-manager/keys/private': async ({ body }, req) => {
const acctEmail = oauth.checkAuthorizationHeaderWithIdToken(req.headers.authorization);
if (isGet(req)) {
if (acctEmail === 'wkd@google.mock.flowcryptlocal.com:8001') {
return { privateKeys: [{ decryptedPrivateKey: wkdAtgooglemockflowcryptlocalcom8001Private }] };
}
if (acctEmail === 'get.key@key-manager-autogen.flowcrypt.com') {
return { privateKeys: [{ decryptedPrivateKey: existingPrv }] };
}
Expand Down
9 changes: 8 additions & 1 deletion test/source/mock/wkd/wkd-endpoints.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
/* ©️ 2016 - present FlowCrypt a.s. Limitations apply. Contact human@flowcrypt.com */

import { KeyUtil } from '../../core/crypto/key.js';
import { PgpArmor } from '../../core/crypto/pgp/pgp-armor.js';
import { wkdAtgooglemockflowcryptlocalcom8001Private } from '../../tests/tooling/consts.js';
import { HandlersDefinition } from '../all-apis-mock';

const alice = `-----BEGIN PGP PUBLIC KEY BLOCK-----
Expand Down Expand Up @@ -190,6 +192,11 @@ ctnWuBzRDeI0n6XDaPv5TpKpS7uqy/fTlJLGE9vZTFUKzeGkQFomBoXNVWs=
// todo - add a not found test with: throw new HttpClientErr('Pubkey not found', 404);

export const mockWkdEndpoints: HandlersDefinition = {
'/.well-known/openpgpkey/hu/st5or5guodbnsiqbzp6i34xw59h1sgmw?l=wkd': async () => {
// direct for wkd@google.mock.flowcryptlocal.com:8001
const pub = await KeyUtil.asPublicKey(await KeyUtil.parse(wkdAtgooglemockflowcryptlocalcom8001Private));
return Buffer.from((await PgpArmor.dearmor(KeyUtil.armor(pub))).data);
},
'/.well-known/openpgpkey/hu/ihyath4noz8dsckzjbuyqnh4kbup6h4i?l=john.doe': async () => {
return Buffer.from((await PgpArmor.dearmor(johnDoe1)).data); // direct for john.doe@localhost
},
Expand Down Expand Up @@ -219,6 +226,6 @@ export const mockWkdEndpoints: HandlersDefinition = {
return ''; // allow advanced for localhost
},
'/.well-known/openpgpkey/policy': async () => {
return ''; // allow direct for localhost
return ''; // allow direct for all
},
};
31 changes: 31 additions & 0 deletions test/source/tests/compose.ts
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,37 @@ export const defineComposeTests = (testVariant: TestVariant, testWithBrowser: Te
expect(recipients).to.eq(['recip1@corp.co', 'recip2@corp.co', 'сс1@corp.co', 'bсс1@corp.co', '1 more'].join(''));
}));

ava.default(`compose - auto include pubkey when our key is not available on Wkd`, testWithBrowser('ci.tests.gmail', async (t, browser) => {
const composePage = await ComposePageRecipe.openStandalone(t, browser, 'compose');
await composePage.page.setViewport({ width: 540, height: 606 });
await ComposePageRecipe.fillMsg(composePage, { to: 'flowcrypt.compatibility@gmail.com' }, 'testing auto include pubkey');
await composePage.waitTillGone('@spinner');
await Util.sleep(3); // wait for the Wkd lookup to complete
expect(await composePage.hasClass('@action-include-pubkey', 'active')).to.be.false;
await composePage.waitAndType(`@input-to`, 'some.unknown@unknown.com');
await composePage.waitAndFocus('@input-body');
await composePage.waitTillGone('@spinner');
await Util.sleep(3); // allow some time to search for messages
expect(await composePage.hasClass('@action-include-pubkey', 'active')).to.be.true;
}));

ava.default(`compose - auto include pubkey is inactive when our key is available on Wkd`, testWithBrowser(undefined, async (t, browser) => {
const acct = 'wkd@google.mock.flowcryptlocal.com:8001';
const settingsPage = await BrowserRecipe.openSettingsLoginApprove(t, browser, acct);
await SetupPageRecipe.autoKeygen(settingsPage);
const composePage = await ComposePageRecipe.openStandalone(t, browser, acct);
await composePage.page.setViewport({ width: 540, height: 606 });
await ComposePageRecipe.fillMsg(composePage, { to: 'ci.tests.gmail@flowcrypt.dev' }, 'testing auto include pubkey');
await composePage.waitTillGone('@spinner');
await Util.sleep(3); // wait for the Wkd lookup to complete
expect(await composePage.hasClass('@action-include-pubkey', 'active')).to.be.false;
await composePage.waitAndType('@input-to', 'some.unknown@unknown.com');
await composePage.waitAndFocus('@input-body');
await composePage.waitTillGone('@spinner');
await Util.sleep(3); // allow some time to search for messages
expect(await composePage.hasClass('@action-include-pubkey', 'active')).to.be.false;
}));

ava.default(`compose - freshly loaded pubkey`, testWithBrowser('ci.tests.gmail', async (t, browser) => {
const composePage = await ComposePageRecipe.openStandalone(t, browser, 'compose');
await ComposePageRecipe.fillMsg(composePage, { to: 'human@flowcrypt.com' }, 'freshly loaded pubkey');
Expand Down
Loading