Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make Surf representative a role on its own #638

Merged
merged 2 commits into from
Jun 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .env.test
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,6 @@ 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'
test_idp_entity_ids='["http://mock-idp","test-idp-1"]'
20 changes: 10 additions & 10 deletions ci/qa/phpstan-baseline.php
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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',
];
Comment on lines +1769 to +1773
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Simple to fix with a ? in front of the variable?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, that could be a solution. But I do not want to loosen type safety, the creating side should be more defensive. And thats where this baseline entry came from.

$ignoreErrors[] = [
'message' => '#^Method Surfnet\\\\ServiceProviderDashboard\\\\Application\\\\Service\\\\ServiceService\\:\\:getServiceNamesById\\(\\) return type has no value type specified in iterable type array\\.$#',
'count' => 1,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
1 change: 0 additions & 1 deletion config/packages/security.yaml
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
security:
role_hierarchy:
ROLE_ADMINISTRATOR: ROLE_USER
ROLE_SURFCONEXT_RESPONSIBLE: ROLE_USER

providers:
saml-provider:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,9 @@ public function findPublishedTestEntitiesByTeamName(string $teamName): ?array
}


/**
* @return array<ManageEntity>|null
*/
public function findPublishedTestEntitiesByInstitutionId(InstitutionId $institutionId)
{
return $this->queryRepositoryProvider
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -56,22 +58,22 @@ public function findByServices(array $services): EntityConnectionCollection
$this->addEntitiesToCollection(
$collection,
$institutionId,
$service,
$this->getTestIdpsIndexed(),
$service,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could consider using named parameters, then the order doesn't matter

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I never considered using that. in this case the service argument became optional, and needed to be moved to the bottom of the stack.

Leaving this as-is. But thanks for the suggestion for using the NamedParameters. Going to read up on them.

);
}
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;
}
Expand All @@ -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);
Expand All @@ -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,
);
Expand All @@ -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,
);
Expand All @@ -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;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
<?php

declare(strict_types = 1);

/**
* Copyright 2024 SURFnet B.V.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

namespace Surfnet\ServiceProviderDashboard\Infrastructure\DashboardBundle\Controller;

use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;
use Symfony\Component\HttpFoundation\RedirectResponse;
use Symfony\Component\Routing\Attribute\Route;

class EntrypointController extends AbstractController
{
#[Route(path: '/', name: 'entrypoint', methods: ['GET'])]
public function __invoke(): RedirectResponse
{
// When surfconext responsible, go tho the connections page
if ($this->isGranted('ROLE_SURFCONEXT_RESPONSIBLE')) {
return $this->redirectToRoute('service_connections');
}
// Otherwise start at the service overview page
return $this->redirectToRoute('service_overview');
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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(
Expand All @@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) !== []) {
Expand Down
13 changes: 10 additions & 3 deletions tests/webtests/Manage/Client/ClientResult.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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';
}
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -116,7 +121,8 @@ public static function decode($data): self
$data['entityId'],
$data['metadataUrl'],
$data['name'],
$data['teamName']
$data['teamName'],
$data['institutionId'],
);
}

Expand All @@ -129,6 +135,7 @@ public function encode(): array
'metadataUrl' => $this->metadataUrl,
'name' => $this->name,
'teamName' => $this->teamName,
'institutionId' => $this->institutionId,
];
}
}
Loading
Loading