diff --git a/ghost/admin/app/controllers/signin-verify.js b/ghost/admin/app/controllers/signin-verify.js index 086e6b7404..b148345930 100644 --- a/ghost/admin/app/controllers/signin-verify.js +++ b/ghost/admin/app/controllers/signin-verify.js @@ -10,6 +10,8 @@ import {tracked} from '@glimmer/tracking'; const {Errors} = DS; +const TASK_SUCCESS = true; +const TASK_FAILURE = false; const DEFAULT_RESEND_TOKEN_COUNTDOWN = 15; // eslint-disable-next-line ghost/ember/alias-model-in-controller @@ -20,13 +22,36 @@ class VerifyData { errors = Errors.create(); validate() { - this.errors.clear(); - this.hasValidated = new TrackedArray(); + this.resetValidations(); if (!this.token?.trim()) { - this.errors.add('token', 'Token is required'); + this.errors.add('token', 'Verification code is required'); this.hasValidated.push('token'); + return false; } + + if (!this.token?.trim().match(/^\d{6}$/)) { + this.errors.add('token', 'Verification code must be 6 numbers'); + this.hasValidated.push('token'); + return false; + } + + return true; + } + + setTokenError(message) { + this.resetValidations(); + this.errors.add('token', message); + this.hasValidated.push('token'); + } + + resetValidations() { + this.errors.clear(); + this.hasValidated = new TrackedArray(); + } + + get validationMessage() { + return this.errors.toArray()[0]?.message; } } @@ -62,30 +87,32 @@ export default class SigninVerifyController extends Controller { this.verifyData = new VerifyData(); } - @action - validateToken() { - this.verifyData.validate(); - } - @action handleTokenInput(event) { + this.resetErrorState(); this.verifyData.token = event.target.value; } @task *verifyTokenTask() { + this.flowErrors = ''; + + if (!this.verifyData.validate()) { + return TASK_FAILURE; + } + try { yield this.session.authenticate('authenticator:cookie', {token: this.verifyData.token}); - return true; + return TASK_SUCCESS; } catch (error) { if (isUnauthorizedError(error)) { - this.flowErrors = 'Your verification code is incorrect.'; + this.verifyData.setTokenError('Your verification code is incorrect.'); } else if (error && error.payload && error.payload.errors) { this.flowErrors = error.payload.errors[0].message; } else { this.flowErrors = 'There was a problem verifying the code. Please try again.'; } - return false; + return TASK_FAILURE; } } @@ -94,19 +121,22 @@ export default class SigninVerifyController extends Controller { const resendTokenPath = `${this.ghostPaths.apiRoot}/session/verify`; try { - yield this.ajax.post(resendTokenPath, { - contentType: 'application/json;charset=utf-8', - // ember-ajax will try and parse the response as JSON if not explicitly set - dataType: 'text' - }); + yield this.ajax.post(resendTokenPath); this.startResendTokenCountdown(); - return true; + return TASK_SUCCESS; } catch (error) { if (error && error.payload && error.payload.errors) { this.flowErrors = error.payload.errors[0].message; } else { this.flowErrors = 'There was a problem resending the verification token.'; } + return TASK_FAILURE; } } + + resetErrorState() { + this.verifyTokenTask.last = null; // resets GhTaskButton state + this.flowErrors = ''; + this.verifyData.resetValidations(); + } } diff --git a/ghost/admin/app/templates/signin-verify.hbs b/ghost/admin/app/templates/signin-verify.hbs index 5d8a5c39ce..b624ab3e00 100644 --- a/ghost/admin/app/templates/signin-verify.hbs +++ b/ghost/admin/app/templates/signin-verify.hbs @@ -27,7 +27,6 @@ value={{this.token}} data-test-input="token" {{on "input" this.handleTokenInput}} - {{on "blur" this.validateToken}} /> {{#if this.resendTokenCountdownStarted}} @@ -58,7 +58,9 @@ data-test-button="verify" /> -

{{if this.flowErrors this.flowErrors this.flowNotification}} 

+ {{#let (or this.flowErrors this.verifyData.validationMessage) as |error|}} +

{{error}} 

+ {{/let}} \ No newline at end of file diff --git a/ghost/admin/mirage/config/authentication.js b/ghost/admin/mirage/config/authentication.js index 7a315d8a44..8e8fabfb23 100644 --- a/ghost/admin/mirage/config/authentication.js +++ b/ghost/admin/mirage/config/authentication.js @@ -4,11 +4,21 @@ import {dasherize} from '@ember/string'; import {isBlank} from '@ember/utils'; export default function mockAuthentication(server) { + // Password sign-in server.post('/session', function () { - // Password sign-in return new Response(201); }); + // 2fa code verification + server.put('/session/verify', function () { + return new Response(201); + }); + + // 2fa code re-send + server.post('/session/verify', function () { + return new Response(200); + }); + server.post('/authentication/password_reset', function (schema, request) { let {password_reset} = JSON.parse(request.requestBody); let email = password_reset[0].email; diff --git a/ghost/admin/tests/acceptance/authentication-test.js b/ghost/admin/tests/acceptance/authentication-test.js index df37ea8f7b..b6e3d73dad 100644 --- a/ghost/admin/tests/acceptance/authentication-test.js +++ b/ghost/admin/tests/acceptance/authentication-test.js @@ -9,6 +9,28 @@ import {run} from '@ember/runloop'; import {setupApplicationTest} from 'ember-mocha'; import {setupMirage} from 'ember-cli-mirage/test-support'; +function setupVerificationRequired(server, responseCode = 403) { + server.post('/session', function () { + return new Response(responseCode, {}, { + errors: [{ + code: '2FA_TOKEN_REQUIRED' + }] + }); + }); +} + +function setupVerificationSuccess(server) { + server.put('/session/verify', function () { + return new Response(201); + }); +} + +function setupVerificationFailed(server) { + server.put('/session/verify', function () { + return new Response(401, {}, null); + }); +} + describe('Acceptance: Authentication', function () { let originalReplaceLocation; @@ -36,28 +58,6 @@ describe('Acceptance: Authentication', function () { describe('general page', function () { let newLocation; - function setupVerificationRequired(server) { - server.post('/session', function () { - return new Response(403, {}, { - errors: [{ - code: '2FA_TOKEN_REQUIRED' - }] - }); - }); - } - - function setupVerificationSuccess(server) { - server.put('/session/verify', function () { - return new Response(201); - }); - } - - function setupVerificationFailed(server) { - server.put('/session/verify', function () { - return new Response(401, {}, null); - }); - } - async function completeSignIn() { await invalidateSession(); await visit('/signin'); @@ -75,6 +75,22 @@ describe('Acceptance: Authentication', function () { expect(find('[data-test-flow-notification]')).to.have.trimmed.text(expectedMessage); } + function testTokenInputHasErrorState(expectHasError = true) { + if (expectHasError) { + expect(find('[data-test-input="token"]').closest('.form-group')).to.have.class('error'); + } else { + expect(find('[data-test-input="token"]').closest('.form-group')).not.to.have.class('error'); + } + } + + function testButtonHasErrorState(expectHasError = true) { + if (expectHasError) { + expect(find('[data-test-button="verify"]')).to.have.class('gh-btn-red'); + } else { + expect(find('[data-test-button="verify"]')).not.to.have.class('gh-btn-red'); + } + } + beforeEach(function () { originalReplaceLocation = windowProxy.replaceLocation; windowProxy.replaceLocation = function (url) { @@ -205,12 +221,7 @@ describe('Acceptance: Authentication', function () { }); it('handles 2fa-required on a 2xx response from signin', async function () { - this.server.post('/session', function () { - return new Response(200, {}, { - errors: [{code: '2FA_TOKEN_REQUIRED'}] - }); - }); - + setupVerificationRequired(this.server, 200); await completeSignIn(); expect(currentURL(), 'url after email+password submit').to.equal('/signin/verify'); @@ -228,6 +239,38 @@ describe('Acceptance: Authentication', function () { expect(currentURL(), 'url after email+password submit').to.equal('/signin'); testMainErrorMessage('Insufficient permissions'); }); + + it('has client-side validation of verification code', async function () { + setupVerificationRequired(this.server); + await completeSignIn(); + + // no error state when starting flow + testTokenInputHasErrorState(false); + testButtonHasErrorState(false); + testMainErrorMessage(''); + + // client-side validation of token presence + await click('[data-test-button="verify"]'); + testTokenInputHasErrorState(); + testMainErrorMessage('Verification code is required'); + + // resets validation state when typing + await fillIn('[data-test-input="token"]', '1234'); + testTokenInputHasErrorState(false); + testButtonHasErrorState(false); + testMainErrorMessage(''); + + // client-side validation of token format + await click('[data-test-button="verify"]'); + testTokenInputHasErrorState(); + testButtonHasErrorState(); + testMainErrorMessage('Verification code must be 6 numbers'); + + // can correctly submit after failed attempts + await fillIn('[data-test-input="token"]', '123456'); + await click('[data-test-button="verify"]'); + expect(currentURL()).to.equal('/dashboard'); + }); }); describe('editor', function () {