Delete the user in a GDPR compatible way (#1151)

* Delete the user in a GDPR compatible way

* Block user from calls

* Style fix

app/Exceptions/UserNotFound.php

@@ -0,0 +1,38 @@
+<?php
+
+namespace App\Exceptions;
+
+class UserNotFound extends AbstractHttpException
+{
+    public function __construct()
+    {
+        parent::__construct(
+            404,
+            'User not found'
+        );
+    }
+
+    /**
+     * Return the RFC 7807 error type (without the URL root)
+     */
+    public function getErrorType(): string
+    {
+        return 'user-not-found';
+    }
+
+    /**
+     * Get the detailed error string
+     */
+    public function getErrorDetails(): string
+    {
+        return $this->getMessage();
+    }
+
+    /**
+     * Return an array with the error details, merged with the RFC7807 response
+     */
+    public function getErrorMetadata(): array
+    {
+        return [];
+    }
+}

app/Http/Controllers/Admin/UserController.php

@@ -252,15 +252,12 @@ class UserController extends Controller
     public function destroy($id)
     {
         $user = $this->userRepo->findWithoutFail($id);
-
         if (empty($user)) {
             Flash::error('User not found');
-
             return redirect(route('admin.users.index'));
         }
 
-        $this->userRepo->delete($id);
+        $this->userSvc->removeUser($user);
-
         Flash::success('User deleted successfully.');
 
         return redirect(route('admin.users.index'));

app/Http/Controllers/Api/UserController.php

@@ -3,6 +3,7 @@
 namespace App\Http\Controllers\Api;
 
 use App\Contracts\Controller;
+use App\Exceptions\UserNotFound;
 use App\Http\Resources\Bid as BidResource;
 use App\Http\Resources\Pirep as PirepResource;
 use App\Http\Resources\Subfleet as SubfleetResource;
@@ -91,6 +92,10 @@ class UserController extends Controller
     public function get($id)
     {
         $user = $this->userSvc->getUser($id);
+        if ($user === null) {
+            throw new UserNotFound();
+        }
+
         return new UserResource($user);
     }
 
@@ -108,6 +113,9 @@ class UserController extends Controller
     {
         $user_id = $this->getUserId($request);
         $user = $this->userSvc->getUser($user_id);
+        if ($user === null) {
+            throw new UserNotFound();
+        }
 
         // Add a bid
         if ($request->isMethod('PUT') || $request->isMethod('POST')) {
@@ -146,6 +154,10 @@ class UserController extends Controller
     public function fleet(Request $request)
     {
         $user = $this->userRepo->find($this->getUserId($request));
+        if ($user === null) {
+            throw new UserNotFound();
+        }
+
         $subfleets = $this->userSvc->getAllowableSubfleets($user);
 
         return SubfleetResource::collection($subfleets);

app/Models/Aircraft.php

@@ -24,6 +24,7 @@ use Carbon\Carbon;
  * @property int      status
  * @property int      state
  * @property Carbon   landing_time
+ * @property float    fuel_onboard
  */
 class Aircraft extends Model
 {

app/Models/Enums/UserState.php

@@ -11,6 +11,7 @@ class UserState extends Enum
     public const REJECTED = 2;
     public const ON_LEAVE = 3;
     public const SUSPENDED = 4;
+    public const DELETED = 5;
 
     protected static $labels = [
         self::PENDING   => 'user.state.pending',
@@ -18,5 +19,6 @@ class UserState extends Enum
         self::REJECTED  => 'user.state.rejected',
         self::ON_LEAVE  => 'user.state.on_leave',
         self::SUSPENDED => 'user.state.suspended',
+        self::DELETED   => 'user.state.deleted',
     ];
 }

app/Models/User.php

@@ -38,6 +38,8 @@ use Laratrust\Traits\LaratrustUserTrait;
  * @property string           last_pirep_id
  * @property Pirep            last_pirep
  * @property UserFieldValue[] fields
+ * @property Role[]           roles
+ * @property Subfleet[]       subfleets
  *
  * @mixin \Illuminate\Database\Eloquent\Builder
  * @mixin \Illuminate\Notifications\Notifiable

app/Notifications/EventHandler.php

@@ -46,7 +46,7 @@ class EventHandler extends Listener
      *
      * @param \App\Contracts\Notification $notification
      */
-    protected function notifyAdmins($notification)
+    protected function notifyAdmins(\App\Contracts\Notification $notification)
     {
         $admin_users = User::whereRoleIs('admin')->get();
 
@@ -67,8 +67,12 @@ class EventHandler extends Listener
      * @param User                        $user
      * @param \App\Contracts\Notification $notification
      */
-    protected function notifyUser($user, $notification)
+    protected function notifyUser(User $user, \App\Contracts\Notification $notification)
     {
+        if ($user->state === UserState::DELETED) {
+            return;
+        }
+
         try {
             $user->notify($notification);
         } catch (Exception $e) {
@@ -90,7 +94,7 @@ class EventHandler extends Listener
         }
 
         /** @var Collection $users */
-        $users = User::where($where)->get();
+        $users = User::where($where)->where('state', '<>', UserState::DELETED)->get();
         if (empty($users) || $users->count() === 0) {
             return;
         }

app/Services/PirepService.php

@@ -147,6 +147,7 @@ class PirepService extends Service
         $dupe_pirep = $this->findDuplicate($pirep);
         if ($dupe_pirep !== false) {
             $pirep = $dupe_pirep;
+            Log::info('Found duplicate PIREP, id='.$dupe_pirep->id);
             if ($pirep->cancelled) {
                 throw new \App\Exceptions\PirepCancelled($pirep);
             }
@@ -293,9 +294,11 @@ class PirepService extends Service
         $time_limit = Carbon::now('UTC')->subMinutes($minutes)->toDateTimeString();
 
         $where = [
-            'user_id'       => $pirep->user_id,
+            'user_id'        => $pirep->user_id,
-            'airline_id'    => $pirep->airline_id,
+            'airline_id'     => $pirep->airline_id,
-            'flight_number' => $pirep->flight_number,
+            'flight_number'  => $pirep->flight_number,
+            'dpt_airport_id' => $pirep->dpt_airport_id,
+            'arr_airport_id' => $pirep->arr_airport_id,
         ];
 
         if (filled($pirep->route_code)) {

app/Services/UserService.php

@@ -9,12 +9,14 @@ use App\Events\UserStatsChanged;
 use App\Exceptions\PilotIdNotFound;
 use App\Exceptions\UserPilotIdExists;
 use App\Models\Airline;
+use App\Models\Bid;
 use App\Models\Enums\PirepState;
 use App\Models\Enums\UserState;
 use App\Models\Pirep;
 use App\Models\Rank;
 use App\Models\Role;
 use App\Models\User;
+use App\Models\UserFieldValue;
 use App\Repositories\AircraftRepository;
 use App\Repositories\AirlineRepository;
 use App\Repositories\SubfleetRepository;
@@ -23,6 +25,7 @@ use App\Support\Units\Time;
 use App\Support\Utils;
 use Carbon\Carbon;
 use Illuminate\Support\Collection;
+use Illuminate\Support\Facades\Hash;
 use Illuminate\Support\Facades\Log;
 use function is_array;
 
@@ -60,14 +63,19 @@ class UserService extends Service
      *
      * @param $user_id
      *
-     * @return User
+     * @return User|null
      */
-    public function getUser($user_id): User
+    public function getUser($user_id): ?User
     {
+        /** @var User $user */
         $user = $this->userRepo
             ->with(['airline', 'bids', 'rank'])
             ->find($user_id);
 
+        if ($user->state === UserState::DELETED) {
+            return null;
+        }
+
         // Load the proper subfleets to the rank
         $user->rank->subfleets = $this->getAllowableSubfleets($user);
         $user->subfleets = $user->rank->subfleets;
@@ -117,6 +125,33 @@ class UserService extends Service
         return $user;
     }
 
+    /**
+     * Remove the user. But don't actually delete them - set the name to deleted, email to
+     * something random
+     *
+     * @param User $user
+     *
+     * @throws \Exception
+     */
+    public function removeUser(User $user)
+    {
+        $user->name = 'Deleted User';
+        $user->email = Utils::generateApiKey().'@deleted-user.com';
+        $user->api_key = Utils::generateApiKey();
+        $user->password = Hash::make(Utils::generateApiKey());
+        $user->state = UserState::DELETED;
+        $user->save();
+
+        // Detach all roles from this user
+        $user->detachRoles($user->roles);
+
+        // Delete any fields which might have personal information
+        UserFieldValue::where('user_id', $user->id)->delete();
+
+        // Remove any bids
+        Bid::where('user_id', $user->id)->delete();
+    }
+
     /**
      * Add a user to a given role
      *
@@ -125,7 +160,7 @@ class UserService extends Service
      *
      * @return User
      */
-    public function addUserToRole(User $user, $roleName): User
+    public function addUserToRole(User $user, string $roleName): User
     {
         $role = Role::where(['name' => $roleName])->first();
         $user->attachRole($role);

resources/lang/en/user.php

@@ -8,5 +8,6 @@ return [
         'rejected'  => 'Rejected',
         'on_leave'  => 'On Leave',
         'suspended' => 'Suspended',
+        'deleted'   => 'Deleted',
     ],
 ];

resources/lang/es/user.php

@@ -8,5 +8,6 @@ return [
         'rejected'  => 'Rechazado',
         'on_leave'  => 'De vacaciones',
         'suspended' => 'Suspendido',
+        'deleted'   => 'Borrar',
     ],
 ];

resources/lang/it/user.php

@@ -8,5 +8,6 @@ return [
         'rejected'  => 'Rifiutato',
         'on_leave'  => 'In Ferie',
         'suspended' => 'Sospeso',
+        'deleted'   => 'Cancellato',
     ],
 ];

resources/lang/pt-br/user.php

@@ -8,5 +8,6 @@ return [
         'rejected'  => 'Rejeitado',
         'on_leave'  => 'Em licença',
         'suspended' => 'Suspensa',
+        'deleted'   => 'Excluído',
     ],
 ];

tests/AcarsTest.php

@@ -418,20 +418,26 @@ class AcarsTest extends TestCase
 
     /**
      * Post a PIREP into a PREFILE state and post ACARS
+     *
+     * @throws \Exception
      */
     public function testAcarsUpdates()
     {
         $subfleet = $this->createSubfleetWithAircraft(2);
         $rank = $this->createRank(10, [$subfleet['subfleet']->id]);
 
-        $this->user = factory(User::class)->create(
+        /** @var User user */
-            [
+        $this->user = factory(User::class)->create([
-                'rank_id' => $rank->id,
+            'rank_id' => $rank->id,
-            ]
+        ]);
-        );
 
+        /** @var Airport $airport */
         $airport = factory(Airport::class)->create();
+
+        /** @var Airline $airline */
         $airline = factory(Airline::class)->create();
+
+        /** @var Aircraft $aircraft */
         $aircraft = $subfleet['aircraft']->random();
 
         $uri = '/api/pireps/prefile';

tests/UserTest.php

@@ -311,6 +311,29 @@ class UserTest extends TestCase
         $this->assertEquals(4, $user3->pilot_id);
     }
 
+    public function testUserPilotDeleted()
+    {
+        $new_user = factory(User::class)->make()->toArray();
+        $new_user['password'] = Hash::make('secret');
+        $admin_user = $this->userSvc->createUser($new_user);
+
+        $new_user = factory(User::class)->make()->toArray();
+        $new_user['password'] = Hash::make('secret');
+        $user = $this->userSvc->createUser($new_user);
+        $this->assertEquals($user->id, $user->pilot_id);
+
+        // Delete the user
+        $this->userSvc->removeUser($user);
+
+        $response = $this->get('/api/user/'.$user->id, [], $admin_user);
+        $response->assertStatus(404);
+
+        // Get from the DB
+        $user = User::find($user->id);
+        $this->assertEquals('Deleted User', $user->name);
+        $this->assertNotEquals($new_user['password'], $user->password);
+    }
+
     /**
      * Test that a user's name is private
      */