Only show "Security Keys" section if "FIDO U2F Security Keys" is enabled

[r-a-y]

Hi,

This PR only shows the "Security Keys" section if the "FIDO U2F Security Keys" provider is enabled. See #243.

Here's a GIF of the PR:

If JS is disabled, the "Security Keys" section gets displayed as expected.

---

I created this PR for better UX. If the "Security Keys" section is shown, but the "FIDO U2F Security Keys" provider is not enabled, this might confuse people thinking that this might be something that needs to be filled in.

I would also recommend changing the Requires an HTTPS connection. Configure your security keys in the "Security Keys" section below. string. I don't think it is necessary to mention Requires an HTTPS connection. Either the server supports HTTPS or it doesn't. The end user doesn't care about this.

Let me know what you think.

[kasparsd]

Thanks for the suggestion @r-a-y!

I agree that the separate location for the security keys is confusing. Ideally, this would be displayed inline the option similar to the other providers.

One issue with the suggested approach is that users can't add or remove the keys unless the feature is enabled. I feel like the security keys section should always be available even when the feature itself is disabled.

I don't think it is necessary to mention Requires an HTTPS connection. Either the server supports HTTPS or it doesn't. The end user doesn't care about this.

This wording was added in #162 after a lot of support requests asking why the feature doesn't work. Users didn't know that HTTPS is required on the login page for this to work. It is added only if the admin area is accessed without the HTTPS so it doesn't display for users with the correct HTTPS setup.

[r-a-y]

One issue with the suggested approach is that users can't add or remove the keys unless the feature is enabled. I feel like the security keys section should always be available even when the feature itself is disabled.

Maybe we could add a button that says "Configure" and that would toggle and scroll down to the Security Keys section instead?

This wording was added in #162 after a lot of support requests asking why the feature doesn't work. Users didn't know that HTTPS is required on the login page for this to work. It is added only if the admin area is accessed without the HTTPS so it doesn't display for users with the correct HTTPS setup.

Ahh right, I'm testing this locally on a non-HTTPS set up, which is why I'm seeing this wording.

Update: Just tested on a HTTPS connection, the Requires an HTTPS connection string still shows up here:

two-factor/providers/class-two-factor-fido-u2f.php

Line 243 in 736473e

<?php esc_html_e( 'Requires an HTTPS connection. Configure your security keys in the "Security Keys" section below.', 'two-factor' ); ?>

Perhaps the FIDO provider shouldn't even be enabled if the server doesn't support HTTPS. Shouldn't a simple is_ssl() suffice to make this determination? A lot has changed since #162. HTTPS is more widespread than it was three years ago.

[jeffpaul]

@kasparsd any thoughts on a review of this PR and whether it's ready for consideration in v0.8.0?

[jeffpaul]

Given that the approach for the 0.8.0 release is to fully deprecate U2F, the changes in this PR will no longer be relevant. Apologies for the long delay on review here and thank you @r-a-y for your contribution and time.