From 071baf9e4464b46849808649cc6d7850e8f8a95f Mon Sep 17 00:00:00 2001 From: Michiel Kodde Date: Mon, 17 Jun 2024 15:26:55 +0200 Subject: [PATCH 1/2] Make Surf representative a role on its own Previously it inherited the default ROLE_USER role. Which also granted access to the /entity and /services pages. That is not required for now. The representative should only see the /connections page. --- .env.test | 2 + ci/qa/phpstan-baseline.php | 20 +++---- config/packages/security.yaml | 1 - .../Application/Service/EntityService.php | 3 + .../Service/ServiceConnectionService.php | 57 +++++++++++-------- .../ViewObject/EntityConnectionCollection.php | 5 ++ .../Domain/ValueObject/IdpCollection.php | 4 +- .../Controller/EntrypointController.php | 39 +++++++++++++ .../ServiceConnectionsController.php | 25 ++++++-- .../Controller/ServiceController.php | 2 +- .../DataFixtures/ORM/WebTestFixtures.php | 6 ++ .../Authentication/Provider/SamlProvider.php | 3 +- tests/webtests/SurfConextResponsibleTest.php | 37 ++++++++++++ tests/webtests/WebTestCase.php | 49 +++++++++++++++- 14 files changed, 206 insertions(+), 47 deletions(-) create mode 100644 src/Surfnet/ServiceProviderDashboard/Infrastructure/DashboardBundle/Controller/EntrypointController.php create mode 100644 tests/webtests/SurfConextResponsibleTest.php diff --git a/.env.test b/.env.test index 13f43426d..d126baebb 100644 --- a/.env.test +++ b/.env.test @@ -8,3 +8,5 @@ DATABASE_URL="sqlite:////tmp/spdashboard-webtests.sqlite" PANTHER_APP_ENV=panther administrator_teams="'urn:collab:group:dev.openconext.local:dev:openconext:local:spd_admin'" jira_test_mode_storage_path='/var/www/html/var/issues.json' +authorization_attribute_name='eduPersonEntitlement' +surfconext_responsible_authorization='urn:mace:surfnet.nl:surfnet.nl:sab:role:SURFconext-verantwoordelijke' diff --git a/ci/qa/phpstan-baseline.php b/ci/qa/phpstan-baseline.php index f492ea793..9328ec9c5 100644 --- a/ci/qa/phpstan-baseline.php +++ b/ci/qa/phpstan-baseline.php @@ -1731,11 +1731,6 @@ 'count' => 1, 'path' => __DIR__ . '/../../src/Surfnet/ServiceProviderDashboard/Application/Service/EntityService.php', ]; -$ignoreErrors[] = [ - 'message' => '#^Method Surfnet\\\\ServiceProviderDashboard\\\\Application\\\\Service\\\\EntityService\\:\\:findPublishedTestEntitiesByInstitutionId\\(\\) has no return type specified\\.$#', - 'count' => 1, - 'path' => __DIR__ . '/../../src/Surfnet/ServiceProviderDashboard/Application/Service/EntityService.php', -]; $ignoreErrors[] = [ 'message' => '#^PHPDoc tag @param references unknown parameter\\: \\$service$#', 'count' => 1, @@ -1771,6 +1766,11 @@ 'count' => 1, 'path' => __DIR__ . '/../../src/Surfnet/ServiceProviderDashboard/Application/Service/LoadEntityService.php', ]; +$ignoreErrors[] = [ + 'message' => '#^Parameter \\#1 \\$entityName of class Surfnet\\\\ServiceProviderDashboard\\\\Application\\\\ViewObject\\\\EntityConnection constructor expects string, string\\|null given\\.$#', + 'count' => 2, + 'path' => __DIR__ . '/../../src/Surfnet/ServiceProviderDashboard/Application/Service/ServiceConnectionService.php', +]; $ignoreErrors[] = [ 'message' => '#^Method Surfnet\\\\ServiceProviderDashboard\\\\Application\\\\Service\\\\ServiceService\\:\\:getServiceNamesById\\(\\) return type has no value type specified in iterable type array\\.$#', 'count' => 1, @@ -2671,11 +2671,6 @@ 'count' => 1, 'path' => __DIR__ . '/../../src/Surfnet/ServiceProviderDashboard/Domain/Entity/Service.php', ]; -$ignoreErrors[] = [ - 'message' => '#^Property Surfnet\\\\ServiceProviderDashboard\\\\Domain\\\\Entity\\\\Service\\:\\:\\$createdAt is unused\\.$#', - 'count' => 1, - 'path' => __DIR__ . '/../../src/Surfnet/ServiceProviderDashboard/Domain/Entity/Service.php', -]; $ignoreErrors[] = [ 'message' => '#^Method Surfnet\\\\ServiceProviderDashboard\\\\Domain\\\\Mailer\\\\Mailer\\:\\:send\\(\\) has no return type specified\\.$#', 'count' => 1, @@ -3431,6 +3426,11 @@ 'count' => 1, 'path' => __DIR__ . '/../../src/Surfnet/ServiceProviderDashboard/Infrastructure/DashboardBundle/Controller/ServiceConnectionsController.php', ]; +$ignoreErrors[] = [ + 'message' => '#^Parameter \\#2 \\$service of method Surfnet\\\\ServiceProviderDashboard\\\\Application\\\\Service\\\\ServiceConnectionService\\:\\:findByInstitutionId\\(\\) expects Surfnet\\\\ServiceProviderDashboard\\\\Domain\\\\Entity\\\\Service\\|null, Surfnet\\\\ServiceProviderDashboard\\\\Domain\\\\Entity\\\\Service\\|false given\\.$#', + 'count' => 1, + 'path' => __DIR__ . '/../../src/Surfnet/ServiceProviderDashboard/Infrastructure/DashboardBundle/Controller/ServiceConnectionsController.php', +]; $ignoreErrors[] = [ 'message' => '#^Call to an undefined method Symfony\\\\Component\\\\Form\\\\FormInterface\\:\\:getClickedButton\\(\\)\\.$#', 'count' => 2, diff --git a/config/packages/security.yaml b/config/packages/security.yaml index 61786b35f..371ab5245 100644 --- a/config/packages/security.yaml +++ b/config/packages/security.yaml @@ -1,7 +1,6 @@ security: role_hierarchy: ROLE_ADMINISTRATOR: ROLE_USER - ROLE_SURFCONEXT_RESPONSIBLE: ROLE_USER providers: saml-provider: diff --git a/src/Surfnet/ServiceProviderDashboard/Application/Service/EntityService.php b/src/Surfnet/ServiceProviderDashboard/Application/Service/EntityService.php index 036276009..3f55b6903 100644 --- a/src/Surfnet/ServiceProviderDashboard/Application/Service/EntityService.php +++ b/src/Surfnet/ServiceProviderDashboard/Application/Service/EntityService.php @@ -243,6 +243,9 @@ public function findPublishedTestEntitiesByTeamName(string $teamName): ?array } + /** + * @return array|null + */ public function findPublishedTestEntitiesByInstitutionId(InstitutionId $institutionId) { return $this->queryRepositoryProvider diff --git a/src/Surfnet/ServiceProviderDashboard/Application/Service/ServiceConnectionService.php b/src/Surfnet/ServiceProviderDashboard/Application/Service/ServiceConnectionService.php index 8e61f7d6e..6bd5844e7 100644 --- a/src/Surfnet/ServiceProviderDashboard/Application/Service/ServiceConnectionService.php +++ b/src/Surfnet/ServiceProviderDashboard/Application/Service/ServiceConnectionService.php @@ -23,7 +23,9 @@ use Surfnet\ServiceProviderDashboard\Application\ViewObject\EntityConnection; use Surfnet\ServiceProviderDashboard\Application\ViewObject\EntityConnectionCollection; use Surfnet\ServiceProviderDashboard\Domain\Entity\Constants; +use Surfnet\ServiceProviderDashboard\Domain\Entity\Entity\MetaData; use Surfnet\ServiceProviderDashboard\Domain\Entity\IdentityProvider; +use Surfnet\ServiceProviderDashboard\Domain\Entity\ManageEntity; use Surfnet\ServiceProviderDashboard\Domain\Entity\Service; use Surfnet\ServiceProviderDashboard\Domain\ValueObject\InstitutionId; @@ -56,22 +58,22 @@ public function findByServices(array $services): EntityConnectionCollection $this->addEntitiesToCollection( $collection, $institutionId, - $service, $this->getTestIdpsIndexed(), + $service, ); } return $collection; } - public function findByInstitutionId(InstitutionId $institutionId, Service $service): EntityConnectionCollection + public function findByInstitutionId(InstitutionId $institutionId, ?Service $service): EntityConnectionCollection { $collection = new EntityConnectionCollection(); $this->addIdpList($collection); $this->addEntitiesToCollection( $collection, $institutionId, - $service, $this->getTestIdpsIndexed(), + $service, ); return $collection; } @@ -95,8 +97,8 @@ private function addIdpList(EntityConnectionCollection $collection): void private function addEntitiesToCollection( EntityConnectionCollection $collection, InstitutionId $institutionId, - Service $service, - array $testIdpsIndexed + array $testIdpsIndexed, + ?Service $service = null, ): void { $list = []; $entities = $this->entityService->findPublishedTestEntitiesByInstitutionId($institutionId); @@ -110,27 +112,12 @@ private function addEntitiesToCollection( // Skipping irrelevant entity types continue; } - $metadata = $entity->getMetaData(); - if ($metadata === null) { - throw new RuntimeException( - sprintf( - 'No metadata available on entity with manage id: %s', - $entity->getId() - ) - ); - } - if ($metadata->getNameEn() === null) { - throw new RuntimeException( - sprintf( - 'No name:en available for entity with manage id: %s', - $entity->getId() - ) - ); - } + $metadata = $this->assertValidMetadata($entity); if ($entity->getAllowedIdentityProviders()->isAllowAll()) { + $serviceName = $service !== null ? $service->getOrganizationNameEn() : 'Unknown service name'; $list[$entity->getId()] = new EntityConnection( $metadata->getNameEn(), - $service->getOrganizationNameEn(), + $serviceName, $testIdpsIndexed, $testIdpsIndexed, ); @@ -145,7 +132,7 @@ private function addEntitiesToCollection( $list[$entity->getId()] = new EntityConnection( $metadata->getNameEn(), - $service->getOrganizationNameEn(), + $service !== null ? $service->getOrganizationNameEn() : 'Unknown service name', $testIdpsIndexed, $connectedIdps, ); @@ -168,4 +155,26 @@ private function getTestIdpsIndexed(): array return $testIdpsIndexed; } + + private function assertValidMetadata(ManageEntity $entity): MetaData + { + $metadata = $entity->getMetaData(); + if ($metadata === null) { + throw new RuntimeException( + sprintf( + 'No metadata available on entity with manage id: %s', + $entity->getId() + ) + ); + } + if ($metadata->getNameEn() === null) { + throw new RuntimeException( + sprintf( + 'No name:en available for entity with manage id: %s', + $entity->getId() + ) + ); + } + return $metadata; + } } diff --git a/src/Surfnet/ServiceProviderDashboard/Application/ViewObject/EntityConnectionCollection.php b/src/Surfnet/ServiceProviderDashboard/Application/ViewObject/EntityConnectionCollection.php index 11cea8d71..8e1414b2b 100644 --- a/src/Surfnet/ServiceProviderDashboard/Application/ViewObject/EntityConnectionCollection.php +++ b/src/Surfnet/ServiceProviderDashboard/Application/ViewObject/EntityConnectionCollection.php @@ -32,6 +32,11 @@ class EntityConnectionCollection */ private array $collection = []; + public static function empty(): self + { + return new self(); + } + public function addIdpList(array $idps): void { $this->idpList = $this->idpList + $idps; diff --git a/src/Surfnet/ServiceProviderDashboard/Domain/ValueObject/IdpCollection.php b/src/Surfnet/ServiceProviderDashboard/Domain/ValueObject/IdpCollection.php index 144c93380..3adcba4a4 100644 --- a/src/Surfnet/ServiceProviderDashboard/Domain/ValueObject/IdpCollection.php +++ b/src/Surfnet/ServiceProviderDashboard/Domain/ValueObject/IdpCollection.php @@ -25,10 +25,10 @@ class IdpCollection { /** @var IdentityProvider[] */ - private array $testEntities; + private array $testEntities = []; /** @var IdentityProvider[] */ - private array $institutionEntities; + private array $institutionEntities = []; /** * @param EntityId[] $testEntities diff --git a/src/Surfnet/ServiceProviderDashboard/Infrastructure/DashboardBundle/Controller/EntrypointController.php b/src/Surfnet/ServiceProviderDashboard/Infrastructure/DashboardBundle/Controller/EntrypointController.php new file mode 100644 index 000000000..af3d368a1 --- /dev/null +++ b/src/Surfnet/ServiceProviderDashboard/Infrastructure/DashboardBundle/Controller/EntrypointController.php @@ -0,0 +1,39 @@ +isGranted('ROLE_SURFCONEXT_RESPONSIBLE')) { + return $this->redirectToRoute('service_connections'); + } + // Otherwise start at the service overview page + return $this->redirectToRoute('service_overview'); + } +} diff --git a/src/Surfnet/ServiceProviderDashboard/Infrastructure/DashboardBundle/Controller/ServiceConnectionsController.php b/src/Surfnet/ServiceProviderDashboard/Infrastructure/DashboardBundle/Controller/ServiceConnectionsController.php index f6e04dde7..580eaa893 100644 --- a/src/Surfnet/ServiceProviderDashboard/Infrastructure/DashboardBundle/Controller/ServiceConnectionsController.php +++ b/src/Surfnet/ServiceProviderDashboard/Infrastructure/DashboardBundle/Controller/ServiceConnectionsController.php @@ -21,6 +21,7 @@ use Surfnet\ServiceProviderDashboard\Application\Exception\RuntimeException; use Surfnet\ServiceProviderDashboard\Application\Service\ServiceConnectionService; use Surfnet\ServiceProviderDashboard\Application\Service\ServiceService; +use Surfnet\ServiceProviderDashboard\Application\ViewObject\EntityConnectionCollection; use Surfnet\ServiceProviderDashboard\Domain\ValueObject\InstitutionId; use Surfnet\ServiceProviderDashboard\Infrastructure\DashboardBundle\Exception\InstitutionIdNotFoundException; use Surfnet\ServiceProviderDashboard\Infrastructure\DashboardBundle\Service\AuthorizationService; @@ -54,18 +55,21 @@ public function myServices() } if ($serviceCount === 1) { $service = reset($services); - $institutionId = $this->getUser()->getContact()->getInstitutionId(); - if ($institutionId === null) { - throw new InstitutionIdNotFoundException('Institution id not found on Contact'); - } $entities = $this->serviceConnectionService->findByInstitutionId( - new InstitutionId($institutionId), + $this->getInstitutionId(), $service ); } + $isSurfConextRepresentative = $serviceCount === 0 && empty($allowedServices); + if ($isSurfConextRepresentative) { + $entities = $this->serviceConnectionService->findByInstitutionId( + $this->getInstitutionId(), + null + ); + } if (!isset($entities)) { - throw new RuntimeException('The Institution SP entities could not be read.'); + $entities = EntityConnectionCollection::empty(); } return $this->render( @@ -76,4 +80,13 @@ public function myServices() ] ); } + + private function getInstitutionId(): InstitutionId + { + $institutionId = $this->getUser()->getContact()->getInstitutionId(); + if ($institutionId === null) { + throw new InstitutionIdNotFoundException('Institution id not found on Contact'); + } + return new InstitutionId($institutionId); + } } diff --git a/src/Surfnet/ServiceProviderDashboard/Infrastructure/DashboardBundle/Controller/ServiceController.php b/src/Surfnet/ServiceProviderDashboard/Infrastructure/DashboardBundle/Controller/ServiceController.php index ff4c5ef29..61e79a8a1 100644 --- a/src/Surfnet/ServiceProviderDashboard/Infrastructure/DashboardBundle/Controller/ServiceController.php +++ b/src/Surfnet/ServiceProviderDashboard/Infrastructure/DashboardBundle/Controller/ServiceController.php @@ -71,7 +71,7 @@ public function __construct( } #[IsGranted('ROLE_USER')] - #[Route(path: '/', name: 'service_overview', methods: ['GET'])] + #[Route(path: '/service-overview', name: 'service_overview', methods: ['GET'])] public function overview(Request $request): RedirectResponse|Response { $allowedServices = $this->authorizationService->getAllowedServiceNamesById(); diff --git a/src/Surfnet/ServiceProviderDashboard/Infrastructure/DashboardBundle/DataFixtures/ORM/WebTestFixtures.php b/src/Surfnet/ServiceProviderDashboard/Infrastructure/DashboardBundle/DataFixtures/ORM/WebTestFixtures.php index 47fbf0660..15b840865 100644 --- a/src/Surfnet/ServiceProviderDashboard/Infrastructure/DashboardBundle/DataFixtures/ORM/WebTestFixtures.php +++ b/src/Surfnet/ServiceProviderDashboard/Infrastructure/DashboardBundle/DataFixtures/ORM/WebTestFixtures.php @@ -41,6 +41,12 @@ public function load(ObjectManager $manager): void $privacyQuestions = $this->createPrivacyQuestions($service); $manager->persist($privacyQuestions); + // A third service is used to test the SURFConext responsible role which pivots on entities with institution_id manage entities + $service = $this->createService('Acme Corporation', 'urn:collab:group:vm.openconext.org:demo:openconext:org:acme.com'); + $service->setServiceType(Service::SERVICE_TYPE_INSTITUTE); + $service->setInstitutionId('ACME Corporation'); + $manager->persist($service); + $manager->flush(); } diff --git a/src/Surfnet/ServiceProviderDashboard/Infrastructure/DashboardSamlBundle/Security/Authentication/Provider/SamlProvider.php b/src/Surfnet/ServiceProviderDashboard/Infrastructure/DashboardSamlBundle/Security/Authentication/Provider/SamlProvider.php index 81d14ba2b..fe7301c0f 100644 --- a/src/Surfnet/ServiceProviderDashboard/Infrastructure/DashboardSamlBundle/Security/Authentication/Provider/SamlProvider.php +++ b/src/Surfnet/ServiceProviderDashboard/Infrastructure/DashboardSamlBundle/Security/Authentication/Provider/SamlProvider.php @@ -128,8 +128,9 @@ private function assignRoles( // Default to the ROLE_USER role for services. $role = 'ROLE_USER'; if ($authorizations && $authorizations->isSurfConextResponsible()) { + $role = 'ROLE_SURFCONEXT_RESPONSIBLE'; $contact->setInstitutionId($authorizations->getOrganizationCode()); - $contact->assignRole('ROLE_SURFCONEXT_RESPONSIBLE'); + $contact->assignRole($role); } if (array_intersect($this->administratorTeams, $teamNames) !== []) { diff --git a/tests/webtests/SurfConextResponsibleTest.php b/tests/webtests/SurfConextResponsibleTest.php new file mode 100644 index 000000000..31eca756e --- /dev/null +++ b/tests/webtests/SurfConextResponsibleTest.php @@ -0,0 +1,37 @@ +loadFixtures(); + $this->teamsQueryClient->registerTeam('demo:openconext:org:surf.nl', 'data'); + $this->logInSurfConextResponsible('ACME Corporation'); + } + + public function test_after_login_i_am_on_connections_page() + { + $crawler = self::$pantherClient->getCrawler(); + self::assertEquals('/connections', $crawler->getBaseHref()); + } +} diff --git a/tests/webtests/WebTestCase.php b/tests/webtests/WebTestCase.php index ced2e81ad..741670a1f 100644 --- a/tests/webtests/WebTestCase.php +++ b/tests/webtests/WebTestCase.php @@ -88,6 +88,9 @@ class WebTestCase extends PantherTestCase protected DevelopmentIssueRepository $jiraIssueRepository; + private string $surfConextRepresentativeAttributeName = ''; + private string $surfConextRepresentativeAttributeValue = ''; + public static function setUpBeforeClass(): void { exec('cd /var/www/html && composer dump-env test -q && chmod 777 /tmp/spdashboard-webtests.sqlite'); @@ -162,6 +165,10 @@ public function setUp(): void $this->teamsQueryClient->reset(); $this->jiraIssueRepository = self::getContainer() ->get('surfnet.dashboard.repository.issue'); + $this->surfConextRepresentativeAttributeName = self::getContainer() + ->getParameter('surfnet.dashboard.security.authentication.authorization_attribute_name'); + $this->surfConextRepresentativeAttributeValue = self::getContainer() + ->getParameter('surfnet.dashboard.security.authentication.surfconext_responsible_authorization'); } protected function registerManageEntity( @@ -323,7 +330,7 @@ protected function logOut() self::$pantherClient->restart(); } - protected function logIn(Service $service = null, Service $secondService = null) + protected function logIn(Service $service = null, Service $secondService = null): Crawler { $crawler = self::$pantherClient->request('GET', 'https://spdashboard.dev.openconext.local'); @@ -347,6 +354,44 @@ protected function logIn(Service $service = null, Service $secondService = null) $isMemberOf = $crawler->filter('input[name="urn:mace:dir:attribute-def:isMemberOf"]')->eq(1); $isMemberOf->sendKeys($secondTeamName); } + + return $this->finishLogin(); + } + + protected function logInSurfConextResponsible(string $institutionId): void + { + $crawler = self::$pantherClient->request('GET', 'https://spdashboard.dev.openconext.local'); + + $form = $crawler->findElement(WebDriverBy::cssSelector('form.login-form')); + $this->fillFormField($form, '#username', 'John Dart'); + $this->fillFormField($form, '#password', 'secret'); + + $select = $crawler->filterXPath( + sprintf( + ".//select[@id='add-attribute']//option[@value='urn:mace:dir:attribute-def:%s']", + $this->surfConextRepresentativeAttributeName + ) + ); + $select->click(); + $entitlement = $crawler->filter(sprintf('input[name="urn:mace:dir:attribute-def:%s"]', $this->surfConextRepresentativeAttributeName)); + $entitlement->sendKeys($institutionId); + // Now also send the attribute value that indicates this user is of role SurfConext representative + $select = $crawler->filterXPath( + sprintf( + ".//select[@id='add-attribute']//option[@value='urn:mace:dir:attribute-def:%s']", + $this->surfConextRepresentativeAttributeName + ) + ); + $select->click(); + $entitlement = $crawler->filter(sprintf('input[name="urn:mace:dir:attribute-def:%s"]', $this->surfConextRepresentativeAttributeName)); + $entitlement->sendKeys($this->surfConextRepresentativeAttributeValue); + + $this->finishLogin(); + self::$pantherClient->takeScreenshot('Foobar.png'); die; + } + + private function finishLogin(): Crawler + { self::findBy('.button')->click(); $crawler = self::$pantherClient->refreshCrawler(); @@ -363,7 +408,7 @@ protected function logIn(Service $service = null, Service $secondService = null) * @return Crawler * @throws InvalidArgumentException */ - protected function switchToService($serviceName): \Symfony\Component\DomCrawler\Crawler + protected function switchToService($serviceName): Crawler { $service = $this->getServiceRepository()->findByName($serviceName); From e66a0ff5bdbeb4ddbbf2af850794c2364a70e525 Mon Sep 17 00:00:00 2001 From: Michiel Kodde Date: Tue, 18 Jun 2024 11:13:15 +0200 Subject: [PATCH 2/2] Test the SURF conext representative page The missing web test coverage was added in this commit. Some notes: - For now all IdPs are allowed for every SP (via allowedall) - Only one test idp is configured, that could be expanded upon --- .env.test | 1 + tests/webtests/Manage/Client/ClientResult.php | 13 +- .../Client/FakeIdentityProviderClient.php | 44 ++++++- .../Manage/Client/FakeQueryClient.php | 6 +- .../webtests/Manage/Client/template/ccc.json | 1 + .../Manage/Client/template/oidc10.json | 1 + .../Manage/Client/template/saml20_idp.json | 3 +- .../Manage/Client/template/saml20_sp.json | 1 + tests/webtests/SurfConextResponsibleTest.php | 118 +++++++++++++++++- tests/webtests/WebTestCase.php | 39 ++++-- 10 files changed, 201 insertions(+), 26 deletions(-) diff --git a/.env.test b/.env.test index d126baebb..b828296cb 100644 --- a/.env.test +++ b/.env.test @@ -10,3 +10,4 @@ administrator_teams="'urn:collab:group:dev.openconext.local:dev:openconext:local jira_test_mode_storage_path='/var/www/html/var/issues.json' authorization_attribute_name='eduPersonEntitlement' surfconext_responsible_authorization='urn:mace:surfnet.nl:surfnet.nl:sab:role:SURFconext-verantwoordelijke' +test_idp_entity_ids='["http://mock-idp","test-idp-1"]' diff --git a/tests/webtests/Manage/Client/ClientResult.php b/tests/webtests/Manage/Client/ClientResult.php index 4db95df02..8d4c31407 100644 --- a/tests/webtests/Manage/Client/ClientResult.php +++ b/tests/webtests/Manage/Client/ClientResult.php @@ -39,13 +39,16 @@ class ClientResult implements ClientResultInterface private $teamName; + private string $institutionId; + public function __construct( string $protocol, string $id, string $entityId, ?string $metadataUrl, string $name, - ?string $teamName + ?string $teamName, + ?string $institutionId, ) { $this->id = $id; $this->protocol = $protocol; @@ -56,6 +59,7 @@ public function __construct( $this->metadataUrl = $metadataUrl; $this->name = $name; $this->teamName = $teamName; + $this->institutionId = $institutionId; if ($teamName === null) { $this->teamName = 'urn:collab:group:vm.openconext.org:demo:openconext:org:surf.nl'; } @@ -87,7 +91,8 @@ public function getEntityResult(): array $this->metadataUrl, $this->name, str_replace('_', '-', $this->protocol), - $this->teamName + $this->teamName, + $this->institutionId ); return json_decode($data, true); } @@ -116,7 +121,8 @@ public static function decode($data): self $data['entityId'], $data['metadataUrl'], $data['name'], - $data['teamName'] + $data['teamName'], + $data['institutionId'], ); } @@ -129,6 +135,7 @@ public function encode(): array 'metadataUrl' => $this->metadataUrl, 'name' => $this->name, 'teamName' => $this->teamName, + 'institutionId' => $this->institutionId, ]; } } diff --git a/tests/webtests/Manage/Client/FakeIdentityProviderClient.php b/tests/webtests/Manage/Client/FakeIdentityProviderClient.php index 51cd1b02e..9df27e6ed 100644 --- a/tests/webtests/Manage/Client/FakeIdentityProviderClient.php +++ b/tests/webtests/Manage/Client/FakeIdentityProviderClient.php @@ -26,14 +26,16 @@ class FakeIdentityProviderClient implements IdentityProviderRepository { + private string $path = __DIR__ . '/../../../../var/webtest-idps.json'; /** * @var ClientResult[] */ private $entities = []; - public function registerEntity(string $protocol, string $id, string $entityId, string $name) + public function registerEntity(string $protocol, string $id, string $entityId, string $name, string $institutionId = '') { - $this->entities[$id] = new ClientResult($protocol, $id, $entityId, null, $name, null); + $this->entities[$id] = new ClientResult($protocol, $id, $entityId, null, $name, null, $institutionId); + $this->storeEntities(); } /** @@ -41,6 +43,7 @@ public function registerEntity(string $protocol, string $id, string $entityId, s */ public function findAll() { + $this->load(); $list = []; foreach ($this->entities as $manageResult) { $list[] = IdentityProviderFactory::fromManageResult($manageResult->getEntityResult()); @@ -50,6 +53,7 @@ public function findAll() public function findByEntityId(EntityId $entityId): ?IdentityProvider { + $this->load(); foreach ($this->entities as $manageResult) { $entity = IdentityProviderFactory::fromManageResult($manageResult->getEntityResult()); if ($entity->getEntityId() === (string) $entityId) { @@ -66,10 +70,46 @@ public function findByEntityId(EntityId $entityId): ?IdentityProvider */ public function findByInstitutionId(InstitutionId $institutionId): array { + $this->load(); $list = []; foreach ($this->entities as $manageResult) { $list[] = IdentityProviderFactory::fromManageResult($manageResult->getEntityResult()); } return $list; } + + + private function read() + { + return json_decode(file_get_contents($this->path), true); + } + + private function write(array $data) + { + file_put_contents($this->path, json_encode($data)); + } + + private function storeEntities() + { + // Also store the new entity in the on-file storage + $data = []; + foreach ($this->entities as $identifier => $entity) { + $data[$identifier] = $entity->encode(); + } + $this->write($data); + } + + private function load() + { + $data = $this->read(); + foreach ($data as $id => $rawClientResult) { + if (array_key_exists('protocol', $rawClientResult)) { + $this->entities[$id] = ClientResult::decode($rawClientResult); + continue; + } + if (array_key_exists('json', $rawClientResult)) { + $this->entities[$id] = ClientResultRaw::decode($rawClientResult); + } + } + } } diff --git a/tests/webtests/Manage/Client/FakeQueryClient.php b/tests/webtests/Manage/Client/FakeQueryClient.php index 6248c6dc4..b4bf2f456 100644 --- a/tests/webtests/Manage/Client/FakeQueryClient.php +++ b/tests/webtests/Manage/Client/FakeQueryClient.php @@ -55,9 +55,10 @@ public function registerEntity( string $entityId, ?string $metadataUrl, string $name, - ?string $teamName = null + ?string $teamName = null, + ?string $institutionId = '', ) { - $this->entities[$id] = new ClientResult($protocol, $id, $entityId, $metadataUrl, $name, $teamName); + $this->entities[$id] = new ClientResult($protocol, $id, $entityId, $metadataUrl, $name, $teamName, $institutionId); $this->storeEntities(); } @@ -142,6 +143,7 @@ public function findResourceServerByEntityId($entityId, $state) $searchResults[] = ManageEntity::fromApiResponse($result); } } + return $searchResults; } public function findByManageIdAndProtocol(string $manageId, string $protocol) :? ManageEntity diff --git a/tests/webtests/Manage/Client/template/ccc.json b/tests/webtests/Manage/Client/template/ccc.json index 7d9c086ed..7bb4ccd92 100644 --- a/tests/webtests/Manage/Client/template/ccc.json +++ b/tests/webtests/Manage/Client/template/ccc.json @@ -36,6 +36,7 @@ "OrganizationDisplayName:nl": "%5$s Organisation Name Dutch", "OrganizationURL:nl": "%5$s Organisation Url Dutch", "coin:service_team_id": "%7$s", + "coin:institution_id": "%8$s", "isPublicClient": true }, "allowedEntities": [], diff --git a/tests/webtests/Manage/Client/template/oidc10.json b/tests/webtests/Manage/Client/template/oidc10.json index ce6fb8923..61dda5267 100644 --- a/tests/webtests/Manage/Client/template/oidc10.json +++ b/tests/webtests/Manage/Client/template/oidc10.json @@ -64,6 +64,7 @@ "OrganizationDisplayName:nl": "%5$s Organisation Name Dutch", "OrganizationURL:nl": "%5$s Organisation Url Dutch", "coin:service_team_id": "%7$s", + "coin:institution_id": "%8$s", "isPublicClient": true }, "allowedEntities": [], diff --git a/tests/webtests/Manage/Client/template/saml20_idp.json b/tests/webtests/Manage/Client/template/saml20_idp.json index 3f6e8f5ab..52a612d3f 100644 --- a/tests/webtests/Manage/Client/template/saml20_idp.json +++ b/tests/webtests/Manage/Client/template/saml20_idp.json @@ -21,7 +21,8 @@ "metaDataFields": { "name:en": "%5$s Name English", "name:nl": "%5$s Name Dutch", - "coin:service_team_id": "%7$s" + "coin:service_team_id": "%7$s", + "coin:institution_id": "%8$s" }, "eid": 31 } diff --git a/tests/webtests/Manage/Client/template/saml20_sp.json b/tests/webtests/Manage/Client/template/saml20_sp.json index a53726291..0929bf7c9 100644 --- a/tests/webtests/Manage/Client/template/saml20_sp.json +++ b/tests/webtests/Manage/Client/template/saml20_sp.json @@ -64,6 +64,7 @@ "OrganizationDisplayName:nl": "%5$s Organisation Name Dutch", "OrganizationURL:nl": "%5$s Organisation Url Dutch", "coin:service_team_id": "%7$s", + "coin:institution_id": "%8$s", "logo:0:url": "%3$s\/images\/logo.png", "logo:0:width": 100, "logo:0:height": 100, diff --git a/tests/webtests/SurfConextResponsibleTest.php b/tests/webtests/SurfConextResponsibleTest.php index 31eca756e..a9a6ba202 100644 --- a/tests/webtests/SurfConextResponsibleTest.php +++ b/tests/webtests/SurfConextResponsibleTest.php @@ -20,18 +20,126 @@ class SurfConextResponsibleTest extends WebTestCase { + private string $institutionId = 'ACME Corporation'; public function setUp(): void { parent::setUp(); - $this->loadFixtures(); - $this->teamsQueryClient->registerTeam('demo:openconext:org:surf.nl', 'data'); - $this->logInSurfConextResponsible('ACME Corporation'); + $this->teamsQueryClient->registerTeam('demo:openconext:org:acme.nl', 'data'); } public function test_after_login_i_am_on_connections_page() { - $crawler = self::$pantherClient->getCrawler(); - self::assertEquals('/connections', $crawler->getBaseHref()); + $this->logInSurfConextResponsible($this->institutionId); + $url = self::$pantherClient->getCurrentURL(); + $urlParts = parse_url($url); + self::assertEquals('/connections', $urlParts['path']); + self::assertOnPage('No entities found'); // At this point there should be no entities + } + + public function test_entities_are_listed_on_the_page() + { + $this->registerManageEntity( + 'test', + 'saml20_sp', + 'aee8f00d-428a-4fbc-9cf8-ad2f3b2af589', + 'ACME Anvil', + 'http://acme-anvil', + 'https://acme-anvil.example.com/metadata', + 'demo:openconext:org:acme.nl', + $this->institutionId, + ); + $this->logInSurfConextResponsible($this->institutionId); + $this->assertOnPage('ACME Anvil Name English'); + // When logging in with only the SURF representative, we do not know the service the entity is associated with + $this->assertOnPage('Unknown service name'); + } + + public function test_entities_are_listed_on_the_page_with_connected_idp() + { + $this->registerManageEntity( + 'test', + 'saml20_sp', + 'aee8f00d-428a-4fbc-9cf8-ad2f3b2af589', + 'ACME Anvil', + 'http://acme-anvil', + 'https://acme-anvil.example.com/metadata', + 'demo:openconext:org:acme.nl', + $this->institutionId, + ); + $this->registerManageEntity( + 'test', + 'saml20_idp', + '1d4abec3-3f67-4b8a-b90d-ce56a3b0abc5', + 'Test IdP', + 'test-idp-1', + 'https://test-idp/metadata', + 'demo:openconext:org:acme.nl', + $this->institutionId, + ); + $this->logInSurfConextResponsible($this->institutionId); + $this->assertOnPage('ACME Anvil Name English'); + $this->assertOnPage('Test IdP Name Dutch'); + } + + public function test_entities_are_listed_on_the_page_with_connected_idp_with_multiple_sps() + { + $this->registerManageEntity( + 'test', + 'saml20_sp', + 'aee8f00d-428a-4fbc-9cf8-ad2f3b2af589', + 'ACME Anvil 1', + 'http://acme-anvil-1', + 'https://acme-anvil.example.com/metadata', + 'demo:openconext:org:acme.nl', + $this->institutionId, + ); + $this->registerManageEntity( + 'test', + 'saml20_sp', + 'bee8f00d-428a-4fbc-9cf8-ad2f3b2af589', + 'ACME Anvil 2', + 'http://acme-anvil-2', + 'https://acme-anvil.example.com/metadata', + 'demo:openconext:org:acme.nl', + $this->institutionId, + ); + $this->registerManageEntity( + 'test', + 'saml20_sp', + 'cee8f00d-428a-4fbc-9cf8-ad2f3b2af589', + 'ACME Anvil 3', + 'http://acme-anvil-3', + 'https://acme-anvil.example.com/metadata', + 'demo:openconext:org:acme.nl', + $this->institutionId, + ); + $this->registerManageEntity( + 'test', + 'saml20_sp', + 'fee8f00d-428a-4fbc-9cf8-ad2f3b2af589', + 'Should not be on page', + 'http://foobar', + 'https://foobar.example.com/metadata', + 'demo:openconext:org:acme.nl', + 'not-acme', + ); + $this->registerManageEntity( + 'test', + 'saml20_idp', + '1d4abec3-3f67-4b8a-b90d-ce56a3b0abc5', + 'Test IdP', + 'test-idp-1', + 'https://test-idp/metadata', + 'demo:openconext:org:acme.nl', + $this->institutionId, + ); + $this->logInSurfConextResponsible($this->institutionId); + $this->assertOnPage('ACME Anvil 1 Name English'); + $this->assertOnPage('ACME Anvil 2 Name English'); + $this->assertOnPage('ACME Anvil 3 Name English'); + // The fourth SP should not show up on the page + $this->assertNotOnPage('Should not be on page'); + $this->assertOnPage('Test IdP Name Dutch'); } } diff --git a/tests/webtests/WebTestCase.php b/tests/webtests/WebTestCase.php index 741670a1f..1b3d578a1 100644 --- a/tests/webtests/WebTestCase.php +++ b/tests/webtests/WebTestCase.php @@ -178,7 +178,8 @@ protected function registerManageEntity( string $name, string $entityId, ?string $metadataUrl = null, - ?string $teamName = null + ?string $teamName = null, + ?string $institutionId = '', ) { switch ($protocol) { case 'saml20_sp': @@ -191,11 +192,12 @@ protected function registerManageEntity( $name, $entityId, $metadataUrl, - $teamName + $teamName, + $institutionId, ); break; case 'saml20_idp': - $this->registerIdP($env, $protocol, $id, $name, $entityId); + $this->registerIdP($env, $protocol, $id, $name, $entityId, $institutionId); break; } } @@ -236,7 +238,8 @@ private function registerSp( string $name, string $entityId, ?string $metadataUrl = null, - ?string $teamName = null + ?string $teamName = null, + ?string $institutionId = '', ) { switch ($env) { case 'production': @@ -246,7 +249,8 @@ private function registerSp( $entityId, $metadataUrl, $name, - $teamName + $teamName, + $institutionId ); break; case 'test': @@ -256,7 +260,8 @@ private function registerSp( $entityId, $metadataUrl, $name, - $teamName + $teamName, + $institutionId ); break; default: @@ -264,7 +269,7 @@ private function registerSp( } } - private function registerIdP(string $env, string $protocol, string $id, string $name, string $entityId) + private function registerIdP(string $env, string $protocol, string $id, string $name, string $entityId, string $institutionId = '') { switch ($env) { case 'production': @@ -272,7 +277,8 @@ private function registerIdP(string $env, string $protocol, string $id, string $ $protocol, $id, $entityId, - $name + $name, + $institutionId ); break; case 'test': @@ -280,7 +286,8 @@ private function registerIdP(string $env, string $protocol, string $id, string $ $protocol, $id, $entityId, - $name + $name, + $institutionId ); break; default: @@ -374,7 +381,7 @@ protected function logInSurfConextResponsible(string $institutionId): void ); $select->click(); $entitlement = $crawler->filter(sprintf('input[name="urn:mace:dir:attribute-def:%s"]', $this->surfConextRepresentativeAttributeName)); - $entitlement->sendKeys($institutionId); + $entitlement->sendKeys('urn:mace:surfnet.nl:surfnet.nl:sab:organizationCode:' . $institutionId); // Now also send the attribute value that indicates this user is of role SurfConext representative $select = $crawler->filterXPath( sprintf( @@ -383,11 +390,12 @@ protected function logInSurfConextResponsible(string $institutionId): void ) ); $select->click(); - $entitlement = $crawler->filter(sprintf('input[name="urn:mace:dir:attribute-def:%s"]', $this->surfConextRepresentativeAttributeName)); - $entitlement->sendKeys($this->surfConextRepresentativeAttributeValue); + $entitlement = $crawler + ->filter(sprintf('input[name="urn:mace:dir:attribute-def:%s"]', $this->surfConextRepresentativeAttributeName)) + ->eq(1); // There are now 2 entitlement attrs, pick the second + $entitlement->sendKeys($this->surfConextRepresentativeAttributeValue); $this->finishLogin(); - self::$pantherClient->takeScreenshot('Foobar.png'); die; } private function finishLogin(): Crawler @@ -491,4 +499,9 @@ protected function getAuthorizationService(): AuthorizationService { return self::getContainer()->get(AuthorizationService::class); } + + protected function screenshot(string $filename) + { + self::$pantherClient->takeScreenshot($filename); + } }