From e0c29f5b5205e37afdf49dd4c6d05890c672556f Mon Sep 17 00:00:00 2001 From: Michiel Kodde Date: Thu, 22 Aug 2024 13:20:17 +0200 Subject: [PATCH] Add all IdPs to the export when ACL allowAll=true It makes sense to show all connected IdP's in the CSV export for the surfconext representative. When the ACL is set to allow all IdPs connected to the SP, we should list all IdPs of the test env there. This can be a long list. But now we would only show the Organizations IdPs, and the explicitly marked test-idps. See: https://www.pivotaltracker.com/story/show/187805216/comments/242257758 --- .../Application/Service/IdpService.php | 9 +++++++++ .../Application/Service/IdpServiceInterface.php | 6 ++++++ .../Service/ServiceConnectionService.php | 14 ++++++++++---- .../Application/ViewObject/EntityConnection.php | 4 ++++ .../Repository/IdentityProviderRepository.php | 2 +- .../Manage/Client/IdentityProviderClient.php | 3 ++- .../ViewObject/EntityConnectionTest.php | 2 ++ .../Manage/Client/IdentityProviderClientTest.php | 4 +++- .../Client/fixture/identity_provider_response.json | 4 ++-- .../Manage/Client/FakeIdentityProviderClient.php | 3 ++- 10 files changed, 41 insertions(+), 10 deletions(-) diff --git a/src/Surfnet/ServiceProviderDashboard/Application/Service/IdpService.php b/src/Surfnet/ServiceProviderDashboard/Application/Service/IdpService.php index f70befb6f..dca5878c5 100644 --- a/src/Surfnet/ServiceProviderDashboard/Application/Service/IdpService.php +++ b/src/Surfnet/ServiceProviderDashboard/Application/Service/IdpService.php @@ -20,6 +20,7 @@ namespace Surfnet\ServiceProviderDashboard\Application\Service; +use Surfnet\ServiceProviderDashboard\Domain\Entity\IdentityProvider; use Surfnet\ServiceProviderDashboard\Domain\Repository\IdentityProviderRepository; use Surfnet\ServiceProviderDashboard\Domain\ValueObject\ConfiguredTestIdpCollection; use Surfnet\ServiceProviderDashboard\Domain\ValueObject\IdpCollection; @@ -47,4 +48,12 @@ public function findInstitutionIdps(InstitutionId $institutionId): InstitutionId { return new InstitutionIdpCollection($this->identityProviderRepository->findByInstitutionId($institutionId)); } + + /** + * @return array + */ + public function findAll(): array + { + return $this->identityProviderRepository->findAll(); + } } diff --git a/src/Surfnet/ServiceProviderDashboard/Application/Service/IdpServiceInterface.php b/src/Surfnet/ServiceProviderDashboard/Application/Service/IdpServiceInterface.php index 7a2b6e276..551a3d887 100644 --- a/src/Surfnet/ServiceProviderDashboard/Application/Service/IdpServiceInterface.php +++ b/src/Surfnet/ServiceProviderDashboard/Application/Service/IdpServiceInterface.php @@ -20,6 +20,7 @@ namespace Surfnet\ServiceProviderDashboard\Application\Service; +use Surfnet\ServiceProviderDashboard\Domain\Entity\IdentityProvider; use Surfnet\ServiceProviderDashboard\Domain\ValueObject\IdpCollection; use Surfnet\ServiceProviderDashboard\Domain\ValueObject\InstitutionId; use Surfnet\ServiceProviderDashboard\Domain\ValueObject\InstitutionIdpCollection; @@ -29,4 +30,9 @@ interface IdpServiceInterface public function createCollection(): IdpCollection; public function findInstitutionIdps(InstitutionId $institutionId): InstitutionIdpCollection; + + /** + * @return array + */ + public function findAll(): array; } diff --git a/src/Surfnet/ServiceProviderDashboard/Application/Service/ServiceConnectionService.php b/src/Surfnet/ServiceProviderDashboard/Application/Service/ServiceConnectionService.php index a69f1253e..cc2765e40 100644 --- a/src/Surfnet/ServiceProviderDashboard/Application/Service/ServiceConnectionService.php +++ b/src/Surfnet/ServiceProviderDashboard/Application/Service/ServiceConnectionService.php @@ -67,6 +67,7 @@ public function findByServices(array $services): EntityConnectionCollection $institutionId, $this->getTestIdpsIndexed(), $connectedOtherIdps, + $this->idpService->findAll() ); } return $collection; @@ -81,7 +82,8 @@ public function findByInstitutionId(InstitutionId $institutionId): EntityConnect $collection, $institutionId, $this->getTestIdpsIndexed(), - $connectedOtherIdps + $connectedOtherIdps, + $this->idpService->findAll() ); return $collection; } @@ -97,12 +99,14 @@ private function listTestIdps() /** * @param array $testIdpsIndexed * @param array $otherIdpsIndexed + * @param array $allIdps */ private function addEntitiesToCollection( EntityConnectionCollection $collection, InstitutionId $institutionId, array $testIdpsIndexed, array $otherIdpsIndexed, + array $allIdps, ): void { $list = []; $entities = $this->entityService->findPublishedTestEntitiesByInstitutionId($institutionId); @@ -131,10 +135,11 @@ private function addEntitiesToCollection( $serviceName, $testIdpsIndexed, $otherIdpsIndexed, - $testIdpsIndexed + $otherIdpsIndexed, + $allIdps, $supportContact, $technicalContact, - $adminContact + $adminContact, + true ); continue; } @@ -148,7 +153,8 @@ private function addEntitiesToCollection( $this->gatherConnectedIdps($entity, $testIdpsIndexed), $supportContact, $technicalContact, - $adminContact + $adminContact, + false ); } $collection->addEntityConnections($list); diff --git a/src/Surfnet/ServiceProviderDashboard/Application/ViewObject/EntityConnection.php b/src/Surfnet/ServiceProviderDashboard/Application/ViewObject/EntityConnection.php index 95218fb75..68d5e2a97 100644 --- a/src/Surfnet/ServiceProviderDashboard/Application/ViewObject/EntityConnection.php +++ b/src/Surfnet/ServiceProviderDashboard/Application/ViewObject/EntityConnection.php @@ -23,6 +23,9 @@ class EntityConnection { + /** + * @SuppressWarnings(PHPMD.ExcessiveParameterList) - Could be decomposed, but for now makes no sense. + */ public function __construct( public string $entityName, public string $entityId, @@ -36,6 +39,7 @@ public function __construct( public string $supportContact, public string $technicalContact, public string $administativeContact, + public bool $isAllowAll, ) { } diff --git a/src/Surfnet/ServiceProviderDashboard/Domain/Repository/IdentityProviderRepository.php b/src/Surfnet/ServiceProviderDashboard/Domain/Repository/IdentityProviderRepository.php index fa2ee5959..75d297181 100644 --- a/src/Surfnet/ServiceProviderDashboard/Domain/Repository/IdentityProviderRepository.php +++ b/src/Surfnet/ServiceProviderDashboard/Domain/Repository/IdentityProviderRepository.php @@ -24,7 +24,7 @@ interface IdentityProviderRepository { /** - * @return IdentityProvider[] + * @return array */ public function findAll(); diff --git a/src/Surfnet/ServiceProviderDashboard/Infrastructure/Manage/Client/IdentityProviderClient.php b/src/Surfnet/ServiceProviderDashboard/Infrastructure/Manage/Client/IdentityProviderClient.php index 378314091..d7b6eea3c 100644 --- a/src/Surfnet/ServiceProviderDashboard/Infrastructure/Manage/Client/IdentityProviderClient.php +++ b/src/Surfnet/ServiceProviderDashboard/Infrastructure/Manage/Client/IdentityProviderClient.php @@ -59,7 +59,8 @@ public function findAll() return $list; } foreach ($result as $manageResult) { - $list[] = IdentityProviderFactory::fromManageResult($manageResult); + $idp = IdentityProviderFactory::fromManageResult($manageResult); + $list[$idp->getEntityId()] = $idp; } return $list; } catch (HttpException $e) { diff --git a/tests/unit/Application/ViewObject/EntityConnectionTest.php b/tests/unit/Application/ViewObject/EntityConnectionTest.php index c79143735..c9cdf434f 100644 --- a/tests/unit/Application/ViewObject/EntityConnectionTest.php +++ b/tests/unit/Application/ViewObject/EntityConnectionTest.php @@ -49,6 +49,7 @@ public function test_correct_lists_are_created() 'James', 'Jenny', 'John', + false, ); $this->assertEquals($availableTest, $connection->listAvailableTestIdps()); @@ -88,6 +89,7 @@ public function test_allowall_results_in_all_selected() 'James', 'Jenny', 'John', + false ); $this->assertEquals($availableTest, $connection->listAvailableTestIdps()); diff --git a/tests/unit/Infrastructure/Manage/Client/IdentityProviderClientTest.php b/tests/unit/Infrastructure/Manage/Client/IdentityProviderClientTest.php index b8acf03e2..669917aa4 100644 --- a/tests/unit/Infrastructure/Manage/Client/IdentityProviderClientTest.php +++ b/tests/unit/Infrastructure/Manage/Client/IdentityProviderClientTest.php @@ -74,6 +74,8 @@ public function test_it_can_return_all_published_idps() ->andReturn('testaccepted'); $idps = $this->client->findAll(); + // Re-index the results to allow for numeric access to the collection. + $idps = array_values($idps); $this->assertCount(4, $idps); $this->assertInstanceOf(IdentityProvider::class, $idps[0]); @@ -105,7 +107,7 @@ public function test_it_can_return_all_published_idps() $this->assertInstanceOf(IdentityProvider::class, $idps[3]); $this->assertSame( - 'https://engine.dev.support.surfconext.nl/authentication/idp/metadata2', + 'https://engine.dev.support.surfconext.nl/authentication/idp/metadata3', $idps[3]->getEntityId() ); $this->assertSame('0c3febd2-3f67-4b8a-b90d-ce56a3b0abb6', $idps[3]->getManageId()); diff --git a/tests/unit/Infrastructure/Manage/Client/fixture/identity_provider_response.json b/tests/unit/Infrastructure/Manage/Client/fixture/identity_provider_response.json index 087970c65..ac6f4eafe 100644 --- a/tests/unit/Infrastructure/Manage/Client/fixture/identity_provider_response.json +++ b/tests/unit/Infrastructure/Manage/Client/fixture/identity_provider_response.json @@ -42,7 +42,7 @@ "_id": "0c3febd2-3f67-4b8a-b90d-ce56a3b0abb6", "version": 0, "data": { - "entityid": "https://engine.dev.support.surfconext.nl/authentication/idp/metadata2", + "entityid": "https://engine.dev.support.surfconext.nl/authentication/idp/metadata3", "state": "prodaccepted", "notes": null, "metaDataFields": { @@ -51,4 +51,4 @@ } } } -] \ No newline at end of file +] diff --git a/tests/webtests/Manage/Client/FakeIdentityProviderClient.php b/tests/webtests/Manage/Client/FakeIdentityProviderClient.php index fb4b66a6f..9622e8d96 100644 --- a/tests/webtests/Manage/Client/FakeIdentityProviderClient.php +++ b/tests/webtests/Manage/Client/FakeIdentityProviderClient.php @@ -46,7 +46,8 @@ public function findAll() $this->load(); $list = []; foreach ($this->entities as $manageResult) { - $list[] = IdentityProviderFactory::fromManageResult($manageResult->getEntityResult()); + $idp = IdentityProviderFactory::fromManageResult($manageResult->getEntityResult()); + $list[$idp->getEntityId()] = $idp; } return $list; }