From 2501fa3393c2239f34cfb05a9c11b56320b83916 Mon Sep 17 00:00:00 2001 From: Brian Krane Date: Thu, 11 Jun 2026 12:21:10 -0400 Subject: [PATCH 1/3] fix: derive username collisions from an email hash instead of sequential probing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit generate_username() resolved collisions by probing info, info_1, info_2, ... with no upper bound. Each username_exists() call hydrates and caches a WP_User, so on deep shared-mailbox chains (info@ being the deepest in production) the walk exhausted the 256M heap and fataled the request before wp_insert_user() ran — WorkOS sessions existed but no WP account was ever created (CONS-513). The generator now appends a 5-hex sha256(email) suffix when the bare local part is taken, widening to 12 hex and then salted re-derivations on the astronomically rare collisions. Lookups are constant (2 on the common path) regardless of chain depth, and the same email always derives the same username, so re-provisioning is idempotent. Also switches local-part extraction from strtok() to explode(): strtok skips leading delimiters, so @example.com bypassed the workos_user fallback. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/WorkOS/Sync/UserSync.php | 39 +++- tests/wpunit/UserSyncGenerateUsernameTest.php | 196 ++++++++++++++++++ 2 files changed, 228 insertions(+), 7 deletions(-) create mode 100644 tests/wpunit/UserSyncGenerateUsernameTest.php 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/UserSyncGenerateUsernameTest.php b/tests/wpunit/UserSyncGenerateUsernameTest.php new file mode 100644 index 0000000..3c2d05d --- /dev/null +++ b/tests/wpunit/UserSyncGenerateUsernameTest.php @@ -0,0 +1,196 @@ +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', + ] + ); + } + + /** + * Test bare local part is used when free. + */ + public function test_uses_bare_local_part_when_base_is_free(): void { + $this->assertSame( 'info', $this->generate( 'info@acme-widgets.com' ) ); + } + + /** + * Test the 5-hex email hash suffix is appended when the base is taken. + */ + public function test_appends_short_email_hash_when_base_is_taken(): void { + $this->seed_login( 'info' ); + + $this->assertSame( 'info_48f25', $this->generate( 'info@acme-widgets.com' ) ); + } + + /** + * Test the suffix widens to 12 hex chars when the 5-hex name is taken. + */ + public function test_widens_suffix_when_short_hash_name_is_taken(): void { + $this->seed_login( 'info' ); + $this->seed_login( 'info_48f25' ); + + $this->assertSame( 'info_48f257791ee0', $this->generate( 'info@acme-widgets.com' ) ); + } + + /** + * Test a salted re-derivation is used when the widened name is taken. + */ + public function test_salted_retry_when_widened_name_is_taken(): void { + $this->seed_login( 'info' ); + $this->seed_login( 'info_48f25' ); + $this->seed_login( 'info_48f257791ee0' ); + + $this->assertSame( 'info_b1dc4def5ca9', $this->generate( 'info@acme-widgets.com' ) ); + } + + /** + * Test salted attempts progress deterministically when the first salt is taken. + */ + public function test_second_salted_retry_when_first_salt_is_taken(): void { + $this->seed_login( 'info' ); + $this->seed_login( 'info_48f25' ); + $this->seed_login( 'info_48f257791ee0' ); + $this->seed_login( 'info_b1dc4def5ca9' ); + + $this->assertSame( 'info_023b84c3d9b3', $this->generate( 'info@acme-widgets.com' ) ); + } + + /** + * 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 a long local part is capped so the widest suffix still fits varchar(60). + */ + public function test_long_local_part_is_capped_for_suffix_headroom(): void { + $username = $this->generate( 'international-wholesale-distribution-and-logistics-coordination@globex.test' ); + + $this->assertSame( 'international-wholesale-distribution-and-logist', $username ); + $this->assertSame( 47, strlen( $username ) ); + } + + /** + * Test the widened suffix on a capped base lands exactly on the 60-char limit. + */ + public function test_generated_username_never_exceeds_sixty_chars(): void { + $this->seed_login( 'international-wholesale-distribution-and-logist' ); + $this->seed_login( 'international-wholesale-distribution-and-logist_6a3ae' ); + + $username = $this->generate( 'international-wholesale-distribution-and-logistics-coordination@globex.test' ); + + $this->assertSame( 'international-wholesale-distribution-and-logist_6a3aeb9adeab', $username ); + $this->assertSame( 60, strlen( $username ) ); + } + + /** + * 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 ); + } + + /** + * Test the fallback base is used when the local part sanitizes to nothing. + */ + public function test_falls_back_to_workos_user_when_local_part_empty(): void { + $this->assertSame( 'workos_user', $this->generate( '@example.com' ) ); + } +} From 5e5545207712c090fc12c248dd24562661c71cdd Mon Sep 17 00:00:00 2001 From: Brian Krane Date: Thu, 11 Jun 2026 12:21:20 -0400 Subject: [PATCH 2/3] test: assert the hash-suffix username end to end through find_or_create_wp_user The generator's unit tests invoke it via reflection; this covers the public provisioning path with a taken local part, pinning the exact created login (info_48f25) rather than just a prefix. Co-Authored-By: Claude Opus 4.8 (1M context) --- tests/wpunit/UserSyncFindOrCreateTest.php | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) 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 ); + } } From a6aba558c4a897bf6ac3e069072aa63fb866bd54 Mon Sep 17 00:00:00 2001 From: Brian Krane Date: Thu, 11 Jun 2026 12:31:22 -0400 Subject: [PATCH 3/3] test: table the collision-ladder scenarios behind a data provider MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replaces the eight per-rung test methods with one provider-driven test whose named cases read top to bottom as the ladder itself (bare base → 5-hex → 12-hex → salted retries), each arranging its collisions via a seed closure. Inline FQCNs move to use imports, and every case picks up a blanket never-exceeds-60-chars assertion. Co-Authored-By: Claude Opus 4.8 (1M context) --- tests/wpunit/UserSyncGenerateUsernameTest.php | 145 +++++++++--------- 1 file changed, 76 insertions(+), 69 deletions(-) diff --git a/tests/wpunit/UserSyncGenerateUsernameTest.php b/tests/wpunit/UserSyncGenerateUsernameTest.php index 3c2d05d..ee79e46 100644 --- a/tests/wpunit/UserSyncGenerateUsernameTest.php +++ b/tests/wpunit/UserSyncGenerateUsernameTest.php @@ -7,7 +7,9 @@ namespace WorkOS\Tests\Wpunit; +use Closure; use lucatume\WPBrowser\TestCase\WPTestCase; +use ReflectionMethod; use WorkOS\Sync\UserSync; /** @@ -39,7 +41,7 @@ public function setUp(): void { * @return string Generated username. */ private function generate( string $email ): string { - $method = new \ReflectionMethod( UserSync::class, 'generate_username' ); + $method = new ReflectionMethod( UserSync::class, 'generate_username' ); $method->setAccessible( true ); return $method->invoke( null, [ 'email' => $email ] ); @@ -60,52 +62,87 @@ private function seed_login( string $login ): void { } /** - * Test bare local part is used when free. - */ - public function test_uses_bare_local_part_when_base_is_free(): void { - $this->assertSame( 'info', $this->generate( 'info@acme-widgets.com' ) ); - } - - /** - * Test the 5-hex email hash suffix is appended when the base is taken. - */ - public function test_appends_short_email_hash_when_base_is_taken(): void { - $this->seed_login( 'info' ); - - $this->assertSame( 'info_48f25', $this->generate( 'info@acme-widgets.com' ) ); - } - - /** - * Test the suffix widens to 12 hex chars when the 5-hex name is taken. + * 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 test_widens_suffix_when_short_hash_name_is_taken(): void { - $this->seed_login( 'info' ); - $this->seed_login( 'info_48f25' ); + 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 ); + } + }; + }; - $this->assertSame( 'info_48f257791ee0', $this->generate( 'info@acme-widgets.com' ) ); + 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 a salted re-derivation is used when the widened name is taken. + * 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_salted_retry_when_widened_name_is_taken(): void { - $this->seed_login( 'info' ); - $this->seed_login( 'info_48f25' ); - $this->seed_login( 'info_48f257791ee0' ); - - $this->assertSame( 'info_b1dc4def5ca9', $this->generate( 'info@acme-widgets.com' ) ); - } + public function test_derives_expected_username( Closure $arrange, string $email, string $expected ): void { + $arrange( $this ); - /** - * Test salted attempts progress deterministically when the first salt is taken. - */ - public function test_second_salted_retry_when_first_salt_is_taken(): void { - $this->seed_login( 'info' ); - $this->seed_login( 'info_48f25' ); - $this->seed_login( 'info_48f257791ee0' ); - $this->seed_login( 'info_b1dc4def5ca9' ); + $username = $this->generate( $email ); - $this->assertSame( 'info_023b84c3d9b3', $this->generate( 'info@acme-widgets.com' ) ); + $this->assertSame( $expected, $username ); + $this->assertLessThanOrEqual( 60, strlen( $username ) ); } /** @@ -148,29 +185,6 @@ public function test_lookup_count_is_constant_for_deep_username_chains(): void { $this->assertSame( 2, $lookups ); } - /** - * Test a long local part is capped so the widest suffix still fits varchar(60). - */ - public function test_long_local_part_is_capped_for_suffix_headroom(): void { - $username = $this->generate( 'international-wholesale-distribution-and-logistics-coordination@globex.test' ); - - $this->assertSame( 'international-wholesale-distribution-and-logist', $username ); - $this->assertSame( 47, strlen( $username ) ); - } - - /** - * Test the widened suffix on a capped base lands exactly on the 60-char limit. - */ - public function test_generated_username_never_exceeds_sixty_chars(): void { - $this->seed_login( 'international-wholesale-distribution-and-logist' ); - $this->seed_login( 'international-wholesale-distribution-and-logist_6a3ae' ); - - $username = $this->generate( 'international-wholesale-distribution-and-logistics-coordination@globex.test' ); - - $this->assertSame( 'international-wholesale-distribution-and-logist_6a3aeb9adeab', $username ); - $this->assertSame( 60, strlen( $username ) ); - } - /** * Test emails sharing a truncated base still derive distinct usernames. * @@ -186,11 +200,4 @@ public function test_truncated_bases_still_get_distinct_usernames(): void { $this->assertSame( 'international-wholesale-distribution-and-logist_6a3ae', $first ); $this->assertSame( 'international-wholesale-distribution-and-logist_1c20f', $second ); } - - /** - * Test the fallback base is used when the local part sanitizes to nothing. - */ - public function test_falls_back_to_workos_user_when_local_part_empty(): void { - $this->assertSame( 'workos_user', $this->generate( '@example.com' ) ); - } }