Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions app/Http/Controllers/App/Settings/AuthenticationController.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
use Illuminate\Validation\Rule;
use Inertia\Inertia;
use Inertia\Response;
use Laravel\Socialite\Facades\Socialite;

class AuthenticationController extends Controller
{
Expand Down Expand Up @@ -63,6 +64,16 @@ public function destroyOtherSessions(Request $request): RedirectResponse
return back()->with('flash.success', __('settings.authentication.sessions.flash_logged_out'));
}

public function connectProvider(string $provider): RedirectResponse
{
abort_unless(in_array($provider, self::PROVIDERS, true), 404);

return match ($provider) {
'google' => Socialite::driver('google-auth')->redirect(),
'github' => Socialite::driver('github')->scopes(['read:user', 'user:email'])->redirect(),
};
}

public function disconnectProvider(Request $request, string $provider): RedirectResponse
{
abort_unless(in_array($provider, self::PROVIDERS, true), 404);
Expand Down
26 changes: 26 additions & 0 deletions app/Http/Controllers/Auth/GitHubController.php
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,13 @@ public function callback(): RedirectResponse
return redirect()->route('login');
}

// The signup/login redirect is gated by the `guest` middleware and
// the connect-from-settings redirect by `auth`, so this is a safe
// signal for which flow we came from.
if (Auth::check()) {
return $this->connectToCurrentUser(Auth::user(), (string) $githubUser->getId());
}

$user = User::where('github_id', (string) $githubUser->getId())
->when($githubUser->getEmail(), fn ($query, $email) => $query->orWhere('email', $email))
->first();
Expand All @@ -52,6 +59,25 @@ public function callback(): RedirectResponse
return $this->registerNewUser($githubUser);
}

private function connectToCurrentUser(User $user, string $githubId): RedirectResponse
{
$existing = User::where('github_id', $githubId)
->where('id', '!=', $user->id)
->first();

if ($existing) {
return redirect()->route('app.authentication.edit')
->with('flash.error', __('settings.authentication.providers.flash_already_linked', ['provider' => 'GitHub']));
}

if ($user->github_id !== $githubId) {
$user->update(['github_id' => $githubId]);
}

return redirect()->route('app.authentication.edit')
->with('flash.success', __('settings.authentication.providers.flash_connected', ['provider' => 'GitHub']));
}

private function loginExistingUser(User $user, string $githubId): RedirectResponse
{
if (! $user->github_id) {
Expand Down
26 changes: 26 additions & 0 deletions app/Http/Controllers/Auth/GoogleController.php
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,13 @@ public function callback(): RedirectResponse
return redirect()->route('login');
}

// The signup/login redirect is gated by the `guest` middleware and
// the connect-from-settings redirect by `auth`, so this is a safe
// signal for which flow we came from.
if (Auth::check()) {
return $this->connectToCurrentUser(Auth::user(), $googleUser->getId());
}

$user = User::where('google_id', $googleUser->getId())
->orWhere('email', $googleUser->getEmail())
->first();
Expand All @@ -44,6 +51,25 @@ public function callback(): RedirectResponse
return $this->registerNewUser($googleUser);
}

private function connectToCurrentUser(User $user, string $googleId): RedirectResponse
{
$existing = User::where('google_id', $googleId)
->where('id', '!=', $user->id)
->first();

if ($existing) {
return redirect()->route('app.authentication.edit')
->with('flash.error', __('settings.authentication.providers.flash_already_linked', ['provider' => 'Google']));
}

if ($user->google_id !== $googleId) {
$user->update(['google_id' => $googleId]);
}

return redirect()->route('app.authentication.edit')
->with('flash.success', __('settings.authentication.providers.flash_connected', ['provider' => 'Google']));
}

private function loginExistingUser(User $user, string $googleId): RedirectResponse
{
if (! $user->google_id) {
Expand Down
2 changes: 2 additions & 0 deletions lang/en/settings.php
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,8 @@
'connect' => 'Connect',
'disconnect' => 'Disconnect',
'flash_disconnected' => ':provider disconnected successfully.',
'flash_connected' => ':provider connected successfully.',
'flash_already_linked' => 'That :provider account is already linked to another user.',
'flash_cannot_disconnect' => 'You cannot disconnect your only sign-in method. Set a password or connect another provider first.',
],
],
Expand Down
2 changes: 2 additions & 0 deletions lang/es/settings.php
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,8 @@
'connect' => 'Conectar',
'disconnect' => 'Desconectar',
'flash_disconnected' => ':provider desconectada correctamente.',
'flash_connected' => ':provider conectada correctamente.',
'flash_already_linked' => 'Esa cuenta de :provider ya está vinculada a otro usuario.',
'flash_cannot_disconnect' => 'No puedes desconectar tu único método de inicio de sesión. Define una contraseña o conecta otro proveedor primero.',
],
],
Expand Down
2 changes: 2 additions & 0 deletions lang/pt-BR/settings.php
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,8 @@
'connect' => 'Conectar',
'disconnect' => 'Desconectar',
'flash_disconnected' => ':provider desconectada com sucesso.',
'flash_connected' => ':provider conectada com sucesso.',
'flash_already_linked' => 'Essa conta do :provider já está vinculada a outro usuário.',
'flash_cannot_disconnect' => 'Você não pode desconectar seu único método de login. Defina uma senha ou conecte outro provedor primeiro.',
],
],
Expand Down
28 changes: 10 additions & 18 deletions resources/js/pages/settings/profile/Authentication.vue
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<script setup lang="ts">
import { Form, Head, Link } from '@inertiajs/vue3';
import { Form, Head } from '@inertiajs/vue3';
import { IconDeviceDesktop, IconDeviceMobile } from '@tabler/icons-vue';
import { trans } from 'laravel-vue-i18n';
import { computed, ref } from 'vue';
Expand All @@ -26,11 +26,9 @@ import { Separator } from '@/components/ui/separator';
import AppLayout from '@/layouts/AppLayout.vue';
import { isMobileDevice, parseBrowserName, parseOsName } from '@/lib/userAgent';
import { settings as settingsHub } from '@/routes/app';
import { edit as editAuthentication } from '@/routes/app/authentication';
import { connectProvider, edit as editAuthentication } from '@/routes/app/authentication';
import { preferences as notificationPreferences } from '@/routes/app/notifications';
import { edit as editProfile } from '@/routes/app/profile';
import { redirect as githubRedirect } from '@/routes/auth/github';
import { redirect as googleRedirect } from '@/routes/auth/google';
import type { BreadcrumbItem } from '@/types';

type Session = {
Expand Down Expand Up @@ -66,10 +64,6 @@ const tabs = computed(() => [
{ name: 'notifications', label: trans('settings.nav.notifications'), href: notificationPreferences().url },
]);

const providerRedirects: Record<ConnectedAccount['provider'], () => { url: string }> = {
google: googleRedirect,
github: githubRedirect,
};

const passwordHeading = computed(() =>
props.hasPassword
Expand Down Expand Up @@ -333,18 +327,16 @@ const logoutDialogOpen = ref(false);
{{ $t('settings.authentication.providers.disconnect') }}
</Button>
</Form>
<Link
<Button
v-else-if="!account.connected"
:href="providerRedirects[account.provider]().url"
variant="outline"
size="sm"
as="a"
:href="connectProvider(account.provider).url"
:data-test="`connect-${account.provider}`"
>
<Button
variant="outline"
size="sm"
:data-test="`connect-${account.provider}`"
>
{{ $t('settings.authentication.providers.connect') }}
</Button>
</Link>
{{ $t('settings.authentication.providers.connect') }}
</Button>
</div>
</div>
</div>
Expand Down
2 changes: 2 additions & 0 deletions routes/app.php
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,8 @@
->name('app.authentication.update-password');
Route::delete('settings/authentication/sessions', [AuthenticationController::class, 'destroyOtherSessions'])
->name('app.authentication.destroy-other-sessions');
Route::get('settings/authentication/providers/{provider}/connect', [AuthenticationController::class, 'connectProvider'])
->name('app.authentication.connect-provider');
Route::delete('settings/authentication/providers/{provider}', [AuthenticationController::class, 'disconnectProvider'])
->name('app.authentication.disconnect-provider');

Expand Down
10 changes: 7 additions & 3 deletions routes/auth.php
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,16 @@
Route::post('/reset-password', [NewPasswordController::class, 'store'])->name('password.store');

Route::get('/auth/google/redirect', [GoogleController::class, 'redirect'])->name('auth.google.redirect');
Route::get('/auth/google/callback', [GoogleController::class, 'callback'])->name('auth.google.callback');

Route::get('/auth/github/redirect', [GitHubController::class, 'redirect'])->name('auth.github.redirect');
Route::get('/auth/github/callback', [GitHubController::class, 'callback'])->name('auth.github.callback');
});

// Callbacks must be reachable by both guests (signup/login) and authenticated
// users (connect-from-settings). The redirect routes that initiate the OAuth
// round-trip enforce the right middleware, so the callback can safely branch
// on `Auth::check()` to dispatch to the matching flow.
Route::get('/auth/google/callback', [GoogleController::class, 'callback'])->name('auth.google.callback');
Route::get('/auth/github/callback', [GitHubController::class, 'callback'])->name('auth.github.callback');

Route::middleware(['auth'])->group(function () {
Route::get('/register/success', SignupSuccessController::class)->name('register.success');

Expand Down
166 changes: 166 additions & 0 deletions tests/Feature/Auth/ConnectProviderTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,166 @@
<?php

declare(strict_types=1);

use App\Models\User;
use Laravel\Socialite\Facades\Socialite;
use Laravel\Socialite\Two\AbstractProvider;
use Laravel\Socialite\Two\User as SocialiteUser;

test('authenticated user can hit the connect-provider route for github', function () {
$user = User::factory()->create();

$driver = Mockery::mock(AbstractProvider::class);
$driver->shouldReceive('scopes')->andReturnSelf();
$driver->shouldReceive('redirect')->andReturn(redirect('https://github.com/login/oauth/authorize'));
Socialite::shouldReceive('driver')->with('github')->andReturn($driver);

$this->actingAs($user)
->get(route('app.authentication.connect-provider', 'github'))
->assertRedirect('https://github.com/login/oauth/authorize');
});

test('authenticated user can hit the connect-provider route for google', function () {
$user = User::factory()->create();

$driver = Mockery::mock(AbstractProvider::class);
$driver->shouldReceive('redirect')->andReturn(redirect('https://accounts.google.com/o/oauth2/auth'));
Socialite::shouldReceive('driver')->with('google-auth')->andReturn($driver);

$this->actingAs($user)
->get(route('app.authentication.connect-provider', 'google'))
->assertRedirect('https://accounts.google.com/o/oauth2/auth');
});

test('connect-provider route rejects unknown provider', function () {
$user = User::factory()->create();

$this->actingAs($user)
->get(route('app.authentication.connect-provider', 'twitter'))
->assertNotFound();
});

test('connect-provider route requires authentication', function () {
$this->get(route('app.authentication.connect-provider', 'github'))
->assertRedirect(route('login'));
});

test('authenticated callback connects github to the current user', function () {
$user = User::factory()->create([
'email' => 'me@example.com',
'google_id' => 'g-me',
'github_id' => null,
]);

$socialiteUser = new SocialiteUser;
$socialiteUser->id = 'gh-me';
$socialiteUser->name = 'Me';
$socialiteUser->email = 'me@example.com';

$driver = Mockery::mock(AbstractProvider::class);
$driver->shouldReceive('user')->andReturn($socialiteUser);
Socialite::shouldReceive('driver')->with('github')->andReturn($driver);

$this->actingAs($user)
->get(route('auth.github.callback'))
->assertRedirect(route('app.authentication.edit'))
->assertSessionHas('flash.success');

expect($user->fresh()->github_id)->toBe('gh-me');
});

test('authenticated callback links github by current user, not by email', function () {
$user = User::factory()->create([
'email' => 'work@example.com',
'google_id' => 'g-me',
'github_id' => null,
]);

$socialiteUser = new SocialiteUser;
$socialiteUser->id = 'gh-personal';
$socialiteUser->name = 'Me';
$socialiteUser->email = 'personal@example.com';

$driver = Mockery::mock(AbstractProvider::class);
$driver->shouldReceive('user')->andReturn($socialiteUser);
Socialite::shouldReceive('driver')->with('github')->andReturn($driver);

$this->actingAs($user)
->get(route('auth.github.callback'))
->assertRedirect(route('app.authentication.edit'))
->assertSessionHas('flash.success');

expect($user->fresh()->github_id)->toBe('gh-personal');
expect(User::where('email', 'personal@example.com')->exists())->toBeFalse();
$this->assertAuthenticatedAs($user);
});

test('authenticated callback rejects when github account is already linked to another user', function () {
User::factory()->create(['github_id' => 'gh-taken']);

$me = User::factory()->create(['email' => 'me@example.com', 'github_id' => null]);

$socialiteUser = new SocialiteUser;
$socialiteUser->id = 'gh-taken';
$socialiteUser->name = 'Me';
$socialiteUser->email = 'me@example.com';

$driver = Mockery::mock(AbstractProvider::class);
$driver->shouldReceive('user')->andReturn($socialiteUser);
Socialite::shouldReceive('driver')->with('github')->andReturn($driver);

$this->actingAs($me)
->get(route('auth.github.callback'))
->assertRedirect(route('app.authentication.edit'))
->assertSessionHas('flash.error');

expect($me->fresh()->github_id)->toBeNull();
$this->assertAuthenticatedAs($me);
});

test('authenticated callback connects google to the current user', function () {
$user = User::factory()->create([
'email' => 'me@example.com',
'github_id' => 'gh-me',
'google_id' => null,
]);

$socialiteUser = new SocialiteUser;
$socialiteUser->id = 'g-me';
$socialiteUser->name = 'Me';
$socialiteUser->email = 'me@example.com';

$driver = Mockery::mock(AbstractProvider::class);
$driver->shouldReceive('user')->andReturn($socialiteUser);
Socialite::shouldReceive('driver')->with('google-auth')->andReturn($driver);

$this->actingAs($user)
->get(route('auth.google.callback'))
->assertRedirect(route('app.authentication.edit'))
->assertSessionHas('flash.success');

expect($user->fresh()->google_id)->toBe('g-me');
});

test('authenticated callback rejects when google account is already linked to another user', function () {
User::factory()->create(['google_id' => 'g-taken']);

$me = User::factory()->create(['email' => 'me@example.com', 'google_id' => null]);

$socialiteUser = new SocialiteUser;
$socialiteUser->id = 'g-taken';
$socialiteUser->name = 'Me';
$socialiteUser->email = 'me@example.com';

$driver = Mockery::mock(AbstractProvider::class);
$driver->shouldReceive('user')->andReturn($socialiteUser);
Socialite::shouldReceive('driver')->with('google-auth')->andReturn($driver);

$this->actingAs($me)
->get(route('auth.google.callback'))
->assertRedirect(route('app.authentication.edit'))
->assertSessionHas('flash.error');

expect($me->fresh()->google_id)->toBeNull();
$this->assertAuthenticatedAs($me);
});
Loading