diff --git a/bootstrap/helpers/socialite.php b/bootstrap/helpers/socialite.php index 5f7e8d868a..f9bbecbd2e 100644 --- a/bootstrap/helpers/socialite.php +++ b/bootstrap/helpers/socialite.php @@ -9,12 +9,20 @@ use Laravel\Socialite\Two\GithubProvider; use SocialiteProviders\Discord\Provider; use SocialiteProviders\Manager\Config; +/** + * Callback URL built from the instance URL, never from the request Host header. + */ +function oauth_default_redirect_uri(string $provider): string +{ + return rtrim(base_url(), '/').route('auth.callback', $provider, false); +} + function get_socialite_provider(string $provider) { $oauth_setting = OauthSetting::firstWhere('provider', $provider); if (! filled($oauth_setting->redirect_uri)) { - $oauth_setting->update(['redirect_uri' => route('auth.callback', $provider)]); + $oauth_setting->redirect_uri = oauth_default_redirect_uri($provider); } if ($provider === 'azure') { diff --git a/database/migrations/2026_09_29_200325_add_created_before_oauth_identities_to_users_table.php b/database/migrations/2026_09_29_200325_add_created_before_oauth_identities_to_users_table.php index 9828ff8e89..8d50838ce4 100644 --- a/database/migrations/2026_09_29_200325_add_created_before_oauth_identities_to_users_table.php +++ b/database/migrations/2026_09_29_200325_add_created_before_oauth_identities_to_users_table.php @@ -8,9 +8,12 @@ use Illuminate\Support\Facades\Schema; return new class extends Migration { /** - * Before OAuth identities existed, OAuth sign-in matched users by email only. - * Mark every user that exists at upgrade time so their first OAuth identity - * links without a provider email verification claim. + * Before OAuth identities existed, OAuth sign-in matched users by email only + * and created users without a password. Mark those password-less users so + * their first OAuth identity links without a provider email verification + * claim. Users with a password (including root) never get this exemption: + * otherwise anyone with an unverified provider account with their email + * could sign in as them. */ public function up(): void { @@ -18,7 +21,9 @@ return new class extends Migration $table->boolean('created_before_oauth_identities')->default(false); }); - DB::table('users')->update(['created_before_oauth_identities' => true]); + DB::table('users') + ->whereNull('password') + ->update(['created_before_oauth_identities' => true]); } public function down(): void diff --git a/resources/views/livewire/settings-oauth.blade.php b/resources/views/livewire/settings-oauth.blade.php index 822c035b31..a24224dbcb 100644 --- a/resources/views/livewire/settings-oauth.blade.php +++ b/resources/views/livewire/settings-oauth.blade.php @@ -58,7 +58,7 @@ @if ($provider === 'oidc') + placeholder="{{ oauth_default_redirect_uri($provider) }}" label="Redirect URI" /> @@ -81,7 +81,7 @@ @else + placeholder="{{ oauth_default_redirect_uri($provider) }}" label="Redirect URI" /> create(); +function rerunCreatedBeforeOauthIdentitiesMigration(): void +{ $migration = require database_path('migrations/2026_09_29_200325_add_created_before_oauth_identities_to_users_table.php'); $migration->down(); $migration->up(); +} - $newUser = User::factory()->create(); +/** + * Brings the users into the state after an upgrade from v4.3.23. + */ +function upgradeUsersToOauthIdentities(): void +{ + rerunCreatedBeforeOauthIdentitiesMigration(); +} - expect($existingUser->refresh()->created_before_oauth_identities)->toBeTrue() +it('marks only password-less users that exist at upgrade time as created before OAuth identities', function () { + $rootUser = User::factory()->create(['id' => 0]); + $passwordUser = User::factory()->create(); + $oauthCreatedUser = User::factory()->create(['password' => null]); + + rerunCreatedBeforeOauthIdentitiesMigration(); + + $newUser = User::factory()->create(['password' => null]); + + expect($oauthCreatedUser->refresh()->created_before_oauth_identities)->toBeTrue() + ->and($rootUser->refresh()->created_before_oauth_identities)->toBeFalse() + ->and($passwordUser->refresh()->created_before_oauth_identities)->toBeFalse() ->and($newUser->refresh()->created_before_oauth_identities)->toBeFalse(); }); +it('does not link an unverified provider email to a password user after the upgrade', function (string $provider, array $rawClaims) { + $user = User::factory()->create(['id' => 0, 'email' => 'root@example.com']); + $setting = OauthSetting::updateOrCreate(['provider' => $provider], [ + 'client_id' => 'client-id', + 'client_secret' => 'client-secret', + 'base_url' => 'https://auth.example.com', + 'enabled' => true, + ]); + upgradeUsersToOauthIdentities(); + + expect(fn () => app(OauthLoginService::class)->login($provider, (object) [ + 'email' => 'root@example.com', + 'name' => 'Attacker', + 'id' => 'attacker-provider-id', + 'user' => $rawClaims, + ], $setting))->toThrow(HttpException::class, 'OAuth provider did not verify the email address'); + + $this->assertGuest(); + expect(OauthIdentity::count())->toBe(0) + ->and($user->refresh()->created_before_oauth_identities)->toBeFalse(); +})->with([ + 'discord unverified' => ['discord', ['verified' => false]], + 'authentik default email scope' => ['authentik', ['email_verified' => false]], +]); + +it('links a verified provider email to a password user after the upgrade', function (string $provider, array $rawClaims) { + $user = User::factory()->create(['email' => 'member@example.com']); + $setting = OauthSetting::updateOrCreate(['provider' => $provider], [ + 'client_id' => 'client-id', + 'client_secret' => 'client-secret', + 'enabled' => true, + ]); + upgradeUsersToOauthIdentities(); + + $resolvedUser = app(OauthLoginService::class)->login($provider, (object) [ + 'email' => 'member@example.com', + 'name' => 'Member', + 'id' => 'member-provider-id', + 'user' => $rawClaims, + ], $setting); + + expect($resolvedUser->is($user))->toBeTrue() + ->and(OauthIdentity::where(['user_id' => $user->id, 'provider' => $provider])->exists())->toBeTrue(); + $this->assertAuthenticatedAs($user); +})->with([ + 'github' => ['github', []], + 'google verified' => ['google', ['email_verified' => true, 'hd' => 'example.com']], + 'discord verified' => ['discord', ['verified' => true]], +]); + +it('links a password-less user from before the upgrade without an email verification claim', function () { + $user = User::factory()->create(['email' => 'legacy@example.com', 'password' => null]); + $setting = OauthSetting::create([ + 'provider' => 'discord', + 'client_id' => 'discord-client-id', + 'client_secret' => 'discord-client-secret', + 'enabled' => true, + ]); + upgradeUsersToOauthIdentities(); + + $resolvedUser = app(OauthLoginService::class)->login('discord', (object) [ + 'email' => 'legacy@example.com', + 'name' => 'Legacy User', + 'id' => 'legacy-discord-id', + 'user' => ['verified' => false], + ], $setting); + + expect($resolvedUser->is($user))->toBeTrue() + ->and($user->refresh()->created_before_oauth_identities)->toBeFalse(); + $this->assertAuthenticatedAs($user); +}); + it('links a user from before the upgrade without an email verification claim', function (string $provider, array $rawClaims, ?string $password) { $user = User::factory()->create([ 'email' => 'legacy@example.com', @@ -610,3 +700,21 @@ it('rejects a Google account outside the configured Workspace even when its emai expect(OauthIdentity::count())->toBe(0); $this->assertGuest(); }); + +it('sends the instance callback url without saving it when a forged host starts an oauth login', function () { + InstanceSettings::query()->whereKey(0)->update(['fqdn' => 'https://coolify.example.com']); + Once::flush(); + OauthSetting::create([ + 'provider' => 'github', + 'client_id' => 'github-client-id', + 'client_secret' => 'github-client-secret', + 'enabled' => true, + ]); + + $response = $this->get('http://attacker.example/auth/github/redirect'); + + $response->assertRedirect(); + parse_str((string) parse_url($response->headers->get('Location'), PHP_URL_QUERY), $query); + expect($query['redirect_uri'])->toBe('https://coolify.example.com/auth/github/callback') + ->and(OauthSetting::where('provider', 'github')->value('redirect_uri'))->toBeNull(); +});