Added emails and fingerprints to PrvKeyInfo #1640

[rrrooommmaaa]

Closes#1640

// edit tom: closes#3220

[rrrooommmaaa]

@tomholub Looks like I broke gmail test setup, probably with Config.secrets refactoring. There is no longer "Secure Compose" button during live tests. Can you point me where the test is wiring to the gmail account so I can debug it properly?

[tomholub]

Can you point me where the test is wiring to the gmail account so I can debug it properly?

If setup went fine but it doesn't work on https://mail.google.com then the content scripts are broken, or they fail during initialization.
You should build the extension and manually run it. Navigate to the gmail page and check error console. Check background page console, too. There will be an error somewhere.

The entrypoint for content scripts is extension/js/content_scripts/webmail/webmail.ts so if you want to use a debugger, you'd probably want to start there.

Also, this method is more or less at the core of content script setup:

constentrypoint=async()=>{try{constacctEmail=awaitwaitForAcctEmail();const{ tabId, notifications, factory, inject }=awaitinitInternalVars(acctEmail);awaitshowNotificationsAndWaitTilAcctSetUp(acctEmail,notifications);browserMsgListen(acctEmail,tabId,inject,factory,notifications);awaitwebmailSpecific.start(acctEmail,inject,notifications,factory,notifyMurdered);}catch(e){if(einstanceofTabIdRequiredError){console.error(`FlowCrypt cannot start: ${String(e)}`);}elseif(einstanceofError&&e.message==='Extension context invalidated.'){console.info(`FlowCrypt cannot start: extension context invalidated. Destroying.`);win.destroy();}elseif(!(einstanceofDestroyTrigger)){Catch.reportErr(e);}}};

[rrrooommmaaa]

@tomholub The tests now went well.
I suggest to refactor decrypted field out of PrvKeyInfo.
Should we create a migration test?

[tomholub]

Agree refactor decrypted away No for migration test, but need to do thorough manual testing.

…

On Sat, 5 Dec 2020, 12:06 Roman, ***@***.***> wrote: @tomholub <https://github.com/tomholub> The tests now went well. I suggest to refactor decrypted field out of PrvKeyInfo. Should we create a migration test? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#3213 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABQDZEMRFHCPKLNAM7MJCOLSTIH25ANCNFSM4UL2RI5A> .

[rrrooommmaaa]

I'm looking at Chrome console, but the local storage seems empty....
@tomholub Can you please assist?

[tomholub]

Extension storage is not visible in browser console at all. There is no mechanism to view it using UI in the browser

…

On Sat, 5 Dec 2020, 15:20 Roman, ***@***.***> wrote: I'm looking at Chrome console, but the local storage seems empty.... @tomholub <https://github.com/tomholub> Can you please assist? [image: image] <https://user-images.githubusercontent.com/23335944/101245456-1f621400-371e-11eb-80f9-a5adc5da516e.png> — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#3213 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABQDZEI4FNUF6I24UDIBAX3STI6SRANCNFSM4UL2RI5A> .

[rrrooommmaaa]

No for migration test, but need to do thorough manual testing.

I have run background_page in the debugger, the migration passed.

[rrrooommmaaa]

We should probably have distinct types for storage and operation. I assume we never want to store passphrase in the storage?
So we should have a type without the passphrase to be stored

[tomholub]

Code looks good with tome improvements to make. I will also do manual testing, once updated. Thanks!

[tomholub]

Looking at KeyInfo and PrvKeyInfo, I'm not sure if there is any use to having both anymore. I think nearly all usecases we have are for PrvKeyInfo. Wherever we need KeyInfo, we should use a Key instead (without storing it in storage).

I'm not 100% sure about this, but I think we could simplify this way.

[rrrooommmaaa]

public property is read in many places, for example, in keyserver (replacePublicKeyHandler)
In other places, like composer module, do you suggest to replace parse(ki.public) with parse(ki.private).asPublicKey?

[tomholub]

I'll have to have a look and maybe attempt refactoring that away. I suspect there are a bunch of usages that are no longer needed, but I'll have to see them one by one to confirm.

[tomholub]

Code looks good. Will test tomorrow. Thanks!

[tomholub]

Code looks ok so far (to my eyes) but need to fix tests again after my changes, and do manual testing.

[tomholub]

I have a concern about this particular original code, I'll have a second look later in a separate PR.

[tomholub]

Tests seem to be passing, manual tests look good too.

[tomholub]

@rrrooommmaaa can you please also review my changes that I added? These were to join PrvKeyInfo and KeyInfo, and also fix #3220 so that the result are: KeyInfo and KeyInfoWithOptionalPassPhrase.

[tomholub]

I'll go ahead and merge this, but please do have a look and let me know if I overlooked anything.

[rrrooommmaaa]

LGTM!

[tomholub]

Thanks!