Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(graphql): allow to disable the introspection query #5711

Merged
merged 13 commits into from
Aug 8, 2023

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.

/**
* {@inheritdoc}
*/
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
@alanpoulain alanpoulain merged commit 92a81f0 into api-platform:main Aug 8, 2023
28 checks passed
@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.

None yet

2 participants