Skip to content

Commit

Permalink
Merge pull request #1787 from OpenConext/feature/1285-pdp_errohandling
Browse files Browse the repository at this point in the history
Improve handling of PDP timeout error
  • Loading branch information
johanib authored Jan 23, 2025
2 parents 20a61c1 + a94dc32 commit 3bcf45e
Show file tree
Hide file tree
Showing 4 changed files with 145 additions and 2 deletions.
16 changes: 16 additions & 0 deletions library/EngineBlock/Application/TestDiContainer.php
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,18 @@
*/

use OpenConext\EngineBlock\Stepup\StepupEndpoint;
use OpenConext\EngineBlockBundle\Pdp\PdpClientInterface;

/**
* Creates mocked versions of dependencies for unit testing
*/
class EngineBlock_Application_TestDiContainer extends EngineBlock_Application_DiContainer
{
/**
* @var PdpClientInterface|null
*/
private $pdpClient;

public function getXmlConverter()
{
return Phake::mock('EngineBlock_Corto_XmlToArray');
Expand All @@ -38,6 +44,16 @@ public function getDatabaseConnectionFactory()
return Phake::mock('EngineBlock_Database_ConnectionFactory');
}

public function getPdpClient()
{
return $this->pdpClient ?? parent::getPdpClient();
}

public function setPdpClient(PdpClientInterface $pdpClient)
{
$this->pdpClient = $pdpClient;
}

public function getConsentFactory()
{
$consentFactoryMock = Phake::mock('EngineBlock_Corto_Model_Consent_Factory');
Expand Down
18 changes: 16 additions & 2 deletions library/EngineBlock/Corto/Filter/Command/EnforcePolicy.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,13 @@

class EngineBlock_Corto_Filter_Command_EnforcePolicy extends EngineBlock_Corto_Filter_Command_Abstract
{
/**
* @throws EngineBlock_Exception_UnknownRequesterIdInAuthnRequest
* @throws EngineBlock_Exception_UnknownServiceProvider
* @throws EngineBlock_Exception
* @throws EngineBlock_Corto_Exception_PEPNoAccess
* @throws EngineBlock_Exception_PdpCheckFailed
*/
public function execute()
{
$log = EngineBlock_ApplicationSingleton::getLog();
Expand Down Expand Up @@ -54,8 +61,15 @@ public function execute()

$log->debug("Policy Enforcement Point: Requesting decision from PDP");

$pdp = $this->getPdpClient();
$policyDecision = $pdp->requestDecisionFor($pdpRequest);
try {
$pdp = $this->getPdpClient();
$policyDecision = $pdp->requestDecisionFor($pdpRequest);
} catch (\OpenConext\EngineBlock\Http\Exception\HttpException $e) {
throw new EngineBlock_Exception_PdpCheckFailed(
'Policy Enforcement Point: Could not perform PDP check: ' . $e->getMessage()
);
}

// The IdP logo is set after getting the PolicyDecision as it would be inappropriate to inject this into the
// decision request.
if ($this->_identityProvider->getMdui()->hasLogo()){
Expand Down
21 changes: 21 additions & 0 deletions library/EngineBlock/Exception/PdpCheckFailed.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
<?php

/**
* Copyright 2025 SURFnet B.V.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

class EngineBlock_Exception_PdpCheckFailed extends EngineBlock_Exception
{
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
<?php

/**
* Copyright 2025 SURFnet B.V.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

use Mockery\Adapter\Phpunit\MockeryPHPUnitIntegration;
use OpenConext\EngineBlock\Http\Exception\UnreadableResourceException;
use OpenConext\EngineBlock\Metadata\Entity\IdentityProvider;
use OpenConext\EngineBlock\Metadata\Entity\ServiceProvider;
use OpenConext\EngineBlock\Metadata\MetadataRepository\MetadataRepositoryInterface;
use OpenConext\EngineBlockBundle\Pdp\PdpClientInterface;
use PHPUnit\Framework\TestCase;
use SAML2\Assertion;
use SAML2\AuthnRequest;

class EngineBlock_Test_Corto_Filter_Command_EnforcePolicyTest extends TestCase
{
use MockeryPHPUnitIntegration;

public function testThrowsEngineBlockExceptionIfPolicyCannotBeChecked()
{
$this->expectException('EngineBlock_Exception_PdpCheckFailed');
$this->expectExceptionMessage('Policy Enforcement Point: Could not perform PDP check: Resource could not be read (status code "503")');

$this->mockPdpClientWithException(new UnreadableResourceException('Resource could not be read (status code "503")'));

$policy = new EngineBlock_Corto_Filter_Command_EnforcePolicy();

$request = $this->mockRequest();
$policy->setRequest($request);

$repo = Mockery::mock(MetadataRepositoryInterface::class);
$server = Mockery::mock(EngineBlock_Corto_ProxyServer::class);
$server->expects('getRepository')->andReturn($repo);

$sp = $this->mockServiceProvider();

$policy->setServiceProvider($sp);
$policy->setProxyServer($server);
$policy->setResponseAttributes([]);

$policy->setCollabPersonId('foo');

$idp = Mockery::mock(IdentityProvider::class);
$idp->entityId = 'bar';
$policy->setIdentityProvider($idp);

$policy->execute();
}

private function mockServiceProvider(): ServiceProvider
{
$sp = Mockery::mock(ServiceProvider::class);
$sp->entityId = 'bar';
$sp->shouldReceive('getCoins->isTrustedProxy')->andReturn(false);
$sp->shouldReceive('getCoins->policyEnforcementDecisionRequired')->andReturn(true);
return $sp;
}

private function mockRequest(): EngineBlock_Saml2_AuthnRequestAnnotationDecorator
{
$assertion = new Assertion();
$request = new AuthnRequest();
$response = new SAML2\Response();
$response->setAssertions(array($assertion));
return new EngineBlock_Saml2_AuthnRequestAnnotationDecorator($request);
}

private function mockPdpClientWithException(Throwable $exception): void
{
$pdpClient = Mockery::mock(PdpClientInterface::class);
$pdpClient->expects('requestDecisionFor')->andThrow($exception);

/** @var EngineBlock_Application_TestDiContainer $container */
$container = EngineBlock_ApplicationSingleton::getInstance()->getDiContainer();
$container->setPdpClient($pdpClient);
}

}

0 comments on commit 3bcf45e

Please sign in to comment.