Fix signin errors

refs TryGhost#5635 - fixes format for server errors - changes signin-api validation errors to be text rather than alerts
sunsern · Aug 10, 2015 · 69d020c · 69d020c
1 parent 1751daa
commit 69d020c
Show file tree

Hide file tree

Showing 7 changed files with 49 additions and 20 deletions.
diff --git a/core/client/app/controllers/signin.js b/core/client/app/controllers/signin.js
@@ -7,26 +7,32 @@ export default Ember.Controller.extend(ValidationEngine, {
 
     ghostPaths: Ember.inject.service('ghost-paths'),
     notifications: Ember.inject.service(),
+    flowErrors: '',
 
     // ValidationEngine settings
     validationType: 'signin',
 
     actions: {
         authenticate: function () {
-            var model = this.get('model'),
+            var self = this,
+                model = this.get('model'),
                 authStrategy = 'simple-auth-authenticator:oauth2-password-grant',
                 data = model.getProperties('identification', 'password');
 
-            this.get('session').authenticate(authStrategy, data).catch(function () {
-                // if authentication fails a rejected promise will be returned.
+            this.get('session').authenticate(authStrategy, data).catch(function (err) {
+                if (err.errors) {
+                    self.set('flowErrors', err.errors[0].message.string);
+                }
+
+                // If authentication fails a rejected promise will be returned.
                 // it needs to be caught so it doesn't generate an exception in the console,
                 // but it's actually "handled" by the sessionAuthenticationFailed action handler.
             });
         },
 
         validateAndAuthenticate: function () {
             var self = this;
-
+            this.set('flowErrors', '');
             // Manually trigger events for input fields, ensuring legacy compatibility with
             // browsers and password managers that don't send proper events on autofill
             $('#login').find('input').trigger('change');
@@ -37,6 +43,8 @@ export default Ember.Controller.extend(ValidationEngine, {
             }).catch(function (error) {
                 if (error) {
                     self.get('notifications').showAPIError(error);
+                } else {
+                    self.set('flowErrors', 'Please fill out the form to sign in.');
                 }
             });
         },
@@ -46,6 +54,7 @@ export default Ember.Controller.extend(ValidationEngine, {
                 notifications = this.get('notifications'),
                 self = this;
 
+            this.set('flowErrors', '');
             this.validate({property: 'identification'}).then(function () {
                 self.set('submitting', true);
 
@@ -62,8 +71,14 @@ export default Ember.Controller.extend(ValidationEngine, {
                     notifications.showAlert('Please check your email for instructions.', {type: 'info'});
                 }).catch(function (resp) {
                     self.set('submitting', false);
-                    notifications.showAPIError(resp, {defaultErrorText: 'There was a problem with the reset, please try again.'});
+                    if (resp && resp.jqXHR && resp.jqXHR.responseJSON && resp.jqXHR.responseJSON.errors) {
+                        self.set('flowErrors', resp.jqXHR.responseJSON.errors[0].message);
+                    } else {
+                        notifications.showAPIError(resp, {defaultErrorText: 'There was a problem with the reset, please try again.'});
+                    }
                 });
+            }).catch(function () {
+                self.set('flowErrors', 'Please enter an email address then click "Forgot?".');
             });
         }
     }

diff --git a/core/client/app/routes/application.js b/core/client/app/routes/application.js
@@ -62,10 +62,8 @@ export default Ember.Route.extend(ApplicationRouteMixin, ShortcutsRoute, {
                 error.errors.forEach(function (err) {
                     err.message = err.message.htmlSafe();
                 });
-
-                this.get('notifications').showErrors(error.errors);
             } else {
-                // connection errors don't return proper status message, only req.body
+                // Connection errors don't return proper status message, only req.body
                 this.get('notifications').showAlert('There was a problem on the server.', {type: 'error'});
             }
         },

diff --git a/core/client/app/templates/signin.hbs b/core/client/app/templates/signin.hbs
@@ -18,7 +18,7 @@
                 <button id="login-button" class="login btn btn-blue btn-block" type="submit" tabindex="3" disabled={{submitting}}>Sign in</button>
             </form>
 
-            <p class="main-error">The password fairy does not approve</p>
+            <p class="main-error">{{{flowErrors}}}</p>
         </section>
     </div>
 </div>
diff --git a/core/server/errors/index.js b/core/server/errors/index.js
@@ -16,6 +16,7 @@ var _                          = require('lodash'),
     UnsupportedMediaTypeError  = require('./unsupported-media-type-error'),
     EmailError                 = require('./email-error'),
     DataImportError            = require('./data-import-error'),
+    TooManyRequestsError       = require('./too-many-requests-error'),
     config,
     errors,
 
@@ -393,3 +394,4 @@ module.exports.UnsupportedMediaTypeError  = UnsupportedMediaTypeError;
 module.exports.EmailError                 = EmailError;
 module.exports.DataImportError            = DataImportError;
 module.exports.MethodNotAllowedError      = MethodNotAllowedError;
+module.exports.TooManyRequestsError       = TooManyRequestsError;
diff --git a/core/server/errors/too-many-requests-error.js b/core/server/errors/too-many-requests-error.js
@@ -0,0 +1,14 @@
+// # Too Many Requests Error
+// Custom error class with status code and type prefilled.
+
+function TooManyRequestsError(message) {
+    this.message = message;
+    this.stack = new Error().stack;
+    this.code = 429;
+    this.errorType = this.name;
+}
+
+TooManyRequestsError.prototype = Object.create(Error.prototype);
+TooManyRequestsError.prototype.name = 'TooManyRequestsError';
+
+module.exports = TooManyRequestsError;
diff --git a/core/server/middleware/spam-prevention.js b/core/server/middleware/spam-prevention.js
@@ -49,7 +49,7 @@ spamPrevention = {
                 'Too many login attempts.'
             );
             message += rateSigninPeriod === 3600 ? ' Please wait 1 hour.' : ' Please try again later';
-            return next(new errors.UnauthorizedError(message));
+            return next(new errors.TooManyRequestsError(message));
         }
         next();
     },
@@ -110,7 +110,7 @@ spamPrevention = {
 
         if (deniedEmailRateLimit || deniedRateLimit) {
             message += rateForgottenPeriod === 3600 ? ' Please wait 1 hour.' : ' Please try again later';
-            return next(new errors.UnauthorizedError(message));
+            return next(new errors.TooManyRequestsError(message));
         }
 
         next();

diff --git a/core/server/models/user.js b/core/server/models/user.js
@@ -115,7 +115,7 @@ User = ghostBookshelf.Model.extend({
         } else if (this.get('id')) {
             return this.get('id');
         } else {
-            errors.logAndThrowError(new Error('missing context'));
+            errors.logAndThrowError(new errors.NotFoundError('missing context'));
         }
     },
 
@@ -541,7 +541,7 @@ User = ghostBookshelf.Model.extend({
             if (user.get('status') === 'invited' || user.get('status') === 'invited-pending' ||
                     user.get('status') === 'inactive'
                 ) {
-                return Promise.reject(new Error('The user with that email address is inactive.'));
+                return Promise.reject(new errors.NoPermissionError('The user with that email address is inactive.'));
             }
             if (user.get('status') !== 'locked') {
                 return bcryptCompare(object.password, user.get('password')).then(function then(matched) {
@@ -664,25 +664,25 @@ User = ghostBookshelf.Model.extend({
 
         // Check if invalid structure
         if (!parts || parts.length !== 3) {
-            return Promise.reject(new Error('Invalid token structure'));
+            return Promise.reject(new errors.BadRequestError('Invalid token structure'));
         }
 
         expires = parseInt(parts[0], 10);
         email = parts[1];
 
         if (isNaN(expires)) {
-            return Promise.reject(new Error('Invalid token expiration'));
+            return Promise.reject(new errors.BadRequestError('Invalid token expiration'));
         }
 
         // Check if token is expired to prevent replay attacks
         if (expires < Date.now()) {
-            return Promise.reject(new Error('Expired token'));
+            return Promise.reject(new errors.ValidationError('Expired token'));
         }
 
         // to prevent brute force attempts to reset the password the combination of email+expires is only allowed for
         // 10 attempts
         if (tokenSecurity[email + '+' + expires] && tokenSecurity[email + '+' + expires].count >= 10) {
-            return Promise.reject(new Error('Token locked'));
+            return Promise.reject(new errors.NoPermissionError('Token locked'));
         }
 
         return this.generateResetToken(email, expires, dbHash).then(function then(generatedToken) {
@@ -707,7 +707,7 @@ User = ghostBookshelf.Model.extend({
             tokenSecurity[email + '+' + expires] = {
                 count: tokenSecurity[email + '+' + expires] ? tokenSecurity[email + '+' + expires].count + 1 : 1
             };
-            return Promise.reject(new Error('Invalid token'));
+            return Promise.reject(new errors.BadRequestError('Invalid token'));
         });
     },
 
@@ -719,7 +719,7 @@ User = ghostBookshelf.Model.extend({
             dbHash = options.dbHash;
 
         if (newPassword !== ne2Password) {
-            return Promise.reject(new Error('Your new passwords do not match'));
+            return Promise.reject(new errors.ValidationError('Your new passwords do not match'));
         }
 
         if (!validatePasswordLength(newPassword)) {
@@ -735,7 +735,7 @@ User = ghostBookshelf.Model.extend({
             );
         }).then(function then(results) {
             if (!results[0]) {
-                return Promise.reject(new Error('User not found'));
+                return Promise.reject(new errors.NotFoundError('User not found'));
             }
 
             // Update the user with the new password hash