[5.x] Improve authentication & account registering logic (#318)

* Improve alreadyRegistered error message * Improve registration logic * Split feature tests into different files * Add failure tests for createAccountOnFirstLogin feature * rename files --------- Co-authored-by: miguilim <[email protected]> Co-authored-by: Joel Butcher <[email protected]>
joelbutcher · Dec 1, 2023 · ccfec50 · ccfec50
1 parent cb2c2e0
commit ccfec50
Show file tree

Hide file tree

Showing 5 changed files with 282 additions and 139 deletions.
diff --git a/src/Actions/AuthenticateOAuthCallback.php b/src/Actions/AuthenticateOAuthCallback.php
@@ -52,12 +52,10 @@ public function authenticate(string $provider, ProviderUser $providerAccount): R
         $previousUrl = session()->get('socialstream.previous_url');
 
         if (
-            class_exists(FortifyFeatures::class) &&
-            FortifyFeatures::enabled(FortifyFeatures::registration()) && ! $account &&
-            (
-                $previousUrl === route('register') ||
-                (Features::hasCreateAccountOnFirstLoginFeatures() && $previousUrl === route('login'))
-            )
+            class_exists(FortifyFeatures::class) 
+            && FortifyFeatures::enabled(FortifyFeatures::registration()) 
+            && ! $account 
+            && ($previousUrl === route('register') || Features::hasCreateAccountOnFirstLoginFeatures())
         ) {
             $user = Socialstream::newUserModel()->where('email', $providerAccount->getEmail())->first();
 
@@ -74,18 +72,6 @@ class_exists(FortifyFeatures::class) &&
             );
         }
 
-        if (Features::hasCreateAccountOnFirstLoginFeatures() && ! $account) {
-            if (Socialstream::newUserModel()->where('email', $providerAccount->getEmail())->exists()) {
-                return $this->redirectAuthFailed(
-                    error: __('An account with that email address already exists. Please login to connect your :Provider account.', ['provider' => Providers::name($provider)])
-                );
-            }
-
-            $user = $this->createsUser->create($provider, $providerAccount);
-
-            return $this->login($user);
-        }
-
         $user = $account->user;
 
         $this->updatesConnectedAccounts->update($user, $account, $provider, $providerAccount);
@@ -148,7 +134,7 @@ protected function alreadyRegistered(Authenticatable $user, ?ConnectedAccount $a
         }
 
         return $this->redirectAuthFailed(
-            __('An account with that :Provider sign in already exists, please login.', ['provider' => Providers::buttonLabel($provider)])
+            __('An account with that email address already exists. Please login to connect your :Provider account.', ['provider' => Providers::buttonLabel($provider)])
         );
     }
 

diff --git a/tests/Feature/CreateAccountOnFirstLoginTest.php b/tests/Feature/CreateAccountOnFirstLoginTest.php
@@ -0,0 +1,159 @@
+<?php
+
+namespace JoelButcher\Socialstream\Tests\Feature;
+
+use App\Providers\RouteServiceProvider;
+use Illuminate\Foundation\Testing\RefreshDatabase;
+use Illuminate\Support\Facades\Config;
+use JoelButcher\Socialstream\Features;
+use Laravel\Socialite\Facades\Socialite;
+use Laravel\Socialite\Two\GithubProvider;
+use Laravel\Socialite\Two\User as SocialiteUser;
+use Mockery;
+
+use function Pest\Laravel\get;
+
+uses(RefreshDatabase::class);
+
+test('new users can register from login page', function (): void {
+    Config::set('socialstream.features', [
+        Features::createAccountOnFirstLogin(),
+    ]);
+
+    $this->assertDatabaseEmpty('users');
+    $this->assertDatabaseEmpty('connected_accounts');
+
+    $user = (new SocialiteUser())
+        ->map([
+            'id' => $githubId = fake()->numerify('########'),
+            'nickname' => 'joel',
+            'name' => 'Joel',
+            'email' => '[email protected]',
+            'avatar' => null,
+            'avatar_original' => null,
+        ])
+        ->setToken('user-token')
+        ->setRefreshToken('refresh-token')
+        ->setExpiresIn(3600);
+
+    $provider = Mockery::mock(GithubProvider::class);
+    $provider->shouldReceive('user')->once()->andReturn($user);
+
+    session()->put('socialstream.previous_url', route('login'));
+
+    Socialite::shouldReceive('driver')->once()->with('github')->andReturn($provider);
+
+    get('http://localhost/oauth/github/callback')
+        ->assertRedirect(RouteServiceProvider::HOME);
+
+    $this->assertAuthenticated();
+    $this->assertDatabaseHas('users', ['email' => '[email protected]']);
+    $this->assertDatabaseHas('connected_accounts', [
+        'provider' => 'github',
+        'provider_id' => $githubId,
+        'email' => '[email protected]',
+    ]);
+});
+
+
+test('new users can register from random page', function (): void {
+    Config::set('socialstream.features', [
+        Features::createAccountOnFirstLogin(),
+    ]);
+
+    $this->assertDatabaseEmpty('users');
+    $this->assertDatabaseEmpty('connected_accounts');
+
+    $user = (new SocialiteUser())
+        ->map([
+            'id' => $githubId = fake()->numerify('########'),
+            'nickname' => 'joel',
+            'name' => 'Joel',
+            'email' => '[email protected]',
+            'avatar' => null,
+            'avatar_original' => null,
+        ])
+        ->setToken('user-token')
+        ->setRefreshToken('refresh-token')
+        ->setExpiresIn(3600);
+
+    $provider = Mockery::mock(GithubProvider::class);
+    $provider->shouldReceive('user')->once()->andReturn($user);
+
+    session()->put('socialstream.previous_url', '/random');
+
+    Socialite::shouldReceive('driver')->once()->with('github')->andReturn($provider);
+
+    get('http://localhost/oauth/github/callback')
+        ->assertRedirect(RouteServiceProvider::HOME);
+
+    $this->assertAuthenticated();
+    $this->assertDatabaseHas('users', ['email' => '[email protected]']);
+    $this->assertDatabaseHas('connected_accounts', [
+        'provider' => 'github',
+        'provider_id' => $githubId,
+        'email' => '[email protected]',
+    ]);
+});
+
+test('new users cannot register from login page without feature enabled', function (): void {
+    $this->assertDatabaseEmpty('users');
+    $this->assertDatabaseEmpty('connected_accounts');
+
+    $user = (new SocialiteUser())
+        ->map([
+            'id' => $githubId = fake()->numerify('########'),
+            'nickname' => 'joel',
+            'name' => 'Joel',
+            'email' => '[email protected]',
+            'avatar' => null,
+            'avatar_original' => null,
+        ])
+        ->setToken('user-token')
+        ->setRefreshToken('refresh-token')
+        ->setExpiresIn(3600);
+
+    $provider = Mockery::mock(GithubProvider::class);
+    $provider->shouldReceive('user')->once()->andReturn($user);
+
+    session()->put('socialstream.previous_url', route('login'));
+
+    Socialite::shouldReceive('driver')->once()->with('github')->andReturn($provider);
+
+    $response = get('http://localhost/oauth/github/callback');
+
+    $this->assertGuest();
+    $response->assertRedirect(route('login'));
+    $response->assertSessionHasErrors();
+});
+
+test('new users cannot register from random page without feature enabled', function (): void {
+    $this->assertDatabaseEmpty('users');
+    $this->assertDatabaseEmpty('connected_accounts');
+
+    $user = (new SocialiteUser())
+        ->map([
+            'id' => $githubId = fake()->numerify('########'),
+            'nickname' => 'joel',
+            'name' => 'Joel',
+            'email' => '[email protected]',
+            'avatar' => null,
+            'avatar_original' => null,
+        ])
+        ->setToken('user-token')
+        ->setRefreshToken('refresh-token')
+        ->setExpiresIn(3600);
+
+    $provider = Mockery::mock(GithubProvider::class);
+    $provider->shouldReceive('user')->once()->andReturn($user);
+
+    session()->put('socialstream.previous_url', '/random');
+
+    Socialite::shouldReceive('driver')->once()->with('github')->andReturn($provider);
+
+    $response = get('http://localhost/oauth/github/callback');
+
+    $this->assertGuest();
+    $response->assertRedirect(route('login'));
+    $response->assertSessionHasErrors();
+});
diff --git a/tests/Feature/GenerateMissingEmailsTest.php b/tests/Feature/GenerateMissingEmailsTest.php
@@ -0,0 +1,55 @@
+<?php
+
+namespace JoelButcher\Socialstream\Tests\Feature;
+
+use App\Models\User;
+use App\Providers\RouteServiceProvider;
+use Illuminate\Foundation\Testing\RefreshDatabase;
+use Illuminate\Support\Facades\Config;
+use JoelButcher\Socialstream\Features;
+use Laravel\Socialite\Facades\Socialite;
+use Laravel\Socialite\Two\GithubProvider;
+use Laravel\Socialite\Two\User as SocialiteUser;
+use Mockery;
+
+use function Pest\Laravel\get;
+
+uses(RefreshDatabase::class);
+
+it('generates missing emails', function (): void {
+    Config::set('socialstream.features', [
+        Features::generateMissingEmails(),
+    ]);
+
+    $user = (new SocialiteUser())
+        ->map([
+            'id' => $githubId = fake()->numerify('########'),
+            'nickname' => 'joel',
+            'name' => 'Joel',
+            'avatar' => null,
+            'avatar_original' => null,
+        ])
+        ->setToken('user-token')
+        ->setRefreshToken('refresh-token')
+        ->setExpiresIn(3600);
+
+    $provider = Mockery::mock(GithubProvider::class);
+    $provider->shouldReceive('user')->once()->andReturn($user);
+
+    Socialite::shouldReceive('driver')->once()->with('github')->andReturn($provider);
+
+    session()->put('socialstream.previous_url', route('register'));
+
+    get('http://localhost/oauth/github/callback')
+        ->assertRedirect(RouteServiceProvider::HOME);
+
+    $user = User::first();
+
+    $this->assertAuthenticated();
+    $this->assertEquals("$githubId@github", $user->email);
+    $this->assertDatabaseHas('connected_accounts', [
+        'provider' => 'github',
+        'provider_id' => $githubId,
+        'email' => $user->email,
+    ]);
+});
diff --git a/tests/Feature/LoginOnRegistrationTest.php b/tests/Feature/LoginOnRegistrationTest.php
@@ -0,0 +1,63 @@
+<?php
+
+namespace JoelButcher\Socialstream\Tests\Feature;
+
+use App\Models\User;
+use App\Providers\RouteServiceProvider;
+use Illuminate\Foundation\Testing\RefreshDatabase;
+use Illuminate\Support\Facades\Config;
+use Illuminate\Support\Facades\Hash;
+use JoelButcher\Socialstream\Features;
+use Laravel\Socialite\Facades\Socialite;
+use Laravel\Socialite\Two\GithubProvider;
+use Laravel\Socialite\Two\User as SocialiteUser;
+use Mockery;
+
+use function Pest\Laravel\get;
+
+uses(RefreshDatabase::class);
+
+test('users can login on registration', function (): void {
+    Config::set('socialstream.features', [
+        Features::loginOnRegistration(),
+    ]);
+
+    User::create([
+        'name' => 'Joel Butcher',
+        'email' => '[email protected]',
+        'password' => Hash::make('password'),
+    ]);
+
+    $this->assertDatabaseHas('users', ['email' => '[email protected]']);
+    $this->assertDatabaseEmpty('connected_accounts');
+
+    $user = (new SocialiteUser())
+        ->map([
+            'id' => $githubId = fake()->numerify('########'),
+            'nickname' => 'joel',
+            'name' => 'Joel',
+            'email' => '[email protected]',
+            'avatar' => null,
+            'avatar_original' => null,
+        ])
+        ->setToken('user-token')
+        ->setRefreshToken('refresh-token')
+        ->setExpiresIn(3600);
+
+    $provider = Mockery::mock(GithubProvider::class);
+    $provider->shouldReceive('user')->once()->andReturn($user);
+
+    Socialite::shouldReceive('driver')->once()->with('github')->andReturn($provider);
+
+    session()->put('socialstream.previous_url', route('register'));
+
+    get('http://localhost/oauth/github/callback')
+        ->assertRedirect(RouteServiceProvider::HOME);
+
+    $this->assertAuthenticated();
+    $this->assertDatabaseHas('connected_accounts', [
+        'provider' => 'github',
+        'provider_id' => $githubId,
+        'email' => '[email protected]',
+    ]);
+});