From 203114cd5e41ed64dbe886bb4960eebb8402f0e6 Mon Sep 17 00:00:00 2001
From: wancat <wancat@wancat.cc>
Date: Wed, 1 Nov 2023 21:35:25 +0800
Subject: [PATCH] feat: add validation for login

---
 app/src/views/LoginPage.vue                |  1 +
 server/src/api/routes/auth.js              | 23 +++++--------
 server/src/api/validators/authValidator.js | 38 ++++++++++++++++++----
 server/src/auth/passwordStrategy.js        |  2 +-
 server/src/middleware/error.js             |  2 +-
 server/src/middleware/validator.js         |  2 +-
 6 files changed, 44 insertions(+), 24 deletions(-)

diff --git a/app/src/views/LoginPage.vue b/app/src/views/LoginPage.vue
index e9e515a..35e354c 100644
--- a/app/src/views/LoginPage.vue
+++ b/app/src/views/LoginPage.vue
@@ -11,6 +11,7 @@ const handleLogin = async ({ username, password, success, error }) => {
     else error('Wrong username or password.')
   } catch (err) {
     console.error(err)
+    error('Something went wrong.')
   }
 }
 </script>
diff --git a/server/src/api/routes/auth.js b/server/src/api/routes/auth.js
index 85d9196..eb42353 100644
--- a/server/src/api/routes/auth.js
+++ b/server/src/api/routes/auth.js
@@ -11,34 +11,28 @@ import {
   registerFromSession,
 } from '../controllers/authController.js'
 import {
-  checkNewEmail,
-  checkNewUsername,
-  checkPassword,
+  checkUsernameValidators,
+  loginValidators,
+  registerFromSessionValidators,
+  registerValidators,
 } from '../validators/authValidator.js'
-import { noError } from '../../middleware/validator.js'
 
 const router = Router()
 
 router.post(
   '/login',
+  loginValidators,
   passport.authenticate('local', { failureMessage: true }),
   (req, res) => {
     res.json({ message: 'success' })
   }
 )
 
-router.post(
-  '/register',
-  checkNewUsername(),
-  checkNewEmail(),
-  checkPassword(),
-  noError,
-  register
-)
+router.post('/register', registerValidators, register)
 
 router.post('/logout', logout)
 
-router.get('/check-username', checkNewUsername(), noError, (req, res) =>
+router.get('/check-username', checkUsernameValidators, (req, res) =>
   res.json({ available: true })
 )
 router.get('/check-session', checkSession)
@@ -59,8 +53,7 @@ if (enabledMethods.github) {
 
 router.post(
   '/register-from-session',
-  checkNewUsername(),
-  noError,
+  registerFromSessionValidators,
   registerFromSession
 )
 
diff --git a/server/src/api/validators/authValidator.js b/server/src/api/validators/authValidator.js
index dbb5b11..69ded44 100644
--- a/server/src/api/validators/authValidator.js
+++ b/server/src/api/validators/authValidator.js
@@ -1,10 +1,11 @@
 import { check } from 'express-validator'
 
-import { isValidUsername } from '@linux-odyssey/utils'
+import { isValidUsername, isValidEmail } from '@linux-odyssey/utils'
 import { User } from '@linux-odyssey/models'
 import { passwordPolicy } from '@linux-odyssey/constants'
+import { noError } from '../../middleware/validator.js'
 
-export const checkUsername = () =>
+const checkUsername = () =>
   check('username')
     .notEmpty()
     .isString()
@@ -16,7 +17,7 @@ export const checkUsername = () =>
     })
     .trim()
 
-export const checkNewUsername = () =>
+const checkNewUsername = () =>
   checkUsername().custom(async (username) => {
     if (await User.exists({ username })) {
       throw new Error('Username taken')
@@ -24,9 +25,9 @@ export const checkNewUsername = () =>
     return true
   })
 
-export const checkEmail = () => check('email').notEmpty().isEmail().trim()
+const checkEmail = () => check('email').notEmpty().isString().isEmail().trim()
 
-export const checkNewEmail = () =>
+const checkNewEmail = () =>
   checkEmail().custom(async (email) => {
     if (await User.exists({ email })) {
       throw new Error('Email taken')
@@ -34,5 +35,30 @@ export const checkNewEmail = () =>
     return true
   })
 
-export const checkPassword = () =>
+const checkPassword = () =>
   check('password').notEmpty().isStrongPassword(passwordPolicy)
+
+const checkLogin = () =>
+  check('username')
+    .isString()
+    .notEmpty()
+    .trim()
+    .custom((login) => {
+      if (!isValidUsername(login) && !isValidEmail(login)) {
+        throw new Error('Invalid login')
+      }
+      return true
+    })
+
+export const registerValidators = [
+  checkNewUsername(),
+  checkNewEmail(),
+  checkPassword(),
+  noError,
+]
+
+export const registerFromSessionValidators = [checkNewUsername(), noError]
+
+export const loginValidators = [checkLogin(), noError]
+
+export const checkUsernameValidators = [checkUsername(), noError]
diff --git a/server/src/auth/passwordStrategy.js b/server/src/auth/passwordStrategy.js
index 0993a9b..6334178 100644
--- a/server/src/auth/passwordStrategy.js
+++ b/server/src/auth/passwordStrategy.js
@@ -8,7 +8,7 @@ async function verifyPassword(username, password, done) {
       $or: [{ username }, { email: username }],
     })
     if (!user) {
-      return done(null, false, { message: 'Incorrect username.' })
+      return done(null, false, { message: 'Incorrect username or password.' })
     }
 
     const isPasswordValid = await bcrypt.compare(password, user.hashedPassword)
diff --git a/server/src/middleware/error.js b/server/src/middleware/error.js
index fc1be7e..5af9688 100644
--- a/server/src/middleware/error.js
+++ b/server/src/middleware/error.js
@@ -1,5 +1,5 @@
 // eslint-disable-next-line no-unused-vars
 export default function errorHandler(err, req, res, next) {
   console.error(err.stack) // Log error stack trace to the console
-  res.status(500).send({ message: err.message }) // Send error response
+  res.status(500).send({ message: 'Something broke!' }) // Send error response
 }
diff --git a/server/src/middleware/validator.js b/server/src/middleware/validator.js
index 8cda838..891886b 100644
--- a/server/src/middleware/validator.js
+++ b/server/src/middleware/validator.js
@@ -6,8 +6,8 @@ export function noError(req, res, next) {
     next()
     return
   }
-  console.log(result.array())
   res.status(400).json({
+    message: 'Validation failed',
     errors: result.array(),
   })
 }