From 90d75b21895f8cd5c2fa0044be7d4e6d5531e770 Mon Sep 17 00:00:00 2001 From: Kevin Ansfield Date: Thu, 20 Aug 2015 17:48:37 +0100 Subject: [PATCH] Fixes for sign-in error handling issue #5652, closes #5641 - removes inline errors for empty fields - separate validation routines for sign-in and forgot password - highlight fields with errors when trying to submit --- core/client/app/controllers/signin.js | 20 +++- core/client/app/styles/layouts/auth.css | 4 + core/client/app/templates/signin.hbs | 4 +- core/client/app/validators/signin.js | 36 +++++-- core/test/functional/client/signin_test.js | 120 +++++++++++++++++++-- 5 files changed, 161 insertions(+), 23 deletions(-) diff --git a/core/client/app/controllers/signin.js b/core/client/app/controllers/signin.js index 20f084e675..377de08e19 100644 --- a/core/client/app/controllers/signin.js +++ b/core/client/app/controllers/signin.js @@ -27,6 +27,14 @@ export default Ember.Controller.extend(ValidationEngine, { if (err.errors) { self.set('flowErrors', err.errors[0].message.string); + + if (err.errors[0].message.string.match(/no user with that email/)) { + self.get('model.errors').add('identification', ''); + } + + if (err.errors[0].message.string.match(/password is incorrect/)) { + self.get('model.errors').add('password', ''); + } } // if authentication fails a rejected promise will be returned. // it needs to be caught so it doesn't generate an exception in the console, @@ -41,7 +49,7 @@ export default Ember.Controller.extend(ValidationEngine, { // browsers and password managers that don't send proper events on autofill $('#login').find('input').trigger('change'); - this.validate().then(function () { + this.validate({property: 'signin'}).then(function () { self.get('notifications').closeNotifications(); self.toggleProperty('loggingIn'); self.send('authenticate'); @@ -60,7 +68,7 @@ export default Ember.Controller.extend(ValidationEngine, { self = this; this.set('flowErrors', ''); - this.validate({property: 'identification'}).then(function () { + this.validate({property: 'forgotPassword'}).then(function () { self.toggleProperty('submitting'); ajax({ @@ -77,7 +85,13 @@ export default Ember.Controller.extend(ValidationEngine, { }).catch(function (resp) { self.toggleProperty('submitting'); if (resp && resp.jqXHR && resp.jqXHR.responseJSON && resp.jqXHR.responseJSON.errors) { - self.set('flowErrors', resp.jqXHR.responseJSON.errors[0].message); + var message = resp.jqXHR.responseJSON.errors[0].message; + + self.set('flowErrors', message); + + if (message.match(/no user with that email/)) { + self.get('model.errors').add('identification', ''); + } } else { notifications.showAPIError(resp, {defaultErrorText: 'There was a problem with the reset, please try again.'}); } diff --git a/core/client/app/styles/layouts/auth.css b/core/client/app/styles/layouts/auth.css index 83e0f40be1..8736cc9b8c 100644 --- a/core/client/app/styles/layouts/auth.css +++ b/core/client/app/styles/layouts/auth.css @@ -26,6 +26,10 @@ position: relative; } +.forgotten-wrap input { + padding-right: 7rem; +} + .forgotten-wrap .forgotten-link { position: absolute; top: 10px; diff --git a/core/client/app/templates/signin.hbs b/core/client/app/templates/signin.hbs index 8a2dd576b2..3e0ee1fba2 100644 --- a/core/client/app/templates/signin.hbs +++ b/core/client/app/templates/signin.hbs @@ -4,13 +4,13 @@
{{#gh-form-group errors=model.errors property="identification"}} - {{gh-trim-focus-input class="gh-input email" type="email" placeholder="Email Address" name="identification" autocapitalize="off" autocorrect="off" tabindex="1" value=model.identification focusOut=(action "validate" "identification")}} + {{gh-trim-focus-input class="gh-input email" type="email" placeholder="Email Address" name="identification" autocapitalize="off" autocorrect="off" tabindex="1" value=model.identification}} {{gh-error-message errors=model.errors property="identification"}} {{/gh-form-group}} {{#gh-form-group errors=model.errors property="password"}} - {{input class="gh-input password" type="password" placeholder="Password" name="password" tabindex="2" value=model.password focusOut=(action "validate" "password")}} + {{input class="gh-input password" type="password" placeholder="Password" name="password" tabindex="2" value=model.password}} {{gh-error-message errors=model.errors property="password"}} diff --git a/core/client/app/validators/signin.js b/core/client/app/validators/signin.js index b44350922d..75e42a4ccb 100644 --- a/core/client/app/validators/signin.js +++ b/core/client/app/validators/signin.js @@ -1,23 +1,41 @@ import BaseValidator from './base'; var SigninValidator = BaseValidator.create({ - properties: ['identification', 'password'], + properties: ['identification', 'signin', 'forgotPassword'], + identification: function (model) { var id = model.get('identification'); - if (validator.empty(id)) { - model.get('errors').add('identification', 'Please enter an email'); - this.invalidate(); - } else if (!validator.isEmail(id)) { + if (!validator.empty(id) && !validator.isEmail(id)) { model.get('errors').add('identification', 'Invalid email'); this.invalidate(); } }, - password: function (model) { - var password = model.get('password') || ''; - if (!validator.isLength(password, 1)) { - model.get('errors').add('password', 'Please enter a password'); + signin: function (model) { + var id = model.get('identification'), + password = model.get('password'); + + model.get('errors').clear(); + + if (validator.empty(id)) { + model.get('errors').add('identification', ''); + this.invalidate(); + } + + if (validator.empty(password)) { + model.get('errors').add('password', ''); + this.invalidate(); + } + }, + + forgotPassword: function (model) { + var id = model.get('identification'); + + model.get('errors').clear(); + + if (validator.empty(id) || !validator.isEmail(id)) { + model.get('errors').add('identification', ''); this.invalidate(); } } diff --git a/core/test/functional/client/signin_test.js b/core/test/functional/client/signin_test.js index 551766ec94..0cb83180e4 100644 --- a/core/test/functional/client/signin_test.js +++ b/core/test/functional/client/signin_test.js @@ -30,7 +30,7 @@ CasperTest.begin('Redirects login to signin', 2, function suite(test) { }); }, true); -CasperTest.begin('Login limit is in place', 4, function suite(test) { +CasperTest.begin('Login limit is in place', 7, function suite(test) { CasperTest.Routines.signout.run(test); casper.thenOpenAndWaitForPageLoad('signin', function testTitleAndUrl() { @@ -53,6 +53,9 @@ CasperTest.begin('Login limit is in place', 4, function suite(test) { casper.waitForText('remaining', function onSuccess() { test.assert(true, 'The login limit is in place.'); test.assertSelectorDoesntHaveText('.gh-alert', '[object Object]'); + test.assertTextExists('password is incorrect'); + test.assertDoesntExist('.form-group.error input[name="identification"]', 'email field was highlighted'); + test.assertExists('.form-group.error input[name="password"]', 'password field was not highlighted'); }, function onTimeout() { test.assert(false, 'We did not trip the login limit.'); }); @@ -110,7 +113,7 @@ CasperTest.begin('Authenticated user is redirected', 6, function suite(test) { }); }, true); -CasperTest.begin('Ensure email field form validation', 4, function suite(test) { +CasperTest.begin('Validates unknown email for sign-in', 5, function suite(test) { CasperTest.Routines.signout.run(test); casper.thenOpenAndWaitForPageLoad('signin', function testTitleAndUrl() { @@ -119,26 +122,125 @@ CasperTest.begin('Ensure email field form validation', 4, function suite(test) { }); casper.waitForOpaque('.gh-signin', - function then() { + function testUnknownEmail() { this.fillAndSave('form.gh-signin', { - identification: 'notanemail' + identification: 'unknown@ghost.org', + password: 'testing' }); }, function onTimeout() { test.fail('Login form didn\'t fade in.'); }); - casper.waitForText('Invalid email', function onSuccess() { - test.assert(true, 'Invalid email error was shown'); - }, casper.failOnTimeout(test, 'Invalid email error was not shown')); + casper.waitForText('no user with that email address', function onSuccess() { + test.assert(true, 'Unknown email error was shown'); + test.assertExists('.form-group.error input[name="identification"]', 'email field was not highlighted'); + test.assertDoesntExist('.form-group.error input[name="password"]', 'password field was highlighted'); + }, casper.failOnTimeout(test, 'Uknown email error was not shown')); +}, true); + +CasperTest.begin('Validates missing details for sign-in', 11, function suite(test) { + CasperTest.Routines.signout.run(test); + + casper.thenOpenAndWaitForPageLoad('signin', function testTitleAndUrl() { + test.assertTitle('Sign In - Test Blog', 'Ghost admin has incorrect title'); + test.assertUrlMatch(/ghost\/signin\/$/, 'Landed on the correct URL'); + }); + + casper.waitForOpaque('.gh-signin', + function testMissingEverything() { + this.fillAndSave('form.gh-signin', { + identification: '', + password: '' + }); + }, + function onTimeout() { + test.fail('Login form didn\'t fade in.'); + }); + + casper.waitForText('fill out the form', function onSuccess() { + test.assert(true, 'Missing details error was shown'); + test.assertExists('.form-group.error input[name="identification"]', 'email field was not highlighted'); + test.assertExists('.form-group.error input[name="password"]', 'password field was not highlighted'); + }, casper.failOnTimeout(test, 'Missing details error was not shown')); casper.then(function testMissingEmail() { this.fillAndSave('form.gh-signin', { - identification: '' + identification: '', + password: 'testing' }); }); - casper.waitForText('Please enter an email', function onSuccess() { + casper.waitForText('fill out the form', function onSuccess() { + test.assert(true, 'Missing details error was shown'); + test.assertExists('.form-group.error input[name="identification"]', 'email field was not highlighted'); + test.assertDoesntExist('.form-group.error input[name="password"]', 'password field was still highlighted'); + }, casper.failOnTimeout(test, 'Missing details error was not shown')); + + casper.then(function testMissingPassword() { + this.fillAndSave('form.gh-signin', { + identification: 'test@test.com', + password: '' + }); + }); + + casper.waitForText('fill out the form', function onSuccess() { + test.assert(true, 'Missing details error was shown'); + test.assertDoesntExist('.form-group.error input[name="identification"]', 'email field was still highlighted'); + test.assertExists('.form-group.error input[name="password"]', 'password field was not highlighted'); + }, casper.failOnTimeout(test, 'Missing details error was not shown')); +}, true); + +CasperTest.begin('Validates missing details for forgotten password', 5, function suite(test) { + CasperTest.Routines.signout.run(test); + + casper.thenOpenAndWaitForPageLoad('signin', function testTitleAndUrl() { + test.assertTitle('Sign In - Test Blog', 'Ghost admin has incorrect title'); + test.assertUrlMatch(/ghost\/signin\/$/, 'Landed on the correct URL'); + }); + + casper.waitForOpaque('.gh-signin', + function testMissingEmail() { + casper.fill('form.gh-signin', { + identification: '', + password: '' + }); + casper.click('.forgotten-link'); + }, + function onTimeout() { + test.fail('Login form didn\'t fade in.'); + }); + + casper.waitForText('enter an email address', function onSuccess() { test.assert(true, 'Missing email error was shown'); + test.assertExists('.form-group.error input[name="identification"]', 'email field was not highlighted'); + test.assertDoesntExist('.form-group.error input[name="password"]', 'password field was highlighted'); }, casper.failOnTimeout(test, 'Missing email error was not shown')); }, true); + +CasperTest.begin('Validates unknown email for forgotten password', 5, function suite(test) { + CasperTest.Routines.signout.run(test); + + casper.thenOpenAndWaitForPageLoad('signin', function testTitleAndUrl() { + test.assertTitle('Sign In - Test Blog', 'Ghost admin has incorrect title'); + test.assertUrlMatch(/ghost\/signin\/$/, 'Landed on the correct URL'); + }); + + casper.waitForOpaque('.gh-signin', + function testMissingEmail() { + casper.fill('form.gh-signin', { + identification: 'unknown@ghost.org', + password: '' + }); + casper.click('.forgotten-link'); + }, + function onTimeout() { + test.fail('Login form didn\'t fade in.'); + }); + + casper.waitForText('no user with that email address', function onSuccess() { + test.assert(true, 'Unknown email error was shown'); + test.assertExists('.form-group.error input[name="identification"]', 'email field was not highlighted'); + test.assertDoesntExist('.form-group.error input[name="password"]', 'password field was highlighted'); + }, casper.failOnTimeout(test, 'Uknown email error was not shown')); +}, true);