handle unexpected params from koa-router

Signed-off-by: Pierre Noël <pierre.noel@strapi.io>

Signed-off-by: Pierre Noël <pierre.noel@strapi.io>

packages/strapi-admin/controllers/Admin.js

@@ -13,9 +13,7 @@ const PLUGIN_NAME_REGEX = /^[A-Za-z][A-Za-z0-9-_]+$/;
  * Validates a plugin name format
  */
 const isValidPluginName = plugin => {
-  return (
+  return _.isString(plugin) && !_.isEmpty(plugin) && PLUGIN_NAME_REGEX.test(plugin);
-    _.isString(plugin) && !_.isEmpty(plugin) && PLUGIN_NAME_REGEX.test(plugin)
-  );
 };
 
 /**
@@ -48,9 +46,7 @@ module.exports = {
       const strapiVersion = _.get(strapi.config, 'info.strapi', null);
       return ctx.send({ strapiVersion });
     } catch (err) {
-      return ctx.badRequest(null, [
+      return ctx.badRequest(null, [{ messages: [{ id: 'The version is not available' }] }]);
-        { messages: [{ id: 'The version is not available' }] },
-      ]);
     }
   },
 
@@ -68,9 +64,7 @@ module.exports = {
 
       return ctx.send({ layout });
     } catch (err) {
-      return ctx.badRequest(null, [
+      return ctx.badRequest(null, [{ messages: [{ id: 'An error occurred' }] }]);
-        { messages: [{ id: 'An error occurred' }] },
-      ]);
     }
   },
 
@@ -179,9 +173,7 @@ module.exports = {
       );
     }
 
-    const adminsWithSameEmail = await strapi
+    const adminsWithSameEmail = await strapi.query('administrator', 'admin').findOne({ email });
-      .query('administrator', 'admin')
-      .findOne({ email });
 
     const adminsWithSameUsername = await strapi
       .query('administrator', 'admin')
@@ -264,18 +256,14 @@ module.exports = {
         })
       );
     }
-    const admin = await strapi
+    const admin = await strapi.query('administrator', 'admin').findOne({ id });
-      .query('administrator', 'admin')
-      .findOne(ctx.params);
 
     // check the user exists
     if (!admin) return ctx.notFound('Administrator not found');
 
     // check there are not user with requested email
     if (email !== admin.email) {
-      const adminsWithSameEmail = await strapi
+      const adminsWithSameEmail = await strapi.query('administrator', 'admin').findOne({ email });
-        .query('administrator', 'admin')
-        .findOne({ email });
 
       if (adminsWithSameEmail && adminsWithSameEmail.id !== admin.id) {
         return ctx.badRequest(
@@ -317,9 +305,7 @@ module.exports = {
       user.password = await strapi.admin.services.auth.hashPassword(password);
     }
 
-    const data = await strapi
+    const data = await strapi.query('administrator', 'admin').update({ id }, user);
-      .query('administrator', 'admin')
-      .update({ id }, user);
 
     // Send 200 `ok`
     ctx.send(data);

packages/strapi-connector-bookshelf/lib/queries.js

@@ -127,8 +127,8 @@ module.exports = function createQueryBuilder({ model, modelKey, strapi }) {
     return wrapTransaction(runUpdate, { transacting });
   }
 
-  async function deleteOne(id, { transacting } = {}) {
+  async function deleteOne(params, { transacting } = {}) {
-    const entry = await model.where({ id }).fetch({ transacting });
+    const entry = await model.where(params).fetch({ transacting });
 
     if (!entry) {
       const err = new Error('entry.notFound');
@@ -155,7 +155,7 @@ module.exports = function createQueryBuilder({ model, modelKey, strapi }) {
       }
     });
 
-    await model.updateRelations({ [model.primaryKey]: id, values }, { transacting });
+    await model.updateRelations({ ...params, values }, { transacting });
 
     const runDelete = async trx => {
       await deleteComponents(entry, { transacting: trx });
@@ -167,10 +167,16 @@ module.exports = function createQueryBuilder({ model, modelKey, strapi }) {
   }
 
   async function deleteMany(params, { transacting } = {}) {
+    if (params[model.primaryKey]) {
+      const entries = await find(params, null, { transacting });
+      if (entries.length > 0) {
+        return deleteOne({ id: entries[0][model.primaryKey] }, { transacting });
+      }
+      return new Promise(resolve => resolve);
+    }
+
     const entries = await find(params, null, { transacting });
-    return await Promise.all(
+    return await Promise.all(entries.map(entry => deleteOne({ id: entry.id }, { transacting })));
-      entries.map(entry => deleteOne(entry[model.primaryKey], { transacting }))
-    );
   }
 
   function search(params, populate) {

packages/strapi-connector-mongoose/lib/queries.js

@@ -450,9 +450,13 @@ module.exports = ({ model, modelKey, strapi }) => {
   }
 
   async function deleteMany(params) {
-    const primaryKey = getPK(params, model);
+    if (params[model.primaryKey]) {
-
+      const entries = await find(params);
-    if (primaryKey) return deleteOne(params);
+      if (entries.length > 0) {
+        return deleteOne({ id: entries[0][model.primaryKey] });
+      }
+      return new Promise(resolve => resolve);
+    }
 
     const entries = await find(params);
     return Promise.all(entries.map(entry => deleteOne(entry[model.primaryKey])));

packages/strapi-plugin-content-manager/controllers/ContentManager.js

@@ -45,13 +45,14 @@ module.exports = {
    * Returns a list of entities of a content-type matching the query parameters
    */
   async find(ctx) {
+    const { model } = ctx.params;
     const contentManagerService = strapi.plugins['content-manager'].services.contentmanager;
 
     let entities = [];
     if (_.has(ctx.request.query, '_q')) {
-      entities = await contentManagerService.search(ctx.params, ctx.request.query);
+      entities = await contentManagerService.search({ model }, ctx.request.query);
     } else {
-      entities = await contentManagerService.fetchAll(ctx.params, ctx.request.query);
+      entities = await contentManagerService.fetchAll({ model }, ctx.request.query);
     }
 
     ctx.body = entities;
@@ -61,9 +62,10 @@ module.exports = {
    * Returns an entity of a content type by id
    */
   async findOne(ctx) {
+    const { model, id } = ctx.params;
     const contentManagerService = strapi.plugins['content-manager'].services.contentmanager;
 
-    const entry = await contentManagerService.fetch(ctx.params);
+    const entry = await contentManagerService.fetch({ model, id });
 
     // Entry not found
     if (!entry) {
@@ -77,13 +79,14 @@ module.exports = {
    * Returns a count of entities of a content type matching query parameters
    */
   async count(ctx) {
+    const { model } = ctx.params;
     const contentManagerService = strapi.plugins['content-manager'].services.contentmanager;
 
     let count;
     if (_.has(ctx.request.query, '_q')) {
-      count = await contentManagerService.countSearch(ctx.params, ctx.request.query);
+      count = await contentManagerService.countSearch({ model }, ctx.request.query);
     } else {
-      count = await contentManagerService.count(ctx.params, ctx.request.query);
+      count = await contentManagerService.count({ model }, ctx.request.query);
     }
 
     ctx.body = {
@@ -102,18 +105,13 @@ module.exports = {
     try {
       if (ctx.is('multipart')) {
         const { data, files } = parseMultipartBody(ctx);
-        ctx.body = await contentManagerService.create(data, {
+        ctx.body = await contentManagerService.create(data, { files, model });
-          files,
-          model,
-        });
       } else {
         // Create an entry using `queries` system
-        ctx.body = await contentManagerService.create(ctx.request.body, {
+        ctx.body = await contentManagerService.create(ctx.request.body, { model });
-          model,
-        });
       }
 
-      strapi.emit('didCreateFirstContentTypeEntry', ctx.params);
+      strapi.emit('didCreateFirstContentTypeEntry', { model });
     } catch (error) {
       strapi.log.error(error);
       ctx.badRequest(null, [
@@ -161,17 +159,19 @@ module.exports = {
    * Deletes one entity of a content type matching a query
    */
   async delete(ctx) {
+    const { id, model } = ctx.params;
     const contentManagerService = strapi.plugins['content-manager'].services.contentmanager;
 
-    ctx.body = await contentManagerService.delete(ctx.params);
+    ctx.body = await contentManagerService.delete({ id, model });
   },
 
   /**
    * Deletes multiple entities of a content type matching a query
    */
   async deleteMany(ctx) {
+    const { model } = ctx.params;
     const contentManagerService = strapi.plugins['content-manager'].services.contentmanager;
 
-    ctx.body = await contentManagerService.deleteMany(ctx.params, ctx.request.query);
+    ctx.body = await contentManagerService.deleteMany({ model }, ctx.request.query);
   },
 };

packages/strapi-plugin-upload/controllers/Upload.js

@@ -148,7 +148,8 @@ module.exports = {
   },
 
   async findOne(ctx) {
-    const data = await strapi.plugins['upload'].services.upload.fetch(ctx.params);
+    const { id } = ctx.params;
+    const data = await strapi.plugins['upload'].services.upload.fetch({ id });
 
     if (!data) {
       return ctx.notFound('file.notFound');

packages/strapi-plugin-users-permissions/controllers/Auth.js

@@ -70,9 +70,7 @@ module.exports = {
       }
 
       // Check if the user exists.
-      const user = await strapi
+      const user = await strapi.query('user', 'users-permissions').findOne(query);
-        .query('user', 'users-permissions')
-        .findOne(query);
 
       if (!user) {
         return ctx.badRequest(
@@ -119,9 +117,10 @@ module.exports = {
         );
       }
 
-      const validPassword = strapi.plugins[
+      const validPassword = strapi.plugins['users-permissions'].services.user.validatePassword(
-        'users-permissions'
+        params.password,
-      ].services.user.validatePassword(params.password, user.password);
+        user.password
+      );
 
       if (!validPassword) {
         return ctx.badRequest(
@@ -155,9 +154,10 @@ module.exports = {
       // Connect the user with the third-party provider.
       let user, error;
       try {
-        [user, error] = await strapi.plugins[
+        [user, error] = await strapi.plugins['users-permissions'].services.providers.connect(
-          'users-permissions'
+          provider,
-        ].services.providers.connect(provider, ctx.query);
+          ctx.query
+        );
       } catch ([user, error]) {
         return ctx.badRequest(null, error === 'array' ? error[0] : error);
       }
@@ -203,14 +203,12 @@ module.exports = {
       // Delete the current code
       user.resetPasswordToken = null;
 
-      user.password = await strapi.plugins[
+      user.password = await strapi.plugins['users-permissions'].services.user.hashPassword({
-        'users-permissions'
+        password: params.password,
-      ].services.user.hashPassword(params);
+      });
 
       // Update the user.
-      await strapi
+      await strapi.query('user', 'users-permissions').update({ id: user.id }, user);
-        .query('user', 'users-permissions')
-        .update({ id: user.id }, user);
 
       ctx.send({
         jwt: strapi.plugins['users-permissions'].services.jwt.issue({
@@ -258,9 +256,7 @@ module.exports = {
 
     const [requestPath] = ctx.request.url.split('?');
     const provider =
-      process.platform === 'win32'
+      process.platform === 'win32' ? requestPath.split('\\')[2] : requestPath.split('/')[2];
-        ? requestPath.split('\\')[2]
-        : requestPath.split('/')[2];
     const config = grantConfig[provider];
 
     if (!_.get(config, 'enabled')) {
@@ -268,9 +264,7 @@ module.exports = {
     }
     // Ability to pass OAuth callback dynamically
     grantConfig[provider].callback =
-      ctx.query && ctx.query.callback
+      ctx.query && ctx.query.callback ? ctx.query.callback : grantConfig[provider].callback;
-        ? ctx.query.callback
-        : grantConfig[provider].callback;
     return grant(grantConfig)(ctx, next);
   },
 
@@ -299,9 +293,7 @@ module.exports = {
     });
 
     // Find the user by email.
-    const user = await strapi
+    const user = await strapi.query('user', 'users-permissions').findOne({ email });
-      .query('user', 'users-permissions')
-      .findOne({ email });
 
     // User not found.
     if (!user) {
@@ -320,43 +312,43 @@ module.exports = {
     // Set the property code.
     user.resetPasswordToken = resetPasswordToken;
 
-    const settings = await pluginStore
+    const settings = await pluginStore.get({ key: 'email' }).then(storeEmail => {
-      .get({ key: 'email' })
+      try {
-      .then(storeEmail => {
+        return storeEmail['reset_password'].options;
-        try {
+      } catch (error) {
-          return storeEmail['reset_password'].options;
+        return {};
-        } catch (error) {
+      }
-          return {};
+    });
-        }
-      });
 
     const advanced = await pluginStore.get({
       key: 'advanced',
     });
 
-    settings.message = await strapi.plugins[
+    settings.message = await strapi.plugins['users-permissions'].services.userspermissions.template(
-      'users-permissions'
+      settings.message,
-    ].services.userspermissions.template(settings.message, {
+      {
-      URL: advanced.email_reset_password,
+        URL: advanced.email_reset_password,
-      USER: _.omit(user.toJSON ? user.toJSON() : user, [
+        USER: _.omit(user.toJSON ? user.toJSON() : user, [
-        'password',
+          'password',
-        'resetPasswordToken',
+          'resetPasswordToken',
-        'role',
+          'role',
-        'provider',
+          'provider',
-      ]),
+        ]),
-      TOKEN: resetPasswordToken,
+        TOKEN: resetPasswordToken,
-    });
+      }
+    );
 
-    settings.object = await strapi.plugins[
+    settings.object = await strapi.plugins['users-permissions'].services.userspermissions.template(
-      'users-permissions'
+      settings.object,
-    ].services.userspermissions.template(settings.object, {
+      {
-      USER: _.omit(user.toJSON ? user.toJSON() : user, [
+        USER: _.omit(user.toJSON ? user.toJSON() : user, [
-        'password',
+          'password',
-        'resetPasswordToken',
+          'resetPasswordToken',
-        'role',
+          'role',
-        'provider',
+          'provider',
-      ]),
+        ]),
-    });
+      }
+    );
 
     try {
       // Send an email to the user.
@@ -376,9 +368,7 @@ module.exports = {
     }
 
     // Update the user.
-    await strapi
+    await strapi.query('user', 'users-permissions').update({ id: user.id }, user);
-      .query('user', 'users-permissions')
-      .update({ id: user.id }, user);
 
     ctx.send({ ok: true });
   },
@@ -432,17 +422,12 @@ module.exports = {
 
     // Throw an error if the password selected by the user
     // contains more than two times the symbol '$'.
-    if (
+    if (strapi.plugins['users-permissions'].services.user.isHashed(params.password)) {
-      strapi.plugins['users-permissions'].services.user.isHashed(
-        params.password
-      )
-    ) {
       return ctx.badRequest(
         null,
         formatError({
           id: 'Auth.form.error.password.format',
-          message:
+          message: 'Your password cannot contain more than three times the symbol `$`.',
-            'Your password cannot contain more than three times the symbol `$`.',
         })
       );
     }
@@ -477,9 +462,7 @@ module.exports = {
     }
 
     params.role = role.id;
-    params.password = await strapi.plugins[
+    params.password = await strapi.plugins['users-permissions'].services.user.hashPassword(params);
-      'users-permissions'
-    ].services.user.hashPassword(params);
 
     const user = await strapi.query('user', 'users-permissions').findOne({
       email: params.email,
@@ -510,32 +493,25 @@ module.exports = {
         params.confirmed = true;
       }
 
-      const user = await strapi
+      const user = await strapi.query('user', 'users-permissions').create(params);
-        .query('user', 'users-permissions')
-        .create(params);
 
       const jwt = strapi.plugins['users-permissions'].services.jwt.issue(
         _.pick(user.toJSON ? user.toJSON() : user, ['id'])
       );
 
       if (settings.email_confirmation) {
-        const settings = await pluginStore
+        const settings = await pluginStore.get({ key: 'email' }).then(storeEmail => {
-          .get({ key: 'email' })
+          try {
-          .then(storeEmail => {
+            return storeEmail['email_confirmation'].options;
-            try {
+          } catch (error) {
-              return storeEmail['email_confirmation'].options;
+            return {};
-            } catch (error) {
+          }
-              return {};
+        });
-            }
-          });
 
         settings.message = await strapi.plugins[
           'users-permissions'
         ].services.userspermissions.template(settings.message, {
-          URL: new URL(
+          URL: new URL('/auth/email-confirmation', strapi.config.url).toString(),
-            '/auth/email-confirmation',
-            strapi.config.url
-          ).toString(),
           USER: _.omit(user.toJSON ? user.toJSON() : user, [
             'password',
             'resetPasswordToken',
@@ -595,9 +571,9 @@ module.exports = {
   async emailConfirmation(ctx) {
     const params = ctx.query;
 
-    const decodedToken = await strapi.plugins[
+    const decodedToken = await strapi.plugins['users-permissions'].services.jwt.verify(
-      'users-permissions'
+      params.confirmation
-    ].services.jwt.verify(params.confirmation);
+    );
 
     await strapi.plugins['users-permissions'].services.user.edit(
       { id: decodedToken.id },
@@ -653,39 +629,39 @@ module.exports = {
       _.pick(user.toJSON ? user.toJSON() : user, ['id'])
     );
 
-    const settings = await pluginStore
+    const settings = await pluginStore.get({ key: 'email' }).then(storeEmail => {
-      .get({ key: 'email' })
+      try {
-      .then(storeEmail => {
+        return storeEmail['email_confirmation'].options;
-        try {
+      } catch (err) {
-          return storeEmail['email_confirmation'].options;
+        return {};
-        } catch (err) {
+      }
-          return {};
-        }
-      });
-
-    settings.message = await strapi.plugins[
-      'users-permissions'
-    ].services.userspermissions.template(settings.message, {
-      URL: new URL('/auth/email-confirmation', strapi.config.url).toString(),
-      USER: _.omit(user.toJSON ? user.toJSON() : user, [
-        'password',
-        'resetPasswordToken',
-        'role',
-        'provider',
-      ]),
-      CODE: jwt,
     });
 
-    settings.object = await strapi.plugins[
+    settings.message = await strapi.plugins['users-permissions'].services.userspermissions.template(
-      'users-permissions'
+      settings.message,
-    ].services.userspermissions.template(settings.object, {
+      {
-      USER: _.omit(user.toJSON ? user.toJSON() : user, [
+        URL: new URL('/auth/email-confirmation', strapi.config.url).toString(),
-        'password',
+        USER: _.omit(user.toJSON ? user.toJSON() : user, [
-        'resetPasswordToken',
+          'password',
-        'role',
+          'resetPasswordToken',
-        'provider',
+          'role',
-      ]),
+          'provider',
-    });
+        ]),
+        CODE: jwt,
+      }
+    );
+
+    settings.object = await strapi.plugins['users-permissions'].services.userspermissions.template(
+      settings.object,
+      {
+        USER: _.omit(user.toJSON ? user.toJSON() : user, [
+          'password',
+          'resetPasswordToken',
+          'role',
+          'provider',
+        ]),
+      }
+    );
 
     try {
       await strapi.plugins['email'].services.email.send({

packages/strapi-plugin-users-permissions/controllers/User.js

@@ -28,14 +28,9 @@ module.exports = {
 
     if (_.has(ctx.query, '_q')) {
       // use core strapi query to search for users
-      users = await strapi
+      users = await strapi.query('user', 'users-permissions').search(ctx.query, populate);
-        .query('user', 'users-permissions')
-        .search(ctx.query, populate);
     } else {
-      users = await strapi.plugins['users-permissions'].services.user.fetchAll(
+      users = await strapi.plugins['users-permissions'].services.user.fetchAll(ctx.query, populate);
-        ctx.query,
-        populate
-      );
     }
 
     const data = users.map(sanitizeUser);
@@ -50,9 +45,7 @@ module.exports = {
     const user = ctx.state.user;
 
     if (!user) {
-      return ctx.badRequest(null, [
+      return ctx.badRequest(null, [{ messages: [{ id: 'No authorization header was found' }] }]);
-        { messages: [{ id: 'No authorization header was found' }] },
-      ]);
     }
 
     const data = sanitizeUser(user);
@@ -113,9 +106,7 @@ module.exports = {
     }
 
     if (advanced.unique_email) {
-      const userWithSameEmail = await strapi
+      const userWithSameEmail = await strapi.query('user', 'users-permissions').findOne({ email });
-        .query('user', 'users-permissions')
-        .findOne({ email });
 
       if (userWithSameEmail) {
         return ctx.badRequest(
@@ -144,9 +135,7 @@ module.exports = {
     }
 
     try {
-      const data = await strapi.plugins['users-permissions'].services.user.add(
+      const data = await strapi.plugins['users-permissions'].services.user.add(user);
-        user
-      );
 
       ctx.created(data);
     } catch (error) {
@@ -183,11 +172,7 @@ module.exports = {
       return ctx.badRequest('username.notNull');
     }
 
-    if (
+    if (_.has(ctx.request.body, 'password') && !password && user.provider === 'local') {
-      _.has(ctx.request.body, 'password') &&
-      !password &&
-      user.provider === 'local'
-    ) {
       return ctx.badRequest('password.notNull');
     }
 
@@ -209,9 +194,7 @@ module.exports = {
     }
 
     if (_.has(ctx.request.body, 'email') && advancedConfigs.unique_email) {
-      const userWithSameEmail = await strapi
+      const userWithSameEmail = await strapi.query('user', 'users-permissions').findOne({ email });
-        .query('user', 'users-permissions')
-        .findOne({ email });
 
       if (userWithSameEmail && userWithSameEmail.id != id) {
         return ctx.badRequest(
@@ -233,10 +216,7 @@ module.exports = {
       delete updateData.password;
     }
 
-    const data = await strapi.plugins['users-permissions'].services.user.edit(
+    const data = await strapi.plugins['users-permissions'].services.user.edit({ id }, updateData);
-      { id },
-      updateData
-    );
 
     ctx.send(data);
   },
@@ -247,16 +227,15 @@ module.exports = {
    */
   async destroy(ctx) {
     const { id } = ctx.params;
-    const data = await strapi.plugins['users-permissions'].services.user.remove(
+    const data = await strapi.plugins['users-permissions'].services.user.remove({ id });
-      { id }
-    );
     ctx.send(data);
   },
 
   async destroyAll(ctx) {
-    const data = await strapi.plugins[
+    const data = await strapi.plugins['users-permissions'].services.user.removeAll(
-      'users-permissions'
+      {},
-    ].services.user.removeAll(ctx.params, ctx.request.query);
+      ctx.request.query
+    );
 
     ctx.send(data);
   },

packages/strapi/lib/core-api/controller.js

@@ -81,7 +81,7 @@ const createCollectionTypeController = ({ model, service }) => {
      * @return {Object}
      */
     async findOne(ctx) {
-      const entity = await service.findOne(ctx.params);
+      const entity = await service.findOne({ id: ctx.params.id });
       return sanitizeEntity(entity, { model });
     },
 
@@ -122,9 +122,9 @@ const createCollectionTypeController = ({ model, service }) => {
       let entity;
       if (ctx.is('multipart')) {
         const { data, files } = parseMultipartData(ctx);
-        entity = await service.update(ctx.params, data, { files });
+        entity = await service.update({ id: ctx.params.id }, data, { files });
       } else {
-        entity = await service.update(ctx.params, ctx.request.body);
+        entity = await service.update({ id: ctx.params.id }, ctx.request.body);
       }
 
       return sanitizeEntity(entity, { model });
@@ -136,7 +136,7 @@ const createCollectionTypeController = ({ model, service }) => {
      * @return {Object}
      */
     async delete(ctx) {
-      const entity = await service.delete(ctx.params);
+      const entity = await service.delete({ id: ctx.params.id });
       return sanitizeEntity(entity, { model });
     },
   };