More granular error responses

server/api/__snapshots__/team.test.js.snap

@@ -17,9 +17,10 @@ Object {
 
 exports[`#team.addAdmin should require admin 1`] = `
 Object {
-  "error": "authorization_error",
-  "message": "Authorization Required",
+  "error": "admin_required",
+  "message": "An admin role is required to access this resource",
   "ok": false,
+  "status": 403,
 }
 `;
 
@@ -40,15 +41,16 @@ Object {
 
 exports[`#team.removeAdmin should require admin 1`] = `
 Object {
-  "error": "authorization_error",
-  "message": "Authorization Required",
+  "error": "admin_required",
+  "message": "An admin role is required to access this resource",
   "ok": false,
+  "status": 403,
 }
 `;
 
 exports[`#team.removeAdmin shouldn't demote admins if only one available  1`] = `
 Object {
-  "error": "at_least_one_admin_is_required",
+  "error": "validation_error",
   "message": "At least one admin is required",
   "ok": false,
   "status": 400,

server/api/apiKeys.js

@@ -1,6 +1,5 @@
 // @flow
 import Router from 'koa-router';
-import httpErrors from 'http-errors';
 
 import auth from './middlewares/authentication';
 import pagination from './middlewares/pagination';
@@ -55,11 +54,7 @@ router.post('apiKeys.delete', auth(), async ctx => {
   const key = await ApiKey.findById(id);
   authorize(user, 'delete', key);
 
-  try {
   await key.destroy();
-  } catch (e) {
-    throw httpErrors.BadRequest('Error while deleting key');
-  }
 
   ctx.body = {
     success: true,

server/api/collections.js

@@ -1,11 +1,11 @@
 // @flow
 import Router from 'koa-router';
-import httpErrors from 'http-errors';
 
 import auth from './middlewares/authentication';
 import pagination from './middlewares/pagination';
 import { presentCollection } from '../presenters';
 import { Collection } from '../models';
+import { ValidationError } from '../errors';
 import policy from '../policies';
 
 const { authorize } = policy;
@@ -95,13 +95,9 @@ router.post('collections.delete', auth(), async ctx => {
   authorize(ctx.state.user, 'delete', collection);
 
   const total = await Collection.count();
-  if (total === 1) throw httpErrors.BadRequest('Cannot delete last collection');
+  if (total === 1) throw new ValidationError('Cannot delete last collection');
 
-  try {
   await collection.destroy();
-  } catch (e) {
-    throw httpErrors.BadRequest('Error while deleting collection');
-  }
 
   ctx.body = {
     success: true,

server/api/collections.test.js

@@ -57,7 +57,7 @@ describe('#collections.info', async () => {
     const res = await server.post('/api/collections.info', {
       body: { token: user.getJwtToken(), id: collection.id },
     });
-    expect(res.status).toEqual(404);
+    expect(res.status).toEqual(403);
   });
 });
 

server/api/index.js

@@ -39,13 +39,7 @@ api.use(async (ctx, next) => {
     }
 
     if (message.match('Authorization error')) {
-      if (ctx.path.match('info')) {
-        ctx.status = 404;
-        message = 'Not Found';
-      } else {
       ctx.status = 403;
-        message = 'Authorization Required';
-      }
     }
 
     if (ctx.status === 500) {

server/api/middlewares/validation.js

@@ -1,43 +1,43 @@
 // @flow
-import apiError from '../../errors';
 import validator from 'validator';
+import { ParamRequiredError, ValidationError } from '../../errors';
 import { validateColorHex } from '../../../shared/utils/color';
 
 export default function validation() {
   return function validationMiddleware(ctx: Object, next: Function) {
-    ctx.assertPresent = function assertPresent(value, message) {
+    ctx.assertPresent = (value, message) => {
       if (value === undefined || value === null || value === '') {
-        throw apiError(400, 'validation_error', message);
+        throw new ParamRequiredError(message);
       }
     };
 
-    ctx.assertNotEmpty = function assertNotEmpty(value, message) {
+    ctx.assertNotEmpty = (value, message) => {
       if (value === '') {
-        throw apiError(400, 'validation_error', message);
+        throw new ValidationError(message);
       }
     };
 
     ctx.assertEmail = (value, message) => {
       if (!validator.isEmail(value)) {
-        throw apiError(400, 'validation_error', message);
+        throw new ValidationError(message);
       }
     };
 
     ctx.assertUuid = (value, message) => {
       if (!validator.isUUID(value)) {
-        throw apiError(400, 'validation_error', message);
+        throw new ValidationError(message);
       }
     };
 
     ctx.assertPositiveInteger = (value, message) => {
       if (!validator.isInt(value, { min: 0 })) {
-        throw apiError(400, 'validation_error', message);
+        throw new ValidationError(message);
       }
     };
 
     ctx.assertHexColor = (value, message) => {
       if (!validateColorHex(value)) {
-        throw apiError(400, 'validation_error', message);
+        throw new ValidationError(message);
       }
     };
 

server/api/team.js

@@ -1,7 +1,6 @@
 // @flow
 import Router from 'koa-router';
-import httpErrors from 'http-errors';
-
+import { ValidationError } from '../errors';
 import { Team, User } from '../models';
 
 import auth from './middlewares/authentication';
@@ -61,7 +60,7 @@ router.post('team.removeAdmin', auth(), async ctx => {
   try {
     await team.removeAdmin(user);
   } catch (err) {
-    throw httpErrors.BadRequest(err.message);
+    throw new ValidationError(err.message);
   }
 
   ctx.body = {

server/errors.js

@@ -1,9 +1,28 @@
 // @flow
 import httpErrors from 'http-errors';
 
-const apiError = (code: number, id: string, message: string) => {
-  return httpErrors(code, message, { id });
-};
+export function ValidationError(message: string = 'Validation failed') {
+  return httpErrors(400, message, { id: 'validation_error' });
+}
 
-export default apiError;
-export { httpErrors };
+export function ParamRequiredError(
+  message: string = 'Required parameter missing'
+) {
+  return httpErrors(400, message, { id: 'param_required' });
+}
+
+export function AuthorizationError(
+  message: string = 'You do not have permission to access this resource'
+) {
+  return httpErrors(403, message, { id: 'permission_required' });
+}
+
+export function AdminRequiredError(
+  message: string = 'An admin role is required to access this resource'
+) {
+  return httpErrors(403, message, { id: 'admin_required' });
+}
+
+export function NotFoundError(message: string = 'Resource not found') {
+  return httpErrors(404, message, { id: 'not_found' });
+}

server/policies/collection.js

@@ -1,6 +1,7 @@
 // @flow
 import policy from './policy';
 import { Collection, User } from '../models';
+import { AdminRequiredError } from '../errors';
 
 const { allow } = policy;
 
@@ -13,12 +14,8 @@ allow(
   (user, collection) => collection && user.teamId === collection.teamId
 );
 
-allow(
-  User,
-  'delete',
-  Collection,
-  (user, collection) =>
-    collection &&
-    user.teamId === collection.teamId &&
-    (user.id === collection.creatorId || user.isAdmin)
-);
+allow(User, 'delete', Collection, (user, collection) => {
+  if (!collection || user.teamId !== collection.teamId) return false;
+  if (user.id === collection.creatorId) return true;
+  if (!user.isAdmin) throw new AdminRequiredError();
+});

server/policies/user.js

@@ -1,6 +1,7 @@
 // @flow
 import policy from './policy';
 import { User } from '../models';
+import { AdminRequiredError } from '../errors';
 
 const { allow } = policy;
 
@@ -11,19 +12,15 @@ allow(
   (actor, user) => user && user.teamId === actor.teamId
 );
 
-allow(
-  User,
-  ['update', 'delete'],
-  User,
-  (actor, user) =>
-    user &&
-    user.teamId === actor.teamId &&
-    (user.id === actor.id || actor.isAdmin)
-);
+allow(User, ['update', 'delete'], User, (actor, user) => {
+  if (!user || user.teamId !== actor.teamId) return false;
+  if (user.id === actor.id) return true;
+  if (actor.isAdmin) return true;
+  throw new AdminRequiredError();
+});
 
-allow(
-  User,
-  ['promote', 'demote'],
-  User,
-  (actor, user) => user && user.teamId === actor.teamId && actor.isAdmin
-);
+allow(User, ['promote', 'demote'], User, (actor, user) => {
+  if (!user || user.teamId !== actor.teamId) return false;
+  if (actor.isAdmin) return true;
+  throw new AdminRequiredError();
+});

server/slack.js

@@ -1,7 +1,7 @@
 // @flow
 import fetch from 'isomorphic-fetch';
 import querystring from 'querystring';
-import { httpErrors } from './errors';
+import httpErrors from 'http-errors';
 
 const SLACK_API_URL = 'https://slack.com/api';