revert: keep one OAuth callback URL per provider

Drops the dedicated /settings/authentication/providers/{provider}/callback
route added in the previous refactor — registering a second callback URL
in each OAuth app is more ops cost than the trade is worth.

Back to one callback URL per provider, with a small `Auth::check()`
branch in the auth controllers' callbacks. The check is safe because
the redirects that initiate the round-trip enforce the right
middleware (signup/login is `guest`-only, connect is `auth`-only),
so the auth state at callback time matches the flow's intent.
This commit is contained in:
Paulo Castellano 2026-05-04 19:42:13 -03:00
parent 6b7b12f191
commit e3acd1bb4b
7 changed files with 73 additions and 94 deletions

View file

@ -123,10 +123,7 @@ 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.
# 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)
# Used for YouTube social account connection AND Google login/signup
GOOGLE_AUTH_ENABLED=false
GOOGLE_CLIENT_ID=
GOOGLE_CLIENT_SECRET=
@ -134,10 +131,7 @@ 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.
# 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)
# Used for GitHub login/signup
GITHUB_AUTH_ENABLED=false
GITHUB_CLIENT_ID=
GITHUB_CLIENT_SECRET=

View file

@ -15,7 +15,6 @@
use Inertia\Inertia;
use Inertia\Response;
use Laravel\Socialite\Facades\Socialite;
use Laravel\Socialite\Two\AbstractProvider;
class AuthenticationController extends Controller
{
@ -69,54 +68,9 @@ 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')->redirectUrl($callback),
'github' => Socialite::driver('github')->scopes(['read:user', 'user:email'])->redirectUrl($callback),
'google' => Socialite::driver('google-auth')->redirect(),
'github' => Socialite::driver('github')->scopes(['read:user', 'user:email'])->redirect(),
};
}

View file

@ -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();
@ -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) {

View file

@ -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();
@ -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) {

View file

@ -251,8 +251,6 @@
->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');

View file

@ -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');

View file

@ -12,7 +12,6 @@
$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);
@ -25,7 +24,6 @@
$user = User::factory()->create();
$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);
@ -47,7 +45,7 @@
->assertRedirect(route('login'));
});
test('connect-provider callback connects github to the current user', function () {
test('authenticated callback connects github to the current user', function () {
$user = User::factory()->create([
'email' => 'me@example.com',
'google_id' => 'g-me',
@ -60,20 +58,18 @@
$socialiteUser->email = 'me@example.com';
$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('app.authentication.connect-provider.callback', 'github'))
->get(route('auth.github.callback'))
->assertRedirect(route('app.authentication.edit'))
->assertSessionHas('flash.success');
expect($user->fresh()->github_id)->toBe('gh-me');
});
test('connect-provider callback links github by current user, not by email', function () {
test('authenticated callback links github by current user, not by email', function () {
$user = User::factory()->create([
'email' => 'work@example.com',
'google_id' => 'g-me',
@ -86,13 +82,11 @@
$socialiteUser->email = 'personal@example.com';
$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('app.authentication.connect-provider.callback', 'github'))
->get(route('auth.github.callback'))
->assertRedirect(route('app.authentication.edit'))
->assertSessionHas('flash.success');
@ -101,7 +95,7 @@
$this->assertAuthenticatedAs($user);
});
test('connect-provider callback rejects when github account is already linked to another user', function () {
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]);
@ -112,13 +106,11 @@
$socialiteUser->email = 'me@example.com';
$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('app.authentication.connect-provider.callback', 'github'))
->get(route('auth.github.callback'))
->assertRedirect(route('app.authentication.edit'))
->assertSessionHas('flash.error');
@ -126,7 +118,7 @@
$this->assertAuthenticatedAs($me);
});
test('connect-provider callback connects google to the current user', function () {
test('authenticated callback connects google to the current user', function () {
$user = User::factory()->create([
'email' => 'me@example.com',
'github_id' => 'gh-me',
@ -139,19 +131,18 @@
$socialiteUser->email = 'me@example.com';
$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('app.authentication.connect-provider.callback', 'google'))
->get(route('auth.google.callback'))
->assertRedirect(route('app.authentication.edit'))
->assertSessionHas('flash.success');
expect($user->fresh()->google_id)->toBe('g-me');
});
test('connect-provider callback rejects when google account is already linked to another user', function () {
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]);
@ -162,28 +153,14 @@
$socialiteUser->email = 'me@example.com';
$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('app.authentication.connect-provider.callback', 'google'))
->get(route('auth.google.callback'))
->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'));
});