mirror of
https://github.com/coollabsio/coolify.git
synced 2026-11-06 13:10:37 -04:00
fix(oauth): let pre-existing users link first identity without verification
Before OAuth identities existed, OAuth sign-in matched users by email only. Add a created_before_oauth_identities flag, set for every user at upgrade time, so their first OAuth identity links without a provider email verification claim. The flag is cleared once an identity links. Also trust Microsoft Entra ID user principal names and GitLab confirmed primary emails as verified, and pass the email to hasVerifiedEmail. Add tests using real provider payloads.
This commit is contained in:
@@ -65,9 +65,11 @@ class User extends Authenticatable implements SendsEmail
|
||||
'remember_token',
|
||||
'two_factor_recovery_codes',
|
||||
'two_factor_secret',
|
||||
'created_before_oauth_identities',
|
||||
];
|
||||
|
||||
protected $casts = [
|
||||
'created_before_oauth_identities' => 'boolean',
|
||||
'current_team_id' => 'integer',
|
||||
'email_verified_at' => 'datetime',
|
||||
'force_password_reset' => 'boolean',
|
||||
|
||||
@@ -98,15 +98,18 @@ class OauthLoginService
|
||||
return $identity->user;
|
||||
}
|
||||
|
||||
if (! $this->hasVerifiedEmail($provider, $rawClaims)) {
|
||||
throw new HttpException(403, 'OAuth provider did not verify the email address');
|
||||
}
|
||||
|
||||
$user = User::whereEmail($email)->first();
|
||||
if ($user?->oauthIdentities()->exists()) {
|
||||
throw new HttpException(403, 'OAuth identity cannot be linked to this account');
|
||||
}
|
||||
|
||||
// Before OAuth identities existed, OAuth sign-in matched users by email only.
|
||||
// Users from that time keep signing in without email delivery, so their
|
||||
// first identity links without a provider verification claim.
|
||||
if ($user?->created_before_oauth_identities !== true && ! $this->hasVerifiedEmail($provider, $rawClaims, $email)) {
|
||||
throw new HttpException(403, 'OAuth provider did not verify the email address');
|
||||
}
|
||||
|
||||
if (! $user) {
|
||||
if (! $this->canCreateUser($oauthSetting)) {
|
||||
throw new HttpException(403, 'Registration is disabled');
|
||||
@@ -125,6 +128,10 @@ class OauthLoginService
|
||||
'last_login_at' => now(),
|
||||
]);
|
||||
|
||||
if ($user->created_before_oauth_identities) {
|
||||
$user->forceFill(['created_before_oauth_identities' => false])->save();
|
||||
}
|
||||
|
||||
return $user;
|
||||
});
|
||||
} catch (UniqueConstraintViolationException $exception) {
|
||||
@@ -134,17 +141,21 @@ class OauthLoginService
|
||||
|
||||
/**
|
||||
* GitHub and Bitbucket select only verified primary email addresses in
|
||||
* their Socialite providers. Other providers must return an explicit
|
||||
* boolean verification claim in the raw provider response.
|
||||
* their Socialite providers. Microsoft Graph has no verification flag, but
|
||||
* Entra ID only issues a user principal name in a domain the tenant
|
||||
* verified, so only that name is trusted. GitLab confirms the primary
|
||||
* email of an account. Other providers must return an explicit boolean
|
||||
* verification claim in the raw provider response.
|
||||
*
|
||||
* @param array<string, mixed> $rawClaims
|
||||
*/
|
||||
private function hasVerifiedEmail(string $provider, array $rawClaims): bool
|
||||
private function hasVerifiedEmail(string $provider, array $rawClaims, string $email): bool
|
||||
{
|
||||
return match ($provider) {
|
||||
'github', 'bitbucket' => true,
|
||||
'discord' => data_get($rawClaims, 'verified') === true,
|
||||
'google' => data_get($rawClaims, 'verified_email') === true,
|
||||
'azure' => strtolower((string) data_get($rawClaims, 'userPrincipalName')) === $email,
|
||||
'gitlab' => filled(data_get($rawClaims, 'confirmed_at')),
|
||||
default => data_get($rawClaims, 'email_verified') === true,
|
||||
};
|
||||
}
|
||||
|
||||
+30
@@ -0,0 +1,30 @@
|
||||
<?php
|
||||
|
||||
use Illuminate\Database\Migrations\Migration;
|
||||
use Illuminate\Database\Schema\Blueprint;
|
||||
use Illuminate\Support\Facades\DB;
|
||||
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.
|
||||
*/
|
||||
public function up(): void
|
||||
{
|
||||
Schema::table('users', function (Blueprint $table) {
|
||||
$table->boolean('created_before_oauth_identities')->default(false);
|
||||
});
|
||||
|
||||
DB::table('users')->update(['created_before_oauth_identities' => true]);
|
||||
}
|
||||
|
||||
public function down(): void
|
||||
{
|
||||
Schema::table('users', function (Blueprint $table) {
|
||||
$table->dropColumn('created_before_oauth_identities');
|
||||
});
|
||||
}
|
||||
};
|
||||
@@ -12,7 +12,10 @@ use Illuminate\Support\Facades\Event;
|
||||
use Illuminate\Support\Once;
|
||||
use Laravel\Fortify\Features;
|
||||
use Laravel\Socialite\Facades\Socialite;
|
||||
use Laravel\Socialite\Two\GitlabProvider;
|
||||
use SocialiteProviders\Azure\Provider as AzureProvider;
|
||||
use SocialiteProviders\Discord\Provider as DiscordProvider;
|
||||
use SocialiteProviders\Google\Provider as GoogleProvider;
|
||||
use Symfony\Component\HttpKernel\Exception\HttpException;
|
||||
|
||||
uses(RefreshDatabase::class);
|
||||
@@ -49,7 +52,7 @@ it('logs in an existing user when the oauth provider returns a mixed-case email'
|
||||
'email' => 'UserName@example.edu',
|
||||
'name' => 'Example User',
|
||||
'id' => 'google-user-id',
|
||||
'user' => ['verified_email' => true, 'hd' => 'example.com'],
|
||||
'user' => ['email_verified' => true, 'hd' => 'example.com'],
|
||||
]);
|
||||
|
||||
Socialite::shouldReceive('driver')->once()->with('google')->andReturn($provider);
|
||||
@@ -86,7 +89,7 @@ it('never moves an existing oauth identity when the provider email changes', fun
|
||||
'email' => 'new@example.com',
|
||||
'name' => 'Example User',
|
||||
'id' => 'google-user-id',
|
||||
'user' => ['verified_email' => true, 'hd' => 'example.com'],
|
||||
'user' => ['email_verified' => true, 'hd' => 'example.com'],
|
||||
]);
|
||||
|
||||
Socialite::shouldReceive('driver')->once()->with('google')->andReturn($provider);
|
||||
@@ -154,7 +157,7 @@ it('continues oauth login when another request creates the identity first', func
|
||||
'email' => 'race@example.com',
|
||||
'name' => 'Race User',
|
||||
'id' => 'google-race-id',
|
||||
'user' => ['verified_email' => true, 'hd' => 'example.com'],
|
||||
'user' => ['email_verified' => true, 'hd' => 'example.com'],
|
||||
], OauthSetting::where('provider', 'google')->firstOrFail());
|
||||
} finally {
|
||||
Event::forget($eventName);
|
||||
@@ -205,6 +208,170 @@ it('keeps the Discord verified claim in the raw Socialite user payload', functio
|
||||
expect($oauthUser->user['verified'])->toBeFalse();
|
||||
});
|
||||
|
||||
function mapOauthPayloadWithVendorProvider(string $providerClass, array $payload): object
|
||||
{
|
||||
$provider = (new ReflectionClass($providerClass))->newInstanceWithoutConstructor();
|
||||
|
||||
return (new ReflectionMethod($providerClass, 'mapUserToObject'))->invoke($provider, $payload);
|
||||
}
|
||||
|
||||
it('links an existing account on the first login after upgrade from the real provider payload', function (string $provider, string $providerClass, array $payload) {
|
||||
$user = User::factory()->create(['email' => 'existing@example.com']);
|
||||
$setting = OauthSetting::updateOrCreate(['provider' => $provider], [
|
||||
'client_id' => 'client-id',
|
||||
'client_secret' => 'client-secret',
|
||||
'enabled' => true,
|
||||
]);
|
||||
|
||||
$resolvedUser = app(OauthLoginService::class)->login(
|
||||
$provider,
|
||||
mapOauthPayloadWithVendorProvider($providerClass, $payload),
|
||||
$setting,
|
||||
);
|
||||
|
||||
expect($resolvedUser->is($user))->toBeTrue()
|
||||
->and(OauthIdentity::where(['user_id' => $user->id, 'provider' => $provider])->exists())->toBeTrue();
|
||||
$this->assertAuthenticatedAs($user);
|
||||
})->with([
|
||||
'google userinfo v3' => ['google', GoogleProvider::class, [
|
||||
'sub' => '110169484474386276334',
|
||||
'name' => 'Existing User',
|
||||
'picture' => 'https://lh3.googleusercontent.com/a/photo.jpg',
|
||||
'email' => 'existing@example.com',
|
||||
'email_verified' => true,
|
||||
'hd' => 'example.com',
|
||||
]],
|
||||
'azure graph me' => ['azure', AzureProvider::class, [
|
||||
'id' => '87d349ed-44d7-43e1-9a83-5f2406dee5bd',
|
||||
'displayName' => 'Existing User',
|
||||
'userPrincipalName' => 'Existing@example.com',
|
||||
'mail' => null,
|
||||
]],
|
||||
'gitlab user' => ['gitlab', GitlabProvider::class, [
|
||||
'id' => 42,
|
||||
'username' => 'existing',
|
||||
'name' => 'Existing User',
|
||||
'email' => 'existing@example.com',
|
||||
'avatar_url' => null,
|
||||
'confirmed_at' => '2026-01-10T09:05:22Z',
|
||||
]],
|
||||
]);
|
||||
|
||||
it('marks only users that exist at upgrade time as created before OAuth identities', function () {
|
||||
$existingUser = User::factory()->create();
|
||||
$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();
|
||||
|
||||
expect($existingUser->refresh()->created_before_oauth_identities)->toBeTrue()
|
||||
->and($newUser->refresh()->created_before_oauth_identities)->toBeFalse();
|
||||
});
|
||||
|
||||
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',
|
||||
'password' => $password,
|
||||
'created_before_oauth_identities' => true,
|
||||
]);
|
||||
$setting = OauthSetting::updateOrCreate(['provider' => $provider], [
|
||||
'client_id' => 'client-id',
|
||||
'client_secret' => 'client-secret',
|
||||
'enabled' => true,
|
||||
]);
|
||||
|
||||
$resolvedUser = app(OauthLoginService::class)->login($provider, (object) [
|
||||
'email' => 'legacy@example.com',
|
||||
'name' => 'Legacy User',
|
||||
'id' => 'legacy-provider-id',
|
||||
'user' => $rawClaims,
|
||||
], $setting);
|
||||
|
||||
expect($resolvedUser->is($user))->toBeTrue()
|
||||
->and(OauthIdentity::where(['user_id' => $user->id, 'provider' => $provider])->exists())->toBeTrue()
|
||||
->and($user->refresh()->created_before_oauth_identities)->toBeFalse();
|
||||
$this->assertAuthenticatedAs($user);
|
||||
})->with([
|
||||
'discord unverified, user created by OAuth' => ['discord', ['verified' => false], null],
|
||||
'authentik default email scope, user created by OAuth' => ['authentik', ['email_verified' => false], null],
|
||||
'discord unverified, user with a password' => ['discord', ['verified' => false], 'password'],
|
||||
'authentik default email scope, user with a password' => ['authentik', ['email_verified' => false], 'password'],
|
||||
]);
|
||||
|
||||
it('does not link a second provider to a user from before the upgrade', function () {
|
||||
$user = User::factory()->create([
|
||||
'email' => 'legacy@example.com',
|
||||
'password' => null,
|
||||
'created_before_oauth_identities' => true,
|
||||
]);
|
||||
OauthIdentity::create([
|
||||
'user_id' => $user->id,
|
||||
'provider' => 'github',
|
||||
'issuer' => 'github',
|
||||
'provider_user_id' => 'github-legacy-id',
|
||||
'email' => $user->email,
|
||||
]);
|
||||
$discordSetting = OauthSetting::create([
|
||||
'provider' => 'discord',
|
||||
'client_id' => 'discord-client-id',
|
||||
'client_secret' => 'discord-client-secret',
|
||||
'enabled' => true,
|
||||
]);
|
||||
|
||||
expect(fn () => app(OauthLoginService::class)->login('discord', (object) [
|
||||
'email' => 'legacy@example.com',
|
||||
'name' => 'Attacker',
|
||||
'id' => 'discord-attacker-id',
|
||||
'user' => ['verified' => false],
|
||||
], $discordSetting))->toThrow(HttpException::class, 'OAuth identity cannot be linked to this account');
|
||||
|
||||
$this->assertGuest();
|
||||
expect(OauthIdentity::count())->toBe(1);
|
||||
});
|
||||
|
||||
it('does not link an existing account when the provider payload does not verify the email', function (string $provider, string $providerClass, array $payload, ?string $unverifiedEmail = null) {
|
||||
User::factory()->create(['email' => 'existing@example.com']);
|
||||
$setting = OauthSetting::updateOrCreate(['provider' => $provider], [
|
||||
'client_id' => 'client-id',
|
||||
'client_secret' => 'client-secret',
|
||||
'enabled' => true,
|
||||
]);
|
||||
$oauthUser = mapOauthPayloadWithVendorProvider($providerClass, $payload);
|
||||
if ($unverifiedEmail !== null) {
|
||||
$oauthUser->email = $unverifiedEmail;
|
||||
}
|
||||
|
||||
expect(fn () => app(OauthLoginService::class)->login($provider, $oauthUser, $setting))
|
||||
->toThrow(HttpException::class, 'OAuth provider did not verify the email address');
|
||||
|
||||
$this->assertGuest();
|
||||
expect(OauthIdentity::count())->toBe(0);
|
||||
})->with([
|
||||
'google unverified' => ['google', GoogleProvider::class, [
|
||||
'sub' => '110169484474386276334',
|
||||
'name' => 'Existing User',
|
||||
'picture' => null,
|
||||
'email' => 'existing@example.com',
|
||||
'email_verified' => false,
|
||||
'hd' => 'example.com',
|
||||
]],
|
||||
'azure mail attribute instead of the user principal name' => ['azure', AzureProvider::class, [
|
||||
'id' => '87d349ed-44d7-43e1-9a83-5f2406dee5bd',
|
||||
'displayName' => 'Existing User',
|
||||
'userPrincipalName' => 'someone@other-tenant.onmicrosoft.com',
|
||||
'mail' => 'existing@example.com',
|
||||
], 'existing@example.com'],
|
||||
'gitlab unconfirmed' => ['gitlab', GitlabProvider::class, [
|
||||
'id' => 42,
|
||||
'username' => 'existing',
|
||||
'name' => 'Existing User',
|
||||
'email' => 'existing@example.com',
|
||||
'avatar_url' => null,
|
||||
'confirmed_at' => null,
|
||||
]],
|
||||
]);
|
||||
|
||||
it('registers a new user from a verified provider identity', function () {
|
||||
InstanceSettings::findOrFail(0)->update(['is_registration_enabled' => true]);
|
||||
|
||||
@@ -212,7 +379,7 @@ it('registers a new user from a verified provider identity', function () {
|
||||
'email' => 'verified@example.com',
|
||||
'name' => 'Verified User',
|
||||
'id' => 'verified-google-id',
|
||||
'user' => ['verified_email' => true, 'hd' => 'example.com'],
|
||||
'user' => ['email_verified' => true, 'hd' => 'example.com'],
|
||||
], OauthSetting::where('provider', 'google')->firstOrFail());
|
||||
|
||||
expect($user->email)->toBe('verified@example.com');
|
||||
@@ -246,7 +413,7 @@ it('does not register a new user through a non-OIDC provider when registration i
|
||||
->and(OauthIdentity::count())->toBe(0);
|
||||
})->with([
|
||||
'github' => ['github', []],
|
||||
'google' => ['google', ['verified_email' => true, 'hd' => 'example.com']],
|
||||
'google' => ['google', ['email_verified' => true, 'hd' => 'example.com']],
|
||||
]);
|
||||
|
||||
it('registers a new user through a non-OIDC provider when registration is enabled', function () {
|
||||
@@ -284,7 +451,7 @@ it('does not link another provider identity to an account by shared email', func
|
||||
'email' => 'shared@example.com',
|
||||
'name' => 'Other Provider User',
|
||||
'id' => 'google-user-id',
|
||||
'user' => ['verified_email' => true, 'hd' => 'example.com'],
|
||||
'user' => ['email_verified' => true, 'hd' => 'example.com'],
|
||||
], OauthSetting::where('provider', 'google')->firstOrFail()))->toThrow(HttpException::class);
|
||||
|
||||
$this->assertGuest();
|
||||
@@ -313,7 +480,7 @@ it('sends an OAuth user with confirmed two factor authentication to the Fortify
|
||||
'email' => $user->email,
|
||||
'name' => $user->name,
|
||||
'id' => 'two-factor-google-id',
|
||||
'user' => ['verified_email' => true, 'hd' => 'example.com'],
|
||||
'user' => ['email_verified' => true, 'hd' => 'example.com'],
|
||||
]);
|
||||
Socialite::shouldReceive('driver')->once()->with('google')->andReturn($provider);
|
||||
|
||||
@@ -341,7 +508,7 @@ it('completes OAuth login without a challenge when two factor authentication is
|
||||
'email' => $user->email,
|
||||
'name' => $user->name,
|
||||
'id' => 'plain-google-id',
|
||||
'user' => ['verified_email' => true, 'hd' => 'example.com'],
|
||||
'user' => ['email_verified' => true, 'hd' => 'example.com'],
|
||||
]);
|
||||
Socialite::shouldReceive('driver')->once()->with('google')->andReturn($provider);
|
||||
|
||||
@@ -436,7 +603,7 @@ it('rejects a Google account outside the configured Workspace even when its emai
|
||||
'email' => $user->email,
|
||||
'name' => 'Outside User',
|
||||
'id' => 'google-outside-id',
|
||||
'user' => ['verified_email' => true, 'hd' => 'outside.example'],
|
||||
'user' => ['email_verified' => true, 'hd' => 'outside.example'],
|
||||
], $setting))->toThrow(HttpException::class);
|
||||
|
||||
expect(OauthIdentity::count())->toBe(0);
|
||||
|
||||
Reference in New Issue
Block a user