diff --git a/app/Services/Auth/OauthLoginService.php b/app/Services/Auth/OauthLoginService.php index 6b7acbc5b..c093e290f 100644 --- a/app/Services/Auth/OauthLoginService.php +++ b/app/Services/Auth/OauthLoginService.php @@ -36,7 +36,11 @@ public function login(string $provider, object $oauthUser, OauthSetting $oauthSe private function resolveOauthUser(object $oauthUser, OauthSetting $oauthSetting, string $email): User { $provider = $oauthSetting->provider; - $providerUserId = (string) $oauthUser->id; + $providerUserId = $oauthUser->id ?? null; + if (! is_scalar($providerUserId) || trim((string) $providerUserId) === '') { + throw new HttpException(403, 'OAuth provider did not return a valid user ID'); + } + $providerUserId = (string) $providerUserId; $rawClaims = is_array($oauthUser->user ?? null) ? $oauthUser->user : []; return DB::transaction(function () use ($oauthUser, $oauthSetting, $email, $provider, $providerUserId, $rawClaims) { diff --git a/tests/Feature/OauthControllerTest.php b/tests/Feature/OauthControllerTest.php index 62940b0ef..f0225e335 100644 --- a/tests/Feature/OauthControllerTest.php +++ b/tests/Feature/OauthControllerTest.php @@ -4,9 +4,11 @@ use App\Models\OauthIdentity; use App\Models\OauthSetting; use App\Models\User; +use App\Services\Auth\OauthLoginService; use Illuminate\Foundation\Testing\RefreshDatabase; use Illuminate\Support\Once; use Laravel\Socialite\Facades\Socialite; +use Symfony\Component\HttpKernel\Exception\HttpException; uses(RefreshDatabase::class); @@ -121,3 +123,31 @@ 'malformed email' => ['not-an-email'], 'missing domain' => ['user@'], ]); + +it('rejects oauth logins when the provider does not return a valid user id', function (mixed $invalidId) { + $oauthUser = (object) [ + 'email' => 'user@example.edu', + 'name' => 'Example User', + ]; + + if ($invalidId !== 'missing') { + $oauthUser->id = $invalidId; + } + + try { + app(OauthLoginService::class)->login('google', $oauthUser, OauthSetting::where('provider', 'google')->firstOrFail()); + } catch (HttpException $exception) { + expect($exception->getStatusCode())->toBe(403) + ->and(OauthIdentity::count())->toBe(0) + ->and(User::count())->toBe(0); + + return; + } + + $this->fail('Expected an invalid OAuth provider user ID to be rejected.'); +})->with([ + 'null id' => [null], + 'missing id' => ['missing'], + 'blank id' => [' '], + 'non-scalar id' => [[]], +]);