From 58d510dfe2e0140c887f5c56263d0841c8da9918 Mon Sep 17 00:00:00 2001 From: Jorge Date: Wed, 26 Apr 2017 17:09:25 +0200 Subject: [PATCH 1/2] [ADDED] If transaction's signature data receives an input of all zeroes, skips sending an SMS. --- .../Controllers/TransactionsController.php | 54 ++++++++--- tests/Feature/TransactionsControllerTest.php | 95 ++++++++++++++++++- 2 files changed, 132 insertions(+), 17 deletions(-) diff --git a/app/Http/Controllers/TransactionsController.php b/app/Http/Controllers/TransactionsController.php index e97bb0b..cb8d642 100644 --- a/app/Http/Controllers/TransactionsController.php +++ b/app/Http/Controllers/TransactionsController.php @@ -21,8 +21,6 @@ class TransactionsController extends AuthController { protected $smsProvider; - - /** * TransactionsController constructor. */ @@ -210,20 +208,48 @@ public function signaturePositions($id) { */ public function signatureOtp(Request $request, $id) { - $current_user = $this->getUserFromToken(); - $transaction = Transaction::where('id', $id)->first(); - if ($transaction == null) { - return $this->response->errorNotFound("Transaction does not exist"); - } - if (strcmp($current_user->id, $transaction->user_id) != 0) { - return $this->response->errorForbidden("User does not have permissions to access this transaction"); + try { + $rules = [ + 'signatureData' => 'required', + 'signaturePositions' => 'required' + ]; + $v = Validator::make($request->all(), $rules); + if ($v->fails()) { + throw new BadRequestHttpException($v->getMessageBag()->first()); + } + + $current_user = $this->getUserFromToken(); + $transaction = Transaction::where('id', $id)->first(); + if ($transaction == null) { + throw new ModelNotFoundException("Transaction does not exist"); + } + if (strcmp($current_user->id, $transaction->user_id) != 0) { + throw new UnauthorizedException("User does not have permissions to access this transaction"); + } + if ($this->in_array_all(0, $request['signatureData'])) { + $ticket = 'test'; + } + else { + $ticket = $this->generateRandomString(); + } + $transaction->ticket_otp = $ticket; + $transaction->save(); + + if (strcmp($ticket, "test") != 0) + $this->smsProvider->send("Your verification code is " . $ticket, $current_user->phone); + return ['ticket' => $ticket]; + } catch (BadRequestHttpException $e) { + return $this->response->errorBadRequest($e->getMessage()); + } catch (ModelNotFoundException $e) { + return $this->response->errorNotFound($e->getMessage()); + } catch (UnauthorizedException $e) { + return $this->response->errorForbidden($e->getMessage()); } - $ticket = $this->generateRandomString(); - $transaction->ticket_otp = $ticket; - $transaction->save(); + } - $this->smsProvider->send("Your verification code is " . $ticket, $current_user->phone); - return ['ticket' => $ticket]; + function in_array_all($value, $array) + { + return (reset($array) == $value && count(array_unique($array)) == 1); } /** diff --git a/tests/Feature/TransactionsControllerTest.php b/tests/Feature/TransactionsControllerTest.php index c3c219e..5d3cc50 100644 --- a/tests/Feature/TransactionsControllerTest.php +++ b/tests/Feature/TransactionsControllerTest.php @@ -851,7 +851,10 @@ public function given_nonExistingTransaction_When_signatureOtp_Then_Returns404() $user = factory(\App\User::class)->create(); // Act - $result = $this->post('/api/transactions/50/signature_otp', [], $this->headers($user)); + $result = $this->post('/api/transactions/50/signature_otp', [ + 'signatureData' => [1, 2, 3], + 'signaturePositions' => [1, 2, 3] + ], $this->headers($user)); // Assert $result->seeStatusCode(404) @@ -873,12 +876,61 @@ public function given_transactionNotPerformedByCurrentUser_When_signatureOtp_The ]); // Act - $result = $this->post('/api/transactions/' . $transaction->id . '/signature_otp', [], $this->headers($current_user)); + $result = $this->post('/api/transactions/' . $transaction->id . '/signature_otp', [ + 'signatureData' => [1, 2, 3], + 'signaturePositions' => [1, 2, 3] + ], $this->headers($current_user)); $result->seeStatusCode(403) ->seeText("User does not have permissions to access this transaction"); } + /** + * @test + * If no signatureData specified, return bad request + */ + public function given_missingSignatureData_When_signatureOtp_Then_ReturnsBadRequest() { + $user = factory(\App\User::class)->create([ + 'phone' => '+34123456789' + ]); + $agent = factory(Agent::class)->create(); + $transaction = factory(\App\Transaction::class)->create([ + 'user_id' => $user->id, + 'agent_destination' => $agent->id + ]); + + // Act + $result = $this->post('/api/transactions/' . $transaction->id . '/signature_otp', [ + 'signaturePositions' => [1, 2, 3] + ], $this->headers($user)); + + // Assert + $result->seeStatusCode(400); + } + + /** + * @test + * If no signaturePositions specified, return bad request + */ + public function given_missingSignaturePositions_When_signatureOtp_Then_ReturnsBadRequest() { + $user = factory(\App\User::class)->create([ + 'phone' => '+34123456789' + ]); + $agent = factory(Agent::class)->create(); + $transaction = factory(\App\Transaction::class)->create([ + 'user_id' => $user->id, + 'agent_destination' => $agent->id + ]); + + // Act + $result = $this->post('/api/transactions/' . $transaction->id . '/signature_otp', [ + 'signatureData' => [1, 2, 3] + ], $this->headers($user)); + + // Assert + $result->seeStatusCode(400); + } + /** * @test * Confirming transactions performed by current user is allowed @@ -895,7 +947,10 @@ public function given_validTransaction_When_signatureOtp_Then_Returns202() { ]); // Act - $result = $this->post('/api/transactions/' . $transaction->id . '/signature_otp', [], $this->headers($user)); + $result = $this->post('/api/transactions/' . $transaction->id . '/signature_otp', [ + 'signatureData' => [1, 2, 3], + 'signaturePositions' => [1, 2, 3] + ], $this->headers($user)); // Assert $result->seeStatusCode(200) @@ -912,6 +967,40 @@ public function given_validTransaction_When_signatureOtp_Then_Returns202() { self::assertEquals($user->phone, $this->smsrepository->requestedDestination); } + /** + * @test + * If receives all zeroes on the payload, then skips sending an SMS + */ + public function given_allZeroesOnPayload_When_signatureOtp_Then_SkipsSMSSending() { + + $user = factory(\App\User::class)->create([ + 'phone' => '+34123456789' + ]); + $agent = factory(Agent::class)->create(); + $transaction = factory(\App\Transaction::class)->create([ + 'user_id' => $user->id, + 'agent_destination' => $agent->id + ]); + + // Act + $result = $this->post('/api/transactions/' . $transaction->id . '/signature_otp', [ + 'signatureData' => [0, 0, 0], + 'signaturePositions' => [1, 2, 3] + ], $this->headers($user)); + + // Assert + $result->seeStatusCode(200) + ->seeJsonStructure(['ticket']); + + // Check a code has been saved for this transaction + $updated_transaction = \App\Transaction::where('id', $transaction->id)->first(); + self::assertNotNull($updated_transaction); + self::assertNotNull($updated_transaction->ticket_otp); + + // Assert SMS was sent + self::assertFalse($this->smsrepository->sendCalled); + } + /** * @test * Cannot confirm SMS if user is not authorized From 41a5c2a053963f5564977c89e4214fb0b39d0e48 Mon Sep 17 00:00:00 2001 From: Jorge Date: Wed, 26 Apr 2017 17:21:08 +0200 Subject: [PATCH 2/2] [FIXED] Lack of env file on repository was causing a complaint from Twilio Client. --- app/Providers/SMSServiceProvider.php | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/app/Providers/SMSServiceProvider.php b/app/Providers/SMSServiceProvider.php index a7c6041..06a60df 100644 --- a/app/Providers/SMSServiceProvider.php +++ b/app/Providers/SMSServiceProvider.php @@ -38,9 +38,8 @@ public function register() $provider = config('SMS_PROVIDER'); } - - $key = env('SMS_API_KEY', null); - $secret = env('SMS_API_SECRET', null); + $key = env('SMS_API_KEY', "foo"); + $secret = env('SMS_API_SECRET', "foosecret"); if (strcmp($provider, 'NEXMO') == 0) { $client = new Nexmo\Client(new Nexmo\Client\Credentials\Basic($key, $secret)); return new NexmoRepository($client);