From dad327a329e77c327e7d3a0b64e7ce496913c57f Mon Sep 17 00:00:00 2001 From: Joel Butcher Date: Wed, 24 Jul 2024 18:03:37 +0100 Subject: [PATCH] Fix auth for users with changed emails --- src/Actions/AuthenticateOAuthCallback.php | 100 +++++++++++++--------- src/Concerns/ConfirmsFilament.php | 41 ++++++++- 2 files changed, 97 insertions(+), 44 deletions(-) diff --git a/src/Actions/AuthenticateOAuthCallback.php b/src/Actions/AuthenticateOAuthCallback.php index 7770824..b456586 100644 --- a/src/Actions/AuthenticateOAuthCallback.php +++ b/src/Actions/AuthenticateOAuthCallback.php @@ -48,11 +48,12 @@ class AuthenticateOAuthCallback implements AuthenticatesOAuthCallback * Create a new controller instance. */ public function __construct( - protected StatefulGuard $guard, - protected CreatesUserFromProvider $createsUser, + protected StatefulGuard $guard, + protected CreatesUserFromProvider $createsUser, protected CreatesConnectedAccounts $createsConnectedAccounts, protected UpdatesConnectedAccounts $updatesConnectedAccounts - ) { + ) + { // } @@ -61,36 +62,39 @@ public function __construct( */ public function authenticate(string $provider, ProviderUser $providerAccount): SocialstreamResponse|RedirectResponse { + // If the user is authenticated, link the provider to the authenticated user. if ($user = auth()->user()) { return $this->link($user, $provider, $providerAccount); } + // Check if the user has an existing OAuth account. $account = Socialstream::findConnectedAccountForProviderAndId($provider, $providerAccount->getId()); - $user = Socialstream::newUserModel()->where('email', $providerAccount->getEmail())->first(); - if ($account && $user) { + // If the user has an existing OAuth account, log the user in. + if ($account) { return $this->login( - user: $user, + user: $account->user, account: $account, provider: $provider, providerAccount: $providerAccount ); } - if ($this->canRegister($user, $account)) { - return $this->register($provider, $providerAccount); - } + // Otherwise, check if a user exists with the same email address. + $user = Socialstream::newUserModel()->where('email', $providerAccount->getEmail())->first(); - if (! $user && $account && $account->user) { - return $this->login( - user: $account->user, - account: $account, - provider: $provider, - providerAccount: $providerAccount - ); - } + // If a user exists, check the features to make sure we can link unlinked existing users. + if ($user) { + if (!Features::authenticatesExistingUnlinkedUsers()) { + // If we cannot link, return an error asking the user to log in to link their account. + return $this->oauthFailed( + error: __('An account already exists with the same email address. Please log in to connect your :provider account.', ['provider' => Providers::name($provider)]), + provider: $provider, + providerAccount: $providerAccount, + ); + } - if ($user && Features::authenticatesExistingUnlinkedUsers()) { + // Otherwise, log the user in. return $this->login( user: $user, account: $this->createsConnectedAccounts->create( @@ -103,13 +107,18 @@ public function authenticate(string $provider, ProviderUser $providerAccount): S ); } - event(new OAuthFailed($provider, $providerAccount)); - - $this->flashError( - __('An account already exists for that email address. Please login to connect your :provider account.', ['provider' => Providers::name($provider)]), - ); + // If a user does not exist for the provider account, check if registration is supported. + if (! $this->canRegister()) { + // If registration is not supported, return an error. + return $this->oauthFailed( + error: __('Registration is disabled.'), + provider: $provider, + providerAccount: $providerAccount, + ); + } - return app(OAuthFailedResponse::class); + // Otherwise, register the user. + return $this->register($provider, $providerAccount); } /** @@ -126,8 +135,8 @@ function ($request, $next) use ($user) { return $next($request); }, - ]))->then(fn () => app(OAuthRegisterResponse::class)), - fn () => event(new NewOAuthRegistration($user, $provider, $providerAccount)) + ]))->then(fn() => app(OAuthRegisterResponse::class)), + fn() => event(new NewOAuthRegistration($user, $provider, $providerAccount)) ); } @@ -139,14 +148,14 @@ protected function login(Authenticatable $user, mixed $account, string $provider $this->updatesConnectedAccounts->update($user, $account, $provider, $providerAccount); return tap( - $this->loginPipeline(request(), $user)->then(fn () => app(OAuthLoginResponse::class)), - fn () => event(new OAuthLogin($user, $provider, $account, $providerAccount)), + $this->loginPipeline(request(), $user)->then(fn() => app(OAuthLoginResponse::class)), + fn() => event(new OAuthLogin($user, $provider, $account, $providerAccount)), ); } protected function loginPipeline(Request $request, Authenticatable $user): Pipeline { - if (! class_exists(Fortify::class)) { + if (!class_exists(Fortify::class)) { return (new Pipeline(app()))->send($request)->through(array_filter([ function ($request, $next) use ($user) { $this->guard->login($user, Socialstream::hasRememberSessionFeatures()); @@ -204,8 +213,8 @@ private function link(Authenticatable $user, string $provider, ProviderUser $pro return app(OAuthProviderLinkFailedResponse::class); } - if (! $account) { - $this->createsConnectedAccounts->create(auth()->user(), $provider, $providerAccount); + if (!$account) { + $this->createsConnectedAccounts->create($user, $provider, $providerAccount); } event(new OAuthProviderLinked($user, $provider, $account, $providerAccount)); @@ -217,6 +226,15 @@ private function link(Authenticatable $user, string $provider, ProviderUser $pro return app(OAuthProviderLinkedResponse::class); } + private function oauthFailed(string $error, string $provider, ProviderUser $providerAccount): OAuthFailedResponse + { + event(new OAuthFailed($provider, $providerAccount)); + + $this->flashError($error); + + return app(OAuthFailedResponse::class); + } + /** * Flash a status message to the session. */ @@ -255,24 +273,26 @@ private function flashError(string $error): void /** * Determine if we can register a new user. */ - private function canRegister(mixed $user, mixed $account): bool + private function canRegister(): bool { - if (! is_null($user) || ! is_null($account)) { - return false; + if ($this->usesFilament() && $this->canRegisterUsingFilament()) { + return true; } - if ($this->usesFilament()) { - return $this->hasFilamentAuthRoutes(); + if (class_exists(Fortify::class) && !FortifyFeatures::enabled(FortifyFeatures::registration())) { + return false; } - if (Route::has('register') && Session::get('socialstream.previous_url') === route('register')) { + $previousRoute = Session::get('socialstream.previous_url'); + + if (Route::has('register') && $previousRoute === route('register')) { return true; } - if (Route::has('login') && Session::get('socialstream.previous_url') !== route('login')) { - return Features::hasGlobalLoginFeatures(); + if (Route::has('login') && $previousRoute === route('register')) { + return Features::hasCreateAccountOnFirstLoginFeatures(); } - return Features::hasCreateAccountOnFirstLoginFeatures(); + return Features::hasCreateAccountOnFirstLoginFeatures() && Features::hasGlobalLoginFeatures(); } } diff --git a/src/Concerns/ConfirmsFilament.php b/src/Concerns/ConfirmsFilament.php index 5965fa4..1b5e045 100644 --- a/src/Concerns/ConfirmsFilament.php +++ b/src/Concerns/ConfirmsFilament.php @@ -6,6 +6,7 @@ use Illuminate\Support\Facades\Route; use Illuminate\Support\Facades\Session; +use JoelButcher\Socialstream\Features; trait ConfirmsFilament { @@ -18,9 +19,41 @@ public function usesFilament(): bool public function hasFilamentAuthRoutes(): bool { - return (Route::has('filament.auth.login') && Session::get('socialstream.previous_url') === route('filament.auth.login')) || - (Route::has('filament.admin.auth.login') && Session::get('socialstream.previous_url') === route('filament.admin.auth.login')) || - (Route::has('filament.auth.register') && Session::get('socialstream.previous_url') === route('filament.auth.register')) || - (Route::has('filament.admin.auth.register') && Session::get('socialstream.previous_url') === route('filament.admin.auth.register')); + return $this->hasFilamentLoginRoutes() || $this->hasFilamentRegistrationRoutes(); + } + + public function canRegisterUsingFilament(): bool + { + $filamentRegistrationEnabled = $this->hasFilamentRegistrationRoutes() || + $this->hasFilamentLoginRoutes() && Features::hasCreateAccountOnFirstLoginFeatures(); + + if (! $filamentRegistrationEnabled) { + return false; + } + + return $this->cameFromFilamentAuthRoute(); + } + + /** Assumes static::canRegisterUsingFilament() returns TRUE. */ + public function cameFromFilamentAuthRoute(): bool + { + $previousRoute = Session::get('socialstream.previous_url'); + + return in_array($previousRoute, [ + route('filament.auth.login'), + route('filament.admin.auth.login'), + route('filament.auth.register'), + route('filament.admin.auth.register'), + ]); + } + + public function hasFilamentLoginRoutes(): bool + { + return Route::has('filament.auth.login') || Route::has('filament.admin.auth.login'); + } + + public function hasFilamentRegistrationRoutes(): bool + { + return Route::has('filament.auth.register') || Route::has('filament.admin.auth.register'); } }