TryGhost/Ghost
1
0
Fork 0
mirror of https://github.com/TryGhost/Ghost.git synced 2024-11-22 11:16:01 +03:00
Code Issues Projects Releases Wiki Activity

Removed resend countdown from 2fa flow

Browse Source 
closes https://linear.app/ghost/issue/ENG-1658

- switched to using a task to match patterns elsewhere and have better cancellation behaviour if code is re-used in a short-lived component
- added `drop: true` task modifier to our main tasks so they can't be triggered again whilst we're waiting on an API request
- removed confusing countdown in button text
- restored forced "text" data type for resend API request to match API behavior
- added acceptance tests for resend behaviour
This commit is contained in:
Kevin Ansfield 2024-10-22 17:27:14 +01:00
parent 76d4ef27c4
commit 9fa0c24be9
5 changed files with 102 additions and 53 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

22
ghost/admin/app/components/gh-task-button.hbs
View File

@ -9,28 +9,28 @@
{{#if this.isRunning}}
<span data-test-task-button-state="running">
{{#if this.showIcon}}
{{svg-jar "spinner" class="gh-icon-spinner"}}
{{svg-jar "spinner" class="gh-icon-spinner"}}
{{/if}}
{{this.runningText}}
</span>
{{/if}}
{{#if (and this.isIdle (not (or this.isRunning this.isSuccess this.isFailure)))}}
<span data-test-task-button-state="idle">
{{#if this.showIcon}}
{{#if this.idleIcon}}
{{svg-jar this.idleIcon}}
<span data-test-task-button-state="idle">
{{#if this.showIcon}}
{{#if this.idleIcon}}
{{svg-jar this.idleIcon}}
{{/if}}
{{/if}}
{{/if}}
{{this.buttonText}}
</span>
{{/if}}
{{this.buttonText}}
</span>
{{/if}}
{{#if this.isSuccess}}
<span {{did-insert this.handleReset}} data-test-task-button-state="success">
{{#if this.showIcon}}
{{svg-jar "check-circle"}}
{{svg-jar "check-circle"}}
{{/if}}
{{this.successText}}
</span>
@ -38,7 +38,7 @@
{{#if this.isFailure}}
<span data-test-task-button-state="failure">
{{#if this.showIcon}}
{{svg-jar "retry"}}
{{svg-jar "retry"}}
{{/if}}
{{this.failureText}}
</span>

45
ghost/admin/app/controllers/signin-verify.js
View File

@ -1,11 +1,12 @@
import Controller from '@ember/controller';
// eslint-disable-next-line
import DS from 'ember-data';
import config from 'ghost-admin/config/environment';
import {TrackedArray} from 'tracked-built-ins';
import {action} from '@ember/object';
import {isUnauthorizedError} from 'ember-ajax/errors';
import {inject as service} from '@ember/service';
import {task} from 'ember-concurrency';
import {task, timeout} from 'ember-concurrency';
import {tracked} from '@glimmer/tracking';
const {Errors} = DS;
@ -63,19 +64,6 @@ export default class SigninVerifyController extends Controller {
@tracked flowErrors = '';
@tracked verifyData = new VerifyData();
@tracked resendTokenCountdown = DEFAULT_RESEND_TOKEN_COUNTDOWN;
@tracked resendTokenCountdownStarted = false;
startResendTokenCountdown() {
this.resendTokenCountdown = DEFAULT_RESEND_TOKEN_COUNTDOWN;
this.resendTokenCountdownStarted = true;
this.resendTokenCountdownInterval = setInterval(() => {
if (this.resendTokenCountdown > 0) {
this.resendTokenCountdown = this.resendTokenCountdown - 1;
} else {
this.resetResendTokenCountdown();
}
}, 1000);
}
resetResendTokenCountdown() {
clearInterval(this.resendTokenCountdownInterval);
@ -93,7 +81,7 @@ export default class SigninVerifyController extends Controller {
this.verifyData.token = event.target.value;
}
@task
@task({drop: true})
*verifyTokenTask() {
this.flowErrors = '';
@ -116,21 +104,17 @@ export default class SigninVerifyController extends Controller {
}
}
@task
@task({drop: true})
*resendTokenTask() {
const resendTokenPath = `${this.ghostPaths.apiRoot}/session/verify`;
try {
try {
yield this.ajax.post(resendTokenPath);
} catch (error) {
// HACK: For some reason, the server returns 200: OK and sends the email but the client still throws an error
// So we need to catch the error and throw it if it's not 'OK'
if (error !== 'OK') {
throw error;
}
}
this.startResendTokenCountdown();
yield this.ajax.post(resendTokenPath, {
contentType: 'application/json;charset=utf-8',
// API responds with "OK" which will throw if we do a default JSON parse
dataType: 'text'
});
this.delayResendAvailabilityTask.perform();
return TASK_SUCCESS;
} catch (error) {
if (error && error.payload && error.payload.errors) {
@ -142,6 +126,15 @@ export default class SigninVerifyController extends Controller {
}
}
@task
*delayResendAvailabilityTask() {
this.resendTokenCountdown = DEFAULT_RESEND_TOKEN_COUNTDOWN;
while (this.resendTokenCountdown > 0) {
yield timeout(config.environment === 'test' ? 10 : 1000);
this.resendTokenCountdown = this.resendTokenCountdown - 1;
}
}
resetErrorState() {
this.verifyTokenTask.last = null; // resets GhTaskButton state
this.flowErrors = '';

6
ghost/admin/app/templates/signin-verify.hbs
View File

@ -35,12 +35,12 @@
@type="button"
@successClass=""
@failureClass=""
@disabled={{this.resendTokenCountdownStarted}}
@disabled={{or this.resendTokenTask.isRunning this.delayResendAvailabilityTask.isRunning}}
data-test-button="resend-token"
as |task|
>
{{#if this.resendTokenCountdownStarted}}
<span>Resend again in {{this.resendTokenCountdown}}s</span>
{{#if this.delayResendAvailabilityTask.isRunning}}
<span>Sent</span>
{{else}}
<span>{{#if task.isRunning}}{{svg-jar "spinner" class="gh-spinner"}}&nbsp;Sending{{else}}Resend{{/if}}</span>
{{/if}}

2
ghost/admin/mirage/config/authentication.js
View File

@ -16,7 +16,7 @@ export default function mockAuthentication(server) {
// 2fa code re-send
server.post('/session/verify', function () {
return new Response(200);
return new Response(200, {}, 'OK');
});
server.post('/authentication/password_reset', function (schema, request) {

80
ghost/admin/tests/acceptance/authentication-test.js
View File

@ -3,7 +3,7 @@ import windowProxy from 'ghost-admin/utils/window-proxy';
import {Response} from 'miragejs';
import {afterEach, beforeEach, describe, it} from 'mocha';
import {authenticateSession, invalidateSession} from 'ember-simple-auth/test-support';
import {click, currentRouteName, currentURL, fillIn, find, findAll, triggerKeyEvent, visit, waitFor} from '@ember/test-helpers';
import {click, currentRouteName, currentURL, fillIn, find, findAll, triggerKeyEvent, visit, waitFor, waitUntil} from '@ember/test-helpers';
import {expect} from 'chai';
import {run} from '@ember/runloop';
import {setupApplicationTest} from 'ember-mocha';
@ -31,6 +31,27 @@ function setupVerificationFailed(server) {
});
}
function setupResendSuccess(server, {timing = 0}) {
// API returns 'OK' in API response - this is important as it will cause
// errors to be thrown if we try to parse it as JSON
server.post('/session/verify', function () {
return new Response(200, {}, 'OK');
}, {timing});
}
function setupResendFailure(server, {responseCode = 400, timing = 0, message} = {}) {
server.post('/session/verify', function () {
if (message) {
return new Response(responseCode, {}, {
errors: [{
message
}]
});
}
return new Response(responseCode);
}, {timing});
}
describe('Acceptance: Authentication', function () {
let originalReplaceLocation;
@ -58,6 +79,10 @@ describe('Acceptance: Authentication', function () {
describe('general page', function () {
let newLocation;
const verifyButton = '[data-test-button="verify"]';
const resendButton = '[data-test-button="resend-token"]';
const codeInput = '[data-test-input="token"]';
async function completeSignIn() {
await invalidateSession();
await visit('/signin');
@ -67,8 +92,8 @@ describe('Acceptance: Authentication', function () {
}
async function completeVerification() {
await fillIn('[data-test-input="token"]', 123456);
await click('[data-test-button="verify"]');
await fillIn(codeInput, 123456);
await click(verifyButton);
}
function testMainErrorMessage(expectedMessage) {
@ -77,17 +102,17 @@ describe('Acceptance: Authentication', function () {
function testTokenInputHasErrorState(expectHasError = true) {
if (expectHasError) {
expect(find('[data-test-input="token"]').closest('.form-group')).to.have.class('error');
expect(find(codeInput).closest('.form-group')).to.have.class('error');
} else {
expect(find('[data-test-input="token"]').closest('.form-group')).not.to.have.class('error');
expect(find(codeInput).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');
expect(find(verifyButton)).to.have.class('gh-btn-red');
} else {
expect(find('[data-test-button="verify"]')).not.to.have.class('gh-btn-red');
expect(find(verifyButton)).not.to.have.class('gh-btn-red');
}
}
@ -250,27 +275,58 @@ describe('Acceptance: Authentication', function () {
testMainErrorMessage('');
// client-side validation of token presence
await click('[data-test-button="verify"]');
await click(verifyButton);
testTokenInputHasErrorState();
testMainErrorMessage('Verification code is required');
// resets validation state when typing
await fillIn('[data-test-input="token"]', '1234');
await fillIn(codeInput, '1234');
testTokenInputHasErrorState(false);
testButtonHasErrorState(false);
testMainErrorMessage('');
// client-side validation of token format
await click('[data-test-button="verify"]');
await click(verifyButton);
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"]');
await fillIn(codeInput, '123456');
await click(verifyButton);
expect(currentURL()).to.equal('/dashboard');
});
it('can resend verification code', async function () {
setupVerificationRequired(this.server);
setupResendSuccess(this.server, {timing: 100});
await completeSignIn();
// no await so we can test for intermediary state
click(resendButton);
// await this.pauseTest();
await waitFor(`${resendButton}[disabled]`, {timeout: 10});
await waitUntil(() => find(resendButton).textContent.includes('Sending'), {timeout: 30, timeoutMessage: 'Check for "Sending" timed out'});
await waitUntil(() => find(resendButton).textContent.includes('Sent'), {timeout: 150, timeoutMessage: 'Check for "Sent" timed out'});
await waitFor(`${resendButton}:not([disabled])`, {timeout: 200});
expect(find(resendButton)).to.have.trimmed.text('Resend');
});
it('handles resend error', async function () {
setupVerificationRequired(this.server);
await completeSignIn();
// default error message
setupResendFailure(this.server);
await click(resendButton);
testMainErrorMessage('There was a problem resending the verification token.');
// server-provided error message
setupResendFailure(this.server, {message: 'Testing error.'});
await click(resendButton);
testMainErrorMessage('Testing error.');
});
});
describe('editor', function () {