Merge pull request #5792 from soyuka/merge

Merge 3.1

src/Doctrine/Common/State/PersistProcessor.php

@@ -109,8 +109,6 @@ public function process(mixed $data, Operation $operation, array $uriVariables =
 
     /**
      * Checks if doctrine does not manage data automatically.
-     *
-     * @param mixed $data
      */
     private function isDeferredExplicit(DoctrineObjectManager $manager, $data): bool
     {

src/Doctrine/Common/State/RemoveProcessor.php

@@ -39,8 +39,6 @@ public function process(mixed $data, Operation $operation, array $uriVariables =
 
     /**
      * Gets the Doctrine object manager associated with given data.
-     *
-     * @param mixed $data
      */
     private function getManager($data): ?DoctrineObjectManager
     {

src/Doctrine/Odm/Filter/AbstractFilter.php

@@ -52,8 +52,6 @@ public function apply(Builder $aggregationBuilder, string $resourceClass, Operat
 
     /**
      * Passes a property through the filter.
-     *
-     * @param mixed $value
      */
     abstract protected function filterProperty(string $property, $value, Builder $aggregationBuilder, string $resourceClass, Operation $operation = null, array &$context = []): void;
 

src/Doctrine/Odm/Filter/DateFilter.php

@@ -194,8 +194,6 @@ protected function filterProperty(string $property, $values, Builder $aggregatio
 
     /**
      * Adds the match stage according to the chosen null management.
-     *
-     * @param mixed $value
      */
     private function addMatch(Builder $aggregationBuilder, string $field, string $operator, $value, string $nullManagement = null): void
     {

src/Doctrine/Orm/Filter/AbstractFilter.php

@@ -46,8 +46,6 @@ public function apply(QueryBuilder $queryBuilder, QueryNameGeneratorInterface $q
 
     /**
      * Passes a property through the filter.
-     *
-     * @param mixed $value
      */
     abstract protected function filterProperty(string $property, $value, QueryBuilder $queryBuilder, QueryNameGeneratorInterface $queryNameGenerator, string $resourceClass, Operation $operation = null, array $context = []): void;
 

src/Elasticsearch/Serializer/ItemNormalizer.php

@@ -111,7 +111,7 @@ public function getSupportedTypes($format): array
             ];
         }
 
-        return DocumentNormalizer::FORMAT === $format ? $this->decorated->getSupportedTypes($format) : [];
+        return DocumentNormalizer::FORMAT !== $format ? $this->decorated->getSupportedTypes($format) : [];
     }
 
     /**

src/Elasticsearch/Tests/Serializer/ItemNormalizerTest.php

@@ -166,7 +166,7 @@ public function getSupportedTypes(?string $format): array
             }
         });
 
-        $this->assertEmpty($this->itemNormalizer->getSupportedTypes('json'));
-        $this->assertSame(['*' => true], $this->itemNormalizer->getSupportedTypes($this->itemNormalizer::FORMAT));
+        $this->assertEmpty($this->itemNormalizer->getSupportedTypes($this->itemNormalizer::FORMAT));
+        $this->assertSame(['*' => true], $this->itemNormalizer->getSupportedTypes('json'));
     }
 }

src/GraphQl/ExecutorInterface.php

@@ -25,8 +25,6 @@ interface ExecutorInterface
 {
     /**
      * @see http://webonyx.github.io/graphql-php/executing-queries/#using-facade-method
-     *
-     * @param mixed $source
      */
     public function executeQuery(Schema $schema, $source, mixed $rootValue = null, mixed $context = null, array $variableValues = null, string $operationName = null, callable $fieldResolver = null, array $validationRules = null): ExecutionResult;
 }

src/GraphQl/Serializer/ItemNormalizer.php

@@ -21,6 +21,7 @@
 use ApiPlatform\Metadata\Resource\Factory\ResourceMetadataCollectionFactoryInterface;
 use ApiPlatform\Metadata\ResourceClassResolverInterface;
 use ApiPlatform\Metadata\Util\ClassInfoTrait;
+use ApiPlatform\Serializer\CacheKeyTrait;
 use ApiPlatform\Serializer\ItemNormalizer as BaseItemNormalizer;
 use ApiPlatform\Symfony\Security\ResourceAccessCheckerInterface;
 use Psr\Log\LoggerInterface;
@@ -37,12 +38,15 @@
  */
 final class ItemNormalizer extends BaseItemNormalizer
 {
+    use CacheKeyTrait;
     use ClassInfoTrait;
 
     public const FORMAT = 'graphql';
     public const ITEM_RESOURCE_CLASS_KEY = '#itemResourceClass';
     public const ITEM_IDENTIFIERS_KEY = '#itemIdentifiers';
 
+    private array $safeCacheKeysCache = [];
+
     public function __construct(PropertyNameCollectionFactoryInterface $propertyNameCollectionFactory, PropertyMetadataFactoryInterface $propertyMetadataFactory, IriConverterInterface $iriConverter, private readonly IdentifiersExtractorInterface $identifiersExtractor, ResourceClassResolverInterface $resourceClassResolver, PropertyAccessorInterface $propertyAccessor = null, NameConverterInterface $nameConverter = null, ClassMetadataFactoryInterface $classMetadataFactory = null, LoggerInterface $logger = null, ResourceMetadataCollectionFactoryInterface $resourceMetadataCollectionFactory = null, ResourceAccessCheckerInterface $resourceAccessChecker = null)
     {
         parent::__construct($propertyNameCollectionFactory, $propertyMetadataFactory, $iriConverter, $resourceClassResolver, $propertyAccessor, $nameConverter, $classMetadataFactory, $logger ?: new NullLogger(), $resourceMetadataCollectionFactory, $resourceAccessChecker);
@@ -81,7 +85,11 @@ public function normalize(mixed $object, string $format = null, array $context =
             return parent::normalize($object, $format, $context);
         }
 
-        unset($context['operation_name'], $context['operation']);
+        if ($this->isCacheKeySafe($context)) {
+            $context['cache_key'] = $this->getCacheKey($format, $context);
+        }
+
+        unset($context['operation_name'], $context['operation']); // Remove operation and operation_name only when cache key has been created
         $data = parent::normalize($object, $format, $context);
         if (!\is_array($data)) {
             throw new UnexpectedValueException('Expected data to be an array.');
@@ -140,4 +148,32 @@ protected function setAttributeValue($object, $attribute, $value, $format = null
 
         parent::setAttributeValue($object, $attribute, $value, $format, $context);
     }
+
+    /**
+     * Check if any property contains a security grants, which makes the cache key not safe,
+     * as allowed_properties can differ for 2 instances of the same object.
+     */
+    private function isCacheKeySafe(array $context): bool
+    {
+        if (!isset($context['resource_class']) || !$this->resourceClassResolver->isResourceClass($context['resource_class'])) {
+            return false;
+        }
+        $resourceClass = $this->resourceClassResolver->getResourceClass(null, $context['resource_class']);
+        if (isset($this->safeCacheKeysCache[$resourceClass])) {
+            return $this->safeCacheKeysCache[$resourceClass];
+        }
+        $options = $this->getFactoryOptions($context);
+        $propertyNames = $this->propertyNameCollectionFactory->create($resourceClass, $options);
+
+        $this->safeCacheKeysCache[$resourceClass] = true;
+        foreach ($propertyNames as $propertyName) {
+            $propertyMetadata = $this->propertyMetadataFactory->create($resourceClass, $propertyName, $options);
+            if (null !== $propertyMetadata->getSecurity()) {
+                $this->safeCacheKeysCache[$resourceClass] = false;
+                break;
+            }
+        }
+
+        return $this->safeCacheKeysCache[$resourceClass];
+    }
 }

src/GraphQl/Tests/Fixtures/ApiResource/SecuredDummy.php

@@ -0,0 +1,85 @@
+<?php
+
+/*
+ * This file is part of the API Platform project.
+ *
+ * (c) Kévin Dunglas <dunglas@gmail.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+declare(strict_types=1);
+
+namespace ApiPlatform\GraphQl\Tests\Fixtures\ApiResource;
+
+use ApiPlatform\Metadata\ApiProperty;
+use ApiPlatform\Metadata\ApiResource;
+use Symfony\Component\Validator\Constraints as Assert;
+
+/**
+ * Secured resource.
+ *
+ * @author Kévin Dunglas <dunglas@gmail.com>
+ */
+#[ApiResource]
+class SecuredDummy
+{
+    private ?int $id = null;
+
+    /**
+     * @var string The title
+     */
+    #[Assert\NotBlank]
+    private string $title;
+
+    /**
+     * @var string The description
+     */
+    private string $description = '';
+
+    /**
+     * @var string Secret property, only readable/writable by owners
+     */
+    #[ApiProperty(security: 'object == null or object.getOwner() == user', securityPostDenormalize: 'object.getOwner() == user')]
+    private string $ownerOnlyProperty = '';
+
+    public function __construct()
+    {
+    }
+
+    public function getId(): ?int
+    {
+        return $this->id;
+    }
+
+    public function getTitle(): ?string
+    {
+        return $this->title;
+    }
+
+    public function setTitle(string $title): void
+    {
+        $this->title = $title;
+    }
+
+    public function getDescription(): string
+    {
+        return $this->description;
+    }
+
+    public function setDescription(string $description): void
+    {
+        $this->description = $description;
+    }
+
+    public function getOwnerOnlyProperty(): ?string
+    {
+        return $this->ownerOnlyProperty;
+    }
+
+    public function setOwnerOnlyProperty(?string $ownerOnlyProperty): void
+    {
+        $this->ownerOnlyProperty = $ownerOnlyProperty;
+    }
+}

src/GraphQl/Tests/Serializer/ItemNormalizerTest.php

@@ -15,6 +15,7 @@
 
 use ApiPlatform\GraphQl\Serializer\ItemNormalizer;
 use ApiPlatform\GraphQl\Tests\Fixtures\ApiResource\Dummy;
+use ApiPlatform\GraphQl\Tests\Fixtures\ApiResource\SecuredDummy;
 use ApiPlatform\Metadata\ApiProperty;
 use ApiPlatform\Metadata\IdentifiersExtractorInterface;
 use ApiPlatform\Metadata\IriConverterInterface;
@@ -23,6 +24,7 @@
 use ApiPlatform\Metadata\Property\PropertyNameCollection;
 use ApiPlatform\Metadata\ResourceClassResolverInterface;
 use ApiPlatform\Metadata\UrlGeneratorInterface;
+use ApiPlatform\Symfony\Security\ResourceAccessCheckerInterface;
 use PHPUnit\Framework\TestCase;
 use Prophecy\Argument;
 use Prophecy\PhpUnit\ProphecyTrait;
@@ -115,6 +117,95 @@ public function testNormalize(): void
         ];
         $this->assertEquals($expected, $normalizer->normalize($dummy, ItemNormalizer::FORMAT, [
             'resources' => [],
+            'resource_class' => Dummy::class,
+        ]));
+    }
+
+    public function testNormalizeWithUnsafeCacheProperty(): void
+    {
+        $securedDummyWithOwnerOnlyPropertyAllowed = new SecuredDummy();
+        $securedDummyWithOwnerOnlyPropertyAllowed->setTitle('hello');
+        $securedDummyWithOwnerOnlyPropertyAllowed->setOwnerOnlyProperty('ownerOnly');
+        $securedDummyWithoutOwnerOnlyPropertyAllowed = clone $securedDummyWithOwnerOnlyPropertyAllowed;
+        $securedDummyWithoutOwnerOnlyPropertyAllowed->setTitle('hello from secured dummy');
+
+        $propertyNameCollection = new PropertyNameCollection(['title', 'ownerOnlyProperty']);
+        $propertyNameCollectionFactoryProphecy = $this->prophesize(PropertyNameCollectionFactoryInterface::class);
+        $propertyNameCollectionFactoryProphecy->create(SecuredDummy::class, [])->willReturn($propertyNameCollection);
+
+        $unsecuredPropertyMetadata = (new ApiProperty())->withReadable(true);
+        $securedPropertyMetadata = (new ApiProperty())->withReadable(true)->withSecurity('object == null or object.getOwner() == user');
+        $propertyMetadataFactoryProphecy = $this->prophesize(PropertyMetadataFactoryInterface::class);
+        $propertyMetadataFactoryProphecy->create(SecuredDummy::class, 'title', [])->willReturn($unsecuredPropertyMetadata);
+        $propertyMetadataFactoryProphecy->create(SecuredDummy::class, 'ownerOnlyProperty', [])->willReturn($securedPropertyMetadata);
+
+        $iriConverterProphecy = $this->prophesize(IriConverterInterface::class);
+        $iriConverterProphecy->getIriFromResource($securedDummyWithOwnerOnlyPropertyAllowed, UrlGeneratorInterface::ABS_URL, Argument::any(), Argument::type('array'))->willReturn('/dummies/1');
+        $iriConverterProphecy->getIriFromResource($securedDummyWithoutOwnerOnlyPropertyAllowed, UrlGeneratorInterface::ABS_URL, Argument::any(), Argument::type('array'))->willReturn('/dummies/2');
+
+        $identifiersExtractorProphecy = $this->prophesize(IdentifiersExtractorInterface::class);
+        $identifiersExtractorProphecy->getIdentifiersFromItem($securedDummyWithOwnerOnlyPropertyAllowed, Argument::any())->willReturn(['id' => 1])->shouldBeCalled();
+        $identifiersExtractorProphecy->getIdentifiersFromItem($securedDummyWithoutOwnerOnlyPropertyAllowed, Argument::any())->willReturn(['id' => 2])->shouldBeCalled();
+
+        $resourceClassResolverProphecy = $this->prophesize(ResourceClassResolverInterface::class);
+        $resourceClassResolverProphecy->getResourceClass($securedDummyWithOwnerOnlyPropertyAllowed, null)->willReturn(SecuredDummy::class);
+        $resourceClassResolverProphecy->getResourceClass($securedDummyWithoutOwnerOnlyPropertyAllowed, null)->willReturn(SecuredDummy::class);
+        $resourceClassResolverProphecy->getResourceClass(null, SecuredDummy::class)->willReturn(SecuredDummy::class);
+        $resourceClassResolverProphecy->isResourceClass(SecuredDummy::class)->willReturn(true);
+
+        $serializerProphecy = $this->prophesize(SerializerInterface::class);
+        $serializerProphecy->willImplement(NormalizerInterface::class);
+        $serializerProphecy->normalize('hello', ItemNormalizer::FORMAT, Argument::type('array'))->willReturn('hello');
+        $serializerProphecy->normalize('hello from secured dummy', ItemNormalizer::FORMAT, Argument::type('array'))->willReturn('hello from secured dummy');
+        $serializerProphecy->normalize('ownerOnly', ItemNormalizer::FORMAT, Argument::type('array'))->willReturn('ownerOnly');
+
+        $resourceAccessCheckerProphecy = $this->prophesize(ResourceAccessCheckerInterface::class);
+        $resourceAccessCheckerProphecy->isGranted(
+            SecuredDummy::class,
+            'object == null or object.getOwner() == user',
+            Argument::type('array')
+        )->will(function (array $args) {
+            return 'hello' === $args[2]['object']->getTitle(); // Allow access only for securedDummyWithOwnerOnlyPropertyAllowed
+        });
+
+        $normalizer = new ItemNormalizer(
+            $propertyNameCollectionFactoryProphecy->reveal(),
+            $propertyMetadataFactoryProphecy->reveal(),
+            $iriConverterProphecy->reveal(),
+            $identifiersExtractorProphecy->reveal(),
+            $resourceClassResolverProphecy->reveal(),
+            null,
+            null,
+            null,
+            null,
+            null,
+            $resourceAccessCheckerProphecy->reveal()
+        );
+        $normalizer->setSerializer($serializerProphecy->reveal());
+
+        $expected = [
+            'title' => 'hello',
+            'ownerOnlyProperty' => 'ownerOnly',
+            ItemNormalizer::ITEM_RESOURCE_CLASS_KEY => SecuredDummy::class,
+            ItemNormalizer::ITEM_IDENTIFIERS_KEY => [
+                'id' => 1,
+            ],
+        ];
+        $this->assertEquals($expected, $normalizer->normalize($securedDummyWithOwnerOnlyPropertyAllowed, ItemNormalizer::FORMAT, [
+            'resources' => [],
+            'resource_class' => SecuredDummy::class,
+        ]));
+
+        $expected = [
+            'title' => 'hello from secured dummy',
+            ItemNormalizer::ITEM_RESOURCE_CLASS_KEY => SecuredDummy::class,
+            ItemNormalizer::ITEM_IDENTIFIERS_KEY => [
+                'id' => 2,
+            ],
+        ];
+        $this->assertEquals($expected, $normalizer->normalize($securedDummyWithoutOwnerOnlyPropertyAllowed, ItemNormalizer::FORMAT, [
+            'resources' => [],
+            'resource_class' => SecuredDummy::class,
         ]));
     }
 

src/Metadata/Tests/Property/Factory/SerializerPropertyMetadataFactoryTest.php

@@ -42,9 +42,6 @@ public static function groupsProvider(): array
 
     /**
      * @dataProvider groupsProvider
-     *
-     * @param mixed $readGroups
-     * @param mixed $writeGroups
      */
     public function testCreate($readGroups, $writeGroups): void
     {

src/Metadata/Tests/Property/PropertyInfoPropertyNameCollectionFactoryTest.php

@@ -93,11 +93,11 @@ public function testCreateMethodReturnsProperPropertyNameCollectionForObjectWith
         $collection = $factory->create(DummyIgnoreProperty::class, ['serializer_groups' => ['dummy']]);
 
         self::assertCount(1, $collection);
-        self::assertNotContains('ignored', $collection);
+        self::assertNotContains('ignored', (array) $collection);
 
         $collection = $factory->create(DummyIgnoreProperty::class);
 
         self::assertCount(2, $collection);
-        self::assertNotContains('ignored', $collection);
+        self::assertNotContains('ignored', (array) $collection);
     }
 }