From 040a37ee2f249b8863229690a2090cd06532bf24 Mon Sep 17 00:00:00 2001 From: Nicolas Perrenoud Date: Wed, 21 Sep 2022 13:27:38 +0300 Subject: [PATCH] Refactor social login, ensure avatars, names are updated, use local avatar storage --- Changelog.md | 1 + app/Http/Controllers/Auth/LoginController.php | 70 ++++++++++++++----- .../API/UserProfileController.php | 1 + app/Http/Resources/User.php | 1 - app/Models/User.php | 15 +++- resources/js/pages/users/UserIndexPage.vue | 2 +- resources/js/pages/users/UserProfilePage.vue | 4 +- resources/js/pages/users/UserShowPage.vue | 2 +- 8 files changed, 71 insertions(+), 25 deletions(-) diff --git a/Changelog.md b/Changelog.md index 3e30c1098..3a6789b11 100644 --- a/Changelog.md +++ b/Changelog.md @@ -3,6 +3,7 @@ ## NEXT * Added support for specifying email domain for Google Oauth login +* Store avatar images locally ## 3.9.0 diff --git a/app/Http/Controllers/Auth/LoginController.php b/app/Http/Controllers/Auth/LoginController.php index 86d4f9c37..bfe5e97d1 100644 --- a/app/Http/Controllers/Auth/LoginController.php +++ b/app/Http/Controllers/Auth/LoginController.php @@ -11,7 +11,9 @@ use Illuminate\Http\Request; use Illuminate\Support\Facades\Auth; use Illuminate\Support\Facades\Log; +use Illuminate\Support\Facades\Storage; use Illuminate\Support\Facades\Validator; +use Illuminate\Support\Str; use Socialite; class LoginController extends Controller @@ -174,7 +176,7 @@ public function redirectToProvider(string $driver) public function handleProviderCallback(string $driver) { try { - $user = Socialite::driver($driver)->user(); + $socialUser = Socialite::driver($driver)->user(); } catch (Exception $e) { Log::error('Socialite login with driver '.$driver.' failed. '.$e->getMessage()); @@ -187,7 +189,7 @@ public function handleProviderCallback(string $driver) if ($driver == 'google') { $orgDomain = config('services.google.organization_domain'); - if (filled($orgDomain) && ! str_ends_with($user->getEmail(), "@$orgDomain")) { + if (filled($orgDomain) && ! str_ends_with($socialUser->getEmail(), "@$orgDomain")) { return redirect() ->route('login') ->with('error', __('Only email addresses of the organization :domain are accepted.', [ @@ -196,21 +198,32 @@ public function handleProviderCallback(string $driver) } } - // check if they're an existing user - $existingUser = User::where('email', $user->getEmail())->first(); - if ($existingUser) { - auth()->login($existingUser, true); - } else { - $newUser = new User(); - $newUser->provider_name = $driver; - $newUser->provider_id = $user->getId(); - $newUser->name = $user->getName(); - $newUser->email = $user->getEmail(); - $newUser->email_verified_at = now(); - $newUser->avatar = $user->getAvatar(); - $newUser->locale = \App::getLocale(); - $newUser->save(); + /** @var User $user */ + $user = User::firstOrCreate( + ['email' => $socialUser->getEmail()], + [ + 'name' => $socialUser->getName(), + 'password' => Str::random(32), + 'provider_name' => $driver, + 'provider_id' => $socialUser->getId(), + 'locale' => \App::getLocale(), + ] + ); + // Update name, avatar in case they have changed on the provider side + $user->name = $socialUser->getName(); + $user->avatar = $this->getAvatar($socialUser->getAvatar(), $user->avatar); + + // Mark email as verified + if ($user->email_verified_at == null) { + $user->email_verified_at = now(); + } + + if ($user->isDirty()) { + $user->save(); + } + + if ($user->wasRecentlyCreated) { Log::info('New user registered with Oauth provider.', [ 'user_id' => $user->id, 'user_name' => $user->name, @@ -219,10 +232,12 @@ public function handleProviderCallback(string $driver) 'client_ip' => request()->ip(), ]); - event(new UserSelfRegistered($newUser)); + event(new UserSelfRegistered($user)); + } - auth()->login($newUser, true); + Auth::login($user); + if ($user->wasRecentlyCreated) { session()->flash('login_message', __('Hello :name. Thanks for registering with :app_name. Your account has been created, and the administrator has been informed, in order to grand you the appropriate permissions.', [ 'name' => Auth::user()->name, 'app_name' => config('app.name'), @@ -233,4 +248,23 @@ public function handleProviderCallback(string $driver) return redirect($this->redirectPath()); } + + private function getAvatar(?string $newAvatar, ?string $currentAvatar): ?string + { + if ($newAvatar === null) { + return null; + } + + if (! ini_get('allow_url_fopen')) { + return $newAvatar; + } + + $avatar = 'public/avatars/'.basename($newAvatar); + if ($currentAvatar !== null && $avatar != $currentAvatar && Storage::exists($currentAvatar)) { + Storage::delete($currentAvatar); + } + Storage::put($avatar, file_get_contents($newAvatar)); + + return $avatar; + } } diff --git a/app/Http/Controllers/UserManagement/API/UserProfileController.php b/app/Http/Controllers/UserManagement/API/UserProfileController.php index c3be61096..8910f1bd7 100644 --- a/app/Http/Controllers/UserManagement/API/UserProfileController.php +++ b/app/Http/Controllers/UserManagement/API/UserProfileController.php @@ -21,6 +21,7 @@ public function index() return response()->json([ 'user' => $user, + 'avatar_url' => $user->avatarUrl(), 'languages' => language()->allowed(), 'locale' => App::getLocale(), 'can_delete' => ! ($user->is_super_admin && User::where('is_super_admin', true)->count() == 1), diff --git a/app/Http/Resources/User.php b/app/Http/Resources/User.php index ca5564410..076625d9a 100644 --- a/app/Http/Resources/User.php +++ b/app/Http/Resources/User.php @@ -23,7 +23,6 @@ public function toArray($request) 'locale' => $this->locale, 'is_super_admin' => $this->is_super_admin, 'is_2fa_enabled' => $this->tfa_secret !== null, - 'avatar' => $this->avatar, 'avatar_url' => $this->avatarUrl(), 'provider_name' => $this->provider_name, 'created_at' => $this->created_at, diff --git a/app/Models/User.php b/app/Models/User.php index 3d71e4270..51e6b3d09 100644 --- a/app/Models/User.php +++ b/app/Models/User.php @@ -8,6 +8,7 @@ use Illuminate\Foundation\Auth\User as Authenticatable; use Illuminate\Notifications\Notifiable; use Illuminate\Support\Collection; +use Storage; class User extends Authenticatable implements HasLocalePreference { @@ -30,6 +31,8 @@ class User extends Authenticatable implements HasLocalePreference 'password', 'is_super_admin', 'locale', + 'provider_name', + 'provider_id', ]; /** @@ -124,9 +127,15 @@ public function permissions(): Collection public function avatarUrl(?int $size = null): string { - return $this->avatar !== null - ? $this->avatar - : route('users.avatar', [$this, 'size' => $size]); + if (blank($this->avatar)) { + return route('users.avatar', [$this, 'size' => $size]); + } + + if (filter_var($this->avatar, FILTER_VALIDATE_URL)) { + return $this->avatar; + } + + return Storage::url($this->avatar); } /** diff --git a/resources/js/pages/users/UserIndexPage.vue b/resources/js/pages/users/UserIndexPage.vue index 40603a002..183f8ac70 100644 --- a/resources/js/pages/users/UserIndexPage.vue +++ b/resources/js/pages/users/UserIndexPage.vue @@ -10,7 +10,7 @@ diff --git a/resources/js/pages/users/UserProfilePage.vue b/resources/js/pages/users/UserProfilePage.vue index 2224e2c02..255963d69 100644 --- a/resources/js/pages/users/UserProfilePage.vue +++ b/resources/js/pages/users/UserProfilePage.vue @@ -5,7 +5,7 @@ @@ -107,6 +107,7 @@ export default { isBusy: false, errorText: null, canDelete: false, + avatar_url: null, } }, computed: { @@ -123,6 +124,7 @@ export default { try { let data = await userprofileApi.list() this.user = data.user + this.avatar_url = data.avatar_url this.languages = data.languages this.canDelete = data.can_delete } catch (err) { diff --git a/resources/js/pages/users/UserShowPage.vue b/resources/js/pages/users/UserShowPage.vue index d89a1f942..8f86ef148 100644 --- a/resources/js/pages/users/UserShowPage.vue +++ b/resources/js/pages/users/UserShowPage.vue @@ -10,7 +10,7 @@