code review changes

packages/tests/src/api/check-params/users-emails.ts

@@ -62,17 +62,6 @@ describe('Test users API validators', function () {
       await makePostBodyRequest({ url: server.url, path, fields })
     })
 
-    it('Should fail with wrong capitalization when multiple users with similar email exists', async function () {
-      const fields = { email: 'USER@example.com' }
-
-      await makePostBodyRequest({
-        url: server.url,
-        path,
-        fields,
-        expectedStatus: HttpStatusCode.NO_CONTENT_204
-      })
-    })
-
     it('Should success with correct capitalization when multiple users with similar email exists', async function () {
       const fields = { email: 'User@example.com' }
 
@@ -149,17 +138,6 @@ describe('Test users API validators', function () {
       await makePostBodyRequest({ url: server.url, path, fields })
     })
 
-    it('Should fail with wrong capitalization when multiple users with similar email exists', async function () {
-      const fields = { email: 'REQUEST1@example.com' }
-
-      await makePostBodyRequest({
-        url: server.url,
-        path,
-        fields,
-        expectedStatus: HttpStatusCode.NO_CONTENT_204
-      })
-    })
-
     it('Should success with wrong capitalization when no similar emails exists', async function () {
       const fields = { email: 'REQUEST2@example.com' }
 

packages/tests/src/api/server/email.ts

@@ -31,6 +31,16 @@ describe('Test emails', function () {
     username: 'user_1',
     password: 'super_password'
   }
+  const similarUsers = [
+    {
+      username: 'lowercase_user_1',
+      email: 'lowercase_user_1@example.com'
+    },
+    {
+      username: 'lowercase_user__1',
+      email: 'Lowercase_user_1@example.com'
+    }
+  ]
 
   before(async function () {
     this.timeout(120000)
@@ -41,6 +51,10 @@ describe('Test emails', function () {
     await setAccessTokensToServers([ server ])
     await server.config.enableSignup(true)
 
+    for (const user of similarUsers) {
+      await server.users.create(user)
+    }
+
     {
       const created = await server.users.create({ username: user.username, password: user.password })
       userId = created.id
@@ -101,6 +115,10 @@ describe('Test emails', function () {
       })
     })
 
+    it('Should fail with wrong capitalization when multiple users with similar email exists', async function () {
+      await server.users.askResetPassword({ email: similarUsers[0].username.toUpperCase(), expectedStatus: HttpStatusCode.BAD_REQUEST_400 })
+    })
+
     it('Should reset the password', async function () {
       await server.users.resetPassword({ userId, verificationString, password: 'super_password2' })
     })
@@ -269,6 +287,13 @@ describe('Test emails', function () {
 
   describe('When verifying a user email', function () {
 
+    it('Should fail with wrong capitalization when multiple users with similar email exists', async function () {
+      await server.users.askSendVerifyEmail({
+        email: similarUsers[0].username.toUpperCase(),
+        expectedStatus: HttpStatusCode.BAD_REQUEST_400
+      })
+    })
+
     it('Should ask to send the verification email', async function () {
       await server.users.askSendVerifyEmail({ email: 'user_1@example.com' })
 

packages/tests/src/api/users/oauth.ts

@@ -81,7 +81,7 @@ describe('Test oauth', function () {
     })
 
     it('Should not login with an invalid password', async function () {
-      const user = { username: 'User@example.com', password: 'password' }
+      const user = { username: server.store.user.username, password: 'mew_three' }
       const body = await server.login.login({ user, expectedStatus: HttpStatusCode.BAD_REQUEST_400 })
 
       expectInvalidCredentials(body)
@@ -112,6 +112,14 @@ describe('Test oauth', function () {
       const user2 = { username: 'admin' + server.internalServerNumber + '@example.com', password: server.store.user.password }
       await server.login.login({ user: user2, expectedStatus: HttpStatusCode.OK_200 })
     })
+
+    it('Should not be able to login with an insensitive email when similar emails exist', async function () {
+      const user = { username: 'uSer@example.com', password: 'AdvancedPassword' }
+      await server.login.login({ user, expectedStatus: HttpStatusCode.BAD_REQUEST_400 })
+
+      const user2 = { username: 'User@example.com', password: 'AdvancedPassword' }
+      await server.login.login({ user: user2, expectedStatus: HttpStatusCode.OK_200 })
+    })
   })
 
   describe('Logout', function () {

server/core/lib/auth/oauth-model.ts

@@ -124,7 +124,7 @@ async function getUser (usernameOrEmail?: string, password?: string, bypassLogin
 
   if (usernameOrEmail.includes('@')) {
     user = getUserByEmailPermissive(users, usernameOrEmail)
-  } else {
+  } else if (users.length === 1) {
     user = users[0]
   }
 

server/core/middlewares/validators/shared/users.ts

@@ -20,9 +20,10 @@ export function checkUserEmailExistPermissive (email: string, res: express.Respo
 }
 
 export async function checkUserNameOrEmailDoNotAlreadyExist (username: string, email: string, res: express.Response) {
-  const user = await UserModel.loadByUsernameOrEmail(username, email)
+  const existingUser = await UserModel.loadByUsernameOrEmailCaseInsensitive(username)
+  const existingEmail = await UserModel.loadByUsernameOrEmailCaseInsensitive(email)
 
-  if (user) {
+  if (existingUser.length > 0 || existingEmail.length > 0) {
     res.fail({
       status: HttpStatusCode.CONFLICT_409,
       message: 'User with this username or email already exists.'

server/core/models/user/user.ts

@@ -663,16 +663,6 @@ export class UserModel extends SequelizeModel<UserModel> {
     return UserModel.scope(ScopeNames.FOR_ME_API).findOne(query)
   }
 
-  static loadByEmail (email: string): Promise<MUserDefault> {
-    const query = {
-      where: {
-        email
-      }
-    }
-
-    return UserModel.findOne(query)
-  }
-
   static loadByEmailCaseInsensitive (email: string): Promise<MUserDefault[]> {
     const query = {
       where: where(
@@ -685,22 +675,6 @@ export class UserModel extends SequelizeModel<UserModel> {
     return UserModel.findAll(query)
   }
 
-  static loadByUsernameOrEmail (username: string, email?: string): Promise<MUserDefault> {
-    if (!email) email = username
-
-    const query = {
-      where: {
-        [Op.or]: [
-          where(fn('lower', col('username')), fn('lower', username) as any),
-
-          { email }
-        ]
-      }
-    }
-
-    return UserModel.findOne(query)
-  }
-
   static loadByUsernameOrEmailCaseInsensitive (usernameOrEmail: string): Promise<MUserDefault[]> {
     const query = {
       where: {