Skip to content

Commit

Permalink
Merge pull request #71 from Yorxxx/fix-63
Browse files Browse the repository at this point in the history
[ADDED] If transaction's signature data receives an input of all zero…
  • Loading branch information
Jorge authored Apr 26, 2017
2 parents 009dfda + 41a5c2a commit 9b48449
Show file tree
Hide file tree
Showing 3 changed files with 134 additions and 20 deletions.
54 changes: 40 additions & 14 deletions app/Http/Controllers/TransactionsController.php
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,6 @@ class TransactionsController extends AuthController
{
protected $smsProvider;



/**
* TransactionsController constructor.
*/
Expand Down Expand Up @@ -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);
}

/**
Expand Down
5 changes: 2 additions & 3 deletions app/Providers/SMSServiceProvider.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
95 changes: 92 additions & 3 deletions tests/Feature/TransactionsControllerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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
Expand All @@ -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)
Expand All @@ -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
Expand Down

0 comments on commit 9b48449

Please sign in to comment.