Skip to content

Commit a3d48ec

Browse files
lozcalverlozcalver
committed
[CVE-2022-29254] Add extra validation on payment completion
1 parent 79253b1 commit a3d48ec

8 files changed

+106
-86
lines changed

‎src/Service/AuthorizeService.php

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2,15 +2,15 @@
22

33
namespace SilverStripe\Omnipay\Service;
44

5-
use SilverStripe\Omnipay\Exception\InvalidStateException;
65
use SilverStripe\Omnipay\Exception\InvalidConfigurationException;
6+
use SilverStripe\Omnipay\Exception\InvalidStateException;
77
use SilverStripe\Omnipay\Helper\ErrorHandling;
8-
use SilverStripe\Omnipay\Model\Message\AuthorizeRequest;
98
use SilverStripe\Omnipay\Model\Message\AuthorizedResponse;
10-
use SilverStripe\Omnipay\Model\Message\CompleteAuthorizeError;
119
use SilverStripe\Omnipay\Model\Message\AuthorizeError;
1210
use SilverStripe\Omnipay\Model\Message\AuthorizeRedirectResponse;
11+
use SilverStripe\Omnipay\Model\Message\AuthorizeRequest;
1312
use SilverStripe\Omnipay\Model\Message\AwaitingAuthorizeResponse;
13+
use SilverStripe\Omnipay\Model\Message\CompleteAuthorizeError;
1414
use SilverStripe\Omnipay\Model\Message\CompleteAuthorizeRequest;
1515

1616
class AuthorizeService extends PaymentService
@@ -67,7 +67,7 @@ public function initiate($data = array())
6767
);
6868
} elseif ($serviceResponse->isError()) {
6969
$this->createMessage(AuthorizeError::class, $response);
70-
} else {
70+
} elseif ($serviceResponse->isSuccessful()) {
7171
$this->markCompleted('Authorized', $serviceResponse, $response);
7272
}
7373

@@ -118,15 +118,12 @@ public function complete($data = array(), $isNotification = false)
118118

119119
$serviceResponse = $this->wrapOmnipayResponse($response, $isNotification);
120120

121-
if ($serviceResponse->isError()) {
121+
if ($serviceResponse->isAwaitingNotification()) {
122+
ErrorHandling::safeExtend($this->payment, 'onAwaitingAuthorized', $serviceResponse);
123+
} elseif ($serviceResponse->isError()) {
122124
$this->createMessage(CompleteAuthorizeError::class, $response);
123-
return $serviceResponse;
124-
}
125-
126-
if (!$serviceResponse->isAwaitingNotification()) {
125+
} elseif ($serviceResponse->isSuccessful()) {
127126
$this->markCompleted('Authorized', $serviceResponse, $response);
128-
} else {
129-
ErrorHandling::safeExtend($this->payment, 'onAwaitingAuthorized', $serviceResponse);
130127
}
131128

132129
return $serviceResponse;

‎src/Service/CaptureService.php

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -138,25 +138,23 @@ public function initiate($data = array())
138138

139139
$serviceResponse = $this->wrapOmnipayResponse($response);
140140

141-
if ($serviceResponse->isAwaitingNotification()) {
141+
if ($serviceResponse->isError()) {
142+
$this->createMessage($this->errorMessageType, $response);
143+
} elseif ($serviceResponse->isRedirect() || $serviceResponse->isAwaitingNotification()) {
142144
if ($diff < 0) {
143145
$this->createPartialPayment(PaymentMath::multiply($amount, '-1'), $this->pendingState);
144146
} elseif ($diff > 0) {
145147
$this->createPartialPayment($diff, $this->pendingState);
146148
}
147149
$this->payment->Status = $this->pendingState;
148150
$this->payment->write();
149-
} else {
150-
if ($serviceResponse->isError()) {
151-
$this->createMessage($this->errorMessageType, $response);
152-
} else {
153-
if ($diff < 0) {
154-
$this->createPartialPayment(PaymentMath::multiply($amount, '-1'), $this->pendingState);
155-
} elseif ($diff > 0) {
156-
$this->createPartialPayment($diff, $this->pendingState);
157-
}
158-
$this->markCompleted($this->endState, $serviceResponse, $response);
151+
} elseif ($serviceResponse->isSuccessful()) {
152+
if ($diff < 0) {
153+
$this->createPartialPayment(PaymentMath::multiply($amount, '-1'), $this->pendingState);
154+
} elseif ($diff > 0) {
155+
$this->createPartialPayment($diff, $this->pendingState);
159156
}
157+
$this->markCompleted($this->endState, $serviceResponse, $response);
160158
}
161159

162160
return $serviceResponse;

‎src/Service/CreateCardService.php

Lines changed: 21 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -3,14 +3,19 @@
33
namespace SilverStripe\Omnipay\Service;
44

55
use Omnipay\Common\Message\RequestInterface;
6-
use SilverStripe\Omnipay\Exception\InvalidStateException;
76
use SilverStripe\Omnipay\Exception\InvalidConfigurationException;
7+
use SilverStripe\Omnipay\Exception\InvalidStateException;
88
use SilverStripe\Omnipay\Helper\ErrorHandling;
9-
use SilverStripe\Omnipay\Model\Message;
9+
use SilverStripe\Omnipay\Model\Message\AwaitingCreateCardResponse;
10+
use SilverStripe\Omnipay\Model\Message\CompleteCreateCardError;
11+
use SilverStripe\Omnipay\Model\Message\CompleteCreateCardRequest;
12+
use SilverStripe\Omnipay\Model\Message\CreateCardError;
13+
use SilverStripe\Omnipay\Model\Message\CreateCardRedirectResponse;
14+
use SilverStripe\Omnipay\Model\Message\CreateCardRequest;
15+
use SilverStripe\Omnipay\Model\Message\CreateCardResponse;
1016

1117
class CreateCardService extends PaymentService
1218
{
13-
1419
/**
1520
* Start a createcard request
1621
*
@@ -39,12 +44,12 @@ public function initiate($data = array())
3944
$request = $this->oGateway()->createCard($gatewayData);
4045
$this->extend('onAfterCreateCard', $request);
4146

42-
$this->createMessage(Message\CreateCardRequest::class, $request);
47+
$this->createMessage(CreateCardRequest::class, $request);
4348

4449
try {
4550
$response = $this->response = $request->send();
4651
} catch (\Omnipay\Common\Exception\OmnipayException $e) {
47-
$this->createMessage(Message\CreateCardError::class, $e);
52+
$this->createMessage(CreateCardError::class, $e);
4853
// create an error response
4954
return $this->generateServiceResponse(ServiceResponse::SERVICE_ERROR);
5055
}
@@ -58,12 +63,12 @@ public function initiate($data = array())
5863
$this->payment->write();
5964

6065
$this->createMessage(
61-
$serviceResponse->isRedirect() ? Message\CreateCardRedirectResponse::class : Message\AwaitingCreateCardResponse::class,
66+
$serviceResponse->isRedirect() ? CreateCardRedirectResponse::class : AwaitingCreateCardResponse::class,
6267
$response
6368
);
6469
} elseif ($serviceResponse->isError()) {
65-
$this->createMessage(Message\CreateCardError::class, $response);
66-
} else {
70+
$this->createMessage(CreateCardError::class, $response);
71+
} elseif ($serviceResponse->isSuccessful()) {
6772
$this->markCompleted('CardCreated', $serviceResponse, $response);
6873
}
6974

@@ -103,26 +108,23 @@ public function complete($data = array(), $isNotification = false)
103108
$request = $gateway->completeCreateCard($gatewayData);
104109
$this->extend('onAfterCompleteCreateCard', $request);
105110

106-
$this->createMessage(Message\CompleteCreateCardRequest::class, $request);
111+
$this->createMessage(CompleteCreateCardRequest::class, $request);
107112
$response = null;
108113
try {
109114
$response = $this->response = $request->send();
110115
} catch (\Omnipay\Common\Exception\OmnipayException $e) {
111-
$this->createMessage(Message\CompleteCreateCardError::class, $e);
116+
$this->createMessage(CompleteCreateCardError::class, $e);
112117
return $this->generateServiceResponse($flags | ServiceResponse::SERVICE_ERROR);
113118
}
114119

115120
$serviceResponse = $this->wrapOmnipayResponse($response, $isNotification);
116121

117-
if ($serviceResponse->isError()) {
118-
$this->createMessage(Message\CompleteCreateCardError::class, $response);
119-
return $serviceResponse;
120-
}
121-
122-
if (!$serviceResponse->isAwaitingNotification()) {
123-
$this->markCompleted('CardCreated', $serviceResponse, $response);
124-
} else {
122+
if ($serviceResponse->isAwaitingNotification()) {
125123
ErrorHandling::safeExtend($this->payment, 'onAwaitingCreateCard', $serviceResponse);
124+
} elseif ($serviceResponse->isError()) {
125+
$this->createMessage(CompleteCreateCardError::class, $response);
126+
} elseif ($serviceResponse->isSuccessful()) {
127+
$this->markCompleted('CardCreated', $serviceResponse, $response);
126128
}
127129

128130
return $serviceResponse;
@@ -131,7 +133,7 @@ public function complete($data = array(), $isNotification = false)
131133
protected function markCompleted($endStatus, ServiceResponse $serviceResponse, $gatewayMessage)
132134
{
133135
parent::markCompleted($endStatus, $serviceResponse, $gatewayMessage);
134-
$this->createMessage(Message\CreateCardResponse::class, $gatewayMessage);
136+
$this->createMessage(CreateCardResponse::class, $gatewayMessage);
135137
ErrorHandling::safeExtend($this->payment, 'onCardCreated', $serviceResponse);
136138
}
137139
}

‎src/Service/NotificationCompleteService.php

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,10 @@ public function complete($data = array(), $isNotification = true)
7979
}
8080

8181
// check if we're done
82-
if (!$serviceResponse->isError() && !$serviceResponse->isAwaitingNotification()) {
82+
if (!$serviceResponse->isError()
83+
&& !$serviceResponse->isAwaitingNotification()
84+
&& $serviceResponse->isSuccessful()
85+
) {
8386
$this->markCompleted($this->endState, $serviceResponse, $serviceResponse->getOmnipayResponse());
8487
}
8588

‎src/Service/PurchaseService.php

Lines changed: 20 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,13 @@
55
use SilverStripe\Omnipay\Exception\InvalidStateException;
66
use SilverStripe\Omnipay\Exception\InvalidConfigurationException;
77
use SilverStripe\Omnipay\Helper\ErrorHandling;
8-
use SilverStripe\Omnipay\Model\Message;
8+
use SilverStripe\Omnipay\Model\Message\AwaitingPurchaseResponse;
9+
use SilverStripe\Omnipay\Model\Message\CompletePurchaseError;
10+
use SilverStripe\Omnipay\Model\Message\CompletePurchaseRequest;
11+
use SilverStripe\Omnipay\Model\Message\PurchasedResponse;
12+
use SilverStripe\Omnipay\Model\Message\PurchaseError;
13+
use SilverStripe\Omnipay\Model\Message\PurchaseRedirectResponse;
14+
use SilverStripe\Omnipay\Model\Message\PurchaseRequest;
915

1016
class PurchaseService extends PaymentService
1117
{
@@ -43,12 +49,12 @@ public function initiate($data = array())
4349
$request = $this->oGateway()->purchase($gatewayData);
4450
$this->extend('onAfterPurchase', $request);
4551

46-
$this->createMessage(Message\PurchaseRequest::class, $request);
52+
$this->createMessage(PurchaseRequest::class, $request);
4753

4854
try {
4955
$response = $this->response = $request->send();
5056
} catch (\Omnipay\Common\Exception\OmnipayException $e) {
51-
$this->createMessage(Message\PurchaseError::class, $e);
57+
$this->createMessage(PurchaseError::class, $e);
5258
// create an error response
5359
return $this->generateServiceResponse(ServiceResponse::SERVICE_ERROR);
5460
}
@@ -62,12 +68,12 @@ public function initiate($data = array())
6268
$this->payment->write();
6369

6470
$this->createMessage(
65-
$serviceResponse->isRedirect() ? Message\PurchaseRedirectResponse::class : Message\AwaitingPurchaseResponse::class,
71+
$serviceResponse->isRedirect() ? PurchaseRedirectResponse::class : AwaitingPurchaseResponse::class,
6672
$response
6773
);
6874
} elseif ($serviceResponse->isError()) {
69-
$this->createMessage(Message\PurchaseError::class, $response);
70-
} else {
75+
$this->createMessage(PurchaseError::class, $response);
76+
} elseif ($serviceResponse->isSuccessful()) {
7177
$this->markCompleted('Captured', $serviceResponse, $response);
7278
}
7379

@@ -105,36 +111,32 @@ public function complete($data = array(), $isNotification = false)
105111
$request = $gateway->completePurchase($gatewayData);
106112
$this->extend('onAfterCompletePurchase', $request);
107113

108-
$this->createMessage(Message\CompletePurchaseRequest::class, $request);
114+
$this->createMessage(CompletePurchaseRequest::class, $request);
109115
$response = null;
110116
try {
111117
$response = $this->response = $request->send();
112118
} catch (\Omnipay\Common\Exception\OmnipayException $e) {
113-
$this->createMessage(Message\CompletePurchaseError::class, $e);
119+
$this->createMessage(CompletePurchaseError::class, $e);
114120
return $this->generateServiceResponse($flags | ServiceResponse::SERVICE_ERROR);
115121
}
116122

117123
$serviceResponse = $this->wrapOmnipayResponse($response, $isNotification);
118-
if ($serviceResponse->isError()) {
119-
$this->createMessage(Message\CompletePurchaseError::class, $response);
120-
return $serviceResponse;
121-
}
122124

123-
// only update payment status if we're not waiting for a notification
124-
if (!$serviceResponse->isAwaitingNotification()) {
125-
$this->markCompleted('Captured', $serviceResponse, $response);
126-
} else {
125+
if ($serviceResponse->isAwaitingNotification()) {
127126
ErrorHandling::safeExtend($this->payment, 'onAwaitingCaptured', $serviceResponse);
127+
} elseif ($serviceResponse->isError()) {
128+
$this->createMessage(CompletePurchaseError::class, $response);
129+
} elseif ($serviceResponse->isSuccessful()) {
130+
$this->markCompleted('Captured', $serviceResponse, $response);
128131
}
129132

130-
131133
return $serviceResponse;
132134
}
133135

134136
protected function markCompleted($endStatus, ServiceResponse $serviceResponse, $gatewayMessage)
135137
{
136138
parent::markCompleted($endStatus, $serviceResponse, $gatewayMessage);
137-
$this->createMessage(Message\PurchasedResponse::class, $gatewayMessage);
139+
$this->createMessage(PurchasedResponse::class, $gatewayMessage);
138140
ErrorHandling::safeExtend($this->payment, 'onCaptured', $serviceResponse);
139141
}
140142
}

‎src/Service/RefundService.php

Lines changed: 16 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,10 @@
99
use SilverStripe\Omnipay\GatewayInfo;
1010
use SilverStripe\Omnipay\Helper\ErrorHandling;
1111
use SilverStripe\Omnipay\Helper\PaymentMath;
12-
use SilverStripe\Omnipay\Model\Message;
12+
use SilverStripe\Omnipay\Model\Message\PartiallyRefundedResponse;
13+
use SilverStripe\Omnipay\Model\Message\RefundedResponse;
14+
use SilverStripe\Omnipay\Model\Message\RefundError;
15+
use SilverStripe\Omnipay\Model\Message\RefundRequest;
1316
use SilverStripe\Omnipay\Model\Payment;
1417

1518
class RefundService extends NotificationCompleteService
@@ -20,9 +23,9 @@ class RefundService extends NotificationCompleteService
2023

2124
protected $pendingState = 'PendingRefund';
2225

23-
protected $requestMessageType = Message\RefundRequest::class;
26+
protected $requestMessageType = RefundRequest::class;
2427

25-
protected $errorMessageType = Message\RefundError::class;
28+
protected $errorMessageType = RefundError::class;
2629

2730
/**
2831
* Return money to the previously charged credit card.
@@ -127,21 +130,20 @@ public function initiate($data = array())
127130

128131
$serviceResponse = $this->wrapOmnipayResponse($response);
129132

130-
if ($serviceResponse->isAwaitingNotification()) {
133+
if ($serviceResponse->isError()) {
134+
$this->createMessage($this->errorMessageType, $response);
135+
} elseif ($serviceResponse->isRedirect() || $serviceResponse->isAwaitingNotification()) {
131136
if ($isPartial) {
132137
$this->createPartialPayment(PaymentMath::multiply($amount, '-1'), $this->pendingState);
133138
}
134139
$this->payment->Status = $this->pendingState;
135140
$this->payment->write();
136-
} else {
137-
if ($serviceResponse->isError()) {
138-
$this->createMessage($this->errorMessageType, $response);
139-
} else {
140-
if ($isPartial) {
141-
$this->createPartialPayment(PaymentMath::multiply($amount, '-1'), $this->pendingState);
142-
}
143-
$this->markCompleted($this->endState, $serviceResponse, $response);
141+
} elseif ($serviceResponse->isSuccessful()) {
142+
if ($isPartial) {
143+
$this->createPartialPayment(PaymentMath::multiply($amount, '-1'), $this->pendingState);
144144
}
145+
146+
$this->markCompleted($this->endState, $serviceResponse, $response);
145147
}
146148

147149
return $serviceResponse;
@@ -182,9 +184,9 @@ protected function markCompleted($endStatus, ServiceResponse $serviceResponse, $
182184

183185
parent::markCompleted($endStatus, $serviceResponse, $gatewayMessage);
184186
if ($endStatus === 'Captured') {
185-
$this->createMessage(Message\PartiallyRefundedResponse::class, $gatewayMessage);
187+
$this->createMessage(PartiallyRefundedResponse::class, $gatewayMessage);
186188
} else {
187-
$this->createMessage(Message\RefundedResponse::class, $gatewayMessage);
189+
$this->createMessage(RefundedResponse::class, $gatewayMessage);
188190
}
189191

190192
ErrorHandling::safeExtend($this->payment, 'onRefunded', $serviceResponse);

‎src/Service/ServiceResponse.php

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,22 @@ public function getPayment()
9797
return $this->payment;
9898
}
9999

100+
/**
101+
* Whether the response is marked as successful by Omnipay.
102+
*
103+
* @return bool
104+
*/
105+
public function isSuccessful()
106+
{
107+
if ($this->omnipayResponse instanceof NotificationInterface) {
108+
return $this->omnipayResponse->getTransactionStatus() === NotificationInterface::STATUS_COMPLETED;
109+
} elseif ($this->omnipayResponse instanceof AbstractResponse) {
110+
return $this->omnipayResponse->isSuccessful();
111+
}
112+
113+
return false;
114+
}
115+
100116
/**
101117
* Whether or not this is an *offsite* redirect.
102118
* This is only the case when there's an Omnipay response present that *is* a redirect.

0 commit comments

Comments
 (0)