Remove duplicate two_factor_providers filter calls to allow disabling core providers

class-two-factor-core.php

@@ -149,7 +149,22 @@ public static function uninstall() {
 
 		$option_keys = array();
 
-		foreach ( self::get_providers_classes() as $provider_class ) {
+		$providers = self::get_default_providers();
+
+		/**
+		 * Include additional providers to be uninstalled.
+		 *
+		 * @param array $providers A key-value array where the key is the class name, and
+		 *                         the value is the path to the file containing the class.
+		 */
+		$additional_providers = apply_filters( 'two_factor_providers', $providers );
+
+		// Merge them with the default providers.
+		if ( ! empty( $additional_providers ) ) {
+			$providers = array_merge( $providers, $additional_providers );
+		}
+
+		foreach ( self::get_providers_classes( $providers ) as $provider_class ) {
 			// Merge with provider-specific user meta keys.
 			if ( method_exists( $provider_class, 'uninstall_user_meta_keys' ) ) {
 				try {
@@ -192,43 +207,28 @@ public static function uninstall() {
 	 *
 	 * @return array List of provider keys and paths to class files.
 	 */
-	public static function get_providers_registered() {
-		$providers = array(
+	private static function get_default_providers() {
+		return array(
 			'Two_Factor_Email'        => TWO_FACTOR_DIR . 'providers/class-two-factor-email.php',
 			'Two_Factor_Totp'         => TWO_FACTOR_DIR . 'providers/class-two-factor-totp.php',
 			'Two_Factor_FIDO_U2F'     => TWO_FACTOR_DIR . 'providers/class-two-factor-fido-u2f.php',
 			'Two_Factor_Backup_Codes' => TWO_FACTOR_DIR . 'providers/class-two-factor-backup-codes.php',
 			'Two_Factor_Dummy'        => TWO_FACTOR_DIR . 'providers/class-two-factor-dummy.php',
 		);
-
-		/**
-		 * Filter the supplied providers.
-		 *
-		 * @param array $providers A key-value array where the key is the class name, and
-		 *                         the value is the path to the file containing the class.
-		 */
-		$additional_providers = apply_filters( 'two_factor_providers', $providers );
-
-		// Merge them with the default providers.
-		if ( ! empty( $additional_providers ) ) {
-			return array_merge( $providers, $additional_providers );
-		}
-
-		return $providers;
 	}
 
 	/**
-	 * Get the classnames for all registered providers.
+	 * Get the classnames for specific providers.
 	 *
-	 * Note some of these providers might not be enabled.
+	 * @param array $providers List of paths to provider class files indexed by class names.
 	 *
 	 * @return array List of provider keys and classnames.
 	 */
-	private static function get_providers_classes() {
-		$providers = self::get_providers_registered();
-
+	private static function get_providers_classes( $providers ) {
 		foreach ( $providers as $provider_key => $path ) {
-			require_once $path;
+			if ( ! empty( $path ) && is_readable( $path ) ) {
+				require_once $path;
+			}
 
 			$class = $provider_key;
 
@@ -241,9 +241,9 @@ private static function get_providers_classes() {
 			$class = apply_filters( "two_factor_provider_classname_{$provider_key}", $class, $path );
 
 			/**
-			 * Confirm that it's been successfully included before instantiating.
+			 * Confirm that it's been successfully included.
 			 */
-			if ( method_exists( $class, 'get_instance' ) ) {
+			if ( class_exists( $class ) ) {
 				$providers[ $provider_key ] = $class;
 			} else {
 				unset( $providers[ $provider_key ] );
@@ -261,7 +261,7 @@ private static function get_providers_classes() {
 	 * @return array
 	 */
 	public static function get_providers() {
-		$providers = self::get_providers_registered();
+		$providers = self::get_default_providers();
 
 		/**
 		 * Filter the supplied providers.
@@ -287,15 +287,13 @@ public static function get_providers() {
 		}
 
 		// Map provider keys to classes so that we can instantiate them.
-		$providers = array_intersect_key( self::get_providers_classes(), $providers );
+		$providers = self::get_providers_classes( $providers );
 
 		foreach ( $providers as $provider_key => $provider_class ) {
-			if ( method_exists( $provider_class, 'get_instance' ) ) {
-				try {
-					$providers[ $provider_key ] = call_user_func( array( $provider_class, 'get_instance' ) );
-				} catch ( Exception $e ) {
-					unset( $providers[ $provider_key ] );
-				}
+			try {
+				$providers[ $provider_key ] = call_user_func( array( $provider_class, 'get_instance' ) );
+			} catch ( Exception $e ) {
+				unset( $providers[ $provider_key ] );
 			}
 		}
 

tests/class-two-factor-core.php

@@ -1581,4 +1581,41 @@ public function test_uninstall_removes_user_meta() {
 			'Provider was disabled due to uninstall'
 		);
 	}
+
+	/**
+	 * Plugin uninstall removes all user meta even for disabled providers.
+	 *
+	 * @covers Two_Factor_Core::uninstall
+	 */
+	public function test_uninstall_removes_disabled_provider_user_meta() {
+		$user = self::factory()->user->create_and_get();
+
+		// Enable a provider for the user.
+		Two_Factor_Core::enable_provider_for_user( $user->ID, 'Two_Factor_Totp' );
+
+		$totp_provider = Two_Factor_Totp::get_instance();
+
+		$totp_provider->set_user_totp_key( $user->ID, 'some_key' );
+
+		$this->assertEquals( 'some_key', $totp_provider->get_user_totp_key( $user->ID ), 'TOTP secret was set for user' );
+
+		add_filter(
+			'two_factor_providers',
+			function( $providers ) {
+				return array_diff_key( $providers, array( 'Two_Factor_Totp' => null ) );
+			}
+		);
+
+		$this->assertNotContains(
+			'Two_Factor_Totp',
+			Two_Factor_Core::get_enabled_providers_for_user( $user->ID ),
+			'TOTP provider is disabled for everyone via filter'
+		);
+
+		Two_Factor_Core::uninstall();
+
+		$this->assertEmpty( $totp_provider->get_user_totp_key( $user->ID ), 'TOTP secret was deleted during uninstall' );
+
+		remove_all_filters( 'two_factor_providers' );
+	}
 }