Skip to content

Conversation

@epourail
Copy link
Contributor

@epourail epourail commented Aug 2, 2023

Q A
Branch? main
Tickets -
License MIT
Doc PR api-platform/docs#1792

For security reason, the introspection query should be disabled to not expose the schema.
This PR allows to enable/disable the introspection query througth the bundle configuration:

api_platform:
  ...
  graphql:
    introspection:
      enabled: true # (or false)    
  ...

By default, the introspection is enabled to avoid BC and keep the current behavior.

TODO:

  • update the documentation
  • how to test ?

@epourail
Copy link
Contributor Author

epourail commented Aug 2, 2023

What do you think of this feature ? If ok with such improvment, I will update the documentation and test.

*/
public function executeQuery(Schema $schema, $source, mixed $rootValue = null, mixed $context = null, array $variableValues = null, string $operationName = null, callable $fieldResolver = null, array $validationRules = null): ExecutionResult
{
$validationRules[] = new DisableIntrospection(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need to handle the case where $validationRules is null.

Copy link
Contributor Author

@epourail epourail Aug 4, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I updated the code to instantiate the rule globally for both reasons:

  1. it will not create a new rule (new DisableIntrospection()) on each graphql query
  2. the configuration yaml-file setup the bundle globally

... but the code is now in the constructor.

@alanpoulain are you ok with that or I keep the previous logic (create the rule in the executeQuery() method) ?

@alanpoulain
Copy link
Member

I think it's a nice feature, thanks. I'm not sure you can write functional tests, but unit ones should be fine.

@epourail epourail reopened this Aug 8, 2023
@epourail epourail requested a review from alanpoulain August 8, 2023 13:18
@alanpoulain alanpoulain merged commit 92a81f0 into api-platform:main Aug 8, 2023
@alanpoulain
Copy link
Member

Thank you @epourail!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants