diff --git a/config/packages/engineblock_features.yaml b/config/packages/engineblock_features.yaml index 467d8c190..d28f7a9e2 100644 --- a/config/packages/engineblock_features.yaml +++ b/config/packages/engineblock_features.yaml @@ -15,3 +15,4 @@ parameters: eb.feature_enable_idp_initiated_flow: "%feature_enable_idp_initiated_flow%" eb.stepup.sfo.override_engine_entityid: "%feature_stepup_sfo_override_engine_entityid%" eb.stepup.send_user_attributes: "%feature_stepup_send_user_attributes%" + eb.stepup.use_idp_response_loa: "%feature_use_idp_loa_in_stepup_decision%" diff --git a/config/packages/parameters.yml.dist b/config/packages/parameters.yml.dist index f8fe5741e..d1de69950 100644 --- a/config/packages/parameters.yml.dist +++ b/config/packages/parameters.yml.dist @@ -223,6 +223,7 @@ parameters: feature_enable_idp_initiated_flow: true feature_stepup_sfo_override_engine_entityid: false feature_stepup_send_user_attributes: false + feature_use_idp_loa_in_stepup_decision: false ########################################################################################## ## PROFILE SETTINGS diff --git a/library/EngineBlock/Corto/Module/Service/AssertionConsumer.php b/library/EngineBlock/Corto/Module/Service/AssertionConsumer.php index 42f99d996..7ce884670 100644 --- a/library/EngineBlock/Corto/Module/Service/AssertionConsumer.php +++ b/library/EngineBlock/Corto/Module/Service/AssertionConsumer.php @@ -188,8 +188,15 @@ public function serve($serviceName, Request $httpRequest) $pdpLoas = $receivedResponse->getPdpRequestedLoas(); $loaRepository = $application->getDiContainer()->getLoaRepository(); $authnRequestLoas = $receivedRequest->getStepupObligations($loaRepository->getStepUpLoas()); + + $idpResponseLoa = null; + // Determine the IdP response LoA if feature is enabled + if($application->getDiContainer()->getFeatureConfiguration()->isEnabled('eb.stepup.use_idp_response_loa')) { + $idpResponseLoa = $originalAssertions->getAuthnContextClassRef(); + } + // Goto consent if no Stepup authentication is needed - if (!$this->_stepupGatewayCallOutHelper->shouldUseStepup($idp, $sp, $authnRequestLoas, $pdpLoas)) { + if (!$this->_stepupGatewayCallOutHelper->shouldUseStepup($idp, $sp, $authnRequestLoas, $pdpLoas, $idpResponseLoa)) { $this->_server->sendConsentAuthenticationRequest($receivedResponse, $receivedRequest, $currentProcessStep->getRole(), $this->getAuthenticationState()); return; } diff --git a/library/EngineBlock/Corto/Module/Service/StepupAssertionConsumer.php b/library/EngineBlock/Corto/Module/Service/StepupAssertionConsumer.php index 62b7d55d6..ee7f025a9 100644 --- a/library/EngineBlock/Corto/Module/Service/StepupAssertionConsumer.php +++ b/library/EngineBlock/Corto/Module/Service/StepupAssertionConsumer.php @@ -298,7 +298,7 @@ private function verifyReceivedLoa( $pdpLoas = $originalResponse->getPdpRequestedLoas(); $authnRequestLoas = $receivedRequest->getStepupObligations($this->_loaRepository->getStepUpLoas()); - $stepupDecision = new StepupDecision($idp, $sp, $authnRequestLoas, $pdpLoas, $this->_loaRepository, $log); + $stepupDecision = new StepupDecision($idp, $sp, $authnRequestLoas, $pdpLoas, null, $this->_loaRepository, $log); $requiredLoa = $stepupDecision->getStepupLoa(); $receivedLoa = $this->_stepupGatewayCallOutHelper->getEbLoa( $receivedResponse->getAssertion()->getAuthnContextClassRef() diff --git a/src/OpenConext/EngineBlock/Stepup/StepupDecision.php b/src/OpenConext/EngineBlock/Stepup/StepupDecision.php index b446b4136..75aded339 100644 --- a/src/OpenConext/EngineBlock/Stepup/StepupDecision.php +++ b/src/OpenConext/EngineBlock/Stepup/StepupDecision.php @@ -43,6 +43,10 @@ class StepupDecision * @var Loa[] */ private $pdpLoas = []; + /** + * @var string|null + */ + private $idpResponseLoa = null; /** * @var bool */ @@ -58,6 +62,7 @@ public function __construct( ServiceProvider $sp, array $authnRequestLoas, array $pdpLoas, + string|null $idpResponseLoa, LoaRepository $loaRepository, LoggerInterface $logger ) { @@ -78,6 +83,16 @@ public function __construct( $this->spNoToken = $sp->getCoins()->stepupAllowNoToken(); + // Only set idpResponseLoa if provided and valid. Use getByIdentifier and ignore when not found. + if ($idpResponseLoa) { + try { + $this->idpResponseLoa = $loaRepository->getByIdentifier($idpResponseLoa); + } catch (\Exception $e) { + // The repository will throw when identifier is not known; log and ignore invalid response LoA + $this->logger->debug(sprintf('StepupDecision: IdP Response LoA "%s" is invalid and will be ignored', $idpResponseLoa)); + } + } + foreach ($pdpLoas as $loaId) { $this->pdpLoas[] = $loaRepository->getByIdentifier($loaId); } @@ -93,6 +108,11 @@ public function shouldUseStepup(): bool if ($isLoaAsked && $isLoaAsked->getLevel() === Loa::LOA_1) { return false; } + // If the Loa is reached by the IDP, no stepup callout is required. + if ($this->checkIDPLoaIsSufficient($isLoaAsked)) { + $this->logger->info('StepupDecision: IdP Response LoA is sufficient, no Stepup required'); + return false; + } return $isLoaAsked instanceof Loa; } @@ -147,6 +167,19 @@ public function getStepupLoa(): ?Loa return $highestLevel; } + /** + * Check if the IDP response LoA is sufficient for the requested LoA. + */ + private function checkIDPLoaIsSufficient($isLoaAsked): bool + { + if (!$this->idpResponseLoa) { + return false; + } + + return $this->idpResponseLoa->levelIsHigherOrEqualTo($isLoaAsked); + } + + public function allowNoToken(): bool { if ($this->isLoaRequirementSet()) { diff --git a/src/OpenConext/EngineBlock/Stepup/StepupGatewayCallOutHelper.php b/src/OpenConext/EngineBlock/Stepup/StepupGatewayCallOutHelper.php index 6ffacca96..77de17283 100644 --- a/src/OpenConext/EngineBlock/Stepup/StepupGatewayCallOutHelper.php +++ b/src/OpenConext/EngineBlock/Stepup/StepupGatewayCallOutHelper.php @@ -53,13 +53,15 @@ public function shouldUseStepup( IdentityProvider $identityProvider, ServiceProvider $serviceProvider, array $authnRequestLoas, - array $pdpLoas + array $pdpLoas, + string|null $idpResponseLoa ) : bool { $stepupDecision = new StepupDecision( $identityProvider, $serviceProvider, $authnRequestLoas, $pdpLoas, + $idpResponseLoa, $this->loaRepository, $this->logger ); @@ -77,6 +79,7 @@ public function getStepupLoa( $serviceProvider, $authnRequestLoas, $pdpLoas, + null, $this->loaRepository, $this->logger ); @@ -100,7 +103,7 @@ public function getEbLoa(string $gatewayLoa) : Loa public function allowNoToken(IdentityProvider $identityProvider, ServiceProvider $serviceProvider) : bool { - $stepupDecision = new StepupDecision($identityProvider, $serviceProvider, [], [], $this->loaRepository, $this->logger); + $stepupDecision = new StepupDecision($identityProvider, $serviceProvider, [], [], null, $this->loaRepository, $this->logger); return $stepupDecision->allowNoToken(); } } diff --git a/src/OpenConext/EngineBlockBundle/Configuration/TestFeatureConfiguration.php b/src/OpenConext/EngineBlockBundle/Configuration/TestFeatureConfiguration.php index 9866bd8f7..b724a8a9b 100644 --- a/src/OpenConext/EngineBlockBundle/Configuration/TestFeatureConfiguration.php +++ b/src/OpenConext/EngineBlockBundle/Configuration/TestFeatureConfiguration.php @@ -48,6 +48,7 @@ public function __construct() $this->setFeature(new Feature('eb.stepup.sfo.override_engine_entityid', false)); $this->setFeature(new Feature('eb.feature_enable_idp_initiated_flow', true)); $this->setFeature(new Feature('eb.stepup.send_user_attributes', true)); + $this->setFeature(new Feature('eb.stepup.use_idp_response_loa', false)); } public function setFeature(Feature $feature): void diff --git a/tests/unit/OpenConext/EngineBlock/Stepup/StepupDecisionTest.php b/tests/unit/OpenConext/EngineBlock/Stepup/StepupDecisionTest.php index 6074b8c7b..2fa95bb99 100644 --- a/tests/unit/OpenConext/EngineBlock/Stepup/StepupDecisionTest.php +++ b/tests/unit/OpenConext/EngineBlock/Stepup/StepupDecisionTest.php @@ -21,6 +21,7 @@ use Mockery as m; use Mockery\Adapter\Phpunit\MockeryPHPUnitIntegration; use OpenConext\EngineBlock\Exception\InvalidStepupConfigurationException; +use OpenConext\EngineBlock\Exception\LoaNotFoundException; use OpenConext\EngineBlock\Metadata\Entity\IdentityProvider; use OpenConext\EngineBlock\Metadata\Entity\ServiceProvider; use OpenConext\EngineBlock\Metadata\Loa; @@ -69,7 +70,7 @@ public function the_correct_stepup_decision_should_be_made_based_on_a_coin_data( $repo = $this->buildMockRepository($input); $logger = m::mock(LoggerInterface::class)->shouldIgnoreMissing(); - $stepupDecision = new StepupDecision($idp, $sp, $input[2], $input[3], $repo, $logger); + $stepupDecision = new StepupDecision($idp, $sp, $input[2], $input[3], null, $repo, $logger); $useStepup = $stepupDecision->shouldUseStepup(); $stepupLoa = $stepupDecision->getStepupLoa(); @@ -160,4 +161,103 @@ private static function buildLoa(string $loaLevel): Loa $loa = Loa::create($level, $loaLevel); return $loa; } + + #[\PHPUnit\Framework\Attributes\Test] + #[\PHPUnit\Framework\Attributes\Group('Stepup')] + public function idp_response_loa_is_sufficient_prevents_stepup() + { + $sp = Utils::instantiate( + ServiceProvider::class, + [ + 'entityId' => 'sp', + 'stepupRequireLoa' => 'loa20', + 'stepupAllowNoToken' => false, + ] + ); + + $idp = Utils::instantiate( + IdentityProvider::class, + [ + 'entityId' => 'idp', + 'stepupConnections' => new StepupConnections([]), + ] + ); + + $repo = m::mock(LoaRepository::class); + $repo->shouldReceive('getByIdentifier')->with('loa20')->andReturn(Loa::create(20, 'loa20')); + $repo->shouldReceive('getByIdentifier')->with('loa30')->andReturn(Loa::create(30, 'loa30')); + + $logger = m::mock(LoggerInterface::class)->shouldIgnoreMissing(); + + $stepupDecision = new StepupDecision($idp, $sp, [], [], 'loa30', $repo, $logger); + + $this->assertFalse($stepupDecision->shouldUseStepup()); + } + + #[\PHPUnit\Framework\Attributes\Test] + #[\PHPUnit\Framework\Attributes\Group('Stepup')] + public function idp_response_loa_is_insufficient_triggers_stepup() + { + $sp = Utils::instantiate( + ServiceProvider::class, + [ + 'entityId' => 'sp', + 'stepupRequireLoa' => 'loa30', + 'stepupAllowNoToken' => false, + ] + ); + + $idp = Utils::instantiate( + IdentityProvider::class, + [ + 'entityId' => 'idp', + 'stepupConnections' => new StepupConnections([]), + ] + ); + + $repo = m::mock(LoaRepository::class); + $repo->shouldReceive('getByIdentifier')->with('loa30')->andReturn(Loa::create(30, 'loa30')); + $repo->shouldReceive('getByIdentifier')->with('loa20')->andReturn(Loa::create(20, 'loa20')); + + $logger = m::mock(LoggerInterface::class)->shouldIgnoreMissing(); + + $stepupDecision = new StepupDecision($idp, $sp, [], [], 'loa20', $repo, $logger); + + $this->assertTrue($stepupDecision->shouldUseStepup()); + $this->assertEquals('loa30', $stepupDecision->getStepupLoa()->getIdentifier()); + } + + #[\PHPUnit\Framework\Attributes\Test] + #[\PHPUnit\Framework\Attributes\Group('Stepup')] + public function unknown_idp_response_loa_is_ignored_and_triggers_stepup() + { + $sp = Utils::instantiate( + ServiceProvider::class, + [ + 'entityId' => 'sp', + 'stepupRequireLoa' => 'loa30', + 'stepupAllowNoToken' => false, + ] + ); + + $idp = Utils::instantiate( + IdentityProvider::class, + [ + 'entityId' => 'idp', + 'stepupConnections' => new StepupConnections([]), + ] + ); + + $repo = m::mock(LoaRepository::class); + // SP expects loa30 + $repo->shouldReceive('getByIdentifier')->with('loa30')->andReturn(Loa::create(30, 'loa30')); + // The idp response contains an unknown LoA + $repo->shouldReceive('getByIdentifier')->with('bad-loa')->andThrow(new LoaNotFoundException('not found')); + + $logger = m::mock(LoggerInterface::class)->shouldIgnoreMissing(); + + $stepupDecision = new StepupDecision($idp, $sp, [], [], 'bad-loa', $repo, $logger); + + $this->assertTrue($stepupDecision->shouldUseStepup()); + } }