From 1eb6d31fc4a7cce21678e23b259207c5bd0d850f Mon Sep 17 00:00:00 2001 From: Jeremiah VALERIE Date: Mon, 28 Mar 2016 11:51:19 +0200 Subject: [PATCH 1/7] init QueryComplexity --- .../Validator/Rule/AbstractQuerySecurity.php | 105 +++++++++++++++ Request/Validator/Rule/MaxQueryComplexity.php | 62 +++++++++ Request/Validator/Rule/MaxQueryDepth.php | 127 +++++++----------- .../Validator/Rule/MaxQueryDepthTest.php | 6 +- 4 files changed, 222 insertions(+), 78 deletions(-) create mode 100644 Request/Validator/Rule/AbstractQuerySecurity.php create mode 100644 Request/Validator/Rule/MaxQueryComplexity.php diff --git a/Request/Validator/Rule/AbstractQuerySecurity.php b/Request/Validator/Rule/AbstractQuerySecurity.php new file mode 100644 index 000000000..db8d23ff8 --- /dev/null +++ b/Request/Validator/Rule/AbstractQuerySecurity.php @@ -0,0 +1,105 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Overblog\GraphQLBundle\Request\Validator\Rule; + +use GraphQL\Language\AST\FragmentDefinition; +use GraphQL\Language\AST\Node; +use GraphQL\Type\Definition\Type; +use GraphQL\Type\Definition\WrappingType; +use GraphQL\Validator\ValidationContext; + +class AbstractQuerySecurity +{ + /** @var FragmentDefinition[] */ + private $fragments = []; + + /** @var Type[] */ + private $rootTypes = []; + + /** + * @return \GraphQL\Language\AST\FragmentDefinition[] + */ + protected function getFragments() + { + return $this->fragments; + } + + /** + * @return \GraphQL\Type\Definition\Type[] + */ + protected function getRootTypes() + { + return $this->rootTypes; + } + + /** + * check if equal to 0 no check is done. Must be greater or equal to 0. + * + * @param $value + */ + protected static function checkIfGreaterOrEqualToZero($name, $value) + { + if ($value < 0) { + throw new \InvalidArgumentException(sprintf('$%s argument must be greater or equal to 0.', $name)); + } + } + + protected function isRootType(ValidationContext $context) + { + $parentType = $context->getParentType(); + $isParentRootType = $parentType && in_array($parentType, $this->getRootTypes()); + + return $isParentRootType; + } + + protected function isIntrospectionType(ValidationContext $context) + { + $type = $this->retrieveCurrentType($context); + $isIntrospectionType = $type && $type->name === '__Schema'; + + return $isIntrospectionType; + } + + protected function gatherRootTypes(ValidationContext $context) + { + $schema = $context->getSchema(); + $this->rootTypes = [$schema->getQueryType(), $schema->getMutationType(), $schema->getSubscriptionType()]; + } + + protected function gatherFragmentDefinition(ValidationContext $context) + { + // Gather all the fragment definition. + // Importantly this does not include inline fragments. + $definitions = $context->getDocument()->definitions; + foreach ($definitions as $node) { + if ($node instanceof FragmentDefinition) { + $this->fragments[$node->name->value] = $node; + } + } + } + + protected function retrieveCurrentType(ValidationContext $context) + { + $type = $context->getType(); + + if ($type instanceof WrappingType) { + $type = $type->getWrappedType(true); + } + + return $type; + } + + protected function getNodeType($node) + { + return $node instanceof Node ? $node->kind : null; + } +} diff --git a/Request/Validator/Rule/MaxQueryComplexity.php b/Request/Validator/Rule/MaxQueryComplexity.php new file mode 100644 index 000000000..77a15fbab --- /dev/null +++ b/Request/Validator/Rule/MaxQueryComplexity.php @@ -0,0 +1,62 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Overblog\GraphQLBundle\Request\Validator\Rule; + +use GraphQL\Language\AST\Field; +use GraphQL\Language\AST\Node; +use GraphQL\Validator\ValidationContext; + +class MaxQueryComplexity extends AbstractQuerySecurity +{ + const DEFAULT_QUERY_MAX_COMPLEXITY = 100; + + private static $maxQueryComplexity; + + public function __construct($maxQueryDepth = self::DEFAULT_QUERY_MAX_COMPLEXITY) + { + $this->setMaxQueryComplexity($maxQueryDepth); + } + + /** + * Set max query complexity. If equal to 0 no check is done. Must be greater or equal to 0. + * + * @param $maxQueryComplexity + */ + public static function setMaxQueryComplexity($maxQueryComplexity) + { + self::checkIfGreaterOrEqualToZero('maxQueryComplexity', $maxQueryComplexity); + + self::$maxQueryComplexity = (int) $maxQueryComplexity; + } + + public static function getMaxQueryComplexity() + { + return self::$maxQueryComplexity; + } + + public function __invoke(ValidationContext $context) + { + $this->gatherFragmentDefinition($context); + $this->gatherRootTypes($context); + + // is disabled? + if (0 === $this->getMaxQueryComplexity()) { + return []; + } + + return [ + Node::FIELD => function (Field $node) use ($context) { + + }, + ]; + } +} diff --git a/Request/Validator/Rule/MaxQueryDepth.php b/Request/Validator/Rule/MaxQueryDepth.php index 4ee7a4b97..081c3a768 100644 --- a/Request/Validator/Rule/MaxQueryDepth.php +++ b/Request/Validator/Rule/MaxQueryDepth.php @@ -13,23 +13,21 @@ use GraphQL\Error; use GraphQL\Language\AST\Field; -use GraphQL\Language\AST\FragmentDefinition; use GraphQL\Language\AST\FragmentSpread; use GraphQL\Language\AST\InlineFragment; use GraphQL\Language\AST\Node; use GraphQL\Language\AST\SelectionSet; -use GraphQL\Type\Definition\WrappingType; use GraphQL\Validator\ValidationContext; -class MaxQueryDepth +class MaxQueryDepth extends AbstractQuerySecurity { const DEFAULT_QUERY_MAX_DEPTH = 100; - const DEFAULT_MAX_COUNT_AFTER_DEPTH_LIMIT = 50; + /** + * @var int + */ private static $maxQueryDepth; - private $fragments = []; - public function __construct($maxQueryDepth = self::DEFAULT_QUERY_MAX_DEPTH) { $this->setMaxQueryDepth($maxQueryDepth); @@ -42,110 +40,89 @@ public function __construct($maxQueryDepth = self::DEFAULT_QUERY_MAX_DEPTH) */ public static function setMaxQueryDepth($maxQueryDepth) { - if ($maxQueryDepth < 0) { - throw new \InvalidArgumentException('$maxQueryDepth argument must be greater or equal to 0. '); - } + self::checkIfGreaterOrEqualToZero('maxQueryDepth', $maxQueryDepth); self::$maxQueryDepth = (int) $maxQueryDepth; } + public static function getMaxQueryDepth() + { + return self::$maxQueryDepth; + } + public static function maxQueryDepthErrorMessage($max, $count) { - return sprintf('Max query depth should be %d but is greater or equal to %d.', $max, $count); + return sprintf('Max query depth should be %d but got %d.', $max, $count); } public function __invoke(ValidationContext $context) { - // Gather all the fragment definition. - // Importantly this does not include inline fragments. - $definitions = $context->getDocument()->definitions; - foreach ($definitions as $node) { - if ($node instanceof FragmentDefinition) { - $this->fragments[$node->name->value] = $node; - } + $this->gatherFragmentDefinition($context); + $this->gatherRootTypes($context); + + // is disabled? + if (0 === $this->getMaxQueryDepth()) { + return []; } - $schema = $context->getSchema(); - $rootTypes = [$schema->getQueryType(), $schema->getMutationType(), $schema->getSubscriptionType()]; - return 0 !== self::$maxQueryDepth ? [Node::FIELD => $this->getFieldClosure($context, $rootTypes)] : []; - } + return [ + Node::FIELD => function (Field $node) use ($context) { + // check depth only on first rootTypes children and ignore check on introspection query + if ($this->isRootType($context) && !$this->isIntrospectionType($context)) { + $depth = $node->selectionSet ? $this->countNodeDepth($node, 0, true) : 0; - private function getFieldClosure(ValidationContext $context, array $rootTypes) - { - return function (Field $node) use ($context, $rootTypes) { - $parentType = $context->getParentType(); - $type = $this->retrieveCurrentTypeFromValidationContext($context); - $isIntrospectionType = $type && $type->name === '__Schema'; - $isParentRootType = $parentType && in_array($parentType, $rootTypes); - - // check depth only on first rootTypes children and ignore check on introspection query - if ($isParentRootType && !$isIntrospectionType) { - $depth = $node->selectionSet ? - $this->countSelectionDepth( - $node->selectionSet, - self::$maxQueryDepth + static::DEFAULT_MAX_COUNT_AFTER_DEPTH_LIMIT, - 0, - true - ) : - 0 - ; - - if ($depth > self::$maxQueryDepth) { - return new Error(static::maxQueryDepthErrorMessage(self::$maxQueryDepth, $depth), [$node]); + if ($depth > $this->getMaxQueryDepth()) { + return new Error($this->maxQueryDepthErrorMessage($this->getMaxQueryDepth(), $depth), [$node]); + } } - } - }; + }, + ]; } - private function retrieveCurrentTypeFromValidationContext(ValidationContext $context) + protected function countNodeDepth(Node $parentNode, $depth = 0, $resetDepthForEachSelection = false) { - $type = $context->getType(); - - if ($type instanceof WrappingType) { - $type = $type->getWrappedType(true); + if (!isset($parentNode->selectionSet)) { + return $depth; } - return $type; - } + foreach ($parentNode->selectionSet->selections as $node) { + $depth = $resetDepthForEachSelection ? 0 : $depth; - private function countSelectionDepth(SelectionSet $selectionSet, $stopCountingAt, $depth = 0, $resetDepthForEachSelection = false) - { - foreach ($selectionSet->selections as $selectionAST) { - if ($depth >= $stopCountingAt) { - break; + $type = $this->getNodeType($node); + if (null === $type) { + continue; } - $depth = $resetDepthForEachSelection ? 0 : $depth; + $method = 'count'.$type.'Depth'; - if ($selectionAST instanceof Field) { - $depth = $this->countFieldDepth($selectionAST->selectionSet, $stopCountingAt, $depth); - } elseif ($selectionAST instanceof FragmentSpread) { - $depth = $this->countFragmentDepth($selectionAST, $stopCountingAt, $depth); - } elseif ($selectionAST instanceof InlineFragment) { - $depth = $this->countInlineFragmentDepth($selectionAST->selectionSet, $stopCountingAt, $depth); - } + $depth = $this->$method($node, $depth); } return $depth; } - private function countFieldDepth(SelectionSet $selectionSet = null, $stopCountingAt, $depth) + private function countFieldDepth(Field $field, $depth) { - return null === $selectionSet ? $depth : $this->countSelectionDepth($selectionSet, $stopCountingAt, ++$depth); + $selectionSet = $field->selectionSet; + + return null === $selectionSet ? $depth : $this->countNodeDepth($field, ++$depth); } - private function countInlineFragmentDepth(SelectionSet $selectionSet = null, $stopCountingAt, $depth) + private function countInlineFragmentDepth(InlineFragment $inlineFragment, $depth) { - return null === $selectionSet ? $depth : $this->countSelectionDepth($selectionSet, $stopCountingAt, $depth); + $selectionSet = $inlineFragment->selectionSet; + + return null === $selectionSet ? $depth : $this->countNodeDepth($inlineFragment, $depth); } - private function countFragmentDepth(FragmentSpread $selectionAST, $stopCountingAt, $depth) + private function countFragmentSpreadDepth(FragmentSpread $fragmentSpread, $depth) { - $spreadName = $selectionAST->name->value; - if (isset($this->fragments[$spreadName])) { - /** @var FragmentDefinition $fragment */ - $fragment = $this->fragments[$spreadName]; - $depth = $this->countSelectionDepth($fragment->selectionSet, $stopCountingAt, $depth); + $spreadName = $fragmentSpread->name->value; + $fragments = $this->getFragments(); + + if (isset($fragments[$spreadName])) { + $fragment = $fragments[$spreadName]; + $depth = $this->countNodeDepth($fragment, $depth); } return $depth; diff --git a/Tests/Request/Validator/Rule/MaxQueryDepthTest.php b/Tests/Request/Validator/Rule/MaxQueryDepthTest.php index e660307bd..806fa9476 100644 --- a/Tests/Request/Validator/Rule/MaxQueryDepthTest.php +++ b/Tests/Request/Validator/Rule/MaxQueryDepthTest.php @@ -86,9 +86,9 @@ public function queryDataProvider() ], // failed because depth over limit (8) [ 60, - 8, - [$this->createFormattedError(8, 58)], - ], // failed because depth over limit (8) and stop count at 58 + 20, + [$this->createFormattedError(20, 60)], + ], // failed because depth over limit (20) ]; } From 7551ce8a526977897d49e75d84762663f594cc83 Mon Sep 17 00:00:00 2001 From: Jeremiah VALERIE Date: Tue, 29 Mar 2016 00:27:37 +0200 Subject: [PATCH 2/7] Init custom complexity calculator --- Request/Executor.php | 4 + .../Validator/Rule/AbstractQuerySecurity.php | 30 +- Request/Validator/Rule/MaxQueryComplexity.php | 270 +++++++++++++++++- Request/Validator/Rule/MaxQueryDepth.php | 104 +++---- Resources/config/services.yml | 4 + 5 files changed, 339 insertions(+), 73 deletions(-) diff --git a/Request/Executor.php b/Request/Executor.php index c520a0002..3a64dd2d8 100644 --- a/Request/Executor.php +++ b/Request/Executor.php @@ -22,6 +22,7 @@ use Overblog\GraphQLBundle\Error\ErrorHandler; use Overblog\GraphQLBundle\Event\Events; use Overblog\GraphQLBundle\Event\ExecutorContextEvent; +use Overblog\GraphQLBundle\Request\Validator\Rule\MaxQueryComplexity; use Symfony\Component\EventDispatcher\EventDispatcherInterface; class Executor @@ -96,6 +97,9 @@ private function executeAndReturnResult(Schema $schema, $requestString, $rootVal try { $source = new Source($requestString ?: '', 'GraphQL request'); $documentAST = GraphQLParser::parse($source); + //todo set using service + MaxQueryComplexity::setRawVariableValues($variableValues); + $validationErrors = DocumentValidator::validate($schema, $documentAST, $this->validationRules); if (!empty($validationErrors)) { diff --git a/Request/Validator/Rule/AbstractQuerySecurity.php b/Request/Validator/Rule/AbstractQuerySecurity.php index db8d23ff8..1a792c402 100644 --- a/Request/Validator/Rule/AbstractQuerySecurity.php +++ b/Request/Validator/Rule/AbstractQuerySecurity.php @@ -12,12 +12,12 @@ namespace Overblog\GraphQLBundle\Request\Validator\Rule; use GraphQL\Language\AST\FragmentDefinition; -use GraphQL\Language\AST\Node; +use GraphQL\Language\AST\FragmentSpread; use GraphQL\Type\Definition\Type; use GraphQL\Type\Definition\WrappingType; use GraphQL\Validator\ValidationContext; -class AbstractQuerySecurity +abstract class AbstractQuerySecurity { /** @var FragmentDefinition[] */ private $fragments = []; @@ -25,6 +25,8 @@ class AbstractQuerySecurity /** @var Type[] */ private $rootTypes = []; + private $initialValue; + /** * @return \GraphQL\Language\AST\FragmentDefinition[] */ @@ -53,7 +55,7 @@ protected static function checkIfGreaterOrEqualToZero($name, $value) } } - protected function isRootType(ValidationContext $context) + protected function isParentRootType(ValidationContext $context) { $parentType = $context->getParentType(); $isParentRootType = $parentType && in_array($parentType, $this->getRootTypes()); @@ -98,8 +100,26 @@ protected function retrieveCurrentType(ValidationContext $context) return $type; } - protected function getNodeType($node) + protected function getFragment(FragmentSpread $fragmentSpread) { - return $node instanceof Node ? $node->kind : null; + $spreadName = $fragmentSpread->name->value; + $fragments = $this->getFragments(); + + return isset($fragments[$spreadName]) ? $fragments[$spreadName] : null; } + + protected function invokeIfNeeded(ValidationContext $context, array $validators) + { + $this->gatherFragmentDefinition($context); + $this->gatherRootTypes($context); + + // is disabled? + if (!$this->isEnabled()) { + return []; + } + + return $validators; + } + + abstract protected function isEnabled(); } diff --git a/Request/Validator/Rule/MaxQueryComplexity.php b/Request/Validator/Rule/MaxQueryComplexity.php index 77a15fbab..d235fdd32 100644 --- a/Request/Validator/Rule/MaxQueryComplexity.php +++ b/Request/Validator/Rule/MaxQueryComplexity.php @@ -11,19 +11,50 @@ namespace Overblog\GraphQLBundle\Request\Validator\Rule; +use GraphQL\Error; +use GraphQL\Executor\Values; use GraphQL\Language\AST\Field; +use GraphQL\Language\AST\FragmentSpread; +use GraphQL\Language\AST\InlineFragment; use GraphQL\Language\AST\Node; +use GraphQL\Language\AST\SelectionSet; +use GraphQL\Language\AST\Type; +use GraphQL\Language\Visitor; +use GraphQL\Type\Definition\FieldDefinition; +use GraphQL\Utils\TypeInfo; use GraphQL\Validator\ValidationContext; class MaxQueryComplexity extends AbstractQuerySecurity { - const DEFAULT_QUERY_MAX_COMPLEXITY = 100; + const DEFAULT_QUERY_MAX_COMPLEXITY = 1000; private static $maxQueryComplexity; + private static $rawVariableValues = []; + + private static $complexityMap = []; + + private $variableDefs; + + private $fieldAstAndDefs; + + /** + * @var ValidationContext + */ + private $context; + public function __construct($maxQueryDepth = self::DEFAULT_QUERY_MAX_COMPLEXITY) { $this->setMaxQueryComplexity($maxQueryDepth); + //todo delete after test +// self::$complexityMap['User'] = function (ValidationContext $context, $args, $childrenComplexity) { +// return 25 + $args['id'] * $childrenComplexity; +// }; + } + + public static function maxQueryComplexityErrorMessage($max, $count) + { + return sprintf('Max query complexity should be %d but got %d.', $max, $count); } /** @@ -43,20 +74,239 @@ public static function getMaxQueryComplexity() return self::$maxQueryComplexity; } + public static function setRawVariableValues(array $rawVariableValues = null) + { + self::$rawVariableValues = $rawVariableValues ?: []; + } + + public static function getRawVariableValues() + { + return self::$rawVariableValues; + } + public function __invoke(ValidationContext $context) { - $this->gatherFragmentDefinition($context); - $this->gatherRootTypes($context); + $this->context = $context; + + $this->variableDefs = new \ArrayObject(); + $this->fieldAstAndDefs = new \ArrayObject(); + $complexity = 0; + + return $this->invokeIfNeeded( + $context, + [ + // Visit FragmentDefinition after visiting FragmentSpread + 'visitSpreadFragments' => true, + Node::SELECTION_SET => function (SelectionSet $selectionSet) use ($context) { + $this->fieldAstAndDefs = $this->collectFieldASTsAndDefs( + $context, + $context->getParentType(), + $selectionSet, + null, + $this->fieldAstAndDefs + ); + }, + Node::VARIABLE_DEFINITION => function ($def) { + $this->variableDefs[] = $def; + + return Visitor::skipNode(); + }, + Node::FIELD => [ + 'leave' => function (Field $node) use ($context, $complexity) { + // check complexity only on first rootTypes children and ignore check on introspection query + if ($this->isParentRootType($context) && !$this->isIntrospectionType($context)) { + $complexity = $this->nodeComplexity($node, $complexity); + + if ($complexity > $this->getMaxQueryComplexity()) { + return new Error($this->maxQueryComplexityErrorMessage($this->getMaxQueryComplexity(), $complexity), [$node]); + } + } + }, + ], + ] + ); + } + + private function fieldComplexity(Node $node, $complexity = 0) + { + if (!isset($node->selectionSet)) { + return $complexity; + } + + foreach ($node->selectionSet->selections as $childNode) { + $complexity = $this->nodeComplexity($childNode, $complexity); + } + + return $complexity; + } + + private function nodeComplexity(Node $node, $complexity = 0) + { + switch ($node->kind) { + case Node::FIELD: + // calculate children complexity if needed + $childrenComplexity = 0; + + // node has children? + if (isset($node->selectionSet)) { + $childrenComplexity = $this->fieldComplexity($node); + } + + $fieldDef = $this->astFieldToFieldDef($node); + + $complexityCalculator = null; + + // custom complexity is set ? + if ($fieldDef && isset(self::$complexityMap[$fieldDef->getType()->name])) { + $args = $this->buildFieldArguments($node); + $typeName = $fieldDef->getType()->name; + + $complexity = call_user_func_array(self::$complexityMap[$typeName], [$this->context, $args, $childrenComplexity]); + } else { + // default complexity calculator + $complexity = $complexity + $childrenComplexity + 1; + } + break; + + case Node::INLINE_FRAGMENT: + // node has children? + if (isset($node->selectionSet)) { + $complexity = $this->fieldComplexity($node, $complexity); + } + break; + + case Node::FRAGMENT_SPREAD: + $fragment = $this->getFragment($node); + + if (null !== $fragment) { + $complexity = $this->fieldComplexity($fragment, $complexity); + } + break; + } + + return $complexity; + } + + private function getFieldName(Field $node) + { + $fieldName = $node->name->value; + $responseName = $node->alias ? $node->alias->value : $fieldName; + + return $responseName; + } - // is disabled? - if (0 === $this->getMaxQueryComplexity()) { - return []; + private function astFieldToFieldDef(Field $field) + { + $fieldName = $this->getFieldName($field); + $fieldDef = null; + if (isset($this->fieldAstAndDefs[$fieldName])) { + foreach ($this->fieldAstAndDefs[$fieldName] as $astAndDef) { + if ($astAndDef[0] == $field) { + /** @var FieldDefinition $fieldDef */ + $fieldDef = $astAndDef[1]; + break; + } + } } - return [ - Node::FIELD => function (Field $node) use ($context) { + return $fieldDef; + } + + private function buildFieldArguments(Field $node) + { + $rawVariableValues = $this->getRawVariableValues(); + $fieldDef = $this->astFieldToFieldDef($node); + if (null === $fieldDef) { + return; + } + + $variableValues = Values::getVariableValues( + $this->context->getSchema(), + $this->variableDefs, + $rawVariableValues + ); + $args = Values::getArgumentValues($fieldDef->args, $node->arguments, $variableValues); + + return $args; + } + + protected function isEnabled() + { + return $this->getMaxQueryComplexity() > 0; + } + + /** + * Given a selectionSet, adds all of the fields in that selection to + * the passed in map of fields, and returns it at the end. + * + * Note: This is not the same as execution's collectFields because at static + * time we do not know what object type will be used, so we unconditionally + * spread in all fragments. + * + * @see GraphQL\Validator\Rules\OverlappingFieldsCanBeMerged + * + * @param ValidationContext $context + * @param Type|null $parentType + * @param SelectionSet $selectionSet + * @param \ArrayObject $visitedFragmentNames + * @param \ArrayObject $astAndDefs + * + * @return \ArrayObject + */ + private function collectFieldASTsAndDefs(ValidationContext $context, $parentType, SelectionSet $selectionSet, \ArrayObject $visitedFragmentNames = null, \ArrayObject $astAndDefs = null) + { + $_visitedFragmentNames = $visitedFragmentNames ?: new \ArrayObject(); + $_astAndDefs = $astAndDefs ?: new \ArrayObject(); + + foreach ($selectionSet->selections as $selection) { + switch ($selection->kind) { + case Node::FIELD: + $fieldName = $selection->name->value; + $fieldDef = null; + if ($parentType && method_exists($parentType, 'getFields')) { + $tmp = $parentType->getFields(); + if (isset($tmp[$fieldName])) { + $fieldDef = $tmp[$fieldName]; + } + } + $responseName = $this->getFieldName($selection); + if (!isset($_astAndDefs[$responseName])) { + $_astAndDefs[$responseName] = new \ArrayObject(); + } + $_astAndDefs[$responseName][] = [$selection, $fieldDef]; + break; + case Node::INLINE_FRAGMENT: + /* @var InlineFragment $inlineFragment */ + $_astAndDefs = $this->collectFieldASTsAndDefs( + $context, + TypeInfo::typeFromAST($context->getSchema(), $selection->typeCondition), + $selection->selectionSet, + $_visitedFragmentNames, + $_astAndDefs + ); + break; + case Node::FRAGMENT_SPREAD: + /* @var FragmentSpread $selection */ + $fragName = $selection->name->value; + if (!empty($_visitedFragmentNames[$fragName])) { + continue; + } + $_visitedFragmentNames[$fragName] = true; + $fragment = $context->getFragment($fragName); + if (!$fragment) { + continue; + } + $_astAndDefs = $this->collectFieldASTsAndDefs( + $context, + TypeInfo::typeFromAST($context->getSchema(), $fragment->typeCondition), + $fragment->selectionSet, + $_visitedFragmentNames, + $_astAndDefs + ); + break; + } + } - }, - ]; + return $_astAndDefs; } } diff --git a/Request/Validator/Rule/MaxQueryDepth.php b/Request/Validator/Rule/MaxQueryDepth.php index 081c3a768..dfe14f31a 100644 --- a/Request/Validator/Rule/MaxQueryDepth.php +++ b/Request/Validator/Rule/MaxQueryDepth.php @@ -13,8 +13,6 @@ use GraphQL\Error; use GraphQL\Language\AST\Field; -use GraphQL\Language\AST\FragmentSpread; -use GraphQL\Language\AST\InlineFragment; use GraphQL\Language\AST\Node; use GraphQL\Language\AST\SelectionSet; use GraphQL\Validator\ValidationContext; @@ -57,74 +55,64 @@ public static function maxQueryDepthErrorMessage($max, $count) public function __invoke(ValidationContext $context) { - $this->gatherFragmentDefinition($context); - $this->gatherRootTypes($context); - - // is disabled? - if (0 === $this->getMaxQueryDepth()) { - return []; - } - - return [ - Node::FIELD => function (Field $node) use ($context) { - // check depth only on first rootTypes children and ignore check on introspection query - if ($this->isRootType($context) && !$this->isIntrospectionType($context)) { - $depth = $node->selectionSet ? $this->countNodeDepth($node, 0, true) : 0; - - if ($depth > $this->getMaxQueryDepth()) { - return new Error($this->maxQueryDepthErrorMessage($this->getMaxQueryDepth(), $depth), [$node]); + return $this->invokeIfNeeded( + $context, + [ + Node::FIELD => function (Field $node) use ($context) { + // check depth only on first rootTypes children and ignore check on introspection query + if ($this->isParentRootType($context) && !$this->isIntrospectionType($context)) { + $maxDepth = $this->nodeMaxDepth($node); + + if ($maxDepth > $this->getMaxQueryDepth()) { + return new Error($this->maxQueryDepthErrorMessage($this->getMaxQueryDepth(), $maxDepth), [$node]); + } } - } - }, - ]; + }, + ] + ); } - protected function countNodeDepth(Node $parentNode, $depth = 0, $resetDepthForEachSelection = false) + protected function isEnabled() { - if (!isset($parentNode->selectionSet)) { - return $depth; - } - - foreach ($parentNode->selectionSet->selections as $node) { - $depth = $resetDepthForEachSelection ? 0 : $depth; - - $type = $this->getNodeType($node); - if (null === $type) { - continue; - } - - $method = 'count'.$type.'Depth'; - - $depth = $this->$method($node, $depth); - } - - return $depth; + return $this->getMaxQueryDepth() > 0; } - private function countFieldDepth(Field $field, $depth) + private function nodeMaxDepth(Node $node, $depth = 1, $maxDepth = 1) { - $selectionSet = $field->selectionSet; - - return null === $selectionSet ? $depth : $this->countNodeDepth($field, ++$depth); - } + if (!isset($node->selectionSet)) { + return $maxDepth; + } - private function countInlineFragmentDepth(InlineFragment $inlineFragment, $depth) - { - $selectionSet = $inlineFragment->selectionSet; + foreach ($node->selectionSet->selections as $childNode) { + switch ($childNode->kind) { + case Node::FIELD: + // node has children? + if (null !== $childNode->selectionSet) { + // update maxDepth if needed + if ($depth > $maxDepth) { + $maxDepth = $depth; + } + $maxDepth = $this->nodeMaxDepth($childNode, $depth + 1, $maxDepth); + } + break; - return null === $selectionSet ? $depth : $this->countNodeDepth($inlineFragment, $depth); - } + case Node::INLINE_FRAGMENT: + // node has children? + if (null !== $childNode->selectionSet) { + $maxDepth = $this->nodeMaxDepth($childNode, $depth, $maxDepth); + } + break; - private function countFragmentSpreadDepth(FragmentSpread $fragmentSpread, $depth) - { - $spreadName = $fragmentSpread->name->value; - $fragments = $this->getFragments(); + case Node::FRAGMENT_SPREAD: + $fragment = $this->getFragment($childNode); - if (isset($fragments[$spreadName])) { - $fragment = $fragments[$spreadName]; - $depth = $this->countNodeDepth($fragment, $depth); + if (null !== $fragment) { + $maxDepth = $this->nodeMaxDepth($fragment, $depth, $maxDepth); + } + break; + } } - return $depth; + return $maxDepth; } } diff --git a/Resources/config/services.yml b/Resources/config/services.yml index 223b01005..469694c70 100644 --- a/Resources/config/services.yml +++ b/Resources/config/services.yml @@ -38,10 +38,14 @@ services: - "@overblog_graphql.error_handler" calls: - ["addValidatorRule", ["@overblog_graphql.request_validator_rule_max_query_depth"]] + - ["addValidatorRule", ["@overblog_graphql.request_validator_rule_max_query_complexity"]] overblog_graphql.request_validator_rule_max_query_depth: class: Overblog\GraphQLBundle\Request\Validator\Rule\MaxQueryDepth + overblog_graphql.request_validator_rule_max_query_complexity: + class: Overblog\GraphQLBundle\Request\Validator\Rule\MaxQueryComplexity + overblog_graphql.request_parser: class: Overblog\GraphQLBundle\Request\Parser From 8e1f017763232574e912458b07d2e18114625a3e Mon Sep 17 00:00:00 2001 From: Jeremiah VALERIE Date: Thu, 31 Mar 2016 10:28:47 +0200 Subject: [PATCH 3/7] Add to configuration --- DependencyInjection/Configuration.php | 36 +++-- .../OverblogGraphQLExtension.php | 10 +- Request/Executor.php | 4 +- .../Validator/Rule/AbstractQuerySecurity.php | 92 ++++++++++++- ...ueryComplexity.php => QueryComplexity.php} | 125 ++++-------------- .../{MaxQueryDepth.php => QueryDepth.php} | 6 +- Resources/config/services.yml | 12 +- ...xQueryDepthTest.php => QueryDepthTest.php} | 10 +- 8 files changed, 166 insertions(+), 129 deletions(-) rename Request/Validator/Rule/{MaxQueryComplexity.php => QueryComplexity.php} (60%) rename Request/Validator/Rule/{MaxQueryDepth.php => QueryDepth.php} (95%) rename Tests/Request/Validator/Rule/{MaxQueryDepthTest.php => QueryDepthTest.php} (93%) diff --git a/DependencyInjection/Configuration.php b/DependencyInjection/Configuration.php index a53dd65fa..33426dc0d 100644 --- a/DependencyInjection/Configuration.php +++ b/DependencyInjection/Configuration.php @@ -11,6 +11,7 @@ namespace Overblog\GraphQLBundle\DependencyInjection; +use Overblog\GraphQLBundle\Request\Validator\Rule\QueryDepth; use Symfony\Component\Config\Definition\Builder\TreeBuilder; use Symfony\Component\Config\Definition\ConfigurationInterface; @@ -92,18 +93,8 @@ public function getConfigTreeBuilder() ->arrayNode('security') ->addDefaultsIfNotSet() ->children() - ->integerNode('query_max_depth') - ->info('Limit query depth. Disabled if equal to false or 0.') - ->beforeNormalization() - ->ifTrue(function ($v) { return false === $v; }) - ->then(function () { return 0; }) - ->end() - ->defaultFalse() - ->validate() - ->ifTrue(function ($v) { return $v < 0; }) - ->thenInvalid('"overblog_graphql.security.query_max_depth" must be greater or equal to 0.') - ->end() - ->end() + ->append($this->addSecurityQuerySection('query_max_depth', QueryDepth::DISABLED)) + ->append($this->addSecurityQuerySection('query_max_complexity', QueryDepth::DISABLED)) ->end() ->end() ->end() @@ -111,4 +102,25 @@ public function getConfigTreeBuilder() return $treeBuilder; } + + private function addSecurityQuerySection($name, $disabledValue) + { + $builder = new TreeBuilder(); + $node = $builder->root($name, 'integer'); + + $node + ->info('Disabled if equal to false.') + ->beforeNormalization() + ->ifTrue(function ($v) { return false === $v; }) + ->then(function () use ($disabledValue) { return $disabledValue; }) + ->end() + ->defaultFalse() + ->validate() + ->ifTrue(function ($v) { return $v < 0; }) + ->thenInvalid('"overblog_graphql.security.'.$name.'" must be greater or equal to 0.') + ->end() + ; + + return $node; + } } diff --git a/DependencyInjection/OverblogGraphQLExtension.php b/DependencyInjection/OverblogGraphQLExtension.php index 39dc983d3..d46cc7403 100644 --- a/DependencyInjection/OverblogGraphQLExtension.php +++ b/DependencyInjection/OverblogGraphQLExtension.php @@ -54,11 +54,19 @@ private function setSecurity(array $config, ContainerBuilder $container) { if (isset($config['security']['query_max_depth'])) { $container - ->getDefinition($this->getAlias().'.request_validator_rule_max_query_depth') + ->getDefinition($this->getAlias().'.request_validator_rule_query_depth') ->addMethodCall('setMaxQueryDepth', [$config['security']['query_max_depth']]) ->setPublic(true) ; } + + if (isset($config['security']['query_max_complexity'])) { + $container + ->getDefinition($this->getAlias().'.request_validator_rule_query_complexity') + ->addMethodCall('setMaxQueryComplexity', [$config['security']['query_max_complexity']]) + ->setPublic(true) + ; + } } private function setGraphiQLTemplate(array $config, ContainerBuilder $container) diff --git a/Request/Executor.php b/Request/Executor.php index 3a64dd2d8..fdc14f60b 100644 --- a/Request/Executor.php +++ b/Request/Executor.php @@ -22,7 +22,7 @@ use Overblog\GraphQLBundle\Error\ErrorHandler; use Overblog\GraphQLBundle\Event\Events; use Overblog\GraphQLBundle\Event\ExecutorContextEvent; -use Overblog\GraphQLBundle\Request\Validator\Rule\MaxQueryComplexity; +use Overblog\GraphQLBundle\Request\Validator\Rule\QueryComplexity; use Symfony\Component\EventDispatcher\EventDispatcherInterface; class Executor @@ -98,7 +98,7 @@ private function executeAndReturnResult(Schema $schema, $requestString, $rootVal $source = new Source($requestString ?: '', 'GraphQL request'); $documentAST = GraphQLParser::parse($source); //todo set using service - MaxQueryComplexity::setRawVariableValues($variableValues); + QueryComplexity::setRawVariableValues($variableValues); $validationErrors = DocumentValidator::validate($schema, $documentAST, $this->validationRules); diff --git a/Request/Validator/Rule/AbstractQuerySecurity.php b/Request/Validator/Rule/AbstractQuerySecurity.php index 1a792c402..51812fcbe 100644 --- a/Request/Validator/Rule/AbstractQuerySecurity.php +++ b/Request/Validator/Rule/AbstractQuerySecurity.php @@ -11,22 +11,27 @@ namespace Overblog\GraphQLBundle\Request\Validator\Rule; +use GraphQL\Language\AST\Field; use GraphQL\Language\AST\FragmentDefinition; use GraphQL\Language\AST\FragmentSpread; +use GraphQL\Language\AST\InlineFragment; +use GraphQL\Language\AST\Node; +use GraphQL\Language\AST\SelectionSet; use GraphQL\Type\Definition\Type; use GraphQL\Type\Definition\WrappingType; +use GraphQL\Utils\TypeInfo; use GraphQL\Validator\ValidationContext; abstract class AbstractQuerySecurity { + const DISABLED = 0; + /** @var FragmentDefinition[] */ private $fragments = []; /** @var Type[] */ private $rootTypes = []; - private $initialValue; - /** * @return \GraphQL\Language\AST\FragmentDefinition[] */ @@ -121,5 +126,88 @@ protected function invokeIfNeeded(ValidationContext $context, array $validators) return $validators; } + /** + * Given a selectionSet, adds all of the fields in that selection to + * the passed in map of fields, and returns it at the end. + * + * Note: This is not the same as execution's collectFields because at static + * time we do not know what object type will be used, so we unconditionally + * spread in all fragments. + * + * @see GraphQL\Validator\Rules\OverlappingFieldsCanBeMerged + * + * @param ValidationContext $context + * @param Type|null $parentType + * @param SelectionSet $selectionSet + * @param \ArrayObject $visitedFragmentNames + * @param \ArrayObject $astAndDefs + * + * @return \ArrayObject + */ + protected function collectFieldASTsAndDefs(ValidationContext $context, $parentType, SelectionSet $selectionSet, \ArrayObject $visitedFragmentNames = null, \ArrayObject $astAndDefs = null) + { + $_visitedFragmentNames = $visitedFragmentNames ?: new \ArrayObject(); + $_astAndDefs = $astAndDefs ?: new \ArrayObject(); + + foreach ($selectionSet->selections as $selection) { + switch ($selection->kind) { + case Node::FIELD: + $fieldName = $selection->name->value; + $fieldDef = null; + if ($parentType && method_exists($parentType, 'getFields')) { + $tmp = $parentType->getFields(); + if (isset($tmp[$fieldName])) { + $fieldDef = $tmp[$fieldName]; + } + } + $responseName = $this->getFieldName($selection); + if (!isset($_astAndDefs[$responseName])) { + $_astAndDefs[$responseName] = new \ArrayObject(); + } + $_astAndDefs[$responseName][] = [$selection, $fieldDef]; + break; + case Node::INLINE_FRAGMENT: + /* @var InlineFragment $inlineFragment */ + $_astAndDefs = $this->collectFieldASTsAndDefs( + $context, + TypeInfo::typeFromAST($context->getSchema(), $selection->typeCondition), + $selection->selectionSet, + $_visitedFragmentNames, + $_astAndDefs + ); + break; + case Node::FRAGMENT_SPREAD: + /* @var FragmentSpread $selection */ + $fragName = $selection->name->value; + if (!empty($_visitedFragmentNames[$fragName])) { + continue; + } + $_visitedFragmentNames[$fragName] = true; + $fragment = $context->getFragment($fragName); + if (!$fragment) { + continue; + } + $_astAndDefs = $this->collectFieldASTsAndDefs( + $context, + TypeInfo::typeFromAST($context->getSchema(), $fragment->typeCondition), + $fragment->selectionSet, + $_visitedFragmentNames, + $_astAndDefs + ); + break; + } + } + + return $_astAndDefs; + } + + protected function getFieldName(Field $node) + { + $fieldName = $node->name->value; + $responseName = $node->alias ? $node->alias->value : $fieldName; + + return $responseName; + } + abstract protected function isEnabled(); } diff --git a/Request/Validator/Rule/MaxQueryComplexity.php b/Request/Validator/Rule/QueryComplexity.php similarity index 60% rename from Request/Validator/Rule/MaxQueryComplexity.php rename to Request/Validator/Rule/QueryComplexity.php index d235fdd32..ea1440225 100644 --- a/Request/Validator/Rule/MaxQueryComplexity.php +++ b/Request/Validator/Rule/QueryComplexity.php @@ -15,24 +15,22 @@ use GraphQL\Executor\Values; use GraphQL\Language\AST\Field; use GraphQL\Language\AST\FragmentSpread; -use GraphQL\Language\AST\InlineFragment; use GraphQL\Language\AST\Node; use GraphQL\Language\AST\SelectionSet; -use GraphQL\Language\AST\Type; use GraphQL\Language\Visitor; use GraphQL\Type\Definition\FieldDefinition; -use GraphQL\Utils\TypeInfo; use GraphQL\Validator\ValidationContext; -class MaxQueryComplexity extends AbstractQuerySecurity +class QueryComplexity extends AbstractQuerySecurity { - const DEFAULT_QUERY_MAX_COMPLEXITY = 1000; + const DEFAULT_QUERY_MAX_COMPLEXITY = self::DISABLED; private static $maxQueryComplexity; private static $rawVariableValues = []; - private static $complexityMap = []; + /** @var callable[] */ + private static $complexityCalculators = []; private $variableDefs; @@ -46,10 +44,6 @@ class MaxQueryComplexity extends AbstractQuerySecurity public function __construct($maxQueryDepth = self::DEFAULT_QUERY_MAX_COMPLEXITY) { $this->setMaxQueryComplexity($maxQueryDepth); - //todo delete after test -// self::$complexityMap['User'] = function (ValidationContext $context, $args, $childrenComplexity) { -// return 25 + $args['id'] * $childrenComplexity; -// }; } public static function maxQueryComplexityErrorMessage($max, $count) @@ -57,6 +51,26 @@ public static function maxQueryComplexityErrorMessage($max, $count) return sprintf('Max query complexity should be %d but got %d.', $max, $count); } + public static function addComplexityCalculator($name, callable $complexityCalculator) + { + self::$complexityCalculators[$name] = $complexityCalculator; + } + + public static function hasComplexityCalculator($name) + { + return isset(self::$complexityCalculators[$name]); + } + + public static function getComplexityCalculator($name) + { + return static::hasComplexityCalculator($name) ? self::$complexityCalculators[$name] : null; + } + + public static function removeComplexityCalculator($name) + { + unset(self::$complexityCalculators[$name]); + } + /** * Set max query complexity. If equal to 0 no check is done. Must be greater or equal to 0. * @@ -157,11 +171,9 @@ private function nodeComplexity(Node $node, $complexity = 0) $complexityCalculator = null; // custom complexity is set ? - if ($fieldDef && isset(self::$complexityMap[$fieldDef->getType()->name])) { + if (null !== $fieldDef && (null !== $complexityCalculator = static::getComplexityCalculator($fieldDef->getType()->name))) { $args = $this->buildFieldArguments($node); - $typeName = $fieldDef->getType()->name; - - $complexity = call_user_func_array(self::$complexityMap[$typeName], [$this->context, $args, $childrenComplexity]); + $complexity = call_user_func_array($complexityCalculator, [$this->context, $args, $childrenComplexity]); } else { // default complexity calculator $complexity = $complexity + $childrenComplexity + 1; @@ -187,14 +199,6 @@ private function nodeComplexity(Node $node, $complexity = 0) return $complexity; } - private function getFieldName(Field $node) - { - $fieldName = $node->name->value; - $responseName = $node->alias ? $node->alias->value : $fieldName; - - return $responseName; - } - private function astFieldToFieldDef(Field $field) { $fieldName = $this->getFieldName($field); @@ -232,81 +236,6 @@ private function buildFieldArguments(Field $node) protected function isEnabled() { - return $this->getMaxQueryComplexity() > 0; - } - - /** - * Given a selectionSet, adds all of the fields in that selection to - * the passed in map of fields, and returns it at the end. - * - * Note: This is not the same as execution's collectFields because at static - * time we do not know what object type will be used, so we unconditionally - * spread in all fragments. - * - * @see GraphQL\Validator\Rules\OverlappingFieldsCanBeMerged - * - * @param ValidationContext $context - * @param Type|null $parentType - * @param SelectionSet $selectionSet - * @param \ArrayObject $visitedFragmentNames - * @param \ArrayObject $astAndDefs - * - * @return \ArrayObject - */ - private function collectFieldASTsAndDefs(ValidationContext $context, $parentType, SelectionSet $selectionSet, \ArrayObject $visitedFragmentNames = null, \ArrayObject $astAndDefs = null) - { - $_visitedFragmentNames = $visitedFragmentNames ?: new \ArrayObject(); - $_astAndDefs = $astAndDefs ?: new \ArrayObject(); - - foreach ($selectionSet->selections as $selection) { - switch ($selection->kind) { - case Node::FIELD: - $fieldName = $selection->name->value; - $fieldDef = null; - if ($parentType && method_exists($parentType, 'getFields')) { - $tmp = $parentType->getFields(); - if (isset($tmp[$fieldName])) { - $fieldDef = $tmp[$fieldName]; - } - } - $responseName = $this->getFieldName($selection); - if (!isset($_astAndDefs[$responseName])) { - $_astAndDefs[$responseName] = new \ArrayObject(); - } - $_astAndDefs[$responseName][] = [$selection, $fieldDef]; - break; - case Node::INLINE_FRAGMENT: - /* @var InlineFragment $inlineFragment */ - $_astAndDefs = $this->collectFieldASTsAndDefs( - $context, - TypeInfo::typeFromAST($context->getSchema(), $selection->typeCondition), - $selection->selectionSet, - $_visitedFragmentNames, - $_astAndDefs - ); - break; - case Node::FRAGMENT_SPREAD: - /* @var FragmentSpread $selection */ - $fragName = $selection->name->value; - if (!empty($_visitedFragmentNames[$fragName])) { - continue; - } - $_visitedFragmentNames[$fragName] = true; - $fragment = $context->getFragment($fragName); - if (!$fragment) { - continue; - } - $_astAndDefs = $this->collectFieldASTsAndDefs( - $context, - TypeInfo::typeFromAST($context->getSchema(), $fragment->typeCondition), - $fragment->selectionSet, - $_visitedFragmentNames, - $_astAndDefs - ); - break; - } - } - - return $_astAndDefs; + return $this->getMaxQueryComplexity() !== static::DISABLED; } } diff --git a/Request/Validator/Rule/MaxQueryDepth.php b/Request/Validator/Rule/QueryDepth.php similarity index 95% rename from Request/Validator/Rule/MaxQueryDepth.php rename to Request/Validator/Rule/QueryDepth.php index dfe14f31a..6a85b5eea 100644 --- a/Request/Validator/Rule/MaxQueryDepth.php +++ b/Request/Validator/Rule/QueryDepth.php @@ -17,9 +17,9 @@ use GraphQL\Language\AST\SelectionSet; use GraphQL\Validator\ValidationContext; -class MaxQueryDepth extends AbstractQuerySecurity +class QueryDepth extends AbstractQuerySecurity { - const DEFAULT_QUERY_MAX_DEPTH = 100; + const DEFAULT_QUERY_MAX_DEPTH = self::DISABLED; /** * @var int @@ -74,7 +74,7 @@ public function __invoke(ValidationContext $context) protected function isEnabled() { - return $this->getMaxQueryDepth() > 0; + return $this->getMaxQueryDepth() !== static::DISABLED; } private function nodeMaxDepth(Node $node, $depth = 1, $maxDepth = 1) diff --git a/Resources/config/services.yml b/Resources/config/services.yml index 469694c70..3dc5626ec 100644 --- a/Resources/config/services.yml +++ b/Resources/config/services.yml @@ -37,14 +37,14 @@ services: - %kernel.debug% - "@overblog_graphql.error_handler" calls: - - ["addValidatorRule", ["@overblog_graphql.request_validator_rule_max_query_depth"]] - - ["addValidatorRule", ["@overblog_graphql.request_validator_rule_max_query_complexity"]] + - ["addValidatorRule", ["@overblog_graphql.request_validator_rule_query_depth"]] + - ["addValidatorRule", ["@overblog_graphql.request_validator_rule_query_complexity"]] - overblog_graphql.request_validator_rule_max_query_depth: - class: Overblog\GraphQLBundle\Request\Validator\Rule\MaxQueryDepth + overblog_graphql.request_validator_rule_query_depth: + class: Overblog\GraphQLBundle\Request\Validator\Rule\QueryDepth - overblog_graphql.request_validator_rule_max_query_complexity: - class: Overblog\GraphQLBundle\Request\Validator\Rule\MaxQueryComplexity + overblog_graphql.request_validator_rule_query_complexity: + class: Overblog\GraphQLBundle\Request\Validator\Rule\QueryComplexity overblog_graphql.request_parser: class: Overblog\GraphQLBundle\Request\Parser diff --git a/Tests/Request/Validator/Rule/MaxQueryDepthTest.php b/Tests/Request/Validator/Rule/QueryDepthTest.php similarity index 93% rename from Tests/Request/Validator/Rule/MaxQueryDepthTest.php rename to Tests/Request/Validator/Rule/QueryDepthTest.php index 806fa9476..8e1879670 100644 --- a/Tests/Request/Validator/Rule/MaxQueryDepthTest.php +++ b/Tests/Request/Validator/Rule/QueryDepthTest.php @@ -16,9 +16,9 @@ use GraphQL\Language\SourceLocation; use GraphQL\Type\Introspection; use GraphQL\Validator\DocumentValidator; -use Overblog\GraphQLBundle\Request\Validator\Rule\MaxQueryDepth; +use Overblog\GraphQLBundle\Request\Validator\Rule\QueryDepth; -class MaxQueryDepthTest extends \PHPUnit_Framework_TestCase +class QueryDepthTest extends \PHPUnit_Framework_TestCase { /** * @param $queryDepth @@ -64,7 +64,7 @@ public function testIgnoreIntrospectionQuery() */ public function testMaxQueryDepthMustBeGreaterOrEqualTo0() { - new MaxQueryDepth(-1); + new QueryDepth(-1); } public function queryDataProvider() @@ -94,7 +94,7 @@ public function queryDataProvider() private function createFormattedError($max, $count) { - return FormattedError::create(MaxQueryDepth::maxQueryDepthErrorMessage($max, $count), [new SourceLocation(1, 17)]); + return FormattedError::create(QueryDepth::maxQueryDepthErrorMessage($max, $count), [new SourceLocation(1, 17)]); } private function buildRecursiveQuery($depth) @@ -148,7 +148,7 @@ private function assertDocumentValidator($queryString, $depth, array $expectedEr $errors = DocumentValidator::validate( Schema::buildSchema(), Parser::parse($queryString), - [new MaxQueryDepth($depth)] + [new QueryDepth($depth)] ); $this->assertEquals($expectedErrors, array_map(['GraphQL\Error', 'formatError'], $errors), $queryString); From c6899c39ff08f5b50070c416e5bc89e77fde2fe6 Mon Sep 17 00:00:00 2001 From: Jeremiah VALERIE Date: Thu, 31 Mar 2016 18:16:56 +0200 Subject: [PATCH 4/7] Complexity direct on field --- Definition/Builder/ConfigBuilderInterface.php | 19 ---- DependencyInjection/TypesConfiguration.php | 3 + README.md | 57 +++++++++- .../Validator/Rule/AbstractQuerySecurity.php | 4 +- Request/Validator/Rule/QueryComplexity.php | 64 ++++++----- Resolver/Config/FieldsConfigSolution.php | 38 ++++++- Resources/config/services.yml | 1 + .../Rule/AbstractQuerySecurityTest.php | 107 ++++++++++++++++++ .../Validator/Rule/QueryComplexityTest.php | 38 +++++++ .../Request/Validator/Rule/QueryDepthTest.php | 99 ++++------------ 10 files changed, 301 insertions(+), 129 deletions(-) delete mode 100644 Definition/Builder/ConfigBuilderInterface.php create mode 100644 Tests/Request/Validator/Rule/AbstractQuerySecurityTest.php create mode 100644 Tests/Request/Validator/Rule/QueryComplexityTest.php diff --git a/Definition/Builder/ConfigBuilderInterface.php b/Definition/Builder/ConfigBuilderInterface.php deleted file mode 100644 index 9fbf6a5fc..000000000 --- a/Definition/Builder/ConfigBuilderInterface.php +++ /dev/null @@ -1,19 +0,0 @@ - - * - * For the full copyright and license information, please view the LICENSE - * file that was distributed with this source code. - */ - -namespace Overblog\GraphQLBundle\Definition\Builder; - -interface ConfigBuilderInterface -{ - public function create($type, array $config); - - public function getBaseClassName($type); -} diff --git a/DependencyInjection/TypesConfiguration.php b/DependencyInjection/TypesConfiguration.php index 678539d83..1d83c68fb 100644 --- a/DependencyInjection/TypesConfiguration.php +++ b/DependencyInjection/TypesConfiguration.php @@ -186,6 +186,9 @@ private function addFieldsSelection($name, $enabledBuilder = true) } $prototype + ->scalarNode('complexity') + ->info('Custom complexity calculator.') + ->end() ->end() ->end(); diff --git a/README.md b/README.md index 36d22f471..a0eb8d95a 100644 --- a/README.md +++ b/README.md @@ -613,9 +613,11 @@ class CharacterResolver } ``` +Security +-------- + +### Access Control -Access Control --------------- An access control can be add on each field using `config.fields.*.access` or globally with `config.fieldsDefaultAccess`. If `config.fields.*.access` value is true field will be normally resolved but will be `null` otherwise. @@ -649,6 +651,57 @@ Human: interfaces: [Character] ``` +### Query Complexity Analysis + +This is a PHP port of [Query Complexity Analysis](http://sangria-graphql.org/learn/#query-complexity-analysis) in Sangria implementation. +The Introspection Query is bypass. + +Define your max accepted complexity: + +```yaml +#app/config/config.yml +overblog_graphql: + security: + query_max_complexity: 1000 +``` + +Default value `false` disabled validation. + +Customize your field complexity using `config.fields.*.complexity` + +```yaml +# src/MyBundle/Resources/config/graphql/Query.types.yml + +Query: + type: object + config: + fields: + droid: + type: "Droid" + complexity: '@=1000 + childrenComplexity' + args: + id: + description: "id of the droid" + type: "String!" + resolve: "@=resolver('character_droid', [args])" +``` + +In the example we add `1000` on the complexity every time using `Query` `droid` field in query. + +### Limiting Query Depth + +This is a PHP port of [Limiting Query Depth](http://sangria-graphql.org/learn/#limiting-query-depth) in Sangria implementation. +The Introspection Query is bypass. + +```yaml +#app/config/config.yml +overblog_graphql: + security: + query_max_depth: 10 +``` + +Default value `false` disabled validation. + Field builder ------------- diff --git a/Request/Validator/Rule/AbstractQuerySecurity.php b/Request/Validator/Rule/AbstractQuerySecurity.php index 51812fcbe..8275b2077 100644 --- a/Request/Validator/Rule/AbstractQuerySecurity.php +++ b/Request/Validator/Rule/AbstractQuerySecurity.php @@ -164,7 +164,7 @@ protected function collectFieldASTsAndDefs(ValidationContext $context, $parentTy if (!isset($_astAndDefs[$responseName])) { $_astAndDefs[$responseName] = new \ArrayObject(); } - $_astAndDefs[$responseName][] = [$selection, $fieldDef]; + $_astAndDefs[$responseName][] = [$selection, $fieldDef, $context]; break; case Node::INLINE_FRAGMENT: /* @var InlineFragment $inlineFragment */ @@ -201,7 +201,7 @@ protected function collectFieldASTsAndDefs(ValidationContext $context, $parentTy return $_astAndDefs; } - protected function getFieldName(Field $node) + protected function getFieldName(Node $node) { $fieldName = $node->name->value; $responseName = $node->alias ? $node->alias->value : $fieldName; diff --git a/Request/Validator/Rule/QueryComplexity.php b/Request/Validator/Rule/QueryComplexity.php index ea1440225..80b0d9b19 100644 --- a/Request/Validator/Rule/QueryComplexity.php +++ b/Request/Validator/Rule/QueryComplexity.php @@ -16,6 +16,7 @@ use GraphQL\Language\AST\Field; use GraphQL\Language\AST\FragmentSpread; use GraphQL\Language\AST\Node; +use GraphQL\Language\AST\OperationDefinition; use GraphQL\Language\AST\SelectionSet; use GraphQL\Language\Visitor; use GraphQL\Type\Definition\FieldDefinition; @@ -80,7 +81,7 @@ public static function setMaxQueryComplexity($maxQueryComplexity) { self::checkIfGreaterOrEqualToZero('maxQueryComplexity', $maxQueryComplexity); - self::$maxQueryComplexity = (int) $maxQueryComplexity; + self::$maxQueryComplexity = (int)$maxQueryComplexity; } public static function getMaxQueryComplexity() @@ -125,65 +126,71 @@ public function __invoke(ValidationContext $context) return Visitor::skipNode(); }, - Node::FIELD => [ - 'leave' => function (Field $node) use ($context, $complexity) { + Node::OPERATION_DEFINITION => [ + 'leave' => function (OperationDefinition $operationDefinition) use ($context, $complexity) { // check complexity only on first rootTypes children and ignore check on introspection query - if ($this->isParentRootType($context) && !$this->isIntrospectionType($context)) { - $complexity = $this->nodeComplexity($node, $complexity); + //if (!$this->isIntrospectionType($context)) { + $type = $context->getType(); + $complexity = $this->fieldComplexity($operationDefinition, $type->name, $complexity); if ($complexity > $this->getMaxQueryComplexity()) { - return new Error($this->maxQueryComplexityErrorMessage($this->getMaxQueryComplexity(), $complexity), [$node]); + return new Error($this->maxQueryComplexityErrorMessage($this->getMaxQueryComplexity(), $complexity)); } - } + //} }, ], ] ); } - private function fieldComplexity(Node $node, $complexity = 0) + private function fieldComplexity(Node $node, $typeName, $complexity = 0) { if (!isset($node->selectionSet)) { return $complexity; } foreach ($node->selectionSet->selections as $childNode) { - $complexity = $this->nodeComplexity($childNode, $complexity); + $complexity = $this->nodeComplexity($childNode, $typeName, $complexity); } return $complexity; } - private function nodeComplexity(Node $node, $complexity = 0) + private function nodeComplexity(Node $node, $typeName, $complexity = 0) { switch ($node->kind) { case Node::FIELD: // calculate children complexity if needed $childrenComplexity = 0; + $astFieldInfo = $this->astFieldInfo($node); + /** @var ValidationContext|null $fieldValidationContext */ + $fieldValidationContext = $astFieldInfo[2]; + /** @var FieldDefinition|null $fieldDef */ + $fieldDef = $astFieldInfo[1]; + // node has children? if (isset($node->selectionSet)) { - $childrenComplexity = $this->fieldComplexity($node); + $type = $fieldDef->getType(); + $childrenComplexity = $this->fieldComplexity($node, $type->name); } - $fieldDef = $this->astFieldToFieldDef($node); - - $complexityCalculator = null; + // default complexity calculator + $complexity = $complexity + $childrenComplexity + 1; + $complexityCalculatorName = $typeName . '.' . $this->getFieldName($node); // custom complexity is set ? - if (null !== $fieldDef && (null !== $complexityCalculator = static::getComplexityCalculator($fieldDef->getType()->name))) { + if (null !== $complexityCalculator = static::getComplexityCalculator($complexityCalculatorName)) { $args = $this->buildFieldArguments($node); - $complexity = call_user_func_array($complexityCalculator, [$this->context, $args, $childrenComplexity]); - } else { - // default complexity calculator - $complexity = $complexity + $childrenComplexity + 1; + //get field complexity using custom complexityCalculator + $complexity = call_user_func_array($complexityCalculator, [$fieldValidationContext, $args, $childrenComplexity]); } break; case Node::INLINE_FRAGMENT: // node has children? if (isset($node->selectionSet)) { - $complexity = $this->fieldComplexity($node, $complexity); + $complexity = $this->fieldComplexity($node, $typeName, $complexity); } break; @@ -191,7 +198,7 @@ private function nodeComplexity(Node $node, $complexity = 0) $fragment = $this->getFragment($node); if (null !== $fragment) { - $complexity = $this->fieldComplexity($fragment, $complexity); + $complexity = $this->fieldComplexity($fragment, $typeName, $complexity); } break; } @@ -199,28 +206,29 @@ private function nodeComplexity(Node $node, $complexity = 0) return $complexity; } - private function astFieldToFieldDef(Field $field) + private function astFieldInfo(Field $field) { $fieldName = $this->getFieldName($field); - $fieldDef = null; + $astFieldInfo = [null, null, null]; if (isset($this->fieldAstAndDefs[$fieldName])) { foreach ($this->fieldAstAndDefs[$fieldName] as $astAndDef) { if ($astAndDef[0] == $field) { - /** @var FieldDefinition $fieldDef */ - $fieldDef = $astAndDef[1]; + $astFieldInfo = $astAndDef; break; } } } - return $fieldDef; + return $astFieldInfo; } private function buildFieldArguments(Field $node) { $rawVariableValues = $this->getRawVariableValues(); - $fieldDef = $this->astFieldToFieldDef($node); - if (null === $fieldDef) { + $astFieldInfo = $this->astFieldInfo($node); + $fieldDef = $astFieldInfo[1]; + + if (!$fieldDef instanceof FieldDefinition) { return; } diff --git a/Resolver/Config/FieldsConfigSolution.php b/Resolver/Config/FieldsConfigSolution.php index 30437ca90..818b5b94b 100644 --- a/Resolver/Config/FieldsConfigSolution.php +++ b/Resolver/Config/FieldsConfigSolution.php @@ -11,10 +11,12 @@ namespace Overblog\GraphQLBundle\Resolver\Config; +use Overblog\GraphQLBundle\Definition\Argument; use Overblog\GraphQLBundle\Definition\Builder\MappingInterface; use Overblog\GraphQLBundle\Error\UserError; use Overblog\GraphQLBundle\Relay\Connection\Output\Connection; use Overblog\GraphQLBundle\Relay\Connection\Output\Edge; +use Overblog\GraphQLBundle\Request\Validator\Rule\QueryComplexity; use Overblog\GraphQLBundle\Resolver\ResolverInterface; class FieldsConfigSolution extends AbstractConfigSolution @@ -40,7 +42,7 @@ public function __construct( public function solve($values, array &$config = null) { // builder must be last - $fieldsTreated = ['type', 'args', 'argsBuilder', 'deprecationReason', 'builder']; + $fieldsTreated = ['complexity', 'type', 'args', 'argsBuilder', 'deprecationReason', 'builder']; $fieldsDefaultAccess = isset($config['fieldsDefaultAccess']) ? $config['fieldsDefaultAccess'] : null; unset($config['fieldsDefaultAccess']); @@ -52,7 +54,7 @@ public function solve($values, array &$config = null) foreach ($fieldsTreated as $fieldTreated) { if (isset($options[$fieldTreated])) { $method = 'solve'.ucfirst($fieldTreated); - $options = $this->$method($options, $field); + $options = $this->$method($options, $field, $config); } } $options = $this->resolveResolveAndAccessIfNeeded($options); @@ -61,6 +63,38 @@ public function solve($values, array &$config = null) return $values; } + private function solveComplexity($options, $field, array $config) + { + if (!isset($options['complexity'])) { + return $options; + } + $treatedOptions = $options; + + $name = $config['name'].'.'.$field; + $value = $treatedOptions['complexity']; + + QueryComplexity::addComplexityCalculator( + $name, + function () use ($value) { + $args = func_get_args(); + $complexity = $this->solveUsingExpressionLanguageIfNeeded( + $value, + [ + 'validationContext' => $args[0], + 'args' => new Argument($args[1]), + 'childrenComplexity' => $args[2], + ] + ); + + return (int) $complexity; + } + ); + + unset($treatedOptions['complexity']); + + return $treatedOptions; + } + private function solveBuilder($options, $field) { $builderConfig = isset($options['builderConfig']) ? $options['builderConfig'] : []; diff --git a/Resources/config/services.yml b/Resources/config/services.yml index 3dc5626ec..d4a3688d4 100644 --- a/Resources/config/services.yml +++ b/Resources/config/services.yml @@ -1,6 +1,7 @@ parameters: overblog_graphql.default_resolver: [Overblog\GraphQLBundle\Resolver\Resolver, defaultResolveFn] + #todo use tagged services overblog_graphql.configs_mapping: fields: {id: "overblog_graphql.fields_config_solution", method: "solve"} isTypeOf: {id: "overblog_graphql.resolve_callback_config_solution", method: "solve"} diff --git a/Tests/Request/Validator/Rule/AbstractQuerySecurityTest.php b/Tests/Request/Validator/Rule/AbstractQuerySecurityTest.php new file mode 100644 index 000000000..69abf49de --- /dev/null +++ b/Tests/Request/Validator/Rule/AbstractQuerySecurityTest.php @@ -0,0 +1,107 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Overblog\GraphQLBundle\Tests\Request\Validator\Rule; + +use GraphQL\FormattedError; +use GraphQL\Language\Parser; +use GraphQL\Language\SourceLocation; +use GraphQL\Type\Introspection; +use GraphQL\Validator\DocumentValidator; +use Overblog\GraphQLBundle\Request\Validator\Rule\AbstractQuerySecurity; + +abstract class AbstractQuerySecurityTest extends \PHPUnit_Framework_TestCase +{ + /** + * @param $max + * + * @return AbstractQuerySecurity + */ + abstract protected function createRule($max); + + /** + * @param $max + * @param $count + * + * @return string + */ + abstract protected function getErrorMessage($max, $count); + + public function testIgnoreIntrospectionQuery() + { + $this->assertDocumentValidator(Introspection::getIntrospectionQuery(true), 1); + } + + protected function createFormattedError($max, $count) + { + return FormattedError::create($this->getErrorMessage($max, $count), [new SourceLocation(1, 17)]); + } + + protected function buildRecursiveQuery($depth) + { + $query = sprintf('query MyQuery { human%s }', $this->buildRecursiveQueryPart($depth)); + + return $query; + } + + protected function buildRecursiveUsingFragmentQuery($depth) + { + $query = sprintf( + 'query MyQuery { human { ...F1 } } fragment F1 on Human %s', + $this->buildRecursiveQueryPart($depth) + ); + + return $query; + } + + protected function buildRecursiveUsingInlineFragmentQuery($depth) + { + $query = sprintf( + 'query MyQuery { human { ...on Human %s } }', + $this->buildRecursiveQueryPart($depth) + ); + + return $query; + } + + protected function buildRecursiveQueryPart($depth) + { + $templates = [ + 'human' => ' { firstName%s } ', + 'dog' => ' dog { name%s } ', + ]; + + $part = $templates['human']; + + for ($i = 1; $i <= $depth; ++$i) { + $key = ($i % 2 == 1) ? 'human' : 'dog'; + $template = $templates[$key]; + + $part = sprintf($part, ('human' == $key ? ' owner ' : '').$template); + } + $part = str_replace('%s', '', $part); + + return $part; + } + + protected function assertDocumentValidator($queryString, $max, array $expectedErrors = []) + { + $errors = DocumentValidator::validate( + Schema::buildSchema(), + Parser::parse($queryString), + [$this->createRule($max)] + ); + + $this->assertEquals($expectedErrors, array_map(['GraphQL\Error', 'formatError'], $errors), $queryString); + + return $errors; + } +} diff --git a/Tests/Request/Validator/Rule/QueryComplexityTest.php b/Tests/Request/Validator/Rule/QueryComplexityTest.php new file mode 100644 index 000000000..8490fd11f --- /dev/null +++ b/Tests/Request/Validator/Rule/QueryComplexityTest.php @@ -0,0 +1,38 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Overblog\GraphQLBundle\Tests\Request\Validator\Rule; + +use Overblog\GraphQLBundle\Request\Validator\Rule\QueryComplexity; + +class QueryComplexityTest extends AbstractQuerySecurityTest +{ + /** + * @param $max + * @param $count + * + * @return string + */ + protected function getErrorMessage($max, $count) + { + return QueryComplexity::maxQueryComplexityErrorMessage($max, $count); + } + + /** + * @param $maxDepth + * + * @return QueryComplexity + */ + protected function createRule($maxDepth) + { + return new QueryComplexity($maxDepth); + } +} diff --git a/Tests/Request/Validator/Rule/QueryDepthTest.php b/Tests/Request/Validator/Rule/QueryDepthTest.php index 8e1879670..b31eb5fc1 100644 --- a/Tests/Request/Validator/Rule/QueryDepthTest.php +++ b/Tests/Request/Validator/Rule/QueryDepthTest.php @@ -11,15 +11,31 @@ namespace Overblog\GraphQLBundle\Tests\Request\Validator\Rule; -use GraphQL\FormattedError; -use GraphQL\Language\Parser; -use GraphQL\Language\SourceLocation; -use GraphQL\Type\Introspection; -use GraphQL\Validator\DocumentValidator; use Overblog\GraphQLBundle\Request\Validator\Rule\QueryDepth; -class QueryDepthTest extends \PHPUnit_Framework_TestCase +class QueryDepthTest extends AbstractQuerySecurityTest { + /** + * @param $max + * @param $count + * + * @return string + */ + protected function getErrorMessage($max, $count) + { + return QueryDepth::maxQueryDepthErrorMessage($max, $count); + } + + /** + * @param $maxDepth + * + * @return QueryDepth + */ + protected function createRule($maxDepth) + { + return new QueryDepth($maxDepth); + } + /** * @param $queryDepth * @param int $maxQueryDepth @@ -53,18 +69,13 @@ public function testInlineFragmentQueries($queryDepth, $maxQueryDepth = 7, $expe $this->assertDocumentValidator($this->buildRecursiveUsingInlineFragmentQuery($queryDepth), $maxQueryDepth, $expectedErrors); } - public function testIgnoreIntrospectionQuery() - { - $this->assertDocumentValidator(Introspection::getIntrospectionQuery(true), 1); - } - /** * @expectedException \InvalidArgumentException * @expectedExceptionMessage $maxQueryDepth argument must be greater or equal to 0. */ public function testMaxQueryDepthMustBeGreaterOrEqualTo0() { - new QueryDepth(-1); + $this->createRule(-1); } public function queryDataProvider() @@ -91,68 +102,4 @@ public function queryDataProvider() ], // failed because depth over limit (20) ]; } - - private function createFormattedError($max, $count) - { - return FormattedError::create(QueryDepth::maxQueryDepthErrorMessage($max, $count), [new SourceLocation(1, 17)]); - } - - private function buildRecursiveQuery($depth) - { - $query = sprintf('query MyQuery { human%s }', $this->buildRecursiveQueryPart($depth)); - - return $query; - } - - private function buildRecursiveUsingFragmentQuery($depth) - { - $query = sprintf( - 'query MyQuery { human { ...F1 } } fragment F1 on Human %s', - $this->buildRecursiveQueryPart($depth) - ); - - return $query; - } - private function buildRecursiveUsingInlineFragmentQuery($depth) - { - $query = sprintf( - 'query MyQuery { human { ...on Human %s } }', - $this->buildRecursiveQueryPart($depth) - ); - - return $query; - } - - private function buildRecursiveQueryPart($depth) - { - $templates = [ - 'human' => ' { firstName%s } ', - 'dog' => ' dog { name%s } ', - ]; - - $part = $templates['human']; - - for ($i = 1; $i <= $depth; ++$i) { - $key = ($i % 2 == 1) ? 'human' : 'dog'; - $template = $templates[$key]; - - $part = sprintf($part, ('human' == $key ? ' owner ' : '').$template); - } - $part = str_replace('%s', '', $part); - - return $part; - } - - private function assertDocumentValidator($queryString, $depth, array $expectedErrors = []) - { - $errors = DocumentValidator::validate( - Schema::buildSchema(), - Parser::parse($queryString), - [new QueryDepth($depth)] - ); - - $this->assertEquals($expectedErrors, array_map(['GraphQL\Error', 'formatError'], $errors), $queryString); - - return $errors; - } } From 8579dc73c4b7ff6fed62ceb4bcd841f4954b0346 Mon Sep 17 00:00:00 2001 From: Jeremiah VALERIE Date: Fri, 1 Apr 2016 11:06:30 +0200 Subject: [PATCH 5/7] Use Custom fieldDefinition with complexity --- Definition/Builder/TypeBuilder.php | 12 +- Definition/EnumType.php | 18 +++ Definition/FieldDefinition.php | 78 +++++++++++++ Definition/FieldsTrait.php | 54 +++++++++ Definition/InputObjectType.php | 18 +++ Definition/InterfaceType.php | 18 +++ Definition/ObjectType.php | 58 ++++++++++ Definition/Type.php | 18 +++ Definition/UnionType.php | 18 +++ README.md | 8 +- Relay/Connection/ConnectionType.php | 2 +- Relay/Connection/EdgeType.php | 2 +- Relay/Connection/PageInfoType.php | 4 +- Relay/Mutation/InputType.php | 6 +- Relay/Mutation/MutationField.php | 2 +- Relay/Mutation/PayloadType.php | 6 +- Relay/Node/GlobalIdField.php | 2 +- Relay/Node/NodeField.php | 2 +- Relay/Node/NodeInterfaceType.php | 4 +- Relay/Node/PluralIdentifyingRootField.php | 2 +- .../Validator/Rule/AbstractQuerySecurity.php | 92 +++++---------- Request/Validator/Rule/QueryComplexity.php | 108 +++++++----------- Request/Validator/Rule/QueryDepth.php | 77 +++++++------ Resolver/Config/FieldsConfigSolution.php | 34 +++--- .../Rule/AbstractQuerySecurityTest.php | 94 ++++++++------- .../Validator/Rule/QueryComplexityTest.php | 79 +++++++++++++ .../Request/Validator/Rule/QueryDepthTest.php | 65 ++++++++++- Tests/Request/Validator/Rule/Schema.php | 16 ++- 28 files changed, 623 insertions(+), 274 deletions(-) create mode 100644 Definition/EnumType.php create mode 100644 Definition/FieldDefinition.php create mode 100644 Definition/FieldsTrait.php create mode 100644 Definition/InputObjectType.php create mode 100644 Definition/InterfaceType.php create mode 100644 Definition/ObjectType.php create mode 100644 Definition/Type.php create mode 100644 Definition/UnionType.php diff --git a/Definition/Builder/TypeBuilder.php b/Definition/Builder/TypeBuilder.php index e88018ea6..c66b65e78 100644 --- a/Definition/Builder/TypeBuilder.php +++ b/Definition/Builder/TypeBuilder.php @@ -11,7 +11,7 @@ namespace Overblog\GraphQLBundle\Definition\Builder; -use GraphQL\Type\Definition\Type; +use Overblog\GraphQLBundle\Definition\Type; use Overblog\GraphQLBundle\Resolver\ResolverInterface; class TypeBuilder @@ -23,11 +23,11 @@ class TypeBuilder 'relay-node' => 'Overblog\\GraphQLBundle\\Relay\\Node\\NodeInterfaceType', 'relay-mutation-input' => 'Overblog\\GraphQLBundle\\Relay\\Mutation\\InputType', 'relay-mutation-payload' => 'Overblog\\GraphQLBundle\\Relay\\Mutation\\PayloadType', - 'object' => 'GraphQL\\Type\\Definition\\ObjectType', - 'enum' => 'GraphQL\\Type\\Definition\\EnumType', - 'interface' => 'GraphQL\\Type\\Definition\\InterfaceType', - 'union' => 'GraphQL\\Type\\Definition\\UnionType', - 'input-object' => 'GraphQL\\Type\\Definition\\InputObjectType', + 'object' => 'Overblog\\GraphQLBundle\\Definition\\ObjectType', + 'enum' => 'Overblog\\GraphQLBundle\\Definition\\EnumType', + 'interface' => 'Overblog\\GraphQLBundle\\Definition\\InterfaceType', + 'union' => 'Overblog\\GraphQLBundle\\Definition\\UnionType', + 'input-object' => 'Overblog\\GraphQLBundle\\Definition\\InputObjectType', ]; public function __construct(ResolverInterface $configResolver) diff --git a/Definition/EnumType.php b/Definition/EnumType.php new file mode 100644 index 000000000..6de97e4d0 --- /dev/null +++ b/Definition/EnumType.php @@ -0,0 +1,18 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Overblog\GraphQLBundle\Definition; + +use GraphQL\Type\Definition\EnumType as BaseEnumType; + +class EnumType extends BaseEnumType +{ +} diff --git a/Definition/FieldDefinition.php b/Definition/FieldDefinition.php new file mode 100644 index 000000000..551712696 --- /dev/null +++ b/Definition/FieldDefinition.php @@ -0,0 +1,78 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Overblog\GraphQLBundle\Definition; + +use GraphQL\Type\Definition\Config; +use GraphQL\Type\Definition\FieldDefinition as BaseFieldDefinition; + +class FieldDefinition extends BaseFieldDefinition +{ + /** + * @var callable + */ + private $complexityFn; + + public static function getDefinition() + { + return array_merge( + parent::getDefinition(), + [ + 'complexity' => Config::CALLBACK, + ] + ); + } + + public static function createMap(array $fields) + { + $map = []; + foreach ($fields as $name => $field) { + if (!isset($field['name'])) { + $field['name'] = $name; + } + $map[$name] = static::create($field); + } + + return $map; + } + + /** + * @param array|Config $field + * + * @return FieldDefinition + */ + public static function create($field) + { + Config::validate($field, static::getDefinition()); + + return new static($field); + } + + protected function __construct(array $config) + { + parent::__construct($config); + + $this->complexityFn = isset($config['complexity']) ? $config['complexity'] : [$this, 'defaultComplexity']; + } + + /** + * @return callable|\Closure + */ + public function getComplexityFn() + { + return $this->complexityFn; + } + + public static function defaultComplexity($childrenComplexity) + { + return $childrenComplexity + 1; + } +} diff --git a/Definition/FieldsTrait.php b/Definition/FieldsTrait.php new file mode 100644 index 000000000..009280352 --- /dev/null +++ b/Definition/FieldsTrait.php @@ -0,0 +1,54 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Overblog\GraphQLBundle\Definition; + +use GraphQL\Type\Definition\Config; +use GraphQL\Utils; + +trait FieldsTrait +{ + /** + * @var FieldDefinition[] + */ + private $_fields; + + /** + * @return FieldDefinition[] + */ + public function getFields() + { + if (null === $this->_fields) { + $fields = isset($this->config['fields']) ? $this->config['fields'] : []; + $fields = is_callable($fields) ? call_user_func($fields) : $fields; + $this->_fields = FieldDefinition::createMap($fields); + } + + return $this->_fields; + } + + /** + * @param string $name + * + * @return FieldDefinition + * + * @throws \Exception + */ + public function getField($name) + { + if (null === $this->_fields) { + $this->getFields(); + } + Utils::invariant(isset($this->_fields[$name]), "Field '%s' is not defined for type '%s'", $name, $this->name); + + return $this->_fields[$name]; + } +} diff --git a/Definition/InputObjectType.php b/Definition/InputObjectType.php new file mode 100644 index 000000000..122802fed --- /dev/null +++ b/Definition/InputObjectType.php @@ -0,0 +1,18 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Overblog\GraphQLBundle\Definition; + +use GraphQL\Type\Definition\InputObjectType as BaseInputObjectType; + +class InputObjectType extends BaseInputObjectType +{ +} diff --git a/Definition/InterfaceType.php b/Definition/InterfaceType.php new file mode 100644 index 000000000..eb5047563 --- /dev/null +++ b/Definition/InterfaceType.php @@ -0,0 +1,18 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Overblog\GraphQLBundle\Definition; + +use GraphQL\Type\Definition\InterfaceType as BaseInterfaceType; + +class InterfaceType extends BaseInterfaceType +{ +} diff --git a/Definition/ObjectType.php b/Definition/ObjectType.php new file mode 100644 index 000000000..f603fce17 --- /dev/null +++ b/Definition/ObjectType.php @@ -0,0 +1,58 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Overblog\GraphQLBundle\Definition; + +use GraphQL\Type\Definition\Config; +use GraphQL\Type\Definition\ObjectType as BaseObjectType; +use GraphQL\Utils; + +class ObjectType extends BaseObjectType +{ + use FieldsTrait; + + private $_isTypeOf; + + /** + * @param array $config + * + * @todo open PR on lib to ease inheritance + */ + public function __construct(array $config) + { + Utils::invariant(!empty($config['name']), 'Every type is expected to have name'); + + Config::validate($config, [ + 'name' => Config::STRING | Config::REQUIRED, + 'fields' => Config::arrayOf( + FieldDefinition::getDefinition(), + Config::KEY_AS_NAME | Config::MAYBE_THUNK + ), + 'description' => Config::STRING, + 'interfaces' => Config::arrayOf( + Config::INTERFACE_TYPE, + Config::MAYBE_THUNK + ), + 'isTypeOf' => Config::CALLBACK, // ($value, ResolveInfo $info) => boolean + 'resolveField' => Config::CALLBACK, + ]); + + $this->name = $config['name']; + $this->description = isset($config['description']) ? $config['description'] : null; + $this->resolveFieldFn = isset($config['resolveField']) ? $config['resolveField'] : null; + $this->_isTypeOf = isset($config['isTypeOf']) ? $config['isTypeOf'] : null; + $this->config = $config; + + if (isset($config['interfaces'])) { + InterfaceType::addImplementationToInterfaces($this); + } + } +} diff --git a/Definition/Type.php b/Definition/Type.php new file mode 100644 index 000000000..260d851e4 --- /dev/null +++ b/Definition/Type.php @@ -0,0 +1,18 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Overblog\GraphQLBundle\Definition; + +use GraphQL\Type\Definition\Type as BaseType; + +class Type extends BaseType +{ +} diff --git a/Definition/UnionType.php b/Definition/UnionType.php new file mode 100644 index 000000000..fcf3aa9d0 --- /dev/null +++ b/Definition/UnionType.php @@ -0,0 +1,18 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Overblog\GraphQLBundle\Definition; + +use GraphQL\Type\Definition\UnionType as BaseUnionType; + +class UnionType extends BaseUnionType +{ +} diff --git a/README.md b/README.md index a0eb8d95a..b46d77c13 100644 --- a/README.md +++ b/README.md @@ -654,7 +654,7 @@ Human: ### Query Complexity Analysis This is a PHP port of [Query Complexity Analysis](http://sangria-graphql.org/learn/#query-complexity-analysis) in Sangria implementation. -The Introspection Query is bypass. +Introspection query with description max complexity is **109**. Define your max accepted complexity: @@ -686,12 +686,13 @@ Query: resolve: "@=resolver('character_droid', [args])" ``` -In the example we add `1000` on the complexity every time using `Query` `droid` field in query. +In the example we add `1000` on the complexity every time using `Query.droid` field in query. +Complexity function signature: `function (int $childrenComplexity = 0, array $args = [])`. ### Limiting Query Depth This is a PHP port of [Limiting Query Depth](http://sangria-graphql.org/learn/#limiting-query-depth) in Sangria implementation. -The Introspection Query is bypass. +Introspection query with description max depth is **7**. ```yaml #app/config/config.yml @@ -764,6 +765,7 @@ Expression | Description | Scope **value** | Resolver value | only available in resolve context **args** | Resolver args array | only available in resolve context **info** | Resolver GraphQL\Type\Definition\ResolveInfo Object | only available in resolve context +**childrenComplexity** | Selection field children complexity | only available in complexity context [For more details on expression syntax](http://symfony.com/doc/current/components/expression_language/syntax.html) diff --git a/Relay/Connection/ConnectionType.php b/Relay/Connection/ConnectionType.php index bffff0302..3bd3789b5 100644 --- a/Relay/Connection/ConnectionType.php +++ b/Relay/Connection/ConnectionType.php @@ -13,10 +13,10 @@ use GraphQL\Type\Definition\Config; use GraphQL\Type\Definition\FieldDefinition; -use GraphQL\Type\Definition\ObjectType; use GraphQL\Type\Definition\Type; use GraphQL\Utils; use Overblog\GraphQLBundle\Definition\MergeFieldTrait; +use Overblog\GraphQLBundle\Definition\ObjectType; class ConnectionType extends ObjectType { diff --git a/Relay/Connection/EdgeType.php b/Relay/Connection/EdgeType.php index 864407478..7a3c3bb82 100644 --- a/Relay/Connection/EdgeType.php +++ b/Relay/Connection/EdgeType.php @@ -11,7 +11,7 @@ namespace Overblog\GraphQLBundle\Relay\Connection; -use GraphQL\Type\Definition\ObjectType; +use Overblog\GraphQLBundle\Definition\ObjectType; class EdgeType extends ObjectType { diff --git a/Relay/Connection/PageInfoType.php b/Relay/Connection/PageInfoType.php index b9f10863b..a8306e686 100644 --- a/Relay/Connection/PageInfoType.php +++ b/Relay/Connection/PageInfoType.php @@ -11,8 +11,8 @@ namespace Overblog\GraphQLBundle\Relay\Connection; -use GraphQL\Type\Definition\ObjectType; -use GraphQL\Type\Definition\Type; +use Overblog\GraphQLBundle\Definition\ObjectType; +use Overblog\GraphQLBundle\Definition\Type; class PageInfoType extends ObjectType { diff --git a/Relay/Mutation/InputType.php b/Relay/Mutation/InputType.php index 0761cf6ca..14c9ee845 100644 --- a/Relay/Mutation/InputType.php +++ b/Relay/Mutation/InputType.php @@ -12,11 +12,11 @@ namespace Overblog\GraphQLBundle\Relay\Mutation; use GraphQL\Type\Definition\Config; -use GraphQL\Type\Definition\FieldDefinition; -use GraphQL\Type\Definition\InputObjectType; -use GraphQL\Type\Definition\Type; use GraphQL\Utils; +use Overblog\GraphQLBundle\Definition\FieldDefinition; +use Overblog\GraphQLBundle\Definition\InputObjectType; use Overblog\GraphQLBundle\Definition\MergeFieldTrait; +use Overblog\GraphQLBundle\Definition\Type; class InputType extends InputObjectType { diff --git a/Relay/Mutation/MutationField.php b/Relay/Mutation/MutationField.php index 4ac237777..2544331aa 100644 --- a/Relay/Mutation/MutationField.php +++ b/Relay/Mutation/MutationField.php @@ -12,11 +12,11 @@ namespace Overblog\GraphQLBundle\Relay\Mutation; use GraphQL\Type\Definition\Config; -use GraphQL\Type\Definition\Type; use GraphQL\Utils; use Overblog\GraphQLBundle\Definition\Argument; use Overblog\GraphQLBundle\Definition\Builder\MappingInterface; use Overblog\GraphQLBundle\Definition\MergeFieldTrait; +use Overblog\GraphQLBundle\Definition\Type; use Overblog\GraphQLBundle\Resolver\Resolver; class MutationField implements MappingInterface diff --git a/Relay/Mutation/PayloadType.php b/Relay/Mutation/PayloadType.php index a5423e9a0..1fce68290 100644 --- a/Relay/Mutation/PayloadType.php +++ b/Relay/Mutation/PayloadType.php @@ -12,11 +12,11 @@ namespace Overblog\GraphQLBundle\Relay\Mutation; use GraphQL\Type\Definition\Config; -use GraphQL\Type\Definition\FieldDefinition; -use GraphQL\Type\Definition\ObjectType; -use GraphQL\Type\Definition\Type; use GraphQL\Utils; +use Overblog\GraphQLBundle\Definition\FieldDefinition; use Overblog\GraphQLBundle\Definition\MergeFieldTrait; +use Overblog\GraphQLBundle\Definition\ObjectType; +use Overblog\GraphQLBundle\Definition\Type; class PayloadType extends ObjectType { diff --git a/Relay/Node/GlobalIdField.php b/Relay/Node/GlobalIdField.php index ffa6a48d6..6bde8735e 100644 --- a/Relay/Node/GlobalIdField.php +++ b/Relay/Node/GlobalIdField.php @@ -13,8 +13,8 @@ use GraphQL\Type\Definition\Config; use GraphQL\Type\Definition\ResolveInfo; -use GraphQL\Type\Definition\Type; use Overblog\GraphQLBundle\Definition\Builder\MappingInterface; +use Overblog\GraphQLBundle\Definition\Type; use Overblog\GraphQLBundle\Resolver\Resolver; class GlobalIdField implements MappingInterface diff --git a/Relay/Node/NodeField.php b/Relay/Node/NodeField.php index 2f595ece4..80d5efe3c 100644 --- a/Relay/Node/NodeField.php +++ b/Relay/Node/NodeField.php @@ -12,8 +12,8 @@ namespace Overblog\GraphQLBundle\Relay\Node; use GraphQL\Type\Definition\Config; -use GraphQL\Type\Definition\Type; use Overblog\GraphQLBundle\Definition\Builder\MappingInterface; +use Overblog\GraphQLBundle\Definition\Type; class NodeField implements MappingInterface { diff --git a/Relay/Node/NodeInterfaceType.php b/Relay/Node/NodeInterfaceType.php index d83e6691e..0a0f72247 100644 --- a/Relay/Node/NodeInterfaceType.php +++ b/Relay/Node/NodeInterfaceType.php @@ -12,8 +12,8 @@ namespace Overblog\GraphQLBundle\Relay\Node; use GraphQL\Type\Definition\Config; -use GraphQL\Type\Definition\InterfaceType; -use GraphQL\Type\Definition\Type; +use Overblog\GraphQLBundle\Definition\InterfaceType; +use Overblog\GraphQLBundle\Definition\Type; class NodeInterfaceType extends InterfaceType { diff --git a/Relay/Node/PluralIdentifyingRootField.php b/Relay/Node/PluralIdentifyingRootField.php index 67d1bc09b..16ebbfb02 100644 --- a/Relay/Node/PluralIdentifyingRootField.php +++ b/Relay/Node/PluralIdentifyingRootField.php @@ -12,8 +12,8 @@ namespace Overblog\GraphQLBundle\Relay\Node; use GraphQL\Type\Definition\Config; -use GraphQL\Type\Definition\Type; use Overblog\GraphQLBundle\Definition\Builder\MappingInterface; +use Overblog\GraphQLBundle\Definition\Type; class PluralIdentifyingRootField implements MappingInterface { diff --git a/Request/Validator/Rule/AbstractQuerySecurity.php b/Request/Validator/Rule/AbstractQuerySecurity.php index 8275b2077..e412d1d59 100644 --- a/Request/Validator/Rule/AbstractQuerySecurity.php +++ b/Request/Validator/Rule/AbstractQuerySecurity.php @@ -18,7 +18,7 @@ use GraphQL\Language\AST\Node; use GraphQL\Language\AST\SelectionSet; use GraphQL\Type\Definition\Type; -use GraphQL\Type\Definition\WrappingType; +use GraphQL\Type\Introspection; use GraphQL\Utils\TypeInfo; use GraphQL\Validator\ValidationContext; @@ -29,9 +29,6 @@ abstract class AbstractQuerySecurity /** @var FragmentDefinition[] */ private $fragments = []; - /** @var Type[] */ - private $rootTypes = []; - /** * @return \GraphQL\Language\AST\FragmentDefinition[] */ @@ -40,14 +37,6 @@ protected function getFragments() return $this->fragments; } - /** - * @return \GraphQL\Type\Definition\Type[] - */ - protected function getRootTypes() - { - return $this->rootTypes; - } - /** * check if equal to 0 no check is done. Must be greater or equal to 0. * @@ -60,28 +49,6 @@ protected static function checkIfGreaterOrEqualToZero($name, $value) } } - protected function isParentRootType(ValidationContext $context) - { - $parentType = $context->getParentType(); - $isParentRootType = $parentType && in_array($parentType, $this->getRootTypes()); - - return $isParentRootType; - } - - protected function isIntrospectionType(ValidationContext $context) - { - $type = $this->retrieveCurrentType($context); - $isIntrospectionType = $type && $type->name === '__Schema'; - - return $isIntrospectionType; - } - - protected function gatherRootTypes(ValidationContext $context) - { - $schema = $context->getSchema(); - $this->rootTypes = [$schema->getQueryType(), $schema->getMutationType(), $schema->getSubscriptionType()]; - } - protected function gatherFragmentDefinition(ValidationContext $context) { // Gather all the fragment definition. @@ -94,17 +61,6 @@ protected function gatherFragmentDefinition(ValidationContext $context) } } - protected function retrieveCurrentType(ValidationContext $context) - { - $type = $context->getType(); - - if ($type instanceof WrappingType) { - $type = $type->getWrappedType(true); - } - - return $type; - } - protected function getFragment(FragmentSpread $fragmentSpread) { $spreadName = $fragmentSpread->name->value; @@ -116,7 +72,6 @@ protected function getFragment(FragmentSpread $fragmentSpread) protected function invokeIfNeeded(ValidationContext $context, array $validators) { $this->gatherFragmentDefinition($context); - $this->gatherRootTypes($context); // is disabled? if (!$this->isEnabled()) { @@ -156,7 +111,17 @@ protected function collectFieldASTsAndDefs(ValidationContext $context, $parentTy $fieldDef = null; if ($parentType && method_exists($parentType, 'getFields')) { $tmp = $parentType->getFields(); - if (isset($tmp[$fieldName])) { + $schemaMetaFieldDef = Introspection::schemaMetaFieldDef(); + $typeMetaFieldDef = Introspection::typeMetaFieldDef(); + $typeNameMetaFieldDef = Introspection::typeNameMetaFieldDef(); + + if ($fieldName === $schemaMetaFieldDef->name && $context->getSchema()->getQueryType() === $parentType) { + $fieldDef = $schemaMetaFieldDef; + } elseif ($fieldName === $typeMetaFieldDef->name && $context->getSchema()->getQueryType() === $parentType) { + $fieldDef = $typeMetaFieldDef; + } elseif ($fieldName === $typeNameMetaFieldDef->name) { + $fieldDef = $typeNameMetaFieldDef; + } elseif (isset($tmp[$fieldName])) { $fieldDef = $tmp[$fieldName]; } } @@ -164,7 +129,8 @@ protected function collectFieldASTsAndDefs(ValidationContext $context, $parentTy if (!isset($_astAndDefs[$responseName])) { $_astAndDefs[$responseName] = new \ArrayObject(); } - $_astAndDefs[$responseName][] = [$selection, $fieldDef, $context]; + // create field context + $_astAndDefs[$responseName][] = [$selection, $fieldDef]; break; case Node::INLINE_FRAGMENT: /* @var InlineFragment $inlineFragment */ @@ -179,21 +145,21 @@ protected function collectFieldASTsAndDefs(ValidationContext $context, $parentTy case Node::FRAGMENT_SPREAD: /* @var FragmentSpread $selection */ $fragName = $selection->name->value; - if (!empty($_visitedFragmentNames[$fragName])) { - continue; - } - $_visitedFragmentNames[$fragName] = true; - $fragment = $context->getFragment($fragName); - if (!$fragment) { - continue; + + if (empty($_visitedFragmentNames[$fragName])) { + $_visitedFragmentNames[$fragName] = true; + $fragment = $context->getFragment($fragName); + + if ($fragment) { + $_astAndDefs = $this->collectFieldASTsAndDefs( + $context, + TypeInfo::typeFromAST($context->getSchema(), $fragment->typeCondition), + $fragment->selectionSet, + $_visitedFragmentNames, + $_astAndDefs + ); + } } - $_astAndDefs = $this->collectFieldASTsAndDefs( - $context, - TypeInfo::typeFromAST($context->getSchema(), $fragment->typeCondition), - $fragment->selectionSet, - $_visitedFragmentNames, - $_astAndDefs - ); break; } } @@ -201,7 +167,7 @@ protected function collectFieldASTsAndDefs(ValidationContext $context, $parentTy return $_astAndDefs; } - protected function getFieldName(Node $node) + protected function getFieldName(Field $node) { $fieldName = $node->name->value; $responseName = $node->alias ? $node->alias->value : $fieldName; diff --git a/Request/Validator/Rule/QueryComplexity.php b/Request/Validator/Rule/QueryComplexity.php index 80b0d9b19..9275a0adf 100644 --- a/Request/Validator/Rule/QueryComplexity.php +++ b/Request/Validator/Rule/QueryComplexity.php @@ -30,9 +30,6 @@ class QueryComplexity extends AbstractQuerySecurity private static $rawVariableValues = []; - /** @var callable[] */ - private static $complexityCalculators = []; - private $variableDefs; private $fieldAstAndDefs; @@ -52,26 +49,6 @@ public static function maxQueryComplexityErrorMessage($max, $count) return sprintf('Max query complexity should be %d but got %d.', $max, $count); } - public static function addComplexityCalculator($name, callable $complexityCalculator) - { - self::$complexityCalculators[$name] = $complexityCalculator; - } - - public static function hasComplexityCalculator($name) - { - return isset(self::$complexityCalculators[$name]); - } - - public static function getComplexityCalculator($name) - { - return static::hasComplexityCalculator($name) ? self::$complexityCalculators[$name] : null; - } - - public static function removeComplexityCalculator($name) - { - unset(self::$complexityCalculators[$name]); - } - /** * Set max query complexity. If equal to 0 no check is done. Must be greater or equal to 0. * @@ -81,7 +58,7 @@ public static function setMaxQueryComplexity($maxQueryComplexity) { self::checkIfGreaterOrEqualToZero('maxQueryComplexity', $maxQueryComplexity); - self::$maxQueryComplexity = (int)$maxQueryComplexity; + self::$maxQueryComplexity = (int) $maxQueryComplexity; } public static function getMaxQueryComplexity() @@ -127,70 +104,63 @@ public function __invoke(ValidationContext $context) return Visitor::skipNode(); }, Node::OPERATION_DEFINITION => [ - 'leave' => function (OperationDefinition $operationDefinition) use ($context, $complexity) { - // check complexity only on first rootTypes children and ignore check on introspection query - //if (!$this->isIntrospectionType($context)) { - $type = $context->getType(); - $complexity = $this->fieldComplexity($operationDefinition, $type->name, $complexity); - - if ($complexity > $this->getMaxQueryComplexity()) { - return new Error($this->maxQueryComplexityErrorMessage($this->getMaxQueryComplexity(), $complexity)); - } - //} + 'leave' => function (OperationDefinition $operationDefinition) use ($context, &$complexity) { + $complexity = $this->fieldComplexity($operationDefinition, $complexity); + + if ($complexity > $this->getMaxQueryComplexity()) { + return new Error($this->maxQueryComplexityErrorMessage($this->getMaxQueryComplexity(), $complexity)); + } }, ], ] ); } - private function fieldComplexity(Node $node, $typeName, $complexity = 0) + private function fieldComplexity(Node $node, $complexity = 0) { - if (!isset($node->selectionSet)) { - return $complexity; - } - - foreach ($node->selectionSet->selections as $childNode) { - $complexity = $this->nodeComplexity($childNode, $typeName, $complexity); + if (isset($node->selectionSet)) { + foreach ($node->selectionSet->selections as $childNode) { + $complexity = $this->nodeComplexity($childNode, $complexity); + } } return $complexity; } - private function nodeComplexity(Node $node, $typeName, $complexity = 0) + private function nodeComplexity(Node $node, $complexity = 0) { switch ($node->kind) { case Node::FIELD: + // default values + $args = []; + $complexityFn = 'Overblog\GraphQLBundle\Definition\FieldDefinition::defaultComplexity'; + // calculate children complexity if needed $childrenComplexity = 0; - $astFieldInfo = $this->astFieldInfo($node); - /** @var ValidationContext|null $fieldValidationContext */ - $fieldValidationContext = $astFieldInfo[2]; - /** @var FieldDefinition|null $fieldDef */ - $fieldDef = $astFieldInfo[1]; - // node has children? if (isset($node->selectionSet)) { - $type = $fieldDef->getType(); - $childrenComplexity = $this->fieldComplexity($node, $type->name); + $childrenComplexity = $this->fieldComplexity($node); } - // default complexity calculator - $complexity = $complexity + $childrenComplexity + 1; + $astFieldInfo = $this->astFieldInfo($node); + $fieldDef = $astFieldInfo[1]; - $complexityCalculatorName = $typeName . '.' . $this->getFieldName($node); - // custom complexity is set ? - if (null !== $complexityCalculator = static::getComplexityCalculator($complexityCalculatorName)) { + if ($fieldDef instanceof FieldDefinition) { $args = $this->buildFieldArguments($node); - //get field complexity using custom complexityCalculator - $complexity = call_user_func_array($complexityCalculator, [$fieldValidationContext, $args, $childrenComplexity]); + //get complexity fn using fieldDef complexity + if (method_exists($fieldDef, 'getComplexityFn')) { + $complexityFn = $fieldDef->getComplexityFn(); + } } + + $complexity += call_user_func_array($complexityFn, [$childrenComplexity, $args]); break; case Node::INLINE_FRAGMENT: // node has children? if (isset($node->selectionSet)) { - $complexity = $this->fieldComplexity($node, $typeName, $complexity); + $complexity = $this->fieldComplexity($node, $complexity); } break; @@ -198,7 +168,7 @@ private function nodeComplexity(Node $node, $typeName, $complexity = 0) $fragment = $this->getFragment($node); if (null !== $fragment) { - $complexity = $this->fieldComplexity($fragment, $typeName, $complexity); + $complexity = $this->fieldComplexity($fragment, $complexity); } break; } @@ -209,7 +179,7 @@ private function nodeComplexity(Node $node, $typeName, $complexity = 0) private function astFieldInfo(Field $field) { $fieldName = $this->getFieldName($field); - $astFieldInfo = [null, null, null]; + $astFieldInfo = [null, null]; if (isset($this->fieldAstAndDefs[$fieldName])) { foreach ($this->fieldAstAndDefs[$fieldName] as $astAndDef) { if ($astAndDef[0] == $field) { @@ -228,16 +198,16 @@ private function buildFieldArguments(Field $node) $astFieldInfo = $this->astFieldInfo($node); $fieldDef = $astFieldInfo[1]; - if (!$fieldDef instanceof FieldDefinition) { - return; - } + $args = []; - $variableValues = Values::getVariableValues( - $this->context->getSchema(), - $this->variableDefs, - $rawVariableValues - ); - $args = Values::getArgumentValues($fieldDef->args, $node->arguments, $variableValues); + if ($fieldDef instanceof FieldDefinition) { + $variableValues = Values::getVariableValues( + $this->context->getSchema(), + $this->variableDefs, + $rawVariableValues + ); + $args = Values::getArgumentValues($fieldDef->args, $node->arguments, $variableValues); + } return $args; } diff --git a/Request/Validator/Rule/QueryDepth.php b/Request/Validator/Rule/QueryDepth.php index 6a85b5eea..c87a4e779 100644 --- a/Request/Validator/Rule/QueryDepth.php +++ b/Request/Validator/Rule/QueryDepth.php @@ -14,6 +14,7 @@ use GraphQL\Error; use GraphQL\Language\AST\Field; use GraphQL\Language\AST\Node; +use GraphQL\Language\AST\OperationDefinition; use GraphQL\Language\AST\SelectionSet; use GraphQL\Validator\ValidationContext; @@ -58,16 +59,15 @@ public function __invoke(ValidationContext $context) return $this->invokeIfNeeded( $context, [ - Node::FIELD => function (Field $node) use ($context) { - // check depth only on first rootTypes children and ignore check on introspection query - if ($this->isParentRootType($context) && !$this->isIntrospectionType($context)) { - $maxDepth = $this->nodeMaxDepth($node); + Node::OPERATION_DEFINITION => [ + 'leave' => function (OperationDefinition $operationDefinition) use ($context) { + $maxDepth = $this->fieldDepth($operationDefinition); if ($maxDepth > $this->getMaxQueryDepth()) { - return new Error($this->maxQueryDepthErrorMessage($this->getMaxQueryDepth(), $maxDepth), [$node]); + return new Error($this->maxQueryDepthErrorMessage($this->getMaxQueryDepth(), $maxDepth)); } - } - }, + }, + ], ] ); } @@ -77,40 +77,45 @@ protected function isEnabled() return $this->getMaxQueryDepth() !== static::DISABLED; } - private function nodeMaxDepth(Node $node, $depth = 1, $maxDepth = 1) + private function fieldDepth(Node $node, $depth = 0, $maxDepth = 0) { - if (!isset($node->selectionSet)) { - return $maxDepth; + if (isset($node->selectionSet)) { + foreach ($node->selectionSet->selections as $childNode) { + $maxDepth = $this->nodeDepth($childNode, $depth, $maxDepth); + } } - foreach ($node->selectionSet->selections as $childNode) { - switch ($childNode->kind) { - case Node::FIELD: - // node has children? - if (null !== $childNode->selectionSet) { - // update maxDepth if needed - if ($depth > $maxDepth) { - $maxDepth = $depth; - } - $maxDepth = $this->nodeMaxDepth($childNode, $depth + 1, $maxDepth); - } - break; - - case Node::INLINE_FRAGMENT: - // node has children? - if (null !== $childNode->selectionSet) { - $maxDepth = $this->nodeMaxDepth($childNode, $depth, $maxDepth); - } - break; - - case Node::FRAGMENT_SPREAD: - $fragment = $this->getFragment($childNode); + return $maxDepth; + } - if (null !== $fragment) { - $maxDepth = $this->nodeMaxDepth($fragment, $depth, $maxDepth); + private function nodeDepth(Node $node, $depth = 0, $maxDepth = 0) + { + switch ($node->kind) { + case Node::FIELD: + // node has children? + if (null !== $node->selectionSet) { + // update maxDepth if needed + if ($depth > $maxDepth) { + $maxDepth = $depth; } - break; - } + $maxDepth = $this->fieldDepth($node, $depth + 1, $maxDepth); + } + break; + + case Node::INLINE_FRAGMENT: + // node has children? + if (null !== $node->selectionSet) { + $maxDepth = $this->fieldDepth($node, $depth, $maxDepth); + } + break; + + case Node::FRAGMENT_SPREAD: + $fragment = $this->getFragment($node); + + if (null !== $fragment) { + $maxDepth = $this->fieldDepth($fragment, $depth, $maxDepth); + } + break; } return $maxDepth; diff --git a/Resolver/Config/FieldsConfigSolution.php b/Resolver/Config/FieldsConfigSolution.php index 818b5b94b..2ee9bec6c 100644 --- a/Resolver/Config/FieldsConfigSolution.php +++ b/Resolver/Config/FieldsConfigSolution.php @@ -16,7 +16,6 @@ use Overblog\GraphQLBundle\Error\UserError; use Overblog\GraphQLBundle\Relay\Connection\Output\Connection; use Overblog\GraphQLBundle\Relay\Connection\Output\Edge; -use Overblog\GraphQLBundle\Request\Validator\Rule\QueryComplexity; use Overblog\GraphQLBundle\Resolver\ResolverInterface; class FieldsConfigSolution extends AbstractConfigSolution @@ -63,34 +62,27 @@ public function solve($values, array &$config = null) return $values; } - private function solveComplexity($options, $field, array $config) + private function solveComplexity($options, $field) { if (!isset($options['complexity'])) { return $options; } $treatedOptions = $options; - $name = $config['name'].'.'.$field; $value = $treatedOptions['complexity']; - QueryComplexity::addComplexityCalculator( - $name, - function () use ($value) { - $args = func_get_args(); - $complexity = $this->solveUsingExpressionLanguageIfNeeded( - $value, - [ - 'validationContext' => $args[0], - 'args' => new Argument($args[1]), - 'childrenComplexity' => $args[2], - ] - ); - - return (int) $complexity; - } - ); - - unset($treatedOptions['complexity']); + $treatedOptions['complexity'] = function () use ($value) { + $args = func_get_args(); + $complexity = $this->solveUsingExpressionLanguageIfNeeded( + $value, + [ + 'childrenComplexity' => $args[0], + 'args' => new Argument($args[1]), + ] + ); + + return (int) $complexity; + }; return $treatedOptions; } diff --git a/Tests/Request/Validator/Rule/AbstractQuerySecurityTest.php b/Tests/Request/Validator/Rule/AbstractQuerySecurityTest.php index 69abf49de..41e1a0efe 100644 --- a/Tests/Request/Validator/Rule/AbstractQuerySecurityTest.php +++ b/Tests/Request/Validator/Rule/AbstractQuerySecurityTest.php @@ -13,7 +13,6 @@ use GraphQL\FormattedError; use GraphQL\Language\Parser; -use GraphQL\Language\SourceLocation; use GraphQL\Type\Introspection; use GraphQL\Validator\DocumentValidator; use Overblog\GraphQLBundle\Request\Validator\Rule\AbstractQuerySecurity; @@ -35,73 +34,72 @@ abstract protected function createRule($max); */ abstract protected function getErrorMessage($max, $count); - public function testIgnoreIntrospectionQuery() + /** + * @expectedException \InvalidArgumentException + * @expectedExceptionMessage argument must be greater or equal to 0. + */ + public function testMaxQueryDepthMustBeGreaterOrEqualTo0() { - $this->assertDocumentValidator(Introspection::getIntrospectionQuery(true), 1); + $this->createRule(-1); } - protected function createFormattedError($max, $count) + protected function createFormattedError($max, $count, $locations = []) { - return FormattedError::create($this->getErrorMessage($max, $count), [new SourceLocation(1, 17)]); + return FormattedError::create($this->getErrorMessage($max, $count), $locations); } - protected function buildRecursiveQuery($depth) + protected function assertDocumentValidator($queryString, $max, array $expectedErrors = []) { - $query = sprintf('query MyQuery { human%s }', $this->buildRecursiveQueryPart($depth)); + $errors = DocumentValidator::validate( + Schema::buildSchema(), + Parser::parse($queryString), + [$this->createRule($max)] + ); - return $query; + $this->assertEquals($expectedErrors, array_map(['GraphQL\Error', 'formatError'], $errors), $queryString); + + return $errors; } - protected function buildRecursiveUsingFragmentQuery($depth) + protected function assertIntrospectionQuery($maxExpected) { - $query = sprintf( - 'query MyQuery { human { ...F1 } } fragment F1 on Human %s', - $this->buildRecursiveQueryPart($depth) - ); + $query = Introspection::getIntrospectionQuery(true); - return $query; + $this->assertMaxValue($query, $maxExpected); } - protected function buildRecursiveUsingInlineFragmentQuery($depth) + protected function assertIntrospectionTypeMetaFieldQuery($maxExpected) { - $query = sprintf( - 'query MyQuery { human { ...on Human %s } }', - $this->buildRecursiveQueryPart($depth) - ); - - return $query; + $query = ' + { + __type(name: "Human") { + name + } + } + '; + + $this->assertMaxValue($query, $maxExpected); } - protected function buildRecursiveQueryPart($depth) + protected function assertTypeNameMetaFieldQuery($maxExpected) { - $templates = [ - 'human' => ' { firstName%s } ', - 'dog' => ' dog { name%s } ', - ]; - - $part = $templates['human']; - - for ($i = 1; $i <= $depth; ++$i) { - $key = ($i % 2 == 1) ? 'human' : 'dog'; - $template = $templates[$key]; - - $part = sprintf($part, ('human' == $key ? ' owner ' : '').$template); - } - $part = str_replace('%s', '', $part); - - return $part; + $query = ' + { + human { + __typename + firstName + } + } + '; + $this->assertMaxValue($query, $maxExpected); } - protected function assertDocumentValidator($queryString, $max, array $expectedErrors = []) + protected function assertMaxValue($query, $maxExpected) { - $errors = DocumentValidator::validate( - Schema::buildSchema(), - Parser::parse($queryString), - [$this->createRule($max)] - ); - - $this->assertEquals($expectedErrors, array_map(['GraphQL\Error', 'formatError'], $errors), $queryString); - - return $errors; + $this->assertDocumentValidator($query, $maxExpected); + $newMax = $maxExpected - 1; + if ($newMax !== AbstractQuerySecurity::DISABLED) { + $this->assertDocumentValidator($query, $newMax, [$this->createFormattedError($newMax, $maxExpected)]); + } } } diff --git a/Tests/Request/Validator/Rule/QueryComplexityTest.php b/Tests/Request/Validator/Rule/QueryComplexityTest.php index 8490fd11f..5fb611f6f 100644 --- a/Tests/Request/Validator/Rule/QueryComplexityTest.php +++ b/Tests/Request/Validator/Rule/QueryComplexityTest.php @@ -35,4 +35,83 @@ protected function createRule($maxDepth) { return new QueryComplexity($maxDepth); } + + public function testSimpleQueries() + { + $query = 'query MyQuery { human { firstName } }'; + + $this->assertDocumentValidators($query, 2, 3); + } + + public function testInlineFragmentQueries() + { + $query = 'query MyQuery { human { ... on Human { firstName } } }'; + + $this->assertDocumentValidators($query, 2, 3); + } + + public function testFragmentQueries() + { + $query = 'query MyQuery { human { ...F1 } } fragment F1 on Human { firstName}'; + + $this->assertDocumentValidators($query, 2, 3); + } + + public function testAliasesQueries() + { + $query = 'query MyQuery { thomas: human(name: "Thomas") { firstName } jeremy: human(name: "Jeremy") { firstName } }'; + + $this->assertDocumentValidators($query, 4, 5); + } + + public function testCustomComplexityQueries() + { + $query = 'query MyQuery { human { dogs { name } } }'; + + $this->assertDocumentValidators($query, 12, 13); + } + + public function testCustomComplexityWithArgsQueries() + { + $query = 'query MyQuery { human { dogs(name: "Root") { name } } }'; + + $this->assertDocumentValidators($query, 3, 4); + } + + public function testCustomComplexityWithVariablesQueries() + { + $query = 'query MyQuery($dog: String!) { human { dogs(name: $dog) { name } } }'; + + QueryComplexity::setRawVariableValues(['dog' => 'Roots']); + + $this->assertDocumentValidators($query, 3, 4); + } + + public function testComplexityIntrospectionQuery() + { + $this->assertIntrospectionQuery(109); + } + + public function testIntrospectionTypeMetaFieldQuery() + { + $this->assertIntrospectionTypeMetaFieldQuery(2); + } + + public function testTypeNameMetaFieldQuery() + { + $this->assertTypeNameMetaFieldQuery(3); + } + + private function assertDocumentValidators($query, $queryComplexity, $startComplexity) + { + for ($maxComplexity = $startComplexity; $maxComplexity >= 0; --$maxComplexity) { + $positions = []; + + if ($maxComplexity < $queryComplexity && $maxComplexity !== QueryComplexity::DISABLED) { + $positions = [$this->createFormattedError($maxComplexity, $queryComplexity)]; + } + + $this->assertDocumentValidator($query, $maxComplexity, $positions); + } + } } diff --git a/Tests/Request/Validator/Rule/QueryDepthTest.php b/Tests/Request/Validator/Rule/QueryDepthTest.php index b31eb5fc1..0e5c56249 100644 --- a/Tests/Request/Validator/Rule/QueryDepthTest.php +++ b/Tests/Request/Validator/Rule/QueryDepthTest.php @@ -69,13 +69,19 @@ public function testInlineFragmentQueries($queryDepth, $maxQueryDepth = 7, $expe $this->assertDocumentValidator($this->buildRecursiveUsingInlineFragmentQuery($queryDepth), $maxQueryDepth, $expectedErrors); } - /** - * @expectedException \InvalidArgumentException - * @expectedExceptionMessage $maxQueryDepth argument must be greater or equal to 0. - */ - public function testMaxQueryDepthMustBeGreaterOrEqualTo0() + public function testComplexityIntrospectionQuery() + { + $this->assertIntrospectionQuery(7); + } + + public function testIntrospectionTypeMetaFieldQuery() + { + $this->assertIntrospectionTypeMetaFieldQuery(1); + } + + public function testTypeNameMetaFieldQuery() { - $this->createRule(-1); + $this->assertTypeNameMetaFieldQuery(1); } public function queryDataProvider() @@ -102,4 +108,51 @@ public function queryDataProvider() ], // failed because depth over limit (20) ]; } + + private function buildRecursiveQuery($depth) + { + $query = sprintf('query MyQuery { human%s }', $this->buildRecursiveQueryPart($depth)); + + return $query; + } + + private function buildRecursiveUsingFragmentQuery($depth) + { + $query = sprintf( + 'query MyQuery { human { ...F1 } } fragment F1 on Human %s', + $this->buildRecursiveQueryPart($depth) + ); + + return $query; + } + + private function buildRecursiveUsingInlineFragmentQuery($depth) + { + $query = sprintf( + 'query MyQuery { human { ...on Human %s } }', + $this->buildRecursiveQueryPart($depth) + ); + + return $query; + } + + private function buildRecursiveQueryPart($depth) + { + $templates = [ + 'human' => ' { firstName%s } ', + 'dog' => ' dogs { name%s } ', + ]; + + $part = $templates['human']; + + for ($i = 1; $i <= $depth; ++$i) { + $key = ($i % 2 == 1) ? 'human' : 'dog'; + $template = $templates[$key]; + + $part = sprintf($part, ('human' == $key ? ' owner ' : '').$template); + } + $part = str_replace('%s', '', $part); + + return $part; + } } diff --git a/Tests/Request/Validator/Rule/Schema.php b/Tests/Request/Validator/Rule/Schema.php index 5c9a03f48..02bf5fa9d 100644 --- a/Tests/Request/Validator/Rule/Schema.php +++ b/Tests/Request/Validator/Rule/Schema.php @@ -12,8 +12,8 @@ namespace Overblog\GraphQLBundle\Tests\Request\Validator\Rule; use GraphQL\Schema as GraphQLSchema; -use GraphQL\Type\Definition\ObjectType; -use GraphQL\Type\Definition\Type; +use Overblog\GraphQLBundle\Definition\ObjectType; +use Overblog\GraphQLBundle\Definition\Type; class Schema { @@ -34,9 +34,6 @@ public static function buildSchema() return self::$schema; } - static::buildHumanType(); - static::buildDogType(); - self::$schema = new GraphQLSchema(static::buildQueryRootType()); return self::$schema; @@ -53,6 +50,7 @@ public static function buildQueryRootType() 'fields' => [ 'human' => [ 'type' => self::buildHumanType(), + 'args' => ['name' => ['type' => Type::string()]], ], ], ]); @@ -71,7 +69,7 @@ public static function buildHumanType() 'name' => 'Human', 'fields' => [ 'firstName' => ['type' => Type::nonNull(Type::string())], - 'Dog' => [ + 'dogs' => [ 'type' => function () { return Type::nonNull( Type::listOf( @@ -79,6 +77,12 @@ public static function buildHumanType() ) ); }, + 'complexity' => function ($childrenComplexity, $args) { + $complexity = isset($args['name']) ? 1 : 10; + + return $childrenComplexity + $complexity; + }, + 'args' => ['name' => ['type' => Type::string()]], ], ], ] From 9e8b6b00a9e66cdc726c5be8c51e955ed2b564bc Mon Sep 17 00:00:00 2001 From: Jeremiah VALERIE Date: Sat, 2 Apr 2016 20:50:07 +0200 Subject: [PATCH 6/7] add access and complexity functional tests --- .scrutinizer.yml | 2 +- .travis.yml | 2 +- Definition/FieldDefinition.php | 2 +- Definition/FieldsTrait.php | 54 ------ Definition/ObjectType.php | 50 +++++- Resolver/Config/FieldsConfigSolution.php | 3 - Tests/Functional/Security/AccessTest.php | 170 ++++++++++++++++++ .../Security/QueryComplexityTest.php | 88 +++++++++ Tests/Functional/app/AppKernel.php | 1 + .../app/Resolver/ConnectionResolver.php | 2 +- Tests/Functional/app/config/access/config.yml | 37 ++++ .../config/access/mapping/access.types.yml | 15 ++ .../app/config/connection/config.yml | 10 +- .../app/config/connection/services.yml | 8 + .../app/config/queryComplexity/config.yml | 19 ++ .../connectionWithComplexity.types.yml | 10 ++ .../Request/Validator/Rule/QueryDepthTest.php | 6 +- composer.json | 2 +- 18 files changed, 404 insertions(+), 77 deletions(-) delete mode 100644 Definition/FieldsTrait.php create mode 100644 Tests/Functional/Security/AccessTest.php create mode 100644 Tests/Functional/Security/QueryComplexityTest.php create mode 100644 Tests/Functional/app/config/access/config.yml create mode 100644 Tests/Functional/app/config/access/mapping/access.types.yml create mode 100644 Tests/Functional/app/config/connection/services.yml create mode 100644 Tests/Functional/app/config/queryComplexity/config.yml create mode 100644 Tests/Functional/app/config/queryComplexity/mapping/connectionWithComplexity.types.yml diff --git a/.scrutinizer.yml b/.scrutinizer.yml index 8d10bf710..8f5365c52 100644 --- a/.scrutinizer.yml +++ b/.scrutinizer.yml @@ -1,6 +1,6 @@ tools: external_code_coverage: - timeout: 600 + timeout: 1200 build: tests: override: diff --git a/.travis.yml b/.travis.yml index 2cee515b9..98ad9656a 100644 --- a/.travis.yml +++ b/.travis.yml @@ -35,7 +35,7 @@ before_install: install: composer update --prefer-dist --no-interaction -script: if [ "$TRAVIS_PHP_VERSION" == "5.6" ]; then php -d xdebug.max_nesting_level=1000 vendor/bin/phpunit --coverage-clover=coverage.clover; else vendor/bin/phpunit; fi +script: if [ "$TRAVIS_PHP_VERSION" == "5.6" ]; then php -d xdebug.max_nesting_level=1000 vendor/bin/phpunit --debug --coverage-clover=coverage.clover; else vendor/bin/phpunit --debug; fi after_script: - if [ "$TRAVIS_PHP_VERSION" == "5.6" ]; then wget https://scrutinizer-ci.com/ocular.phar && php ocular.phar code-coverage:upload --format=php-clover coverage.clover; fi diff --git a/Definition/FieldDefinition.php b/Definition/FieldDefinition.php index 551712696..38c28b5fe 100644 --- a/Definition/FieldDefinition.php +++ b/Definition/FieldDefinition.php @@ -45,7 +45,7 @@ public static function createMap(array $fields) } /** - * @param array|Config $field + * @param array $field * * @return FieldDefinition */ diff --git a/Definition/FieldsTrait.php b/Definition/FieldsTrait.php deleted file mode 100644 index 009280352..000000000 --- a/Definition/FieldsTrait.php +++ /dev/null @@ -1,54 +0,0 @@ - - * - * For the full copyright and license information, please view the LICENSE - * file that was distributed with this source code. - */ - -namespace Overblog\GraphQLBundle\Definition; - -use GraphQL\Type\Definition\Config; -use GraphQL\Utils; - -trait FieldsTrait -{ - /** - * @var FieldDefinition[] - */ - private $_fields; - - /** - * @return FieldDefinition[] - */ - public function getFields() - { - if (null === $this->_fields) { - $fields = isset($this->config['fields']) ? $this->config['fields'] : []; - $fields = is_callable($fields) ? call_user_func($fields) : $fields; - $this->_fields = FieldDefinition::createMap($fields); - } - - return $this->_fields; - } - - /** - * @param string $name - * - * @return FieldDefinition - * - * @throws \Exception - */ - public function getField($name) - { - if (null === $this->_fields) { - $this->getFields(); - } - Utils::invariant(isset($this->_fields[$name]), "Field '%s' is not defined for type '%s'", $name, $this->name); - - return $this->_fields[$name]; - } -} diff --git a/Definition/ObjectType.php b/Definition/ObjectType.php index f603fce17..46353c7bd 100644 --- a/Definition/ObjectType.php +++ b/Definition/ObjectType.php @@ -13,13 +13,17 @@ use GraphQL\Type\Definition\Config; use GraphQL\Type\Definition\ObjectType as BaseObjectType; +use GraphQL\Type\Definition\ResolveInfo; use GraphQL\Utils; class ObjectType extends BaseObjectType { - use FieldsTrait; + private $isTypeOf; - private $_isTypeOf; + /** + * @var FieldDefinition[] + */ + private $fields; /** * @param array $config @@ -48,11 +52,51 @@ public function __construct(array $config) $this->name = $config['name']; $this->description = isset($config['description']) ? $config['description'] : null; $this->resolveFieldFn = isset($config['resolveField']) ? $config['resolveField'] : null; - $this->_isTypeOf = isset($config['isTypeOf']) ? $config['isTypeOf'] : null; + $this->isTypeOf = isset($config['isTypeOf']) ? $config['isTypeOf'] : null; $this->config = $config; if (isset($config['interfaces'])) { InterfaceType::addImplementationToInterfaces($this); } } + + /** + * @return FieldDefinition[] + */ + public function getFields() + { + if (null === $this->fields) { + $fields = isset($this->config['fields']) ? $this->config['fields'] : []; + $fields = is_callable($fields) ? call_user_func($fields) : $fields; + $this->fields = FieldDefinition::createMap($fields); + } + + return $this->fields; + } + + /** + * @param string $name + * + * @return FieldDefinition + * + * @throws \Exception + */ + public function getField($name) + { + if (null === $this->fields) { + $this->getFields(); + } + Utils::invariant(isset($this->fields[$name]), "Field '%s' is not defined for type '%s'", $name, $this->name); + + return $this->fields[$name]; + } + + /** + * @param $value + * @return bool|null + */ + public function isTypeOf($value, ResolveInfo $info) + { + return isset($this->isTypeOf) ? call_user_func($this->isTypeOf, $value, $info) : null; + } } diff --git a/Resolver/Config/FieldsConfigSolution.php b/Resolver/Config/FieldsConfigSolution.php index 2ee9bec6c..41b014439 100644 --- a/Resolver/Config/FieldsConfigSolution.php +++ b/Resolver/Config/FieldsConfigSolution.php @@ -64,9 +64,6 @@ public function solve($values, array &$config = null) private function solveComplexity($options, $field) { - if (!isset($options['complexity'])) { - return $options; - } $treatedOptions = $options; $value = $treatedOptions['complexity']; diff --git a/Tests/Functional/Security/AccessTest.php b/Tests/Functional/Security/AccessTest.php new file mode 100644 index 000000000..429adf811 --- /dev/null +++ b/Tests/Functional/Security/AccessTest.php @@ -0,0 +1,170 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Overblog\GraphQLBundle\Tests\Functional\Security; + +use Overblog\GraphQLBundle\Tests\Functional\TestCase; + +class AccessTest extends TestCase +{ + const USER_RYAN = 'ryan'; + const USER_ADMIN = 'admin'; + const ANONYMOUS_USER = null; + + private $userNameQuery = 'query MyQuery { user { name } }'; + + private $userRolesQuery = 'query MyQuery { user { roles } }'; + + private $userIsEnabledQuery = 'query MyQuery { user { isEnabled } }'; + + private $userFriendsQuery = << [ + 'user' => [ + 'name' => null, + ], + ], + 'errors' => [ + [ + 'message' => 'Access denied to this field.', + 'locations' => [['line' => 1, 'column' => 24]], + ], + ], + ]; + + $this->assertResponse($this->userNameQuery, $expected, static::ANONYMOUS_USER); + } + + public function testFullyAuthenticatedUserAccessToUserName() + { + $expected = [ + 'data' => [ + 'user' => [ + 'name' => 'Dan', + ], + ], + ]; + + $this->assertResponse($this->userNameQuery, $expected, static::USER_RYAN); + } + + public function testNotAuthenticatedUserAccessToUserRoles() + { + $this->assertResponse($this->userRolesQuery, $this->expectedFailedUserRoles(), static::ANONYMOUS_USER); + } + + public function testAuthenticatedUserAccessToUserRolesWithoutEnoughRights() + { + $this->assertResponse($this->userRolesQuery, $this->expectedFailedUserRoles(), static::USER_RYAN); + } + + public function testUserWithCorrectRightsAccessToUserRoles() + { + $expected = [ + 'data' => [ + 'user' => [ + 'roles' => ['ROLE_USER'], + ], + ], + ]; + + $this->assertResponse($this->userRolesQuery, $expected, static::USER_ADMIN); + } + + public function testUserAccessToUserFriends() + { + $expected = [ + 'data' => [ + 'user' => [ + 'friends' => [ + 'edges' => [ + ['node' => ['name' => 'Nick']], + ['node' => null], + ], + ], + ], + ], + ]; + + $this->assertResponse($this->userFriendsQuery, $expected, static::USER_ADMIN); + } + + public function testUserAccessToUserIsEnabledWithExpressionLanguageEvaluationFailed() + { + $expected = [ + 'data' => [ + 'user' => [ + 'isEnabled' => null, + ], + ], + 'errors' => [ + [ + 'message' => 'Access denied to this field.', + 'locations' => [['line' => 1, 'column' => 24]], + ], + ], + ]; + + $this->assertResponse($this->userIsEnabledQuery, $expected, static::USER_ADMIN); + } + + private function expectedFailedUserRoles() + { + return [ + 'data' => [ + 'user' => [ + 'roles' => [], + ], + ], + ]; + } + + private static function assertResponse($query, array $expected, $username) + { + $client = self::createClientAuthenticated($username); + $client->request('GET', '/', ['query' => $query]); + + $result = $client->getResponse()->getContent(); + + static::assertEquals($expected, json_decode($result, true), $result); + + return $client; + } + + private static function createClientAuthenticated($username) + { + $client = static::createClient(['test_case' => 'access']); + + if ($username) { + $client->setServerParameters([ + 'PHP_AUTH_USER' => $username, + 'PHP_AUTH_PW' => '123', + ]); + } + + return $client; + } +} diff --git a/Tests/Functional/Security/QueryComplexityTest.php b/Tests/Functional/Security/QueryComplexityTest.php new file mode 100644 index 000000000..2c895ea04 --- /dev/null +++ b/Tests/Functional/Security/QueryComplexityTest.php @@ -0,0 +1,88 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Overblog\GraphQLBundle\Tests\Functional\Security; + +use Overblog\GraphQLBundle\Tests\Functional\TestCase; + +class QueryComplexityTest extends TestCase +{ + private $userFriendsWithoutLimitQuery = << null, + 'errors' => [ + [ + 'message' => 'Max query complexity should be 10 but got 54.', + ], + ], + ]; + + $this->assertResponse($this->userFriendsWithoutLimitQuery, $expected); + } + + public function testComplexityUnderLimitation() + { + $expected = [ + 'data' => [ + 'user' => [ + 'friends' => [ + 'edges' => [ + ['node' => ['name' => 'Nick']], + ], + ], + ], + ], + ]; + + $this->assertResponse($this->userFriendsWithLimitQuery, $expected); + } + + private static function assertResponse($query, array $expected) + { + $client = static::createClient(['test_case' => 'queryComplexity']); + $client->request('GET', '/', ['query' => $query]); + + $result = $client->getResponse()->getContent(); + + static::assertEquals($expected, json_decode($result, true), $result); + + return $client; + } +} diff --git a/Tests/Functional/app/AppKernel.php b/Tests/Functional/app/AppKernel.php index 415f3aebd..80f843b73 100644 --- a/Tests/Functional/app/AppKernel.php +++ b/Tests/Functional/app/AppKernel.php @@ -28,6 +28,7 @@ public function registerBundles() { return [ new \Symfony\Bundle\FrameworkBundle\FrameworkBundle(), + new \Symfony\Bundle\SecurityBundle\SecurityBundle(), new \Symfony\Bundle\TwigBundle\TwigBundle(), new \Overblog\GraphQLBundle\OverblogGraphQLBundle(), ]; diff --git a/Tests/Functional/app/Resolver/ConnectionResolver.php b/Tests/Functional/app/Resolver/ConnectionResolver.php index e7271b99f..4aa9d3da7 100644 --- a/Tests/Functional/app/Resolver/ConnectionResolver.php +++ b/Tests/Functional/app/Resolver/ConnectionResolver.php @@ -46,7 +46,7 @@ public function friendsResolver($user, $args) public function resolveNode(Edge $edge) { - return $this->allUsers[$edge->node]; + return isset($this->allUsers[$edge->node]) ? $this->allUsers[$edge->node] : null; } public function resolveConnection() diff --git a/Tests/Functional/app/config/access/config.yml b/Tests/Functional/app/config/access/config.yml new file mode 100644 index 000000000..49e0a0c2b --- /dev/null +++ b/Tests/Functional/app/config/access/config.yml @@ -0,0 +1,37 @@ +imports: + - { resource: ../config.yml } + - { resource: ../connection/services.yml } + +overblog_graphql: + definitions: + schema: + query: Query + mutation: ~ + mappings: + types: + - + type: yml + dir: %kernel.root_dir%/config/connection/mapping + - + type: yml + dir: %kernel.root_dir%/config/access/mapping + +security: + providers: + in_memory: + memory: + users: + ryan: + password: 123 + roles: 'ROLE_USER' + admin: + password: 123 + roles: 'ROLE_ADMIN' + encoders: + Symfony\Component\Security\Core\User\User: plaintext + firewalls: + graph: + pattern: ^/ + http_basic: ~ + stateless: true + anonymous: true diff --git a/Tests/Functional/app/config/access/mapping/access.types.yml b/Tests/Functional/app/config/access/mapping/access.types.yml new file mode 100644 index 000000000..905b39dd2 --- /dev/null +++ b/Tests/Functional/app/config/access/mapping/access.types.yml @@ -0,0 +1,15 @@ +User: + config: + fields: + name: + access: "@=isFullyAuthenticated()" + roles: + type: '[String]' + access: "@=hasRole('ROLE_ADMIN')" + resolve: ['ROLE_USER'] + isEnabled: + type: Boolean + access: "@=wrong expression function" + resolve: true + friends: + access: "@=object === 1" diff --git a/Tests/Functional/app/config/connection/config.yml b/Tests/Functional/app/config/connection/config.yml index 73851062f..6b83910c6 100644 --- a/Tests/Functional/app/config/connection/config.yml +++ b/Tests/Functional/app/config/connection/config.yml @@ -1,14 +1,6 @@ imports: - { resource: ../config.yml } - -services: - overblog_graphql.test.resolver.node: - class: Overblog\GraphQLBundle\Tests\Functional\app\Resolver\ConnectionResolver - tags: - - { name: "overblog_graphql.resolver", alias: "friends", method: "friendsResolver" } - - { name: "overblog_graphql.resolver", alias: "node", method: "resolveNode" } - - { name: "overblog_graphql.resolver", alias: "query", method: "resolveQuery" } - - { name: "overblog_graphql.resolver", alias: "connection", method: "resolveConnection" } + - { resource: services.yml } overblog_graphql: definitions: diff --git a/Tests/Functional/app/config/connection/services.yml b/Tests/Functional/app/config/connection/services.yml new file mode 100644 index 000000000..b792daed1 --- /dev/null +++ b/Tests/Functional/app/config/connection/services.yml @@ -0,0 +1,8 @@ +services: + overblog_graphql.test.resolver.node: + class: Overblog\GraphQLBundle\Tests\Functional\app\Resolver\ConnectionResolver + tags: + - { name: "overblog_graphql.resolver", alias: "friends", method: "friendsResolver" } + - { name: "overblog_graphql.resolver", alias: "node", method: "resolveNode" } + - { name: "overblog_graphql.resolver", alias: "query", method: "resolveQuery" } + - { name: "overblog_graphql.resolver", alias: "connection", method: "resolveConnection" } diff --git a/Tests/Functional/app/config/queryComplexity/config.yml b/Tests/Functional/app/config/queryComplexity/config.yml new file mode 100644 index 000000000..90de17b48 --- /dev/null +++ b/Tests/Functional/app/config/queryComplexity/config.yml @@ -0,0 +1,19 @@ +imports: + - { resource: ../config.yml } + - { resource: ../connection/services.yml } + +overblog_graphql: + security: + query_max_complexity: 10 + definitions: + schema: + query: Query + mutation: ~ + mappings: + types: + - + type: yml + dir: %kernel.root_dir%/config/connection/mapping + - + type: yml + dir: %kernel.root_dir%/config/queryComplexity/mapping diff --git a/Tests/Functional/app/config/queryComplexity/mapping/connectionWithComplexity.types.yml b/Tests/Functional/app/config/queryComplexity/mapping/connectionWithComplexity.types.yml new file mode 100644 index 000000000..b35934e62 --- /dev/null +++ b/Tests/Functional/app/config/queryComplexity/mapping/connectionWithComplexity.types.yml @@ -0,0 +1,10 @@ +User: + type: object + config: + fields: + name: + type: String + friends: + type: friendConnection + argsBuilder: ConnectionArgs + complexity: "@=(!args['first'] ? 50 : 1) + childrenComplexity" diff --git a/Tests/Request/Validator/Rule/QueryDepthTest.php b/Tests/Request/Validator/Rule/QueryDepthTest.php index 0e5c56249..f78858a5e 100644 --- a/Tests/Request/Validator/Rule/QueryDepthTest.php +++ b/Tests/Request/Validator/Rule/QueryDepthTest.php @@ -102,10 +102,10 @@ public function queryDataProvider() [$this->createFormattedError(8, 10)], ], // failed because depth over limit (8) [ - 60, 20, - [$this->createFormattedError(20, 60)], - ], // failed because depth over limit (20) + 15, + [$this->createFormattedError(15, 20)], + ], // failed because depth over limit (15) ]; } diff --git a/composer.json b/composer.json index 2d1340ad5..262f41454 100644 --- a/composer.json +++ b/composer.json @@ -21,7 +21,7 @@ "symfony/expression-language": "^2.7|^3.0", "symfony/options-resolver": "^2.7|^3.0", "doctrine/doctrine-cache-bundle": "^1.2", - "webonyx/graphql-php": "^0.5.8", + "webonyx/graphql-php": "^0.5.9", "symfony/property-access": "^2.7|^3.0" }, "suggest": { From e3405f053bee7fe727fb69d2144837eecabaf262 Mon Sep 17 00:00:00 2001 From: Jeremiah VALERIE Date: Sun, 3 Apr 2016 00:11:01 +0200 Subject: [PATCH 7/7] fix hhvm "Fatal error: infinite recursion detected" --- Definition/FieldDefinition.php | 4 +++- Definition/ObjectType.php | 14 -------------- Request/Validator/Rule/AbstractQuerySecurity.php | 7 ++++--- Request/Validator/Rule/QueryComplexity.php | 10 +++++++--- Request/Validator/Rule/QueryDepth.php | 9 +++++++-- 5 files changed, 21 insertions(+), 23 deletions(-) diff --git a/Definition/FieldDefinition.php b/Definition/FieldDefinition.php index 38c28b5fe..3674afbee 100644 --- a/Definition/FieldDefinition.php +++ b/Definition/FieldDefinition.php @@ -16,6 +16,8 @@ class FieldDefinition extends BaseFieldDefinition { + const DEFAULT_COMPLEXITY_FN = '\Overblog\GraphQLBundle\Definition\FieldDefinition::defaultComplexity'; + /** * @var callable */ @@ -60,7 +62,7 @@ protected function __construct(array $config) { parent::__construct($config); - $this->complexityFn = isset($config['complexity']) ? $config['complexity'] : [$this, 'defaultComplexity']; + $this->complexityFn = isset($config['complexity']) ? $config['complexity'] : static::DEFAULT_COMPLEXITY_FN; } /** diff --git a/Definition/ObjectType.php b/Definition/ObjectType.php index 46353c7bd..2cae4022a 100644 --- a/Definition/ObjectType.php +++ b/Definition/ObjectType.php @@ -60,9 +60,6 @@ public function __construct(array $config) } } - /** - * @return FieldDefinition[] - */ public function getFields() { if (null === $this->fields) { @@ -74,13 +71,6 @@ public function getFields() return $this->fields; } - /** - * @param string $name - * - * @return FieldDefinition - * - * @throws \Exception - */ public function getField($name) { if (null === $this->fields) { @@ -91,10 +81,6 @@ public function getField($name) return $this->fields[$name]; } - /** - * @param $value - * @return bool|null - */ public function isTypeOf($value, ResolveInfo $info) { return isset($this->isTypeOf) ? call_user_func($this->isTypeOf, $value, $info) : null; diff --git a/Request/Validator/Rule/AbstractQuerySecurity.php b/Request/Validator/Rule/AbstractQuerySecurity.php index e412d1d59..d7c834cb8 100644 --- a/Request/Validator/Rule/AbstractQuerySecurity.php +++ b/Request/Validator/Rule/AbstractQuerySecurity.php @@ -71,13 +71,13 @@ protected function getFragment(FragmentSpread $fragmentSpread) protected function invokeIfNeeded(ValidationContext $context, array $validators) { - $this->gatherFragmentDefinition($context); - // is disabled? if (!$this->isEnabled()) { return []; } + $this->gatherFragmentDefinition($context); + return $validators; } @@ -107,6 +107,7 @@ protected function collectFieldASTsAndDefs(ValidationContext $context, $parentTy foreach ($selectionSet->selections as $selection) { switch ($selection->kind) { case Node::FIELD: + /* @var Field $selection */ $fieldName = $selection->name->value; $fieldDef = null; if ($parentType && method_exists($parentType, 'getFields')) { @@ -133,7 +134,7 @@ protected function collectFieldASTsAndDefs(ValidationContext $context, $parentTy $_astAndDefs[$responseName][] = [$selection, $fieldDef]; break; case Node::INLINE_FRAGMENT: - /* @var InlineFragment $inlineFragment */ + /* @var InlineFragment $selection */ $_astAndDefs = $this->collectFieldASTsAndDefs( $context, TypeInfo::typeFromAST($context->getSchema(), $selection->typeCondition), diff --git a/Request/Validator/Rule/QueryComplexity.php b/Request/Validator/Rule/QueryComplexity.php index 9275a0adf..3b7885126 100644 --- a/Request/Validator/Rule/QueryComplexity.php +++ b/Request/Validator/Rule/QueryComplexity.php @@ -15,6 +15,7 @@ use GraphQL\Executor\Values; use GraphQL\Language\AST\Field; use GraphQL\Language\AST\FragmentSpread; +use GraphQL\Language\AST\InlineFragment; use GraphQL\Language\AST\Node; use GraphQL\Language\AST\OperationDefinition; use GraphQL\Language\AST\SelectionSet; @@ -116,9 +117,9 @@ public function __invoke(ValidationContext $context) ); } - private function fieldComplexity(Node $node, $complexity = 0) + private function fieldComplexity($node, $complexity = 0) { - if (isset($node->selectionSet)) { + if (isset($node->selectionSet) && $node->selectionSet instanceof SelectionSet) { foreach ($node->selectionSet->selections as $childNode) { $complexity = $this->nodeComplexity($childNode, $complexity); } @@ -131,9 +132,10 @@ private function nodeComplexity(Node $node, $complexity = 0) { switch ($node->kind) { case Node::FIELD: + /* @var Field $node */ // default values $args = []; - $complexityFn = 'Overblog\GraphQLBundle\Definition\FieldDefinition::defaultComplexity'; + $complexityFn = \Overblog\GraphQLBundle\Definition\FieldDefinition::DEFAULT_COMPLEXITY_FN; // calculate children complexity if needed $childrenComplexity = 0; @@ -158,6 +160,7 @@ private function nodeComplexity(Node $node, $complexity = 0) break; case Node::INLINE_FRAGMENT: + /* @var InlineFragment $node */ // node has children? if (isset($node->selectionSet)) { $complexity = $this->fieldComplexity($node, $complexity); @@ -165,6 +168,7 @@ private function nodeComplexity(Node $node, $complexity = 0) break; case Node::FRAGMENT_SPREAD: + /* @var FragmentSpread $node */ $fragment = $this->getFragment($node); if (null !== $fragment) { diff --git a/Request/Validator/Rule/QueryDepth.php b/Request/Validator/Rule/QueryDepth.php index c87a4e779..223fde96c 100644 --- a/Request/Validator/Rule/QueryDepth.php +++ b/Request/Validator/Rule/QueryDepth.php @@ -13,6 +13,8 @@ use GraphQL\Error; use GraphQL\Language\AST\Field; +use GraphQL\Language\AST\FragmentSpread; +use GraphQL\Language\AST\InlineFragment; use GraphQL\Language\AST\Node; use GraphQL\Language\AST\OperationDefinition; use GraphQL\Language\AST\SelectionSet; @@ -77,9 +79,9 @@ protected function isEnabled() return $this->getMaxQueryDepth() !== static::DISABLED; } - private function fieldDepth(Node $node, $depth = 0, $maxDepth = 0) + private function fieldDepth($node, $depth = 0, $maxDepth = 0) { - if (isset($node->selectionSet)) { + if (isset($node->selectionSet) && $node->selectionSet instanceof SelectionSet) { foreach ($node->selectionSet->selections as $childNode) { $maxDepth = $this->nodeDepth($childNode, $depth, $maxDepth); } @@ -92,6 +94,7 @@ private function nodeDepth(Node $node, $depth = 0, $maxDepth = 0) { switch ($node->kind) { case Node::FIELD: + /* @var Field $node */ // node has children? if (null !== $node->selectionSet) { // update maxDepth if needed @@ -103,6 +106,7 @@ private function nodeDepth(Node $node, $depth = 0, $maxDepth = 0) break; case Node::INLINE_FRAGMENT: + /* @var InlineFragment $node */ // node has children? if (null !== $node->selectionSet) { $maxDepth = $this->fieldDepth($node, $depth, $maxDepth); @@ -110,6 +114,7 @@ private function nodeDepth(Node $node, $depth = 0, $maxDepth = 0) break; case Node::FRAGMENT_SPREAD: + /* @var FragmentSpread $node */ $fragment = $this->getFragment($node); if (null !== $fragment) {