Store TOTP keys encrypted (#103)

* Add a Two_Factor_TOTP class variant that uses encryption. * Upgrade unencrypted keys to encrypted upon retrieval. * Switch to using authenticated encryption. The encrypted keys are only valid per user ID. * Tests: Test that the class is expected, but also that it returns the expected key. * Tests: Validate that the TOTP key is stored in an encrypted format in the user_meta. * Set the users TOTP key via the provider, to allow it to be encrypted. * Tests: Add a test that verifies that Secret keys are upgraded from stored non-encrypted to encrypted. --------- Co-authored-by: Paul Kevan <[email protected]>
WordPress · Apr 20, 2023 · aae7ec9 · aae7ec9
1 parent fbfafc0
commit aae7ec9
Show file tree

Hide file tree

Showing 3 changed files with 121 additions and 8 deletions.
diff --git a/class-encrypted-totp-provider.php b/class-encrypted-totp-provider.php
@@ -0,0 +1,53 @@
+<?php
+namespace WordPressdotorg\Two_Factor;
+use Two_Factor_Totp;
+
+/**
+ * Extends the default Two_Factor_Totp class to encrypt the TOTP key.
+ */
+class Encrypted_Totp_Provider extends Two_Factor_Totp {
+	/**
+	 * Use the parent class as the "key" in the Two Factor UI.
+	 */
+	public function get_key() {
+		return parent::class;
+	}
+
+	/**
+	 * When saving the key, encrypt it first.
+	 *
+	 * @param int    $user_id User ID.
+	 * @param string $key     TOTP key.
+	 * @return bool True if the key was saved, false otherwise.
+	 */
+	public function set_user_totp_key( $user_id, $key ) {
+		if ( function_exists( 'wporg_encrypt' ) ) {
+			$key = wporg_encrypt( $key, (string) $user_id, 'two-factor' );
+		}
+
+		return parent::set_user_totp_key( $user_id, (string) $key );
+	}
+
+	/**
+	 * When retrieving the key, decrypt it first.
+	 *
+	 * If the key isn't currently stored encrypted, it's upgraded to encrypted status.
+	 *
+	 * @param int $user_id User ID.
+	 * @return string|false TOTP key, or false if not set.
+	 */
+	public function get_user_totp_key( $user_id ) {
+		$key = parent::get_user_totp_key( $user_id );
+
+		if ( $key && function_exists( 'wporg_is_encrypted' ) ) {
+			if ( wporg_is_encrypted( $key ) ) {
+				$key = (string) wporg_decrypt( $key, (string) $user_id, 'two-factor' );
+			} else {
+				// Upgrade the key to be encrypted.
+				$this->set_user_totp_key( $user_id, $key );
+			}
+		}
+
+		return $key;
+	}
+}
diff --git a/tests/test-wporg-two-factor.php b/tests/test-wporg-two-factor.php
@@ -1,6 +1,7 @@
 <?php
 
 use function WordPressdotorg\Two_Factor\{ user_requires_2fa };
+use function WordPressdotorg\MU_Plugins\Encryption\{ generate_encryption_key };
 
 defined( 'WPINC' ) || die();
 
@@ -19,6 +20,17 @@ public static function wpSetUpBeforeClass( WP_UnitTest_Factory $factory ) : void
 			'user_login' => 'regular_user',
 			'role'       => 'contributor',
 		) );
+
+		// Generate an encryption key for testing with.
+		if ( ! function_exists( 'wporg_encryption_keys' ) ) {
+			function wporg_encryption_keys() {
+				static $keys = null;
+
+				return $keys ?? $keys = [
+					'two-factor' => generate_encryption_key(),
+				];
+			}
+		}
 	}
 
 	/**
@@ -53,7 +65,9 @@ protected function enable_2fa_for_user( int $user_id ) : void {
 		// This should start counting at one instead of zero, to match `Two_Factor_Core`.
 		update_user_meta( $user_id, Two_Factor_Core::ENABLED_PROVIDERS_USER_META_KEY, array( 1 => 'Two_Factor_Totp' ) );
 		update_user_meta( $user_id, Two_Factor_Core::PROVIDER_USER_META_KEY, 'Two_Factor_Totp' );
-		update_user_meta( $user_id, Two_Factor_Totp::SECRET_META_KEY, 'foo bar bax quiz' );
+
+		$totp_provider = Two_Factor_Core::get_providers()['Two_Factor_Totp'];
+		$totp_provider->set_user_totp_key( $user_id, $totp_provider->generate_key() );
 
 		$this->assertTrue( Two_Factor_Core::is_user_using_two_factor( $user_id ) );
 	}
@@ -225,19 +239,56 @@ public function test_set_primary_provider_for_user() {
 		$this->assertSame( $expected, $actual );
 
 		// Enable TOTP (as secondary).
-		$totp_provider = Two_Factor_Totp::get_instance();
-		$totp_provider->set_user_totp_key( self::$regular_user->ID, Two_Factor_Totp::generate_key() );
-		$enabled = Two_Factor_Core::enable_provider_for_user( self::$regular_user->ID, 'Two_Factor_Totp' );
-
-		$expected = 'Two_Factor_Totp';
-		$actual   = get_class( Two_Factor_Core::get_primary_provider_for_user( self::$regular_user->ID ) );
+		$totp_provider = Two_Factor_Core::get_providers()['Two_Factor_Totp'];
+		$totp_provider->set_user_totp_key( self::$regular_user->ID, $totp_provider->generate_key() );
+		$enabled       = Two_Factor_Core::enable_provider_for_user( self::$regular_user->ID, 'Two_Factor_Totp' );
 		$this->assertTrue( $enabled );
-		$this->assertSame( $expected, $actual );
+
+		// Validate that the TOTP key was stored in an encrypted form.
+		$totp_key       = $totp_provider->get_user_totp_key( self::$regular_user->ID );
+		$totp_user_meta = get_user_meta( self::$regular_user->ID, $totp_provider::SECRET_META_KEY, true );
+		$this->assertNotSame( $totp_key, $totp_user_meta );
+
+		// Validate that TOTP is now the primary provider.
+		$provider       = Two_Factor_Core::get_primary_provider_for_user( self::$regular_user->ID );
+		$expected_class = 'WordPressdotorg\Two_Factor\Encrypted_Totp_Provider';
+		$actual_class   = get_class( $provider );
+		$this->assertSame( $expected_class, $actual_class );
+
+		$expected_key = 'Two_Factor_Totp';
+		$actual_key   = $provider->get_key();
+		$this->assertSame( $expected_key, $provider->get_key() );
 
 		// Validate that Backup Codes are now available as secondary.
 		$expected = [ 'Two_Factor_Backup_Codes', 'Two_Factor_Totp' ];
 		$actual   = Two_Factor_Core::get_enabled_providers_for_user( self::$regular_user );
 
 		$this->assertSame( $expected, $actual );
 	}
+
+	/**
+	 * Verify that the TOTP key is encrypted if a non-encrypted key is encounted.
+	 *
+	 * @covers WordPressdotorg\Two_Factor\Encrypted_Totp_Provider::get_user_totp_key
+	 * @covers WordPressdotorg\Two_Factor\Encrypted_Totp_Provider::set_user_totp_key
+	 */
+	public function test_totp_key_upgraded_to_encrypted() {
+		$totp_provider = Two_Factor_Core::get_providers()['Two_Factor_Totp'];
+		$totp_key      = $totp_provider->generate_key();
+
+		// Set the user meta with the unencrypted key
+		update_user_meta( self::$regular_user->ID, Two_Factor_Totp::SECRET_META_KEY, $totp_key );
+		$meta_value = get_user_meta( self::$regular_user->ID, Two_Factor_Totp::SECRET_META_KEY, true );
+		$this->assertSame( $totp_key, $meta_value );
+
+		// Fetch the key, triggering the encryption upgrade.
+		$returned_key = $totp_provider->get_user_totp_key( self::$regular_user->ID );
+		$this->assertSame( $totp_key, $returned_key );
+
+		// Check the user meta has been encrypted.
+		$meta_value = get_user_meta( self::$regular_user->ID, Two_Factor_Totp::SECRET_META_KEY, true );
+		$this->assertNotSame( $totp_key, $meta_value );
+		$this->assertTrue( wporg_is_encrypted( $meta_value ) );
+
+	}
 }
diff --git a/wporg-two-factor.php b/wporg-two-factor.php
@@ -244,6 +244,15 @@ function get_edit_account_url() : string {
 	return $url;
 }
 
+/*
+ * Switch out the TOTP provider for one that encrypts the TOTP key.
+ */
+add_filter( 'two_factor_provider_classname_Two_Factor_Totp', function( string $provider ) : string {
+	require_once __DIR__ . '/class-encrypted-totp-provider.php';
+
+	return __NAMESPACE__ . '\Encrypted_Totp_Provider';
+} );
+
 // Temp fix for TOTP QR code being broken, see: https://meta.trac.wordpress.org/timeline?from=2023-02-21T04%3A40%3A07Z&precision=second.
 // Hotfix for https://github.com/WordPress/gutenberg/pull/48268
 add_filter( 'block_type_metadata', function( $metadata ) {