Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ECP-9484] Adobe Commerce solving Security Errors #2794

Merged
merged 21 commits into from
Nov 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
b28cdc0
Adobe Commerce solving Security Errors
khushboo-singhvi Nov 1, 2024
a14676e
Adobe Commerce solving Security Errors
khushboo-singhvi Nov 1, 2024
da9b09c
Merge branch 'main' into ECP-9484
khushboo-singhvi Nov 1, 2024
7f3e810
Updating unit tests
khushboo-singhvi Nov 1, 2024
a4e8710
Merge remote-tracking branch 'origin/ECP-9484' into ECP-9484
khushboo-singhvi Nov 1, 2024
181ba53
Updating unit test coverage
khushboo-singhvi Nov 1, 2024
1ab7d67
Updating unit test
khushboo-singhvi Nov 4, 2024
521edd5
Merge branch 'main' into ECP-9484
khushboo-singhvi Nov 4, 2024
dfe47a5
Updating unit test
khushboo-singhvi Nov 4, 2024
876514c
Merge remote-tracking branch 'origin/ECP-9484' into ECP-9484
khushboo-singhvi Nov 4, 2024
7ad273a
Updating unit test for Index.php with more case scenarios
khushboo-singhvi Nov 5, 2024
7c271c1
Merge branch 'main' into ECP-9484
khushboo-singhvi Nov 5, 2024
b48286f
Updating unit test for Index.php with more case scenarios
khushboo-singhvi Nov 5, 2024
7af26f9
Merge branch 'main' into ECP-9484
khushboo-singhvi Nov 5, 2024
3386fc2
Merge branch 'main' into ECP-9484
candemiralp Nov 6, 2024
2707735
Adding more Unit tests
khushboo-singhvi Nov 7, 2024
15a7e89
Resolving merge conflicts
khushboo-singhvi Nov 7, 2024
f113345
Merge branch 'main' into ECP-9484
khushboo-singhvi Nov 7, 2024
07c3c86
A scenario was caught while writing unit test, After mocking, When I …
khushboo-singhvi Nov 8, 2024
02ee38c
Merge remote-tracking branch 'origin/ECP-9484' into ECP-9484
khushboo-singhvi Nov 8, 2024
14bcbb1
Merge branch 'main' into ECP-9484
khushboo-singhvi Nov 12, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading