From d542975b9fdcf3ea9118eda66764b04e8db436f9 Mon Sep 17 00:00:00 2001 From: Jonny Barnes Date: Thu, 22 Sep 2016 15:15:47 +0100 Subject: [PATCH 1/8] No need to define class types in postgisFields array --- app/Place.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/Place.php b/app/Place.php index 7ce34599..14682f65 100644 --- a/app/Place.php +++ b/app/Place.php @@ -33,8 +33,8 @@ class Place extends Model * @var array */ protected $postgisFields = [ - 'location' => Point::class, - 'polygon' => Polygon::class, + 'location', + 'polygon', ]; /** From 0a9aafb620103b582a6a8d29954cef29c6918d8b Mon Sep 17 00:00:00 2001 From: Jonny Barnes Date: Thu, 22 Sep 2016 15:18:29 +0100 Subject: [PATCH 2/8] Remove non-needed use statements --- app/Place.php | 2 -- 1 file changed, 2 deletions(-) diff --git a/app/Place.php b/app/Place.php index 14682f65..13daa290 100644 --- a/app/Place.php +++ b/app/Place.php @@ -4,9 +4,7 @@ namespace App; use DB; use Illuminate\Database\Eloquent\Model; -use Phaza\LaravelPostgis\Geometries\Point; use MartinBean\Database\Eloquent\Sluggable; -use Phaza\LaravelPostgis\Geometries\Polygon; use Phaza\LaravelPostgis\Eloquent\PostgisTrait; class Place extends Model From 32af94fab8eb5ce292ed942af5f6c05d9b1f6e5b Mon Sep 17 00:00:00 2001 From: Jonny Barnes Date: Thu, 22 Sep 2016 16:00:04 +0100 Subject: [PATCH 3/8] Better error handling in case of place creation failure --- app/Http/Controllers/MicropubClientController.php | 2 +- app/Http/Controllers/MicropubController.php | 6 +++++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/app/Http/Controllers/MicropubClientController.php b/app/Http/Controllers/MicropubClientController.php index adac3d33..cbdcf1dc 100644 --- a/app/Http/Controllers/MicropubClientController.php +++ b/app/Http/Controllers/MicropubClientController.php @@ -263,7 +263,7 @@ class MicropubClientController extends Controller 'headers' => $headers, ]); } catch (ClientException $e) { - //not sure yet... + return false; } if ($response->getStatusCode() == 201) { return $response->getHeader('Location')[0]; diff --git a/app/Http/Controllers/MicropubController.php b/app/Http/Controllers/MicropubController.php index 0ff28497..c1b6dc64 100644 --- a/app/Http/Controllers/MicropubController.php +++ b/app/Http/Controllers/MicropubController.php @@ -70,7 +70,11 @@ EOD; ->header('Content-Type', 'application/json'); } if ($request->input('h') == 'card' || $request->input('type')[0] == 'h-card') { - $place = $this->placeService->createPlace($request); + try { + $place = $this->placeService->createPlace($request); + } catch (Exception $exception) { + return response()->json(['error' => true], 400); + } $content = << Date: Fri, 23 Sep 2016 12:24:23 +0100 Subject: [PATCH 4/8] Better error handling for fetch requests --- public/assets/js/newnote.js | 21 +++++++++++---------- resources/assets/js/newnote.js | 23 ++++++++++++----------- 2 files changed, 23 insertions(+), 21 deletions(-) diff --git a/public/assets/js/newnote.js b/public/assets/js/newnote.js index 54d2edd5..0ea0874b 100644 --- a/public/assets/js/newnote.js +++ b/public/assets/js/newnote.js @@ -12,11 +12,11 @@ if ('geolocation' in navigator) { function getLocation() { navigator.geolocation.getCurrentPosition(function (position) { //the locate button has been clicked so add the places/map - addPlaces(position.coords.latitude, position.coords.longitude); + addPlacesMap(position.coords.latitude, position.coords.longitude); }); } -function addPlaces(latitude, longitude) { +function addPlacesMap(latitude, longitude) { //get the nearby places fetch('/places/near/' + latitude + '/' + longitude, { credentials: 'same-origin', @@ -24,6 +24,10 @@ function addPlaces(latitude, longitude) { }).then(function (response) { return response.json(); }).then(function (j) { + if (j.error == true) { + alertify.reset(); + alertify.error(j.error_description); + } if (j.length > 0) { var i; var places = []; @@ -195,17 +199,13 @@ function addMap(latitude, longitude, places) { method: 'post', body: formData }) - .then(function (response) { - if (response.status >= 200 && response.status < 300) { - return Promise.resolve(response); - } else { - return Promise.reject(new Error(response.statusText)); - } - }) .then(function (response) { return response.json(); }) .then(function (placeJson) { + if (placeJson.error == true) { + throw new Error(placeJson.error_description); + } //create the slug from the url var urlParts = placeJson.split('/'); var slug = urlParts.pop(); @@ -247,7 +247,8 @@ function addMap(latitude, longitude, places) { //make selected selectPlace(slug); }).catch(function (placeError) { - console.error(placeError); + alertify.reset(); + alertify.error(placeError); }); }); }); diff --git a/resources/assets/js/newnote.js b/resources/assets/js/newnote.js index 54d2edd5..3e6d7792 100644 --- a/resources/assets/js/newnote.js +++ b/resources/assets/js/newnote.js @@ -1,4 +1,4 @@ -/* global L */ +/* global L, alertify */ if ('geolocation' in navigator) { var button = document.querySelector('#locate'); if (button.addEventListener) { @@ -12,11 +12,11 @@ if ('geolocation' in navigator) { function getLocation() { navigator.geolocation.getCurrentPosition(function (position) { //the locate button has been clicked so add the places/map - addPlaces(position.coords.latitude, position.coords.longitude); + addPlacesMap(position.coords.latitude, position.coords.longitude); }); } -function addPlaces(latitude, longitude) { +function addPlacesMap(latitude, longitude) { //get the nearby places fetch('/places/near/' + latitude + '/' + longitude, { credentials: 'same-origin', @@ -24,6 +24,10 @@ function addPlaces(latitude, longitude) { }).then(function (response) { return response.json(); }).then(function (j) { + if (j.error == true) { + alertify.reset(); + alertify.error(j.error_description); + } if (j.length > 0) { var i; var places = []; @@ -195,17 +199,13 @@ function addMap(latitude, longitude, places) { method: 'post', body: formData }) - .then(function (response) { - if (response.status >= 200 && response.status < 300) { - return Promise.resolve(response); - } else { - return Promise.reject(new Error(response.statusText)); - } - }) .then(function (response) { return response.json(); }) .then(function (placeJson) { + if (placeJson.error == true) { + throw new Error(placeJson.error_description); + } //create the slug from the url var urlParts = placeJson.split('/'); var slug = urlParts.pop(); @@ -247,7 +247,8 @@ function addMap(latitude, longitude, places) { //make selected selectPlace(slug); }).catch(function (placeError) { - console.error(placeError); + alertify.reset(); + alertify.error(placeError); }); }); }); From 4133efb8a80dda9c73871b7b71e70d871b2e05b2 Mon Sep 17 00:00:00 2001 From: Jonny Barnes Date: Fri, 23 Sep 2016 12:26:43 +0100 Subject: [PATCH 5/8] Return json for all the requests the newnote.js makes, this allows for better error handling on the frontend --- .../Controllers/MicropubClientController.php | 42 +++++++++++++------ 1 file changed, 29 insertions(+), 13 deletions(-) diff --git a/app/Http/Controllers/MicropubClientController.php b/app/Http/Controllers/MicropubClientController.php index cbdcf1dc..9035bae8 100644 --- a/app/Http/Controllers/MicropubClientController.php +++ b/app/Http/Controllers/MicropubClientController.php @@ -204,34 +204,37 @@ class MicropubClientController extends Controller */ public function postNewPlace(Request $request) { + if ($request->session()->has('token') === false) { + return response()->json([ + 'error' => true, + 'error_description' => 'No known token', + ], 400); + } $domain = $request->session()->get('me'); $token = $request->session()->get('token'); $micropubEndpoint = $this->indieAuthService->discoverMicropubEndpoint($domain, $this->indieClient); if (! $micropubEndpoint) { - return (new Response(json_encode([ + return response()->json([ 'error' => true, - 'message' => 'Could not determine the micropub endpoint.', - ]), 400)) - ->header('Content-Type', 'application/json'); + 'error_description' => 'Could not determine the micropub endpoint.', + ], 400); } $place = $this->postPlaceRequest($request, $micropubEndpoint, $token); if ($place === false) { - return (new Response(json_encode([ + return response()->json([ 'error' => true, - 'message' => 'Unable to create the new place', - ]), 400)) - ->header('Content-Type', 'application/json'); + 'error_description' => 'Unable to create the new place', + ], 400); } - return (new Response(json_encode([ + return response()->json([ 'url' => $place, 'name' => $request->input('place-name'), 'latitude' => $request->input('place-latitude'), 'longitude' => $request->input('place-longitude'), - ]), 200)) - ->header('Content-Type', 'application/json'); + ]); } /** @@ -285,12 +288,22 @@ class MicropubClientController extends Controller $latitude, $longitude ) { + if ($request->session()->has('token') === false) { + return response()->json([ + 'error' => true, + 'error_description' => 'No known token', + ], 400); + } $domain = $request->session()->get('me'); $token = $request->session()->get('token'); + $micropubEndpoint = $this->indieAuthService->discoverMicropubEndpoint($domain, $this->indieClient); if (! $micropubEndpoint) { - return; + return response()->json([ + 'error' => true, + 'error_description' => 'No known endpoint', + ], 400); } try { @@ -299,7 +312,10 @@ class MicropubClientController extends Controller 'query' => ['q' => 'geo:' . $latitude . ',' . $longitude], ]); } catch (\GuzzleHttp\Exception\BadResponseException $e) { - return; + return response()->json([ + 'error' => true, + 'error_description' => 'The endpoint returned a non-good response', + ], 400); } return (new Response($response->getBody(), 200)) From 0d1787d69a7fc22b312a23afa78b8a23a15d8300 Mon Sep 17 00:00:00 2001 From: Jonny Barnes Date: Fri, 23 Sep 2016 16:51:48 +0100 Subject: [PATCH 6/8] Add support for JSON requests --- app/Services/PlaceService.php | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/app/Services/PlaceService.php b/app/Services/PlaceService.php index c5e49b18..85a27972 100644 --- a/app/Services/PlaceService.php +++ b/app/Services/PlaceService.php @@ -16,21 +16,26 @@ class PlaceService */ public function createPlace(Request $request) { - //we’ll either have latitude and longitude sent together in a - //geo-url (micropub), or seperatley (/admin) - if ($request->input('geo') !== null) { - $parts = explode(':', $request->input('geo')); - $latlng = explode(',', $parts[1]); - $latitude = $latlng[0]; - $longitude = $latlng[1]; + if ($request->header('Content-Type') == 'application/json') { + $name = $request->input('properties.name'); + $description = $request->input('properties.description') ?? null; + $geo = $request->input('properties.geo'); + } else { + $name = $request->input('name'); + $description = $request->input('description'); + $geo = $request->input('geo'); } + $parts = explode(':', $geo); + $latlng = explode(',', $parts[1]); + $latitude = $latlng[0]; + $longitude = $latlng[1]; if ($request->input('latitude') !== null) { $latitude = $request->input('latitude'); $longitude = $request->input('longitude'); } $place = new Place(); - $place->name = $request->input('name'); - $place->description = $request->input('description'); + $place->name = $name; + $place->description = $description; $place->location = new Point((float) $latitude, (float) $longitude); $place->save(); From 3c689c09ed09d16f2be1b4ce7f370f24e35b2193 Mon Sep 17 00:00:00 2001 From: Jonny Barnes Date: Fri, 23 Sep 2016 16:55:02 +0100 Subject: [PATCH 7/8] Much better json response code, add an extra try/catch --- app/Http/Controllers/MicropubController.php | 61 ++++++++------------- 1 file changed, 23 insertions(+), 38 deletions(-) diff --git a/app/Http/Controllers/MicropubController.php b/app/Http/Controllers/MicropubController.php index c1b6dc64..61e82f49 100644 --- a/app/Http/Controllers/MicropubController.php +++ b/app/Http/Controllers/MicropubController.php @@ -57,17 +57,15 @@ class MicropubController extends Controller if (array_search('post', $scopes) !== false) { $clientId = $tokenData->getClaim('client_id'); if (($request->input('h') == 'entry') || ($request->input('type')[0] == 'h-entry')) { - $note = $this->noteService->createNote($request, $clientId); - $content = <<longurl" -} -EOD; - - return (new Response($content, 201)) - ->header('Location', $note->longurl) - ->header('Content-Type', 'application/json'); + try { + $note = $this->noteService->createNote($request, $clientId); + } catch (Exception $exception) { + return response()->json(['error' => true], 400); + } + return response()->json([ + 'response' => 'created', + 'location' => $note->longurl + ], 201)->header('Location', $note->longurl); } if ($request->input('h') == 'card' || $request->input('type')[0] == 'h-card') { try { @@ -75,40 +73,27 @@ EOD; } catch (Exception $exception) { return response()->json(['error' => true], 400); } - $content = <<longurl" -} -EOD; - return (new Response($content, 201)) - ->header('Location', $place->longurl) - ->header('Content-Type', 'application/json'); + return response()->json([ + 'response' => 'created', + 'location' => $place->longurl + ], 201)->header('Location', $place->longurl); } } } - $content = <<<'EOD' -{ - "response": "error", - "error": "invalid_token", - "error_description": "The token provided is not valid or does not have the necessary scope", -} -EOD; - return (new Response($content, 400)) - ->header('Content-Type', 'application/json'); + return response()->json([ + 'response' => 'error', + 'error' => 'invalid_token', + 'error_description' => 'The token provided is not valid or does not have the necessary scope' + ], 400); } - $content = <<<'EOD' -{ - "response": "error", - "error": "no_token", - "error_description": "No OAuth token sent with request" -} -EOD; - return (new Response($content, 400)) - ->header('Content-Type', 'application/json'); + return response()->json([ + 'response' => 'error', + 'error' => 'no_token', + 'error_description' => 'No OAuth token sent with request' + ], 400); } /** From 3b21fb5f1f4ad0a480650374858dcb00c959158f Mon Sep 17 00:00:00 2001 From: Jonny Barnes Date: Fri, 23 Sep 2016 16:55:25 +0100 Subject: [PATCH 8/8] More thorough test suite for micropub code --- tests/MicropubTest.php | 111 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 111 insertions(+) diff --git a/tests/MicropubTest.php b/tests/MicropubTest.php index 26addf48..88a9838f 100644 --- a/tests/MicropubTest.php +++ b/tests/MicropubTest.php @@ -116,6 +116,103 @@ class MicropubTest extends TestCase ['HTTP_Authorization' => 'Bearer ' . $this->getToken()] )->seeJson([ 'response' => 'created' + ])->assertResponseStatus(201); + } + + public function testMicropubJSONRequestCreateNewNoteWithoutToken() + { + $faker = \Faker\Factory::create(); + $note = $faker->text; + $this->json( + 'POST', + $this->appurl . '/api/post', + [ + 'type' => ['h-entry'], + 'properties' => [ + 'content' => [$note], + ], + ] + )->seeJson([ + 'response' => 'error', + 'error' => 'no_token' + ])->assertResponseStatus(400); + } + + public function testMicropubJSONRequestCreateNewNoteWithInvalidToken() + { + $faker = \Faker\Factory::create(); + $note = $faker->text; + $this->json( + 'POST', + $this->appurl . '/api/post', + [ + 'type' => ['h-entry'], + 'properties' => [ + 'content' => [$note], + ], + ], + ['HTTP_Authorization' => 'Bearer ' . $this->getInvalidToken()] + )->seeJson([ + 'response' => 'error', + 'error' => 'invalid_token' + ]); + } + + public function testMicropubJSONRequestCreateNewPlace() + { + $faker = \Faker\Factory::create(); + $this->json( + 'POST', + $this->appurl . '/api/post', + [ + 'type' => ['h-card'], + 'properties' => [ + 'name' => $faker->name, + 'geo' => 'geo:' . $faker->latitude . ',' . $faker->longitude + ], + ], + ['HTTP_Authorization' => 'Bearer ' . $this->getToken()] + )->seeJson([ + 'response' => 'created' + ])->assertResponseStatus(201); + } + + public function testMicropubJSONRequestCreateNewPlaceWithoutToken() + { + $faker = \Faker\Factory::create(); + $this->json( + 'POST', + $this->appurl . '/api/post', + [ + 'type' => ['h-entry'], + 'properties' => [ + 'name' => $faker->name, + 'geo' => 'geo:' . $faker->latitude . ',' . $faker->longitude + ], + ] + )->seeJson([ + 'response' => 'error', + 'error' => 'no_token' + ])->assertResponseStatus(400); + } + + public function testMicropubJSONRequestCreateNewPlaceWithInvalidToken() + { + $faker = \Faker\Factory::create(); + $this->json( + 'POST', + $this->appurl . '/api/post', + [ + 'type' => ['h-entry'], + 'properties' => [ + 'name' => $faker->name, + 'geo' => 'geo:' . $faker->latitude . ',' . $faker->longitude + ], + ], + ['HTTP_Authorization' => 'Bearer ' . $this->getInvalidToken()] + )->seeJson([ + 'response' => 'error', + 'error' => 'invalid_token' ]); } @@ -132,4 +229,18 @@ class MicropubTest extends TestCase return $token; } + + private function getInvalidToken() + { + $signer = new Sha256(); + $token = (new Builder()) + ->set('client_id', 'https://quill.p3k.io') + ->set('me', 'https://jonnybarnes.localhost') + ->set('scope', 'view') + ->set('issued_at', time()) + ->sign($signer, env('APP_KEY')) + ->getToken(); + + return $token; + } }