From deb8868c92e21f39f8cc468395b70036a68f6fde Mon Sep 17 00:00:00 2001 From: Dion Hulse Date: Fri, 19 May 2023 03:26:20 +0000 Subject: [PATCH 1/7] The WebAuthn plugin does not work well with shared user tables, ensure that it reads from the correct tables. --- wporg-two-factor.php | 27 +++++++++++++++++++++++---- 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/wporg-two-factor.php b/wporg-two-factor.php index cc470935..aeefdc2c 100644 --- a/wporg-two-factor.php +++ b/wporg-two-factor.php @@ -12,6 +12,7 @@ namespace WordPressdotorg\Two_Factor; use Two_Factor_Core; use WildWolf\WordPress\TwoFactorWebAuthn\Plugin as WebAuthn_Plugin; +use WildWolf\WordPress\TwoFactorWebAuthn\Constants as WebAuthn_Plugin_Constants; use WP_User, WP_Error; defined( 'WPINC' ) || die(); @@ -29,15 +30,33 @@ function is_2fa_beta_tester() : bool { require_once __DIR__ . '/settings/settings.php'; -/* +/** + * Load the WebAuthn plugin. + * * Make sure the WebAuthn plugin loads early, because all of our functions that call * `Two_Factor_Core::is_user_using_two_factor()` etc assume that all providers are loaded. If WebAuthn is loaded * too late, then `remove_capabilities_until_2fa_enabled()` would cause `get_enable_2fa_notice()` to be shown on * the front end if WebAuthn is enabled and TOTP isn't. */ -$webauthn = WebAuthn_Plugin::instance(); -$webauthn->init(); -$webauthn->maybe_update_schema(); // This needs to run before plugins_loaded, as jetpack and wporg-two-factor do things way too early to the $current_user. +function load_webauthn_plugin() { + global $wpdb; + + $webauthn = WebAuthn_Plugin::instance(); + $webauthn->init(); + + // Use central WebAuthn tables, instead of ones for each site that shares our user tables. + $wpdb->webauthn_credentials = 'wporg_' . WebAuthn_Plugin_Constants::WA_CREDENTIALS_TABLE_NAME; + $wpdb->webauthn_users = 'wporg_' . WebAuthn_Plugin_Constants::WA_USERS_TABLE_NAME; + + // The schema update checks should not check for updates on every request. + remove_action( 'plugins_loaded', [ $webauthn, 'maybe_update_schema' ] ); + + // The schema update checks do need occur, but only on admin requests on the main network. + if ( 'wporg_' === $wpdb->base_prefix || 'local' === wp_get_environment_type() ) { + add_action( 'admin_init', [ $webauthn, 'maybe_update_schema' ] ); + } +} +load_webauthn_plugin(); add_filter( 'two_factor_providers', __NAMESPACE__ . '\two_factor_providers', 99 ); // Must run _after_ all other plugins. add_filter( 'two_factor_primary_provider_for_user', __NAMESPACE__ . '\set_primary_provider_for_user', 10, 2 ); From c36df99745cd371c0150857cae44a97456b10dc3 Mon Sep 17 00:00:00 2001 From: Dion Hulse Date: Fri, 19 May 2023 04:24:19 +0000 Subject: [PATCH 2/7] Add a custom provider for some overrides. --- class-cached-webauthn-provider.php | 15 +++++++++++++++ wporg-two-factor.php | 9 +++++++++ 2 files changed, 24 insertions(+) create mode 100644 class-cached-webauthn-provider.php diff --git a/class-cached-webauthn-provider.php b/class-cached-webauthn-provider.php new file mode 100644 index 00000000..59589c84 --- /dev/null +++ b/class-cached-webauthn-provider.php @@ -0,0 +1,15 @@ + Date: Fri, 19 May 2023 04:25:17 +0000 Subject: [PATCH 3/7] Change the alternate provider text from 'Use WebAuthn' to 'Use your security key'. --- class-cached-webauthn-provider.php | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/class-cached-webauthn-provider.php b/class-cached-webauthn-provider.php index 59589c84..593b5b5e 100644 --- a/class-cached-webauthn-provider.php +++ b/class-cached-webauthn-provider.php @@ -12,4 +12,13 @@ class WPORG_TwoFactor_Provider_WebAuthn extends TwoFactor_Provider_WebAuthn { public function get_key() { return parent::class; } + + /** + * See https://github.com/sjinks/wp-two-factor-provider-webauthn/pull/468 + * + * @return string + */ + public function get_alternative_provider_label() { + return __( 'Use your security key', 'wporg-two-factor' ); + } } From 23653a1cdf21d8ad5fecb8bbdcb1cf1290bc628f Mon Sep 17 00:00:00 2001 From: Dion Hulse Date: Fri, 19 May 2023 04:30:24 +0000 Subject: [PATCH 4/7] Disable the WebAuthn interface if it requires revalidation. --- class-cached-webauthn-provider.php | 63 +++++++++++++++++++++++++++++- 1 file changed, 62 insertions(+), 1 deletion(-) diff --git a/class-cached-webauthn-provider.php b/class-cached-webauthn-provider.php index 593b5b5e..2c8bd9d3 100644 --- a/class-cached-webauthn-provider.php +++ b/class-cached-webauthn-provider.php @@ -1,6 +1,6 @@ _add_filters(); + } + + return $instance; + } + /** * See https://github.com/sjinks/wp-two-factor-provider-webauthn/pull/468 * @@ -21,4 +43,43 @@ public function get_key() { public function get_alternative_provider_label() { return __( 'Use your security key', 'wporg-two-factor' ); } + + /** + * Add some filters to watch for WebAuthn events. + */ + public function _add_filters() { + // Clear the cache when a user is updated. + add_action( 'wp_ajax_webauthn_preregister', [ $this, '_webauthn_ajax_request' ], 1 ); + add_action( 'wp_ajax_webauthn_register', [ $this, '_webauthn_ajax_request' ], 1 ); + add_action( 'wp_ajax_webauthn_delete_key', [ $this, '_webauthn_ajax_request' ], 1 ); + add_action( 'wp_ajax_webauthn_rename_key', [ $this, '_webauthn_ajax_request' ], 1 ); + + // Disable the admin UI if it needs revalidation. + add_action( 'show_user_security_settings', [ $this, '_show_user_security_settings' ], -1 ); + } + + /** + * Force the user to revalidate their 2FA if they're updating their WebAuthn keys. + * + * This is pending an upstream PR for the revalidation. + */ + public function _webauthn_ajax_request() { + // Check the users session is still active and 2FA revalidation isn't required. + if ( ! Two_Factor_Core::current_user_can_update_two_factor_options() ) { + wp_send_json_error( __( 'Your session has expired. Please refresh the page and try again.', 'wporg-two-factor' ) ); + } + } + + /** + * Wrap the additional providers in a disabled fieldset if the user needs to revalidate. + * + * This is pending an upstream PR. + */ + public function _show_user_security_settings() { + $show_2fa_options = Two_Factor_Core::current_user_can_update_two_factor_options(); + if ( ! $show_2fa_options ) { + echo '
'; + add_action( 'show_user_security_settings', function() { echo '
'; }, 1001 ); + } + } } From 3f726fd3bd02d12aac07e3faa24f10b55df88a50 Mon Sep 17 00:00:00 2001 From: Dion Hulse Date: Fri, 19 May 2023 04:31:30 +0000 Subject: [PATCH 5/7] Cache WPORG_TwoFactor_Provider_WebAuthn::is_available_for_user() which is called during set_current_user. --- class-cached-webauthn-provider.php | 28 +++++++++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/class-cached-webauthn-provider.php b/class-cached-webauthn-provider.php index 2c8bd9d3..9c724a17 100644 --- a/class-cached-webauthn-provider.php +++ b/class-cached-webauthn-provider.php @@ -35,6 +35,24 @@ public static function get_instance() { return $instance; } + /** + * Check if the provider is available for the given user. + * + * This method includes caching for WordPress.org, as it's called on most pageloads. + */ + public function is_available_for_user( $user ) { + $is_available = wp_cache_get( 'webauthn:' . $user->ID, 'users', false, $found ); + if ( $found ) { + return $is_available; + } + + $is_available = parent::is_available_for_user( $user ); + + wp_cache_set( 'webauthn:' . $user->ID, $is_available, 'users', HOUR_IN_SECONDS ); + + return $is_available; + } + /** * See https://github.com/sjinks/wp-two-factor-provider-webauthn/pull/468 * @@ -66,8 +84,11 @@ public function _add_filters() { public function _webauthn_ajax_request() { // Check the users session is still active and 2FA revalidation isn't required. if ( ! Two_Factor_Core::current_user_can_update_two_factor_options() ) { - wp_send_json_error( __( 'Your session has expired. Please refresh the page and try again.', 'wporg-two-factor' ) ); + wp_send_json_error( 'Your session has expired. Please refresh the page and try again.' ); } + + // Clear the caches for this class after the request is finished. + add_action( 'shutdown', [ $this, '_clear_cache' ] ); } /** @@ -78,8 +99,13 @@ public function _webauthn_ajax_request() { public function _show_user_security_settings() { $show_2fa_options = Two_Factor_Core::current_user_can_update_two_factor_options(); if ( ! $show_2fa_options ) { + // TODO: Perhaps the Core UI should extend it's `
` to wrap the additional providers? echo '
'; add_action( 'show_user_security_settings', function() { echo '
'; }, 1001 ); } } + + public function _clear_cache() { + wp_cache_delete( 'webauthn:' . get_current_user_id(), 'users' ); + } } From b91bb2d0dc811cfe3f609e499b049f8f571c688f Mon Sep 17 00:00:00 2001 From: Dion Hulse Date: Fri, 19 May 2023 04:52:15 +0000 Subject: [PATCH 6/7] Extend the 2FA revalidation after successfully adding a key, as adding a key gives account access. --- class-cached-webauthn-provider.php | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/class-cached-webauthn-provider.php b/class-cached-webauthn-provider.php index 9c724a17..d279321b 100644 --- a/class-cached-webauthn-provider.php +++ b/class-cached-webauthn-provider.php @@ -74,6 +74,9 @@ public function _add_filters() { // Disable the admin UI if it needs revalidation. add_action( 'show_user_security_settings', [ $this, '_show_user_security_settings' ], -1 ); + + // Extend the session revalidation after registering a new key. + add_action( 'wp_ajax_webauthn_register', [ $this, '_extend_revalidation' ], 1 ); } /** @@ -105,6 +108,23 @@ public function _show_user_security_settings() { } } + /** + * Extend the session revalidation after registering a new key. + */ + public function _extend_revalidation() { + ob_start( function( $output ) { + $json = json_decode( $output, true ); + if ( ! empty( $json['success'] ) ) { + // Bump session revalidation. + Two_Factor_Core::update_current_user_session( [ + 'two-factor-login' => time(), + ] ); + } + + return $output; + } ); + } + public function _clear_cache() { wp_cache_delete( 'webauthn:' . get_current_user_id(), 'users' ); } From 0a9be6891fb23a2c06fec329a418df522ec4d2e1 Mon Sep 17 00:00:00 2001 From: Dion Hulse Date: Fri, 19 May 2023 04:53:24 +0000 Subject: [PATCH 7/7] Rename the provider filename. --- ...d-webauthn-provider.php => class-wporg-webauthn-provider.php | 0 wporg-two-factor.php | 2 +- 2 files changed, 1 insertion(+), 1 deletion(-) rename class-cached-webauthn-provider.php => class-wporg-webauthn-provider.php (100%) diff --git a/class-cached-webauthn-provider.php b/class-wporg-webauthn-provider.php similarity index 100% rename from class-cached-webauthn-provider.php rename to class-wporg-webauthn-provider.php diff --git a/wporg-two-factor.php b/wporg-two-factor.php index 370210a2..1e7db3ac 100644 --- a/wporg-two-factor.php +++ b/wporg-two-factor.php @@ -303,7 +303,7 @@ function get_edit_account_url() : string { * Switch out the WebAuthN provider for one that uses a tiny bit of caching. */ add_filter( 'two_factor_provider_classname_TwoFactor_Provider_WebAuthn', function( string $provider ) : string { - require_once __DIR__ . '/class-cached-webauthn-provider.php'; + require_once __DIR__ . '/class-wporg-webauthn-provider.php'; return __NAMESPACE__ . '\WPORG_TwoFactor_Provider_WebAuthn'; } );