Remove usage of getimagesize

We can not use the url_fopen portion of the getimagesize function. This is the case when you use getimagesize with a remote file location. 1. For the ValidLogoValidator: We can exclusively use the curl based validation (already implemented in the CurlLogoValidationHelper 2. The message sent to Manage does not require a valid with and height to be set, as they will be esteablished on the logo 'cdn' See: https://www.pivotaltracker.com/n/projects/1400064/stories/186921877
SURFnet · Sep 16, 2024 · 218d33d · 218d33d
1 parent 109455b
commit 218d33d
Show file tree

Hide file tree

Showing 8 changed files with 20 additions and 158 deletions.
diff --git a/ci/qa/phpstan-baseline.php b/ci/qa/phpstan-baseline.php
@@ -928,7 +928,7 @@
 ];
 $ignoreErrors[] = [
  'message' => '#^If condition is always true\\.$#',
- 'count' => 2,
+ 'count' => 1,
  'path' => __DIR__ . '/../../src/Surfnet/ServiceProviderDashboard/Application/Metadata/JsonGenerator.php',
 ];
 $ignoreErrors[] = [
@@ -991,11 +991,6 @@
  'count' => 1,
  'path' => __DIR__ . '/../../src/Surfnet/ServiceProviderDashboard/Application/Metadata/JsonGenerator.php',
 ];
-$ignoreErrors[] = [
- 'message' => '#^Method Surfnet\\\\ServiceProviderDashboard\\\\Application\\\\Metadata\\\\JsonGenerator\\:\\:generateLogoMetadata\\(\\) return type has no value type specified in iterable type array\\.$#',
- 'count' => 1,
- 'path' => __DIR__ . '/../../src/Surfnet/ServiceProviderDashboard/Application/Metadata/JsonGenerator.php',
-];
 $ignoreErrors[] = [
  'message' => '#^Method Surfnet\\\\ServiceProviderDashboard\\\\Application\\\\Metadata\\\\JsonGenerator\\:\\:generateOrganizationMetadata\\(\\) return type has no value type specified in iterable type array\\.$#',
  'count' => 1,
@@ -1011,11 +1006,6 @@
  'count' => 1,
  'path' => __DIR__ . '/../../src/Surfnet/ServiceProviderDashboard/Application/Metadata/JsonGenerator.php',
 ];
-$ignoreErrors[] = [
- 'message' => '#^Parameter \\#1 \\$filename of function getimagesize expects string, string\\|null given\\.$#',
- 'count' => 1,
- 'path' => __DIR__ . '/../../src/Surfnet/ServiceProviderDashboard/Application/Metadata/JsonGenerator.php',
-];
 $ignoreErrors[] = [
  'message' => '#^Right side of && is always true\\.$#',
  'count' => 1,
@@ -1181,11 +1171,6 @@
  'count' => 1,
  'path' => __DIR__ . '/../../src/Surfnet/ServiceProviderDashboard/Application/Metadata/OauthClientCredentialsClientJsonGenerator.php',
 ];
-$ignoreErrors[] = [
- 'message' => '#^Method Surfnet\\\\ServiceProviderDashboard\\\\Application\\\\Metadata\\\\OauthClientCredentialsClientJsonGenerator\\:\\:generateLogoMetadata\\(\\) return type has no value type specified in iterable type array\\.$#',
- 'count' => 1,
- 'path' => __DIR__ . '/../../src/Surfnet/ServiceProviderDashboard/Application/Metadata/OauthClientCredentialsClientJsonGenerator.php',
-];
 $ignoreErrors[] = [
  'message' => '#^Method Surfnet\\\\ServiceProviderDashboard\\\\Application\\\\Metadata\\\\OauthClientCredentialsClientJsonGenerator\\:\\:generateMetadataFields\\(\\) never returns float so it can be removed from the return type\\.$#',
  'count' => 1,
@@ -1221,11 +1206,6 @@
  'count' => 2,
  'path' => __DIR__ . '/../../src/Surfnet/ServiceProviderDashboard/Application/Metadata/OauthClientCredentialsClientJsonGenerator.php',
 ];
-$ignoreErrors[] = [
- 'message' => '#^Parameter \\#1 \\$filename of function getimagesize expects string, string\\|null given\\.$#',
- 'count' => 1,
- 'path' => __DIR__ . '/../../src/Surfnet/ServiceProviderDashboard/Application/Metadata/OauthClientCredentialsClientJsonGenerator.php',
-];
 $ignoreErrors[] = [
  'message' => '#^Cannot call method getClientSecret\\(\\) on Surfnet\\\\ServiceProviderDashboard\\\\Domain\\\\Entity\\\\Entity\\\\OidcClientInterface\\|null\\.$#',
  'count' => 1,
@@ -1341,11 +1321,6 @@
  'count' => 1,
  'path' => __DIR__ . '/../../src/Surfnet/ServiceProviderDashboard/Application/Metadata/OidcngJsonGenerator.php',
 ];
-$ignoreErrors[] = [
- 'message' => '#^Method Surfnet\\\\ServiceProviderDashboard\\\\Application\\\\Metadata\\\\OidcngJsonGenerator\\:\\:generateLogoMetadata\\(\\) return type has no value type specified in iterable type array\\.$#',
- 'count' => 1,
- 'path' => __DIR__ . '/../../src/Surfnet/ServiceProviderDashboard/Application/Metadata/OidcngJsonGenerator.php',
-];
 $ignoreErrors[] = [
  'message' => '#^Method Surfnet\\\\ServiceProviderDashboard\\\\Application\\\\Metadata\\\\OidcngJsonGenerator\\:\\:generateMetadataFields\\(\\) never returns float so it can be removed from the return type\\.$#',
  'count' => 1,
@@ -1381,11 +1356,6 @@
  'count' => 2,
  'path' => __DIR__ . '/../../src/Surfnet/ServiceProviderDashboard/Application/Metadata/OidcngJsonGenerator.php',
 ];
-$ignoreErrors[] = [
- 'message' => '#^Parameter \\#1 \\$filename of function getimagesize expects string, string\\|null given\\.$#',
- 'count' => 1,
- 'path' => __DIR__ . '/../../src/Surfnet/ServiceProviderDashboard/Application/Metadata/OidcngJsonGenerator.php',
-];
 $ignoreErrors[] = [
  'message' => '#^Cannot call method getClientSecret\\(\\) on Surfnet\\\\ServiceProviderDashboard\\\\Domain\\\\Entity\\\\Entity\\\\OidcClientInterface\\|null\\.$#',
  'count' => 1,
@@ -3866,11 +3836,6 @@
  'count' => 1,
  'path' => __DIR__ . '/../../src/Surfnet/ServiceProviderDashboard/Infrastructure/DashboardBundle/Service/CurlLogoValidationHelper.php',
 ];
-$ignoreErrors[] = [
- 'message' => '#^Method Surfnet\\\\ServiceProviderDashboard\\\\Infrastructure\\\\DashboardBundle\\\\Service\\\\LogoValidationHelperInterface\\:\\:validateLogo\\(\\) has no return type specified\\.$#',
- 'count' => 1,
- 'path' => __DIR__ . '/../../src/Surfnet/ServiceProviderDashboard/Infrastructure/DashboardBundle/Service/LogoValidationHelperInterface.php',
-];
 $ignoreErrors[] = [
  'message' => '#^Method Surfnet\\\\ServiceProviderDashboard\\\\Infrastructure\\\\DashboardBundle\\\\Service\\\\LogoValidationHelperInterface\\:\\:validateLogo\\(\\) has parameter \\$url with no type specified\\.$#',
  'count' => 1,
@@ -3956,16 +3921,6 @@
  'count' => 2,
  'path' => __DIR__ . '/../../src/Surfnet/ServiceProviderDashboard/Infrastructure/DashboardBundle/Validator/Constraints/ValidLogoValidator.php',
 ];
-$ignoreErrors[] = [
- 'message' => '#^Method Surfnet\\\\ServiceProviderDashboard\\\\Infrastructure\\\\DashboardBundle\\\\Validator\\\\Constraints\\\\ValidLogoValidator\\:\\:getImageSizeValidation\\(\\) has parameter \\$value with no type specified\\.$#',
- 'count' => 1,
- 'path' => __DIR__ . '/../../src/Surfnet/ServiceProviderDashboard/Infrastructure/DashboardBundle/Validator/Constraints/ValidLogoValidator.php',
-];
-$ignoreErrors[] = [
- 'message' => '#^Method Surfnet\\\\ServiceProviderDashboard\\\\Infrastructure\\\\DashboardBundle\\\\Validator\\\\Constraints\\\\ValidLogoValidator\\:\\:validate\\(\\) has parameter \\$value with no type specified\\.$#',
- 'count' => 1,
- 'path' => __DIR__ . '/../../src/Surfnet/ServiceProviderDashboard/Infrastructure/DashboardBundle/Validator/Constraints/ValidLogoValidator.php',
-];
 $ignoreErrors[] = [
  'message' => '#^Access to an undefined property Symfony\\\\Component\\\\Validator\\\\Constraint\\:\\:\\$protocols\\.$#',
  'count' => 1,

diff --git a/src/Surfnet/ServiceProviderDashboard/Application/Metadata/JsonGenerator.php b/src/Surfnet/ServiceProviderDashboard/Application/Metadata/JsonGenerator.php
@@ -227,7 +227,7 @@ private function generateMetadataFields(ManageEntity $entity): array
  $metadata['coin:exclude_from_push'] = '0';
  }
  if ($entity->getMetaData()->getLogo() instanceof Logo && $entity->getMetaData()->getLogo()->getUrl() !== '') {
- $metadata = array_merge($metadata, $this->generateLogoMetadata($entity));
+ $metadata['logo:0:url'] = $entity->getMetaData()->getLogo()->getUrl();
  }
  if ($entity->getMetaData()?->getCoin()->getContractualBase() !== null) {
  $metadata['coin:contractual_base'] = $entity->getMetaData()->getCoin()->getContractualBase();
@@ -308,41 +308,6 @@ private function generateContactMetadata(string $contactType, int $index, Contac
  return $metadata;
  }
 
- /**
- * Generate logo metadata fields.
- *
- * Logo dimensions are required in the SAML spec. They are always present,
- * except when the user just created the entity in the interface. We
- * determine the dimensions in those situations.
- *
- * @SuppressWarnings(PHPMD.ErrorControlOperator)
- */
- private function generateLogoMetadata(ManageEntity $entity): array
- {
- $logo = $entity->getMetaData()->getLogo();
- $metadata = [];
- if ($logo) {
- $metadata = [
- 'logo:0:url' => $logo->getUrl(),
- ];
-
- $logoData = @getimagesize(
- $logo->getUrl()
- );
-
- if ($logoData !== false) {
- [$width, $height] = $logoData;
- } else {
- $width = 50;
- $height = 50;
- }
-
- $metadata['logo:0:width'] = (string)$width;
- $metadata['logo:0:height'] = (string)$height;
- }
- return $metadata;
- }
-
  private function generateAclData(ManageEntity $entity): array
  {
  if ($entity->getAllowedIdentityProviders()->isAllowAll()) {

diff --git a/...rviceProviderDashboard/Application/Metadata/OauthClientCredentialsClientJsonGenerator.php b/...rviceProviderDashboard/Application/Metadata/OauthClientCredentialsClientJsonGenerator.php
@@ -192,7 +192,7 @@ private function generateMetadataFields(ManageEntity $entity): int|float|array
  $metadata += $this->generateOidcClient($entity);
 
  if ($entity->getMetaData()->getLogo() instanceof Logo && $entity->getMetaData()->getLogo()->getUrl() !== '') {
- $metadata += $this->generateLogoMetadata($entity);
+ $metadata['logo:0:url'] = $entity->getMetaData()->getLogo()->getUrl();
  }
 
  return $metadata;
@@ -288,37 +288,6 @@ private function generateContactMetadata(string $contactType, int $index, Contac
  return $metadata;
  }
 
- /**
- * Generate logo metadata fields.
- *
- * Logo dimensions are required in the SAML spec. They are always present,
- * except when the user just created the entity in the interface. We
- * determine the dimensions in those situations.
- *
- * @SuppressWarnings(PHPMD.ErrorControlOperator)
- */
- private function generateLogoMetadata(ManageEntity $entity): array
- {
- $logoUrl = $entity->getMetaData()->getLogo()->getUrl();
- $metadata = [
- 'logo:0:url' => $logoUrl,
- ];
-
- $logoData = @getimagesize($logoUrl);
-
- if ($logoData !== false) {
- [$width, $height] = $logoData;
- } else {
- $width = 50;
- $height = 50;
- }
-
- $metadata['logo:0:width'] = (string) $width;
- $metadata['logo:0:height'] = (string) $height;
-
- return $metadata;
- }
-
  private function generateAclData(ManageEntity $entity): array
  {
  $acl = $entity->getAllowedIdentityProviders();

diff --git a/src/Surfnet/ServiceProviderDashboard/Application/Metadata/OidcngJsonGenerator.php b/src/Surfnet/ServiceProviderDashboard/Application/Metadata/OidcngJsonGenerator.php
@@ -196,7 +196,7 @@ private function generateMetadataFields(ManageEntity $entity): int|float|array
  $metadata += $this->generateOidcClient($entity);
 
  if ($entity->getMetaData()->getLogo() instanceof Logo && $entity->getMetaData()->getLogo()->getUrl() !== '') {
- $metadata += $this->generateLogoMetadata($entity);
+ $metadata['logo:0:url'] = $entity->getMetaData()->getLogo()->getUrl();
  }
  if ($entity->getMetaData()?->getCoin()->getContractualBase() !== null) {
  $metadata['coin:contractual_base'] = $entity->getMetaData()->getCoin()->getContractualBase();
@@ -297,37 +297,6 @@ private function generateContactMetadata(string $contactType, int $index, Contac
  return $metadata;
  }
 
- /**
- * Generate logo metadata fields.
- *
- * Logo dimensions are required in the SAML spec. They are always present,
- * except when the user just created the entity in the interface. We
- * determine the dimensions in those situations.
- *
- * @SuppressWarnings(PHPMD.ErrorControlOperator)
- */
- private function generateLogoMetadata(ManageEntity $entity): array
- {
- $logoUrl = $entity->getMetaData()->getLogo()->getUrl();
- $metadata = [
- 'logo:0:url' => $logoUrl,
- ];
-
- $logoData = @getimagesize($logoUrl);
-
- if ($logoData !== false) {
- [$width, $height] = $logoData;
- } else {
- $width = 50;
- $height = 50;
- }
-
- $metadata['logo:0:width'] = (string) $width;
- $metadata['logo:0:height'] = (string) $height;
-
- return $metadata;
- }
-
  private function generateAclData(ManageEntity $entity): array
  {
  $acl = $entity->getAllowedIdentityProviders();

diff --git a/...viceProviderDashboard/Infrastructure/DashboardBundle/Service/CurlLogoValidationHelper.php b/...viceProviderDashboard/Infrastructure/DashboardBundle/Service/CurlLogoValidationHelper.php
@@ -33,12 +33,12 @@ public function __construct(private readonly LoggerInterface $logger)
  * Using Curl: tests:
  * - is the curl response code erroneous (>= 400)
  * - if the content type is correct
+ * Returns the location where the logo is stored temporarily
  *
- * @param $url
  * @throws LogoInvalidTypeException
  * @throws LogoNotFoundException
  */
- public function validateLogo($url): void
+ public function validateLogo($url): string
  {
  $this->logger->debug(sprintf('Validating logo: "%s" using curl', $url));
 
@@ -50,7 +50,9 @@ public function validateLogo($url): void
  curl_setopt($ch, CURLOPT_CONNECTTIMEOUT, 10);
  curl_setopt($ch, CURLOPT_TIMEOUT, 10);
 
- curl_exec($ch);
+ $content = curl_exec($ch);
+ $fileLocation = '/tmp/logo-validation-' . uniqid();
+ file_put_contents($fileLocation, $content);
 
  $contentType = curl_getinfo($ch, CURLINFO_CONTENT_TYPE);
  $responseCode = curl_getinfo($ch, CURLINFO_RESPONSE_CODE);
@@ -76,5 +78,7 @@ public function validateLogo($url): void
  $this->logger->info('The logo file type is invalid');
  throw new LogoInvalidTypeException('The logo file type is invalid');
  }
+
+ return $fileLocation;
  }
 }
diff --git a/...roviderDashboard/Infrastructure/DashboardBundle/Service/LogoValidationHelperInterface.php b/...roviderDashboard/Infrastructure/DashboardBundle/Service/LogoValidationHelperInterface.php
@@ -34,10 +34,10 @@ interface LogoValidationHelperInterface
 
  /**
  * Validates the logo, throws an exception if validation failed.
- *
+ * Returns the local path to the curl'ed image
  * @param $url
  * @throws LogoInvalidTypeException
  * @throws LogoNotFoundException
  */
- public function validateLogo($url);
+ public function validateLogo($url): string;
 }
diff --git a/...iderDashboard/Infrastructure/DashboardBundle/Validator/Constraints/ValidLogoValidator.php b/...iderDashboard/Infrastructure/DashboardBundle/Validator/Constraints/ValidLogoValidator.php
@@ -41,15 +41,15 @@ public function __construct(private readonly LogoValidationHelperInterface $logo
  {
  }
 
- public function validate($value, Constraint $constraint): void
+ public function validate(mixed $value, Constraint $constraint): void
  {
  if (empty($value)) {
  return;
  }
 
  try {
- $this->logoValidationHelper->validateLogo($value);
- $this->getImageSizeValidation($value, $constraint);
+ $filePath = $this->logoValidationHelper->validateLogo($value);
+ $this->getImageSizeValidation($filePath, $constraint);
  } catch (LogoNotFoundException) {
  $this->context->addViolation(self::STATUS_DOWNLOAD_FAILED);
  return;
@@ -64,7 +64,7 @@ public function validate($value, Constraint $constraint): void
  *
  * @param $value
  */
- private function getImageSizeValidation($value, Constraint $constraint): void
+ private function getImageSizeValidation(string $value, Constraint $constraint): void
  {
  try {
  $imgData = getimagesize($value);

diff --git a/...tegration/Infrastructure/DashboardBundle/Validator/Constraints/ValidLogoValidatorTest.php b/...tegration/Infrastructure/DashboardBundle/Validator/Constraints/ValidLogoValidatorTest.php
@@ -47,7 +47,7 @@ public function test_success_png()
 
  $this->validationHelper
  ->shouldReceive('validateLogo')
- ->andReturn(null);
+ ->andReturn('file://'.__DIR__.'/fixture/logo_validator/small.png');
 
  $this->validator->validate('file://'.__DIR__.'/fixture/logo_validator/small.png', $constraint);
 
@@ -60,7 +60,7 @@ public function test_success_gif()
 
  $this->validationHelper
  ->shouldReceive('validateLogo')
- ->andReturn(null);
+ ->andReturn('file://'.__DIR__.'/fixture/logo_validator/small.gif');
 
  $this->validator->validate('file://'.__DIR__.'/fixture/logo_validator/small.gif', $constraint);
 
@@ -81,7 +81,7 @@ public function test_invalid_image()
 
  $this->validationHelper
  ->shouldReceive('validateLogo')
- ->andReturn(null);
+ ->andReturn('file://'.__DIR__.'/fixture/logo_validator/ufjd');
 
  $this->validator->validate('ufjd', $constraint);