From 69d020ce448b8b5e8d5022bb8de16d2528ca963f Mon Sep 17 00:00:00 2001 From: cobbspur Date: Mon, 10 Aug 2015 12:26:39 +0100 Subject: [PATCH] Fix signin errors refs #5635 - fixes format for server errors - changes signin-api validation errors to be text rather than alerts --- core/client/app/controllers/signin.js | 25 +++++++++++++++---- core/client/app/routes/application.js | 4 +-- core/client/app/templates/signin.hbs | 2 +- core/server/errors/index.js | 2 ++ core/server/errors/too-many-requests-error.js | 14 +++++++++++ core/server/middleware/spam-prevention.js | 4 +-- core/server/models/user.js | 18 ++++++------- 7 files changed, 49 insertions(+), 20 deletions(-) create mode 100644 core/server/errors/too-many-requests-error.js diff --git a/core/client/app/controllers/signin.js b/core/client/app/controllers/signin.js index 76c352f915..6a0cad34b8 100644 --- a/core/client/app/controllers/signin.js +++ b/core/client/app/controllers/signin.js @@ -7,18 +7,24 @@ 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. }); @@ -26,7 +32,7 @@ export default Ember.Controller.extend(ValidationEngine, { 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 index 7a12f7075e..ed92f6e33e 100644 --- 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 index 2ee759527b..2f31a32867 100644 --- a/core/client/app/templates/signin.hbs +++ b/core/client/app/templates/signin.hbs @@ -18,7 +18,7 @@ -

The password fairy does not approve

+

{{{flowErrors}}}

diff --git a/core/server/errors/index.js b/core/server/errors/index.js index cc9e959fe0..8e3d9dbd34 100644 --- 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 new file mode 100644 index 0000000000..15e5c0f813 --- /dev/null +++ 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 index f2664a4f56..140955e166 100644 --- 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 index bdc7b180f1..3b687f44a4 100644 --- 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