Be more tolerant of user input for auth codes

class-two-factor-core.php

@@ -804,8 +804,48 @@ public static function login_html( $user, $login_nonce, $redirect_to, $error_msg
 			#login form p.two-factor-prompt {
 			margin-bottom: 1em;
 			}
+			.input.authcode {
+				letter-spacing: .3em;
+			}
+			.input.authcode::placeholder {
+				opacity: 0.5;
+			}
 		</style>
-
+		<script>
+			(function() {
+				// Enforce numeric-only input for numeric inputmode elements.
+				const form = document.querySelector( '#loginform' ),
+					inputEl = document.querySelector( 'input.authcode[inputmode="numeric"]' ),
+					expectedLength = inputEl?.dataset.digits || 0;
+
+				if ( inputEl ) {
+					let spaceInserted = false;
+					inputEl.addEventListener(
+						'input',
+						function() {
+							let value = this.value.replace( /[^0-9 ]/g, '' ).trimStart();
+
+							if ( ! spaceInserted && expectedLength && value.length === Math.floor( expectedLength / 2 ) ) {
+								value += ' ';
+								spaceInserted = true;
+							} else if ( spaceInserted && ! this.value ) {
+								spaceInserted = false;
+							}
+
+							this.value = value;
+
+							// Auto-submit if it's the expected length.
+							if ( expectedLength && value.replace( / /g, '' ).length == expectedLength ) {
+								if ( undefined !== form.requestSubmit ) {
+									form.requestSubmit();
+									form.submit.disabled = "disabled";
+								}
+							}
+						}
+					);
+				}
+			})();
+		</script>
 		<?php
 		if ( ! function_exists( 'login_footer' ) ) {
 			include_once TWO_FACTOR_DIR . 'includes/function.login-footer.php';

providers/class-two-factor-backup-codes.php

@@ -330,7 +330,7 @@ public function authentication_page( $user ) {
 		<p class="two-factor-prompt"><?php esc_html_e( 'Enter a backup verification code.', 'two-factor' ); ?></p>
 		<p>
 			<label for="authcode"><?php esc_html_e( 'Verification Code:', 'two-factor' ); ?></label>
-			<input type="tel" name="two-factor-backup-code" id="authcode" class="input" value="" size="20" pattern="[0-9]*" />
+			<input type="text" inputmode="numeric" name="two-factor-backup-code" id="authcode" class="input authcode" value="" size="20" pattern="[0-9 ]*" placeholder="1234 5678" data-digits="8" />
 		</p>
 		<?php
 		submit_button( __( 'Submit', 'two-factor' ) );
@@ -347,7 +347,11 @@ public function authentication_page( $user ) {
 	 * @return boolean
 	 */
 	public function validate_authentication( $user ) {
-		$backup_code = isset( $_POST['two-factor-backup-code'] ) ? sanitize_text_field( wp_unslash( $_POST['two-factor-backup-code'] ) ) : '';
+		$backup_code = $this->sanitize_code_from_request( 'two-factor-backup-code' );
+		if ( ! $backup_code ) {
+			return false;
+		}
+
 		return $this->validate_code( $user, $backup_code );
 	}
 

providers/class-two-factor-email.php

@@ -269,7 +269,7 @@ public function authentication_page( $user ) {
 		<p class="two-factor-prompt"><?php esc_html_e( 'A verification code has been sent to the email address associated with your account.', 'two-factor' ); ?></p>
 		<p>
 			<label for="authcode"><?php esc_html_e( 'Verification Code:', 'two-factor' ); ?></label>
-			<input type="tel" name="two-factor-email-code" id="authcode" class="input" value="" size="20" />
+			<input type="text" inputmode="numeric" name="two-factor-email-code" id="authcode" class="input authcode" value="" size="20" pattern="[0-9 ]*" placeholder="1234 5678" data-digits="8" />
 			<?php submit_button( __( 'Log In', 'two-factor' ) ); ?>
 		</p>
 		<p class="two-factor-email-resend">
@@ -313,13 +313,11 @@ public function pre_process_authentication( $user ) {
 	 * @return boolean
 	 */
 	public function validate_authentication( $user ) {
-		if ( ! isset( $user->ID ) || ! isset( $_REQUEST['two-factor-email-code'] ) ) {
+		$code = $this->sanitize_code_from_request( 'two-factor-email-code' );
+		if ( ! isset( $user->ID ) || ! $code ) {
 			return false;
 		}
 
-		// Ensure there are no spaces or line breaks around the code.
-		$code = trim( sanitize_text_field( $_REQUEST['two-factor-email-code'] ) ); // phpcs:ignore WordPress.Security.NonceVerification.Recommended, handled by the core method already.
-
 		return $this->validate_token( $user->ID, $code );
 	}
 

providers/class-two-factor-provider.php

@@ -99,4 +99,27 @@ public static function get_code( $length = 8, $chars = '1234567890' ) {
 		}
 		return $code;
 	}
+
+	/**
+	 * Sanitizes a numeric code to be used as an auth code.
+	 *
+	 * @param string $field  The _REQUEST field to check for the code.
+	 * @param int    $length The valid expected length of the field.
+	 * @return false|string Auth code on success, false if the field is not set or not expected length.
+	 */
+	public static function sanitize_code_from_request( $field, $length = 0 ) {
+		if ( empty( $_REQUEST[ $field ] ) ) {
+			return false;
+		}
+
+		$code = wp_unslash( $_REQUEST[ $field ] ); // phpcs:ignore WordPress.Security.NonceVerification.Recommended, handled by the core method already.
+		$code = preg_replace( '/\s+/', '', $code );
+
+		// Maybe validate the length.
+		if ( $length && strlen( $code ) !== $length ) {
+			return false;
+		}
+
+		return (string) $code;
+	}
 }

providers/class-two-factor-totp.php

@@ -176,7 +176,7 @@ public function rest_setup_totp( $request ) {
 		$user    = get_user_by( 'id', $user_id );
 
 		$key  = $request['key'];
-		$code = $request['code'];
+		$code = preg_replace( '/\s+/', '', $request['code'] );
 
 		if ( ! $this->is_valid_key( $key ) ) {
 			return new WP_Error( 'invalid_key', __( 'Invalid Two Factor Authentication secret key.', 'two-factor' ), array( 'status' => 400 ) );
@@ -338,7 +338,11 @@ public function user_two_factor_options( $user ) {
 				<input type="hidden" id="two-factor-totp-key" name="two-factor-totp-key" value="<?php echo esc_attr( $key ); ?>" />
 				<label for="two-factor-totp-authcode">
 					<?php esc_html_e( 'Authentication Code:', 'two-factor' ); ?>
-					<input type="tel" name="two-factor-totp-authcode" id="two-factor-totp-authcode" class="input" value="" size="20" pattern="[0-9]*" />
+					<?php
+						/* translators: Example auth code. */
+						$placeholder = sprintf( __( 'eg. %s', 'two-factor' ), '123456' );
+					?>
+					<input type="tel" name="two-factor-totp-authcode" id="two-factor-totp-authcode" class="input" value="" size="20" pattern="[0-9 ]*" placeholder="<?php echo esc_attr( $placeholder ); ?>" />
 				</label>
 				<input type="submit" class="button totp-submit" name="two-factor-totp-submit" value="<?php esc_attr_e( 'Submit', 'two-factor' ); ?>" />
 			</p>
@@ -468,14 +472,12 @@ public function is_valid_key( $key ) {
 	 * @return bool Whether the user gave a valid code
 	 */
 	public function validate_authentication( $user ) {
-		if ( empty( $_REQUEST['authcode'] ) ) {
+		$code = $this->sanitize_code_from_request( 'authcode', self::DEFAULT_DIGIT_COUNT );
+		if ( ! $code ) {
 			return false;
 		}
 
-		return $this->validate_code_for_user(
-			$user,
-			sanitize_text_field( $_REQUEST['authcode'] )
-		);
+		return $this->validate_code_for_user( $user, $code );
 	}
 
 	/**
@@ -667,7 +669,7 @@ public function authentication_page( $user ) {
 		</p>
 		<p>
 			<label for="authcode"><?php esc_html_e( 'Authentication Code:', 'two-factor' ); ?></label>
-			<input type="tel" autocomplete="one-time-code" name="authcode" id="authcode" class="input" value="" size="20" pattern="[0-9]*" />
+			<input type="text" inputmode="numeric" autocomplete="one-time-code" name="authcode" id="authcode" class="input authcode" value="" size="20" pattern="[0-9 ]*" placeholder="123 456" data-digits="<?php echo esc_attr( self::DEFAULT_DIGIT_COUNT ); ?>" />
 		</p>
 		<script type="text/javascript">
 			setTimeout( function(){

tests/providers/class-two-factor-backup-codes.php

@@ -69,13 +69,17 @@ public function test_authentication_page() {
 	 * @covers Two_Factor_Backup_Codes::validate_authentication
 	 */
 	public function test_validate_authentication() {
-		$user                            = new WP_User( self::factory()->user->create() );
-		$code                            = $this->provider->generate_codes( $user, array( 'number' => 1 ) );
-		$_POST['two-factor-backup-code'] = $code[0];
+		$user = new WP_User( self::factory()->user->create() );
+		$code = $this->provider->generate_codes( $user, array( 'number' => 1 ) );
+
+		// Verify authentication correctly fails without a set code.
+		$this->assertFalse( $this->provider->validate_authentication( $user ) );
 
+		// Set the code and verify it passes.
+		$_REQUEST['two-factor-backup-code'] = $code[0];
 		$this->assertTrue( $this->provider->validate_authentication( $user ) );
 
-		unset( $_POST['two-factor-backup-code'] );
+		unset( $_REQUEST['two-factor-backup-code'] );
 	}
 
 	/**

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

@@ -34,4 +34,35 @@ function test_get_code() {
 		$code = Two_Factor_Provider::get_code( 8 );
 		$this->assertEquals( 8, strlen( $code ) );
 	}
+
+	/**
+	 * Validate that sanitize_code_from_request() works as intended.
+	 *
+	 * @covers Two_Factor_Provider::sanitize_code_from_request
+	 * @dataProvider provider_sanitize_code_from_request
+	 */
+	function test_sanitize_code_from_request( $expected, $field, $value, $length = 0) {
+		$_REQUEST[ $field ] = '';
+		if ( $value ) {
+			$_REQUEST[ $field ] = $value;
+		}
+
+		$this->assertEquals( $expected, Two_Factor_Provider::sanitize_code_from_request( $field, $length ) );
+
+		unset( $_REQUEST[ $field ] );
+	}
+
+	function provider_sanitize_code_from_request() {
+		return [
+			[ '123123', 'authcode', '123123', 6 ],
+			[ false, 'authcode', '123123123', 6 ],
+			[ '123123', 'code', '123 123' ],
+			[ '123123', 'code', "\n123123\n" ],
+			[ '123123', 'code', "123\t123", 6 ],
+			[ false, 'code', '' ],
+			[ 'helloworld', 'code', 'helloworld' ],
+			[ false, false, false ],
+		];
+	}
+
 }

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

@@ -244,6 +244,33 @@ function test_validate_authentication() {
 		$this->assertFalse( $this->provider->validate_authentication( $user ) );
 	}
 
+	/**
+	 * Validate that a TOTP code works even if presented with spaces, but invalid characters cause a rejection.
+	 *
+	 * @covers Two_Factor_Totp::validate_authentication
+	 */
+	function test_validate_authentication_invalid_chars_spaces() {
+		$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 an authcode with HTML in the string is not accepted.
+		$_REQUEST['authcode'] = '<strong>' . $authcode . '</strong>';
+		$this->assertFalse( $this->provider->validate_authentication( $user ), $_REQUEST['authcode'] );
+
+		// Validate that an authcode with leading encoded spaces aren't accepted.
+		$_REQUEST['authcode'] = '%20%20' . $authcode;
+		$this->assertFalse( $this->provider->validate_authentication( $user ), $_REQUEST['authcode'] );
+
+		// Validate that an authcode with leading, trailing, and middle whitespace is accepted.
+		$_REQUEST['authcode'] = ' ' . substr( $authcode, 0, 3 ) . ' ' . substr( $authcode, 3 ) . " \n"; // eg ' 123 456 \n'
+		$this->assertTrue( $this->provider->validate_authentication( $user ), $_REQUEST['authcode'] );
+	}
+
 	/**
 	 * Test that the validation function works.
 	 *