Skip to content
Open
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
12 changes: 6 additions & 6 deletions ci/qa/phpstan-baseline.php
Original file line number Diff line number Diff line change
Expand Up @@ -343,6 +343,12 @@
'count' => 1,
'path' => __DIR__ . '/../../src/Surfnet/StepupRa/RaBundle/Controller/SecondFactorController.php',
];
$ignoreErrors[] = [
'message' => '#^Call to an undefined method Symfony\\\\Component\\\\Form\\\\FormInterface\\<Surfnet\\\\StepupRa\\\\RaBundle\\\\Command\\\\SearchRaListingCommand\\>\\:\\:getClickedButton\\(\\)\\.$#',
'identifier' => 'method.notFound',
'count' => 1,
'path' => __DIR__ . '/../../src/Surfnet/StepupRa/RaBundle/Controller/RaManagementController.php',
];
$ignoreErrors[] = [
'message' => '#^Call to an undefined method Symfony\\\\Component\\\\Security\\\\Core\\\\User\\\\UserProviderInterface\\:\\:findById\\(\\)\\.$#',
'identifier' => 'method.notFound',
Expand Down Expand Up @@ -715,12 +721,6 @@
'count' => 1,
'path' => __DIR__ . '/../../src/Surfnet/StepupRa/RaBundle/Service/RaCandidateService.php',
];
$ignoreErrors[] = [
'message' => '#^Left side of && is always true\\.$#',
'identifier' => 'booleanAnd.leftAlwaysTrue',
'count' => 2,
'path' => __DIR__ . '/../../src/Surfnet/StepupRa/RaBundle/Service/RaListingService.php',
];
$ignoreErrors[] = [
'message' => '#^Parameter \\#1 \\$raInstitution of method Surfnet\\\\StepupMiddlewareClient\\\\Identity\\\\Dto\\\\RaListingSearchQuery\\:\\:setRaInstitution\\(\\) expects string, string\\|null given\\.$#',
'identifier' => 'argument.type',
Expand Down
99 changes: 99 additions & 0 deletions src/Surfnet/StepupRa/RaBundle/Command/ExportRaListingCommand.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,99 @@
<?php

/**
* Copyright 2026 SURFnet bv
*
* 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\StepupRa\RaBundle\Command;

use DateTime;
use Surfnet\StepupRa\RaBundle\Value\RoleAtInstitution;
use Symfony\Component\Validator\Constraints as Assert;

final class ExportRaListingCommand
{
/**
* @var string
*/
#[Assert\NotBlank(message: 'ra.search_ra_listing.actor_id.blank')]
#[Assert\Type('string', message: 'ra.search_ra_listing.actor_id.type')]
public $actorId;

/**
* @var string|null
*/
public $name;

/**
* @var string|null
*/
public $email;

/**
* @var string|null
*/
public $institution;

/**
* @var RoleAtInstitution|null
*/
public $roleAtInstitution;

/**
* @var string|null
*/
#[Assert\Choice(choices: ['name', 'email'], message: 'ra.search_ra_candidates.order_by.invalid_choice')]
public $orderBy;

/**
* @var string|null
*/
#[Assert\Choice(choices: ['asc', 'desc'], message: 'ra.search_ra_candidates.order_direction.invalid_choice')]
public $orderDirection;

/**
* Builds the command from a SearchRaListingCommand
*/
public static function fromSearchCommand(SearchRaListingCommand $command): ExportRaListingCommand
{
$exportCommand = new self;

$exportCommand->actorId = $command->actorId;
$exportCommand->name = $command->name;
$exportCommand->email = $command->email;
$exportCommand->institution = $command->institution;
$exportCommand->roleAtInstitution = $command->roleAtInstitution;
$exportCommand->orderBy = $command->orderBy;
$exportCommand->orderDirection = $command->orderDirection;

return $exportCommand;
}

public function getFileName(): string
{
$date = new DateTime();
$date = $date->format('Y-m-d');

$role = $this->roleAtInstitution instanceof RoleAtInstitution && $this->roleAtInstitution->hasRole()
? $this->roleAtInstitution->getRole()
: null;

return match ($role) {
'ra' => "ra_export_{$date}",
'raa' => "raa_export_{$date}",
default => "ra-raa-export_{$date}",
};
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
use Surfnet\StepupMiddlewareClientBundle\Identity\Dto\RaListing;
use Surfnet\StepupRa\RaBundle\Command\AccreditCandidateCommand;
use Surfnet\StepupRa\RaBundle\Command\AmendRegistrationAuthorityInformationCommand;
use Surfnet\StepupRa\RaBundle\Command\ExportRaListingCommand;
use Surfnet\StepupRa\RaBundle\Command\RetractRegistrationAuthorityCommand;
use Surfnet\StepupRa\RaBundle\Command\SearchRaCandidatesCommand;
use Surfnet\StepupRa\RaBundle\Command\SearchRaListingCommand;
Expand Down Expand Up @@ -89,6 +90,11 @@ public function manage(Request $request): Response
$form = $this->createForm(SearchRaListingType::class, $command, ['method' => 'get']);
$form->handleRequest($request);

if ($form->isSubmitted() && $form->getClickedButton()?->getName() === 'export') {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🤔 The getClickedButton does seem to be officially supported (https://symfony.com/doc/current/forms.html#handling-multiple-submit-buttons)

But it still needs a phpstan exception. Is the $this->createForm() typehint correct?

nitpick

$this->logger->notice('Forwarding to export RA(A) listing action');
return $this->forward('\Surfnet\StepupRa\RaBundle\Controller\RaManagementController::export', ['command' => $command]);
}

$raList = $this->raListingService->search($command);

$pagination = $this->paginator->paginate(
Expand Down Expand Up @@ -117,6 +123,16 @@ public function manage(Request $request): Response
);
}

#[IsGranted('ROLE_RAA')]
public function export(SearchRaListingCommand $command): Response
{
$this->logger->notice('Starting export of searched RA(A) listing');

$exportCommand = ExportRaListingCommand::fromSearchCommand($command);

return $this->raListingService->export($exportCommand);
}

#[Route(
path: '/management/search-ra-candidate',
name: 'ra_management_ra_candidate_search',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ public function buildForm(FormBuilderInterface $builder, array $options): void
'choices' => RaRoleChoiceList::create(),
'choice_value' => fn($choice) => $choice,
'required' => $isRequired,
'placeholder' => $isRequired ? null : 'ra.form.role_at_institution.placeholder.role',
])->add('institution', ChoiceType::class, [
'label' => 'ra.form.role_at_institution.label.institution',
'choices' => $selectRaaOptions,
Expand Down
17 changes: 16 additions & 1 deletion src/Surfnet/StepupRa/RaBundle/Form/Type/SearchRaListingType.php
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,25 @@ public function buildForm(FormBuilderInterface $builder, array $options): void
'choices' => $data->raInstitutionFilterOptions,
'label' => 'ra.form.ra_search_ra_listing.label.role',
'required' => false,
])->add('search', SubmitType::class, [
]);

$buttonGroup = $builder->create(
'button-group',
ButtonGroupType::class,
[
'mapped' => false,
],
)
->add('search', SubmitType::class, [
'label' => 'ra.form.ra_search_ra_listing.button.search',
'attr' => ['class' => 'btn btn-primary search-button'],
])
->add('export', SubmitType::class, [
'label' => 'ra.form.ra_search_ra_listing.button.export',
'attr' => ['class' => 'btn btn-secondary'],
]);

$builder->add($buttonGroup);
}

public function configureOptions(OptionsResolver $resolver): void
Expand Down
7 changes: 6 additions & 1 deletion src/Surfnet/StepupRa/RaBundle/Resources/config/services.yml
Original file line number Diff line number Diff line change
Expand Up @@ -215,11 +215,16 @@ services:
Surfnet\StepupRa\RaBundle\Service\RaLocationService:
alias: ra.service.ra_location

ra.service.ra_listing_exporter:
class: Surfnet\StepupRa\RaBundle\Service\RaListingExport
arguments:
- "@logger"

ra.service.ra_listing:
class: Surfnet\StepupRa\RaBundle\Service\RaListingService
arguments:
- "@surfnet_stepup_middleware_client.identity.service.ra_listing"
- "@logger"
- "@ra.service.ra_listing_exporter"
Surfnet\StepupRa\RaBundle\Service\RaListingService:
alias: ra.service.ra_listing

Expand Down
115 changes: 115 additions & 0 deletions src/Surfnet/StepupRa/RaBundle/Service/RaListingExport.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,115 @@
<?php

/**
* Copyright 2026 SURFnet bv
*
* 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\StepupRa\RaBundle\Service;

use Psr\Log\LoggerInterface;
use RuntimeException;
use Surfnet\StepupMiddlewareClientBundle\Identity\Dto\RaListing;
use Symfony\Component\HttpFoundation\Response;
use Symfony\Component\HttpFoundation\StreamedResponse;

class RaListingExport
{
public function __construct(private readonly LoggerInterface $logger)
{
}

/**
* @param iterable<RaListing> $raListings
*/
public function export(iterable $raListings, string $fileName): StreamedResponse
{
$this->logger->notice(sprintf('Starting RA(A) listing export to "%s"', $fileName));

$columnNames = $this->getColumnNames();

return new StreamedResponse(
function () use ($raListings, $columnNames, $fileName) {
$handle = fopen('php://output', 'r+');
if ($handle === false) {
throw new RuntimeException('Unable to open php://output for writing the RA(A) listing export');
}
fputcsv($handle, $columnNames);

$rowCount = 0;
foreach ($raListings as $raListing) {
fputcsv($handle, [
Comment on lines +48 to +52

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

fputcsv() is called without $escape

PHP 8.4 deprecates the implicit default (the new tests emit two deprecations on newer PHP), and the legacy \\ escape produces non-RFC4180 output for values ending in a backslash. The test itself reads back with str_getcsv(..., escape: ''), which shows the asymmetry.

Suggested approach: pass escape: '' explicitly to both fputcsv() calls.

$this->sanitizeCsvCell($raListing->commonName),
$this->sanitizeCsvCell($raListing->email),
$this->sanitizeCsvCell($raListing->institution),
$this->sanitizeCsvCell($raListing->role),
$this->sanitizeCsvCell($raListing->raInstitution),
$this->sanitizeCsvCell($raListing->location),
$this->sanitizeCsvCell($raListing->contactInformation),
]);
$rowCount++;
}

$this->logger->notice(sprintf('Exported %d rows to "%s"', $rowCount, $fileName));

fflush($handle);
fclose($handle);
},
Response::HTTP_OK,
[
'Content-Type' => 'application/csv',
'Content-Disposition' => sprintf('attachment; filename="%s.csv"', $fileName),
],
);
}

/**
* Neutralizes CSV/spreadsheet formula injection: values coming from identity/profile
* data are attacker-influenceable and must not be allowed to start a formula when the
* exported file is opened in Excel/LibreOffice/Google Sheets. The leading "+" is
* intentionally not treated as a formula trigger here, since Contact Information
* routinely holds international phone numbers (e.g. "+31 6 12345678") that must be
* exported unmodified.
*
* @see https://owasp.org/www-community/attacks/CSV_Injection
*/
private function sanitizeCsvCell(?string $value): ?string
{
if ($value === null || $value === '') {
return $value;
}

if (in_array($value[0], ['=', '-', '@', "\t", "\r"], true)) {
return "'" . $value;
}

return $value;
}

/**
* @return string[]
*/
private function getColumnNames(): array
{
return [
'Common Name',
'Email',
'Institution',
'Role',
'RA Institution',
'Location',
'Contact Information',
];
}
}
Loading