GraphQL Plugin (bug): Content type's attributes marked as private are being exposed in the exported GraphQL schema

[ralphsomeday]

What does it do?

It fixes a bug in the strapi graphql plugin in the functions generating input and types for graphql. The detection of an attribute as private was only applied to the privateAttributes array in the model.json file instead of also looking at the private property on an attribute within a model. There was also a bug in how the private attributes were filtered the attribute name was not being passed in the function so added the right filtering rule to only process fields that are not private and export them to the graphql schema file.

Why is it needed?

This fix will remove all private attributes from the graphql schema as this data is not meant to be exposed to the outside world if marked as private by the user.

How to test it?

In the content type builder, create an enum attribute and a string attribute as part of a content type and mark both as private and hit the save button. Open the graphql playground and click on the schema button on the right side of the screen and see if your enum and string value are not present in the exported schema.

Related issue(s)/PR(s)

Fixes issue #9804

[strapi-cla]

All committers have signed the CLA.

[codecov]

Codecov Report

Merging #9805 (9b0e40b) into master (3198d19) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #9805   +/-   ##
=======================================
  Coverage   57.86%   57.86%           
=======================================
  Files         183      183           
  Lines        6348     6348           
  Branches     1392     1392           
=======================================
  Hits         3673     3673           
  Misses       2215     2215           
  Partials      460      460           

Flag	Coverage Δ
front	∅ <ø> (∅)
unit	57.86% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3198d19...9b0e40b. Read the comment docs.

[ralphsomeday]

@alexandrebodin can you have a look at this one as this bug is blocking us as it exposes private data in graphql schema.

[ralphsomeday]

@alexandrebodin not sure why those tests are failing, not related to what I changed. Maybe you can help me understand why is it failing?

[rmaroun]

@Convly is there any chance we can review this one this week? thanks

[alexandrebodin]

@ralphsomeday We won't be able to review this until at least next week

[ralphsomeday]

@alexandrebodin ok noted. Will then wait for next week. Cheers.

[ralphsomeday]

@alexandrebodin is there any chance this can be reviewed this week?

[alexandrebodin]

Will need to refactor in v4 to avoid doing all the filtering everywhere that's too error prone

[ralphsomeday]

@alexandrebodin updated the code with your input. Let me know if all is fine and if we can look into the reason why the tests are failing. thanks.

[alexandrebodin]

@ralphsomeday the code looks good. Indeed the tests are breaking in graphql because it now prevents users from creating entries with private attributes but the API should actually allow creating/editing private fields for now if the mutation is allowed. We should not remove private fields from the inputs

[ralphsomeday]

@ralphsomeday the code looks good. Indeed the tests are breaking in graphql because it now prevents users from creating entries with private attributes but the API should actually allow creating/editing private fields for now if the mutation is allowed. We should not remove private fields from the inputs

@alexandrebodin the privately marked inputs should not be part of the graphql export. It should have the same behaviour as in the REST API. Those private fields are not available via the REST API and should not be via graphql. Isn't this the idea of the private fields?

[alexandrebodin]

@ralphsomeday the code looks good. Indeed the tests are breaking in graphql because it now prevents users from creating entries with private attributes but the API should actually allow creating/editing private fields for now if the mutation is allowed. We should not remove private fields from the inputs

@alexandrebodin the privately marked inputs should not be part of the graphql export. It should have the same behaviour as in the REST API. Those private fields are not available via the REST API and should not be via graphql. Isn't this the idea of the private fields?

In the API If you have the right to Create/Update an entry you should be able to fill the private fields. Private is only for Read queries. In Rest too.

[ralphsomeday]

@ralphsomeday the code looks good. Indeed the tests are breaking in graphql because it now prevents users from creating entries with private attributes but the API should actually allow creating/editing private fields for now if the mutation is allowed. We should not remove private fields from the inputs

@alexandrebodin the privately marked inputs should not be part of the graphql export. It should have the same behaviour as in the REST API. Those private fields are not available via the REST API and should not be via graphql. Isn't this the idea of the private fields?

In the API If you have the right to Create/Update an entry you should be able to fill the private fields. Private is only for Read queries. In Rest too.

Ok and where do you recon to do a check on this in the code?

[alexandrebodin]

The only thing we can do is remove them from the Type but not from the input Type

[alexandrebodin]

Looks good. Last thing would be to add some documentation in our documentation repository :) @pwizla & @derrickmehaffy can help you with that :)

[ralphsomeday]

Hi @pwizla & @derrickmehaffy can you guys point me out to where would be the best place in the documentation to add this? thanks

[alexandrebodin]

I think youc an add this arround here https://github.com/strapi/documentation/blob/main/docs/developer-docs/latest/development/plugins/graphql.md#disable-a-query-or-a-type

[ralphsomeday]

@alexandrebodin done PR#269

[ralphsomeday]

@alexandrebodin can you let me know if this PR can now be merged. thanks

[alexandrebodin]

The PR looks good We will wait for the doc PR to be approved first :)

[pwizla]

The PR looks good We will wait for the doc PR to be approved first :)

I've just approved the docs PR, thank you @ralphsomeday ! 👍
@alexandrebodin please let me know when you merge this PR and I'll merge the docs one (won't be live before next docs deployment, so probably next week, though)

[ralphsomeday]

@alexandrebodin do you think this one can be added to v3.6.2? and when do you plan to release v3.6.2?