From 2d651fee2247bd4317c6c69a5557ebc320472665 Mon Sep 17 00:00:00 2001 From: Kay Joosten Date: Thu, 13 Aug 2026 11:32:51 +0200 Subject: [PATCH] Dispatch IdentityForgottenEvent to RecoveryToken child entities (#628) If applied, this commit will make forgetting an identity (used by the deprovisioning flow) cascade to its recovery tokens the same way it already cascades to second factors, and ensure the documented backfill path for older forgotten identities actually works. Why is this change needed? Prior to this change, Identity::getChildEntities() included the second-factor and registration-authority collections but not recovery tokens, so RecoveryToken never received the IdentityForgottenEvent dispatch that Broadway cascades to child entities. The read-model projector already cleaned up recovery-token rows on forget, but the write-model side was inconsistent with the SecondFactor entities. Additionally, RecoveryTokenProjector was never tagged for event replay, so the documented one-time backfill for identities forgotten before the projector handled IdentityForgottenEvent could not actually be run. How does it address the issue? - Adds RecoveryToken::applyIdentityForgottenEvent() (a no-op, since a RecoveryToken only holds a token id and type, no PII) purely so the entity participates in the cascade like the other child entities. - Adds RecoveryTokenCollection::getValues() and wires it into Identity::getChildEntities(), mirroring the existing SecondFactorCollection/RegistrationAuthorityCollection pattern. - Tags RecoveryTokenProjector with projector.register_for_replay so it can be selected by the stepup:event:replay console command. - Adds a phpstan baseline entry for the new nullable-collection access, following the existing pattern used for the other collections on Identity. - Adds unit tests proving the cascade actually reaches RecoveryToken (IdentityTest), that RecoveryTokenCollection::getValues() returns all tokens (RecoveryTokenCollectionTest), and that an identity with a recovery token can be forgotten end to end (RightToBeForgottenCommandHandlerTest). - Documents the required one-time operational backfill step in CHANGELOG.md, with the correct historical cutoff commit and the prod_event_replay environment required to run it. Links / references: https://github.com/OpenConext/Stepup-Middleware/issues/628 --- CHANGELOG.md | 8 ++ ci/qa/phpstan-baseline.neon | 6 ++ .../Stepup/Identity/Entity/RecoveryToken.php | 8 ++ .../Entity/RecoveryTokenCollection.php | 8 ++ src/Surfnet/Stepup/Identity/Identity.php | 1 + .../Entity/RecoveryTokenCollectionTest.php | 63 +++++++++++ .../Stepup/Tests/Identity/IdentityTest.php | 102 ++++++++++++++++++ .../ApiBundle/Resources/config/projection.yml | 4 +- .../RightToBeForgottenCommandHandlerTest.php | 59 ++++++++++ 9 files changed, 258 insertions(+), 1 deletion(-) create mode 100644 src/Surfnet/Stepup/Tests/Identity/Entity/RecoveryTokenCollectionTest.php create mode 100644 src/Surfnet/Stepup/Tests/Identity/IdentityTest.php diff --git a/CHANGELOG.md b/CHANGELOG.md index eb6ee5422..ba48d59fe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,13 @@ # Changelog +# Unreleased +**Notable Changes** +- Deprovision (right-to-be-forgotten) now also dispatches to recovery tokens, matching the existing second-factor handling (#628) + +**Deployment action required** +- `RecoveryTokenProjector` only started removing recovery-token read-model rows on `IdentityForgottenEvent` as of commit `e6c81940` (2022-07-13). Identities forgotten before that date may still have stale `recovery_tokens` rows (containing PII). `RecoveryTokenProjector` is now tagged `projector.register_for_replay` so it can be targeted for a backfill. Operators should run, under the `prod_event_replay` environment: + `APP_ENV=prod_event_replay bin/console stepup:event:replay`, selecting `IdentityForgottenEvent` and `RecoveryTokenProjector`, to clean up those rows. + # 7.0.1 **Notable Changes** - Upgrade to Symfony 7.4 (from 6.4) diff --git a/ci/qa/phpstan-baseline.neon b/ci/qa/phpstan-baseline.neon index d8a93d387..de88b6b19 100644 --- a/ci/qa/phpstan-baseline.neon +++ b/ci/qa/phpstan-baseline.neon @@ -1656,6 +1656,12 @@ parameters: count: 1 path: ../../src/Surfnet/Stepup/Identity/Identity.php + - + message: '#^Cannot call method getValues\(\) on Surfnet\\Stepup\\Identity\\Entity\\RecoveryTokenCollection\|null\.$#' + identifier: method.nonObject + count: 1 + path: ../../src/Surfnet/Stepup/Identity/Identity.php + - message: '#^Cannot call method count\(\) on Surfnet\\Stepup\\Identity\\Entity\\RegistrationAuthorityCollection\|null\.$#' identifier: method.nonObject diff --git a/src/Surfnet/Stepup/Identity/Entity/RecoveryToken.php b/src/Surfnet/Stepup/Identity/Entity/RecoveryToken.php index 0790f5096..70f6f0d07 100644 --- a/src/Surfnet/Stepup/Identity/Entity/RecoveryToken.php +++ b/src/Surfnet/Stepup/Identity/Entity/RecoveryToken.php @@ -22,6 +22,7 @@ use Broadway\EventSourcing\SimpleEventSourcedEntity; use Surfnet\Stepup\Identity\Api\Identity; use Surfnet\Stepup\Identity\Event\CompliedWithRecoveryCodeRevocationEvent; +use Surfnet\Stepup\Identity\Event\IdentityForgottenEvent; use Surfnet\Stepup\Identity\Event\RecoveryTokenRevokedEvent; use Surfnet\Stepup\Identity\Value\IdentityId; use Surfnet\Stepup\Identity\Value\RecoveryTokenId; @@ -86,4 +87,11 @@ public function complyWithRevocation(IdentityId $authorityId): void ), ); } + + protected function applyIdentityForgottenEvent(IdentityForgottenEvent $event): void + { + // No PII is stored on a RecoveryToken (only a token id and type), so there is nothing to + // anonymize here. This handler exists so the entity is included in Identity::getChildEntities() + // dispatch, matching the SecondFactor entities. + } } diff --git a/src/Surfnet/Stepup/Identity/Entity/RecoveryTokenCollection.php b/src/Surfnet/Stepup/Identity/Entity/RecoveryTokenCollection.php index 638558ced..5b30bdad7 100644 --- a/src/Surfnet/Stepup/Identity/Entity/RecoveryTokenCollection.php +++ b/src/Surfnet/Stepup/Identity/Entity/RecoveryTokenCollection.php @@ -57,6 +57,14 @@ public function count(): int return count($this->recoveryTokens); } + /** + * @return RecoveryToken[] + */ + public function getValues(): array + { + return array_values($this->recoveryTokens); + } + public function remove(RecoveryTokenId $recoveryTokenId): void { unset($this->recoveryTokens[(string)$recoveryTokenId]); diff --git a/src/Surfnet/Stepup/Identity/Identity.php b/src/Surfnet/Stepup/Identity/Identity.php index f033216bf..1ca88bb72 100644 --- a/src/Surfnet/Stepup/Identity/Identity.php +++ b/src/Surfnet/Stepup/Identity/Identity.php @@ -1465,6 +1465,7 @@ protected function getChildEntities(): array $this->verifiedSecondFactors->getValues(), $this->vettedSecondFactors->getValues(), $this->registrationAuthorities->getValues(), + $this->recoveryTokens->getValues(), ); } diff --git a/src/Surfnet/Stepup/Tests/Identity/Entity/RecoveryTokenCollectionTest.php b/src/Surfnet/Stepup/Tests/Identity/Entity/RecoveryTokenCollectionTest.php new file mode 100644 index 000000000..3916b6240 --- /dev/null +++ b/src/Surfnet/Stepup/Tests/Identity/Entity/RecoveryTokenCollectionTest.php @@ -0,0 +1,63 @@ +createRecoveryToken('RT-1'); + $second = $this->createRecoveryToken('RT-2'); + + $collection->set($first); + $collection->set($second); + + $this->assertSame([$first, $second], $collection->getValues()); + } + + #[Test] + #[Group('domain')] + public function get_values_returns_an_empty_array_for_an_empty_collection(): void + { + $collection = new RecoveryTokenCollection(); + + $this->assertSame([], $collection->getValues()); + } + + private function createRecoveryToken(string $id): RecoveryToken + { + return RecoveryToken::create( + new RecoveryTokenId($id), + RecoveryTokenType::sms(), + new Identity(), + ); + } +} diff --git a/src/Surfnet/Stepup/Tests/Identity/IdentityTest.php b/src/Surfnet/Stepup/Tests/Identity/IdentityTest.php new file mode 100644 index 000000000..6286ddf3c --- /dev/null +++ b/src/Surfnet/Stepup/Tests/Identity/IdentityTest.php @@ -0,0 +1,102 @@ +initializeState(new DomainEventStream([ + $this->wrap($identityId, new IdentityCreatedEvent( + $identityId, + $institution, + new NameId('urn:eeva-kuopio'), + new CommonName('Eeva Kuopio'), + new Email('e.kuopio@hy.fi'), + new Locale('fi_FI'), + ), 0), + $this->wrap($identityId, new PhoneRecoveryTokenPossessionProvenEvent( + $identityId, + $institution, + $recoveryTokenId, + new PhoneNumber('+31 (0) 12345678'), + new CommonName('Eeva Kuopio'), + new Email('e.kuopio@hy.fi'), + new Locale('fi_FI'), + ), 1), + ])); + + $getChildEntities = new ReflectionMethod(Identity::class, 'getChildEntities'); + $getChildEntities->setAccessible(true); + /** @var array $childEntities */ + $childEntities = $getChildEntities->invoke($identity); + + $recoveryTokens = array_filter( + $childEntities, + static fn($entity): bool => $entity instanceof RecoveryToken, + ); + $this->assertCount( + 1, + $recoveryTokens, + 'Identity::getChildEntities() should dispatch IdentityForgottenEvent to registered RecoveryTokens', + ); + $recoveryToken = array_values($recoveryTokens)[0]; + $this->assertSame( + (string)$recoveryTokenId, + (string)$recoveryToken->getTokenId(), + ); + } + + private function wrap(IdentityId $identityId, object $event, int $playhead): DomainMessage + { + return DomainMessage::recordNow( + $identityId->getIdentityId(), + $playhead, + new Metadata([]), + $event, + ); + } +} diff --git a/src/Surfnet/StepupMiddleware/ApiBundle/Resources/config/projection.yml b/src/Surfnet/StepupMiddleware/ApiBundle/Resources/config/projection.yml index c0799aa3f..c801a3a44 100644 --- a/src/Surfnet/StepupMiddleware/ApiBundle/Resources/config/projection.yml +++ b/src/Surfnet/StepupMiddleware/ApiBundle/Resources/config/projection.yml @@ -53,7 +53,9 @@ services: class: Surfnet\StepupMiddleware\ApiBundle\Identity\Projector\RecoveryTokenProjector arguments: - '@Surfnet\StepupMiddleware\ApiBundle\Identity\Repository\RecoveryTokenRepository' - tags: [ { name: event_bus.event_listener, disable_for_replay: false } ] + tags: + - { name: event_bus.event_listener, disable_for_replay: false } + - { name: projector.register_for_replay } surfnet_stepup_middleware_api.projector.vetting_type_hint: class: Surfnet\StepupMiddleware\ApiBundle\Identity\Projector\VettingTypeHintProjector diff --git a/src/Surfnet/StepupMiddleware/CommandHandlingBundle/Tests/Identity/CommandHandler/RightToBeForgottenCommandHandlerTest.php b/src/Surfnet/StepupMiddleware/CommandHandlingBundle/Tests/Identity/CommandHandler/RightToBeForgottenCommandHandlerTest.php index ec85f7fee..4e9eee420 100644 --- a/src/Surfnet/StepupMiddleware/CommandHandlingBundle/Tests/Identity/CommandHandler/RightToBeForgottenCommandHandlerTest.php +++ b/src/Surfnet/StepupMiddleware/CommandHandlingBundle/Tests/Identity/CommandHandler/RightToBeForgottenCommandHandlerTest.php @@ -35,6 +35,7 @@ use Surfnet\Stepup\Identity\Event\IdentityAccreditedAsRaForInstitutionEvent; use Surfnet\Stepup\Identity\Event\IdentityCreatedEvent; use Surfnet\Stepup\Identity\Event\IdentityForgottenEvent; +use Surfnet\Stepup\Identity\Event\PhoneRecoveryTokenPossessionProvenEvent; use Surfnet\Stepup\Identity\Event\YubikeySecondFactorBootstrappedEvent; use Surfnet\Stepup\Identity\EventSourcing\IdentityRepository; use Surfnet\Stepup\Identity\Value\CommonName; @@ -45,6 +46,8 @@ use Surfnet\Stepup\Identity\Value\Locale; use Surfnet\Stepup\Identity\Value\Location; use Surfnet\Stepup\Identity\Value\NameId; +use Surfnet\Stepup\Identity\Value\PhoneNumber; +use Surfnet\Stepup\Identity\Value\RecoveryTokenId; use Surfnet\Stepup\Identity\Value\RegistrationAuthorityRole; use Surfnet\Stepup\Identity\Value\SecondFactorId; use Surfnet\Stepup\Identity\Value\YubikeyPublicId; @@ -155,6 +158,62 @@ public function an_identity_can_be_forgotten(): void ]); } + #[Test] + #[Group('command-handler')] + #[Group('sensitive-data')] + public function an_identity_with_a_recovery_token_can_be_forgotten(): void + { + $identityId = new IdentityId('A'); + $institution = new Institution('Helsingin Yliopisto'); + $nameId = new NameId('urn:eeva-kuopio'); + $commonName = new CommonName('Eeva Kuopio'); + $email = new Email('e.kuopio@hy.fi'); + $locale = new Locale('fi_FI'); + + $this->apiIdentityRepository + ->shouldReceive('findOneByNameIdAndInstitution') + ->once() + ->with(new IsEqual($nameId), new IsEqual($institution)) + ->andReturn($this->createIdentity($identityId->getIdentityId())); + + $this->sensitiveDataService + ->shouldReceive('forgetSensitiveData') + ->once() + ->with(new IsEqual($identityId)); + + $this->sraaRepository->shouldReceive('contains')->once()->with(new IsEqual($nameId))->andReturn(false); + + $command = new ForgetIdentityCommand(); + $command->nameId = $nameId->getNameId(); + $command->institution = $institution->getInstitution(); + + $this->scenario + ->withAggregateId('A') + ->given([ + new IdentityCreatedEvent( + $identityId, + $institution, + $nameId, + $commonName, + $email, + $locale, + ), + new PhoneRecoveryTokenPossessionProvenEvent( + $identityId, + $institution, + new RecoveryTokenId('RT-ID'), + new PhoneNumber('+31 (0) 12345678'), + $commonName, + $email, + $locale, + ), + ]) + ->when($command) + ->then([ + new IdentityForgottenEvent($identityId, $institution), + ]); + } + #[Test] #[Group('command-handler')] #[Group('sensitive-data')]