From ae4559bda18b3a513bcf2f4b47b71594f3ca1de7 Mon Sep 17 00:00:00 2001 From: Joel Butcher Date: Thu, 25 Jul 2024 07:59:53 +0100 Subject: [PATCH] [6x] Fix auth for users with changed emails (#364) * Fix auth for users with changed emails * Fix RedirectIfTwoFactorAuthenticatable::validateCredentials and registration login / error * fix the bug --- src/Actions/AuthenticateOAuthCallback.php | 95 ++++++++++++------- .../RedirectIfTwoFactorAuthenticatable.php | 8 +- src/Concerns/ConfirmsFilament.php | 41 +++++++- src/Http/Responses/OAuthFailedResponse.php | 2 +- 4 files changed, 102 insertions(+), 44 deletions(-) diff --git a/src/Actions/AuthenticateOAuthCallback.php b/src/Actions/AuthenticateOAuthCallback.php index 7770824..8f9eacf 100644 --- a/src/Actions/AuthenticateOAuthCallback.php +++ b/src/Actions/AuthenticateOAuthCallback.php @@ -61,36 +61,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 +106,22 @@ public function authenticate(string $provider, ProviderUser $providerAccount): S ); } - event(new OAuthFailed($provider, $providerAccount)); + // If a user does not exist for the provider account, check if registration is supported. + if ($this->canRegister()) { + // If registration is supported, register the user. + return $this->register($provider, $providerAccount); + } - $this->flashError( - __('An account already exists for that email address. Please login to connect your :provider account.', ['provider' => Providers::name($provider)]), - ); + // Otherwise, return an error. + $error = Route::has('login') && Session::get('socialstream.previous_url') === route('login') + ? __('Account not found, please register to create an account.') + : __('Registration is disabled.'); - return app(OAuthFailedResponse::class); + return $this->oauthFailed( + error: $error, + provider: $provider, + providerAccount: $providerAccount, + ); } /** @@ -126,8 +138,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 +151,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 +216,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 +229,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 +276,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('login')) { + return Features::hasCreateAccountOnFirstLoginFeatures(); } - return Features::hasCreateAccountOnFirstLoginFeatures(); + return Features::hasCreateAccountOnFirstLoginFeatures() && Features::hasGlobalLoginFeatures(); } } diff --git a/src/Actions/RedirectIfTwoFactorAuthenticatable.php b/src/Actions/RedirectIfTwoFactorAuthenticatable.php index 24d285a..2896763 100644 --- a/src/Actions/RedirectIfTwoFactorAuthenticatable.php +++ b/src/Actions/RedirectIfTwoFactorAuthenticatable.php @@ -26,12 +26,14 @@ protected function validateCredentials($request) $socialUser = app(ResolvesSocialiteUsers::class) ->resolve($request->route('provider')); - return tap(Socialstream::$userModel::where('email', $socialUser->getEmail())->first(), function ($user) use ($request, $socialUser) { - if (! $user || ! Socialstream::$connectedAccountModel::where('email', $socialUser->getEmail())->first()) { - $this->fireFailedEvent($request, $user); + $connectedAccount = tap(Socialstream::$connectedAccountModel::where('email', $socialUser->getEmail())->first(), function ($connectedAccount) use ($request, $socialUser) { + if (! $connectedAccount) { + $this->fireFailedEvent($request, $connectedAccount->user); $this->throwFailedAuthenticationException($request); } }); + + return $connectedAccount->user; } } 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'); } } diff --git a/src/Http/Responses/OAuthFailedResponse.php b/src/Http/Responses/OAuthFailedResponse.php index 75614d3..48d3b3c 100644 --- a/src/Http/Responses/OAuthFailedResponse.php +++ b/src/Http/Responses/OAuthFailedResponse.php @@ -6,7 +6,7 @@ use Illuminate\Support\Facades\Route; use Illuminate\Support\Facades\Session; use JoelButcher\Socialstream\Concerns\InteractsWithComposer; -use JoelButcher\Socialstream\Contracts\OAuthLoginFailedResponse as OAuthFailedResponseContract; +use JoelButcher\Socialstream\Contracts\OAuthFailedResponse as OAuthFailedResponseContract; use JoelButcher\Socialstream\Socialstream; class OAuthFailedResponse implements OAuthFailedResponseContract