diff --git a/app/Http/Controllers/Auth/SocialiteController.php b/app/Http/Controllers/Auth/SocialiteController.php index a6705c2a..cf446fdc 100644 --- a/app/Http/Controllers/Auth/SocialiteController.php +++ b/app/Http/Controllers/Auth/SocialiteController.php @@ -42,6 +42,10 @@ class SocialiteController extends Controller return redirect('/error?err=sso_failed'); } + $uniqueName = $socialiteUser->getId() . '@' . $driver; + $socialiteEmail = $socialiteUser->getEmail() ?? $uniqueName; + $socialiteName = ($socialiteUser->getNickname() ?? $socialiteUser->getName()) . ' (' . $uniqueName . ')'; + /** @var User|null $user */ $user = User::firstOrNew([ 'oauth_id' => $socialiteUser->getId(), @@ -49,17 +53,20 @@ class SocialiteController extends Controller ]); if (! $user->exists) { - if (User::count() === 0) { + if (User::where('email', $socialiteEmail)->exists()) { + return redirect('/error?err=sso_email_already_used'); + } + else if (User::count() === 0) { $user->is_admin = true; } else if (Settings::get('disableRegistration')) { - return redirect('/error?err=no_register'); + return redirect('/error?err=sso_no_register'); } $user->password = bcrypt(Str::random()); } - $user->email = $socialiteUser->getEmail() ?? $socialiteUser->getId() . '@' . $driver; - $user->name = $socialiteUser->getNickname() ?? $socialiteUser->getName() ?? $driver . ' #' . $socialiteUser->getId(); + $user->email = $socialiteEmail; + $user->name = $socialiteName; $user->last_seen_at = Carbon::now()->format('Y-m-d H:i:s'); $user->save(); diff --git a/resources/lang/en/errors.php b/resources/lang/en/errors.php index e0edd203..907dd4e1 100644 --- a/resources/lang/en/errors.php +++ b/resources/lang/en/errors.php @@ -62,6 +62,7 @@ return [ 'sso_disabled' => 'SSO is disabled', 'sso_bad_provider_setup' => 'This SSO provider is not fully setup in your .env file', 'sso_failed' => 'Authentication via SSO rejected', - 'no_register' => 'Registrations are disabled', + 'sso_no_register' => 'Registrations are disabled', + 'sso_email_already_used' => 'A user account with the same email address already exists but it does not match your external account ID. Do not use SSO if you are already registered on 2FAuth with this email.', 'account_managed_by_external_provider' => 'Account managed by an external provider', ]; \ No newline at end of file diff --git a/tests/Feature/Http/Auth/SocialiteControllerTest.php b/tests/Feature/Http/Auth/SocialiteControllerTest.php index 31aff07d..f21c3603 100644 --- a/tests/Feature/Http/Auth/SocialiteControllerTest.php +++ b/tests/Feature/Http/Auth/SocialiteControllerTest.php @@ -129,7 +129,6 @@ class SocialiteControllerTest extends FeatureTestCase $this->assertDatabaseHas('users', [ 'oauth_id' => self::USER_OAUTH_ID, 'oauth_provider' => self::USER_OAUTH_PROVIDER, - 'name' => 'new_nickname', 'email' => 'new_email', ]); } @@ -152,7 +151,6 @@ class SocialiteControllerTest extends FeatureTestCase $this->assertDatabaseHas('users', [ 'oauth_id' => self::USER_OAUTH_ID, 'oauth_provider' => self::USER_OAUTH_PROVIDER, - 'name' => 'new_name', 'email' => 'new_email', ]); } @@ -175,12 +173,34 @@ class SocialiteControllerTest extends FeatureTestCase $this->assertDatabaseHas('users', [ 'oauth_id' => 'new_id', 'oauth_provider' => self::USER_OAUTH_PROVIDER, - 'name' => 'jane', 'email' => 'jane@provider.com', 'is_admin' => 0, ]); } + /** + * @test + */ + public function test_callback_registers_new_user_with_existing_name() + { + $socialiteUserWithSameName = new \Laravel\Socialite\Two\User; + $socialiteUserWithSameName->id = 'socialiteUserWithSameNameId'; + $socialiteUserWithSameName->name = self::USER_NAME; + $socialiteUserWithSameName->email = 'socialiteuserwithsamename@example.com'; + $socialiteUserWithSameName->nickname = self::USER_NICKNAME; + + Socialite::shouldReceive('driver->user') + ->andReturn($socialiteUserWithSameName); + + $response = $this->get('/socialite/callback/github', ['driver' => 'github']); + + $this->assertDatabaseHas('users', [ + 'oauth_id' => 'socialiteUserWithSameNameId', + 'oauth_provider' => self::USER_OAUTH_PROVIDER, + 'email' => 'socialiteuserwithsamename@example.com', + ]); + } + /** * @test */ @@ -202,6 +222,35 @@ class SocialiteControllerTest extends FeatureTestCase ]); } + /** + * @test + */ + public function test_callback_returns_error_when_email_is_already_used() + { + $userWithSameEmail = User::factory()->create([ + 'name' => 'userWithSameEmail', + 'email' => 'other@example.com', + 'password' => 'password', + ]); + + $socialiteUserWithSameEmail = new \Laravel\Socialite\Two\User; + $socialiteUserWithSameEmail->id = '666'; + $socialiteUserWithSameEmail->name = 'socialiteUserWithSameEmail'; + $socialiteUserWithSameEmail->email = 'other@example.com'; + $socialiteUserWithSameEmail->nickname = self::USER_NICKNAME; + + Socialite::shouldReceive('driver->user') + ->andReturn($socialiteUserWithSameEmail); + + $response = $this->get('/socialite/callback/github', ['driver' => 'github']); + + $response->assertRedirect('/error?err=sso_email_already_used'); + $this->assertDatabaseMissing('users', [ + 'oauth_id' => '666', + 'oauth_provider' => self::USER_OAUTH_PROVIDER, + ]); + } + /** * @test */ @@ -219,7 +268,7 @@ class SocialiteControllerTest extends FeatureTestCase $response = $this->get('/socialite/callback/github', ['driver' => 'github']); - $response->assertRedirect('/error?err=no_register'); + $response->assertRedirect('/error?err=sso_no_register'); } /**