From b33bd073c75678b323ff85a6d5a638924a5e368a Mon Sep 17 00:00:00 2001 From: Jason Varga Date: Thu, 25 Jun 2026 22:54:27 -0400 Subject: [PATCH 1/2] avoid matching by emails when provider cannot be trusted to verify emails --- src/Http/Controllers/OAuthController.php | 3 +- src/OAuth/Provider.php | 16 +++- tests/OAuth/OAuthCallbackTest.php | 95 ++++++++++++++++++++++++ tests/OAuth/ProviderTest.php | 31 ++++++++ 4 files changed, 140 insertions(+), 5 deletions(-) create mode 100644 tests/OAuth/OAuthCallbackTest.php diff --git a/src/Http/Controllers/OAuthController.php b/src/Http/Controllers/OAuthController.php index e46d068832f..d220b7080d7 100644 --- a/src/Http/Controllers/OAuthController.php +++ b/src/Http/Controllers/OAuthController.php @@ -9,6 +9,7 @@ use Statamic\Exceptions\NotFoundHttpException; use Statamic\Facades\OAuth; use Statamic\Facades\URL; +use Statamic\Facades\User; use Statamic\Support\Arr; use Statamic\Support\Str; @@ -50,7 +51,7 @@ public function handleProviderCallback(Request $request, string $provider) if (config('statamic.oauth.merge_user_data', true)) { $user = $oauth->mergeUser($user, $providerUser); } - } elseif (config('statamic.oauth.create_user', true)) { + } elseif (config('statamic.oauth.create_user', true) && ! User::findByEmail($providerUser->getEmail())) { $user = $oauth->createUser($providerUser); } diff --git a/src/OAuth/Provider.php b/src/OAuth/Provider.php index 0aeafdf8fe9..254c62da87b 100644 --- a/src/OAuth/Provider.php +++ b/src/OAuth/Provider.php @@ -62,16 +62,24 @@ public function findOrCreateUser($socialite): StatamicUser */ public function findUser($socialite): ?StatamicUser { - if ( - ($user = User::findByOAuthId($this, $socialite->getId())) || - ($user = User::findByEmail($socialite->getEmail())) - ) { + if ($user = User::findByOAuthId($this, $socialite->getId())) { + return $user; + } + + if ($this->trustsEmails() && $user = User::findByEmail($socialite->getEmail())) { return $user; } return null; } + private function trustsEmails(): bool + { + return in_array($this->name, config('statamic.oauth.trusted_providers', [ + 'google', 'github', 'apple', 'bitbucket', 'slack', 'slack-openid', 'twitter-oauth-2', + ])); + } + /** * Create a Statamic user from a Socialite user. * diff --git a/tests/OAuth/OAuthCallbackTest.php b/tests/OAuth/OAuthCallbackTest.php new file mode 100644 index 00000000000..7b550dc65e0 --- /dev/null +++ b/tests/OAuth/OAuthCallbackTest.php @@ -0,0 +1,95 @@ +set('statamic.oauth.enabled', true); + $app['config']->set('statamic.oauth.providers', ['evil', 'google']); + } + + public function tearDown(): void + { + app('files')->deleteDirectory(storage_path('statamic/oauth')); + + parent::tearDown(); + } + + private function fakeProvider(string $name) + { + $socialiteUser = new FakeSocialiteUser(); + + $provider = Mockery::mock(Provider::class.'[getSocialiteUser]', [$name, []]); + $provider->shouldReceive('getSocialiteUser')->andReturn($socialiteUser); + + OAuth::partialMock()->shouldReceive('provider')->with($name)->andReturn($provider); + } + + #[Test] + public function an_untrusted_provider_does_not_overwrite_or_log_into_an_existing_account() + { + config(['statamic.oauth.trusted_providers' => ['google']]); // 'evil' is untrusted + config(['statamic.oauth.create_user' => true]); + + $admin = UserFacade::make()->email('admin@target.tld')->data(['name' => 'Admin'])->makeSuper()->save(); + + $this->fakeProvider('evil'); + + $response = $this->get('/oauth/evil/callback'); + + // No login happened, and they were sent to the unauthorized redirect. + $this->assertGuest(); + $response->assertRedirect(); + + // The existing super admin is untouched and no duplicate was created. + $this->assertCount(1, UserFacade::all()); + $admin = $admin->fresh(); + $this->assertTrue($admin->isSuper()); + $this->assertEquals('Admin', $admin->name()); + } + + #[Test] + public function a_trusted_provider_logs_into_the_existing_account() + { + config(['statamic.oauth.trusted_providers' => ['google']]); + + $admin = UserFacade::make()->email('admin@target.tld')->data(['name' => 'Admin'])->makeSuper()->save(); + + $this->fakeProvider('google'); + + $this->get('/oauth/google/callback'); + + $this->assertAuthenticatedAs($admin->fresh()); + $this->assertCount(1, UserFacade::all()); + } +} + +class FakeSocialiteUser +{ + public function getId() + { + return 'attacker-1'; + } + + public function getName() + { + return 'Mallory'; + } + + public function getEmail() + { + return 'admin@target.tld'; + } +} diff --git a/tests/OAuth/ProviderTest.php b/tests/OAuth/ProviderTest.php index 917aba151e6..b241e8fee69 100644 --- a/tests/OAuth/ProviderTest.php +++ b/tests/OAuth/ProviderTest.php @@ -22,6 +22,8 @@ public function setUp(): void 'driver' => 'local', 'root' => $this->tempDir = __DIR__.'/tmp', ]]); + + config(['statamic.oauth.trusted_providers' => ['test']]); } public function tearDown(): void @@ -155,6 +157,35 @@ public function it_does_not_find_or_create_a_user_via_find_user_method() $this->assertNull($user); } + #[Test] + public function it_does_not_find_an_existing_user_by_email_for_an_untrusted_provider() + { + config(['statamic.oauth.trusted_providers' => ['google']]); + + $this->user()->save(); + + $foundUser = (new Provider('test'))->findUser($this->socialite()); + + $this->assertNull($foundUser); + } + + #[Test] + public function it_still_finds_an_existing_user_by_oauth_id_for_an_untrusted_provider() + { + config(['statamic.oauth.trusted_providers' => ['google']]); + + $provider = new Provider('test'); + $savedUser = $this->user()->save(); + + // Link the account to the provider, then ensure a subsequent lookup matches + // on the stored OAuth id even though email matching is not trusted. + $provider->mergeUser($savedUser, $this->socialite()); + + $foundUser = $provider->findUser($this->socialite()); + + $this->assertEquals($savedUser, $foundUser); + } + #[Test] public function it_finds_an_existing_user_via_find_or_create_user_method() { From d29ac547b2c8ed10838d11e10f0ae6a5d9257134 Mon Sep 17 00:00:00 2001 From: Jason Varga Date: Fri, 26 Jun 2026 12:12:57 -0400 Subject: [PATCH 2/2] add happy path test --- tests/OAuth/OAuthCallbackTest.php | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/tests/OAuth/OAuthCallbackTest.php b/tests/OAuth/OAuthCallbackTest.php index 7b550dc65e0..b96ccf7f3c2 100644 --- a/tests/OAuth/OAuthCallbackTest.php +++ b/tests/OAuth/OAuthCallbackTest.php @@ -60,6 +60,26 @@ public function an_untrusted_provider_does_not_overwrite_or_log_into_an_existing $this->assertEquals('Admin', $admin->name()); } + #[Test] + public function an_untrusted_provider_creates_and_logs_in_a_new_user_when_the_email_is_free() + { + config(['statamic.oauth.trusted_providers' => ['google']]); // 'evil' is untrusted + config(['statamic.oauth.create_user' => true]); + + // No existing account owns the email, so there's nothing to overwrite — the + // untrusted provider should still onboard a brand new user. + $this->assertCount(0, UserFacade::all()); + + $this->fakeProvider('evil'); + + $this->get('/oauth/evil/callback'); + + $this->assertCount(1, UserFacade::all()); + $user = UserFacade::all()->first(); + $this->assertEquals('admin@target.tld', $user->email()); + $this->assertAuthenticatedAs($user->fresh()); + } + #[Test] public function a_trusted_provider_logs_into_the_existing_account() {