fix: connect Google from settings without a 404

app/Http/Controllers/Auth/SocialAuthController.php

@@ -11,6 +11,7 @@
 use App\Http\Controllers\Auth\Concerns\PreservesAttributionParameters;
 use App\Models\User;
 use Illuminate\Auth\Events\Registered;
+use Illuminate\Http\RedirectResponse;
 use Illuminate\Http\Request;
 use Inertia\Inertia;
 use Laravel\Socialite\Facades\Socialite;
@@ -30,11 +31,6 @@ public function redirectToProvider(Request $request, string $provider)
         // Carry campaign parameters across the bounce out to the provider.
         $this->storeAttributionParameters($request);
 
-        // An authenticated user is linking, not signing in.
-        if ($request->user()) {
-            $request->session()->put('social_connect', $socialProvider->value);
-        }
-
         return Inertia::location(
             Socialite::driver($socialProvider->value)->redirect()->getTargetUrl(),
         );
@@ -52,21 +48,10 @@ public function handleProviderCallback(Request $request, string $provider)
 
         $column = $socialProvider->column();
 
-        // Linking an extra provider to the account already signed in.
-        if ($request->session()->pull('social_connect') === $socialProvider->value && $request->user()) {
-            if (LinkSocialAccount::isTaken($socialProvider, $socialUser->getId(), $request->user())) {
-                session()->flash('flash.banner', "That {$socialProvider->label()} account is already linked to another user.");
-                session()->flash('flash.bannerStyle', 'danger');
-
-                return redirect(route('setting.authentication.edit'));
-            }
-
-            LinkSocialAccount::execute($request->user(), $socialProvider, $socialUser->getId());
-
-            session()->flash('flash.banner', "{$socialProvider->label()} connected.");
-            session()->flash('flash.bannerStyle', 'success');
-
-            return redirect(route('setting.authentication.edit'));
+        // `guest` middleware gates login/signup; `auth` gates the settings
+        // connect flow. The callback is reachable by both and branches here.
+        if ($request->user()) {
+            return $this->connectToCurrentUser($request->user(), $socialProvider, $socialUser->getId());
         }
 
         $existingUser = User::where($column, $socialUser->getId())
@@ -99,6 +84,25 @@ public function handleProviderCallback(Request $request, string $provider)
         return redirect(route('links.index'));
     }
 
+    private function connectToCurrentUser(User $user, SocialAuthProvider $socialProvider, string $providerId): RedirectResponse
+    {
+        if (LinkSocialAccount::isTaken($socialProvider, $providerId, $user)) {
+            session()->flash('flash.banner', "That {$socialProvider->label()} account is already linked to another user.");
+            session()->flash('flash.bannerStyle', 'danger');
+
+            return redirect(route('setting.authentication.edit'));
+        }
+
+        if ($user->{$socialProvider->column()} !== $providerId) {
+            LinkSocialAccount::execute($user, $socialProvider, $providerId);
+        }
+
+        session()->flash('flash.banner', "{$socialProvider->label()} connected.");
+        session()->flash('flash.bannerStyle', 'success');
+
+        return redirect(route('setting.authentication.edit'));
+    }
+
     private function provider(string $provider): SocialAuthProvider
     {
         $socialProvider = SocialAuthProvider::tryFrom($provider);

app/Http/Controllers/Setting/AuthenticationController.php

@@ -17,6 +17,7 @@
 use Illuminate\Support\Facades\DB;
 use Inertia\Inertia;
 use Inertia\Response;
+use Laravel\Socialite\Facades\Socialite;
 
 class AuthenticationController extends Controller
 {
@@ -53,6 +54,15 @@ public function destroyOtherSessions(DestroySessionsRequest $request): RedirectR
         return back();
     }
 
+    public function connectProvider(string $provider): RedirectResponse
+    {
+        $socialProvider = SocialAuthProvider::tryFrom($provider);
+
+        abort_unless($socialProvider?->isEnabled(), 404);
+
+        return Socialite::driver($socialProvider->value)->redirect();
+    }
+
     public function disconnectProvider(Request $request, string $provider): RedirectResponse
     {
         $socialProvider = SocialAuthProvider::tryFrom($provider);
@@ -75,7 +85,7 @@ public function disconnectProvider(Request $request, string $provider): Redirect
     }
 
     /**
-     * @return array<int, array{provider: string, label: string, connected: bool}>
+     * @return array<int, array{provider: string, label: string, connected: bool, enabled: bool}>
      */
     private function connectedAccounts(User $user): array
     {
@@ -84,6 +94,7 @@ private function connectedAccounts(User $user): array
                 'provider' => $provider->value,
                 'label' => $provider->label(),
                 'connected' => filled($user->{"{$provider->value}_id"}),
+                'enabled' => $provider->isEnabled(),
             ])->values()->all();
     }
 

resources/js/pages/Setting/Authentication/Index.vue

@@ -9,15 +9,18 @@ import { Input } from '@/components/ui/input';
 import { Label } from '@/components/ui/label';
 import AppLayout from '@/layouts/AppLayout.vue';
 import SettingsLayout from '@/layouts/settings/Layout.vue';
-import { social } from '@/routes/auth';
 import { password as passwordRoute } from '@/routes/setting/authentication';
-import { destroy as destroyProvider } from '@/routes/setting/authentication/providers';
+import {
+    connect as connectProvider,
+    destroy as destroyProvider,
+} from '@/routes/setting/authentication/providers';
 import { destroy as destroySessions } from '@/routes/setting/authentication/sessions';
 
 type ConnectedAccount = {
     provider: string;
     label: string;
     connected: boolean;
+    enabled: boolean;
 };
 
 type Session = {
@@ -213,8 +216,15 @@ const disconnect = (provider: string) => {
                         >
                             Disconnect
                         </Button>
-                        <Button v-else variant="outline" size="sm" as-child>
-                            <a :href="social(account.provider).url">Connect</a>
+                        <Button
+                            v-else-if="account.enabled"
+                            variant="outline"
+                            size="sm"
+                            as-child
+                        >
+                            <a :href="connectProvider.url(account.provider)"
+                                >Connect</a
+                            >
                         </Button>
                     </div>
                 </div>

routes/auth.php

@@ -42,6 +42,12 @@ function () {
         Route::get('/invites/{id}', [InviteController::class, 'show'])->name('auth.invites.show');
         Route::post('/invites/{id}', [InviteController::class, 'accept'])->name('auth.invites.accept');
 
+        // Sign-in / register only. Connecting a provider to an account that
+        // is already signed in goes through settings (see connectProvider).
+        Route::get('/auth/{provider}/login', [SocialAuthController::class, 'redirectToProvider'])
+            ->whereIn('provider', ['google', 'github'])
+            ->name('auth.social');
+
     }
 );
 
@@ -66,20 +72,14 @@ function () {
 );
 
 /**
- * Social sign-in, reachable signed in as well as out: signed out it signs you
- * in or registers you, signed in it links the provider to the account you are
- * already using. These sat inside the guest group, so the Connect button on the
- * authentication settings screen bounced off the middleware and never arrived.
+ * Callbacks must be reachable by both guests (signup/login) and authenticated
+ * users (connect-from-settings). The routes that start the OAuth round-trip
+ * enforce the right middleware, so the callback can branch on Auth::check().
+ *
+ * Under /auth, which is what config/services.php already told each provider
+ * to redirect to. At the root these took two first-level segments, so
+ * `google` and `github` were reserved as short-link back-halves for no reason.
  */
-Route::middleware('web')->group(function () {
-    // Under /auth, which is what config/services.php already told each
-    // provider to redirect to. At the root these took two first-level
-    // segments, so `google` and `github` were reserved as short-link
-    // back-halves for no reason.
-    Route::get('/auth/{provider}/login', [SocialAuthController::class, 'redirectToProvider'])
-        ->whereIn('provider', ['google', 'github'])
-        ->name('auth.social');
-    Route::get('/auth/{provider}/callback', [SocialAuthController::class, 'handleProviderCallback'])
-        ->whereIn('provider', ['google', 'github'])
-        ->name('auth.social.callback');
-});
+Route::get('/auth/{provider}/callback', [SocialAuthController::class, 'handleProviderCallback'])
+    ->whereIn('provider', ['google', 'github'])
+    ->name('auth.social.callback');

routes/web.php

@@ -70,6 +70,10 @@ function () {
             Route::get('/authentication', [AuthenticationController::class, 'edit'])->name('setting.authentication.edit')->withoutMiddleware(['set-store']);
             Route::put('/authentication/password', [AuthenticationController::class, 'updatePassword'])->name('setting.authentication.password')->withoutMiddleware(['set-store']);
             Route::delete('/authentication/sessions', [AuthenticationController::class, 'destroyOtherSessions'])->name('setting.authentication.sessions.destroy')->withoutMiddleware(['set-store']);
+            Route::get('/authentication/providers/{provider}/connect', [AuthenticationController::class, 'connectProvider'])
+                ->whereIn('provider', ['google', 'github'])
+                ->name('setting.authentication.providers.connect')
+                ->withoutMiddleware(['set-store']);
             Route::delete('/authentication/providers/{provider}', [AuthenticationController::class, 'disconnectProvider'])->name('setting.authentication.providers.destroy')->withoutMiddleware(['set-store']);
 
             // mcp

tests/Feature/Auth/ConnectProviderTest.php

@@ -0,0 +1,62 @@
+<?php
+
+declare(strict_types=1);
+
+use App\Models\User;
+use Illuminate\Foundation\Testing\RefreshDatabase;
+use Laravel\Socialite\Facades\Socialite;
+use Laravel\Socialite\Two\AbstractProvider;
+
+uses(RefreshDatabase::class);
+
+beforeEach(function () {
+    config([
+        'lua.auth.google' => true,
+        'lua.auth.github' => true,
+        'services.google.client_id' => 'id',
+        'services.google.client_secret' => 'secret',
+        'services.google.redirect' => 'https://lua.test/auth/google/callback',
+        'services.github.client_id' => 'id',
+        'services.github.client_secret' => 'secret',
+        'services.github.redirect' => 'https://lua.test/auth/github/callback',
+    ]);
+});
+
+it('sends a signed-in user to google from the settings connect route', function () {
+    $driver = Mockery::mock(AbstractProvider::class);
+    $driver->shouldReceive('redirect')->andReturn(redirect('https://accounts.google.com/o/oauth2/auth'));
+    Socialite::shouldReceive('driver')->with('google')->andReturn($driver);
+
+    $this->actingAs(User::factory()->withWorkspace()->create())
+        ->get(route('setting.authentication.providers.connect', 'google'))
+        ->assertRedirect('https://accounts.google.com/o/oauth2/auth');
+});
+
+it('sends a signed-in user to github from the settings connect route', function () {
+    $driver = Mockery::mock(AbstractProvider::class);
+    $driver->shouldReceive('redirect')->andReturn(redirect('https://github.com/login/oauth/authorize'));
+    Socialite::shouldReceive('driver')->with('github')->andReturn($driver);
+
+    $this->actingAs(User::factory()->withWorkspace()->create())
+        ->get(route('setting.authentication.providers.connect', 'github'))
+        ->assertRedirect('https://github.com/login/oauth/authorize');
+});
+
+it('refuses an unknown provider on the settings connect route', function () {
+    $this->actingAs(User::factory()->withWorkspace()->create())
+        ->get(route('setting.authentication.providers.connect', 'twitter'))
+        ->assertNotFound();
+});
+
+it('404s the settings connect route when the provider is off', function () {
+    config(['lua.auth.github' => false]);
+
+    $this->actingAs(User::factory()->withWorkspace()->create())
+        ->get(route('setting.authentication.providers.connect', 'github'))
+        ->assertNotFound();
+});
+
+it('asks a guest to sign in before connecting a provider', function () {
+    $this->get(route('setting.authentication.providers.connect', 'google'))
+        ->assertRedirect(route('login'));
+});

tests/Feature/Auth/SocialAuthTest.php

@@ -47,15 +47,10 @@ function fakeSocialite(?SocialiteUser $user): void
     $this->get(route('auth.social', 'google'))->assertRedirect();
 });
 
-it('remembers you were linking when you were already signed in', function () {
-    fakeSocialite(fakeSocialUser('g-1', 'ada@example.com'));
-
-    // The Connect button on the authentication screen used to bounce off the
-    // guest middleware and never reach this at all.
+it('sends a signed-in user away from the guest login start', function () {
     $this->actingAs(User::factory()->withWorkspace()->create())
         ->get(route('auth.social', 'google'))
-        ->assertRedirect()
-        ->assertSessionHas('social_connect', 'google');
+        ->assertRedirect('/links');
 });
 
 it('refuses a provider we do not offer', function () {
@@ -117,7 +112,6 @@ function fakeSocialite(?SocialiteUser $user): void
     fakeSocialite(fakeSocialUser('g-1', 'someone@example.com'));
 
     $this->actingAs($user)
-        ->withSession(['social_connect' => 'google'])
         ->get(route('auth.social.callback', 'google'))
         ->assertRedirect(route('setting.authentication.edit'));
 
@@ -131,13 +125,28 @@ function fakeSocialite(?SocialiteUser $user): void
     fakeSocialite(fakeSocialUser('g-1', 'someone@example.com'));
 
     $this->actingAs($user)
-        ->withSession(['social_connect' => 'google'])
         ->get(route('auth.social.callback', 'google'))
         ->assertRedirect(route('setting.authentication.edit'));
 
     expect($user->fresh()->google_id)->toBeNull();
 });
 
+it('links the provider to the signed-in user even when the emails differ', function () {
+    $user = User::factory()->withWorkspace()->create([
+        'email' => 'work@example.com',
+        'google_id' => null,
+    ]);
+
+    fakeSocialite(fakeSocialUser('g-personal', 'personal@example.com'));
+
+    $this->actingAs($user)
+        ->get(route('auth.social.callback', 'google'))
+        ->assertRedirect(route('setting.authentication.edit'));
+
+    expect($user->fresh()->google_id)->toBe('g-personal')
+        ->and(User::where('email', 'personal@example.com')->exists())->toBeFalse();
+});
+
 it('falls back to the nickname when the provider has no name', function () {
     fakeSocialite(fakeSocialUser('g-1', 'ada@example.com', name: null));
 

tests/Feature/Setting/AuthenticationTest.php

@@ -28,6 +28,27 @@
     actingAs($this->user)->get(route('setting.authentication.edit'))->assertOk();
 });
 
+it('tells the screen which social providers can be connected', function () {
+    config([
+        'lua.auth.google' => true,
+        'lua.auth.github' => false,
+        'services.google.client_id' => 'id',
+        'services.google.client_secret' => 'secret',
+    ]);
+
+    actingAs($this->user)
+        ->get(route('setting.authentication.edit'))
+        ->assertOk()
+        ->assertInertia(fn (Assert $page) => $page
+            ->has('connectedAccounts', 2)
+            ->where('connectedAccounts.0.provider', 'google')
+            ->where('connectedAccounts.0.enabled', true)
+            ->where('connectedAccounts.1.provider', 'github')
+            ->where('connectedAccounts.1.enabled', false)
+            ->etc()
+        );
+});
+
 it('updates the password when the current one is right', function () {
     actingAs($this->user)
         ->put(route('setting.authentication.password'), [