refactor: dedicated callback URL for connect-from-settings flow
Splits the OAuth connect flow off entirely from signup/login so each
route has a single responsibility.
- routes/auth.php returns to its original state — redirect + callback
both back inside the `guest` group.
- routes/app.php gains a paired callback route at
/settings/authentication/providers/{provider}/callback.
- AuthenticationController::connectProvider /
connectProviderCallback override Socialite's redirectUrl so the
round-trip stays on the connect-flow URL. The Auth::check() branch
in the auth controllers is gone.
The OAuth apps in Google Cloud and GitHub Developer Settings need the
new callback URL registered alongside the existing one — documented in
.env.example.
This commit is contained in:
parent
d89f6ceede
commit
79f800f858
7 changed files with 108 additions and 90 deletions
10
.env.example
10
.env.example
|
|
@ -123,7 +123,10 @@ THREADS_CLIENT_SECRET=
|
|||
THREADS_CLIENT_REDIRECT="${APP_URL}/accounts/threads/callback"
|
||||
|
||||
# Google (https://console.cloud.google.com)
|
||||
# Used for YouTube social account connection AND Google login/signup
|
||||
# Used for YouTube social account connection AND Google login/signup.
|
||||
# Register BOTH callback URLs in the OAuth app's authorized redirect URIs:
|
||||
# - ${APP_URL}/auth/google/callback (signup/login)
|
||||
# - ${APP_URL}/settings/authentication/providers/google/callback (link from Settings)
|
||||
GOOGLE_AUTH_ENABLED=false
|
||||
GOOGLE_CLIENT_ID=
|
||||
GOOGLE_CLIENT_SECRET=
|
||||
|
|
@ -131,7 +134,10 @@ GOOGLE_CLIENT_REDIRECT="${APP_URL}/accounts/youtube/callback"
|
|||
GOOGLE_AUTH_CALLBACK="${APP_URL}/auth/google/callback"
|
||||
|
||||
# GitHub (https://github.com/settings/developers)
|
||||
# Used for GitHub login/signup
|
||||
# Used for GitHub login/signup.
|
||||
# Register BOTH callback URLs in the OAuth app's authorized redirect URIs:
|
||||
# - ${APP_URL}/auth/github/callback (signup/login)
|
||||
# - ${APP_URL}/settings/authentication/providers/github/callback (link from Settings)
|
||||
GITHUB_AUTH_ENABLED=false
|
||||
GITHUB_CLIENT_ID=
|
||||
GITHUB_CLIENT_SECRET=
|
||||
|
|
|
|||
|
|
@ -15,6 +15,7 @@
|
|||
use Inertia\Inertia;
|
||||
use Inertia\Response;
|
||||
use Laravel\Socialite\Facades\Socialite;
|
||||
use Laravel\Socialite\Two\AbstractProvider;
|
||||
|
||||
class AuthenticationController extends Controller
|
||||
{
|
||||
|
|
@ -68,9 +69,54 @@ public function connectProvider(string $provider): RedirectResponse
|
|||
{
|
||||
abort_unless(in_array($provider, self::PROVIDERS, true), 404);
|
||||
|
||||
return $this->driver($provider)->redirect();
|
||||
}
|
||||
|
||||
public function connectProviderCallback(Request $request, string $provider): RedirectResponse
|
||||
{
|
||||
abort_unless(in_array($provider, self::PROVIDERS, true), 404);
|
||||
|
||||
try {
|
||||
$providerUser = $this->driver($provider)->user();
|
||||
} catch (\Exception) {
|
||||
return redirect()->route('app.authentication.edit')
|
||||
->with('flash.error', __('settings.authentication.providers.flash_already_linked', ['provider' => ucfirst($provider)]));
|
||||
}
|
||||
|
||||
$user = $request->user();
|
||||
$column = "{$provider}_id";
|
||||
$providerId = (string) $providerUser->getId();
|
||||
|
||||
$existing = User::where($column, $providerId)
|
||||
->where('id', '!=', $user->id)
|
||||
->first();
|
||||
|
||||
if ($existing) {
|
||||
return redirect()->route('app.authentication.edit')
|
||||
->with('flash.error', __('settings.authentication.providers.flash_already_linked', ['provider' => ucfirst($provider)]));
|
||||
}
|
||||
|
||||
if ($user->{$column} !== $providerId) {
|
||||
$user->update([$column => $providerId]);
|
||||
}
|
||||
|
||||
return redirect()->route('app.authentication.edit')
|
||||
->with('flash.success', __('settings.authentication.providers.flash_connected', ['provider' => ucfirst($provider)]));
|
||||
}
|
||||
|
||||
/**
|
||||
* Build a Socialite driver for the connect flow with its dedicated
|
||||
* callback URL. The signup/login flow uses the driver's default
|
||||
* redirect URL configured in `config/services.php`; here we override
|
||||
* so each flow round-trips through its own route.
|
||||
*/
|
||||
private function driver(string $provider): AbstractProvider
|
||||
{
|
||||
$callback = route('app.authentication.connect-provider.callback', $provider);
|
||||
|
||||
return match ($provider) {
|
||||
'google' => Socialite::driver('google-auth')->redirect(),
|
||||
'github' => Socialite::driver('github')->scopes(['read:user', 'user:email'])->redirect(),
|
||||
'google' => Socialite::driver('google-auth')->redirectUrl($callback),
|
||||
'github' => Socialite::driver('github')->scopes(['read:user', 'user:email'])->redirectUrl($callback),
|
||||
};
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -35,10 +35,6 @@ public function callback(): RedirectResponse
|
|||
return redirect()->route('login');
|
||||
}
|
||||
|
||||
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();
|
||||
|
|
@ -56,25 +52,6 @@ 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) {
|
||||
|
|
|
|||
|
|
@ -33,10 +33,6 @@ public function callback(): RedirectResponse
|
|||
return redirect()->route('login');
|
||||
}
|
||||
|
||||
if (Auth::check()) {
|
||||
return $this->connectToCurrentUser(Auth::user(), $googleUser->getId());
|
||||
}
|
||||
|
||||
$user = User::where('google_id', $googleUser->getId())
|
||||
->orWhere('email', $googleUser->getEmail())
|
||||
->first();
|
||||
|
|
@ -48,25 +44,6 @@ 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) {
|
||||
|
|
|
|||
|
|
@ -251,6 +251,8 @@
|
|||
->name('app.authentication.destroy-other-sessions');
|
||||
Route::get('settings/authentication/providers/{provider}/connect', [AuthenticationController::class, 'connectProvider'])
|
||||
->name('app.authentication.connect-provider');
|
||||
Route::get('settings/authentication/providers/{provider}/callback', [AuthenticationController::class, 'connectProviderCallback'])
|
||||
->name('app.authentication.connect-provider.callback');
|
||||
Route::delete('settings/authentication/providers/{provider}', [AuthenticationController::class, 'disconnectProvider'])
|
||||
->name('app.authentication.disconnect-provider');
|
||||
|
||||
|
|
|
|||
|
|
@ -31,15 +31,11 @@
|
|||
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/github/redirect', [GitHubController::class, 'redirect'])->name('auth.github.redirect');
|
||||
});
|
||||
Route::get('/auth/google/callback', [GoogleController::class, 'callback'])->name('auth.google.callback');
|
||||
|
||||
// Callbacks must be reachable by both guests (signup/login flow) and
|
||||
// authenticated users (connect-from-settings flow). Branching on
|
||||
// `Auth::check()` inside the callback is safe because the redirect that
|
||||
// initiated the round-trip enforces the right middleware.
|
||||
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::get('/auth/github/redirect', [GitHubController::class, 'redirect'])->name('auth.github.redirect');
|
||||
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');
|
||||
|
|
|
|||
|
|
@ -4,13 +4,15 @@
|
|||
|
||||
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();
|
||||
$driver = Mockery::mock(AbstractProvider::class);
|
||||
$driver->shouldReceive('scopes')->andReturnSelf();
|
||||
$driver->shouldReceive('redirectUrl')->andReturnSelf();
|
||||
$driver->shouldReceive('redirect')->andReturn(redirect('https://github.com/login/oauth/authorize'));
|
||||
Socialite::shouldReceive('driver')->with('github')->andReturn($driver);
|
||||
|
||||
|
|
@ -22,7 +24,8 @@
|
|||
test('authenticated user can hit the connect-provider route for google', function () {
|
||||
$user = User::factory()->create();
|
||||
|
||||
$driver = Mockery::mock();
|
||||
$driver = Mockery::mock(AbstractProvider::class);
|
||||
$driver->shouldReceive('redirectUrl')->andReturnSelf();
|
||||
$driver->shouldReceive('redirect')->andReturn(redirect('https://accounts.google.com/o/oauth2/auth'));
|
||||
Socialite::shouldReceive('driver')->with('google-auth')->andReturn($driver);
|
||||
|
||||
|
|
@ -44,7 +47,7 @@
|
|||
->assertRedirect(route('login'));
|
||||
});
|
||||
|
||||
test('authenticated callback connects github to the current user', function () {
|
||||
test('connect-provider callback connects github to the current user', function () {
|
||||
$user = User::factory()->create([
|
||||
'email' => 'me@example.com',
|
||||
'google_id' => 'g-me',
|
||||
|
|
@ -56,21 +59,21 @@
|
|||
$socialiteUser->name = 'Me';
|
||||
$socialiteUser->email = 'me@example.com';
|
||||
|
||||
Socialite::shouldReceive('driver')
|
||||
->with('github')
|
||||
->andReturn($driver = Mockery::mock());
|
||||
|
||||
$driver = Mockery::mock(AbstractProvider::class);
|
||||
$driver->shouldReceive('scopes')->andReturnSelf();
|
||||
$driver->shouldReceive('redirectUrl')->andReturnSelf();
|
||||
$driver->shouldReceive('user')->andReturn($socialiteUser);
|
||||
Socialite::shouldReceive('driver')->with('github')->andReturn($driver);
|
||||
|
||||
$this->actingAs($user)
|
||||
->get(route('auth.github.callback'))
|
||||
->get(route('app.authentication.connect-provider.callback', 'github'))
|
||||
->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 () {
|
||||
test('connect-provider callback links github by current user, not by email', function () {
|
||||
$user = User::factory()->create([
|
||||
'email' => 'work@example.com',
|
||||
'google_id' => 'g-me',
|
||||
|
|
@ -82,14 +85,14 @@
|
|||
$socialiteUser->name = 'Me';
|
||||
$socialiteUser->email = 'personal@example.com';
|
||||
|
||||
Socialite::shouldReceive('driver')
|
||||
->with('github')
|
||||
->andReturn($driver = Mockery::mock());
|
||||
|
||||
$driver = Mockery::mock(AbstractProvider::class);
|
||||
$driver->shouldReceive('scopes')->andReturnSelf();
|
||||
$driver->shouldReceive('redirectUrl')->andReturnSelf();
|
||||
$driver->shouldReceive('user')->andReturn($socialiteUser);
|
||||
Socialite::shouldReceive('driver')->with('github')->andReturn($driver);
|
||||
|
||||
$this->actingAs($user)
|
||||
->get(route('auth.github.callback'))
|
||||
->get(route('app.authentication.connect-provider.callback', 'github'))
|
||||
->assertRedirect(route('app.authentication.edit'))
|
||||
->assertSessionHas('flash.success');
|
||||
|
||||
|
|
@ -98,7 +101,7 @@
|
|||
$this->assertAuthenticatedAs($user);
|
||||
});
|
||||
|
||||
test('authenticated callback rejects when github account is already linked to another user', function () {
|
||||
test('connect-provider 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]);
|
||||
|
|
@ -108,14 +111,14 @@
|
|||
$socialiteUser->name = 'Me';
|
||||
$socialiteUser->email = 'me@example.com';
|
||||
|
||||
Socialite::shouldReceive('driver')
|
||||
->with('github')
|
||||
->andReturn($driver = Mockery::mock());
|
||||
|
||||
$driver = Mockery::mock(AbstractProvider::class);
|
||||
$driver->shouldReceive('scopes')->andReturnSelf();
|
||||
$driver->shouldReceive('redirectUrl')->andReturnSelf();
|
||||
$driver->shouldReceive('user')->andReturn($socialiteUser);
|
||||
Socialite::shouldReceive('driver')->with('github')->andReturn($driver);
|
||||
|
||||
$this->actingAs($me)
|
||||
->get(route('auth.github.callback'))
|
||||
->get(route('app.authentication.connect-provider.callback', 'github'))
|
||||
->assertRedirect(route('app.authentication.edit'))
|
||||
->assertSessionHas('flash.error');
|
||||
|
||||
|
|
@ -123,7 +126,7 @@
|
|||
$this->assertAuthenticatedAs($me);
|
||||
});
|
||||
|
||||
test('authenticated callback connects google to the current user', function () {
|
||||
test('connect-provider callback connects google to the current user', function () {
|
||||
$user = User::factory()->create([
|
||||
'email' => 'me@example.com',
|
||||
'github_id' => 'gh-me',
|
||||
|
|
@ -135,21 +138,20 @@
|
|||
$socialiteUser->name = 'Me';
|
||||
$socialiteUser->email = 'me@example.com';
|
||||
|
||||
Socialite::shouldReceive('driver')
|
||||
->with('google-auth')
|
||||
->andReturn($driver = Mockery::mock());
|
||||
|
||||
$driver = Mockery::mock(AbstractProvider::class);
|
||||
$driver->shouldReceive('redirectUrl')->andReturnSelf();
|
||||
$driver->shouldReceive('user')->andReturn($socialiteUser);
|
||||
Socialite::shouldReceive('driver')->with('google-auth')->andReturn($driver);
|
||||
|
||||
$this->actingAs($user)
|
||||
->get(route('auth.google.callback'))
|
||||
->get(route('app.authentication.connect-provider.callback', 'google'))
|
||||
->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 () {
|
||||
test('connect-provider 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]);
|
||||
|
|
@ -159,17 +161,29 @@
|
|||
$socialiteUser->name = 'Me';
|
||||
$socialiteUser->email = 'me@example.com';
|
||||
|
||||
Socialite::shouldReceive('driver')
|
||||
->with('google-auth')
|
||||
->andReturn($driver = Mockery::mock());
|
||||
|
||||
$driver = Mockery::mock(AbstractProvider::class);
|
||||
$driver->shouldReceive('redirectUrl')->andReturnSelf();
|
||||
$driver->shouldReceive('user')->andReturn($socialiteUser);
|
||||
Socialite::shouldReceive('driver')->with('google-auth')->andReturn($driver);
|
||||
|
||||
$this->actingAs($me)
|
||||
->get(route('auth.google.callback'))
|
||||
->get(route('app.authentication.connect-provider.callback', 'google'))
|
||||
->assertRedirect(route('app.authentication.edit'))
|
||||
->assertSessionHas('flash.error');
|
||||
|
||||
expect($me->fresh()->google_id)->toBeNull();
|
||||
$this->assertAuthenticatedAs($me);
|
||||
});
|
||||
|
||||
test('connect-provider callback rejects unknown provider', function () {
|
||||
$user = User::factory()->create();
|
||||
|
||||
$this->actingAs($user)
|
||||
->get(route('app.authentication.connect-provider.callback', 'twitter'))
|
||||
->assertNotFound();
|
||||
});
|
||||
|
||||
test('connect-provider callback requires authentication', function () {
|
||||
$this->get(route('app.authentication.connect-provider.callback', 'github'))
|
||||
->assertRedirect(route('login'));
|
||||
});
|
||||
|
|
|
|||
Loading…
Reference in a new issue