From 6be12f8a061a2f7228e254c314a46ad7a78966d1 Mon Sep 17 00:00:00 2001
From: Michael Leanos <michael.leanos@gmail.com>
Date: Fri, 7 Oct 2016 22:03:31 -0700
Subject: [PATCH] fix(core): Add custom 400 and 404 error messages (#1547)

* Added 400 and 404 custom error messages

* nicer error message views

* Sign Up & Sign In error responses

Changed the error responses returned from the Sign Up & Sign In API
calls to use 422 rather than 400.

For insight into why this change was made:
https://github.com/meanjs/mean/pull/1510#issuecomment-247435378

For reference on why to use 422 over 400:
https://www.bennadel.com/blog/2434-http-status-codes-for-invalid-data-400-vs-422.htm
---
 .../core/client/config/core.client.routes.js   | 18 ++++++++++++++++--
 .../controllers/error.client.controller.js     | 18 ++++++++++++++++++
 .../auth-interceptor.client.service.js         |  6 ++++++
 modules/core/client/views/400.client.view.html |  7 +++++--
 modules/core/client/views/403.client.view.html |  4 +++-
 modules/core/client/views/404.client.view.html |  7 +++++--
 .../users.authentication.server.controller.js  |  4 ++--
 7 files changed, 55 insertions(+), 9 deletions(-)
 create mode 100644 modules/core/client/controllers/error.client.controller.js

diff --git a/modules/core/client/config/core.client.routes.js b/modules/core/client/config/core.client.routes.js
index 6fc0d79f..9a89d6c3 100644
--- a/modules/core/client/config/core.client.routes.js
+++ b/modules/core/client/config/core.client.routes.js
@@ -36,17 +36,31 @@
       .state('not-found', {
         url: '/not-found',
         templateUrl: 'modules/core/client/views/404.client.view.html',
+        controller: 'ErrorController',
+        controllerAs: 'vm',
+        params: {
+          message: function($stateParams) {
+            return $stateParams.message;
+          }
+        },
         data: {
           ignoreState: true,
-          pageTitle: 'Not-Found'
+          pageTitle: 'Not Found'
         }
       })
       .state('bad-request', {
         url: '/bad-request',
         templateUrl: 'modules/core/client/views/400.client.view.html',
+        controller: 'ErrorController',
+        controllerAs: 'vm',
+        params: {
+          message: function($stateParams) {
+            return $stateParams.message;
+          }
+        },
         data: {
           ignoreState: true,
-          pageTitle: 'Bad-Request'
+          pageTitle: 'Bad Request'
         }
       })
       .state('forbidden', {
diff --git a/modules/core/client/controllers/error.client.controller.js b/modules/core/client/controllers/error.client.controller.js
new file mode 100644
index 00000000..5968f343
--- /dev/null
+++ b/modules/core/client/controllers/error.client.controller.js
@@ -0,0 +1,18 @@
+(function () {
+  'use strict';
+
+  angular
+    .module('core')
+    .controller('ErrorController', ErrorController);
+
+  ErrorController.$inject = ['$stateParams'];
+
+  function ErrorController($stateParams) {
+    var vm = this;
+    vm.errorMessage = null;
+
+    // Display custom message if it was set
+    if ($stateParams.message) vm.errorMessage = $stateParams.message;
+  }
+}());
+
diff --git a/modules/core/client/services/interceptors/auth-interceptor.client.service.js b/modules/core/client/services/interceptors/auth-interceptor.client.service.js
index 29294028..97486209 100644
--- a/modules/core/client/services/interceptors/auth-interceptor.client.service.js
+++ b/modules/core/client/services/interceptors/auth-interceptor.client.service.js
@@ -17,6 +17,9 @@
     function responseError(rejection) {
       if (!rejection.config.ignoreAuthModule) {
         switch (rejection.status) {
+          case 400:
+            $injector.get('$state').go('bad-request', { message: rejection.data.message });
+            break;
           case 401:
             // Deauthenticate the global user
             Authentication.user = null;
@@ -25,6 +28,9 @@
           case 403:
             $injector.get('$state').transitionTo('forbidden');
             break;
+          case 404:
+            $injector.get('$state').go('not-found', { message: rejection.data.message });
+            break;
         }
       }
       // otherwise, default behaviour
diff --git a/modules/core/client/views/400.client.view.html b/modules/core/client/views/400.client.view.html
index efc28045..f1192d49 100644
--- a/modules/core/client/views/400.client.view.html
+++ b/modules/core/client/views/400.client.view.html
@@ -1,6 +1,9 @@
-<h1>Bad Request</h1>
+<div class="page-header">
+  <h1>Bad Request</h1>
+</div>
 <div class="alert alert-danger" role="alert">
   <span class="glyphicon glyphicon-exclamation-sign" aria-hidden="true"></span>
   <span class="sr-only">Error:</span>
-  You made a bad request
+  <span ng-if="vm.errorMessage" ng-bind="vm.errorMessage"></span>
+  <span ng-if="!vm.errorMessage">You made a bad request</span>
 </div>
diff --git a/modules/core/client/views/403.client.view.html b/modules/core/client/views/403.client.view.html
index 151968d5..320980c9 100644
--- a/modules/core/client/views/403.client.view.html
+++ b/modules/core/client/views/403.client.view.html
@@ -1,4 +1,6 @@
-<h1>Forbidden</h1>
+<div class="page-header">
+  <h1>Forbidden</h1>
+</div>
 <div class="alert alert-danger" role="alert">
   <span class="glyphicon glyphicon-exclamation-sign" aria-hidden="true"></span>
   <span class="sr-only">Error:</span>
diff --git a/modules/core/client/views/404.client.view.html b/modules/core/client/views/404.client.view.html
index e8e6d82c..c19e9be6 100644
--- a/modules/core/client/views/404.client.view.html
+++ b/modules/core/client/views/404.client.view.html
@@ -1,5 +1,8 @@
-<h1>Page Not Found</h1>
+<div class="page-header">
+  <h1>Page Not Found</h1>
+</div>
 <div class="alert alert-danger" role="alert">
   <span class="glyphicon glyphicon-exclamation-sign" aria-hidden="true"></span>
-  <span class="sr-only">Error:</span> Page Not Found
+  <span ng-if="vm.errorMessage" ng-bind="vm.errorMessage"></span>
+  <span ng-if="!vm.errorMessage">Page Not Found</span>
 </div>
diff --git a/modules/users/server/controllers/users/users.authentication.server.controller.js b/modules/users/server/controllers/users/users.authentication.server.controller.js
index b4777854..20893854 100644
--- a/modules/users/server/controllers/users/users.authentication.server.controller.js
+++ b/modules/users/server/controllers/users/users.authentication.server.controller.js
@@ -30,7 +30,7 @@ exports.signup = function (req, res) {
   // Then save the user
   user.save(function (err) {
     if (err) {
-      return res.status(400).send({
+      return res.status(422).send({
         message: errorHandler.getErrorMessage(err)
       });
     } else {
@@ -55,7 +55,7 @@ exports.signup = function (req, res) {
 exports.signin = function (req, res, next) {
   passport.authenticate('local', function (err, user, info) {
     if (err || !user) {
-      res.status(400).send(info);
+      res.status(422).send(info);
     } else {
       // Remove sensitive data before login
       user.password = undefined;