Skip to content

Commit

Permalink
Improve handling of PDP timeout error
Browse files Browse the repository at this point in the history
Prior to this change, mulitple stacktraces would be printed in the logs
when this error occured without making it clear which error was triggered.
This change ensures a specific exception gets thrown so it won't trigger
the exception stack dump.

Fixes #1285
  • Loading branch information
johanib committed Jan 22, 2025
1 parent 20a61c1 commit 8733cf3
Show file tree
Hide file tree
Showing 3 changed files with 115 additions and 2 deletions.
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,78 @@
<?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\Metadata\Entity\IdentityProvider;
use OpenConext\EngineBlock\Metadata\Entity\ServiceProvider;
use OpenConext\EngineBlock\Metadata\MetadataRepository\MetadataRepositoryInterface;
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")');

$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);
}

}

0 comments on commit 8733cf3

Please sign in to comment.