From f9f26c547c8d8eef015a32f30ef904e334d303df Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Tue, 18 Aug 2026 15:27:27 +0200 Subject: [PATCH] fix(auth): preserve OAuth identity across email changes Link OAuth logins by provider user ID before matching email, refresh identity claims on login, and skip password confirmation for SSO-linked users. --- app/Models/User.php | 6 ++- app/Services/Auth/OauthLoginService.php | 50 +++++++++++++++---- bootstrap/helpers/shared.php | 9 ++-- tests/Feature/OauthControllerTest.php | 38 ++++++++++++++ tests/v4/Feature/DangerDeleteResourceTest.php | 18 ++++++- 5 files changed, 106 insertions(+), 15 deletions(-) diff --git a/app/Models/User.php b/app/Models/User.php index e8f59835be..10303422bd 100644 --- a/app/Models/User.php +++ b/app/Models/User.php @@ -520,10 +520,14 @@ class User extends Authenticatable implements SendsEmail /** * Check if the user has a password set. - * OAuth users are created without passwords. */ public function hasPassword(): bool { return ! empty($this->password); } + + public function requiresPasswordConfirmation(): bool + { + return $this->hasPassword() && ! $this->hasSsoIdentity(); + } } diff --git a/app/Services/Auth/OauthLoginService.php b/app/Services/Auth/OauthLoginService.php index f656fad08f..6b7acbc5b4 100644 --- a/app/Services/Auth/OauthLoginService.php +++ b/app/Services/Auth/OauthLoginService.php @@ -35,16 +35,48 @@ class OauthLoginService private function resolveOauthUser(object $oauthUser, OauthSetting $oauthSetting, string $email): User { - $user = User::whereEmail($email)->first(); - if ($user) { + $provider = $oauthSetting->provider; + $providerUserId = (string) $oauthUser->id; + $rawClaims = is_array($oauthUser->user ?? null) ? $oauthUser->user : []; + + return DB::transaction(function () use ($oauthUser, $oauthSetting, $email, $provider, $providerUserId, $rawClaims) { + $identity = OauthIdentity::where([ + 'provider' => $provider, + 'issuer' => $provider, + 'provider_user_id' => $providerUserId, + ])->first(); + + if ($identity) { + $identity->update([ + 'email' => $email, + 'raw_claims' => $rawClaims, + 'last_login_at' => now(), + ]); + + return $identity->user; + } + + $user = User::whereEmail($email)->first(); + if (! $user) { + if (! $this->canCreateUser($oauthSetting)) { + throw new HttpException(403, 'Registration is disabled'); + } + + $user = $this->createUser($oauthUser->name ?: $email, $email, $oauthSetting); + } + + OauthIdentity::create([ + 'user_id' => $user->id, + 'provider' => $provider, + 'issuer' => $provider, + 'provider_user_id' => $providerUserId, + 'email' => $email, + 'raw_claims' => $rawClaims, + 'last_login_at' => now(), + ]); + return $user; - } - - if (! $this->canCreateUser($oauthSetting)) { - throw new HttpException(403, 'Registration is disabled'); - } - - return $this->createUser($oauthUser->name ?: $email, $email, $oauthSetting); + }); } private function resolveOidcUser(object $oauthUser, OauthSetting $oauthSetting, string $email): User diff --git a/bootstrap/helpers/shared.php b/bootstrap/helpers/shared.php index cf63c4776a..d3cc6d826b 100644 --- a/bootstrap/helpers/shared.php +++ b/bootstrap/helpers/shared.php @@ -4544,7 +4544,7 @@ function formatContainerStatus(string $status): string * Check if password confirmation should be skipped. * Returns true if: * - Two-step confirmation is globally disabled - * - User has no password (OAuth users) + * - User has no usable local password confirmation (including SSO users) * * Used by modal-confirmation.blade.php to determine if password step should be shown. * @@ -4557,8 +4557,9 @@ function shouldSkipPasswordConfirmation(): bool return true; } - // Skip if user has no password (OAuth users) - if (! Auth::user()?->hasPassword()) { + // OAuth users may have an unusable generated password, so the linked + // identity is the source of truth for whether confirmation is possible. + if (! Auth::user()?->requiresPasswordConfirmation()) { return true; } @@ -4569,7 +4570,7 @@ function shouldSkipPasswordConfirmation(): bool * Verify password for two-step confirmation. * Skips verification if: * - Two-step confirmation is globally disabled - * - User has no password (OAuth users) + * - User has no usable local password confirmation (including SSO users) * * @param mixed $password The password to verify (may be array if skipped by frontend) * @param Component|null $component Optional Livewire component to add errors to diff --git a/tests/Feature/OauthControllerTest.php b/tests/Feature/OauthControllerTest.php index 27660a6bb3..62940b0ef7 100644 --- a/tests/Feature/OauthControllerTest.php +++ b/tests/Feature/OauthControllerTest.php @@ -1,6 +1,7 @@ assertRedirect('/'); $this->assertAuthenticatedAs($user); expect(User::count())->toBe(1); + expect(OauthIdentity::where([ + 'user_id' => $user->id, + 'provider' => 'google', + 'provider_user_id' => 'google-user-id', + ])->exists())->toBeTrue(); +}); + +it('never moves an existing oauth identity when the provider email changes', function () { + config()->set('app.maintenance.driver', 'file'); + + $identityOwner = User::factory()->create(['email' => 'old@example.com']); + $otherUser = User::factory()->create(['email' => 'new@example.com']); + $identity = OauthIdentity::create([ + 'user_id' => $identityOwner->id, + 'provider' => 'google', + 'issuer' => 'google', + 'provider_user_id' => 'google-user-id', + 'email' => 'old@example.com', + ]); + + $provider = Mockery::mock(); + $provider->shouldReceive('setConfig')->once()->andReturnSelf(); + $provider->shouldReceive('with')->once()->with(['hd' => 'example.com'])->andReturnSelf(); + $provider->shouldReceive('user')->once()->andReturn((object) [ + 'email' => 'new@example.com', + 'name' => 'Example User', + 'id' => 'google-user-id', + ]); + + Socialite::shouldReceive('driver')->once()->with('google')->andReturn($provider); + + $this->get(route('auth.callback', 'google'))->assertRedirect('/'); + + $this->assertAuthenticatedAs($identityOwner); + expect($identity->refresh()->user_id)->toBe($identityOwner->id) + ->and($identity->email)->toBe('new@example.com') + ->and($identity->user_id)->not->toBe($otherUser->id); }); it('rejects oauth logins when the provider does not return an email address', function (?string $providerEmail) { diff --git a/tests/v4/Feature/DangerDeleteResourceTest.php b/tests/v4/Feature/DangerDeleteResourceTest.php index 7a73f59795..4a275ad484 100644 --- a/tests/v4/Feature/DangerDeleteResourceTest.php +++ b/tests/v4/Feature/DangerDeleteResourceTest.php @@ -4,6 +4,7 @@ use App\Livewire\Project\Shared\Danger; use App\Models\Application; use App\Models\Environment; use App\Models\InstanceSettings; +use App\Models\OauthIdentity; use App\Models\Project; use App\Models\Server; use App\Models\StandaloneDocker; @@ -18,7 +19,7 @@ use Livewire\Livewire; uses(RefreshDatabase::class); beforeEach(function () { - InstanceSettings::create(['id' => 0]); + InstanceSettings::forceCreate(['id' => 0]); Queue::fake(); $this->user = User::factory()->create([ @@ -70,6 +71,21 @@ test('delete succeeds with correct password and redirects', function () { expect(Application::find($this->application->id))->toBeNull(); }); +test('delete succeeds without password for an oauth user', function () { + OauthIdentity::create([ + 'user_id' => $this->user->id, + 'provider' => 'oidc', + 'issuer' => 'https://idp.example.com', + 'provider_user_id' => 'oauth-user-id', + ]); + + Livewire::test(Danger::class, ['resource' => $this->application]) + ->call('delete', '') + ->assertHasNoErrors(); + + expect(Application::find($this->application->id))->toBeNull(); +}); + test('delete applies selectedActions from checkbox state', function () { $component = Livewire::test(Danger::class, ['resource' => $this->application]) ->call('delete', 'test-password', ['delete_configurations', 'docker_cleanup']);