Merge pull request #13185 from strapi/fix/user-hidden-attributes

Sanitize hidden attributes from admin API responses
strapi · May 11, 2022 · d69b49b · d69b49b
2 parents a727b1f + 620418c
commit d69b49b
Show file tree

Hide file tree

Showing 8 changed files with 145 additions and 3 deletions.
diff --git a/packages/core/admin/server/services/__tests__/permissions-manager-sanitize.test.js b/packages/core/admin/server/services/__tests__/permissions-manager-sanitize.test.js
@@ -0,0 +1,100 @@
+'use strict';
+
+const { AbilityBuilder, Ability } = require('@casl/ability');
+const { pick } = require('lodash/fp');
+const sift = require('sift');
+
+const createSanitizeHelpers = require('../permission/permissions-manager/sanitize');
+
+const allowedOperations = [
+  '$or',
+  '$and',
+  '$eq',
+  '$ne',
+  '$in',
+  '$nin',
+  '$lt',
+  '$lte',
+  '$gt',
+  '$gte',
+  '$exists',
+  '$elemMatch',
+];
+
+const operations = pick(allowedOperations, sift);
+
+const conditionsMatcher = conditions => {
+  return sift.createQueryTester(conditions, { operations });
+};
+
+const defineAbility = register => {
+  const { can, build } = new AbilityBuilder(Ability);
+
+  register(can);
+
+  return build({ conditionsMatcher });
+};
+
+const fooModel = {
+  uid: 'api::foo.foo',
+  attributes: {
+    a: {
+      type: 'string',
+    },
+    b: {
+      type: 'password',
+    },
+    c: {
+      type: 'string',
+    },
+  },
+  config: {
+    attributes: {
+      a: {
+        hidden: true,
+      },
+    },
+  },
+};
+
+const sanitizeHelpers = {
+  sanitizeOutput: null,
+  sanitizeInput: null,
+};
+
+describe('Permissions Manager - Sanitize', () => {
+  beforeAll(() => {
+    global.strapi = {
+      getModel() {
+        return fooModel;
+      },
+    };
+
+    Object.assign(
+      sanitizeHelpers,
+      createSanitizeHelpers({
+        action: 'read',
+        model: fooModel,
+        ability: defineAbility(can => can('read', 'api::foo.foo')),
+      })
+    );
+  });
+
+  describe('Sanitize Output', () => {
+    it('Removes hidden fields', async () => {
+      const data = { a: 'Foo', c: 'Bar' };
+      const result = await sanitizeHelpers.sanitizeOutput(data, { subject: fooModel.uid });
+
+      expect(result).toEqual({ c: 'Bar' });
+    });
+  });
+
+  describe('Sanitize Input', () => {
+    it('Removes hidden fields', async () => {
+      const data = { a: 'Foo', c: 'Bar' };
+      const result = await sanitizeHelpers.sanitizeInput(data, { subject: fooModel.uid });
+
+      expect(result).toEqual({ c: 'Bar' });
+    });
+  });
+});
diff --git a/packages/core/admin/server/services/permission/permissions-manager/sanitize.js b/packages/core/admin/server/services/permission/permissions-manager/sanitize.js
@@ -14,6 +14,7 @@ const {
   uniq,
   intersection,
   pick,
+  getOr,
 } = require('lodash/fp');
 
 const { contentTypes, traverseEntity, sanitize, pipeAsync } = require('@strapi/utils');
@@ -46,6 +47,8 @@ module.exports = ({ action, ability, model }) => {
     const permittedFields = fields.shouldIncludeAll ? null : getOutputFields(fields.permitted);
 
     return pipeAsync(
+      // Remove fields hidden from the admin
+      traverseEntity(omitHiddenFields, { schema }),
       // Remove unallowed fields from admin::user relations
       traverseEntity(pickAllowedAdminUserFields, { schema }),
       // Remove not allowed fields (RBAC)
@@ -61,6 +64,8 @@ module.exports = ({ action, ability, model }) => {
     const permittedFields = fields.shouldIncludeAll ? null : getInputFields(fields.permitted);
 
     return pipeAsync(
+      // Remove fields hidden from the admin
+      traverseEntity(omitHiddenFields, { schema }),
       // Remove not allowed fields (RBAC)
       traverseEntity(allowedFields(permittedFields), { schema }),
       // Remove roles from createdBy & updateBy fields
@@ -107,8 +112,25 @@ module.exports = ({ action, ability, model }) => {
     return defaults({ subject: asSubject(model, data), action }, options);
   };
 
+  /**
+   * Omit creator fields' (createdBy & updatedBy) roles from the admin API responses
+   */
   const omitCreatorRoles = omit([`${CREATED_BY_ATTRIBUTE}.roles`, `${UPDATED_BY_ATTRIBUTE}.roles`]);
 
+  /**
+   * Visitor used to remove hidden fields from the admin API responses
+   */
+  const omitHiddenFields = ({ key, schema }, { remove }) => {
+    const isHidden = getOr(false, ['config', 'attributes', key, 'hidden'], schema);
+
+    if (isHidden) {
+      remove(key);
+    }
+  };
+
+  /**
+   * Visitor used to only select needed fields from the admin users entities & avoid leaking sensitive information
+   */
   const pickAllowedAdminUserFields = ({ attribute, key, value }, { set }) => {
     const pickAllowedFields = pick(['id', 'firstname', 'lastname', 'username']);
 

diff --git a/packages/core/admin/server/services/user.js b/packages/core/admin/server/services/user.js
@@ -17,7 +17,7 @@ const sanitizeUserRoles = role => _.pick(role, ['id', 'name', 'description', 'co
  */
 const sanitizeUser = user => {
   return {
-    ..._.omit(user, ['password', 'resetPasswordToken', 'roles']),
+    ..._.omit(user, ['password', 'resetPasswordToken', 'registrationToken', 'roles']),
     roles: user.roles && user.roles.map(sanitizeUserRoles),
   };
 };

diff --git a/packages/core/content-manager/server/tests/api/basic.test.e2e.js b/packages/core/content-manager/server/tests/api/basic.test.e2e.js
@@ -1,5 +1,7 @@
 'use strict';
 
+const { omit } = require('lodash/fp');
+
 const { createStrapiInstance } = require('../../../../../../test/helpers/strapi');
 const { createTestBuilder } = require('../../../../../../test/helpers/builder');
 const { createAuthRequest } = require('../../../../../../test/helpers/request');
@@ -22,6 +24,16 @@ const product = {
       minLength: 4,
       maxLength: 30,
     },
+    hiddenAttribute: {
+      type: 'string',
+    },
+  },
+  config: {
+    attributes: {
+      hiddenAttribute: {
+        hidden: true,
+      },
+    },
   },
   displayName: 'Product',
   singularName: 'product',
@@ -47,6 +59,7 @@ describe('CM API - Basic', () => {
     const product = {
       name: 'Product 1',
       description: 'Product description',
+      hiddenAttribute: 'Secret value',
     };
     const res = await rq({
       method: 'POST',
@@ -55,7 +68,8 @@ describe('CM API - Basic', () => {
     });
 
     expect(res.statusCode).toBe(200);
-    expect(res.body).toMatchObject(product);
+    expect(res.body).toMatchObject(omit('hiddenAttribute', product));
+    expect(res.body).not.toHaveProperty('hiddenAttribute');
     expect(res.body.publishedAt).toBeUndefined();
     data.products.push(res.body);
   });
@@ -84,6 +98,7 @@ describe('CM API - Basic', () => {
     const product = {
       name: 'Product 1 updated',
       description: 'Updated Product description',
+      hiddenAttribute: 'Secret value',
     };
     const res = await rq({
       method: 'PUT',
@@ -92,7 +107,7 @@ describe('CM API - Basic', () => {
     });
 
     expect(res.statusCode).toBe(200);
-    expect(res.body).toMatchObject(product);
+    expect(res.body).toMatchObject(omit('hiddenAttribute', product));
     expect(res.body.id).toEqual(data.products[0].id);
     expect(res.body.publishedAt).toBeUndefined();
     data.products[0] = res.body;

diff --git a/packages/core/content-type-builder/server/services/schema-builder/component-builder.js b/packages/core/content-type-builder/server/services/schema-builder/component-builder.js
@@ -54,6 +54,7 @@ module.exports = function createComponentBuilder() {
         .set(['info', 'icon'], infos.icon)
         .set(['info', 'description'], infos.description)
         .set('pluginOptions', infos.pluginOptions)
+        .set('config', infos.config)
         .setAttributes(this.convertAttributes(infos.attributes));
 
       if (this.components.size === 0) {

diff --git a/packages/core/content-type-builder/server/services/schema-builder/content-type-builder.js b/packages/core/content-type-builder/server/services/schema-builder/content-type-builder.js
@@ -105,6 +105,7 @@ module.exports = function createComponentBuilder() {
         })
         .set('options', { draftAndPublish: infos.draftAndPublish || false })
         .set('pluginOptions', infos.pluginOptions)
+        .set('config', infos.config)
         .setAttributes(this.convertAttributes(infos.attributes));
 
       Object.keys(infos.attributes).forEach(key => {

diff --git a/packages/core/content-type-builder/server/services/schema-builder/index.js b/packages/core/content-type-builder/server/services/schema-builder/index.js
@@ -25,6 +25,7 @@ module.exports = function createBuilder() {
       filename: compo.__filename__,
       dir: join(strapi.dirs.components, compo.category),
       schema: compo.__schema__,
+      config: compo.config,
     };
   });
 
@@ -47,6 +48,7 @@ module.exports = function createBuilder() {
       filename: 'schema.json',
       dir,
       schema: contentType.__schema__,
+      config: contentType.config,
     };
   });
 

diff --git a/packages/core/content-type-builder/server/services/schema-builder/schema-handler.js b/packages/core/content-type-builder/server/services/schema-builder/schema-handler.js
@@ -231,6 +231,7 @@ module.exports = function createSchemaHandler(infos) {
             options: state.schema.options,
             pluginOptions: state.schema.pluginOptions,
             attributes: state.schema.attributes,
+            config: state.schema.config,
           },
           { spaces: 2 }
         );