diff --git a/CHANGELOG.md b/CHANGELOG.md index eb6ee542..ba48d59f 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 d8a93d38..de88b6b1 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 0790f509..70f6f0d0 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 638558ce..5b30bdad 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 f033216b..1ca88bb7 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 00000000..3916b624 --- /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 00000000..6286ddf3 --- /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 c0799aa3..c801a3a4 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 ec85f7fe..4e9eee42 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')]