Skip to content

Commit fb00492

Browse files
MKoddeMKodde
authored
Merge pull request #893 from OpenConext/bugfix/mfa-based-on-original-sp
Whether MFA AuthnContext must be added should be based on original SP
2 parents 69b5c7e + 9358fc0 commit fb00492

File tree

4 files changed

+75
-20
lines changed

4 files changed

+75
-20
lines changed

‎library/EngineBlock/Corto/Filter/Command/ValidateMfaAuthnContextClassRef.php‎

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,14 +27,25 @@
2727
**/
2828
class EngineBlock_Corto_Filter_Command_ValidateMfaAuthnContextClassRef extends EngineBlock_Corto_Filter_Command_Abstract
2929
{
30+
/**
31+
* @var LoggerInterface
32+
*/
33+
private $logger;
34+
3035
const MICROSOFT_AUTHN_METHODS_REFERENCES_ATTRIBUTE = 'http://schemas.microsoft.com/claims/authnmethodsreferences';
3136

37+
public function __construct(LoggerInterface $logger)
38+
{
39+
$this->logger = $logger;
40+
}
41+
3242
/**
3343
* @throws EngineBlock_Corto_Exception_AuthnContextClassRefBlacklisted
3444
*/
3545
public function execute()
3646
{
37-
$mfaEntity = $this->_identityProvider->getCoins()->mfaEntities()->findByEntityId($this->_serviceProvider->entityId);
47+
$originalSP = $this->_server->findOriginalServiceProvider($this->_request, $this->logger)->entityId;
48+
$mfaEntity = $this->_identityProvider->getCoins()->mfaEntities()->findByEntityId($originalSP);
3849
// SP's configured to pass auth context transparently are not checked for an expected (configured) class ref.
3950
if (!$mfaEntity || $mfaEntity instanceof TransparentMfaEntity) {
4051
return;
@@ -47,7 +58,7 @@ public function execute()
4758
sprintf(
4859
'Assertion from MFA IdP "%s" for SP "%s" does not contain the requested AuthnContextClassRef "%s"',
4960
$this->_identityProvider->entityId,
50-
$this->_serviceProvider->entityId,
61+
$originalSP,
5162
$mfaEntity->level()
5263
)
5364
);

‎library/EngineBlock/Corto/Filter/Input.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ public function getCommands()
4444
new EngineBlock_Corto_Filter_Command_ValidateAuthnContextClassRef($logger, $authnContextClassRefBlacklistPattern),
4545

4646
// Validate if the MFA authnContextClassRef is valid when configured
47-
new EngineBlock_Corto_Filter_Command_ValidateMfaAuthnContextClassRef(),
47+
new EngineBlock_Corto_Filter_Command_ValidateMfaAuthnContextClassRef($logger),
4848

4949
// Convert all OID attributes to URN and remove the OID variant
5050
new EngineBlock_Corto_Filter_Command_NormalizeAttributes(),

‎library/EngineBlock/Corto/ProxyServer.php‎

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,15 +16,16 @@
1616
* limitations under the License.
1717
*/
1818

19-
use OpenConext\EngineBlock\Metadata\Loa;
20-
use OpenConext\EngineBlock\Metadata\MfaEntity;
21-
use OpenConext\EngineBlock\Metadata\TransparentMfaEntity;
22-
use OpenConext\EngineBlockBundle\Authentication\AuthenticationState;
2319
use OpenConext\EngineBlock\Metadata\Entity\AbstractRole;
2420
use OpenConext\EngineBlock\Metadata\Entity\IdentityProvider;
2521
use OpenConext\EngineBlock\Metadata\Entity\ServiceProvider;
22+
use OpenConext\EngineBlock\Metadata\Loa;
23+
use OpenConext\EngineBlock\Metadata\MetadataRepository\EntityNotFoundException;
2624
use OpenConext\EngineBlock\Metadata\MetadataRepository\MetadataRepositoryInterface;
25+
use OpenConext\EngineBlock\Metadata\MfaEntity;
2726
use OpenConext\EngineBlock\Metadata\Service;
27+
use OpenConext\EngineBlock\Metadata\TransparentMfaEntity;
28+
use OpenConext\EngineBlockBundle\Authentication\AuthenticationState;
2829
use OpenConext\Value\Saml\Entity;
2930
use OpenConext\Value\Saml\EntityId;
3031
use OpenConext\Value\Saml\EntityType;
@@ -416,8 +417,15 @@ public function sendAuthenticationRequest(
416417
throw new EngineBlock_Corto_ProxyServer_Exception(sprintf('Unknown message type: "%s"', get_class($sspMessage)));
417418
}
418419

420+
try {
421+
$originalSpEnitytId = $this->findOriginalServiceProvider($spRequest, $this->_logger)->entityId;
422+
} catch (EntityNotFoundException $e) {
423+
// On debug requests, the entity ID can not be found in the database as this is the EngineBlock internal
424+
// entity which does not reside in the database.
425+
$originalSpEnitytId = $spEntityId;
426+
}
419427
// Add authncontextclassref if configured
420-
$service = $identityProvider->getCoins()->mfaEntities()->findByEntityId($serviceProvider->getEntityId()->getEntityId());
428+
$service = $identityProvider->getCoins()->mfaEntities()->findByEntityId($originalSpEnitytId);
421429
if ($service instanceof MfaEntity) {
422430
$sspMessage->setRequestedAuthnContext([
423431
'AuthnContextClassRef' => [

‎tests/library/EngineBlock/Test/Corto/Filter/Command/ValidateMfaAuthnContextClassRefTest.php‎

Lines changed: 48 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
* limitations under the License.
1717
*/
1818

19+
use Mockery as m;
1920
use Mockery\Adapter\Phpunit\MockeryPHPUnitIntegration;
2021
use Monolog\Handler\TestHandler;
2122
use Monolog\Logger;
@@ -26,7 +27,8 @@
2627
use OpenConext\EngineBlock\Metadata\MfaEntityCollection;
2728
use PHPUnit\Framework\TestCase;
2829
use SAML2\Assertion;
29-
use SAML2\Response;
30+
use SAML2\AuthnRequest;
31+
use SAML2\Response as SAMLResponse;
3032

3133
class EngineBlock_Test_Corto_Filter_Command_ValidateMfaAuthnContextClassRefTest extends TestCase
3234
{
@@ -42,22 +44,42 @@ class EngineBlock_Test_Corto_Filter_Command_ValidateMfaAuthnContextClassRefTest
4244
*/
4345
private $logger;
4446

47+
/**
48+
* @var EngineBlock_Saml2_AuthnRequestAnnotationDecorator
49+
*/
50+
private $request;
51+
private $server;
52+
4553
public function setUp()
4654
{
4755
$this->handler = new TestHandler();
4856
$this->logger = new Logger('Test', array($this->handler));
57+
$assertion = new Assertion();
58+
59+
$request = new AuthnRequest();
60+
$response = new SAMLResponse();
61+
$response->setAssertions(array($assertion));
62+
63+
$this->request = new EngineBlock_Saml2_AuthnRequestAnnotationDecorator($request);
64+
65+
$this->sp = new ServiceProvider('Test SP');
66+
$this->server = m::mock(EngineBlock_Corto_ProxyServer::class);
67+
$this->server
68+
->shouldReceive('findOriginalServiceProvider')
69+
->andReturn($this->sp)
70+
;
4971
}
5072

5173
public function testNoConfiguredMfaCombinationShouldPass()
5274
{
53-
$this->expectNotToPerformAssertions();
54-
5575
$response = $this->createTestResponse('urn:oasis:names:tc:SAML:2.0:ac:classes:Password');
5676

5777
$verifier = new EngineBlock_Corto_Filter_Command_ValidateMfaAuthnContextClassRef($this->logger);
5878
$verifier->setResponse($response);
5979
$verifier->setIdentityProvider(new IdentityProvider('MFA IdP'));
60-
$verifier->setServiceProvider(new ServiceProvider('Test SP'));
80+
$verifier->setServiceProvider($this->sp);
81+
$verifier->setRequest($this->request);
82+
$verifier->setProxyServer($this->server);
6183

6284
$verifier->execute();
6385
}
@@ -71,7 +93,9 @@ public function testMatchedAuthnContextClassRefShouldPass()
7193
$verifier = new EngineBlock_Corto_Filter_Command_ValidateMfaAuthnContextClassRef($this->logger);
7294
$verifier->setResponse($response);
7395
$verifier->setIdentityProvider($identityProvider);
74-
$verifier->setServiceProvider(new ServiceProvider('Test SP'));
96+
$verifier->setServiceProvider($this->sp);
97+
$verifier->setRequest($this->request);
98+
$verifier->setProxyServer($this->server);
7599

76100
$verifier->execute();
77101

@@ -87,7 +111,9 @@ public function testMatchedAuthnMethodsReferenceAttributeShouldPass()
87111
$verifier = new EngineBlock_Corto_Filter_Command_ValidateMfaAuthnContextClassRef($this->logger);
88112
$verifier->setResponse($response);
89113
$verifier->setIdentityProvider($identityProvider);
90-
$verifier->setServiceProvider(new ServiceProvider('Test SP'));
114+
$verifier->setServiceProvider($this->sp);
115+
$verifier->setRequest($this->request);
116+
$verifier->setProxyServer($this->server);
91117

92118
$verifier->execute();
93119

@@ -107,7 +133,9 @@ public function testMatchedAuthnMethodsReferenceAttributeWithMultipleValuesShoul
107133
$verifier = new EngineBlock_Corto_Filter_Command_ValidateMfaAuthnContextClassRef($this->logger);
108134
$verifier->setResponse($response);
109135
$verifier->setIdentityProvider($identityProvider);
110-
$verifier->setServiceProvider(new ServiceProvider('Test SP'));
136+
$verifier->setServiceProvider($this->sp);
137+
$verifier->setRequest($this->request);
138+
$verifier->setProxyServer($this->server);
111139

112140
$verifier->execute();
113141

@@ -127,7 +155,9 @@ public function testMatchedAuthnclassrefAndAuthnMethodsReferenceAttributeWithMul
127155
$verifier = new EngineBlock_Corto_Filter_Command_ValidateMfaAuthnContextClassRef($this->logger);
128156
$verifier->setResponse($response);
129157
$verifier->setIdentityProvider($identityProvider);
130-
$verifier->setServiceProvider(new ServiceProvider('Test SP'));
158+
$verifier->setServiceProvider($this->sp);
159+
$verifier->setRequest($this->request);
160+
$verifier->setProxyServer($this->server);
131161

132162
$verifier->execute();
133163

@@ -151,7 +181,9 @@ public function testNotMatchedAuthnContextClassRefShouldThrowException()
151181
$verifier = new EngineBlock_Corto_Filter_Command_ValidateMfaAuthnContextClassRef($this->logger);
152182
$verifier->setResponse($response);
153183
$verifier->setIdentityProvider($identityProvider);
154-
$verifier->setServiceProvider(new ServiceProvider('Test SP'));
184+
$verifier->setServiceProvider($this->sp);
185+
$verifier->setRequest($this->request);
186+
$verifier->setProxyServer($this->server);
155187

156188
$verifier->execute();
157189
}
@@ -171,7 +203,9 @@ public function testNotMatchedAuthnMethodsReferenceAttributeWithMultipleValuesSh
171203
$verifier = new EngineBlock_Corto_Filter_Command_ValidateMfaAuthnContextClassRef($this->logger);
172204
$verifier->setResponse($response);
173205
$verifier->setIdentityProvider($identityProvider);
174-
$verifier->setServiceProvider(new ServiceProvider('Test SP'));
206+
$verifier->setServiceProvider($this->sp);
207+
$verifier->setRequest($this->request);
208+
$verifier->setProxyServer($this->server);
175209

176210
$verifier->execute();
177211

@@ -187,7 +221,9 @@ public function testMatchedTransparentAuthnContextClassRefShouldPass()
187221
$verifier = new EngineBlock_Corto_Filter_Command_ValidateMfaAuthnContextClassRef($this->logger);
188222
$verifier->setResponse($response);
189223
$verifier->setIdentityProvider($identityProvider);
190-
$verifier->setServiceProvider(new ServiceProvider('Test SP'));
224+
$verifier->setServiceProvider($this->sp);
225+
$verifier->setRequest($this->request);
226+
$verifier->setProxyServer($this->server);
191227

192228
$verifier->execute();
193229

@@ -227,7 +263,7 @@ private function createTestResponse($authnContextClassRef, $authMethodsReference
227263
]);
228264
}
229265

230-
$response = new Response();
266+
$response = new SAMLResponse();
231267
$response->setAssertions(array($assertion));
232268

233269
return new EngineBlock_Saml2_ResponseAnnotationDecorator($response);

0 commit comments

Comments
 (0)