Skip to content

Commit

Permalink
Fix missing deprovision data
Browse files Browse the repository at this point in the history
Not all data was anonimised and should be fixed.
Also an interface was added to the projections to
force the implemenation of the forget event in
order to prevent this in the future.
Also made the forget command idempotent.
  • Loading branch information
pablothedude committed Jan 16, 2025
1 parent bdc8b9b commit 3226b8a
Show file tree
Hide file tree
Showing 6 changed files with 68 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,14 @@
namespace Surfnet\StepupMiddleware\ApiBundle\Controller;

use Exception;
use RuntimeException;
use Surfnet\Stepup\Exception\DomainException;
use Surfnet\Stepup\Helper\UserDataFormatterInterface;
use Surfnet\Stepup\Identity\Value\IdentityId;
use Surfnet\Stepup\Identity\Value\NameId;
use Surfnet\StepupMiddleware\ApiBundle\Identity\Repository\IdentityRepository;
use Surfnet\StepupMiddleware\ApiBundle\Identity\Repository\RaListingRepository;
use Surfnet\StepupMiddleware\ApiBundle\Identity\Repository\SraaRepository;
use Surfnet\StepupMiddleware\ApiBundle\Service\DeprovisionServiceInterface;
use Surfnet\StepupMiddleware\ApiBundle\Controller\AbstractController;
use Symfony\Component\HttpFoundation\JsonResponse;
Expand All @@ -30,6 +36,9 @@ class DeprovisionController extends AbstractController
public function __construct(
private readonly DeprovisionServiceInterface $deprovisionService,
private readonly UserDataFormatterInterface $formatHelper,
private readonly IdentityRepository $identityRepository,
private readonly SraaRepository $sraaRepository,
private readonly RaListingRepository $raListingRepository,
) {
}

Expand All @@ -42,11 +51,12 @@ public function deprovision(string $collabPersonId): JsonResponse
if ($userData !== []) {
$this->deprovisionService->deprovision($collabPersonId);
}
} catch (DomainException) {
} catch (DomainException $e) {
// On domain exceptions, like when the identity is forgotten, we return OK, with empty data
// just so the deprovision run does not end prematurely. At this point, no other domain exceptions
// are thrown.
$userData = [];
$errors = [$e->getMessage()];
} catch (Exception $e) {
$userData = [];
$errors = [$e->getMessage()];
Expand All @@ -57,6 +67,22 @@ public function deprovision(string $collabPersonId): JsonResponse
public function dryRun(string $collabPersonId): JsonResponse
{
$this->denyAccessUnlessGrantedOneOff(['ROLE_DEPROVISION']);

$nameId = new NameId($collabPersonId);
$identity = $this->identityRepository->findOneByNameId($nameId);

if ($identity === null) {
throw new RuntimeException('Cannot forget an identity that does not exist.');
}

if ($this->sraaRepository->contains($identity->nameId)) {
throw new RuntimeException('Cannot forget an identity that is currently accredited as an SRAA');
}

if ($this->raListingRepository->contains(new IdentityId($identity->id))) {
throw new RuntimeException('Cannot forget an identity that is currently accredited as an RA(A)');
}

$errors = [];
try {
$userData = $this->deprovisionService->readUserData($collabPersonId);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
use Broadway\ReadModel\Projector;
use Surfnet\Stepup\Identity\Event\IdentityCreatedEvent;
use Surfnet\Stepup\Identity\Event\IdentityEmailChangedEvent;
use Surfnet\Stepup\Identity\Event\IdentityForgottenEvent;
use Surfnet\Stepup\Identity\Event\IdentityRenamedEvent;
use Surfnet\Stepup\Identity\Event\LocalePreferenceExpressedEvent;
use Surfnet\Stepup\Identity\Event\SecondFactorVettedEvent;
Expand Down Expand Up @@ -95,4 +96,9 @@ private function determinePossessionOfSelfAssertedToken(VettingType $vettingType
}
}
}

protected function applyIdentityForgottenEvent(IdentityForgottenEvent $event): void
{
$this->identityRepository->updateStatusByIdentityIdToForgotten($event->identityId);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@
use Doctrine\Common\Collections\ArrayCollection;
use Doctrine\ORM\Query;
use Doctrine\Persistence\ManagerRegistry;
use Surfnet\Stepup\Identity\Value\CommonName;
use Surfnet\Stepup\Identity\Value\DocumentNumber;
use Surfnet\Stepup\Identity\Value\Email;
use Surfnet\Stepup\Identity\Value\IdentityId;
use Surfnet\Stepup\Identity\Value\Institution;
use Surfnet\Stepup\Identity\Value\NameId;
Expand Down Expand Up @@ -140,4 +143,18 @@ public function findOneByNameId(string $nameId): ?Identity
{
return $this->findOneBy(['nameId' => $nameId]);
}

public function updateStatusByIdentityIdToForgotten(IdentityId $identityId): void
{
$this->getEntityManager()->createQueryBuilder()
->update($this->getEntityName(), 'i')
->set('i.commonName', ":name")
->set('i.email', ":email")
->where('i.id = :id')
->setParameter('id', $identityId->getIdentityId())
->setParameter('name', CommonName::unknown())
->setParameter('email', Email::unknown())
->getQuery()
->execute();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -237,4 +237,9 @@ public function removeByIdentityIdAndInstitution(IdentityId $identityId, Institu
->getQuery()
->execute();
}

public function contains(IdentityId $identityId): bool
{
return count(parent::findBy(['identityId' => (string)$identityId])) > 0;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@
use Doctrine\ORM\Query;
use Doctrine\Persistence\ManagerRegistry;
use Surfnet\Stepup\Exception\RuntimeException;
use Surfnet\Stepup\Identity\Value\CommonName;
use Surfnet\Stepup\Identity\Value\DocumentNumber;
use Surfnet\Stepup\Identity\Value\Email;
use Surfnet\Stepup\Identity\Value\IdentityId;
use Surfnet\StepupMiddleware\ApiBundle\Authorization\Filter\InstitutionAuthorizationRepositoryFilter;
use Surfnet\StepupMiddleware\ApiBundle\Doctrine\Type\SecondFactorStatusType;
Expand All @@ -31,6 +34,7 @@
use Surfnet\StepupMiddleware\ApiBundle\Identity\Value\SecondFactorStatus;

/**
* @SuppressWarnings(PHPMD.CouplingBetweenObjects)
* @extends ServiceEntityRepository<RaSecondFactor>
*/
class RaSecondFactorRepository extends ServiceEntityRepository
Expand Down Expand Up @@ -203,9 +207,15 @@ public function updateStatusByIdentityIdToForgotten(IdentityId $identityId): voi
$this->getEntityManager()->createQueryBuilder()
->update($this->getEntityName(), 'rasf')
->set('rasf.status', ":forgotten")
->set('rasf.name', ":name")
->set('rasf.email', ":email")
->set('rasf.documentNumber', ":documentNumber")
->where('rasf.identityId = :identityId')
->setParameter('identityId', $identityId->getIdentityId())
->setParameter('forgotten', 40)
->setParameter('name', CommonName::unknown())
->setParameter('email', Email::unknown())
->setParameter('documentNumber', DocumentNumber::unknown())
->getQuery()
->execute();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@ services:
arguments:
$deprovisionService: '@Surfnet\StepupMiddleware\ApiBundle\Service\DeprovisionService'
$formatHelper: '@Surfnet\Stepup\Helper\UserDataFormatterInterface'
$identityRepository: '@surfnet_stepup_middleware_api.repository.identity'
$sraaRepository: '@surfnet_stepup_middleware_api.repository.sraa'
$raListingRepository: '@surfnet_stepup_middleware_api.repository.ra_listing'
tags: [ 'controller.service_arguments' ]

# Repositories
Expand Down

0 comments on commit 3226b8a

Please sign in to comment.