Replace "Forgot pass phrase?" on pass phrase dialog when key_manager_url OrgRule is set

[tomholub]

Instead of the "Forgot pass phrase?" link, there should be text that says "Ask your IT staff for help if you lost your pass phrase."

[martgil]

I think I have managed to fix this issue by adding a check using this.orgRules.usesKeyManager() in passphrase.ts

[tomholub]

That looks good but is that still a link? It looks like the user could click it and go somewhere we don't want them to go.

[martgil]

When the link was clicked, the pop-up help for "Non-FES" user is shown.

Maybe we can update or add the description for "FES" user. Or maybe we could just disable the link?

[tomholub]

Let's disable the link when the user uses EKM as you say

[tomholub]

There's a test that uses EKM and also FORBID_STORING_PASS_PHRASE org rule. That test will be using the pass phrase dialog, I think. You can add a line to that test to verify the content has changed and is not clickable. Thanks!

[martgil]

I have tried to understand how ava testing was implemented in the flowcrypt-browser but I can't able to absorb what's happening inside the code. Can I help somebody once I get it, I'm going to add the test for the changes that I have made hopefully. Pardon me for any inconvenience.

[tomholub]

@martgil see if "Running tests locally" in https://github.com/FlowCrypt/flowcrypt-browser/wiki will help you. After that, please ask a more specific question and we'll be able to help or demonstrate. Thanks!

[martgil]

Hi @tomholub. Thanks for that. I think I have found it. There is currently a test that involves the passphrase.htm but the test is not specifically dedicated to user who has a FES account. Would you like me to add a specific test instead? The test will check if the user does have FORBID_STORING_PASS_PHRASE org rule and then if the DOM has indeed change the text and removed the href. I will try to do this and hopefully finished the test on moved on to the other tasks.

[tomholub]

Could you point me to the test you found?

[martgil]

I found this test located in 'test/source/tests/compose.ts' and the exact test im referring is the 'compose - signed with entered pass phrase + will remember pass phrase in session' from this ava test. Because it has the passphraseDiaglog which I need to fetch the DOM tree from.

[tomholub]

Here's how to find the ideal test to modify (or how to write a new test) when testing OrgRules.

1. make a decision if you will only be adding/editting test for the behavior of the org rule, or also add/edit a test for "default" behavior. If default behavior is already tested well, then you only address the specific OrgRule behavior.

Here, it's not quite clear - but since this will be just a small change, it doesn't hurt to address both sides. Let's start with the specific OrgRule test as an example.

2. search for the org rule you want to test eg FORBID_STORING_PASS_PHRASE, and see where it's used in the tests: (this example is test/source/mock/backend-data.ts

3. notice the test domain that it's definining: forbid-storing-passphrase-org-rule.flowcrypt.test and search for it

here's one:

another one:

4. find the most generic test that has the org rules setup you want (or create one)

In our case, it seems to be this one:

    ava.default('[email protected] - do not store passphrase', testWithBrowser(undefined, async (t, browser) => {
      const acctEmail = '[email protected]';
      const settingsPage = await BrowserRecipe.openSettingsLoginApprove(t, browser, acctEmail);
      await Util.sleep(5);
      const passphrase = 'long enough to suit requirements';
      await SetupPageRecipe.createKey(settingsPage, 'unused', 'none', { key: { passphrase }, usedPgpBefore: false },
        { isSavePassphraseDisabled: true, isSavePassphraseChecked: false });
      await settingsPage.notPresent('.swal2-container');
      const inboxPage = await browser.newPage(t, TestUrls.extensionInbox(acctEmail));
      await InboxPageRecipe.finishSessionOnInboxPage(inboxPage);
      const composeFrame = await InboxPageRecipe.openAndGetComposeFrame(inboxPage);
      await ComposePageRecipe.fillMsg(composeFrame, { to: '[email protected]' }, 'should not send as pass phrase is not known', { encrypt: false });
      await composeFrame.waitAndClick('@action-send');
      await inboxPage.waitAll('@dialog-passphrase');
      const passphraseDialog = await inboxPage.getFrame(['passphrase.htm']);
      const forgetPassPhraseElement = await passphraseDialog.waitAny('@forget-pass-phrase');
      expect(await InboxPageRecipe.isElementDisabled(forgetPassPhraseElement)).to.equal(true);
      expect(await InboxPageRecipe.isElementChecked(forgetPassPhraseElement)).to.equal(true);
      await inboxPage.close();
      await settingsPage.close();
    }));

If you notice, it already uses passphrase.htm. Plus this is a test domain that uses FORBID_STORING_PASS_PHRASE. That's what we wanted. In this issue, we are looking for specific text in specific element. Therefore you can use waitForContent just under the line const passphraseDialog = await inboxPage.getFrame(['passphrase.htm']); to test the behavior.

You can run the test per wiki (with .only) to make sure it runs properly.

5. find and edit an appropriate test that does not have OrgRules set. You've already found one: compose - signed with entered pass phrase + will remember pass phrase in session

    ava.default('compose - signed with entered pass phrase + will remember pass phrase in session', testWithBrowser('ci.tests.gmail', async (t, browser) => {
      const k = Config.key('ci.tests.gmail');
      const settingsPage = await browser.newPage(t, TestUrls.extensionSettings('[email protected]'));
      await SettingsPageRecipe.forgetAllPassPhrasesInStorage(settingsPage, k.passphrase);
      const inboxPage = await browser.newPage(t, TestUrls.extensionInbox('[email protected]'));
      const composeFrame = await InboxPageRecipe.openAndGetComposeFrame(inboxPage);
      await ComposePageRecipe.fillMsg(composeFrame, { to: '[email protected]' }, 'sign with entered pass phrase', { encrypt: false });
      await composeFrame.waitAndClick('@action-send');
      await inboxPage.waitAll('@dialog-passphrase');
      const passphraseDialog = await inboxPage.getFrame(['passphrase.htm']);
      await passphraseDialog.waitAndType('@input-pass-phrase', k.passphrase);
      await passphraseDialog.waitAndClick('@action-confirm-pass-phrase-entry');
      await inboxPage.waitTillGone('@dialog-passphrase');
      await inboxPage.waitTillGone('@container-new-message'); // confirming pass phrase will auto-send the message
      // signed - done, now try to see if it remembered pp in session
      const composePage = await ComposePageRecipe.openStandalone(t, browser, 'compose');
      await ComposePageRecipe.fillMsg(composePage, { to: '[email protected]' }, 'signed message pp in session', { encrypt: false });
      await ComposePageRecipe.sendAndClose(composePage);
      await settingsPage.close();
      await inboxPage.close();
    }));

Notice that it has a generic domain: [email protected]. This should mean that it doesn't have any OrgRules set up (unlike for example [email protected] which does).

Here you can similarly edit the test, but this time to test for the opposite behavior.

[martgil]

Hi @tomholub. Thanks for the reference, It helps me to understand it better now. I have tried what exactly we need to do but I am having an error on compose.ts for some reason:

As we can see even though I haven't used any of the .passphrase property it still insists TypeError: Cannot read property 'passphrase' of undefined at Function.SetupPageRecipe.recover (/home/mart/project/flowcrypt-browser/test/source/tests/page-recipe/setup-page-recipe.ts:208:69).

The npm command I used for testing is the npm run test_local_chrome_consumer_mock; So that the if (testVariant !== 'CONSUMER-LIVE-GMAIL') will pass since I'm running the local test for consumer and not a consumer-live variant of testing. I believed that the flowcrypt-browser/test/source/tests/page-recipe/setup-page-recipe.ts has nothing to do with the error. The error for .passphrase property is undefined keeps appearing even on the untouched version of the code from compose.ts so It's really strange.

[tomholub]

Please submit it to the Draft PR, then we'll be able to try it and help more specifically.

[martgil]

Understood @tomholub. I have converted the original PR into a draft PR for this issue.