Add ability to automatically disable 2FA when in developer mode

[drpayyne]

Closes #3 and is backwards-compatible.

New possible combinations

• disable 2FA throughout (existing)

Build in the ability to automatically disable 2FA if dev mode is enabled.

• disable 2FA only when Magento is set to developer mode (new)

We also need an option (env var) to keep 2FA enabled in dev mode if debugging is needed.

• enable 2FA at all times (existing)

[drpayyne]

Hi @markshust, could you please review this? Thank you!

[markshust]

• disable 2FA only when Magento is set to developer mode (new)

Thanks for the PR! But this actually wasn't the intention of #3... the idea was to have the 2FA "automatically" disabled if Magento developer mode is enabled.

The config addition needed:

• Create a config var for "Enable 2FA for developer mode" -- default it to "No".

Workflow:

Here is some pseudo code for how I think it would work:

public function afterIsGranted(
    TfaSession $subject,
    $result
): bool {
    $is2faEnabled = $this->scopeConfig->isSetFlag(self::XML_PATH_CONFIG_ENABLE);

    if ($this->isMagentoDeveloperMode()) {
        $is2faEnabled = $this->scopeConfig->isSetFlag(self::XML_PATH_CONFIG_ENABLE_DEVELOPER_MODE);
    }

    return $is2faEnabled
            ? $result
            : true;
}

This would also need to apply to the Api class for API-requests.

[drpayyne]

Hi @markshust, thanks for your review. Just to confirm

My logic involves the dev mode 2FA switch being dependent on the global 2FA switch. Whereas what you're saying is that the two switches should be independent and the right switch is chosen based on the deploy mode. Right?

• What is the advantage of making it independent instead of being a nested switch? It does still disable 2FA automatically on dev mode.
• Is it still backward compatible? Asking since I made sure my logic is. 🤔

[markshust]

@drpayyne I find the naming implemented in your PR confusing. One toggle saying "2FA is enabled, default to yes", and another toggle saying "disable only in developer, default to yes" -- one uses a positive statement, and another uses a negative statement. See https://www.obsidiansecurity.com/blog/why-to-stop-writing-negative-code/ for more info.

I strive to only use positive statements as it makes a lot more sense and makes things easier to follow.

This module is only intended to be used in development, and the only way it would be a breaking change is if they ran it on production while in developer mode. Since developer mode causes tons of inherent security issues, disabling 2FA by default, if a site has developer mode enabled on prod, is really not of my concern. It's not any worse than writing errors to the browser.

That said, 100% of the developers installing this module would like 2FA disabled for dev, so this update shouldn't be breaking as "enable in developer mode" would default to Yes. In dev, developer mode is always on. This update would also allow them to toggle 2FA back on in developer mode if they wish. I think the multiple toggles makes sense here. It would also allow us to get rid of the need to set config variables after install, so someone can just install it and 2FA would automatically be disabled in developer mode.

[drpayyne]

Thank you so much for the detailed reply @markshust. I understand and agree with you.

I will follow up with the changes very soon. Thank you!

[markshust]

@drpayyne I made some updates, let me know what you think 0cd8e1f (#13)

I simplified the initial idea by adding a "Disable 2FA in developer mode" toggle. By default this is set to Yes, so 2FA is auto-disabled when in developer mode. If set to No, it falls back to the other 2 config options. So, this should be backwards-compatible, unless if they are in developer mode, in which it auto-disables 2FA. This is a breaking update, so I'll need to tag as 2.0.

[markshust]

@drpayyne made a few other updates before merging in. I really like this workflow/toggle, it's sort of a combination of both of our ideas. Testing and works really well! Tagging 2.0.0.

[drpayyne]

Awesome! Looks amazing.