Skip to content

Commit

Permalink
Merge pull request #2794 from Adyen/ECP-9484
Browse files Browse the repository at this point in the history
[ECP-9484] Adobe Commerce solving Security Errors
  • Loading branch information
khushboo-singhvi authored Nov 12, 2024
2 parents 331afcd + 14bcbb1 commit e9d13e1
Show file tree
Hide file tree
Showing 8 changed files with 633 additions and 44 deletions.
61 changes: 31 additions & 30 deletions Controller/Webhook/Index.php
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
use Magento\Framework\App\Action\Action;
use Magento\Framework\App\Action\Context;
use Magento\Framework\App\CsrfAwareActionInterface;
use Magento\Framework\App\Request\Http as Http;
use Magento\Framework\App\Request\Http;
use Magento\Framework\Exception\LocalizedException;
use Magento\Framework\Serialize\SerializerInterface;
use Symfony\Component\Config\Definition\Exception\Exception;
Expand Down Expand Up @@ -89,6 +89,8 @@ class Index extends Action
*/
private $remoteAddress;

private Http $request;

/**
* Json constructor.
*
Expand All @@ -114,7 +116,8 @@ public function __construct(
RateLimiter $rateLimiterHelper,
HmacSignature $hmacSignature,
NotificationReceiver $notificationReceiver,
RemoteAddress $remoteAddress
RemoteAddress $remoteAddress,
Http $request
) {
parent::__construct($context);
$this->notificationFactory = $notificationFactory;
Expand All @@ -127,6 +130,7 @@ public function __construct(
$this->hmacSignature = $hmacSignature;
$this->notificationReceiver = $notificationReceiver;
$this->remoteAddress = $remoteAddress;
$this->request = $request;

// Fix for Magento2.3 adding isAjax to the request params
if (interface_exists(CsrfAwareActionInterface::class)) {
Expand Down Expand Up @@ -376,36 +380,33 @@ private function isDuplicate(array $response)
*/
private function fixCgiHttpAuthentication()
{
// do nothing if values are already there
// Exit if authentication values are already set
if (!empty($_SERVER['PHP_AUTH_USER']) && !empty($_SERVER['PHP_AUTH_PW'])) {
return;
} elseif (isset($_SERVER['REDIRECT_REMOTE_AUTHORIZATION']) &&
$_SERVER['REDIRECT_REMOTE_AUTHORIZATION'] != ''
) {
list(
$_SERVER['PHP_AUTH_USER'], $_SERVER['PHP_AUTH_PW']
) =
explode(':', base64_decode((string) $_SERVER['REDIRECT_REMOTE_AUTHORIZATION']), 2);
} elseif (!empty($_SERVER['REDIRECT_HTTP_AUTHORIZATION'])) {
list(
$_SERVER['PHP_AUTH_USER'], $_SERVER['PHP_AUTH_PW']
) =
explode(':', base64_decode(substr((string) $_SERVER['REDIRECT_HTTP_AUTHORIZATION'], 6)), 2);
} elseif (!empty($_SERVER['HTTP_AUTHORIZATION'])) {
list(
$_SERVER['PHP_AUTH_USER'], $_SERVER['PHP_AUTH_PW']
) =
explode(':', base64_decode(substr((string) $_SERVER['HTTP_AUTHORIZATION'], 6)), 2);
} elseif (!empty($_SERVER['REMOTE_USER'])) {
list(
$_SERVER['PHP_AUTH_USER'], $_SERVER['PHP_AUTH_PW']
) =
explode(':', base64_decode(substr((string) $_SERVER['REMOTE_USER'], 6)), 2);
} elseif (!empty($_SERVER['REDIRECT_REMOTE_USER'])) {
list(
$_SERVER['PHP_AUTH_USER'], $_SERVER['PHP_AUTH_PW']
) =
explode(':', base64_decode(substr((string) $_SERVER['REDIRECT_REMOTE_USER'], 6)), 2);
}

// Define potential authorization headers to check
$authHeaders = [
'REDIRECT_REMOTE_AUTHORIZATION',
'REDIRECT_HTTP_AUTHORIZATION',
'HTTP_AUTHORIZATION',
'REMOTE_USER',
'REDIRECT_REMOTE_USER'
];

// Check each header, decode and assign credentials if found
foreach ($authHeaders as $header) {
if (!empty($_SERVER[$header])) {
$authValue = $_SERVER[$header];

// Remove 'Basic ' prefix if present
if (str_starts_with($authValue, 'Basic ')) {
$authValue = substr($authValue, 6);
}

list($_SERVER['PHP_AUTH_USER'], $_SERVER['PHP_AUTH_PW']) = explode(':', base64_decode($authValue), 2);
return;
}
}
}

Expand Down
19 changes: 13 additions & 6 deletions Helper/Requests.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
use Adyen\Payment\Model\Ui\AdyenPayByLinkConfigProvider;
use Adyen\Util\Uuid;
use Magento\Framework\App\Helper\AbstractHelper;
use Magento\Framework\App\Request\Http as Http;

class Requests extends AbstractHelper
{
Expand All @@ -36,19 +37,22 @@ class Requests extends AbstractHelper
private Address $addressHelper;
private StateData $stateData;
private Vault $vaultHelper;
private Http $request;

public function __construct(
Data $adyenHelper,
Config $adyenConfig,
Address $addressHelper,
StateData $stateData,
Vault $vaultHelper
Vault $vaultHelper,
Http $request
) {
$this->adyenHelper = $adyenHelper;
$this->adyenConfig = $adyenConfig;
$this->addressHelper = $addressHelper;
$this->stateData = $stateData;
$this->vaultHelper = $vaultHelper;
$this->request = $request;
}

/**
Expand Down Expand Up @@ -294,14 +298,17 @@ public function buildPaymentData($amount, $currencyCode, $reference, array $requ
* @param array $request
* @return array
*/
public function buildBrowserData($request = [])
public function buildBrowserData(array $request = []): array
{
if (!empty($_SERVER['HTTP_USER_AGENT'])) {
$request['browserInfo']['userAgent'] = $_SERVER['HTTP_USER_AGENT'];
$userAgent = $this->request->getServer('HTTP_USER_AGENT');
$acceptHeader = $this->request->getServer('HTTP_ACCEPT');

if (!empty($userAgent)) {
$request['browserInfo']['userAgent'] = $userAgent;
}

if (!empty($_SERVER['HTTP_ACCEPT'])) {
$request['browserInfo']['acceptHeader'] = $_SERVER['HTTP_ACCEPT'];
if (!empty($acceptHeader)) {
$request['browserInfo']['acceptHeader'] = $acceptHeader;
}

return $request;
Expand Down
104 changes: 102 additions & 2 deletions Test/Unit/Controller/Webhook/IndexTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
use Adyen\Payment\Model\Notification;
use Adyen\Webhook\Receiver\HmacSignature;
use Adyen\Webhook\Receiver\NotificationReceiver;
use Magento\Framework\App\Request\Http as Http;
use Magento\Framework\HTTP\PhpEnvironment\RemoteAddress;
use Magento\Framework\App\Action\Context;
use Magento\Framework\App\RequestInterface;
Expand Down Expand Up @@ -78,6 +79,9 @@ protected function setUp(): void
$this->contextMock = $this->getMockBuilder(Context::class)
->disableOriginalConstructor()
->getMock();
$this->httpMock = $this->getMockBuilder(http::class)
->disableOriginalConstructor()
->getMock();
$this->requestMock = $this->getMockBuilder(RequestInterface::class)
->getMockForAbstractClass();
$this->responseMock = $this->getMockBuilder(ResponseInterface::class)
Expand Down Expand Up @@ -136,7 +140,8 @@ protected function setUp(): void
$this->rateLimiterHelperMock,
$this->hmacSignatureMock,
$this->notificationReceiverMock,
$this->remoteAddressMock
$this->remoteAddressMock,
$this->httpMock
);
}

Expand All @@ -152,6 +157,101 @@ public function testLoadNotificationFromRequest()
'loadNotificationFromRequest',
[$notificationMock, []]
);
}

protected function tearDown(): void
{
// Reset $_SERVER global after each test
$_SERVER = [];
}

public function testFixCgiHttpAuthenticationWithExistingAuth()
{
$_SERVER['PHP_AUTH_USER'] = 'existingUser';
$_SERVER['PHP_AUTH_PW'] = 'existingPassword';

$this->invokeMethod(
$this->indexController,
'fixCgiHttpAuthentication'
);

$this->assertEquals('existingUser', $_SERVER['PHP_AUTH_USER']);
$this->assertEquals('existingPassword', $_SERVER['PHP_AUTH_PW']);
}

public function testFixCgiHttpAuthenticationWithRedirectRemoteAuthorization()
{
$_SERVER['REDIRECT_REMOTE_AUTHORIZATION'] = 'Basic ' . base64_encode('testUser:testPassword');

$this->invokeMethod(
$this->indexController,
'fixCgiHttpAuthentication'
);

$this->assertEquals('testUser', $_SERVER['PHP_AUTH_USER']);
$this->assertEquals('testPassword', $_SERVER['PHP_AUTH_PW']);
}

public function testFixCgiHttpAuthenticationWithRedirectHttpAuthorization()
{
$_SERVER['REDIRECT_HTTP_AUTHORIZATION'] = 'Basic ' . base64_encode('testUser:testPassword');

$this->invokeMethod(
$this->indexController,
'fixCgiHttpAuthentication'
);

$this->assertEquals('testUser', $_SERVER['PHP_AUTH_USER']);
$this->assertEquals('testPassword', $_SERVER['PHP_AUTH_PW']);
}

public function testFixCgiHttpAuthenticationWithHttpAuthorization()
{
$_SERVER['HTTP_AUTHORIZATION'] = 'Basic ' . base64_encode('testUser:testPassword');

$this->invokeMethod(
$this->indexController,
'fixCgiHttpAuthentication'
);

$this->assertEquals('testUser', $_SERVER['PHP_AUTH_USER']);
$this->assertEquals('testPassword', $_SERVER['PHP_AUTH_PW']);
}

public function testFixCgiHttpAuthenticationWithRemoteUser()
{
$_SERVER['REMOTE_USER'] = 'Basic ' . base64_encode('testUser:testPassword');

$this->invokeMethod(
$this->indexController,
'fixCgiHttpAuthentication'
);

$this->assertEquals('testUser', $_SERVER['PHP_AUTH_USER']);
$this->assertEquals('testPassword', $_SERVER['PHP_AUTH_PW']);
}

public function testFixCgiHttpAuthenticationWithRedirectRemoteUser()
{
$_SERVER['REDIRECT_REMOTE_USER'] = 'Basic ' . base64_encode('testUser:testPassword');

$this->invokeMethod(
$this->indexController,
'fixCgiHttpAuthentication'
);

$this->assertEquals('testUser', $_SERVER['PHP_AUTH_USER']);
$this->assertEquals('testPassword', $_SERVER['PHP_AUTH_PW']);
}

public function testFixCgiHttpAuthenticationWithNoAuthorizationHeaders()
{
$this->invokeMethod(
$this->indexController,
'fixCgiHttpAuthentication'
);

$this->assertArrayNotHasKey('PHP_AUTH_USER', $_SERVER);
$this->assertArrayNotHasKey('PHP_AUTH_PW', $_SERVER);
}
}
}
Loading

0 comments on commit e9d13e1

Please sign in to comment.