mirror of
https://github.com/coollabsio/coolify.git
synced 2026-08-24 02:24:11 -05:00
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.
This commit is contained in:
+5
-1
@@ -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();
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
<?php
|
||||
|
||||
use App\Models\InstanceSettings;
|
||||
use App\Models\OauthIdentity;
|
||||
use App\Models\OauthSetting;
|
||||
use App\Models\User;
|
||||
use Illuminate\Foundation\Testing\RefreshDatabase;
|
||||
@@ -50,6 +51,43 @@ it('logs in an existing user when the oauth provider returns a mixed-case email'
|
||||
$response->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) {
|
||||
|
||||
@@ -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']);
|
||||
|
||||
Reference in New Issue
Block a user