Skip to content

Commit

Permalink
Add all IdPs to the export when ACL allowAll=true
Browse files Browse the repository at this point in the history
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
  • Loading branch information
MKodde committed Aug 22, 2024
1 parent 7aed592 commit e0c29f5
Show file tree
Hide file tree
Showing 10 changed files with 41 additions and 10 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -47,4 +48,12 @@ public function findInstitutionIdps(InstitutionId $institutionId): InstitutionId
{
return new InstitutionIdpCollection($this->identityProviderRepository->findByInstitutionId($institutionId));
}

/**
* @return array<string, IdentityProvider>
*/
public function findAll(): array
{
return $this->identityProviderRepository->findAll();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -29,4 +30,9 @@ interface IdpServiceInterface
public function createCollection(): IdpCollection;

public function findInstitutionIdps(InstitutionId $institutionId): InstitutionIdpCollection;

/**
* @return array<string, IdentityProvider>
*/
public function findAll(): array;
}
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ public function findByServices(array $services): EntityConnectionCollection
$institutionId,
$this->getTestIdpsIndexed(),
$connectedOtherIdps,
$this->idpService->findAll()
);
}
return $collection;
Expand All @@ -81,7 +82,8 @@ public function findByInstitutionId(InstitutionId $institutionId): EntityConnect
$collection,
$institutionId,
$this->getTestIdpsIndexed(),
$connectedOtherIdps
$connectedOtherIdps,
$this->idpService->findAll()
);
return $collection;
}
Expand All @@ -97,12 +99,14 @@ private function listTestIdps()
/**
* @param array<string, IdentityProvider> $testIdpsIndexed
* @param array<string, IdentityProvider> $otherIdpsIndexed
* @param array<string, IdentityProvider> $allIdps
*/
private function addEntitiesToCollection(
EntityConnectionCollection $collection,
InstitutionId $institutionId,
array $testIdpsIndexed,
array $otherIdpsIndexed,
array $allIdps,
): void {
$list = [];
$entities = $this->entityService->findPublishedTestEntitiesByInstitutionId($institutionId);
Expand Down Expand Up @@ -131,10 +135,11 @@ private function addEntitiesToCollection(
$serviceName,
$testIdpsIndexed,
$otherIdpsIndexed,
$testIdpsIndexed + $otherIdpsIndexed,
$allIdps,
$supportContact,
$technicalContact,
$adminContact
$adminContact,
true
);
continue;
}
Expand All @@ -148,7 +153,8 @@ private function addEntitiesToCollection(
$this->gatherConnectedIdps($entity, $testIdpsIndexed),
$supportContact,
$technicalContact,
$adminContact
$adminContact,
false
);
}
$collection->addEntityConnections($list);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -36,6 +39,7 @@ public function __construct(
public string $supportContact,
public string $technicalContact,
public string $administativeContact,
public bool $isAllowAll,
) {
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
interface IdentityProviderRepository
{
/**
* @return IdentityProvider[]
* @return array<string, IdentityProvider>
*/
public function findAll();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
2 changes: 2 additions & 0 deletions tests/unit/Application/ViewObject/EntityConnectionTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ public function test_correct_lists_are_created()
'James',
'Jenny',
'John',
false,
);

$this->assertEquals($availableTest, $connection->listAvailableTestIdps());
Expand Down Expand Up @@ -88,6 +89,7 @@ public function test_allowall_results_in_all_selected()
'James',
'Jenny',
'John',
false
);

$this->assertEquals($availableTest, $connection->listAvailableTestIdps());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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]);
Expand Down Expand Up @@ -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());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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": {
Expand All @@ -51,4 +51,4 @@
}
}
}
]
]
3 changes: 2 additions & 1 deletion tests/webtests/Manage/Client/FakeIdentityProviderClient.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down

0 comments on commit e0c29f5

Please sign in to comment.