Merge pull request #1416 from hackmdio/fix/user-type-error

Fix some issues after code refactoring

app.js

@@ -222,7 +222,7 @@ app.use(require('./lib/routes').router)
 
 // response not found if no any route matxches
 app.get('*', function (req, res) {
-  response.errorNotFound(res)
+  response.errorNotFound(req, res)
 })
 
 // socket.io secure

lib/auth/email/index.js

@@ -33,8 +33,8 @@ passport.use(new LocalStrategy({
 
 if (config.allowEmailRegister) {
   emailAuth.post('/register', urlencodedParser, function (req, res, next) {
-    if (!req.body.email || !req.body.password) return response.errorBadRequest(res)
+    if (!req.body.email || !req.body.password) return response.errorBadRequest(req, res)
-    if (!validator.isEmail(req.body.email)) return response.errorBadRequest(res)
+    if (!validator.isEmail(req.body.email)) return response.errorBadRequest(req, res)
     models.User.findOrCreate({
       where: {
         email: req.body.email
@@ -57,14 +57,14 @@ if (config.allowEmailRegister) {
       return res.redirect(config.serverURL + '/')
     }).catch(function (err) {
       logger.error('auth callback failed: ' + err)
-      return response.errorInternalError(res)
+      return response.errorInternalError(req, res)
     })
   })
 }
 
 emailAuth.post('/login', urlencodedParser, function (req, res, next) {
-  if (!req.body.email || !req.body.password) return response.errorBadRequest(res)
+  if (!req.body.email || !req.body.password) return response.errorBadRequest(req, res)
-  if (!validator.isEmail(req.body.email)) return response.errorBadRequest(res)
+  if (!validator.isEmail(req.body.email)) return response.errorBadRequest(req, res)
   setReturnToFromReferer(req)
   passport.authenticate('local', {
     successReturnToOrRedirect: config.serverURL + '/',

lib/auth/ldap/index.js

@@ -81,7 +81,7 @@ passport.use(new LDAPStrategy({
 }))
 
 ldapAuth.post('/auth/ldap', urlencodedParser, function (req, res, next) {
-  if (!req.body.username || !req.body.password) return response.errorBadRequest(res)
+  if (!req.body.username || !req.body.password) return response.errorBadRequest(req, res)
   setReturnToFromReferer(req)
   passport.authenticate('ldapauth', {
     successReturnToOrRedirect: config.serverURL + '/',

lib/history/index.js

@@ -116,14 +116,14 @@ function parseHistoryToObject (history) {
 function historyGet (req, res) {
   if (req.isAuthenticated()) {
     getHistory(req.user.id, function (err, history) {
-      if (err) return response.errorInternalError(res)
+      if (err) return response.errorInternalError(req, res)
-      if (!history) return response.errorNotFound(res)
+      if (!history) return response.errorNotFound(req, res)
       res.send({
         history: parseHistoryToArray(history)
       })
     })
   } else {
-    return response.errorForbidden(res)
+    return response.errorForbidden(req, res)
   }
 }
 
@@ -131,40 +131,40 @@ function historyPost (req, res) {
   if (req.isAuthenticated()) {
     var noteId = req.params.noteId
     if (!noteId) {
-      if (typeof req.body['history'] === 'undefined') return response.errorBadRequest(res)
+      if (typeof req.body['history'] === 'undefined') return response.errorBadRequest(req, res)
       if (config.debug) { logger.info('SERVER received history from [' + req.user.id + ']: ' + req.body.history) }
       try {
         var history = JSON.parse(req.body.history)
       } catch (err) {
-        return response.errorBadRequest(res)
+        return response.errorBadRequest(req, res)
       }
       if (Array.isArray(history)) {
         setHistory(req.user.id, history, function (err, count) {
-          if (err) return response.errorInternalError(res)
+          if (err) return response.errorInternalError(req, res)
           res.end()
         })
       } else {
-        return response.errorBadRequest(res)
+        return response.errorBadRequest(req, res)
       }
     } else {
-      if (typeof req.body['pinned'] === 'undefined') return response.errorBadRequest(res)
+      if (typeof req.body['pinned'] === 'undefined') return response.errorBadRequest(req, res)
       getHistory(req.user.id, function (err, history) {
-        if (err) return response.errorInternalError(res)
+        if (err) return response.errorInternalError(req, res)
-        if (!history) return response.errorNotFound(res)
+        if (!history) return response.errorNotFound(req, res)
-        if (!history[noteId]) return response.errorNotFound(res)
+        if (!history[noteId]) return response.errorNotFound(req, res)
         if (req.body.pinned === 'true' || req.body.pinned === 'false') {
           history[noteId].pinned = (req.body.pinned === 'true')
           setHistory(req.user.id, history, function (err, count) {
-            if (err) return response.errorInternalError(res)
+            if (err) return response.errorInternalError(req, res)
             res.end()
           })
         } else {
-          return response.errorBadRequest(res)
+          return response.errorBadRequest(req, res)
         }
       })
     }
   } else {
-    return response.errorForbidden(res)
+    return response.errorForbidden(req, res)
   }
 }
 
@@ -173,22 +173,22 @@ function historyDelete (req, res) {
     var noteId = req.params.noteId
     if (!noteId) {
       setHistory(req.user.id, [], function (err, count) {
-        if (err) return response.errorInternalError(res)
+        if (err) return response.errorInternalError(req, res)
         res.end()
       })
     } else {
       getHistory(req.user.id, function (err, history) {
-        if (err) return response.errorInternalError(res)
+        if (err) return response.errorInternalError(req, res)
-        if (!history) return response.errorNotFound(res)
+        if (!history) return response.errorNotFound(req, res)
         delete history[noteId]
         setHistory(req.user.id, history, function (err, count) {
-          if (err) return response.errorInternalError(res)
+          if (err) return response.errorInternalError(req, res)
           res.end()
         })
       })
     }
   } else {
-    return response.errorForbidden(res)
+    return response.errorForbidden(req, res)
   }
 }
 

lib/imageRouter/index.js

@@ -21,7 +21,7 @@ imageRouter.post('/uploadimage', function (req, res) {
 
   form.parse(req, function (err, fields, files) {
     if (err || !files.image || !files.image.path) {
-      response.errorForbidden(res)
+      response.errorForbidden(req, res)
     } else {
       if (config.debug) {
         logger.info('SERVER received uploadimage: ' + JSON.stringify(files.image))

lib/middleware/checkURIValid.js

@@ -8,7 +8,7 @@ module.exports = function (req, res, next) {
     decodeURIComponent(req.path)
   } catch (err) {
     logger.error(err)
-    return response.errorBadRequest(res)
+    return response.errorBadRequest(req, res)
   }
   next()
 }

lib/middleware/tooBusy.js

@@ -9,7 +9,7 @@ toobusy.maxLag(config.responseMaxLag)
 
 module.exports = function (req, res, next) {
   if (toobusy()) {
-    response.errorServiceUnavailable(res)
+    response.errorServiceUnavailable(req, res)
   } else {
     next()
   }

lib/note/index.js

@@ -60,13 +60,13 @@ async function showNote (req, res) {
   if (!note) {
     // if allow free url enable, auto create note
     if (!config.allowFreeURL || config.forbiddenNoteIDs.includes(noteId)) {
-      return errorNotFound(res)
+      return errorNotFound(req, res)
     }
     note = await createNote(userId, noteId)
   }
 
   if (!newCheckViewPermission(note, req.isAuthenticated(), userId)) {
-    return errorForbidden(res)
+    return errorForbidden(req, res)
   }
 
   // force to use note id
@@ -94,12 +94,12 @@ async function showPublishNote (req, res) {
     includeUser: true
   })
 
-  if (!canViewNote(note, req.isAuthenticated(), req.user ? req.user.id : null)) {
+  if (!note) {
-    return errorForbidden(req)
+    return errorNotFound(req, res)
   }
 
-  if (!note) {
+  if (!canViewNote(note, req.isAuthenticated(), req.user ? req.user.id : null)) {
-    return errorNotFound(res)
+    return errorForbidden(req, res)
   }
 
   if ((note.alias && shortid !== note.alias) || (!note.alias && shortid !== note.shortid)) {
@@ -146,11 +146,11 @@ async function noteActions (req, res) {
   const note = await getNoteById(noteId)
 
   if (!note) {
-    return errorNotFound(res)
+    return errorNotFound(req, res)
   }
 
   if (!canViewNote(note, req.isAuthenticated(), req.user ? req.user.id : null)) {
-    return errorForbidden(req)
+    return errorForbidden(req, res)
   }
 
   const action = req.params.action
@@ -171,7 +171,7 @@ async function noteActions (req, res) {
         actionPDF(req, res, note)
       } else {
         logger.error('PDF export failed: Disabled by config. Set "allowPDFExport: true" to enable. Check the documentation for details')
-        errorForbidden(res)
+        errorForbidden(req, res)
       }
       break
     case 'gist':

lib/note/noteActions.js

@@ -84,7 +84,7 @@ function actionPDF (req, res, note) {
   markdownpdf(markdownpdfOptions).from.string(content).to(pdfPath, function () {
     if (!fs.existsSync(pdfPath)) {
       logger.error('PDF seems to not be generated as expected. File doesn\'t exist: ' + pdfPath)
-      return errorInternalError(res)
+      return errorInternalError(req, res)
     }
     const stream = fs.createReadStream(pdfPath)
     let filename = title
@@ -173,15 +173,15 @@ function actionRevision (req, res, note) {
   if (actionId) {
     const time = moment(parseInt(actionId))
     if (!time.isValid()) {
-      return errorNotFound(res)
+      return errorNotFound(req, res)
     }
     Revision.getPatchedNoteRevisionByTime(note, time, function (err, content) {
       if (err) {
         logger.error(err)
-        return errorInternalError(res)
+        return errorInternalError(req, res)
       }
       if (!content) {
-        return errorNotFound(res)
+        return errorNotFound(req, res)
       }
       res.set({
         'Access-Control-Allow-Origin': '*', // allow CORS as API
@@ -196,7 +196,7 @@ function actionRevision (req, res, note) {
     Revision.getNoteRevisions(note, function (err, data) {
       if (err) {
         logger.error(err)
-        return errorInternalError(res)
+        return errorInternalError(req, res)
       }
       const result = {
         revision: data

lib/response.js

@@ -28,8 +28,7 @@ exports.checkViewPermission = checkViewPermission
 exports.newCheckViewPermission = newCheckViewPermission
 exports.responseCodiMD = responseCodiMD
 
-function errorForbidden (res) {
+function errorForbidden (req, res) {
-  const { req } = res
   if (req.user) {
     responseError(res, '403', 'Forbidden', 'oh no.')
   } else {
@@ -38,23 +37,23 @@ function errorForbidden (res) {
   }
 }
 
-function errorNotFound (res) {
+function errorNotFound (req, res) {
   responseError(res, '404', 'Not Found', 'oops.')
 }
 
-function errorBadRequest (res) {
+function errorBadRequest (req, res) {
   responseError(res, '400', 'Bad Request', 'something not right.')
 }
 
-function errorTooLong (res) {
+function errorTooLong (req, res) {
   responseError(res, '413', 'Payload Too Large', 'Shorten your note!')
 }
 
-function errorInternalError (res) {
+function errorInternalError (req, res) {
   responseError(res, '500', 'Internal Error', 'wtf.')
 }
 
-function errorServiceUnavailable (res) {
+function errorServiceUnavailable (req, res) {
   res.status(503).send('I\'m busy right now, try again later.')
 }
 
@@ -92,7 +91,7 @@ function newNote (req, res, next) {
   var owner = null
   var body = ''
   if (req.body && req.body.length > config.documentMaxLength) {
-    return errorTooLong(res)
+    return errorTooLong(req, res)
   } else if (req.body) {
     body = req.body
   }
@@ -100,7 +99,7 @@ function newNote (req, res, next) {
   if (req.isAuthenticated()) {
     owner = req.user.id
   } else if (!config.allowAnonymous) {
-    return errorForbidden(res)
+    return errorForbidden(req, res)
   }
   models.Note.create({
     ownerId: owner,
@@ -114,7 +113,7 @@ function newNote (req, res, next) {
     return res.redirect(config.serverURL + '/' + models.Note.encodeNoteId(note.id))
   }).catch(function (err) {
     logger.error(err)
-    return errorInternalError(res)
+    return errorInternalError(req, res)
   })
 }
 
@@ -144,7 +143,7 @@ function findNote (req, res, callback, include) {
   models.Note.parseNoteId(id, function (err, _id) {
     if (err) {
       logger.error(err)
-      return errorInternalError(res)
+      return errorInternalError(req, res)
     }
     models.Note.findOne({
       where: {
@@ -157,17 +156,17 @@ function findNote (req, res, callback, include) {
           req.alias = noteId
           return newNote(req, res)
         } else {
-          return errorNotFound(res)
+          return errorNotFound(req, res)
         }
       }
       if (!checkViewPermission(req, note)) {
-        return errorForbidden(res)
+        return errorForbidden(req, res)
       } else {
         return callback(note)
       }
     }).catch(function (err) {
       logger.error(err)
-      return errorInternalError(res)
+      return errorInternalError(req, res)
     })
   })
 }
@@ -239,7 +238,7 @@ function githubActionGist (req, res, note) {
   var code = req.query.code
   var state = req.query.state
   if (!code || !state) {
-    return errorForbidden(res)
+    return errorForbidden(req, res)
   } else {
     var data = {
       client_id: config.github.clientID,
@@ -279,14 +278,14 @@ function githubActionGist (req, res, note) {
               res.setHeader('referer', '')
               res.redirect(body.html_url)
             } else {
-              return errorForbidden(res)
+              return errorForbidden(req, res)
             }
           })
         } else {
-          return errorForbidden(res)
+          return errorForbidden(req, res)
         }
       } else {
-        return errorForbidden(res)
+        return errorForbidden(req, res)
       }
     })
   }
@@ -314,7 +313,7 @@ function gitlabActionProjects (req, res, note) {
         id: req.user.id
       }
     }).then(function (user) {
-      if (!user) { return errorNotFound(res) }
+      if (!user) { return errorNotFound(req, res) }
       var ret = { baseURL: config.gitlab.baseURL, version: config.gitlab.version }
       ret.accesstoken = user.accessToken
       ret.profileid = user.profileid
@@ -331,10 +330,10 @@ function gitlabActionProjects (req, res, note) {
       )
     }).catch(function (err) {
       logger.error('gitlab action projects failed: ' + err)
-      return errorInternalError(res)
+      return errorInternalError(req, res)
     })
   } else {
-    return errorForbidden(res)
+    return errorForbidden(req, res)
   }
 }
 
@@ -352,7 +351,7 @@ function showPublishSlide (req, res, next) {
     if ((note.alias && shortid !== note.alias) || (!note.alias && shortid !== note.shortid)) { return res.redirect(config.serverURL + '/p/' + (note.alias || note.shortid)) }
     note.increment('viewcount').then(function (note) {
       if (!note) {
-        return errorNotFound(res)
+        return errorNotFound(req, res)
       }
       var body = note.content
       var extracted = models.Note.extractMeta(body)
@@ -386,7 +385,7 @@ function showPublishSlide (req, res, next) {
       res.render('slide.ejs', data)
     }).catch(function (err) {
       logger.error(err)
-      return errorInternalError(res)
+      return errorInternalError(req, res)
     })
   }, include)
 }

lib/user/index.js

@@ -11,7 +11,7 @@ const { generateAvatar } = require('../letter-avatars')
 
 exports.getMe = async (req, res) => {
   if (!req.isAuthenticated()) {
-    res.status(401).send({
+    return res.status(401).send({
       status: 'forbidden'
     })
   }
@@ -23,7 +23,7 @@ exports.getMe = async (req, res) => {
   })
 
   if (!user) {
-    return response.errorNotFound(res)
+    return response.errorNotFound(req, res)
   }
   const profile = models.User.getProfile(user)
 
@@ -37,7 +37,7 @@ exports.getMe = async (req, res) => {
 
 exports.deleteUser = async (req, res) => {
   if (!req.isAuthenticated()) {
-    return response.errorForbidden(res)
+    return response.errorForbidden(req, res)
   }
 
   const user = await models.User.findOne({
@@ -47,11 +47,11 @@ exports.deleteUser = async (req, res) => {
   })
 
   if (!user) {
-    return response.errorNotFound(res)
+    return response.errorNotFound(req, res)
   }
 
   if (user.deleteToken !== req.params.token) {
-    return response.errorForbidden(res)
+    return response.errorForbidden(req, res)
   }
 
   await user.destroy()
@@ -60,7 +60,7 @@ exports.deleteUser = async (req, res) => {
 
 exports.exportMyData = (req, res) => {
   if (!req.isAuthenticated()) {
-    return response.errorForbidden(res)
+    return response.errorForbidden(req, res)
   }
 
   const archive = archiver('zip', {
@@ -72,7 +72,7 @@ exports.exportMyData = (req, res) => {
   archive.pipe(res)
   archive.on('error', function (err) {
     logger.error('export user data failed: ' + err)
-    return response.errorInternalError(res)
+    return response.errorInternalError(req, res)
   })
 
   models.User.findOne({
@@ -102,7 +102,7 @@ exports.exportMyData = (req, res) => {
         callback(null, null)
       }, function (err) {
         if (err) {
-          return response.errorInternalError(res)
+          return response.errorInternalError(req, res)
         }
 
         archive.finalize()
@@ -110,7 +110,7 @@ exports.exportMyData = (req, res) => {
     })
   }).catch(function (err) {
     logger.error('export user data failed: ' + err)
-    return response.errorInternalError(res)
+    return response.errorInternalError(req, res)
   })
 }