Skip to content

Commit

Permalink
#4609 stop using initialLegacySubmit
Browse files Browse the repository at this point in the history
  • Loading branch information
ioanmo226 committed Sep 21, 2022
1 parent 343ca09 commit 5dba991
Show file tree
Hide file tree
Showing 9 changed files with 112 additions and 40 deletions.
11 changes: 10 additions & 1 deletion extension/chrome/settings/modules/keyserver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ import { KeyStore } from '../../../js/common/platform/store/key-store.js';
import { KeyStoreUtil } from "../../../js/common/core/crypto/key-store-util.js";
import { AcctStore } from '../../../js/common/platform/store/acct-store.js';
import { KeyUtil } from '../../../js/common/core/crypto/key.js';
import { InMemoryStore } from '../../../js/common/platform/store/in-memory-store.js';
import { InMemoryStoreKeys } from '../../../js/common/core/const.js';

type AttesterKeyserverDiagnosis = { hasPubkeyMissing: boolean, hasPubkeyMismatch: boolean, results: Dict<{ pubkeys: string[], match: boolean }> };

Expand Down Expand Up @@ -96,7 +98,14 @@ View.run(class KeyserverView extends View {
return;
}
try {
await this.pubLookup.attester.initialLegacySubmit(String($(target).attr('email')), mostUsefulPrv.keyInfo.public);
const email = String($(target).attr('email'));
// Use submitPrimaryEmailPubkey if email is primary email
if (email === this.acctEmail) {
const idToken = await InMemoryStore.get(this.acctEmail, InMemoryStoreKeys.ID_TOKEN);
await this.pubLookup.attester.submitPrimaryEmailPubkey(email, mostUsefulPrv.keyInfo.public, idToken!);
} else { // If email is alias email
await this.pubLookup.attester.replacePubkey(email, mostUsefulPrv.keyInfo.public);
}
} catch (e) {
ApiErr.reportIfSignificant(e);
await Ui.modal.error(ApiErr.eli5(e));
Expand Down
23 changes: 18 additions & 5 deletions extension/chrome/settings/setup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,10 @@
'use strict';

import { Bm, BrowserMsg } from '../../js/common/browser/browser-msg.js';
import { Url } from '../../js/common/core/common.js';
import { asyncSome, Url, Value } from '../../js/common/core/common.js';
import { ApiErr } from '../../js/common/api/shared/api-error.js';
import { Assert } from '../../js/common/assert.js';
import { Catch } from '../../js/common/platform/catch.js';
import { Catch, SubmitPubKeyError } from '../../js/common/platform/catch.js';
import { KeyInfoWithIdentity, KeyUtil } from '../../js/common/core/crypto/key.js';
import { Gmail } from '../../js/common/api/email-provider/gmail/gmail.js';
import { Google } from '../../js/common/api/email-provider/gmail/google.js';
Expand Down Expand Up @@ -330,16 +330,29 @@ export class SetupView extends View {
};

private submitPubkeys = async (addresses: string[], pubkey: string) => {
if (this.clientConfiguration.useLegacyAttesterSubmit()) {
if (this.clientConfiguration.setupEnsureImportedPrvMatchLdapPub()) {
// this will generally ignore errors if conflicting key already exists, except for certain orgs
await this.pubLookup.attester.initialLegacySubmit(this.acctEmail, pubkey);
const result = await this.pubLookup.attester.doLookupLdap(this.acctEmail);
if (result.pubkeys.length) {
const prvs = await KeyStoreUtil.parse(await KeyStore.getRequired(this.acctEmail));
const parsedPubKeys = await KeyUtil.parseMany(result.pubkeys.join('\n'));
const hasMatchingKey = await asyncSome(prvs, (async (privateKey) => {
return parsedPubKeys.some((parsedPubKey) => Value.arr.hasIntersection(privateKey.key.allIds, parsedPubKey.allIds));
}));
if (!hasMatchingKey) {
// eslint-disable-next-line max-len
throw new SubmitPubKeyError(`Imported private key with ids ${prvs.map(prv => prv.key.id).join(', ')} does not match public keys on company LDAP server with ids ${parsedPubKeys.map(pub => pub.id).join(', ')}. Please ask your help desk.`);
}
} else {
throw new SubmitPubKeyError('Your organization requires public keys to be present on company LDAP server, but no public key was found. Please ask your internal help desk.');
}
} else {
// this will actually replace the submitted public key if there was a conflict, better ux
await this.pubLookup.attester.submitPrimaryEmailPubkey(this.acctEmail, pubkey, this.idToken!);
}
const aliases = addresses.filter(a => a !== this.acctEmail);
if (aliases.length) {
await Promise.all(aliases.map(a => this.pubLookup.attester.initialLegacySubmit(a, pubkey)));
await Promise.all(aliases.map(a => this.pubLookup.attester.replacePubkey(a, pubkey)));
}
};

Expand Down
10 changes: 0 additions & 10 deletions extension/js/common/api/key-server/attester.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,16 +103,6 @@ export class Attester extends Api {
return r.responseText;
};

/**
* Looking to deprecate this, but still used for some customers
*/
public initialLegacySubmit = async (email: string, pubkey: string): Promise<{ saved: boolean }> => {
if (!this.clientConfiguration.canSubmitPubToAttester()) {
throw new Error('Cannot submit pubkey to attester because your organisation rules forbid it');
}
return await this.jsonCall<{ saved: boolean }>('initial/legacy_submit', { email: Str.parseEmail(email).email, pubkey: pubkey.trim() });
};

public testWelcome = async (email: string, pubkey: string): Promise<{ sent: boolean }> => {
return await this.jsonCall<{ sent: boolean }>('test/welcome', { email, pubkey });
};
Expand Down
10 changes: 4 additions & 6 deletions extension/js/common/client-configuration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { AcctStore } from './platform/store/acct-store.js';
import { KeyAlgo } from './core/crypto/key.js';

type ClientConfiguration$flag = 'NO_PRV_CREATE' | 'NO_PRV_BACKUP' | 'PRV_AUTOIMPORT_OR_AUTOGEN' | 'PASS_PHRASE_QUIET_AUTOGEN' |
'ENFORCE_ATTESTER_SUBMIT' | 'NO_ATTESTER_SUBMIT' | 'USE_LEGACY_ATTESTER_SUBMIT' |
'ENFORCE_ATTESTER_SUBMIT' | 'NO_ATTESTER_SUBMIT' | 'SETUP_ENSURE_IMPORTED_PRV_MATCH_LDAP_PUB' |
'DEFAULT_REMEMBER_PASS_PHRASE' | 'HIDE_ARMOR_META' | 'FORBID_STORING_PASS_PHRASE';

export type ClientConfigurationJson = {
Expand Down Expand Up @@ -192,12 +192,10 @@ export class ClientConfiguration {
};

/**
* Some orgs use flows that are only implemented in POST /initial/legacy_submit and not in POST /pub/[email protected]:
* -> enforcing that submitted keys match customer key server
* Until the newer endpoint is ready, this flag will point users in those orgs to the original endpoint
* Some orgs will require user's imported private key to match their LDAP pub key result
*/
public useLegacyAttesterSubmit = (): boolean => {
return (this.clientConfigurationJson.flags || []).includes('USE_LEGACY_ATTESTER_SUBMIT');
public setupEnsureImportedPrvMatchLdapPub = (): boolean => {
return (this.clientConfigurationJson.flags || []).includes('SETUP_ENSURE_IMPORTED_PRV_MATCH_LDAP_PUB');
};

/**
Expand Down
1 change: 1 addition & 0 deletions extension/js/common/platform/catch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import { FLAVOR, VERSION } from '../core/const.js';

export class UnreportableError extends Error { }
export class SubmitPubKeyError extends UnreportableError { }
type ObjWithStack = { stack: string };
export type ErrorReport = {
name: string;
Expand Down
11 changes: 8 additions & 3 deletions extension/js/common/settings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { ApiErr, AjaxErr } from './api/shared/api-error.js';
import { Attachment } from './core/attachment.js';
import { Browser } from './browser/browser.js';
import { Buf } from './core/buf.js';
import { Catch } from './platform/catch.js';
import { Catch, SubmitPubKeyError } from './platform/catch.js';
import { Env } from './browser/env.js';
import { Gmail } from './api/email-provider/gmail/gmail.js';
import { GoogleAuth } from './api/email-provider/gmail/google-auth.js';
Expand Down Expand Up @@ -258,11 +258,16 @@ export class Settings {
* todo - could probably replace most usages of this method with retryPromptUntilSuccessful which is more intuitive
*/
public static promptToRetry = async (lastErr: unknown, userMsg: string, retryCb: () => Promise<void>, contactSentence: string): Promise<void> => {
let userErrMsg = `${userMsg} ${ApiErr.eli5(lastErr)}`;
let errorMsg!: string;
if (lastErr instanceof AjaxErr && (lastErr.status === 400 || lastErr.status === 405)) {
// this will make reason for err 400 obvious to user - eg on EKM 405 error
userErrMsg = `${userMsg}, ${lastErr.resMsg}`;
errorMsg = lastErr.resMsg ?? '';
} else if (lastErr instanceof SubmitPubKeyError) {
errorMsg = lastErr.message;
} else {
errorMsg = ApiErr.eli5(lastErr);
}
const userErrMsg = `${userMsg}, ${errorMsg}`;
while (await Ui.renderOverlayPromptAwaitUserChoice({ retry: {} }, userErrMsg, ApiErr.detailsAsHtmlWithNewlines(lastErr), contactSentence) === 'retry') {
try {
return await retryCb();
Expand Down
71 changes: 58 additions & 13 deletions test/source/mock/attester/attester-endpoints.ts
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,12 @@ export const mockAttesterEndpoints: HandlersDefinition = {
if (emailOrLongid === '[email protected]') {
return protonMailCompatKey;
}
if (emailOrLongid === '[email protected]') {
return hasPubKey;
}
if (emailOrLongid === '[email protected]') {
return protonMailCompatKey;
}
if (emailOrLongid === '[email protected]' && server === 'keyserver.pgp.com') {
return [protonMailCompatKey, testMatchPubKey].join('\n');
}
Expand All @@ -136,19 +142,6 @@ export const mockAttesterEndpoints: HandlersDefinition = {
throw new HttpClientErr(`Not implemented: ${req.method}`);
}
},
'/attester/initial/legacy_submit': async ({ body }, req) => {
if (!isPost(req)) {
throw new HttpClientErr(`Wrong method: ${req.method}`);
}
const { email, pubkey } = body as Dict<string>;
expect(email).to.contain('@');
expect(pubkey).to.contain('-----BEGIN PGP PUBLIC KEY BLOCK-----');
if (email === '[email protected]') {
throw new HttpClientErr(`Could not find LDAP pubkey on a LDAP-only domain for email ${email} on server keys.flowcrypt.test`);
}
MOCK_ATTESTER_LAST_INSERTED_PUB[email] = pubkey;
return { saved: true };
},
'/attester/test/welcome': async ({ body }, req) => {
if (!isPost(req)) {
throw new HttpClientErr(`Wrong method: ${req.method}`);
Expand Down Expand Up @@ -387,3 +380,55 @@ qHJuDNEmMUvx3cMUd5HtsOFO9JapCp1iCVo2p49CIXA4NUrLETNM2ZddknhbFm8bsK48tTJEH6l4
Wq3aCVXYGg==
=Ldag
-----END PGP PUBLIC KEY BLOCK-----`;

const hasPubKey = `-----BEGIN PGP PUBLIC KEY BLOCK-----
mQINBF3H8N8BEADHCZdJnXkMn+asjH6eodMCqIcd4LJdLtOZljWEBsErV2vf+zZl
sle3lzo/LK5uGThwg0kY7QRFhLS2QII8atYw0E/+WWuWNhm+9UpmuZXpLcfkxKEr
P+xXJaX8ktSt0KXj0RUdFrkgh5EeqG2sdrflFjp90z9XQzt3BEpfr+9IhIQZDe23
9Xu0Zde4VzrxuA2WFHnsZiqTF41CGQmFapcroAZ5JmP+zXLrig62LFxkACJq4Fxb
WX6k1JrFjD+QXs78hB4sVdULDnxI4J8rXvryzW2PMXGr9GFAEmIx7ksOHp51Hyyq
Br8Zcj/YCmwDvm+9GaNKY1oVBeviphBHNXOBYLYO9py/1OSS9lCW7Ja0YsrPp+wh
VC+pDk0NmY0HSZPOXwkJBL8VkcYI4ZMFSLqqMpQFN86y61uezUD2gdX+zbEoS+Pn
dIV5lZUuLEO0trRjg1b1/PJSyi8bBgcoduhEUtGD+n5J3NGyOP9w1jHWaDhjMJ6n
p/Su8RUC7p4/uXzBxn77jwkYnO4kULuK9K6it5gB8kXgaaQ6tc21p2VyID8fb1nP
Hi3Jg524NkkqsmRA7Ulmo087B+k3yK/rbI7rVAs5sFQI45hqKyVvTITm9MSMBDFk
+T7VA2NLuM7QhS7hHxhokTzo80HEFdQxbzwFZbzdZf8RdTFDN6exnYRuFwARAQAB
tDNIYXMgUHViIFRlc3QgPGhhcy5wdWJAb3JnLXJ1bGVzLXRlc3QuZmxvd2NyeXB0
LmNvbT6JAjUEEAEIAB8FAl3H8N8GCwkHCAMCBBUICgIDFgIBAhkBAhsDAh4BAAoJ
ELxhT3Bo224jrzUP/0Cfu7gpyzP9y0tAQFTOZfsRrPoxwQZ3uie003dwUJs8+cBn
WVg6t3iKvDbAxubm6K1vS+OvbaeE8DUa6uP7yIQzblgaFg1rFl5uIxORaBwqbOFl
BmbndkkWiHUER4GG2a18KC4PLE22xmJRmAd7HJQbAs/JfQ5d4sECFeFjMGMn7RGe
NWJ8CnoeVFQbXmKjtQvzmmmzalDVm3JyUu1MjDI3y2nuhE7ESTELj8BT4LHHpOIp
5xhD6YnY56AlOe7mzkY/9PKHck/Qwhuj+622yhVDSUFYtn6nnSCkUo9b2Yn18Npm
NuVZEZunG6ZecJ9pxa482bzq4aL6QEFBbQ+Ubf82Lh7Ir8g+zuvGfICq6m9rXNMX
d80UrmiI1vIRjudBJ0c5v/ovgLp+JPESnFVekUVtWjQrcEGndQrRVmPXsaCOqw7g
nKqzxs+2SntVOpzsSLKfijtQdw6qpQsL0LGi+lDgYMMFrDLaeuzZJ6bx714kN+mP
oTLk++6bnaWtvXdgcEoUAFaQFaAWrZKDb0UyfZ5ZI7oPIPdhT5Z6EoQGxEofnOnk
UN73iXY5OGH+X5UUnm2di59g2tftDAmmmU1Mp6Aedn7xJuGmx+yWq3IMLPbm1+zH
hU0t2iXh4D/33YzPd/3QIo++o0ewKf8lYC5iG2OfSKQAANm01vMti2/sJ/EhuQIN
BF3H8N8BEAC1zF/bc4+cdSQ1TOhm9k+BpHZa/Tp179Q7zHB5GwfGFNTkdeJffzhX
Yl/ey4jAnvfAouT+mdsPuLIrPzF0iLqya3SRiLBaxsX77kjrGTIgL29pPL9v3xfb
iO0kHF/M3RPbNqb+e1iJgxtQ/T/jtJmA9r+BfaoUPMRBrbj5D3EemyXVp1BZGRga
Bi8s2av97ABlxF9arS5m1sH1AuEsQa81nAX2vvCVF2MexOUViHZVNfoeTEc1B5XU
9RnlgBnFsDL9NUI6C6M2AT5ZrGCNIerNM4sUWcWTVbrblSJUAooCbM7GX0Tgm3Ir
5zdVlFo9RRIWJHXn5mVMN3olD2JysUnzYkvhSrVNOzjtyXpTiCjJnGwvjIPcD9U/
uh7kwNrcitB96cPE0bzOjtkOP47BMb/SeRpPcNx8+gTcB017uW97d1LNNjMbX57D
m7UIAVOZr0UGwlDAHla8mS4iVC3ztyhmnF8o3T1fLKKAFNiV7tzYSYBwkMWVTIFc
+iWGoq2hU/ysYalpf1ZLsqTmEKNaSG8QXviWOTuSa8IiciE9CpPwyRvo90IJfB13
xP39kiF6kBSA4LvUihZ+5LN90DYJYKAtilQdaLhO3x3KYdiIKfs0P8ApM44vfZ2u
ipToxMXCcw/te0Y0o67twzjipTfwF94VmuMmNi/sLNvZhWwuVmoVHQARAQABiQIf
BBgBCAAJBQJdx/DfAhsMAAoJELxhT3Bo224jmVkP/0ss9UqRFiZaJp5kSI/8FycU
oN/ZFMQ8nQl/CC7ZiUjA2GvNtRu93rDi7n4jqwDCF8WCUMu5drTWDGizwli2QFLK
K0nTtDn7JWeblMwDzfQBgV0fkoBkUlsEgcS8+ELsPTEGMyY8IENeCBpuo2FZ61kV
RXYvbuxLMq6sVFIeYei6ZfhGdrR6sozr0ed5AI07epRsQk1TuxMQuVG5W4L//hUF
XJQTpaWZhkaWcYx2DWfSfhgnxhU5TQeUCGlOLPOcy2RsY9N8Mz9YLi+rxsXI9TAC
2EQTyhbZyKWE3n8TYe+wkIQFkn0tHTmY2cmrmckTQs/WPVXvOTQOwkHxaUujfj01
05hziBbzTeQW5xIX0YFFzOEJGJnm0v2XtaqCUq+cVzYP98NU/fOcDSCG6u6X6UyZ
AbivCgdNtKkCJS6c0bDmLxnwxqfrXtJBHb//RcKOIt29ULwDDpg7pbyLHPVeHoww
wAsfVpC/5GYoG5beanFou5ssHr6yUwEo/sSjj8FiLISPb2MewWl6SREpqQf3zuaa
bHoV/t27o3082KxgFH7zyJ6nOCrP4AT8nmPSu4uypYIanmn97+RdjIeJjN/VDt3p
NaqFqwp9jMCYEnjfM2VSYFTKFrQYm9zYUTIKnQxNtWC8Z85UKz2eMk/INukzWe7g
cvQabqad/ZghLSTzo/Kf
=sshZ
-----END PGP PUBLIC KEY BLOCK-----`;
2 changes: 1 addition & 1 deletion test/source/mock/backend/backend-data.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ export class BackendData {
"NO_PRV_BACKUP",
"HIDE_ARMOR_META",
"ENFORCE_ATTESTER_SUBMIT",
"USE_LEGACY_ATTESTER_SUBMIT",
"SETUP_ENSURE_IMPORTED_PRV_MATCH_LDAP_PUB",
]
};
}
Expand Down
13 changes: 12 additions & 1 deletion test/source/tests/setup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -499,6 +499,17 @@ AN8G3r5Htj8olot+jm9mIa5XLXWzMNUZgg==
expect(ki[0].public).to.not.include('Comment');
}));

ava.default('[email protected] - no backup, no keygen', testWithBrowser(undefined, async (t, browser) => {
const settingsPage = await BrowserRecipe.openSettingsLoginApprove(t, browser, '[email protected]');
await SetupPageRecipe.manualEnter(settingsPage, 'has.pub.client.configuration.test', { noPrvCreateClientConfiguration: true, enforceAttesterSubmitClientConfiguration: true, fillOnly: true },
{ isSavePassphraseChecked: false, isSavePassphraseHidden: false });
await settingsPage.waitAndClick('@input-step2bmanualenter-save');
await settingsPage.waitAll(['@container-overlay-prompt-text', '@action-overlay-retry']);
const renderedErr = await settingsPage.read('@container-overlay-prompt-text');
expect(renderedErr).to.contain('Failed to submit to Attester');
expect(renderedErr).to.contain('Imported private key with ids 576C48E8E9E33B772FF07B11BC614F7068DB6E23 does not match public keys on company LDAP server with ids AB8CF86E37157C3F290D72007ED43D79E9617655. Please ask your help desk.');
}));

ava.default('no.pub@client-configurations-test - no backup, no keygen, enforce attester submit with submit err', testWithBrowser(undefined, async (t, browser) => {
const settingsPage = await BrowserRecipe.openSettingsLoginApprove(t, browser, '[email protected]');
await SetupPageRecipe.manualEnter(settingsPage, 'no.pub.client.configuration', { noPrvCreateClientConfiguration: true, enforceAttesterSubmitClientConfiguration: true, fillOnly: true },
Expand All @@ -507,7 +518,7 @@ AN8G3r5Htj8olot+jm9mIa5XLXWzMNUZgg==
await settingsPage.waitAll(['@container-overlay-prompt-text', '@action-overlay-retry']);
const renderedErr = await settingsPage.read('@container-overlay-prompt-text');
expect(renderedErr).to.contain(`Failed to submit to Attester`);
expect(renderedErr).to.contain(`Could not find LDAP pubkey on a LDAP-only domain for email [email protected] on server keys.flowcrypt.test`);
expect(renderedErr).to.contain(`Your organization requires public keys to be present on company LDAP server, but no public key was found. Please ask your internal help desk.`);
}));

ava.default('[email protected] - do not submit to attester', testWithBrowser(undefined, async (t, browser) => {
Expand Down

0 comments on commit 5dba991

Please sign in to comment.