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

Private fields of components are show in response #7335

Closed
Naxos84 opened this issue Aug 5, 2020 · 8 comments · Fixed by #7350
Closed

Private fields of components are show in response #7335

Naxos84 opened this issue Aug 5, 2020 · 8 comments · Fixed by #7350
Assignees
Labels
issue: bug Issue reporting a bug source: core:strapi Source is core/strapi package status: confirmed Confirmed by a Strapi Team member or multiple community members

Comments

@Naxos84
Copy link

Naxos84 commented Aug 5, 2020

Describe the bug
When adding a private field (advanced settings) to a component the field is still present in the REST response.

The field does not show up qhen querying it with GraphQL and private fields in content-types also do not show up as expected.

Steps to reproduce the behavior

  1. Create a content-type
  2. Add a private field to that content-type
  3. Create a component
  4. Add a private field to that component
  5. Add a field from the content-type you created
  6. Save everything
  7. Fetch it via REST

Expected behavior
I expect private fields from components not to show up in any response. Similar to private fields in content-types.

Screenshots

Code snippets
Attribute of created component:

"demo": {
      "type": "string",
      "private": true,
      "default": "demo-value"
    }

Attribute of created content-type:

"demo": {
      "type": "string",
      "private": true,
      "default": "Demo-Value"
    }

REST response when fetching content-type with configured component (snippet):

...
{
            "__component": "ui.slider",
            "id": 2,
            "demo": "some-value",
            ...
}

System

  • Node.js version: v12.13.1
  • NPM version: v6.12.1
  • Strapi version: 3.1.3
  • Database: sqlite
  • Operating system: CentOs 7

Additional context

@Convly
Copy link
Member

Convly commented Aug 5, 2020

Hello there, thanks for the issue!

I tried to reproduce your scenario but didn't succeed. In my tests, the private fields remain hidden from the public REST API.

Here are my models:

./api/test/models/test.settings.json

{
  "kind": "collectionType",
  "collectionName": "tests",
  "info": {
    "name": "Test"
  },
  "options": {
    "increments": true,
    "timestamps": true
  },
  "attributes": {
    "demo": {
      "type": "string",
      "private": true,
      "default": "demo-value"
    },
    "slider": {
      "type": "component",
      "repeatable": false,
      "component": "default.compo"
    },
    "other": {
      "type": "string"
    }
  }
}

./components/default/compo.json

{
  "collectionName": "components_default_compos",
  "info": {
    "name": "Compo",
    "icon": "allergies"
  },
  "options": {},
  "attributes": {
    "demo": {
      "type": "string",
      "private": true,
      "default": "Demo-value"
    }
  }
}

This is the entry I've created

image

And the result of my request at localhost:1337/tests

image

If you have further information about how to reproduce it, I'll try it again.

@Convly Convly closed this as completed Aug 5, 2020
@Convly Convly reopened this Aug 5, 2020
@Convly
Copy link
Member

Convly commented Aug 5, 2020

Follow-up: I've just tested with a dynamic zone instead of a simple component and I've successfully reproduced the issue.

@Naxos84
Copy link
Author

Naxos84 commented Aug 5, 2020

I'm happy that you could reproduce it. Otherwise I would have checked tomorrow morning.
I think the mentioned component was in a dynamic zone. Though I'm not 100% sure and have to check that tomorrow.

Thx for your effort so far.

@Naxos84
Copy link
Author

Naxos84 commented Aug 6, 2020

I checked and can confirm that my configured component is used within a dynamic zone.
Sorry that I forgot that in the first place.

@dalbitresb12
Copy link
Contributor

@Convly I was doing some testing about this issue and found that the sanitizeEntity function isn't sanitizing dynamic zones because of this allowedFieldsHasKey in line 63:

// Dynamic zones
if (attribute && attribute.type === 'dynamiczone' && value !== null && allowedFieldsHasKey) {
const nextVal = value.map(elem =>
sanitizeEntity(elem, {
model: strapi.getModel(elem.__component),
withPrivate,
isOutput,
})
);
return { ...acc, [key]: nextVal };
}

The function never enters the if statement because of it. Removing it works, but I don't know if this is safe to do.

@Convly
Copy link
Member

Convly commented Aug 6, 2020

I'm working on this very file right now. The issue indeed comes from here.
I would rather try to makes allowedFieldsHasKey returns true here instead of removing the check.
(it might be due to the fact we're settings allowedFields to includeFields || [] in getAllowedFields instead of includeFields || Object.keys(model.attributes))

@dalbitresb12
Copy link
Contributor

dalbitresb12 commented Aug 6, 2020

@Convly What I couldn't understand is what the includeFields parameter is for. While working on another PR (#7331) I also had to check it, but in the end I did not fully understand it.

@Convly Convly added status: confirmed Confirmed by a Strapi Team member or multiple community members issue: bug Issue reporting a bug source: core:strapi Source is core/strapi package labels Aug 6, 2020
@Convly
Copy link
Member

Convly commented Aug 6, 2020

Simply put, includeFields is an array of strings (fieldA, fieldA.fieldB, etc...) used to determine which fields you want to keep during the sanitation process.

If this variable is set to null, it means you want to keep every field.

Bear in mind though, it doesn't force the field to be present in the final response, it only allows it (a password will be removed even if includeFields is set to ['password'])

It is for example used in the permissions-manager to target specific fields based on some permissions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
issue: bug Issue reporting a bug source: core:strapi Source is core/strapi package status: confirmed Confirmed by a Strapi Team member or multiple community members
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants