TOTP: Enforce single-use of TOTP one-time passwords.

providers/class-two-factor-totp.php

@@ -11,12 +11,19 @@
 class Two_Factor_Totp extends Two_Factor_Provider {
 
 	/**
-	 * The user meta token key.
+	 * The user meta key for the TOTP Secret key.
 	 *
 	 * @var string
 	 */
 	const SECRET_META_KEY = '_two_factor_totp_key';
 
+	/**
+	 * The user meta key for the last successful TOTP token timestamp logged in with.
+	 *
+	 * @var string
+	 */
+	const LAST_SUCCESSFUL_LOGIN_META_KEY = '_two_factor_totp_last_successful_login';
+
 	const DEFAULT_KEY_BIT_SIZE        = 160;
 	const DEFAULT_CRYPTO              = 'sha1';
 	const DEFAULT_DIGIT_COUNT         = 6;
@@ -408,6 +415,7 @@ public function set_user_totp_key( $user_id, $key ) {
 	 * @return boolean If the key was deleted successfully.
 	 */
 	public function delete_user_totp_key( $user_id ) {
+		delete_user_meta( $user_id, self::LAST_SUCCESSFUL_LOGIN_META_KEY );
 		return delete_user_meta( $user_id, self::SECRET_META_KEY );
 	}
 
@@ -434,29 +442,70 @@ public function is_valid_key( $key ) {
 	 * @param WP_User $user WP_User object of the logged-in user.
 	 *
 	 * @return bool Whether the user gave a valid code
-	 *
-	 * @codeCoverageIgnore
 	 */
 	public function validate_authentication( $user ) {
-		if ( ! empty( $_REQUEST['authcode'] ) ) {
-			return $this->is_valid_authcode(
-				$this->get_user_totp_key( $user->ID ),
-				sanitize_text_field( $_REQUEST['authcode'] )
-			);
+		if ( empty( $_REQUEST['authcode'] ) ) {
+			return false;
 		}
 
-		return false;
+		return $this->validate_code_for_user(
+			$user,
+			sanitize_text_field( $_REQUEST['authcode'] )
+		);
 	}
 
 	/**
-	 * Checks if a given code is valid for a given key, allowing for a certain amount of time drift
+	 * Validates a authentication code for a given user, preventing re-use and older TOTP keys.
+	 *
+	 * @param WP_User $user WP_User object of the logged-in user.
+	 * @param int     $code The TOTP token to validate.
+	 *
+	 * @return bool Whether the code is valid for the user and a newer code has not been used.
+	 */
+	public function validate_code_for_user( $user, $code ) {
+		$valid_timestamp = $this->get_authcode_valid_ticktime(
+			$this->get_user_totp_key( $user->ID ),
+			$code,
+		);
+
+		if ( ! $valid_timestamp ) {
+			return false;
+		}
+
+		$last_totp_login = (int) get_user_meta( $user->ID, self::LAST_SUCCESSFUL_LOGIN_META_KEY, true );
+
+		// The TOTP authentication is not valid, if we've seen the same or newer code.
+		if ( $last_totp_login && $last_totp_login >= $valid_timestamp ) {
+			return false;
+		}
+
+		update_user_meta( $user->ID, self::LAST_SUCCESSFUL_LOGIN_META_KEY, $valid_timestamp );
+
+		return true;
+	}
+
+
+	/**
+	 * Checks if a given code is valid for a given key, allowing for a certain amount of time drift.
 	 *
 	 * @param string $key      The share secret key to use.
 	 * @param string $authcode The code to test.
 	 *
-	 * @return bool Whether the code is valid within the time frame
+	 * @return bool Whether the code is valid within the time frame.
 	 */
 	public static function is_valid_authcode( $key, $authcode ) {
+		return (bool) self::get_authcode_valid_ticktime( $key, $authcode );
+	}
+
+	/**
+	 * Checks if a given code is valid for a given key, allowing for a certain amount of time drift.
+	 *
+	 * @param string $key      The share secret key to use.
+	 * @param string $authcode The code to test.
+	 *
+	 * @return false|int Returns the timestamp of the authcode on success, False otherwise.
+	 */
+	public static function get_authcode_valid_ticktime( $key, $authcode ) {
 		/**
 		 * Filter the maximum ticks to allow when checking valid codes.
 		 *
@@ -478,10 +527,12 @@ public static function is_valid_authcode( $key, $authcode ) {
 
 		foreach ( $ticks as $offset ) {
 			$log_time = $time + $offset;
-			if ( hash_equals(self::calc_totp( $key, $log_time ), $authcode ) ) {
-				return true;
+			if ( hash_equals( self::calc_totp( $key, $log_time ), $authcode ) ) {
+				// Return the tick timestamp.
+				return $log_time * self::DEFAULT_TIME_STEP_SEC;
 			}
 		}
+
 		return false;
 	}
 

tests/providers/class-two-factor-totp.php

@@ -140,6 +140,7 @@ public function test_base32_decode_exception() {
 	 * Verify authcode validation.
 	 *
 	 * @covers Two_Factor_Totp::is_valid_authcode
+	 * @covers Two_Factor_Totp::get_authcode_valid_ticktime
 	 * @covers Two_Factor_Totp::generate_key
 	 * @covers Two_Factor_Totp::calc_totp
 	 * @covers Two_Factor_Totp::pack64
@@ -157,6 +158,7 @@ public function test_is_valid_authcode() {
 	 * Verify authcode rejection.
 	 *
 	 * @covers Two_Factor_Totp::is_valid_authcode
+	 * @covers Two_Factor_Totp::get_authcode_valid_ticktime
 	 */
 	public function test_invalid_authcode_rejected() {
 		$key = $this->provider->generate_key();
@@ -208,4 +210,72 @@ public function test_is_valid_key() {
 		$this->assertFalse( $this->provider->is_valid_key( 'abc233' ), 'Lowercase chars are invalid' );
 		$this->assertFalse( $this->provider->is_valid_key( 'has a space' ), 'Spaces not allowed' );
 	}
+
+	/**
+	 * Test that the validation function works.
+	 *
+	 * @covers Two_Factor_Totp::validate_authentication
+	 * @covers Two_Factor_Totp::validate_code_for_user
+	 * @covers Two_Factor_Totp::get_authcode_valid_ticktime
+	 */
+	function test_validate_authentication() {
+		$user = new WP_User( self::factory()->user->create() );
+		$key  = $this->provider->generate_key();
+
+		// Configure secret for the user.
+		$this->provider->set_user_totp_key( $user->ID, $key );
+
+		$authcode = $this->provider->calc_totp( $key );
+
+		// Validate that no key doesn't suceed.
+		unset( $_REQUEST['authcode'] );
+		$this->assertFalse( $this->provider->validate_authentication( $user ) );
+
+		// Validate that an invalid key doesn't succeed.
+		$_REQUEST['authcode'] = '123456'; // Okay, that's valid once in a blue moon.
+		$this->assertFalse( $this->provider->validate_authentication( $user ) );
+
+		// Validate that the login would succeed using the current authcode.
+		$_REQUEST['authcode'] = $authcode;
+		$this->assertTrue( $this->provider->validate_authentication( $user ) );
+
+		// Validate that a second attempt with the same authcode will fail.
+		$this->assertFalse( $this->provider->validate_authentication( $user ) );
+	}
+
+	/**
+	 * Test that the validation function works.
+	 *
+	 * @covers Two_Factor_Totp::validate_code_for_user
+	 * @covers Two_Factor_Totp::get_authcode_valid_ticktime
+	 */
+	function test_validate_code_for_user() {
+		$user = new WP_User( self::factory()->user->create() );
+		$key  = $this->provider->generate_key();
+
+		// Configure secret for the user.
+		$this->provider->set_user_totp_key( $user->ID, $key );
+
+		$oldcode  = $this->provider->calc_totp( $key, floor( time() / Two_Factor_Totp::DEFAULT_TIME_STEP_SEC ) - 2 );
+		$prevcode = $this->provider->calc_totp( $key, floor( time() / Two_Factor_Totp::DEFAULT_TIME_STEP_SEC ) - 1 );
+		$authcode = $this->provider->calc_totp( $key );
+		$nextcode = $this->provider->calc_totp( $key, floor( time() / Two_Factor_Totp::DEFAULT_TIME_STEP_SEC ) + 1 );
+
+		// Validate that the login would succeed using the previous authcode.
+		$this->assertTrue( $this->provider->validate_code_for_user( $user, $prevcode ) );
+
+		// Validate that the login would succeed using the current authcode.
+		$this->assertTrue( $this->provider->validate_code_for_user( $user, $authcode ) );
+
+		// Validate that a second attempt with the same authcode will fail.
+		$this->assertFalse( $this->provider->validate_code_for_user( $user, $authcode ) );
+
+		// Validate that the future authcode will succeed (but not more than once)
+		$this->assertTrue( $this->provider->validate_code_for_user( $user, $nextcode ) );
+		$this->assertFalse( $this->provider->validate_code_for_user( $user, $nextcode ) );
+
+		// Validate that the older unused authcode will not succeed.
+		$this->assertFalse( $this->provider->validate_code_for_user( $user, $oldcode ) );
+
+	}
 }