From 60108fb7cc4a226c2a34e767a02fabcb7f8670f0 Mon Sep 17 00:00:00 2001 From: Graham Campbell Date: Sat, 21 Nov 2015 21:18:40 +0000 Subject: [PATCH 1/6] Improved api validation --- .../Displayers/JsonValidationDisplayer.php | 53 +++++++++++++++++++ .../Controllers/Api/ComponentController.php | 6 +-- .../Api/ComponentGroupController.php | 6 +-- .../Controllers/Api/IncidentController.php | 6 +-- app/Http/Controllers/Api/MetricController.php | 6 +-- .../Controllers/Api/MetricPointController.php | 4 +- .../Controllers/Api/SubscriberController.php | 4 +- config/exceptions.php | 1 + 8 files changed, 70 insertions(+), 16 deletions(-) create mode 100644 app/Exceptions/Displayers/JsonValidationDisplayer.php diff --git a/app/Exceptions/Displayers/JsonValidationDisplayer.php b/app/Exceptions/Displayers/JsonValidationDisplayer.php new file mode 100644 index 00000000..80c01a9a --- /dev/null +++ b/app/Exceptions/Displayers/JsonValidationDisplayer.php @@ -0,0 +1,53 @@ +info->generate($exception, $id, $code); + + $error = ['id' => $id, 'status' => $info['code'], 'title' => $info['name'], 'detail' => $info['detail'], 'meta' => ['details' => $exception->getMessageBag()->all()]]; + + return new JsonResponse(['errors' => [$error]], $code, array_merge($headers, ['Content-Type' => $this->contentType()])); + } + + /** + * Can we display the exception? + * + * @param \Exception $original + * @param \Exception $transformed + * @param int $code + * + * @return bool + */ + public function canDisplay(Exception $original, Exception $transformed, $code) + { + return $exception instanceof ValidationException; + } +} diff --git a/app/Http/Controllers/Api/ComponentController.php b/app/Http/Controllers/Api/ComponentController.php index 47710feb..f01dc53b 100644 --- a/app/Http/Controllers/Api/ComponentController.php +++ b/app/Http/Controllers/Api/ComponentController.php @@ -16,9 +16,9 @@ use CachetHQ\Cachet\Commands\Component\RemoveComponentCommand; use CachetHQ\Cachet\Commands\Component\UpdateComponentCommand; use CachetHQ\Cachet\Models\Component; use CachetHQ\Cachet\Models\Tag; -use Exception; use GrahamCampbell\Binput\Facades\Binput; use Illuminate\Contracts\Auth\Guard; +use Illuminate\Database\QueryException; use Illuminate\Foundation\Bus\DispatchesJobs; use Illuminate\Http\Request; use Symfony\Component\HttpKernel\Exception\BadRequestHttpException; @@ -75,7 +75,7 @@ class ComponentController extends AbstractApiController Binput::get('group_id'), (bool) Binput::get('enabled', true) )); - } catch (Exception $e) { + } catch (QueryException $e) { throw new BadRequestHttpException(); } @@ -116,7 +116,7 @@ class ComponentController extends AbstractApiController Binput::get('group_id'), (bool) Binput::get('enabled', true) )); - } catch (Exception $e) { + } catch (QueryException $e) { throw new BadRequestHttpException(); } diff --git a/app/Http/Controllers/Api/ComponentGroupController.php b/app/Http/Controllers/Api/ComponentGroupController.php index 1d0a1702..2c378e73 100644 --- a/app/Http/Controllers/Api/ComponentGroupController.php +++ b/app/Http/Controllers/Api/ComponentGroupController.php @@ -15,8 +15,8 @@ use CachetHQ\Cachet\Commands\ComponentGroup\AddComponentGroupCommand; use CachetHQ\Cachet\Commands\ComponentGroup\RemoveComponentGroupCommand; use CachetHQ\Cachet\Commands\ComponentGroup\UpdateComponentGroupCommand; use CachetHQ\Cachet\Models\ComponentGroup; -use Exception; use GrahamCampbell\Binput\Facades\Binput; +use Illuminate\Database\QueryException; use Illuminate\Foundation\Bus\DispatchesJobs; use Illuminate\Http\Request; use Symfony\Component\HttpKernel\Exception\BadRequestHttpException; @@ -63,7 +63,7 @@ class ComponentGroupController extends AbstractApiController Binput::get('name'), Binput::get('order', 0) )); - } catch (Exception $e) { + } catch (QueryException $e) { throw new BadRequestHttpException(); } @@ -85,7 +85,7 @@ class ComponentGroupController extends AbstractApiController Binput::get('name'), Binput::get('order', 0) )); - } catch (Exception $e) { + } catch (QueryException $e) { throw new BadRequestHttpException(); } diff --git a/app/Http/Controllers/Api/IncidentController.php b/app/Http/Controllers/Api/IncidentController.php index c0d482a2..f373a9c6 100644 --- a/app/Http/Controllers/Api/IncidentController.php +++ b/app/Http/Controllers/Api/IncidentController.php @@ -15,9 +15,9 @@ use CachetHQ\Cachet\Commands\Incident\RemoveIncidentCommand; use CachetHQ\Cachet\Commands\Incident\ReportIncidentCommand; use CachetHQ\Cachet\Commands\Incident\UpdateIncidentCommand; use CachetHQ\Cachet\Models\Incident; -use Exception; use GrahamCampbell\Binput\Facades\Binput; use Illuminate\Contracts\Auth\Guard; +use Illuminate\Database\QueryException; use Illuminate\Foundation\Bus\DispatchesJobs; use Illuminate\Http\Request; use Symfony\Component\HttpKernel\Exception\BadRequestHttpException; @@ -77,7 +77,7 @@ class IncidentController extends AbstractApiController Binput::get('template'), Binput::get('vars') )); - } catch (Exception $e) { + } catch (QueryException $e) { throw new BadRequestHttpException(); } @@ -107,7 +107,7 @@ class IncidentController extends AbstractApiController Binput::get('template'), Binput::get('vars') )); - } catch (Exception $e) { + } catch (QueryException $e) { throw new BadRequestHttpException(); } diff --git a/app/Http/Controllers/Api/MetricController.php b/app/Http/Controllers/Api/MetricController.php index 15b1fb45..88b199c6 100644 --- a/app/Http/Controllers/Api/MetricController.php +++ b/app/Http/Controllers/Api/MetricController.php @@ -15,8 +15,8 @@ use CachetHQ\Cachet\Commands\Metric\AddMetricCommand; use CachetHQ\Cachet\Commands\Metric\RemoveMetricCommand; use CachetHQ\Cachet\Commands\Metric\UpdateMetricCommand; use CachetHQ\Cachet\Models\Metric; -use Exception; use GrahamCampbell\Binput\Facades\Binput; +use Illuminate\Database\QueryException; use Illuminate\Foundation\Bus\DispatchesJobs; use Illuminate\Http\Request; use Symfony\Component\HttpKernel\Exception\BadRequestHttpException; @@ -80,7 +80,7 @@ class MetricController extends AbstractApiController Binput::get('display_chart'), Binput::get('places', 2) )); - } catch (Exception $e) { + } catch (QueryException $e) { throw new BadRequestHttpException(); } @@ -107,7 +107,7 @@ class MetricController extends AbstractApiController Binput::get('display_chart'), Binput::get('places', 2) )); - } catch (Exception $e) { + } catch (QueryException $e) { throw new BadRequestHttpException(); } diff --git a/app/Http/Controllers/Api/MetricPointController.php b/app/Http/Controllers/Api/MetricPointController.php index 1c615ab4..9dc59a58 100644 --- a/app/Http/Controllers/Api/MetricPointController.php +++ b/app/Http/Controllers/Api/MetricPointController.php @@ -16,8 +16,8 @@ use CachetHQ\Cachet\Commands\Metric\RemoveMetricPointCommand; use CachetHQ\Cachet\Commands\Metric\UpdateMetricPointCommand; use CachetHQ\Cachet\Models\Metric; use CachetHQ\Cachet\Models\MetricPoint; -use Exception; use GrahamCampbell\Binput\Facades\Binput; +use Illuminate\Database\QueryException; use Illuminate\Foundation\Bus\DispatchesJobs; use Symfony\Component\HttpKernel\Exception\BadRequestHttpException; @@ -53,7 +53,7 @@ class MetricPointController extends AbstractApiController Binput::get('value'), Binput::get('timestamp')) ); - } catch (Exception $e) { + } catch (QueryException $e) { throw new BadRequestHttpException(); } diff --git a/app/Http/Controllers/Api/SubscriberController.php b/app/Http/Controllers/Api/SubscriberController.php index 0f5d75ab..1adbd23e 100644 --- a/app/Http/Controllers/Api/SubscriberController.php +++ b/app/Http/Controllers/Api/SubscriberController.php @@ -14,8 +14,8 @@ namespace CachetHQ\Cachet\Http\Controllers\Api; use CachetHQ\Cachet\Commands\Subscriber\SubscribeSubscriberCommand; use CachetHQ\Cachet\Commands\Subscriber\UnsubscribeSubscriberCommand; use CachetHQ\Cachet\Models\Subscriber; -use Exception; use GrahamCampbell\Binput\Facades\Binput; +use Illuminate\Database\QueryException; use Illuminate\Foundation\Bus\DispatchesJobs; use Illuminate\Http\Request; use Symfony\Component\HttpKernel\Exception\BadRequestHttpException; @@ -47,7 +47,7 @@ class SubscriberController extends AbstractApiController { try { $subscriber = $this->dispatch(new SubscribeSubscriberCommand(Binput::get('email'), Binput::get('verify', false))); - } catch (Exception $e) { + } catch (QueryException $e) { throw new BadRequestHttpException(); } diff --git a/config/exceptions.php b/config/exceptions.php index ecc20d59..d1ac8b8a 100644 --- a/config/exceptions.php +++ b/config/exceptions.php @@ -48,6 +48,7 @@ return [ 'CachetHQ\Cachet\Exceptions\Displayers\RedirectDisplayer', 'GrahamCampbell\Exceptions\Displayers\DebugDisplayer', 'GrahamCampbell\Exceptions\Displayers\HtmlDisplayer', + 'GrahamCampbell\Exceptions\Displayers\JsonValidationDisplayer', 'GrahamCampbell\Exceptions\Displayers\JsonDisplayer', 'GrahamCampbell\Exceptions\Displayers\JsonApiDisplayer', ], From 025b92c51c3d92a21c9cf22aa7711c30c67805f3 Mon Sep 17 00:00:00 2001 From: Graham Campbell Date: Sat, 21 Nov 2015 21:20:15 +0000 Subject: [PATCH 2/6] Force status code 400 --- app/Exceptions/Displayers/JsonValidationDisplayer.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/Exceptions/Displayers/JsonValidationDisplayer.php b/app/Exceptions/Displayers/JsonValidationDisplayer.php index 80c01a9a..d24a5f93 100644 --- a/app/Exceptions/Displayers/JsonValidationDisplayer.php +++ b/app/Exceptions/Displayers/JsonValidationDisplayer.php @@ -30,11 +30,11 @@ class JsonValidationDisplayer extends JsonDisplayer implements DisplayerInterfac */ public function display(Exception $exception, $id, $code, array $headers) { - $info = $this->info->generate($exception, $id, $code); + $info = $this->info->generate($exception, $id, 400); $error = ['id' => $id, 'status' => $info['code'], 'title' => $info['name'], 'detail' => $info['detail'], 'meta' => ['details' => $exception->getMessageBag()->all()]]; - return new JsonResponse(['errors' => [$error]], $code, array_merge($headers, ['Content-Type' => $this->contentType()])); + return new JsonResponse(['errors' => [$error]], 400, array_merge($headers, ['Content-Type' => $this->contentType()])); } /** From 7a322d6032abf29e98dbab56b370e655671e1a48 Mon Sep 17 00:00:00 2001 From: Graham Campbell Date: Sat, 21 Nov 2015 21:37:54 +0000 Subject: [PATCH 3/6] Added missing import --- app/Exceptions/Displayers/JsonValidationDisplayer.php | 1 + 1 file changed, 1 insertion(+) diff --git a/app/Exceptions/Displayers/JsonValidationDisplayer.php b/app/Exceptions/Displayers/JsonValidationDisplayer.php index d24a5f93..6d7464df 100644 --- a/app/Exceptions/Displayers/JsonValidationDisplayer.php +++ b/app/Exceptions/Displayers/JsonValidationDisplayer.php @@ -15,6 +15,7 @@ use AltThree\Validator\ValidationException; use Exception; use GrahamCampbell\Exceptions\Displayers\DisplayerInterface; use GrahamCampbell\Exceptions\Displayers\JsonDisplayer; +use Symfony\Component\HttpFoundation\JsonResponse; class JsonValidationDisplayer extends JsonDisplayer implements DisplayerInterface { From 75deb97059044c2d06609ad7989745d8ba10314b Mon Sep 17 00:00:00 2001 From: James Brooks Date: Sat, 21 Nov 2015 21:42:35 +0000 Subject: [PATCH 4/6] Correct the JsonValidationDisplayer class config --- config/exceptions.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/exceptions.php b/config/exceptions.php index d1ac8b8a..06dcaffe 100644 --- a/config/exceptions.php +++ b/config/exceptions.php @@ -45,10 +45,10 @@ return [ */ 'displayers' => [ + 'CachetHQ\Cachet\Exceptions\Displayers\JsonValidationDisplayer', 'CachetHQ\Cachet\Exceptions\Displayers\RedirectDisplayer', 'GrahamCampbell\Exceptions\Displayers\DebugDisplayer', 'GrahamCampbell\Exceptions\Displayers\HtmlDisplayer', - 'GrahamCampbell\Exceptions\Displayers\JsonValidationDisplayer', 'GrahamCampbell\Exceptions\Displayers\JsonDisplayer', 'GrahamCampbell\Exceptions\Displayers\JsonApiDisplayer', ], From c266a419e0d8cab4c76f8314b3deca2ea839b107 Mon Sep 17 00:00:00 2001 From: James Brooks Date: Sat, 21 Nov 2015 21:45:15 +0000 Subject: [PATCH 5/6] Fix canDisplay method --- app/Exceptions/Displayers/JsonValidationDisplayer.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/Exceptions/Displayers/JsonValidationDisplayer.php b/app/Exceptions/Displayers/JsonValidationDisplayer.php index 6d7464df..f0db8381 100644 --- a/app/Exceptions/Displayers/JsonValidationDisplayer.php +++ b/app/Exceptions/Displayers/JsonValidationDisplayer.php @@ -49,6 +49,6 @@ class JsonValidationDisplayer extends JsonDisplayer implements DisplayerInterfac */ public function canDisplay(Exception $original, Exception $transformed, $code) { - return $exception instanceof ValidationException; + return $original instanceof ValidationException || $transformed instanceof ValidationException; } } From beca42b5326021f9d308f8a60bd86f5f04e8a0ec Mon Sep 17 00:00:00 2001 From: James Brooks Date: Sat, 21 Nov 2015 21:45:36 +0000 Subject: [PATCH 6/6] Only check $transformed exception --- app/Exceptions/Displayers/JsonValidationDisplayer.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/Exceptions/Displayers/JsonValidationDisplayer.php b/app/Exceptions/Displayers/JsonValidationDisplayer.php index f0db8381..5c3ee86f 100644 --- a/app/Exceptions/Displayers/JsonValidationDisplayer.php +++ b/app/Exceptions/Displayers/JsonValidationDisplayer.php @@ -49,6 +49,6 @@ class JsonValidationDisplayer extends JsonDisplayer implements DisplayerInterfac */ public function canDisplay(Exception $original, Exception $transformed, $code) { - return $original instanceof ValidationException || $transformed instanceof ValidationException; + return $transformed instanceof ValidationException; } }