diff --git a/src/WorkOS/Sync/UserSync.php b/src/WorkOS/Sync/UserSync.php index 343d0ee..4d7b592 100644 --- a/src/WorkOS/Sync/UserSync.php +++ b/src/WorkOS/Sync/UserSync.php @@ -642,24 +642,49 @@ public static function get_wp_user_id_by_workos_id( string $workos_id ): ?int { /** * Generate a unique username from WorkOS user data. * + * Uses the email local part as the base and resolves collisions with a + * short hash of the full email instead of probing sequential suffixes — + * popular shared-mailbox local parts (info, sales, …) build chains deep + * enough that a sequential probe walks thousands of users and exhausts + * memory. Hashing keeps the lookup count constant regardless of chain + * depth, and the same email always derives the same username. + * * @param array $workos_user WorkOS user data. * * @return string */ private static function generate_username( array $workos_user ): string { - $email = $workos_user['email'] ?? ''; - $base = sanitize_user( strtok( $email, '@' ), true ); + $email = strtolower( trim( (string) ( $workos_user['email'] ?? '' ) ) ); + $base = sanitize_user( explode( '@', $email, 2 )[0], true ); if ( ! $base ) { $base = 'workos_user'; } - $username = $base; - $counter = 1; + // Cap so the widest suffix ('_' + 12 hex) fits user_login's varchar(60). + $base = substr( $base, 0, 47 ); + + if ( ! username_exists( $base ) ) { + return $base; + } + + $hash = hash( 'sha256', $email ); + $username = $base . '_' . substr( $hash, 0, 5 ); + + if ( ! username_exists( $username ) ) { + return $username; + } + + $username = $base . '_' . substr( $hash, 0, 12 ); + + // ~2^-48 territory. The generator is deterministic, so recovery must + // change the input — salt with an attempt counter, never re-roll. + for ( $attempt = 1; $attempt <= 3; $attempt++ ) { + if ( ! username_exists( $username ) ) { + return $username; + } - while ( username_exists( $username ) ) { - $username = $base . '_' . $counter; - ++$counter; + $username = $base . '_' . substr( hash( 'sha256', $email . '|' . $attempt ), 0, 12 ); } return $username; diff --git a/tests/wpunit/UserSyncFindOrCreateTest.php b/tests/wpunit/UserSyncFindOrCreateTest.php index 02015cc..047fa3c 100644 --- a/tests/wpunit/UserSyncFindOrCreateTest.php +++ b/tests/wpunit/UserSyncFindOrCreateTest.php @@ -242,4 +242,21 @@ public function test_generates_unique_username_on_conflict(): void { $this->assertNotSame( 'conflicting', $result->user_login ); $this->assertStringStartsWith( 'conflicting', $result->user_login ); } + + /** + * Test the created username carries the deterministic email-hash suffix on conflict. + */ + public function test_creates_user_with_hash_suffix_when_local_part_taken(): void { + self::factory()->user->create( [ 'user_login' => 'info', 'user_email' => 'other-info@example.com' ] ); + + $result = UserSync::find_or_create_wp_user( + [ + 'id' => 'user_info_hash_test', + 'email' => 'info@acme-widgets.com', + ] + ); + + $this->assertInstanceOf( \WP_User::class, $result ); + $this->assertSame( 'info_48f25', $result->user_login ); + } } diff --git a/tests/wpunit/UserSyncGenerateUsernameTest.php b/tests/wpunit/UserSyncGenerateUsernameTest.php new file mode 100644 index 0000000..ee79e46 --- /dev/null +++ b/tests/wpunit/UserSyncGenerateUsernameTest.php @@ -0,0 +1,203 @@ +setAccessible( true ); + + return $method->invoke( null, [ 'email' => $email ] ); + } + + /** + * Create a user occupying the given login. + * + * @param string $login Login to occupy. + */ + private function seed_login( string $login ): void { + self::factory()->user->create( + [ + 'user_login' => $login, + 'user_email' => md5( $login ) . '@seed.test', + ] + ); + } + + /** + * Collision-ladder scenarios. + * + * Each case seeds the usernames the email's derivation collides with, + * then states the expected outcome — reading top to bottom walks the + * full ladder: bare base → 5-hex suffix → 12-hex widening → salted + * retries. + * + * @return array + */ + public function username_scenario_provider(): array { + $seed = static function ( string ...$logins ): Closure { + return static function ( self $test ) use ( $logins ): void { + foreach ( $logins as $login ) { + $test->seed_login( $login ); + } + }; + }; + + return [ + 'bare local part when base is free' => [ + $seed(), + 'info@acme-widgets.com', + 'info', + ], + '5-hex email hash suffix when base is taken' => [ + $seed( 'info' ), + 'info@acme-widgets.com', + 'info_48f25', + ], + 'widened 12-hex suffix when 5-hex name taken' => [ + $seed( 'info', 'info_48f25' ), + 'info@acme-widgets.com', + 'info_48f257791ee0', + ], + 'first salted retry when widened name taken' => [ + $seed( 'info', 'info_48f25', 'info_48f257791ee0' ), + 'info@acme-widgets.com', + 'info_b1dc4def5ca9', + ], + 'second salted retry when first salt taken' => [ + $seed( 'info', 'info_48f25', 'info_48f257791ee0', 'info_b1dc4def5ca9' ), + 'info@acme-widgets.com', + 'info_023b84c3d9b3', + ], + 'long local part capped to 47 chars' => [ + $seed(), + 'international-wholesale-distribution-and-logistics-coordination@globex.test', + 'international-wholesale-distribution-and-logist', + ], + 'widened suffix on capped base lands on 60' => [ + $seed( + 'international-wholesale-distribution-and-logist', + 'international-wholesale-distribution-and-logist_6a3ae' + ), + 'international-wholesale-distribution-and-logistics-coordination@globex.test', + 'international-wholesale-distribution-and-logist_6a3aeb9adeab', + ], + 'workos_user fallback when local part empty' => [ + $seed(), + '@example.com', + 'workos_user', + ], + ]; + } + + /** + * Test the generator walks the collision ladder deterministically. + * + * @dataProvider username_scenario_provider + * + * @param Closure $arrange Seeds the usernames the scenario collides with. + * @param string $email Email to derive a username from. + * @param string $expected Expected username. + */ + public function test_derives_expected_username( Closure $arrange, string $email, string $expected ): void { + $arrange( $this ); + + $username = $this->generate( $email ); + + $this->assertSame( $expected, $username ); + $this->assertLessThanOrEqual( 60, strlen( $username ) ); + } + + /** + * Test the same email derives the identical username across runs. + */ + public function test_same_email_derives_identical_username_across_runs(): void { + $this->seed_login( 'info' ); + + $first = $this->generate( 'info@acme-widgets.com' ); + $second = $this->generate( 'info@acme-widgets.com' ); + + $this->assertSame( $first, $second ); + } + + /** + * Test lookup count stays constant no matter how deep the existing chain is. + * + * This is the regression test for the production OOM: the old sequential + * probe performed one lookup per existing info_* user. With 150 seeded + * users it would need 152 lookups; the generator must need exactly 2. + */ + public function test_lookup_count_is_constant_for_deep_username_chains(): void { + $this->seed_login( 'info' ); + for ( $i = 1; $i <= 150; $i++ ) { + $this->seed_login( 'info_' . $i ); + } + + $lookups = 0; + $counter = static function ( $user_id ) use ( &$lookups ) { + ++$lookups; + + return $user_id; + }; + + add_filter( 'username_exists', $counter ); + $username = $this->generate( 'info@acme-widgets.com' ); + remove_filter( 'username_exists', $counter ); + + $this->assertSame( 'info_48f25', $username ); + $this->assertSame( 2, $lookups ); + } + + /** + * Test emails sharing a truncated base still derive distinct usernames. + * + * The hash is computed from the full email, never the truncated base, so + * truncation must not create collisions. + */ + public function test_truncated_bases_still_get_distinct_usernames(): void { + $this->seed_login( 'international-wholesale-distribution-and-logist' ); + + $first = $this->generate( 'international-wholesale-distribution-and-logistics-coordination@globex.test' ); + $second = $this->generate( 'international-wholesale-distribution-and-logistics-department@globex.test' ); + + $this->assertSame( 'international-wholesale-distribution-and-logist_6a3ae', $first ); + $this->assertSame( 'international-wholesale-distribution-and-logist_1c20f', $second ); + } +}