From f2d737374bdde23dc694394845348bb36d58f208 Mon Sep 17 00:00:00 2001 From: Michiel Kodde Date: Thu, 21 Mar 2024 08:56:33 +0100 Subject: [PATCH] Address level 5 PHPStan issues Three warnings have been ignored. They are regarding a hidden use of three RegistraoinAuthority fields. Static analysis does not evaluate that they are written when persisted to db storage The rest of the fixes are mainly removal of unused code fragments. Or removal of redundant sections of logical expressions. --- src/Surfnet/Stepup/DateTime/DateTime.php | 29 +++--------- .../Stepup/Identity/AuditLog/Metadata.php | 24 ++-------- .../Identity/Entity/RegistrationAuthority.php | 6 +-- .../Event/SecondFactorVettedEvent.php | 3 +- ...torVettedWithoutTokenProofOfPossession.php | 3 +- src/Surfnet/Stepup/Identity/Identity.php | 14 +++--- .../Service/AuthorizationService.php | 2 +- .../InstitutionAuthorizationProjector.php | 1 - .../Projector/RaLocationProjector.php | 11 +---- .../Repository/RaLocationRepository.php | 7 +-- .../AllowedSecondFactorListService.php | 2 - .../Service/RaLocationService.php | 9 ++-- .../Controller/IdentityController.php | 9 +--- .../Identity/Entity/VettedSecondFactor.php | 23 +--------- .../Identity/Projector/AuditLogProjector.php | 3 +- .../Identity/Projector/RaListingProjector.php | 22 +++------ .../Identity/Query/RaListingQuery.php | 37 ++------------- .../Identity/Query/RaSecondFactorQuery.php | 46 ++++++------------- .../Identity/Query/RecoveryTokenQuery.php | 45 ++++-------------- .../Query/UnverifiedSecondFactorQuery.php | 5 +- .../VerifiedSecondFactorOfIdentityQuery.php | 5 +- .../Query/VerifiedSecondFactorQuery.php | 15 ++---- .../Query/VettedSecondFactorQuery.php | 5 +- .../Repository/RaSecondFactorRepository.php | 2 +- .../Repository/RecoveryTokenRepository.php | 2 +- .../Request/InstitutionParamConverter.php | 3 +- .../CommandAuthorizationServiceTest.php | 3 -- .../AddPipelineStagesCompilerPass.php | 4 -- .../EventHandling/BufferedEventBus.php | 2 +- .../SensitiveDataMessageStream.php | 5 +- 30 files changed, 82 insertions(+), 265 deletions(-) diff --git a/src/Surfnet/Stepup/DateTime/DateTime.php b/src/Surfnet/Stepup/DateTime/DateTime.php index 77556611d..00778cc8c 100644 --- a/src/Surfnet/Stepup/DateTime/DateTime.php +++ b/src/Surfnet/Stepup/DateTime/DateTime.php @@ -22,6 +22,7 @@ use DateTime as CoreDateTime; use Stringable; use Surfnet\Stepup\Exception\InvalidArgumentException; +use TypeError; /** * @SuppressWarnings(PHPMD.TooManyMethods) @@ -37,14 +38,15 @@ class DateTime implements Stringable /** * Allows for mocking of time. - * + * @see DateTimeHelper::setCurrentTime here you can see how now can be overridden using reflection * @var self|null */ - private static ?DateTime $now; + private static ?DateTime $now; // @phpstan-ignore-line PHPStan can not see that the DateTimeHelper is able to set the now value using reflection private readonly CoreDateTime $dateTime; /** + * @see DateTimeHelper::setCurrentTime here you can see how now can be overridden using reflection * @return self */ public static function now(): DateTime @@ -118,9 +120,7 @@ public function comesBeforeOrIsEqual(DateTime $dateTime): bool public function comesAfter(DateTime $dateTime): bool { - $end = $this->dateTime->getTimestamp(); - $start = $dateTime->dateTime->getTimestamp(); - return $end > $start; + return $this->dateTime > $dateTime->dateTime; } public function comesAfterOrIsEqual(DateTime $dateTime): bool @@ -128,24 +128,9 @@ public function comesAfterOrIsEqual(DateTime $dateTime): bool return $this->dateTime >= $dateTime->dateTime; } - /** - * @param $format - * @return string - */ - public function format($format): string + public function format(string $format): string { - $formatted = $this->dateTime->format($format); - - if ($formatted === false) { - throw new InvalidArgumentException( - sprintf( - 'Given format "%s" is not a valid format for DateTime', - $format, - ), - ); - } - - return $formatted; + return $this->dateTime->format($format); } /** diff --git a/src/Surfnet/Stepup/Identity/AuditLog/Metadata.php b/src/Surfnet/Stepup/Identity/AuditLog/Metadata.php index 1435235ad..ebc62e980 100644 --- a/src/Surfnet/Stepup/Identity/AuditLog/Metadata.php +++ b/src/Surfnet/Stepup/Identity/AuditLog/Metadata.php @@ -39,19 +39,10 @@ final class Metadata */ public Institution $identityInstitution; - /** - * @var Institution - */ - public Institution $raInstitution; + public ?Institution $raInstitution; - /** - * @var SecondFactorId|null - */ public ?SecondFactorId $secondFactorId; - /** - * @var SecondFactorType|null - */ public ?SecondFactorType $secondFactorType; /** @@ -59,16 +50,9 @@ final class Metadata */ public ?SecondFactorIdentifier $secondFactorIdentifier; - /** @var VettingType */ - public VettingType $vettingType; + public ?VettingType $vettingType; - /** - * @var RecoveryTokenId - */ - public RecoveryTokenId $recoveryTokenId; + public ?RecoveryTokenId $recoveryTokenId; - /** - * @var RecoveryTokenType - */ - public RecoveryTokenType $recoveryTokenType; + public ?RecoveryTokenType $recoveryTokenType; } diff --git a/src/Surfnet/Stepup/Identity/Entity/RegistrationAuthority.php b/src/Surfnet/Stepup/Identity/Entity/RegistrationAuthority.php index 87dbc7c11..92bdcf620 100644 --- a/src/Surfnet/Stepup/Identity/Entity/RegistrationAuthority.php +++ b/src/Surfnet/Stepup/Identity/Entity/RegistrationAuthority.php @@ -31,11 +31,11 @@ final class RegistrationAuthority extends SimpleEventSourcedEntity { private ?RegistrationAuthorityRole $role = null; - private ?Location $location = null; + private ?Location $location = null; // @phpstan-ignore-line PHPStan can not see that this field is written when serialized to the database - private ?ContactInformation $contactInformation = null; + private ?ContactInformation $contactInformation = null; // @phpstan-ignore-line PHPStan can not see that this field is written when serialized to the database - private ?Institution $institution = null; + private ?Institution $institution = null; // @phpstan-ignore-line PHPStan can not see that this field is written when serialized to the database public static function accreditWith( RegistrationAuthorityRole $role, diff --git a/src/Surfnet/Stepup/Identity/Event/SecondFactorVettedEvent.php b/src/Surfnet/Stepup/Identity/Event/SecondFactorVettedEvent.php index b1b9aa517..5ed1ac3f7 100644 --- a/src/Surfnet/Stepup/Identity/Event/SecondFactorVettedEvent.php +++ b/src/Surfnet/Stepup/Identity/Event/SecondFactorVettedEvent.php @@ -88,8 +88,7 @@ class SecondFactorVettedEvent extends IdentityEvent implements Forgettable, Righ */ public Locale $preferredLocale; - /** @var VettingType */ - public VettingType $vettingType; + public ?VettingType $vettingType; /** * @SuppressWarnings(PHPMD.ExcessiveParameterList) diff --git a/src/Surfnet/Stepup/Identity/Event/SecondFactorVettedWithoutTokenProofOfPossession.php b/src/Surfnet/Stepup/Identity/Event/SecondFactorVettedWithoutTokenProofOfPossession.php index 81d819c64..058b6334e 100644 --- a/src/Surfnet/Stepup/Identity/Event/SecondFactorVettedWithoutTokenProofOfPossession.php +++ b/src/Surfnet/Stepup/Identity/Event/SecondFactorVettedWithoutTokenProofOfPossession.php @@ -96,8 +96,7 @@ class SecondFactorVettedWithoutTokenProofOfPossession extends IdentityEvent impl */ public Locale $preferredLocale; - /** @var VettingType */ - public VettingType $vettingType; + public ?VettingType $vettingType; /** * @SuppressWarnings(PHPMD.ExcessiveParameterList) diff --git a/src/Surfnet/Stepup/Identity/Identity.php b/src/Surfnet/Stepup/Identity/Identity.php index d3da0c230..8f6dfed00 100644 --- a/src/Surfnet/Stepup/Identity/Identity.php +++ b/src/Surfnet/Stepup/Identity/Identity.php @@ -821,9 +821,10 @@ public function complyWithSecondFactorRevocation(SecondFactorId $secondFactorId, public function revokeRecoveryToken(RecoveryTokenId $recoveryTokenId): void { $this->assertNotForgotten(); - $recoveryToken = $this->recoveryTokens->get($recoveryTokenId); - if (!$recoveryToken) { - throw new DomainException('Cannot revoke recovery token: no token with given id exists.'); + try { + $recoveryToken = $this->recoveryTokens->get($recoveryTokenId); + } catch (DomainException $e) { + throw new DomainException('Cannot revoke recovery token: no token with given id exists.', 0, $e); } $recoveryToken->revoke(); } @@ -831,9 +832,10 @@ public function revokeRecoveryToken(RecoveryTokenId $recoveryTokenId): void public function complyWithRecoveryTokenRevocation(RecoveryTokenId $recoveryTokenId, IdentityId $authorityId): void { $this->assertNotForgotten(); - $recoveryToken = $this->recoveryTokens->get($recoveryTokenId); - if (!$recoveryToken) { - throw new DomainException('Cannot revoke recovery token: no token with given id exists.'); + try { + $recoveryToken = $this->recoveryTokens->get($recoveryTokenId); + } catch (DomainException $e) { + throw new DomainException('Cannot revoke recovery token: no token with given id exists.', 0, $e); } $recoveryToken->complyWithRevocation($authorityId); } diff --git a/src/Surfnet/StepupMiddleware/ApiBundle/Authorization/Service/AuthorizationService.php b/src/Surfnet/StepupMiddleware/ApiBundle/Authorization/Service/AuthorizationService.php index c3019a7cb..0b3b96314 100644 --- a/src/Surfnet/StepupMiddleware/ApiBundle/Authorization/Service/AuthorizationService.php +++ b/src/Surfnet/StepupMiddleware/ApiBundle/Authorization/Service/AuthorizationService.php @@ -96,7 +96,7 @@ public function assertRegistrationOfSelfAssertedTokensIsAllowed(IdentityId $iden } // The Identity is not allowed to do a SAT when he had a RT, but lost it. And also currently has no SF $hasActiveRecoveryToken = $this->recoveryTokenService->identityHasActiveRecoveryToken($identity); - if ($options->possessedSelfAssertedToken && !$hasActiveRecoveryToken && !$hasVettedSecondFactorToken) { + if ($options->possessedSelfAssertedToken && !$hasActiveRecoveryToken) { return $this->deny('Identity lost both Recovery and Second Factor token, SAT is not allowed'); } diff --git a/src/Surfnet/StepupMiddleware/ApiBundle/Configuration/Projector/InstitutionAuthorizationProjector.php b/src/Surfnet/StepupMiddleware/ApiBundle/Configuration/Projector/InstitutionAuthorizationProjector.php index 7fd76f770..a13db3a70 100644 --- a/src/Surfnet/StepupMiddleware/ApiBundle/Configuration/Projector/InstitutionAuthorizationProjector.php +++ b/src/Surfnet/StepupMiddleware/ApiBundle/Configuration/Projector/InstitutionAuthorizationProjector.php @@ -34,7 +34,6 @@ final class InstitutionAuthorizationProjector extends Projector { public function __construct( private readonly InstitutionAuthorizationRepository $institutionAuthorizationRepository, - private readonly InstitutionConfigurationOptionsRepository $institutionConfigurationOptionsRepository, ) { } diff --git a/src/Surfnet/StepupMiddleware/ApiBundle/Configuration/Projector/RaLocationProjector.php b/src/Surfnet/StepupMiddleware/ApiBundle/Configuration/Projector/RaLocationProjector.php index cc308a544..183ec7946 100644 --- a/src/Surfnet/StepupMiddleware/ApiBundle/Configuration/Projector/RaLocationProjector.php +++ b/src/Surfnet/StepupMiddleware/ApiBundle/Configuration/Projector/RaLocationProjector.php @@ -88,10 +88,7 @@ public function applyInstitutionConfigurationRemovedEvent(InstitutionConfigurati $this->repository->removeRaLocationsFor($event->institution); } - /** - * @return RaLocation - */ - private function fetchRaLocationById(RaLocationId $raLocationId) + private function fetchRaLocationById(RaLocationId $raLocationId): RaLocation { $raLocation = $this->repository->findByRaLocationId($raLocationId); @@ -101,12 +98,6 @@ private function fetchRaLocationById(RaLocationId $raLocationId) ); } - if (!$raLocation instanceof RaLocation) { - throw new RuntimeException( - 'Tried to update an RA Locations contact information, but location is of the wrong type', - ); - } - return $raLocation; } } diff --git a/src/Surfnet/StepupMiddleware/ApiBundle/Configuration/Repository/RaLocationRepository.php b/src/Surfnet/StepupMiddleware/ApiBundle/Configuration/Repository/RaLocationRepository.php index d659a8032..924a31fad 100644 --- a/src/Surfnet/StepupMiddleware/ApiBundle/Configuration/Repository/RaLocationRepository.php +++ b/src/Surfnet/StepupMiddleware/ApiBundle/Configuration/Repository/RaLocationRepository.php @@ -55,10 +55,7 @@ public function search(RaLocationQuery $query) ->getResult(); } - /** - * @return RaLocation[] - */ - public function findByRaLocationId(RaLocationId $raLocationId) + public function findByRaLocationId(RaLocationId $raLocationId): ?RaLocation { return $this->createQueryBuilder('rl') ->where('rl.id = :id') @@ -84,7 +81,7 @@ public function remove(RaLocation $raLocation): void /** * @return RaLocation[] */ - public function findByInstitution(Institution $institution) + public function findByInstitution(Institution $institution): array { return $this->createQueryBuilder('rl') ->where('rl.institution = :institution') diff --git a/src/Surfnet/StepupMiddleware/ApiBundle/Configuration/Service/AllowedSecondFactorListService.php b/src/Surfnet/StepupMiddleware/ApiBundle/Configuration/Service/AllowedSecondFactorListService.php index 127ab72c1..0f9bfb775 100644 --- a/src/Surfnet/StepupMiddleware/ApiBundle/Configuration/Service/AllowedSecondFactorListService.php +++ b/src/Surfnet/StepupMiddleware/ApiBundle/Configuration/Service/AllowedSecondFactorListService.php @@ -22,13 +22,11 @@ use Surfnet\Stepup\Configuration\Value\Institution; use Surfnet\StepupMiddleware\ApiBundle\Configuration\Entity\AllowedSecondFactor; use Surfnet\StepupMiddleware\ApiBundle\Configuration\Repository\AllowedSecondFactorRepository; -use Surfnet\StepupMiddleware\ApiBundle\Configuration\Repository\ConfiguredInstitutionRepository; class AllowedSecondFactorListService { public function __construct( private readonly AllowedSecondFactorRepository $allowedSecondFactorRepository, - private readonly ConfiguredInstitutionRepository $configuredInstitutionRepository, ) { } diff --git a/src/Surfnet/StepupMiddleware/ApiBundle/Configuration/Service/RaLocationService.php b/src/Surfnet/StepupMiddleware/ApiBundle/Configuration/Service/RaLocationService.php index 29dc42e14..0100df773 100644 --- a/src/Surfnet/StepupMiddleware/ApiBundle/Configuration/Service/RaLocationService.php +++ b/src/Surfnet/StepupMiddleware/ApiBundle/Configuration/Service/RaLocationService.php @@ -33,15 +33,12 @@ public function __construct(private readonly RaLocationRepository $repository) /** * @return null|RaLocation[] */ - public function search(RaLocationQuery $query) + public function search(RaLocationQuery $query): ?array { return $this->repository->search($query); } - /** - * @return RaLocation[] - */ - public function findByRaLocationId(RaLocationId $raLocationId) + public function findByRaLocationId(RaLocationId $raLocationId): ?RaLocation { return $this->repository->findByRaLocationId($raLocationId); } @@ -50,7 +47,7 @@ public function findByRaLocationId(RaLocationId $raLocationId) /** * @return RaLocation[] */ - public function listRaLocationsFor(Institution $institution) + public function listRaLocationsFor(Institution $institution): ?array { return $this->repository->findByInstitution($institution); } diff --git a/src/Surfnet/StepupMiddleware/ApiBundle/Controller/IdentityController.php b/src/Surfnet/StepupMiddleware/ApiBundle/Controller/IdentityController.php index 1119e39a6..a231c531a 100644 --- a/src/Surfnet/StepupMiddleware/ApiBundle/Controller/IdentityController.php +++ b/src/Surfnet/StepupMiddleware/ApiBundle/Controller/IdentityController.php @@ -18,9 +18,7 @@ namespace Surfnet\StepupMiddleware\ApiBundle\Controller; -use Surfnet\Stepup\Configuration\Value\InstitutionRole; use Surfnet\Stepup\Identity\Value\Institution; -use Surfnet\StepupMiddleware\ApiBundle\Authorization\Value\InstitutionRoleSet; use Surfnet\StepupMiddleware\ApiBundle\Identity\Entity\Identity; use Surfnet\StepupMiddleware\ApiBundle\Identity\Query\IdentityQuery; use Surfnet\StepupMiddleware\ApiBundle\Identity\Service\IdentityService; @@ -33,14 +31,9 @@ class IdentityController extends AbstractController { - private readonly InstitutionRoleSet $roleRequirements; - public function __construct( - private IdentityService $identityService, + private readonly IdentityService $identityService, ) { - $this->roleRequirements = new InstitutionRoleSet( - [new InstitutionRole(InstitutionRole::ROLE_USE_RA), new InstitutionRole(InstitutionRole::ROLE_USE_RAA)], - ); } public function get($id): JsonResponse diff --git a/src/Surfnet/StepupMiddleware/ApiBundle/Identity/Entity/VettedSecondFactor.php b/src/Surfnet/StepupMiddleware/ApiBundle/Identity/Entity/VettedSecondFactor.php index 58e91b756..f3ea7d99b 100644 --- a/src/Surfnet/StepupMiddleware/ApiBundle/Identity/Entity/VettedSecondFactor.php +++ b/src/Surfnet/StepupMiddleware/ApiBundle/Identity/Entity/VettedSecondFactor.php @@ -29,41 +29,22 @@ #[ORM\Entity(repositoryClass: VettedSecondFactorRepository::class)] class VettedSecondFactor implements JsonSerializable { - /** - * @var string - */ #[ORM\Id] #[ORM\Column(length: 36)] public string $id; - /** - * @var string - */ #[ORM\Column(length: 36)] public string $identityId; - /** - * @var string - */ #[ORM\Column(length: 16)] public string $type; - /** - * The second factor identifier, ie. telephone number, Yubikey public ID, Tiqr ID - * @var string - */ #[ORM\Column(length: 255)] public string $secondFactorIdentifier; - /** - * @var string - */ #[ORM\Column(length: 255, nullable: true)] - public string $vettingType; + public ?string $vettingType; - /** - * @return bool - */ public function isEqual(VettedSecondFactor $vettedSecondFactor): bool { return $vettedSecondFactor->type == $this->type && $vettedSecondFactor->secondFactorIdentifier == $this->secondFactorIdentifier; @@ -71,7 +52,7 @@ public function isEqual(VettedSecondFactor $vettedSecondFactor): bool public function vettingType(): string { - if (is_null($this->vettingType)) { + if (!$this->vettingType) { return VettingType::TYPE_ON_PREMISE; } return $this->vettingType; diff --git a/src/Surfnet/StepupMiddleware/ApiBundle/Identity/Projector/AuditLogProjector.php b/src/Surfnet/StepupMiddleware/ApiBundle/Identity/Projector/AuditLogProjector.php index 11ebe9274..73babc76e 100644 --- a/src/Surfnet/StepupMiddleware/ApiBundle/Identity/Projector/AuditLogProjector.php +++ b/src/Surfnet/StepupMiddleware/ApiBundle/Identity/Projector/AuditLogProjector.php @@ -31,6 +31,7 @@ use Surfnet\Stepup\Identity\Value\CommonName; use Surfnet\Stepup\Identity\Value\RecoveryTokenIdentifierFactory; use Surfnet\Stepup\Identity\Value\RecoveryTokenType; +use Surfnet\Stepup\Identity\Value\VettingType; use Surfnet\StepupMiddleware\ApiBundle\Exception\RuntimeException; use Surfnet\StepupMiddleware\ApiBundle\Identity\Entity\AuditLogEntry; use Surfnet\StepupMiddleware\ApiBundle\Identity\Entity\Identity; @@ -160,7 +161,7 @@ private function applyIdentityForgottenEvent(IdentityForgottenEvent $event): voi private function augmentActorCommonName(AuditLogEntry $entry, Metadata $auditLogMetadata): void { - if (property_exists($auditLogMetadata, 'vettingType') && !is_null($auditLogMetadata->vettingType)) { + if (property_exists($auditLogMetadata, 'vettingType') && $auditLogMetadata->vettingType instanceof VettingType) { $entry->actorCommonName = new CommonName( $entry->actorCommonName->getCommonName() . $auditLogMetadata->vettingType->auditLog() ); diff --git a/src/Surfnet/StepupMiddleware/ApiBundle/Identity/Projector/RaListingProjector.php b/src/Surfnet/StepupMiddleware/ApiBundle/Identity/Projector/RaListingProjector.php index 8a5337575..dfaca7205 100644 --- a/src/Surfnet/StepupMiddleware/ApiBundle/Identity/Projector/RaListingProjector.php +++ b/src/Surfnet/StepupMiddleware/ApiBundle/Identity/Projector/RaListingProjector.php @@ -50,9 +50,6 @@ public function __construct( ) { } - /** - * @return void - */ public function applyIdentityAccreditedAsRaForInstitutionEvent(IdentityAccreditedAsRaForInstitutionEvent $event,): void { $identity = $this->identityRepository->find((string)$event->identityId); @@ -71,9 +68,6 @@ public function applyIdentityAccreditedAsRaForInstitutionEvent(IdentityAccredite $this->raListingRepository->save($raListing); } - /** - * @return void - */ public function applyIdentityAccreditedAsRaaForInstitutionEvent(IdentityAccreditedAsRaaForInstitutionEvent $event,): void { $identity = $this->identityRepository->find((string)$event->identityId); @@ -95,7 +89,6 @@ public function applyIdentityAccreditedAsRaaForInstitutionEvent(IdentityAccredit public function applyRegistrationAuthorityInformationAmendedForInstitutionEvent( RegistrationAuthorityInformationAmendedForInstitutionEvent $event, ): void { - /** @var RaListing $raListing */ $raListing = $this->raListingRepository->findByIdentityIdAndRaInstitution( $event->identityId, $event->raInstitution, @@ -116,7 +109,6 @@ public function applyRegistrationAuthorityInformationAmendedForInstitutionEvent( public function applyAppointedAsRaForInstitutionEvent(AppointedAsRaForInstitutionEvent $event): void { - /** @var RaListing $raListing */ $raListing = $this->raListingRepository->findByIdentityIdAndRaInstitution( $event->identityId, $event->raInstitution, @@ -129,7 +121,6 @@ public function applyAppointedAsRaForInstitutionEvent(AppointedAsRaForInstitutio public function applyAppointedAsRaaForInstitutionEvent(AppointedAsRaaForInstitutionEvent $event): void { - /** @var RaListing $raListing */ $raListing = $this->raListingRepository->findByIdentityIdAndRaInstitution( $event->identityId, $event->raInstitution, @@ -160,7 +151,6 @@ protected function applyIdentityForgottenEvent(IdentityForgottenEvent $event): v public function applyIdentityAccreditedAsRaEvent(IdentityAccreditedAsRaEvent $event): void { $identity = $this->identityRepository->find((string)$event->identityId); - $raListing = RaListing::create( (string)$event->identityId, $event->identityInstitution, @@ -183,7 +173,6 @@ public function applyIdentityAccreditedAsRaEvent(IdentityAccreditedAsRaEvent $ev public function applyIdentityAccreditedAsRaaEvent(IdentityAccreditedAsRaaEvent $event): void { $identity = $this->identityRepository->find((string)$event->identityId); - $raListing = RaListing::create( (string)$event->identityId, $event->identityInstitution, @@ -204,7 +193,6 @@ public function applyIdentityAccreditedAsRaaEvent(IdentityAccreditedAsRaaEvent $ public function applyRegistrationAuthorityInformationAmendedEvent( RegistrationAuthorityInformationAmendedEvent $event, ): void { - /** @var RaListing $raListing */ $raListing = $this->raListingRepository->findByIdentityIdAndRaInstitution( $event->identityId, $event->identityInstitution, @@ -228,13 +216,14 @@ public function applyRegistrationAuthorityInformationAmendedEvent( */ public function applyAppointedAsRaEvent(AppointedAsRaEvent $event): void { - /** @var RaListing $raListing */ $raListing = $this->raListingRepository->findByIdentityIdAndInstitution( $event->identityId, $event->identityInstitution, ); - $raListing->role = AuthorityRole::ra(); + foreach ($raListing as $listing) { + $listing->role = AuthorityRole::ra(); + } $this->raListingRepository->save($raListing); } @@ -244,13 +233,14 @@ public function applyAppointedAsRaEvent(AppointedAsRaEvent $event): void */ public function applyAppointedAsRaaEvent(AppointedAsRaaEvent $event): void { - /** @var RaListing $raListing */ $raListing = $this->raListingRepository->findByIdentityIdAndInstitution( $event->identityId, $event->identityInstitution, ); - $raListing->role = AuthorityRole::raa(); + foreach ($raListing as $listing) { + $listing->role = AuthorityRole::raa(); + } $this->raListingRepository->save($raListing); } diff --git a/src/Surfnet/StepupMiddleware/ApiBundle/Identity/Query/RaListingQuery.php b/src/Surfnet/StepupMiddleware/ApiBundle/Identity/Query/RaListingQuery.php index fbfda7f93..4a85b8bf3 100644 --- a/src/Surfnet/StepupMiddleware/ApiBundle/Identity/Query/RaListingQuery.php +++ b/src/Surfnet/StepupMiddleware/ApiBundle/Identity/Query/RaListingQuery.php @@ -24,48 +24,21 @@ class RaListingQuery extends AbstractQuery { - /** - * @var string|Institution - */ public string|Institution $institution; - /** - * @var IdentityId - */ - public IdentityId $identityId; + public ?IdentityId $identityId = null; - /** - * @var string|null - */ - public ?string $name; + public ?string $name = null; - /** - * @var string|null - */ - public ?string $email; + public ?string $email = null; - /** - * @var string|null - */ - public ?string $role; + public ?string $role = null; - /** - * @var string|null - */ - public ?string $raInstitution; + public ?string $raInstitution = null; - /** - * @var string - */ public string $orderBy; - /** - * @var string - */ public string $orderDirection; - /** - * @var InstitutionAuthorizationContextInterface - */ public InstitutionAuthorizationContextInterface $authorizationContext; } diff --git a/src/Surfnet/StepupMiddleware/ApiBundle/Identity/Query/RaSecondFactorQuery.php b/src/Surfnet/StepupMiddleware/ApiBundle/Identity/Query/RaSecondFactorQuery.php index 12d2bfaf9..7c26d277d 100644 --- a/src/Surfnet/StepupMiddleware/ApiBundle/Identity/Query/RaSecondFactorQuery.php +++ b/src/Surfnet/StepupMiddleware/ApiBundle/Identity/Query/RaSecondFactorQuery.php @@ -22,48 +22,30 @@ final class RaSecondFactorQuery extends AbstractQuery { - /** - * @var string|null - */ - public ?string $name; + public ?string $name = null; - /** - * @var string|null - */ - public ?string $type; + public ?string $type = null; - /** - * @var string|null The second factor type's ID (eg. Yubikey public ID) + /* + * The second factor type's ID (eg. Yubikey public ID) */ - public ?string $secondFactorId; + public ?string $secondFactorId = null; - /** - * @var string|null - */ - public ?string $email; + public ?string $email = null; - /** - * @var string|null the filter value, not to be confused with the actorInstitution which is used for authorizations. + /* + * the filter value, not to be confused with the actorInstitution which is used for authorizations. */ - public ?string $institution; + public ?string $institution = null; - /** - * @var string|null One of the ApiBundle\Identity\Entity\RaSecondFactor::STATUS_* constants. + /* + * One of the ApiBundle\Identity\Entity\RaSecondFactor::STATUS_* constants. */ - public ?string $status; + public ?string $status = null; - /** - * @var string|null - */ - public ?string $orderBy; + public ?string $orderBy = null; - /** - * @var string|null - */ - public ?string $orderDirection; + public ?string $orderDirection = null; - /** - * @var InstitutionAuthorizationContextInterface - */ public InstitutionAuthorizationContextInterface $authorizationContext; } diff --git a/src/Surfnet/StepupMiddleware/ApiBundle/Identity/Query/RecoveryTokenQuery.php b/src/Surfnet/StepupMiddleware/ApiBundle/Identity/Query/RecoveryTokenQuery.php index 450d1e857..713c64be2 100644 --- a/src/Surfnet/StepupMiddleware/ApiBundle/Identity/Query/RecoveryTokenQuery.php +++ b/src/Surfnet/StepupMiddleware/ApiBundle/Identity/Query/RecoveryTokenQuery.php @@ -23,48 +23,21 @@ class RecoveryTokenQuery extends AbstractQuery { - /** - * @var IdentityId - */ - public IdentityId $identityId; + public ?IdentityId $identityId = null; - /** - * @var string|null - */ - public ?string $type; + public ?string $type = null; - /** - * @var string|null - */ - public ?string $status; + public ?string $status = null; - /** - * @var string|null - */ - public ?string $institution; + public ?string $institution = null; - /** - * @var string|null - */ - public ?string $name; + public ?string $name = null; - /** - * @var string|null - */ - public ?string $email; + public ?string $email = null; - /** - * @var string|null - */ - public ?string $orderBy; + public ?string $orderBy = null; - /** - * @var string|null - */ - public ?string $orderDirection; + public ?string $orderDirection = null; - /** - * @var InstitutionAuthorizationContextInterface - */ - public InstitutionAuthorizationContextInterface $authorizationContext; + public ?InstitutionAuthorizationContextInterface $authorizationContext = null; } diff --git a/src/Surfnet/StepupMiddleware/ApiBundle/Identity/Query/UnverifiedSecondFactorQuery.php b/src/Surfnet/StepupMiddleware/ApiBundle/Identity/Query/UnverifiedSecondFactorQuery.php index 5e43a6caf..5bbc0a001 100644 --- a/src/Surfnet/StepupMiddleware/ApiBundle/Identity/Query/UnverifiedSecondFactorQuery.php +++ b/src/Surfnet/StepupMiddleware/ApiBundle/Identity/Query/UnverifiedSecondFactorQuery.php @@ -22,10 +22,7 @@ class UnverifiedSecondFactorQuery extends AbstractQuery { - /** - * @var IdentityId - */ - public IdentityId $identityId; + public ?IdentityId $identityId = null; /** * @var string|null diff --git a/src/Surfnet/StepupMiddleware/ApiBundle/Identity/Query/VerifiedSecondFactorOfIdentityQuery.php b/src/Surfnet/StepupMiddleware/ApiBundle/Identity/Query/VerifiedSecondFactorOfIdentityQuery.php index 8a9b3d80c..fb5775f15 100644 --- a/src/Surfnet/StepupMiddleware/ApiBundle/Identity/Query/VerifiedSecondFactorOfIdentityQuery.php +++ b/src/Surfnet/StepupMiddleware/ApiBundle/Identity/Query/VerifiedSecondFactorOfIdentityQuery.php @@ -22,8 +22,5 @@ class VerifiedSecondFactorOfIdentityQuery extends AbstractQuery { - /** - * @var IdentityId|null - */ - public ?IdentityId $identityId; + public ?IdentityId $identityId = null; } diff --git a/src/Surfnet/StepupMiddleware/ApiBundle/Identity/Query/VerifiedSecondFactorQuery.php b/src/Surfnet/StepupMiddleware/ApiBundle/Identity/Query/VerifiedSecondFactorQuery.php index 03f0b9e40..f8a9b2e1a 100644 --- a/src/Surfnet/StepupMiddleware/ApiBundle/Identity/Query/VerifiedSecondFactorQuery.php +++ b/src/Surfnet/StepupMiddleware/ApiBundle/Identity/Query/VerifiedSecondFactorQuery.php @@ -24,20 +24,11 @@ class VerifiedSecondFactorQuery extends AbstractQuery { - /** - * @var IdentityId|null - */ - public ?IdentityId $identityId; + public ?IdentityId $identityId = null; - /** - * @var SecondFactorId|null - */ - public ?SecondFactorId $secondFactorId; + public ?SecondFactorId $secondFactorId = null; - /** - * @var string|null - */ - public ?string $registrationCode; + public ?string $registrationCode = null; /** * @var InstitutionAuthorizationContext diff --git a/src/Surfnet/StepupMiddleware/ApiBundle/Identity/Query/VettedSecondFactorQuery.php b/src/Surfnet/StepupMiddleware/ApiBundle/Identity/Query/VettedSecondFactorQuery.php index 4fad4cef3..d8d1cad7c 100644 --- a/src/Surfnet/StepupMiddleware/ApiBundle/Identity/Query/VettedSecondFactorQuery.php +++ b/src/Surfnet/StepupMiddleware/ApiBundle/Identity/Query/VettedSecondFactorQuery.php @@ -22,8 +22,5 @@ class VettedSecondFactorQuery extends AbstractQuery { - /** - * @var IdentityId - */ - public IdentityId $identityId; + public ?IdentityId $identityId = null; } diff --git a/src/Surfnet/StepupMiddleware/ApiBundle/Identity/Repository/RaSecondFactorRepository.php b/src/Surfnet/StepupMiddleware/ApiBundle/Identity/Repository/RaSecondFactorRepository.php index 9a997be5e..e9bd671dc 100644 --- a/src/Surfnet/StepupMiddleware/ApiBundle/Identity/Repository/RaSecondFactorRepository.php +++ b/src/Surfnet/StepupMiddleware/ApiBundle/Identity/Repository/RaSecondFactorRepository.php @@ -113,7 +113,7 @@ public function createSearchQuery(RaSecondFactorQuery $query): Query throw new RuntimeException( sprintf( 'Received invalid status "%s" in RaSecondFactorRepository::createSearchQuery', - is_object($stringStatus) ? $stringStatus::class : (string)$stringStatus, + $stringStatus, ), ); } diff --git a/src/Surfnet/StepupMiddleware/ApiBundle/Identity/Repository/RecoveryTokenRepository.php b/src/Surfnet/StepupMiddleware/ApiBundle/Identity/Repository/RecoveryTokenRepository.php index 050c63281..9923a9a25 100644 --- a/src/Surfnet/StepupMiddleware/ApiBundle/Identity/Repository/RecoveryTokenRepository.php +++ b/src/Surfnet/StepupMiddleware/ApiBundle/Identity/Repository/RecoveryTokenRepository.php @@ -86,7 +86,7 @@ public function createSearchQuery(RecoveryTokenQuery $query): Query throw new RuntimeException( sprintf( 'Received invalid status "%s" in RecoveryTokenRepository::createSearchQuery', - is_object($stringStatus) ? $stringStatus::class : (string)$stringStatus, + $stringStatus, ), ); } diff --git a/src/Surfnet/StepupMiddleware/ApiBundle/Request/InstitutionParamConverter.php b/src/Surfnet/StepupMiddleware/ApiBundle/Request/InstitutionParamConverter.php index 9f06abeba..b4c5d7f74 100644 --- a/src/Surfnet/StepupMiddleware/ApiBundle/Request/InstitutionParamConverter.php +++ b/src/Surfnet/StepupMiddleware/ApiBundle/Request/InstitutionParamConverter.php @@ -23,6 +23,7 @@ use Surfnet\Stepup\Identity\Value\Institution; use Surfnet\StepupMiddleware\ApiBundle\Exception\BadApiRequestException; use Symfony\Component\HttpFoundation\Request; +use function is_bool; class InstitutionParamConverter implements ParamConverterInterface { @@ -31,7 +32,7 @@ public function apply(Request $request, ParamConverter $configuration): bool $query = $request->query; $institution = $query->get('institution', false); - if ($institution === false) { + if (!is_string($institution)) { throw new BadApiRequestException(['This API-call MUST include the institution as get parameter']); } diff --git a/src/Surfnet/StepupMiddleware/ApiBundle/Tests/Authorization/Service/CommandAuthorizationServiceTest.php b/src/Surfnet/StepupMiddleware/ApiBundle/Tests/Authorization/Service/CommandAuthorizationServiceTest.php index ff665b3dc..5d89ca7c1 100644 --- a/src/Surfnet/StepupMiddleware/ApiBundle/Tests/Authorization/Service/CommandAuthorizationServiceTest.php +++ b/src/Surfnet/StepupMiddleware/ApiBundle/Tests/Authorization/Service/CommandAuthorizationServiceTest.php @@ -83,8 +83,6 @@ class CommandAuthorizationServiceTest extends TestCase { use MockeryPHPUnitIntegration; - private WhitelistService&MockInterface $whitelistService; - private IdentityService&MockInterface $identityService; private LoggerInterface&MockInterface $logger; @@ -107,7 +105,6 @@ public function setUp(): void $authorizationContextService, ); - $this->whitelistService = $whitelistService; $this->identityService = $identityService; $this->logger = $logger; $this->authorizationContextService = $authorizationContextService; diff --git a/src/Surfnet/StepupMiddleware/CommandHandlingBundle/DependencyInjection/CompilerPass/AddPipelineStagesCompilerPass.php b/src/Surfnet/StepupMiddleware/CommandHandlingBundle/DependencyInjection/CompilerPass/AddPipelineStagesCompilerPass.php index f1f95c04c..a43d46fb9 100644 --- a/src/Surfnet/StepupMiddleware/CommandHandlingBundle/DependencyInjection/CompilerPass/AddPipelineStagesCompilerPass.php +++ b/src/Surfnet/StepupMiddleware/CommandHandlingBundle/DependencyInjection/CompilerPass/AddPipelineStagesCompilerPass.php @@ -53,10 +53,6 @@ public function process(ContainerBuilder $container): void $prioritized[$priority] = new Reference($stageServiceId); } - if (!ksort($prioritized)) { - throw new RuntimeException('Could not sort stages based on prioritization (ksort failed)'); - } - // ksort sorts low -> high, so reversing to get them sorted correctly. $prioritized = array_reverse($prioritized); diff --git a/src/Surfnet/StepupMiddleware/CommandHandlingBundle/EventHandling/BufferedEventBus.php b/src/Surfnet/StepupMiddleware/CommandHandlingBundle/EventHandling/BufferedEventBus.php index 3ebbe965f..47e24d705 100644 --- a/src/Surfnet/StepupMiddleware/CommandHandlingBundle/EventHandling/BufferedEventBus.php +++ b/src/Surfnet/StepupMiddleware/CommandHandlingBundle/EventHandling/BufferedEventBus.php @@ -103,7 +103,7 @@ public function flush(): void unset($buffer); // if during the handling of events new events have been queued, we need to flush them - if ($this->buffer !== []) { + if (!empty($this->buffer)) { $this->flush(); } } diff --git a/src/Surfnet/StepupMiddleware/CommandHandlingBundle/SensitiveData/EventSourcing/SensitiveDataMessageStream.php b/src/Surfnet/StepupMiddleware/CommandHandlingBundle/SensitiveData/EventSourcing/SensitiveDataMessageStream.php index fd2183eba..d61b41f25 100644 --- a/src/Surfnet/StepupMiddleware/CommandHandlingBundle/SensitiveData/EventSourcing/SensitiveDataMessageStream.php +++ b/src/Surfnet/StepupMiddleware/CommandHandlingBundle/SensitiveData/EventSourcing/SensitiveDataMessageStream.php @@ -69,9 +69,6 @@ public function getIterator(): Iterator return new ArrayIterator($this->messages); } - /** - * @param SensitiveDataMessage|null $sensitiveDataMessage - */ private function setSensitiveData( DomainMessage $domainMessage, SensitiveDataMessage $sensitiveDataMessage = null, @@ -93,7 +90,7 @@ private function setSensitiveData( ); } - if (!$eventIsForgettable && $sensitiveDataMessage) { + if (!$eventIsForgettable) { throw new SensitiveDataApplicationException( sprintf( 'Encountered sensitive data for event which does not support sensitive data, UUID %s, playhead %d',