Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disable Fido U2F? #93

Closed
r-a-y opened this issue May 13, 2022 · 3 comments · Fixed by #123
Closed

Disable Fido U2F? #93

r-a-y opened this issue May 13, 2022 · 3 comments · Fixed by #123

Comments

@r-a-y
Copy link

r-a-y commented May 13, 2022

Hi, thanks for creating this plugin!

We just tested your WebAuthn plugin and the Fido U2F migration process over to WebAuthn works well. However, do you think it would make sense to disable the core Two Factor's Fido U2F provider if your plugin is active?

I know that the core Two Factor plugin is looking at removing U2F in a future release: WordPress/two-factor#439, but at the moment, it is kind of jarring and confusing to see two Security Key sections when your WebAuthn provider works and basically replaces the core Fido U2F one.

If you decide to disable the Fido U2F provider in your plugin, this can be done where you're already registering your provider here:

public function two_factor_providers( array $providers ): array {
$providers[ TwoFactor_Provider_WebAuthn::class ] = __DIR__ . '/class-twofactor-provider-webauthn.php';
return $providers;
}

Also, I noticed that it is possible to delete the U2F key for each user with the 'WEBAUTHN_DELETE_U2F_KEYS_ON_MIGRATION' constant after migration. This should be documented publicly in the readme (or in some other place like the wiki) so others are aware of it.

@sjinks
Copy link
Owner

sjinks commented May 17, 2022

The WEBAUTHN_DELETE_U2F_KEYS_ON_MIGRATION exists mainly for development, as I did not like removing other plugin's data in production. For example, if the user installs this plugin, dislikes it, and then deletes it, the original keys will be lost. And it is usually too late to define this constant after playing with the plugin because the migration happens on the first access to the keys.

// It is still possible to use U2F with Firefox.

sjinks added a commit that referenced this issue May 17, 2022
GH-93: add an option to turn off the old U2F provider
@sjinks
Copy link
Owner

sjinks commented May 17, 2022

b39e4e7 adds an option to disable the U2F provider

@r-a-y
Copy link
Author

r-a-y commented May 17, 2022

The WEBAUTHN_DELETE_U2F_KEYS_ON_MIGRATION exists mainly for development, as I did not like removing other plugin's data in production.

Yeah, I came to this realization myself. If the core Two Factor plugin does their own migration from FIDO U2F to something else, then they will need to have access to those keys as well.

b39e4e7 adds an option to disable the U2F provider

Thanks for adding in the option to disable the core FIDO U2F provider. Feel free to close this issue.

I've also added support for your plugin in my BuddyPress integration for the Two Factor plugin here: r-a-y/bp-two-factor@30e2164.

I had to make one main adjustment and that was to remove the "required" attribute for the Key name field:

<input type="text" required="required" id="webauthn-key-name" value="" style="vertical-align: middle" />

Since the Key name field isn't really required in order to submit the form, perhaps you can remove the "required" attribute?

sjinks added a commit that referenced this issue Jun 10, 2022
GH-93: remove required attribute from webauthn_key_name
@sjinks sjinks mentioned this issue Jun 10, 2022
r-a-y added a commit to r-a-y/bp-two-factor that referenced this issue Jan 26, 2023
This is no longer required as of two-factor-provider-webauthn v1.0.7:
sjinks/wp-two-factor-provider-webauthn#93 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants