Add stricter validation around ACARS/PIREP API updates #149

app/Http/Controllers/Api/PirepController.php

@@ -2,6 +2,7 @@
 
 namespace App\Http\Controllers\Api;
 
+use App\Rules\Minutes;
 use Log;
 use Auth;
 use Illuminate\Database\Eloquent\ModelNotFoundException;
@@ -19,47 +20,13 @@ use App\Services\PIREPService;
 use App\Repositories\AcarsRepository;
 use App\Repositories\PirepRepository;
 
-use App\Http\Resources\Acars as AcarsResource;
 use App\Http\Resources\Pirep as PirepResource;
-use App\Http\Resources\AcarsLog as AcarsLogResource;
 use App\Http\Resources\AcarsRoute as AcarsRouteResource;
 
 use Symfony\Component\HttpKernel\Exception\BadRequestHttpException;
 
 class PirepController extends RestController
 {
-    public static $acars_rules = [
-        'altitude',
-        'level',
-        'heading',
-        'vs',
-        'gs',
-        'transponder',
-        'autopilot',
-        'fuel_flow',
-        'log',
-        'lat',
-        'lon',
-        'created_at',
-    ];
-
-    public static $pirep_rules = [
-        'airline_id',
-        'aircraft_id',
-        'dpt_airport_id',
-        'arr_airport_id',
-        'flight_id',
-        'flight_number',
-        'route_code',
-        'route_leg',
-        'flight_time',
-        'planned_flight_time',
-        'level',
-        'route',
-        'notes',
-        'created_at',
-    ];
-
     protected $acarsRepo,
               $geoSvc,
               $pirepRepo,
@@ -102,7 +69,24 @@ class PirepController extends RestController
     {
         Log::info('PIREP Prefile, user '.Auth::user()->id, $request->toArray());
 
-        $attrs = $this->getFromReq($request, self::$pirep_rules, [
+        $prefile_rules = [
+            'airline_id'            => 'required|exists:airlines,id',
+            'aircraft_id'           => 'required|exists:aircraft,id',
+            'dpt_airport_id'        => 'required',
+            'arr_airport_id'        => 'required',
+            'flight_id'             => 'nullable',
+            'flight_number'         => 'required',
+            'route_code'            => 'nullable',
+            'route_leg'             => 'nullable',
+            'flight_time'           => ['nullable', new Minutes],
+            'planned_flight_time'   => ['nullable', new Minutes],
+            'level'                 => 'required|integer',
+            'route'                 => 'nullable',
+            'notes'                 => 'nullable',
+            'created_at'            => 'nullable|date',
+        ];
+
+        $attrs = $this->getFromReq($request, $prefile_rules, [
             'user_id' => Auth::user()->id,
             'state' => PirepState::IN_PROGRESS,
             'status' => PirepStatus::PREFILE,
@@ -135,7 +119,7 @@ class PirepController extends RestController
      */
     public function file($id, Request $request)
     {
-        Log::info('PIREP Prefile, user ' . Auth::user()->pilot_id, $request->toArray());
+        Log::info('PIREP file, user ' . Auth::user()->id, $request->toArray());
 
         $pirep = $this->pirepRepo->find($id);
         if (empty($pirep)) {
@@ -147,7 +131,25 @@ class PirepController extends RestController
             throw new BadRequestHttpException('PIREP has been cancelled, updates can\'t be posted');
         }
 
-        $attrs = $this->getFromReq($request, self::$pirep_rules, [
+        $file_rules = [
+            # actual flight time is required
+            'flight_time'           => ['required', new Minutes],
+            'flight_number'         => 'nullable',
+            'dpt_airport_id'        => 'nullable',
+            'arr_airport_id'        => 'nullable',
+            'airline_id'            => 'nullable|exists:airlines,id',
+            'aircraft_id'           => 'nullable|exists:aircraft,id',
+            'flight_id'             => 'nullable',
+            'route_code'            => 'nullable',
+            'route_leg'             => 'nullable',
+            'planned_flight_time'   => ['nullable', new Minutes],
+            'level'                 => 'nullable',
+            'route'                 => 'nullable',
+            'notes'                 => 'nullable',
+            'created_at'            => 'nullable|date',
+        ];
+
+        $attrs = $this->getFromReq($request, $file_rules, [
             'state' => PirepState::PENDING,
             'status' => PirepStatus::ARRIVED,
         ]);
@@ -229,7 +231,7 @@ class PirepController extends RestController
      * Post ACARS updates for a PIREP
      * @param $id
      * @param Request $request
-     * @return AcarsRouteResource
+     * @return \Illuminate\Http\JsonResponse
      * @throws \Symfony\Component\HttpKernel\Exception\BadRequestHttpException
      */
     public function acars_store($id, Request $request)
@@ -246,13 +248,28 @@ class PirepController extends RestController
         $this->validate($request, ['positions' => 'required']);
         $positions = $request->post('positions');
 
+        $acars_rules = [
+            'lat'           => 'required|numeric',
+            'lon'           => 'required|numeric',
+            'altitude'      => 'nullable',
+            'level'         => 'nullable',
+            'heading'       => 'nullable',
+            'vs'            => 'nullable',
+            'gs'            => 'nullable',
+            'transponder'   => 'nullable',
+            'autopilot'     => 'nullable',
+            'fuel_flow'     => 'nullable',
+            'log'           => 'nullable',
+            'created_at'    => 'nullable|date',
+        ];
+
         $count = 0;
         foreach($positions as $position)
         {
             try {
                 $attrs = $this->getFromReq(
                     $position,
-                    self::$acars_rules,
+                    $acars_rules,
                     ['pirep_id' => $id, 'type' => AcarsType::FLIGHT_PATH]
                 );
 
@@ -293,14 +310,16 @@ class PirepController extends RestController
         $this->validate($request, ['logs' => 'required']);
         $logs = $request->post('logs');
 
-        $count = 0;
+        $rules = [
-        foreach($logs as $log) {
-            $attrs = $this->getFromReq($log, [
             'log'           => 'required',
             'lat'           => 'nullable',
             'lon'           => 'nullable',
-                'created_at' => 'nullable',
+            'created_at'    => 'nullable|date',
-            ], ['pirep_id' => $id, 'type' => AcarsType::LOG]);
+        ];
+
+        $count = 0;
+        foreach($logs as $log) {
+            $attrs = $this->getFromReq($log, $rules, ['pirep_id' => $id, 'type' => AcarsType::LOG]);
 
             $acars = Acars::create($attrs);
             $acars->save();

app/Http/Kernel.php

@@ -15,6 +15,8 @@ class Kernel extends HttpKernel
      */
     protected $middleware = [
         \Illuminate\Foundation\Http\Middleware\CheckForMaintenanceMode::class,
+        \Illuminate\Foundation\Http\Middleware\TrimStrings::class,
+        \Illuminate\Foundation\Http\Middleware\ConvertEmptyStringsToNull::class,
     ];
 
     /**
@@ -57,8 +59,5 @@ class Kernel extends HttpKernel
         'guest' => \App\Http\Middleware\RedirectIfAuthenticated::class,
         'json' => \App\Http\Middleware\JsonResponse::class,
         'throttle' => \Illuminate\Routing\Middleware\ThrottleRequests::class,
-        #'role' => \Laratrust\Middleware\LaratrustRole::class,
-        #'permission' => \Laratrust\Middleware\LaratrustPermission::class,
-        #'ability' => \Laratrust\Middleware\LaratrustAbility::class,
     ];
 }

app/Rules/Minutes.php

@@ -0,0 +1,25 @@
+<?php
+
+namespace App\Rules;
+
+use Illuminate\Contracts\Validation\Rule;
+
+/**
+ * Class Minutes
+ * @package App\Rules
+ *
+ * Make sure that a given value is an integer, but the custom
+ * validation message is what really matters here
+ */
+class Minutes implements Rule
+{
+    public function passes($attribute, $value): bool
+    {
+        return \is_int(filter_var($value, FILTER_VALIDATE_INT));
+    }
+
+    public function message(): string
+    {
+        return ':attribute must be an integer, in minutes';
+    }
+}

tests/AcarsTest.php

@@ -54,6 +54,34 @@ class AcarsTest extends TestCase
         return $resp->json();
     }
 
+    /**
+     * Post a PIREP into a PREFILE state and post ACARS
+     */
+    public function testPrefileErrors()
+    {
+        $this->user = factory(App\Models\User::class)->create();
+
+        $airport = factory(App\Models\Airport::class)->create();
+        $airline = factory(App\Models\Airline::class)->create();
+        $aircraft = factory(App\Models\Aircraft::class)->create();
+
+        $uri = '/api/pireps/prefile';
+        $pirep = [
+            '_airline_id' => $airline->id,
+            'aircraft_id' => $aircraft->id,
+            'dpt_airport_id' => $airport->icao,
+            'arr_airport_id' => $airport->icao,
+            'flight_number' => '6000',
+            'level' => 38000,
+            'planned_flight_time' => 120,
+            'route' => 'POINTA POINTB',
+        ];
+
+        $response = $this->post($uri, $pirep);
+        $response->assertStatus(400);
+
+    }
+
     /**
      * Post a PIREP into a PREFILE state and post ACARS
      */
@@ -121,6 +149,17 @@ class AcarsTest extends TestCase
         $this->assertCount(1, $body);
         $this->assertEquals(round($acars['lat'], 2), round($body[0]['lat'], 2));
         $this->assertEquals(round($acars['lon'], 2), round($body[0]['lon'], 2));
+
+        # File the PIREP now
+        $uri = '/api/pireps/'.$pirep_id.'/file';
+        $response = $this->post($uri, []);
+        $response->assertStatus(400); // missing the flight time
+
+        $response = $this->post($uri, ['flight_time' => '1:30']);
+        $response->assertStatus(400); // invalid flight time
+
+        $response = $this->post($uri, ['flight_time' => '130']);
+        $response->assertStatus(200); // invalid flight time
     }
 
     /**