Merge pull request #272 from developmentseed/fix/error-handling-middleware

Improve request error handling

Author: vgeorge

app/index.js

@@ -58,6 +58,9 @@ async function init () {
    * Error handler
    */
   app.use(function (err, req, res, next) {
+    if (err.message === 'Forbidden') {
+      return nextApp.render(req, res, '/uh-oh')
+    }
     res.status(err.status || 500)
     console.error('error', err)
     res.boom.internal('An internal error occurred.')

app/lib/osm.js

@@ -85,7 +85,7 @@ function openstreetmap (req, res) {
         hydra.acceptLoginRequest(challenge, {
           subject: user.id,
           remember: true,
-          remember_for: 9999
+          remember_for: 0
         }).then(response => {
           if (response.redirect_to) {
             return res.redirect(response.redirect_to)

app/manage/permissions/clients.js

@@ -11,10 +11,14 @@ const db = require('../../db')
  * @returns {boolean} can the request go through?
  */
 async function clients (uid) {
-  let conn = await db()
-  const [user] = await conn('users').where('id', uid)
-  if (user) {
-    return true
+  try {
+    let conn = await db()
+    const [user] = await conn('users').where('id', uid)
+    if (user) {
+      return true
+    }
+  } catch (error) {
+    throw Error('Forbidden')
   }
 }
 

app/manage/permissions/index.js

@@ -43,6 +43,10 @@ const permissions = mergeAll([
   organizationPermissions
 ])
 
+function isApiRequest ({ path }) {
+  return path.indexOf('/api') === 0
+}
+
 /**
  * Check if a user has a specific permission
  *
@@ -159,18 +163,27 @@ function check (ability) {
       if (allowed) {
         next()
       } else {
-        res.boom.unauthorized('Forbidden')
+        if (isApiRequest(req)) {
+          res.boom.unauthorized('Forbidden')
+        } else {
+          next(new Error('Forbidden'))
+        }
       }
     } catch (e) {
       console.error('error checking permission', e)
-      // An error occurred checking the permissions
-      // if user id is missing it's an authentication problem
-      if (e.message.includes('osm id is required')) {
-        return res.boom.unauthorized('Forbidden')
-      }
 
-      // otherwise it could be the resource not existing, we send 404
-      res.boom.notFound('Could not find resource')
+      if (isApiRequest(req)) {
+        // Handle API request errors
+        if (e.message.includes('osm id is required')) {
+          return res.boom.unauthorized('Forbidden')
+        }
+
+        // otherwise it could be the resource not existing, we send 404
+        res.boom.notFound('Could not find resource')
+      } else {
+        // This should be web page errors, which are handled at app/index.js#L60
+        next(new Error('Forbidden'))
+      }
     }
   }
 }

pages/uh-oh.js

@@ -0,0 +1,14 @@
+import React, { Component } from 'react'
+
+export default class UhOh extends Component {
+  render () {
+    return (
+      <article className='inner page'>
+        <h1>Page not found</h1>
+        <div>Sorry, the page you are looking for is not available.</div>
+        <br />
+        <div>Still having problems? Try logging out and back in or contacting a system administrator.</div>
+      </article>
+    )
+  }
+}