diff --git a/src/OpenConext/EngineBlock/Metadata/Entity/Assembler/DiscoveryAssembler.php b/src/OpenConext/EngineBlock/Metadata/Entity/Assembler/DiscoveryAssembler.php index 4e353cda24..811e4c4a97 100644 --- a/src/OpenConext/EngineBlock/Metadata/Entity/Assembler/DiscoveryAssembler.php +++ b/src/OpenConext/EngineBlock/Metadata/Entity/Assembler/DiscoveryAssembler.php @@ -18,6 +18,7 @@ namespace OpenConext\EngineBlock\Metadata\Entity\Assembler; +use OpenConext\EngineBlock\Exception\InvalidDiscoveryException; use OpenConext\EngineBlock\Metadata\Discovery; use OpenConext\EngineBlock\Metadata\Logo; use OpenConext\EngineBlockBundle\Localization\LanguageSupportProvider; @@ -47,9 +48,13 @@ public function assembleDiscoveries(stdClass $connection): array $keywords = $this->extractLocalizedFields($discovery, 'keywords'); $logo = $this->assembleLogo($discovery); - if (isset($names['en'])) { - $discoveries[] = Discovery::create($names, $keywords, $logo); + if (!isset($names['en'])) { + throw new InvalidDiscoveryException( + 'A discovery entry is missing a required English name (name_en). ' . + 'Refusing to process metadata push.' + ); } + $discoveries[] = Discovery::create($names, $keywords, $logo); } return empty($discoveries) ? [] : ['discoveries' => $discoveries]; diff --git a/tests/functional/OpenConext/EngineBlockBundle/Controller/Api/ConnectionsControllerTest.php b/tests/functional/OpenConext/EngineBlockBundle/Controller/Api/ConnectionsControllerTest.php index 55cb1e59e7..73dcb4e8a6 100644 --- a/tests/functional/OpenConext/EngineBlockBundle/Controller/Api/ConnectionsControllerTest.php +++ b/tests/functional/OpenConext/EngineBlockBundle/Controller/Api/ConnectionsControllerTest.php @@ -396,6 +396,39 @@ public function pushing_entity_with_valid_manipulation_code_succeeds() $this->assertStatusCode(Response::HTTP_OK, $client); } + #[Group('Api')] + #[Group('Connections')] + #[Group('MetadataPush')] + #[Test] + public function pushing_idp_with_discovery_missing_english_name_returns_bad_request(): void + { + $client = $this->createAuthorizedClient(); + + $payload = '{"connections":{"idp1":{ + "allow_all_entities":true, + "allowed_connections":[], + "metadata":{ + "name":{"en":"Test IdP","nl":"Test IdP nl"}, + "discoveries":[{"name_nl":"Ontbrekend Engels"}], + "SingleSignOnService":[{ + "Binding":"urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST", + "Location":"https://idp.example.com/sso" + }] + }, + "name":"https://idp.example.com", + "state":"prodaccepted", + "type":"saml20-idp" + }}}'; + + $client->request('POST', 'https://engine-api.dev.openconext.local/api/connections', [], [], [], $payload); + + $this->assertStatusCode(Response::HTTP_BAD_REQUEST, $client); + $this->assertJson($client->getResponse()->getContent()); + $isContentTypeJson = $client->getResponse()->headers->contains('Content-Type', 'application/json'); + $this->assertTrue($isContentTypeJson, 'Response should have Content-Type: application/json header'); + $this->assertStringContainsString('name_en', $client->getResponse()->getContent()); + } + public static function invalidHttpMethodProvider() { return [ diff --git a/tests/integration/OpenConext/EngineBlock/Metadata/Entity/Assembler/DiscoveryAssemblerTest.php b/tests/integration/OpenConext/EngineBlock/Metadata/Entity/Assembler/DiscoveryAssemblerTest.php index 9d633195c3..ec01565374 100644 --- a/tests/integration/OpenConext/EngineBlock/Metadata/Entity/Assembler/DiscoveryAssemblerTest.php +++ b/tests/integration/OpenConext/EngineBlock/Metadata/Entity/Assembler/DiscoveryAssemblerTest.php @@ -18,6 +18,7 @@ namespace OpenConext\EngineBlock\Tests; +use OpenConext\EngineBlock\Exception\InvalidDiscoveryException; use OpenConext\EngineBlock\Metadata\Discovery; use OpenConext\EngineBlock\Metadata\Entity\Assembler\DiscoveryAssembler; use OpenConext\EngineBlock\Metadata\Logo; @@ -84,8 +85,11 @@ public function testAssembleDiscoveriesProcessesValidDiscovery(): void $this->assertEquals(100, $discovery->getLogo()->height); } - public function testAssembleDiscoveriesSkipsDiscoveryWithoutEnglishName(): void + public function testAssembleDiscoveriesThrowsWhenDiscoveryMissingEnglishName(): void { + $this->expectException(InvalidDiscoveryException::class); + $this->expectExceptionMessage('missing a required English name'); + $connection = new stdClass(); $connection->metadata = new stdClass(); $connection->metadata->discoveries = [ @@ -95,9 +99,7 @@ public function testAssembleDiscoveriesSkipsDiscoveryWithoutEnglishName(): void ]) ]; - $result = $this->discoveryAssembler->assembleDiscoveries($connection); - - $this->assertEquals([], $result); + $this->discoveryAssembler->assembleDiscoveries($connection); } public function testAssembleDiscoveriesWithTrimmedValues(): void diff --git a/tests/integration/OpenConext/EngineBlock/Metadata/Entity/Assembler/PushMetadataAssemblerTest.php b/tests/integration/OpenConext/EngineBlock/Metadata/Entity/Assembler/PushMetadataAssemblerTest.php index 18ea7b8db0..8051a769df 100644 --- a/tests/integration/OpenConext/EngineBlock/Metadata/Entity/Assembler/PushMetadataAssemblerTest.php +++ b/tests/integration/OpenConext/EngineBlock/Metadata/Entity/Assembler/PushMetadataAssemblerTest.php @@ -20,6 +20,7 @@ use Mockery as m; use Mockery\Adapter\Phpunit\MockeryPHPUnitIntegration; +use OpenConext\EngineBlock\Exception\InvalidDiscoveryException; use OpenConext\EngineBlock\Metadata\AttributeReleasePolicy; use OpenConext\EngineBlock\Metadata\Discovery; use OpenConext\EngineBlock\Metadata\Entity\Assembler\PushMetadataAssembler; @@ -569,6 +570,15 @@ public function test_it_assembles_metadata_without_discoveries_details() $this->assertSame('https://engine.dev.openconext.local/images/logo.png?v=dev', $idp->logo->url); } + public function test_it_rejects_idp_with_discovery_missing_english_name(): void + { + $this->expectException(InvalidDiscoveryException::class); + $this->expectExceptionMessage('missing a required English name'); + + $input = $this->readFixture('metadata_idp_discovery_missing_english.json'); + $this->assembler->assemble($input); + } + private function readFixture(string $file): object { return json_decode(file_get_contents(__DIR__ . '/fixtures/'. basename($file)), false); diff --git a/tests/integration/OpenConext/EngineBlock/Metadata/Entity/Assembler/fixtures/metadata_idp_discovery_missing_english.json b/tests/integration/OpenConext/EngineBlock/Metadata/Entity/Assembler/fixtures/metadata_idp_discovery_missing_english.json new file mode 100644 index 0000000000..fb1533f510 --- /dev/null +++ b/tests/integration/OpenConext/EngineBlock/Metadata/Entity/Assembler/fixtures/metadata_idp_discovery_missing_english.json @@ -0,0 +1,24 @@ +{ + "2d96e27a-76cf-4ca2-ac70-ece5d4c49525": { + "name": "https:\/\/serviceregistry.dev.openconext.local\/simplesaml\/module.php\/saml\/sp\/metadata.php\/default-idp", + "state": "prodaccepted", + "type": "saml20-idp", + "metadata": { + "discoveries": [ + { + "name_nl": "Ontbrekend Engels" + } + ], + "SingleSignOnService": [ + { + "Binding": "urn:oasis:names:tc:SAML:2.0:bindings:HTTP-POST", + "Location": "https:\/\/mujina-idp.dev.openConext.local\/SingleSignOnService" + } + ], + "name": { + "en": "Dummy IdP", + "nl": "Dummy IdP nl" + } + } + } +}