App Passwords: Improve validation and sanitization of the application name.

Application names are now required to be unique and cannot contain solely whitespace characters. Additionally, invalid characters are now stripped from the application name using `sanitize_text_field()`.

Props Boniu91, hellofromTonya, engahmeds3ed, xkon, francina.
Fixes #51941.


git-svn-id: https://develop.svn.wordpress.org/trunk@50030 602fd350-edb4-49c9-b593-d223f7449a82
This commit is contained in:
Timothy Jacobs 2021-01-27 19:03:42 +00:00
parent 52d751751a
commit e1c98a744b
7 changed files with 247 additions and 4 deletions

View File

@ -57,7 +57,7 @@
$newAppPassButton.prop( 'disabled', false );
$newAppPassForm.after( tmplNewAppPass( {
name: name,
name: response.name,
password: response.password
} ) );
$( '.new-application-password-notice' ).focus();

View File

@ -98,7 +98,7 @@
.append( '<p>' + wp.i18n.__( 'Be sure to save this in a safe location. You will not be able to retrieve it.' ) + '</p>' );
// We're using .text() to write the variables to avoid any chance of XSS.
$( 'strong', $notice ).text( name );
$( 'strong', $notice ).text( response.name );
$( 'input', $notice ).val( response.password );
$form.replaceWith( $notice );

View File

@ -58,6 +58,7 @@ class WP_Application_Passwords {
* Creates a new application password.
*
* @since 5.6.0
* @since 5.7.0 Returns WP_Error if application name already exists.
*
* @param int $user_id User ID.
* @param array $args Information about the application password.
@ -65,8 +66,16 @@ class WP_Application_Passwords {
* A WP_Error instance is returned on error.
*/
public static function create_new_application_password( $user_id, $args = array() ) {
if ( ! empty( $args['name'] ) ) {
$args['name'] = sanitize_text_field( $args['name'] );
}
if ( empty( $args['name'] ) ) {
return new WP_Error( 'application_password_empty_name', __( 'An application name is required to create an application password.' ) );
return new WP_Error( 'application_password_empty_name', __( 'An application name is required to create an application password.' ), array( 'status' => 400 ) );
}
if ( self::application_name_exists_for_user( $user_id, $args['name'] ) ) {
return new WP_Error( 'application_password_duplicate_name', __( 'Each application name should be unique.' ), array( 'status' => 409 ) );
}
$new_password = wp_generate_password( static::PW_LENGTH, false );
@ -162,6 +171,28 @@ class WP_Application_Passwords {
return null;
}
/**
* Check if application name exists before for this user.
*
* @since 5.7.0
*
* @param int $user_id User ID.
* @param string $name Application name.
*
* @return bool Provided application name exists or not.
*/
public static function application_name_exists_for_user( $user_id, $name ) {
$passwords = static::get_user_application_passwords( $user_id );
foreach ( $passwords as $password ) {
if ( strtolower( $password['name'] ) === strtolower( $name ) ) {
return true;
}
}
return false;
}
/**
* Updates an application password.
*
@ -180,6 +211,10 @@ class WP_Application_Passwords {
continue;
}
if ( ! empty( $update['name'] ) ) {
$update['name'] = sanitize_text_field( $update['name'] );
}
$save = false;
if ( ! empty( $update['name'] ) && $item['name'] !== $update['name'] ) {

View File

@ -631,6 +631,8 @@ class WP_REST_Application_Passwords_Controller extends WP_REST_Controller {
'type' => 'string',
'required' => true,
'context' => array( 'view', 'edit', 'embed' ),
'minLength' => 1,
'pattern' => '.*\S.*',
),
'password' => array(
'description' => __( 'The generated password. Only available after adding an application.' ),

View File

@ -0,0 +1,201 @@
<?php
/**
* Unit tests covering WP_Application_Passwords functionality.
*
* @package WordPress
* @subpackage REST API
*/
/**
* @group restapi
* @group app_password
*/
class Test_WP_Application_Passwords extends WP_UnitTestCase {
/**
* Administrator user id.
*
* @var int
*/
private static $user_id;
public static function wpSetUpBeforeClass( WP_UnitTest_Factory $factory ) {
self::$user_id = $factory->user->create(
array(
'role' => 'administrator',
)
);
if ( is_multisite() ) {
grant_super_admin( self::$user_id );
}
}
/**
* @covers WP_Application_Passwords::create_new_application_password
* @ticket 51941
* @dataProvider data_create_new_application_password_validation
*/
public function test_create_new_application_password_validation( $expected, array $args = array(), array $names = array() ) {
// Create the existing passwords.
foreach ( $names as $name ) {
WP_Application_Passwords::create_new_application_password( self::$user_id, array( 'name' => $name ) );
}
$actual = WP_Application_Passwords::create_new_application_password( self::$user_id, $args );
$this->assertInstanceOf( WP_Error::class, $actual );
$this->assertSame( $expected['error_code'], $actual->get_error_code() );
$this->assertSame( $expected['error_message'], $actual->get_error_message( $expected['error_code'] ) );
}
public function data_create_new_application_password_validation() {
return array(
'application_password_empty_name when no args' => array(
'expected' => array(
'error_code' => 'application_password_empty_name',
'error_message' => 'An application name is required to create an application password.',
),
),
'application_password_empty_name when no name' => array(
'expected' => array(
'error_code' => 'application_password_empty_name',
'error_message' => 'An application name is required to create an application password.',
),
'args' => array( 'app_id' => 1 ),
),
'application_password_empty_name when empty name' => array(
'expected' => array(
'error_code' => 'application_password_empty_name',
'error_message' => 'An application name is required to create an application password.',
),
'args' => array( 'name' => ' ' ),
),
'application_password_empty_name when <script>' => array(
'expected' => array(
'error_code' => 'application_password_empty_name',
'error_message' => 'An application name is required to create an application password.',
),
'args' => array( 'name' => '<script>console.log("Hello")</script>' ),
),
'application_password_duplicate_name when name exists' => array(
'expected' => array(
'error_code' => 'application_password_duplicate_name',
'error_message' => 'Each application name should be unique.',
),
'args' => array( 'name' => 'test2' ),
'names' => array( 'test1', 'test2' ),
),
);
}
/**
* @covers WP_Application_Passwords::create_new_application_password
* @ticket 51941
* @dataProvider data_create_new_application_password
*/
public function test_create_new_application_password( array $args, array $names = array() ) {
// Create the existing passwords.
foreach ( $names as $name ) {
WP_Application_Passwords::create_new_application_password( self::$user_id, array( 'name' => $name ) );
}
list( $new_password, $new_item ) = WP_Application_Passwords::create_new_application_password( self::$user_id, $args );
$this->assertNotEmpty( $new_password );
$this->assertSame(
array( 'uuid', 'app_id', 'name', 'password', 'created', 'last_used', 'last_ip' ),
array_keys( $new_item )
);
$this->assertSame( $args['name'], $new_item['name'] );
}
public function data_create_new_application_password() {
return array(
'should create new password when no passwords exists' => array(
'args' => array( 'name' => 'test3' ),
),
'should create new password when name is unique' => array(
'args' => array( 'name' => 'test3' ),
'names' => array( 'test1', 'test2' ),
),
);
}
/**
* @covers WP_Application_Passwords::application_name_exists_for_user
* @ticket 51941
* @dataProvider data_application_name_exists_for_user
*/
public function test_application_name_exists_for_user( $expected, $name ) {
if ( $expected ) {
WP_Application_Passwords::create_new_application_password( self::$user_id, array( 'name' => $name ) );
}
$this->assertSame( $expected, WP_Application_Passwords::application_name_exists_for_user( self::$user_id, $name ) );
}
public function data_application_name_exists_for_user() {
return array(
array( false, 'test1' ),
array( false, 'baz' ),
array( false, 'bar' ),
array( true, 'App 1' ),
array( true, 'Some Test' ),
array( true, 'Baz' ),
);
}
/**
* @covers WP_Application_Passwords::update_application_password
* @ticket 51941
* @dataProvider data_update_application_password
*/
public function test_update_application_password( array $update, array $existing ) {
// Create the original item.
list( , $original_item ) = WP_Application_Passwords::create_new_application_password( self::$user_id, $existing );
$uuid = $original_item['uuid'];
$actual = WP_Application_Passwords::update_application_password( self::$user_id, $uuid, $update );
$this->assertTrue( $actual );
// Check updated only given values.
$updated_item = WP_Application_Passwords::get_user_application_password( self::$user_id, $uuid );
foreach ( $updated_item as $key => $update_value ) {
$expected_value = isset( $update[ $key ] ) ? $update[ $key ] : $original_item[ $key ];
$this->assertSame( $expected_value, $update_value );
}
}
/**
* @covers WP_Application_Passwords::update_application_password
* @ticket 51941
* @dataProvider data_update_application_password
*/
public function test_update_application_password_when_no_password_found( array $update ) {
$actual = WP_Application_Passwords::update_application_password( self::$user_id, '', $update );
$this->assertInstanceOf( WP_Error::class, $actual );
$this->assertSame( 'application_password_not_found', $actual->get_error_code() );
$this->assertSame( 'Could not find an application password with that id.', $actual->get_error_message( 'application_password_not_found' ) );
}
public function data_update_application_password() {
return array(
'should not update when no values given to update' => array(
'update' => array(),
'existing' => array( 'name' => 'Test' ),
),
'should not update when given same name' => array(
'update' => array( 'name' => 'Test' ),
'existing' => array( 'name' => 'Test' ),
),
'should update name' => array(
'update' => array( 'name' => 'Test Updated' ),
'existing' => array( 'name' => 'Test' ),
),
);
}
}

View File

@ -538,6 +538,7 @@ class WP_Test_REST_Application_Passwords_Controller extends WP_Test_REST_Control
/**
* @ticket 51583
* @ticket 51941
*/
public function test_update_item_cannot_overwrite_app_id() {
wp_set_current_user( self::$admin );
@ -554,7 +555,7 @@ class WP_Test_REST_Application_Passwords_Controller extends WP_Test_REST_Control
list( , $item ) = WP_Application_Passwords::create_new_application_password(
self::$admin,
array(
'name' => 'App',
'name' => 'App 2',
'app_id' => $app_id,
)
);

View File

@ -4974,6 +4974,8 @@ mockedApiResponse.Schema = {
"name": {
"description": "The name of the application password.",
"type": "string",
"minLength": 1,
"pattern": ".*\\S.*",
"required": true
}
}
@ -5030,6 +5032,8 @@ mockedApiResponse.Schema = {
"name": {
"description": "The name of the application password.",
"type": "string",
"minLength": 1,
"pattern": ".*\\S.*",
"required": false
}
}