From 2842e3445fecc4617e7f0041c567d695f94a23a0 Mon Sep 17 00:00:00 2001 From: eileen Date: Mon, 26 Nov 2018 18:13:40 +1300 Subject: [PATCH 1/3] Refine handling of 'card' parameter to support generic integrations When setting up a paypal rest Authorize request there are a few possibilities we have a payment token we have a credit card number & cvv & expiry we have neither of the above and should be redirected to paypal for us to log in and authorize the payment. Currently the code understands the difference as cardReference is present 'card' object is present neither of the above. However, the card object also holds other information - email, address, phone etc. Even if these are not used by Paypal Rest the calling code should not need to know that it can't set these under a specific combination of circumstances. This fix looks more deeply into the passed card parameter for the presence of the actual card fields - ie.card number, expiry - and otherwise concludes this is not a card-present transations and allows the redirect to proceed --- src/Message/RestAuthorizeRequest.php | 31 ++++++++++++++++++++++++++-- 1 file changed, 29 insertions(+), 2 deletions(-) diff --git a/src/Message/RestAuthorizeRequest.php b/src/Message/RestAuthorizeRequest.php index 19762f9..321910e 100644 --- a/src/Message/RestAuthorizeRequest.php +++ b/src/Message/RestAuthorizeRequest.php @@ -5,6 +5,8 @@ namespace Omnipay\PayPal\Message; +use Omnipay\Common\Exception\InvalidCreditCardException; + /** * PayPal REST Authorize Request * @@ -259,9 +261,8 @@ public function getData() 'credit_card_id' => $this->getCardReference(), ), ); - } elseif ($this->getCard()) { + } elseif ($this->validCardPresent()) { $this->validate('amount', 'card'); - $this->getCard()->validate(); $data['payer']['funding_instruments'][] = array( 'credit_card' => array( @@ -304,6 +305,32 @@ public function getData() return $data; } + /** + * Has a valid card been passed in the Omnipay parameters. + * + * Omnipay supports details other than card details in the card parameter (e.g. + * billing address) so a generic omnipay integration might set the 'card' when + * there is no number present. In which case the Rest integration should fall + * back to the next method. + */ + public function validCardPresent() + { + $card = $this->getCard(); + if (!$card) { + return false; + } + try { + $card->validate(); + } catch (InvalidCreditCardException $e) { + if (stristr($e->getMessage(), 'is required')) { + return false; + } else { + throw $e; + } + } + return true; + } + /** * Get the experience profile id * From a7acfe9f1f7ea5d40442c4ef4e69488682c60787 Mon Sep 17 00:00:00 2001 From: eileen Date: Mon, 6 Aug 2018 14:35:19 +1200 Subject: [PATCH 2/3] Add test for Refine handling of 'card' parameter to support generic integrations. In a generic integration the card parameter might exist to hold billing address details even when the card number is not provided. We don't expect the integration to hold knowledge that 'paypal rest won't work if you pass a 'card' paramter when you are pre-authorizing a token' so we should look more deeply into the passed card parameter for the presence of the actual card fields - ie. card number, expiry ,cvv --- tests/Message/RestAuthorizeRequestTest.php | 27 ++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/tests/Message/RestAuthorizeRequestTest.php b/tests/Message/RestAuthorizeRequestTest.php index 5f0f962..ec04f85 100644 --- a/tests/Message/RestAuthorizeRequestTest.php +++ b/tests/Message/RestAuthorizeRequestTest.php @@ -47,6 +47,33 @@ public function testGetDataWithoutCard() $this->assertSame('https://www.example.com/cancel', $data['redirect_urls']['cancel_url']); } + /** + * This tests that having a card object with no card details acts as 'no card'. + * + * We may have a card object holding billing details but no card details. This + * should be treated as a card-not-present rather than as invalid. + */ + public function testGetDataWitLimitedCard() + { + $this->request->setTransactionId('abc123'); + $this->request->setDescription('Sheep'); + $this->request->setCard(new CreditCard(['firstName' => 'Example'])); + + $data = $this->request->getData(); + + $this->assertSame('authorize', $data['intent']); + $this->assertSame('paypal', $data['payer']['payment_method']); + $this->assertSame('10.00', $data['transactions'][0]['amount']['total']); + $this->assertSame('USD', $data['transactions'][0]['amount']['currency']); + $this->assertSame('abc123 : Sheep', $data['transactions'][0]['description']); + + // Funding instruments must not be set, otherwise paypal API will give error 500. + $this->assertArrayNotHasKey('funding_instruments', $data['payer']); + + $this->assertSame('https://www.example.com/return', $data['redirect_urls']['return_url']); + $this->assertSame('https://www.example.com/cancel', $data['redirect_urls']['cancel_url']); + } + public function testGetDataWithCard() { $card = new CreditCard($this->getValidCard()); From 537aff3a4144b5cb6d5a47ee5c0bfe1a1c103009 Mon Sep 17 00:00:00 2001 From: eileen Date: Mon, 10 Dec 2018 14:24:34 +1300 Subject: [PATCH 3/3] Add fix for error 'Missing Header delimiter' Error is caused by changes in dependencies & version change on psr parse function --- tests/Mock/RestGenericSubscriptionSuccess.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/Mock/RestGenericSubscriptionSuccess.txt b/tests/Mock/RestGenericSubscriptionSuccess.txt index e099ab1..922bb53 100644 --- a/tests/Mock/RestGenericSubscriptionSuccess.txt +++ b/tests/Mock/RestGenericSubscriptionSuccess.txt @@ -5,3 +5,4 @@ Paypal-Debug-Id: 217a9ddefd384 SERVER_INFO: identitysecuretokenserv:v1.oauth2.token&CalThreadId=91&TopLevelTxnStartTime=146fbfe679a&Host=slcsbidensectoken502.slc.paypal.com&pid=29059 CORRELATION-ID: 217a9ddefd384 Date: Thu, 03 Jul 2014 11:31:32 GMT +