From e8dc788896939afb39fb38b3626780f30de9dcf6 Mon Sep 17 00:00:00 2001 From: Kay Joosten Date: Wed, 29 Jul 2026 11:02:43 +0200 Subject: [PATCH 1/2] Authorize console-triggered pipeline commands during bootstrap Why is this change needed? Prior to this change, console bootstrap commands (identity, yubikey, sms, gssp, migrate) ran without a security token. Creating an identity for a not-yet-configured institution triggers InstitutionConfiguration- Processor to dispatch CreateInstitutionConfigurationCommand back through the full pipeline, which requires ROLE_MANAGEMENT. With no token present, AuthorizingStage rejected it and the whole bootstrap transaction rolled back with a ForbiddenException. How does it address the issue? TransactionHelper::beginTransaction() now sets a fully authorized console security token when none is present, so commands the pipeline dispatches internally during a console run are no longer rejected. Removed BootstrapCommandService::setToken(), an unused method from an earlier, unfinished attempt at this same fix. Refs: #629 --- .../Resources/config/console_commands.yml | 2 +- .../Service/BootstrapCommandService.php | 8 -- .../Service/TransactionHelper.php | 17 ++++ .../Tests/Service/TransactionHelperTest.php | 92 +++++++++++++++++++ 4 files changed, 110 insertions(+), 9 deletions(-) create mode 100644 src/Surfnet/StepupMiddleware/MiddlewareBundle/Tests/Service/TransactionHelperTest.php diff --git a/src/Surfnet/StepupMiddleware/MiddlewareBundle/Resources/config/console_commands.yml b/src/Surfnet/StepupMiddleware/MiddlewareBundle/Resources/config/console_commands.yml index d274fc65f..14b92d166 100644 --- a/src/Surfnet/StepupMiddleware/MiddlewareBundle/Resources/config/console_commands.yml +++ b/src/Surfnet/StepupMiddleware/MiddlewareBundle/Resources/config/console_commands.yml @@ -7,7 +7,6 @@ services: arguments: - "@surfnet_stepup_middleware_command_handling.pipeline.transaction_aware_pipeline" - "@surfnet_stepup_middleware_command_handling.metadata_enricher.actor" - - "@security.token_storage" - "@surfnet_stepup_middleware_api.repository.identity" - "@surfnet_stepup_middleware_api.repository.unverified_second_factor" - "@surfnet_stepup_middleware_api.repository.verified_second_factor" @@ -19,6 +18,7 @@ services: - "@surfnet_stepup_middleware_command_handling.pipeline.transaction_aware_pipeline" - "@surfnet_stepup_middleware_command_handling.event_bus.buffered" - "@surfnet_stepup_middleware_middleware.dbal_connection_helper" + - "@security.token_storage" Surfnet\StepupMiddleware\MiddlewareBundle\Console\Command\ReplaySpecificEventsCommand: arguments: diff --git a/src/Surfnet/StepupMiddleware/MiddlewareBundle/Service/BootstrapCommandService.php b/src/Surfnet/StepupMiddleware/MiddlewareBundle/Service/BootstrapCommandService.php index 18f2512e0..2a8e27fb7 100644 --- a/src/Surfnet/StepupMiddleware/MiddlewareBundle/Service/BootstrapCommandService.php +++ b/src/Surfnet/StepupMiddleware/MiddlewareBundle/Service/BootstrapCommandService.php @@ -41,8 +41,6 @@ use Surfnet\StepupMiddleware\CommandHandlingBundle\Identity\Command\VetSecondFactorCommand; use Surfnet\StepupMiddleware\CommandHandlingBundle\Pipeline\Pipeline; use Surfnet\StepupMiddleware\MiddlewareBundle\Exception\InvalidArgumentException; -use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface; -use Symfony\Component\Security\Core\Authentication\Token\TokenInterface; /** * @SuppressWarnings("PHPMD.CouplingBetweenObjects") @@ -58,7 +56,6 @@ class BootstrapCommandService public function __construct( private readonly Pipeline $pipeline, private readonly MetadataEnricher $enricher, - private readonly TokenStorageInterface $tokenStorage, private readonly IdentityRepository $identityRepository, private readonly UnverifiedSecondFactorRepository $unverifiedSecondFactorRepository, private readonly VerifiedSecondFactorRepository $verifiedSecondFactorRepository, @@ -67,11 +64,6 @@ public function __construct( ) { } - public function setToken(TokenInterface $token): void - { - $this->tokenStorage->setToken($token); - } - public function validRegistrationStatus(string $registrationStatus): void { if (!in_array($registrationStatus, $this->validRegistrationStatuses)) { diff --git a/src/Surfnet/StepupMiddleware/MiddlewareBundle/Service/TransactionHelper.php b/src/Surfnet/StepupMiddleware/MiddlewareBundle/Service/TransactionHelper.php index 9764ce1ec..2522c900e 100644 --- a/src/Surfnet/StepupMiddleware/MiddlewareBundle/Service/TransactionHelper.php +++ b/src/Surfnet/StepupMiddleware/MiddlewareBundle/Service/TransactionHelper.php @@ -21,6 +21,9 @@ use Surfnet\StepupMiddleware\CommandHandlingBundle\Command\AbstractCommand; use Surfnet\StepupMiddleware\CommandHandlingBundle\EventHandling\BufferedEventBus; use Surfnet\StepupMiddleware\CommandHandlingBundle\Pipeline\Pipeline; +use Symfony\Component\Security\Core\Authentication\Token\Storage\TokenStorageInterface; +use Symfony\Component\Security\Core\Authentication\Token\UsernamePasswordToken; +use Symfony\Component\Security\Core\User\InMemoryUser; final readonly class TransactionHelper { @@ -28,11 +31,25 @@ public function __construct( private Pipeline $pipeline, private BufferedEventBus $eventBus, private DBALConnectionHelper $connection, + private TokenStorageInterface $tokenStorage, ) { } + /** + * Console commands run without an authenticated security token. Commands processed through the + * pipeline (including ones triggered internally by event processors, e.g. institution + * configuration bootstrapping) are checked by the AuthorizingStage, so a console-context token + * granting every role is set here to authorize such internally-triggered commands. + */ public function beginTransaction(): void { + if ($this->tokenStorage->getToken() === null) { + $roles = ['ROLE_SS', 'ROLE_RA', 'ROLE_MANAGEMENT', 'ROLE_DEPROVISION']; + $this->tokenStorage->setToken( + new UsernamePasswordToken(new InMemoryUser('console', null, $roles), 'api', $roles), + ); + } + $this->connection->beginTransaction(); } diff --git a/src/Surfnet/StepupMiddleware/MiddlewareBundle/Tests/Service/TransactionHelperTest.php b/src/Surfnet/StepupMiddleware/MiddlewareBundle/Tests/Service/TransactionHelperTest.php new file mode 100644 index 000000000..f5c9ec58b --- /dev/null +++ b/src/Surfnet/StepupMiddleware/MiddlewareBundle/Tests/Service/TransactionHelperTest.php @@ -0,0 +1,92 @@ +pipeline = m::mock(Pipeline::class); + $this->eventBus = m::mock(BufferedEventBus::class); + $this->connection = m::mock(DBALConnectionHelper::class); + $this->tokenStorage = m::mock(TokenStorageInterface::class); + + $this->transactionHelper = new TransactionHelper( + $this->pipeline, + $this->eventBus, + $this->connection, + $this->tokenStorage, + ); + } + + #[Test] + public function begin_transaction_sets_a_fully_authorized_console_token_when_none_is_present(): void + { + $this->tokenStorage->shouldReceive('getToken')->once()->andReturn(null); + $this->tokenStorage->shouldReceive('setToken') + ->once() + ->with(m::on(function (TokenInterface $token): bool { + $roles = $token->getRoleNames(); + sort($roles); + + return $roles === ['ROLE_DEPROVISION', 'ROLE_MANAGEMENT', 'ROLE_RA', 'ROLE_SS']; + })); + + $this->connection->shouldReceive('beginTransaction')->once(); + + $this->transactionHelper->beginTransaction(); + } + + #[Test] + public function begin_transaction_does_not_overwrite_an_existing_token(): void + { + $existingToken = m::mock(TokenInterface::class); + + $this->tokenStorage->shouldReceive('getToken')->once()->andReturn($existingToken); + $this->tokenStorage->shouldNotReceive('setToken'); + + $this->connection->shouldReceive('beginTransaction')->once(); + + $this->transactionHelper->beginTransaction(); + } +} From 5024d34953aa265f2a82c5634b3aa0d6a16066f3 Mon Sep 17 00:00:00 2001 From: Kay Joosten Date: Thu, 30 Jul 2026 09:31:04 +0200 Subject: [PATCH 2/2] Make console authorization explicit and least-privilege Why is this change needed? The console token was set implicitly inside TransactionHelper::begin- Transaction(), granting ROLE_SS, ROLE_RA, ROLE_MANAGEMENT and ROLE_DEPROVISION to every caller. That conflated starting a database transaction with authorizing privileged pipeline commands: any future console command wired to TransactionHelper for its transaction handling would silently inherit elevated roles it never asked for, including ROLE_DEPROVISION, which nothing in these bootstrap flows actually needs. How does it address the issue? Split the responsibility into TransactionHelper::authorizeConsole- Context(), an explicit, opt-in call that each of the six bootstrap console commands now makes before starting its transaction. Dropped ROLE_DEPROVISION from the granted roles. beginTransaction() itself no longer touches the token storage. Refs: #629 --- .../BootstrapGsspSecondFactorCommand.php | 1 + .../Command/BootstrapIdentityCommand.php | 1 + ...IdentityWithYubikeySecondFactorCommand.php | 1 + .../BootstrapSmsSecondFactorCommand.php | 1 + .../BootstrapYubikeySecondFactorCommand.php | 1 + .../Command/MigrateSecondFactorCommand.php | 1 + .../Service/TransactionHelper.php | 17 ++++++++++++----- .../Tests/Service/TransactionHelperTest.php | 19 +++++++++++++------ 8 files changed, 31 insertions(+), 11 deletions(-) diff --git a/src/Surfnet/StepupMiddleware/MiddlewareBundle/Console/Command/BootstrapGsspSecondFactorCommand.php b/src/Surfnet/StepupMiddleware/MiddlewareBundle/Console/Command/BootstrapGsspSecondFactorCommand.php index 66fde3b72..d926c8d79 100644 --- a/src/Surfnet/StepupMiddleware/MiddlewareBundle/Console/Command/BootstrapGsspSecondFactorCommand.php +++ b/src/Surfnet/StepupMiddleware/MiddlewareBundle/Console/Command/BootstrapGsspSecondFactorCommand.php @@ -87,6 +87,7 @@ public function __invoke( $identity->commonName, ), ); + $this->transactionHelper->authorizeConsoleContext(); $this->transactionHelper->beginTransaction(); $secondFactorId = Uuid::uuid4()->toString(); diff --git a/src/Surfnet/StepupMiddleware/MiddlewareBundle/Console/Command/BootstrapIdentityCommand.php b/src/Surfnet/StepupMiddleware/MiddlewareBundle/Console/Command/BootstrapIdentityCommand.php index fede46a4b..58d62ab63 100644 --- a/src/Surfnet/StepupMiddleware/MiddlewareBundle/Console/Command/BootstrapIdentityCommand.php +++ b/src/Surfnet/StepupMiddleware/MiddlewareBundle/Console/Command/BootstrapIdentityCommand.php @@ -67,6 +67,7 @@ public function __invoke( return 1; } try { + $this->transactionHelper->authorizeConsoleContext(); $this->transactionHelper->beginTransaction(); $output->writeln('Creating a new identity'); $identity = $this->bootstrapService->createIdentity( diff --git a/src/Surfnet/StepupMiddleware/MiddlewareBundle/Console/Command/BootstrapIdentityWithYubikeySecondFactorCommand.php b/src/Surfnet/StepupMiddleware/MiddlewareBundle/Console/Command/BootstrapIdentityWithYubikeySecondFactorCommand.php index 45d615c98..e6ce00077 100644 --- a/src/Surfnet/StepupMiddleware/MiddlewareBundle/Console/Command/BootstrapIdentityWithYubikeySecondFactorCommand.php +++ b/src/Surfnet/StepupMiddleware/MiddlewareBundle/Console/Command/BootstrapIdentityWithYubikeySecondFactorCommand.php @@ -85,6 +85,7 @@ public function __invoke( $command->secondFactorId = $secondFactorId; $command->yubikeyPublicId = $yubikey; + $this->transactionHelper->authorizeConsoleContext(); $this->transactionHelper->beginTransaction(); try { diff --git a/src/Surfnet/StepupMiddleware/MiddlewareBundle/Console/Command/BootstrapSmsSecondFactorCommand.php b/src/Surfnet/StepupMiddleware/MiddlewareBundle/Console/Command/BootstrapSmsSecondFactorCommand.php index 0154146d8..44390b209 100644 --- a/src/Surfnet/StepupMiddleware/MiddlewareBundle/Console/Command/BootstrapSmsSecondFactorCommand.php +++ b/src/Surfnet/StepupMiddleware/MiddlewareBundle/Console/Command/BootstrapSmsSecondFactorCommand.php @@ -68,6 +68,7 @@ public function __invoke(#[Argument(description: 'The NameID of the identity to $output->writeln( sprintf('Adding a %s SMS token for %s', $registrationStatus, $identity->commonName), ); + $this->transactionHelper->authorizeConsoleContext(); $this->transactionHelper->beginTransaction(); $secondFactorId = Uuid::uuid4()->toString(); diff --git a/src/Surfnet/StepupMiddleware/MiddlewareBundle/Console/Command/BootstrapYubikeySecondFactorCommand.php b/src/Surfnet/StepupMiddleware/MiddlewareBundle/Console/Command/BootstrapYubikeySecondFactorCommand.php index 9be3326c3..c2cd7b93e 100644 --- a/src/Surfnet/StepupMiddleware/MiddlewareBundle/Console/Command/BootstrapYubikeySecondFactorCommand.php +++ b/src/Surfnet/StepupMiddleware/MiddlewareBundle/Console/Command/BootstrapYubikeySecondFactorCommand.php @@ -74,6 +74,7 @@ public function __invoke( $output->writeln( sprintf('Adding a %s Yubikey token for %s', $registrationStatus, $identity->commonName), ); + $this->transactionHelper->authorizeConsoleContext(); $this->transactionHelper->beginTransaction(); $secondFactorId = Uuid::uuid4()->toString(); diff --git a/src/Surfnet/StepupMiddleware/MiddlewareBundle/Console/Command/MigrateSecondFactorCommand.php b/src/Surfnet/StepupMiddleware/MiddlewareBundle/Console/Command/MigrateSecondFactorCommand.php index f1768c0d3..cbd7781f3 100644 --- a/src/Surfnet/StepupMiddleware/MiddlewareBundle/Console/Command/MigrateSecondFactorCommand.php +++ b/src/Surfnet/StepupMiddleware/MiddlewareBundle/Console/Command/MigrateSecondFactorCommand.php @@ -64,6 +64,7 @@ public function __invoke( $targetIdentity = $this->bootstrapService->getIdentityByNameId($targetNameId); try { + $this->transactionHelper->authorizeConsoleContext(); $this->transactionHelper->beginTransaction(); // Check if target identity should be created diff --git a/src/Surfnet/StepupMiddleware/MiddlewareBundle/Service/TransactionHelper.php b/src/Surfnet/StepupMiddleware/MiddlewareBundle/Service/TransactionHelper.php index 2522c900e..4f62e2ba9 100644 --- a/src/Surfnet/StepupMiddleware/MiddlewareBundle/Service/TransactionHelper.php +++ b/src/Surfnet/StepupMiddleware/MiddlewareBundle/Service/TransactionHelper.php @@ -38,18 +38,25 @@ public function __construct( /** * Console commands run without an authenticated security token. Commands processed through the * pipeline (including ones triggered internally by event processors, e.g. institution - * configuration bootstrapping) are checked by the AuthorizingStage, so a console-context token - * granting every role is set here to authorize such internally-triggered commands. + * configuration bootstrapping) are checked by the AuthorizingStage, so bootstrap console commands + * that need to authorize such internally-triggered commands must call this explicitly. Limited to + * ROLE_SS, ROLE_RA and ROLE_MANAGEMENT: the identity/vetting/configuration commands the bootstrap + * console commands dispatch through this pipeline. ROLE_DEPROVISION is deliberately excluded, + * nothing in these flows needs it. Guarded to CLI only, so this can never grant privileges to an + * HTTP request even if called from a context that shouldn't. */ - public function beginTransaction(): void + public function authorizeConsoleContext(): void { - if ($this->tokenStorage->getToken() === null) { - $roles = ['ROLE_SS', 'ROLE_RA', 'ROLE_MANAGEMENT', 'ROLE_DEPROVISION']; + if (PHP_SAPI === 'cli' && $this->tokenStorage->getToken() === null) { + $roles = ['ROLE_SS', 'ROLE_RA', 'ROLE_MANAGEMENT']; $this->tokenStorage->setToken( new UsernamePasswordToken(new InMemoryUser('console', null, $roles), 'api', $roles), ); } + } + public function beginTransaction(): void + { $this->connection->beginTransaction(); } diff --git a/src/Surfnet/StepupMiddleware/MiddlewareBundle/Tests/Service/TransactionHelperTest.php b/src/Surfnet/StepupMiddleware/MiddlewareBundle/Tests/Service/TransactionHelperTest.php index f5c9ec58b..19c9fe62d 100644 --- a/src/Surfnet/StepupMiddleware/MiddlewareBundle/Tests/Service/TransactionHelperTest.php +++ b/src/Surfnet/StepupMiddleware/MiddlewareBundle/Tests/Service/TransactionHelperTest.php @@ -60,7 +60,7 @@ public function setUp(): void } #[Test] - public function begin_transaction_sets_a_fully_authorized_console_token_when_none_is_present(): void + public function authorize_console_context_sets_a_console_token_with_the_bootstrap_roles_when_none_is_present(): void { $this->tokenStorage->shouldReceive('getToken')->once()->andReturn(null); $this->tokenStorage->shouldReceive('setToken') @@ -69,22 +69,29 @@ public function begin_transaction_sets_a_fully_authorized_console_token_when_non $roles = $token->getRoleNames(); sort($roles); - return $roles === ['ROLE_DEPROVISION', 'ROLE_MANAGEMENT', 'ROLE_RA', 'ROLE_SS']; + return $roles === ['ROLE_MANAGEMENT', 'ROLE_RA', 'ROLE_SS']; })); - $this->connection->shouldReceive('beginTransaction')->once(); - - $this->transactionHelper->beginTransaction(); + $this->transactionHelper->authorizeConsoleContext(); } #[Test] - public function begin_transaction_does_not_overwrite_an_existing_token(): void + public function authorize_console_context_does_not_overwrite_an_existing_token(): void { $existingToken = m::mock(TokenInterface::class); $this->tokenStorage->shouldReceive('getToken')->once()->andReturn($existingToken); $this->tokenStorage->shouldNotReceive('setToken'); + $this->transactionHelper->authorizeConsoleContext(); + } + + #[Test] + public function begin_transaction_does_not_touch_the_token_storage(): void + { + $this->tokenStorage->shouldNotReceive('getToken'); + $this->tokenStorage->shouldNotReceive('setToken'); + $this->connection->shouldReceive('beginTransaction')->once(); $this->transactionHelper->beginTransaction();