diff --git a/app/Http/Controllers/Auth/UserController.php b/app/Http/Controllers/Auth/UserController.php index 825c85ed..f3c2b4c2 100644 --- a/app/Http/Controllers/Auth/UserController.php +++ b/app/Http/Controllers/Auth/UserController.php @@ -24,6 +24,8 @@ public function update(UserUpdateRequest $request) $user = $request->user(); $validated = $request->validated(); + $this->authorize('update', $user); + if (config('auth.defaults.guard') === 'reverse-proxy-guard' || $user->oauth_provider) { Log::notice('Account update rejected: reverse-proxy-guard enabled or account from external sso provider'); @@ -57,37 +59,16 @@ public function delete(UserDeleteRequest $request) $validated = $request->validated(); $user = Auth::user(); - Log::info(sprintf('Deletion of user ID #%s requested', $user->id)); - - if ($user->isAdministrator() && User::admins()->count() == 1) { - return response()->json(['message' => __('errors.cannot_delete_the_only_admin')], 400); - } - if (! Hash::check($validated['password'], Auth::user()->password)) { return response()->json(['message' => __('errors.wrong_current_password')], 400); } - try { - DB::transaction(function () use ($user) { - DB::table('twofaccounts')->where('user_id', $user->id)->delete(); - DB::table('groups')->where('user_id', $user->id)->delete(); - DB::table('webauthn_credentials')->where('authenticatable_id', $user->id)->delete(); - DB::table(config('auth.passwords.webauthn.table'))->where('email', $user->email)->delete(); - DB::table('oauth_access_tokens')->where('user_id', $user->id)->delete(); - DB::table(config('auth.passwords.users.table'))->where('email', $user->email)->delete(); - DB::table('users')->where('id', $user->id)->delete(); - }); - } - // @codeCoverageIgnoreStart - catch (\Throwable $e) { - Log::error(sprintf('Deletion of user ID #%s failed, transaction has been rolled-back', $user->id)); - - return response()->json(['message' => __('errors.user_deletion_failed')], 400); - } - // @codeCoverageIgnoreEnd - - Log::info(sprintf('User ID #%s deleted', $user->id)); - - return response()->json(null, 204); + // This will delete the user and all its 2FAs & Groups thanks to the onCascadeDelete constrains. + // Deletion will not be done (and returns False) if the user is the only existing admin (see UserObserver clas) + return $user->delete() === false + ? response()->json([ + 'message' => __('errors.cannot_delete_the_only_admin'), + ], 400) + : response()->json(null, 204); } } diff --git a/app/Observers/UserObserver.php b/app/Observers/UserObserver.php new file mode 100644 index 00000000..f0dda42d --- /dev/null +++ b/app/Observers/UserObserver.php @@ -0,0 +1,97 @@ +id, Auth::user()->id ?? 'unknown')); + + // Prevent deletion of the only administrator + $isLastAdmin = $user->isAdministrator() && User::admins()->count() == 1; + + if ($isLastAdmin) { + Log::notice(sprintf('Deletion of user ID #%s refused, cannot delete the only administrator', $user->id)); + + return false; + } + + // Deleting user's twofaccounts icon + $iconPathes = $user->twofaccounts->filter(function ($twofaccount, $key) { + return $twofaccount->icon; + })->map(function ($twofaccount, $key) { + return $twofaccount->icon; + }); + Storage::disk('icons')->delete($iconPathes->toArray()); + + return true; + } + + /** + * Handle the User "deleted" event. + */ + public function deleted(User $user): void + { + // DB has cascade delete enabled to flush 2FA and Groups but, + // for an unknown reason, SQLite refuses to delete these related. + // (despite DB_FOREIGN_KEYS=true which is supposed to enable it) + // So it ends with a direct db delete for SQLite... + if (DB::getDriverName() === 'sqlite') { + DB::table('twofaccounts')->where('user_id', $user->id)->delete(); + DB::table('groups')->where('user_id', $user->id)->delete(); + } + + // We flush webauthn credentials & tokens + Password::broker('webauthn')->deleteToken($user); + $user->flushCredentials(); + + // And Passport tokens + Password::broker()->deleteToken($user); + DB::table('oauth_access_tokens')->where('user_id', $user->id)->delete(); + + Log::info(sprintf('User ID #%s and all user traces deleted', $user->id)); + } + + /** + * Handle the User "restored" event. + */ + public function restored(User $user): void + { + // + } + + /** + * Handle the User "force deleted" event. + */ + public function forceDeleted(User $user): void + { + // + } +} diff --git a/app/Providers/EventServiceProvider.php b/app/Providers/EventServiceProvider.php index 8eb1cfad..21684a0f 100644 --- a/app/Providers/EventServiceProvider.php +++ b/app/Providers/EventServiceProvider.php @@ -11,6 +11,8 @@ use App\Listeners\RegisterOpenId; use App\Listeners\ReleaseRadar; use App\Listeners\ResetUsersPreference; +use App\Models\User; +use App\Observers\UserObserver; use Illuminate\Auth\Events\Registered; use Illuminate\Auth\Listeners\SendEmailVerificationNotification; use Illuminate\Foundation\Support\Providers\EventServiceProvider as ServiceProvider; @@ -43,6 +45,15 @@ class EventServiceProvider extends ServiceProvider RegisterOpenId::class, ], ]; + + /** + * The model observers for your application. + * + * @var array> + */ + protected $observers = [ + User::class => [UserObserver::class], + ]; /** * Register any events for your application. diff --git a/tests/Feature/Http/Auth/UserControllerTest.php b/tests/Feature/Http/Auth/UserControllerTest.php index 37ca78f3..1a42dadb 100644 --- a/tests/Feature/Http/Auth/UserControllerTest.php +++ b/tests/Feature/Http/Auth/UserControllerTest.php @@ -8,6 +8,8 @@ use App\Models\Group; use App\Models\TwoFAccount; use App\Models\User; +use App\Observers\UserObserver; +use App\Policies\UserPolicy; use Illuminate\Support\Facades\Config; use PHPUnit\Framework\Attributes\CoversClass; use Tests\FeatureTestCase; @@ -16,6 +18,8 @@ * UserControllerTest test class */ #[CoversClass(UserController::class)] +#[CoversClass(UserObserver::class)] +#[CoversClass(UserPolicy::class)] #[CoversClass(RejectIfDemoMode::class)] #[CoversClass(UserUpdateRequest::class)] class UserControllerTest extends FeatureTestCase