Fix username unique in users-permissions plugin and implement sql db unique option

packages/strapi-admin/controllers/Admin.js

@@ -108,25 +108,21 @@ module.exports = {
    */
 
   async create(ctx) {
-    const values = ctx.request.body;
+    const { email, username, password, blocked } = ctx.request.body;
 
-    if (!values.email) return ctx.badRequest('Missing email');
-    if (!values.username) return ctx.badRequest('Missing username');
-    if (!values.password) return ctx.badRequest('Missing password');
+    if (!email) return ctx.badRequest('missing.email');
+    if (!username) return ctx.badRequest('missing.username');
+    if (!password) return ctx.badRequest('missing.password');
 
     const adminsWithSameEmail = await strapi
       .query('administrator', 'admin')
-      .find({
-        email: values.email,
-      });
+      .findOne({ email });
 
     const adminsWithSameUsername = await strapi
       .query('administrator', 'admin')
-      .find({
-        username: values.username,
-      });
+      .findOne({ username });
 
-    if (adminsWithSameEmail.length > 0) {
+    if (adminsWithSameEmail) {
       return ctx.badRequest(
         null,
         ctx.request.admin
@@ -137,11 +133,11 @@ module.exports = {
                 ],
               },
             ]
-          : 'Email is already taken.'
+          : 'email.alreadyTaken'
       );
     }
 
-    if (adminsWithSameUsername.length > 0) {
+    if (adminsWithSameUsername) {
       return ctx.badRequest(
         null,
         ctx.request.admin
@@ -155,15 +151,15 @@ module.exports = {
                 ],
               },
             ]
-          : 'Username is already taken.'
+          : 'username.alreadyTaken.'
       );
     }
 
     const user = {
-      email: values.email,
-      username: values.username,
-      blocked: values.blocked === true ? true : false,
-      password: await strapi.admin.services.auth.hashPassword(values.password),
+      email: email,
+      username: username,
+      blocked: blocked === true ? true : false,
+      password: await strapi.admin.services.auth.hashPassword(password),
     };
 
     const data = await strapi.query('administrator', 'admin').create(user);
@@ -180,11 +176,11 @@ module.exports = {
 
   async update(ctx) {
     const { id } = ctx.params;
-    const values = ctx.request.body;
+    const { email, username, password, blocked } = ctx.request.body;
 
-    if (!values.email) return ctx.badRequest('Missing email');
-    if (!values.username) return ctx.badRequest('Missing username');
-    if (!values.password) return ctx.badRequest('Missing password');
+    if (!email) return ctx.badRequest('Missing email');
+    if (!username) return ctx.badRequest('Missing username');
+    if (!password) return ctx.badRequest('Missing password');
 
     const admin = await strapi
       .query('administrator', 'admin')
@@ -194,12 +190,10 @@ module.exports = {
     if (!admin) return ctx.notFound('Administrator not found');
 
     // check there are not user with requested email
-    if (values.email !== admin.email) {
+    if (email !== admin.email) {
       const adminsWithSameEmail = await strapi
         .query('administrator', 'admin')
-        .findOne({
-          email: values.email,
-        });
+        .findOne({ email });
 
       if (adminsWithSameEmail && adminsWithSameEmail.id !== admin.id) {
         return ctx.badRequest(
@@ -218,12 +212,10 @@ module.exports = {
     }
 
     // check there are not user with requested username
-    if (values.username !== admin.username) {
+    if (username !== admin.username) {
       const adminsWithSameUsername = await strapi
         .query('administrator', 'admin')
-        .findOne({
-          username: values.username,
-        });
+        .findOne({ username });
 
       if (adminsWithSameUsername && adminsWithSameUsername.id !== admin.id) {
         return ctx.badRequest(
@@ -245,15 +237,13 @@ module.exports = {
     }
 
     const user = {
-      email: values.email,
-      username: values.username,
-      blocked: values.blocked === true ? true : false,
+      email: email,
+      username: username,
+      blocked: blocked === true ? true : false,
     };
 
-    if (values.password !== admin.password) {
-      user.password = await strapi.admin.services.auth.hashPassword(
-        values.password
-      );
+    if (password !== admin.password) {
+      user.password = await strapi.admin.services.auth.hashPassword(password);
     }
 
     const data = await strapi

packages/strapi-hook-bookshelf/lib/buildDatabaseSchema.js

@@ -16,36 +16,10 @@ module.exports = async ({
     [createAtCol, updatedAtCol] = hasTimestamps;
   }
 
-  const quote = definition.client === 'pg' ? '"' : '`';
-
   // Equilize database tables
   const createOrUpdateTable = async (table, attributes) => {
     const tableExists = await ORM.knex.schema.hasTable(table);
 
-    // Apply field type of attributes definition
-    const generateColumns = (attrs, start, tableExists = false) => {
-      return Object.keys(attrs).reduce((acc, attr) => {
-        const attribute = attributes[attr];
-
-        const type = getType({
-          definition,
-          attribute,
-          name: attr,
-          tableExists,
-        });
-
-        if (type) {
-          acc.push(
-            `${quote}${attr}${quote} ${type} ${
-              attribute.required ? 'NOT NULL' : ''
-            }`
-          );
-        }
-
-        return acc;
-      }, start);
-    };
-
     const generateIndexes = async table => {
       try {
         const connection = strapi.config.connections[definition.connection];
@@ -104,40 +78,60 @@ module.exports = async ({
       }
     };
 
-    const createTable = async table => {
-      const defaultAttributeDefinitions = {
-        mysql: ['id INT AUTO_INCREMENT NOT NULL PRIMARY KEY'],
-        pg: ['id SERIAL NOT NULL PRIMARY KEY'],
-        sqlite3: ['id INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL'],
-      };
+    const buildColumns = (tbl, columns, opts = {}) => {
+      const { tableExists, alter = false } = opts;
 
-      let idAttributeBuilder = defaultAttributeDefinitions[definition.client];
-
-      if (definition.primaryKeyType === 'uuid' && definition.client === 'pg') {
-        idAttributeBuilder = [
-          'id uuid NOT NULL DEFAULT uuid_generate_v4() NOT NULL PRIMARY KEY',
-        ];
-      } else if (definition.primaryKeyType !== 'integer') {
+      Object.keys(columns).forEach(key => {
+        const attribute = columns[key];
         const type = getType({
           definition,
-          attribute: {
-            type: definition.primaryKeyType,
-          },
+          attribute,
+          name: key,
+          tableExists,
         });
 
-        idAttributeBuilder = [`id ${type} NOT NULL PRIMARY KEY`];
-      }
-      const columns = generateColumns(attributes, idAttributeBuilder).join(
-        ',\n\r'
-      );
+        if (type) {
+          const col = tbl.specificType(key, type);
 
-      // Create table
-      await ORM.knex.raw(`CREATE TABLE ${quote}${table}${quote} (${columns})`);
+          if (attribute.required === true) {
+            if (definition.client !== 'sqlite3' || !tableExists) {
+              col.notNullable();
+            }
+          } else {
+            col.nullable();
+          }
+
+          if (attribute.unique === true) {
+            if (definition.client !== 'sqlite3' || !tableExists) {
+              tbl.unique(key, uniqueColName(table, key));
+            }
+          }
+
+          if (alter) {
+            col.alter();
+          }
+        }
+      });
+    };
+
+    const createColumns = (tbl, columns, opts = {}) => {
+      return buildColumns(tbl, columns, opts);
+    };
+
+    const alterColumns = (tbl, columns, opts = {}) => {
+      return createColumns(tbl, columns, { ...opts, alter: true });
+    };
+
+    const createTable = (table, { trx = ORM.knex, ...opts } = {}) => {
+      return trx.schema.createTable(table, tbl => {
+        tbl.specificType('id', getIdType(definition));
+        createColumns(tbl, attributes, { ...opts, tableExists: false });
+      });
     };
 
     if (!tableExists) {
       await createTable(table);
-      await generateIndexes(table, attributes);
+      await generateIndexes(table);
       await storeTable(table, attributes);
       return;
     }
@@ -160,31 +154,16 @@ module.exports = async ({
       }
     });
 
-    // Generate indexes for new attributes.
-    await generateIndexes(table, columnsToAdd);
-
     // Generate and execute query to add missing column
     if (Object.keys(columnsToAdd).length > 0) {
       await ORM.knex.schema.table(table, tbl => {
-        Object.keys(columnsToAdd).forEach(key => {
-          const attribute = columnsToAdd[key];
-          const type = getType({
-            definition,
-            attribute,
-            name: key,
-            tableExists,
-          });
-
-          if (type) {
-            const col = tbl.specificType(key, type);
-            if (attribute.required && definition.client !== 'sqlite3') {
-              col.notNullable();
-            }
-          }
-        });
+        createColumns(tbl, columnsToAdd, { tableExists });
       });
     }
 
+    // Generate indexes for new attributes.
+    await generateIndexes(table, columnsToAdd);
+
     let previousAttributes;
     try {
       previousAttributes = JSON.parse(
@@ -201,14 +180,26 @@ module.exports = async ({
       );
     }
 
-    if (JSON.stringify(previousAttributes) === JSON.stringify(attributes))
+    if (JSON.stringify(previousAttributes) === JSON.stringify(attributes)) {
       return;
+    }
 
     if (definition.client === 'sqlite3') {
       const tmpTable = `tmp_${table}`;
-      await createTable(tmpTable);
 
-      try {
+      const rebuildTable = async trx => {
+        await trx.schema.renameTable(table, tmpTable);
+
+        // drop possible conflicting indexes
+        await Promise.all(
+          columns.map(key =>
+            trx.raw('DROP INDEX IF EXISTS ??', uniqueColName(table, key))
+          )
+        );
+
+        // create the table
+        await createTable(table, { trx });
+
         const attrs = Object.keys(attributes).filter(attribute =>
           getType({
             definition,
@@ -217,51 +208,50 @@ module.exports = async ({
           })
         );
 
-        await ORM.knex.raw(`INSERT INTO ?? (${attrs.join(', ')}) ??`, [
-          tmpTable,
-          ORM.knex.select(attrs).from(table),
+        await trx.raw(`INSERT INTO ?? (${attrs.join(', ')}) ??`, [
+          table,
+          trx.select(attrs).from(tmpTable),
         ]);
+
+        await trx.schema.dropTableIfExists(tmpTable);
+      };
+
+      try {
+        await ORM.knex.transaction(trx => rebuildTable(trx));
+        await generateIndexes(table);
       } catch (err) {
         strapi.log.error('Migration failed');
         strapi.log.error(err);
-
-        await ORM.knex.schema.dropTableIfExists(tmpTable);
         return false;
       }
-
-      await ORM.knex.schema.dropTableIfExists(table);
-      await ORM.knex.schema.renameTable(tmpTable, table);
-
-      await generateIndexes(table, attributes);
     } else {
-      await ORM.knex.schema.alterTable(table, tbl => {
-        columns.forEach(key => {
-          if (
-            JSON.stringify(previousAttributes[key]) ===
-            JSON.stringify(attributes[key])
-          )
-            return;
+      const columnsToAlter = columns.filter(
+        key =>
+          JSON.stringify(previousAttributes[key]) !==
+          JSON.stringify(attributes[key])
+      );
 
-          const attribute = attributes[key];
-          const type = getType({
-            definition,
-            attribute,
-            name: key,
+      const alterTable = async trx => {
+        await Promise.all(
+          columnsToAlter.map(col => {
+            return ORM.knex.schema
+              .alterTable(table, tbl => {
+                tbl.dropUnique(col, uniqueColName(table, col));
+              })
+              .catch(() => {});
+          })
+        );
+        await trx.schema.alterTable(table, tbl => {
+          alterColumns(tbl, _.pick(attributes, columnsToAlter), {
             tableExists,
           });
-
-          if (type) {
-            let col = tbl.specificType(key, type);
-            if (attribute.required) {
-              col = col.notNullable();
-            }
-            col.alter();
-          }
         });
-      });
+      };
 
-      await storeTable(table, attributes);
+      await ORM.knex.transaction(trx => alterTable(trx));
     }
+
+    await storeTable(table, attributes);
   };
 
   // Add created_at and updated_at field if timestamp option is true
@@ -437,3 +427,30 @@ const storeTable = async (table, attributes) => {
     value: JSON.stringify(attributes),
   }).save();
 };
+
+const defaultIdType = {
+  mysql: 'INT AUTO_INCREMENT NOT NULL PRIMARY KEY',
+  pg: 'SERIAL NOT NULL PRIMARY KEY',
+  sqlite3: 'INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL',
+};
+
+const getIdType = definition => {
+  if (definition.primaryKeyType === 'uuid' && definition.client === 'pg') {
+    return 'uuid NOT NULL DEFAULT uuid_generate_v4() NOT NULL PRIMARY KEY';
+  }
+
+  if (definition.primaryKeyType !== 'integer') {
+    const type = getType({
+      definition,
+      attribute: {
+        type: definition.primaryKeyType,
+      },
+    });
+
+    return `${type} NOT NULL PRIMARY KEY`;
+  }
+
+  return defaultIdType[definition.client];
+};
+
+const uniqueColName = (table, key) => `${table}_${key}_unique`;

packages/strapi-hook-bookshelf/lib/generate-group-relations.js

@@ -37,7 +37,9 @@ module.exports = async ({ model, definition, ORM, GLOBALS }) => {
       };
     });
 
-    await ORM.knex.schema.createTableIfNotExists(joinTable, table => {
+    if (await ORM.knex.schema.hasTable(joinTable)) return;
+
+    await ORM.knex.schema.createTable(joinTable, table => {
       table.increments();
       table.string('field').notNullable();
       table
@@ -45,14 +47,8 @@ module.exports = async ({ model, definition, ORM, GLOBALS }) => {
         .unsigned()
         .notNullable();
       table.string('slice_type').notNullable();
-      table
-        .integer('slice_id')
-        .unsigned()
-        .notNullable();
-      table
-        .integer(joinColumn)
-        .unsigned()
-        .notNullable();
+      table.integer('slice_id').notNullable();
+      table.integer(joinColumn).notNullable();
 
       table
         .foreign(joinColumn)

packages/strapi-hook-bookshelf/lib/mount-models.js

@@ -114,8 +114,6 @@ module.exports = ({ models, target, plugin = false }, ctx) => {
       global[definition.globalName] = {};
     }
 
-    await genGroupRelatons({ model: loadedModel, definition, ORM, GLOBALS });
-
     // Add every relationships to the loaded model for Bookshelf.
     // Basic attributes don't need this-- only relations.
     Object.keys(definition.attributes).forEach(name => {
@@ -709,13 +707,15 @@ module.exports = ({ models, target, plugin = false }, ctx) => {
       target[model]._attributes = definition.attributes;
       target[model].updateRelations = relations.update;
 
-      return buildDatabaseSchema({
+      await buildDatabaseSchema({
         ORM,
         definition,
         loadedModel,
         connection,
         model: target[model],
       });
+
+      await genGroupRelatons({ model: loadedModel, definition, ORM, GLOBALS });
     } catch (err) {
       strapi.log.error(`Impossible to register the '${model}' model.`);
       strapi.log.error(err);

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

@@ -8,12 +8,10 @@
 
 const _ = require('lodash');
 
-const sanitizeUser = user => {
-  return _.omit(user.toJSON ? user.toJSON() : user, [
-    'password',
-    'resetPasswordToken',
-  ]);
-};
+const sanitizeUser = user => _.omit(user, ['password', 'resetPasswordToken']);
+const adminError = error => [
+  { messages: [{ id: error.message, field: error.field }] },
+];
 
 module.exports = {
   /**
@@ -87,49 +85,72 @@ module.exports = {
       })
       .get();
 
-    if (advanced.unique_email && ctx.request.body.email) {
+    const { email, username, password, role } = ctx.request.body;
+
+    if (!email) return ctx.badRequest('missing.email');
+    if (!username) return ctx.badRequest('missing.username');
+    if (!password) return ctx.badRequest('missing.password');
+
+    const adminsWithSameUsername = await strapi
+      .query('user', 'users-permissions')
+      .findOne({ username });
+
+    if (adminsWithSameUsername) {
+      return ctx.badRequest(
+        null,
+        ctx.request.admin
+          ? adminError({
+              message: 'Auth.form.error.username.taken',
+              field: ['username'],
+            })
+          : 'username.alreadyTaken.'
+      );
+    }
+
+    if (advanced.unique_email) {
       const user = await strapi
         .query('user', 'users-permissions')
-        .findOne({ email: ctx.request.body.email });
+        .findOne({ email });
 
       if (user) {
         return ctx.badRequest(
           null,
           ctx.request.admin
-            ? [
-                {
-                  messages: [
-                    { id: 'Auth.form.error.email.taken', field: ['email'] },
-                  ],
-                },
-              ]
-            : 'Email is already taken.'
+            ? adminError({
+                message: 'Auth.form.error.email.taken',
+                field: ['email'],
+              })
+            : 'email.alreadyTaken'
         );
       }
     }
 
-    if (!ctx.request.body.role) {
+    const user = {
+      email,
+      username,
+      password,
+      role,
+      provider: 'local',
+    };
+
+    if (!role) {
       const defaultRole = await strapi
         .query('role', 'users-permissions')
         .findOne({ type: advanced.default_role }, []);
 
-      ctx.request.body.role = defaultRole._id || defaultRole.id;
+      user.role = defaultRole.id;
     }
 
-    ctx.request.body.provider = 'local';
-
     try {
       const data = await strapi.plugins['users-permissions'].services.user.add(
-        ctx.request.body
+        user
       );
 
       ctx.created(data);
     } catch (error) {
       ctx.badRequest(
         null,
-        ctx.request.admin
-          ? [{ messages: [{ id: error.message, field: error.field }] }]
-          : error.message
+        ctx.request.admin ? adminError(error) : error.message
       );
     }
   },
@@ -166,13 +187,10 @@ module.exports = {
           return ctx.badRequest(
             null,
             ctx.request.admin
-              ? [
-                  {
-                    messages: [
-                      { id: 'Auth.form.error.email.taken', field: ['email'] },
-                    ],
-                  },
-                ]
+              ? adminError({
+                  message: 'Auth.form.error.email.taken',
+                  field: ['email'],
+                })
               : 'Email is already taken.'
           );
         }
@@ -206,13 +224,10 @@ module.exports = {
           return ctx.badRequest(
             null,
             ctx.request.admin
-              ? [
-                  {
-                    messages: [
-                      { id: 'Auth.form.error.email.taken', field: ['email'] },
-                    ],
-                  },
-                ]
+              ? adminError({
+                  message: 'Auth.form.error.email.taken',
+                  field: ['email'],
+                })
               : 'Email is already taken.'
           );
         }

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

@@ -22,19 +22,6 @@ module.exports = {
       ].services.user.hashPassword(values);
     }
 
-    // Use Content Manager business logic to handle relation.
-    if (strapi.plugins['content-manager']) {
-      return await strapi.plugins['content-manager'].services[
-        'contentmanager'
-      ].add(
-        {
-          model: 'user',
-        },
-        values,
-        'users-permissions'
-      );
-    }
-
     return strapi.query('user', 'users-permissions').create(values);
   },