Skip to content

Commit

Permalink
Add validation: At least 1 idp must be selected
Browse files Browse the repository at this point in the history
Prior to this change, when no idp was selected in the Entity IDP
selection when publishing an Entity, the form validation would handle, and and error was thrown.

This change adds form validation that at least one IDP must be selected.

Resolves #1331
  • Loading branch information
johanib committed Dec 17, 2024
1 parent c9dd07b commit 676d082
Show file tree
Hide file tree
Showing 8 changed files with 189 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,10 @@
use Surfnet\ServiceProviderDashboard\Domain\Entity\Contact;
use Surfnet\ServiceProviderDashboard\Domain\Entity\IdentityProvider;
use Surfnet\ServiceProviderDashboard\Domain\Entity\ManageEntity;
use Surfnet\ServiceProviderDashboard\Infrastructure\DashboardBundle\Validator\Constraints\AtLeastOneSelected;
use Symfony\Component\Validator\Constraints as Assert;

#[AtLeastOneSelected(fieldNames: ['testEntities', 'institutionEntities'])]
class UpdateEntityIdpsCommand implements Command
{
public function __construct(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,3 +22,4 @@ validator.unique_redirect_urls.duplicate_found: 'Duplicate redirect URIs are not
validator.redirect_url.reverse_does_not_contain_client_id: 'A reverse redirect URL must have the client id as hostname.'
validator.type-of-service.min: 'You must choose at least one type of service'
validator.type-of-service.max: 'You are allowed to choose a maximum of 3 types of service'
validator.entity.idps.institution-idps: 'You must choose at least one IDP'
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
<?php

/**
* 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.
*/

declare(strict_types=1);

namespace Surfnet\ServiceProviderDashboard\Infrastructure\DashboardBundle\Validator\Constraints;

use Attribute;
use InvalidArgumentException;
use Symfony\Component\Validator\Attribute\HasNamedArguments;
use Symfony\Component\Validator\Constraint;

#[Attribute]
class AtLeastOneSelected extends Constraint
{

/**
* @param string[] $fieldNames
*/
#[HasNamedArguments]
public function __construct(
public array $fieldNames,
?array $groups = null,
) {
foreach ($this->fieldNames as $fieldName) {
if (!is_string($fieldName)) {
throw new InvalidArgumentException($fieldName . ' must be a string.');
}
}
parent::__construct([], $groups);
}

public function getTargets(): string
{
return self::CLASS_CONSTRAINT;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
<?php

/**
* 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.
*/

declare(strict_types=1);

namespace Surfnet\ServiceProviderDashboard\Infrastructure\DashboardBundle\Validator\Constraints;

use InvalidArgumentException;
use Symfony\Component\Validator\Constraint;
use Symfony\Component\Validator\ConstraintValidator;
use Symfony\Component\Validator\Exception\UnexpectedTypeException;

class AtLeastOneSelectedValidator extends ConstraintValidator
{
public function validate(mixed $value, Constraint $constraint): void
{
if (!$constraint instanceof AtLeastOneSelected) {
throw new UnexpectedTypeException($constraint, AtLeastOneSelected::class);
}

foreach ($constraint->fieldNames as $fieldName) {
if (!isset($value->{$fieldName}) || !is_array($value->{$fieldName})) {
throw new InvalidArgumentException('$value must have array field with name: "' . $fieldName. '"');
}

if (!empty(($value->{$fieldName}))) {
return;
}
}

$this->context->addViolation('validator.entity.idps.institution-idps');
}
}
4 changes: 3 additions & 1 deletion templates/EntityAcl/idps.html.twig
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,9 @@
{% endfor %}
{% endfor %}

{{ form_errors(form) }}
{% for error in form.vars.errors %}
<div class="message error">{{ error.message|trans }}</div>
{% endfor %}

<h2>{{ ('entity.idps.info.title')|trans }}</h2>
<div class="wysiwyg">{{ ('entity.idps.info.html')|trans|wysiwyg }}</div>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
<?php

/**
* 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.
*/

declare(strict_types=1);

namespace Surfnet\ServiceProviderDashboard\Tests\Integration\Infrastructure\DashboardBundle\Validator\Constraints;


use PHPUnit\Framework\TestCase;
use Surfnet\ServiceProviderDashboard\Infrastructure\DashboardBundle\Validator\Constraints\AtLeastOneSelected;
use Surfnet\ServiceProviderDashboard\Infrastructure\DashboardBundle\Validator\Constraints\AtLeastOneSelectedValidator;
use Symfony\Component\Validator\Context\ExecutionContextInterface;

class AtLeastOneSelectedValidatorTest extends TestCase
{
private $context;
private $validator;

protected function setUp(): void
{
$this->context = $this->createMock(ExecutionContextInterface::class);
$this->validator = new AtLeastOneSelectedValidator();
$this->validator->initialize($this->context);
}

public function testNoFieldSelected(): void
{
$constraint = new AtLeastOneSelected(fieldNames: ['field1', 'field2']);
$value = (object) ['field1' => [], 'field2' => []];

$this->context->expects($this->once())
->method('addViolation');

$this->validator->validate($value, $constraint);
}

public function testOneFieldSelected(): void
{
$constraint = new AtLeastOneSelected(fieldNames: ['field1', 'field2']);
$value = (object) ['field1' => [0], 'field2' => ''];

$this->context->expects($this->never())
->method('buildViolation');

$this->validator->validate($value, $constraint);
}

public function testAllFieldsSelected(): void
{
$constraint = new AtLeastOneSelected(fieldNames: ['field1', 'field2']);
$value = (object) ['field1' => [1], 'field2' => [2]];

$this->context->expects($this->never())
->method('buildViolation');

$this->validator->validate($value, $constraint);
}
}
5 changes: 4 additions & 1 deletion tests/webtests/EntityCreateOidcngTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -128,10 +128,13 @@ public function test_it_can_publish_the_form()
$crawler = self::$pantherClient->reload();
$this->assertOnPage('Connect some Idp\'s to your entity');

// Continue without selecting test IdPs
$form = $crawler
->selectButton('Save')
->form();

$checkbox = $crawler->filter('input[name="idp_entity[testEntities][]"][value="bfe8f00d-317a-4fbc-9cf8-ad2f3b2af578"]')->first();
$checkbox->click();

$crawler = self::$pantherClient->submit($form);

$label = $crawler->filter('.monospaced label')->first()->text();
Expand Down
7 changes: 6 additions & 1 deletion tests/webtests/EntityCreateSamlTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -265,10 +265,15 @@ public function test_it_can_publish_multiple_acs_locations()
);
$crawler = self::$pantherClient->reload();
$this->assertOnPage('Connect some Idp\'s to your entity');
// Continue without selecting test IdPs

$form = $crawler
->selectButton('Save')
->form();

$checkbox = $crawler->filter('input[name="idp_entity[testEntities][]"][value="bfe8f00d-317a-4fbc-9cf8-ad2f3b2af578"]')->first();
$checkbox->click();


$crawler = self::$pantherClient->submit($form);

$pageTitle = $crawler->filter('h1')->first()->text();
Expand Down

0 comments on commit 676d082

Please sign in to comment.