Fix & cleanup OAuthController (#1599)

app/Extensions/OAuth/OAuthSchemaInterface.php

@@ -34,4 +34,6 @@ interface OAuthSchemaInterface
     public function isEnabled(): bool;
 
     public function shouldCreateMissingUsers(): bool;
+
+    public function shouldLinkMissingUsers(): bool;
 }

app/Extensions/OAuth/Schemas/OAuthSchema.php

@@ -63,9 +63,20 @@ abstract class OAuthSchema implements OAuthSchemaInterface
                 ->offIcon('tabler-x')
                 ->onColor('success')
                 ->offColor('danger')
-                ->formatStateUsing(fn ($state): bool => (bool) $state)
+                ->formatStateUsing(fn ($state) => (bool) $state)
                 ->afterStateUpdated(fn ($state, Set $set) => $set("OAUTH_{$id}_SHOULD_CREATE_MISSING_USERS", (bool) $state))
                 ->default(env("OAUTH_{$id}_SHOULD_CREATE_MISSING_USERS")),
+            Toggle::make("OAUTH_{$id}_SHOULD_LINK_MISSING_USERS")
+                ->label(trans('admin/setting.oauth.link_missing_users'))
+                ->columnSpanFull()
+                ->inline(false)
+                ->onIcon('tabler-check')
+                ->offIcon('tabler-x')
+                ->onColor('success')
+                ->offColor('danger')
+                ->formatStateUsing(fn ($state) => (bool) $state)
+                ->afterStateUpdated(fn ($state, Set $set) => $set("OAUTH_{$id}_SHOULD_LINK_MISSING_USERS", (bool) $state))
+                ->default(env("OAUTH_{$id}_SHOULD_LINK_MISSING_USERS")),
         ];
     }
 
@@ -116,4 +127,11 @@ abstract class OAuthSchema implements OAuthSchemaInterface
 
         return env("OAUTH_{$id}_SHOULD_CREATE_MISSING_USERS", false);
     }
+
+    public function shouldLinkMissingUsers(): bool
+    {
+        $id = Str::upper($this->getId());
+
+        return env("OAUTH_{$id}_SHOULD_LINK_MISSING_USERS", false);
+    }
 }

app/Http/Controllers/Auth/OAuthController.php

@@ -2,38 +2,37 @@
 
 namespace App\Http\Controllers\Auth;
 
+use App\Extensions\OAuth\OAuthSchemaInterface;
 use App\Extensions\OAuth\OAuthService;
 use App\Filament\Pages\Auth\EditProfile;
 use App\Http\Controllers\Controller;
 use App\Models\User;
 use App\Services\Users\UserCreationService;
-use App\Services\Users\UserUpdateService;
+use Exception;
 use Filament\Notifications\Notification;
-use Illuminate\Auth\AuthManager;
 use Illuminate\Http\RedirectResponse;
 use Illuminate\Http\Request;
+use Laravel\Socialite\Contracts\User as OAuthUser;
 use Laravel\Socialite\Facades\Socialite;
+use Symfony\Component\HttpFoundation\RedirectResponse as SymfonyRedirectResponse;
 
 class OAuthController extends Controller
 {
     public function __construct(
-        private readonly AuthManager $auth,
-        private UserCreationService $userCreation,
-        private readonly UserUpdateService $updateService,
+        private readonly UserCreationService $userCreation,
         private readonly OAuthService $oauthService,
     ) {}
 
     /**
      * Redirect user to the OAuth provider
      */
-    public function redirect(string $driver): RedirectResponse
+    public function redirect(string $driver): SymfonyRedirectResponse|RedirectResponse
     {
-        // Driver is disabled - redirect to normal login
         if (!$this->oauthService->get($driver)->isEnabled()) {
             return redirect()->route('auth.login');
         }
 
-        return Socialite::with($driver)->redirect();
+        return Socialite::driver($driver)->redirect();
     }
 
     /**
@@ -43,7 +42,6 @@ class OAuthController extends Controller
     {
         $driver = $this->oauthService->get($driver);
 
-        // Unknown driver or driver is disabled - redirect to normal login
         if (!$driver || !$driver->isEnabled()) {
             return redirect()->route('auth.login');
         }
@@ -52,67 +50,89 @@ class OAuthController extends Controller
         if ($request->get('error')) {
             report($request->get('error_description') ?? $request->get('error'));
 
-            Notification::make()
-                ->title('Something went wrong')
-                ->body($request->get('error'))
-                ->danger()
-                ->persistent()
-                ->send();
-
-            return redirect()->route('auth.login');
+            return $this->errorRedirect($request->get('error'));
         }
 
         $oauthUser = Socialite::driver($driver->getId())->user();
 
-        // User is already logged in and wants to link a new OAuth Provider
         if ($request->user()) {
-            $oauth = $request->user()->oauth;
-            $oauth[$driver->getId()] = $oauthUser->getId();
-
-            $this->updateService->handle($request->user(), ['oauth' => $oauth]);
+            $this->linkUser($request->user(), $driver, $oauthUser);
 
             return redirect(EditProfile::getUrl(['tab' => '-oauth-tab'], panel: 'app'));
         }
 
         $user = User::whereJsonContains('oauth->'. $driver->getId(), $oauthUser->getId())->first();
-
-        if (!$user) {
-            // No user found and auto creation is disabled - redirect to normal login
-            if (!$driver->shouldCreateMissingUsers()) {
-                Notification::make()
-                    ->title('No linked User found')
-                    ->danger()
-                    ->persistent()
-                    ->send();
-
-                return redirect()->route('auth.login');
+        if ($user) {
+            return $this->loginUser($user);
         }
 
-            $username = $oauthUser->getNickname();
+        return $this->handleMissingUser($driver, $oauthUser);
+    }
+
+    private function linkUser(User $user, OAuthSchemaInterface $driver, OAuthUser $oauthUser): User
+    {
+        $oauth = $user->oauth;
+        $oauth[$driver->getId()] = $oauthUser->getId();
+
+        $user->update(['oauth' => $oauth]);
+
+        return $user->refresh();
+    }
+
+    private function handleMissingUser(OAuthSchemaInterface $driver, OAuthUser $oauthUser): RedirectResponse
+    {
         $email = $oauthUser->getEmail();
 
-            // Incomplete data, can't create user - redirect to normal login
         if (!$email) {
-                Notification::make()
-                    ->title('No linked User found')
-                    ->danger()
-                    ->persistent()
-                    ->send();
-
-                return redirect()->route('auth.login');
+            return $this->errorRedirect();
         }
 
+        $user = User::whereEmail($email)->first();
+        if ($user) {
+            if (!$driver->shouldLinkMissingUsers()) {
+                return $this->errorRedirect();
+            }
+
+            $user = $this->linkUser($user, $driver, $oauthUser);
+        } else {
+            if (!$driver->shouldCreateMissingUsers()) {
+                return $this->errorRedirect();
+            }
+
+            try {
                 $user = $this->userCreation->handle([
-                'username' => $username,
+                    'username' => $oauthUser->getNickname(),
                     'email' => $email,
                     'oauth' => [
                         $driver->getId() => $oauthUser->getId(),
                     ],
                 ]);
+            } catch (Exception $exception) {
+                report($exception);
+
+                return $this->errorRedirect();
+            }
         }
 
-        $this->auth->guard()->login($user, true);
+        return $this->loginUser($user);
+    }
+
+    private function loginUser(User $user): RedirectResponse
+    {
+        auth()->guard()->login($user, true);
 
         return redirect('/');
     }
+
+    private function errorRedirect(?string $error = null): RedirectResponse
+    {
+        Notification::make()
+            ->title($error ? 'Something went wrong' : 'No linked User found')
+            ->body($error)
+            ->danger()
+            ->persistent()
+            ->send();
+
+        return redirect()->route('auth.login');
+    }
 }

lang/en/admin/setting.php

@@ -98,6 +98,7 @@ return [
         'display_name' => 'Display Name',
         'auth_url' => 'Authorization callback URL',
         'create_missing_users' => 'Auto Create Missing Users?',
+        'link_missing_users' => 'Auto Link Missing Users?',
     ],
     'misc' => [
         'auto_allocation' => [