Skip to content

REST: harden guest author username collision handling #177

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
dknauss wants to merge 1 commit into
base: develop
Choose a base branch
from
Draft

REST: harden guest author username collision handling #177

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
72 changes: 63 additions & 9 deletions inc/class-users-controller.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ class Users_Controller extends WP_REST_Users_Controller {

const _NAMESPACE = 'authorship/v1';
const BASE = 'users';
const DEFAULT_GUEST_USERNAME = 'guestauthor';
const MAX_USERNAME_LENGTH = 60;

/**
* Constructor.
Expand Down Expand Up @@ -191,8 +193,10 @@ public function create_item_permissions_check( $request ) {
* @return WP_REST_Response|WP_Error Response object on success, or WP_Error object on failure.
*/
public function create_item( $request ) {
$username = sanitize_title( sanitize_user( $request->get_param( 'name' ), true ) );
$username = preg_replace( '/[^a-z0-9]/', '', $username );
$name_param = $request->get_param( 'name' );
$name = is_string( $name_param ) ? $name_param : '';
$username = $this->normalize_guest_username( $name );
$username = $this->get_unique_guest_username( $username );

$request->set_param( 'username', $username );

Expand All @@ -208,15 +212,65 @@ public function create_item( $request ) {
* @type WP_Error $errors WP_Error object containing any errors found.
* }
*/
add_filter( 'wpmu_validate_user_signup', function( array $result ) : array {
/** @var WP_Error $errors */
$errors = $result['errors'];
$errors->remove( 'user_email' );
add_filter( 'wpmu_validate_user_signup', [ $this, 'filter_guest_author_signup_validation' ] );

try {
return parent::create_item( $request );
} finally {
remove_filter( 'wpmu_validate_user_signup', [ $this, 'filter_guest_author_signup_validation' ] );
}
}

/**
* Filters validated signup data for guest author creation.
*
* @param mixed[] $result Signup validation result.
* @return mixed[] Signup validation result.
*/
public function filter_guest_author_signup_validation( array $result ) : array {
if ( isset( $result['errors'] ) && $result['errors'] instanceof WP_Error ) {
$result['errors']->remove( 'user_email' );
}

return $result;
}

/**
* Normalizes a guest author username from the provided name.
*
* @param string $name Guest author display name.
* @return string Normalized username candidate.
*/
protected function normalize_guest_username( string $name ) : string {
$username = sanitize_title( sanitize_user( $name, true ) );
$username = preg_replace( '/[^a-z0-9]/', '', $username );

if ( ! is_string( $username ) || '' === $username ) {
$username = self::DEFAULT_GUEST_USERNAME;
}

return $result;
} );
return substr( $username, 0, self::MAX_USERNAME_LENGTH );
}

/**
* Ensures the guest author username is unique.
*
* @param string $username Username candidate.
* @return string Unique username.
*/
protected function get_unique_guest_username( string $username ) : string {
$candidate = $username;
$suffix = 2;

while ( username_exists( $candidate ) ) {
$suffix_string = (string) $suffix;
$max_base_length = max( 1, self::MAX_USERNAME_LENGTH - strlen( $suffix_string ) - 1 );
$base = substr( $username, 0, $max_base_length );
$candidate = "{$base}-{$suffix_string}";
++$suffix;
}

return parent::create_item( $request );
return $candidate;
}

/**
Expand Down
47 changes: 47 additions & 0 deletions tests/phpunit/test-rest-api-user-endpoint.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,53 @@ public function testGuestAuthorCanBeCreatedWithJustAName() : void {
$this->assertSame( [ GUEST_ROLE ], $data['roles'] );
}

public function testGuestAuthorDuplicateNameGetsUniqueUsername() : void {
wp_set_current_user( self::$users['editor']->ID );

$request = new WP_REST_Request( 'POST', self::$route );
$request->set_param( 'name', 'Duplicate Name' );

$first_response = self::rest_do_request( $request );
$first_data = $first_response->get_data();
$first_message = self::get_message( $first_response );

$this->assertSame( WP_Http::CREATED, $first_response->get_status(), $first_message );

$request = new WP_REST_Request( 'POST', self::$route );
$request->set_param( 'name', 'Duplicate Name' );

$second_response = self::rest_do_request( $request );
$second_data = $second_response->get_data();
$second_message = self::get_message( $second_response );

$this->assertSame( WP_Http::CREATED, $second_response->get_status(), $second_message );

$first_user = get_userdata( (int) $first_data['id'] );
$second_user = get_userdata( (int) $second_data['id'] );

$this->assertNotFalse( $first_user );
$this->assertNotFalse( $second_user );
$this->assertSame( 'duplicatename', $first_user->user_login );
$this->assertMatchesRegularExpression( '/^duplicatename-[0-9]+$/', $second_user->user_login );
}

public function testGuestAuthorNonAsciiNameGetsFallbackUsername() : void {
wp_set_current_user( self::$users['editor']->ID );

$request = new WP_REST_Request( 'POST', self::$route );
$request->set_param( 'name', '李小龍' );

$response = self::rest_do_request( $request );
$data = $response->get_data();
$message = self::get_message( $response );

$this->assertSame( WP_Http::CREATED, $response->get_status(), $message );

$user = get_userdata( (int) $data['id'] );
$this->assertNotFalse( $user );
$this->assertMatchesRegularExpression( '/^guestauthor(?:-[0-9]+)?$/', $user->user_login );
}

/**
* @dataProvider dataDisallowedFields
*
Expand Down