From edb72e462f6077f29be8b4e1bb79b237db947885 Mon Sep 17 00:00:00 2001 From: Nabeel Shahzad Date: Tue, 23 Jan 2018 11:46:26 -0600 Subject: [PATCH] Add stricter validation around ACARS/PIREP API updates #149 --- app/Http/Controllers/Api/PirepController.php | 109 +++++++++++-------- app/Http/Kernel.php | 5 +- app/Rules/Minutes.php | 25 +++++ tests/AcarsTest.php | 39 +++++++ 4 files changed, 130 insertions(+), 48 deletions(-) create mode 100644 app/Rules/Minutes.php diff --git a/app/Http/Controllers/Api/PirepController.php b/app/Http/Controllers/Api/PirepController.php index 0d0dc4b0..4f369fbf 100644 --- a/app/Http/Controllers/Api/PirepController.php +++ b/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'); + $rules = [ + 'log' => 'required', + 'lat' => 'nullable', + 'lon' => 'nullable', + 'created_at' => 'nullable|date', + ]; + $count = 0; foreach($logs as $log) { - $attrs = $this->getFromReq($log, [ - 'log' => 'required', - 'lat' => 'nullable', - 'lon' => 'nullable', - 'created_at' => 'nullable', - ], ['pirep_id' => $id, 'type' => AcarsType::LOG]); + $attrs = $this->getFromReq($log, $rules, ['pirep_id' => $id, 'type' => AcarsType::LOG]); $acars = Acars::create($attrs); $acars->save(); diff --git a/app/Http/Kernel.php b/app/Http/Kernel.php index af709e8a..ea4eb178 100755 --- a/app/Http/Kernel.php +++ b/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, ]; } diff --git a/app/Rules/Minutes.php b/app/Rules/Minutes.php new file mode 100644 index 00000000..12585bb5 --- /dev/null +++ b/app/Rules/Minutes.php @@ -0,0 +1,25 @@ +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 } /**