From 332dd4fbf1b36499a774268fce10bfae08c17537 Mon Sep 17 00:00:00 2001 From: Kevin Ansfield Date: Fri, 11 Nov 2022 16:21:03 +0000 Subject: [PATCH] Switched editor re-authenticate modal to new modal pattern refs https://github.com/TryGhost/Team/issues/1734 refs https://github.com/TryGhost/Team/issues/559 refs https://github.com/TryGhost/Ghost/issues/14101 - switches to newer modal patterns ready for later Ember upgrades - simplified unauthed save behaviour because we now have a promise for the modal enabling us to wait for the modal to close before continuing --- .../editor/modals/re-authenticate.hbs | 37 ++++++++ .../editor/modals/re-authenticate.js | 93 +++++++++++++++++++ .../app/components/modal-re-authenticate.hbs | 26 ------ .../app/components/modal-re-authenticate.js | 78 ---------------- ghost/admin/app/controllers/editor.js | 33 ++----- ghost/admin/app/controllers/lexical-editor.js | 33 ++----- ghost/admin/app/routes/editor.js | 3 +- ghost/admin/app/templates/editor.hbs | 6 -- .../tests/acceptance/authentication-test.js | 12 ++- 9 files changed, 153 insertions(+), 168 deletions(-) create mode 100644 ghost/admin/app/components/editor/modals/re-authenticate.hbs create mode 100644 ghost/admin/app/components/editor/modals/re-authenticate.js delete mode 100644 ghost/admin/app/components/modal-re-authenticate.hbs delete mode 100644 ghost/admin/app/components/modal-re-authenticate.js diff --git a/ghost/admin/app/components/editor/modals/re-authenticate.hbs b/ghost/admin/app/components/editor/modals/re-authenticate.hbs new file mode 100644 index 0000000000..b68a8f921b --- /dev/null +++ b/ghost/admin/app/components/editor/modals/re-authenticate.hbs @@ -0,0 +1,37 @@ +
+
+

Please re-authenticate

+
+ + +
+ + + {{#if this.authenticationError}} +

{{this.authenticationError}}

+ {{/if}} +
+
\ No newline at end of file diff --git a/ghost/admin/app/components/editor/modals/re-authenticate.js b/ghost/admin/app/components/editor/modals/re-authenticate.js new file mode 100644 index 0000000000..951d627929 --- /dev/null +++ b/ghost/admin/app/components/editor/modals/re-authenticate.js @@ -0,0 +1,93 @@ +import Component from '@glimmer/component'; +import EmberObject from '@ember/object'; +import ValidationEngine from 'ghost-admin/mixins/validation-engine'; +import classic from 'ember-classic-decorator'; +import {action} from '@ember/object'; +import {htmlSafe} from '@ember/template'; +import {isVersionMismatchError} from 'ghost-admin/services/ajax'; +import {inject as service} from '@ember/service'; +import {task} from 'ember-concurrency'; +import {tracked} from '@glimmer/tracking'; + +// EmberObject is still needed here for ValidationEngine +@classic +class Signin extends EmberObject.extend(ValidationEngine) { + @tracked identification = ''; + @tracked password = ''; + + validationType = 'signin'; +} + +export default class ReAuthenticateModal extends Component { + @service notifications; + @service session; + + @tracked authenticationError = null; + + constructor() { + super(...arguments); + this.signin = Signin.create(); + this.signin.identification = this.session.user.email; + } + + @action + setPassword(event) { + this.signin.password = event.target.value; + } + + @task({drop: true}) + *reauthenticateTask() { + // Manually trigger events for input fields, ensuring legacy compatibility with + // browsers and password managers that don't send proper events on autofill + const inputs = document.querySelectorAll('#login input'); + inputs.forEach(input => input.dispatchEvent(new Event('change'))); + + this.authenticationError = null; + + try { + yield this.signin.validate({property: 'signin'}); + } catch (error) { + this.signin.hasValidated.pushObject('password'); + return false; + } + + try { + yield this._authenticate(); + this.notifications.closeAlerts(); + this.args.close(); + return true; + } catch (error) { + if (!error) { + return; + } + + if (error?.payload?.errors) { + error.payload.errors.forEach((err) => { + if (isVersionMismatchError(err)) { + return this.notifications.showAPIError(error); + } + err.message = htmlSafe(err.context || err.message); + }); + + this.signin.errors.add('password', 'Incorrect password'); + this.signin.hasValidated.pushObject('password'); + this.authenticationError = error.payload.errors[0].message; + } + + this.notifications.showAPIError(error); + } + } + + async _authenticate() { + const authStrategy = 'authenticator:cookie'; + const {identification, password} = this.signin; + + this.session.skipAuthSuccessHandler = true; + + try { + await this.session.authenticate(authStrategy, identification, password); + } finally { + this.session.skipAuthSuccessHandler = undefined; + } + } +} diff --git a/ghost/admin/app/components/modal-re-authenticate.hbs b/ghost/admin/app/components/modal-re-authenticate.hbs deleted file mode 100644 index a7ee140649..0000000000 --- a/ghost/admin/app/components/modal-re-authenticate.hbs +++ /dev/null @@ -1,26 +0,0 @@ -
-

Please re-authenticate

-
-{{svg-jar "close"}} - -
- - - - {{#if this.authenticationError}} -

{{this.authenticationError}}

- {{/if}} -
diff --git a/ghost/admin/app/components/modal-re-authenticate.js b/ghost/admin/app/components/modal-re-authenticate.js deleted file mode 100644 index 9e3d79ec38..0000000000 --- a/ghost/admin/app/components/modal-re-authenticate.js +++ /dev/null @@ -1,78 +0,0 @@ -import ModalComponent from 'ghost-admin/components/modal-base'; -import ValidationEngine from 'ghost-admin/mixins/validation-engine'; -import {htmlSafe} from '@ember/template'; -import {inject} from 'ghost-admin/decorators/inject'; -import {isVersionMismatchError} from 'ghost-admin/services/ajax'; -import {reads} from '@ember/object/computed'; -import {inject as service} from '@ember/service'; -import {task} from 'ember-concurrency'; - -export default ModalComponent.extend(ValidationEngine, { - notifications: service(), - session: service(), - - validationType: 'signin', - - authenticationError: null, - - config: inject(), - - identification: reads('session.user.email'), - - actions: { - confirm() { - this.reauthenticate.perform(); - } - }, - - _authenticate() { - let session = this.session; - let authStrategy = 'authenticator:cookie'; - let identification = this.identification; - let password = this.password; - - session.set('skipAuthSuccessHandler', true); - - this.toggleProperty('submitting'); - - return session.authenticate(authStrategy, identification, password).finally(() => { - this.toggleProperty('submitting'); - session.set('skipAuthSuccessHandler', undefined); - }); - }, - - _passwordConfirm() { - // Manually trigger events for input fields, ensuring legacy compatibility with - // browsers and password managers that don't send proper events on autofill - const inputs = document.querySelectorAll('#login input'); - inputs.forEach(input => input.dispatchEvent(new Event('change'))); - - this.set('authenticationError', null); - - return this.validate({property: 'signin'}).then(() => this._authenticate().then(() => { - this.notifications.closeAlerts(); - this.send('closeModal'); - return true; - }).catch((error) => { - if (error && error.payload && error.payload.errors) { - error.payload.errors.forEach((err) => { - if (isVersionMismatchError(err)) { - return this.notifications.showAPIError(error); - } - err.message = htmlSafe(err.context || err.message); - }); - - this.errors.add('password', 'Incorrect password'); - this.hasValidated.pushObject('password'); - this.set('authenticationError', error.payload.errors[0].message); - } - }), () => { - this.hasValidated.pushObject('password'); - return false; - }); - }, - - reauthenticate: task(function* () { - return yield this._passwordConfirm(); - }).drop() -}); diff --git a/ghost/admin/app/controllers/editor.js b/ghost/admin/app/controllers/editor.js index 2169f8d72c..59fb25737d 100644 --- a/ghost/admin/app/controllers/editor.js +++ b/ghost/admin/app/controllers/editor.js @@ -4,6 +4,7 @@ import DeletePostModal from '../components/modals/delete-post'; import DeleteSnippetModal from '../components/editor/modals/delete-snippet'; import PostModel from 'ghost-admin/models/post'; import PublishLimitModal from '../components/modals/limits/publish-limit'; +import ReAuthenticateModal from '../components/editor/modals/re-authenticate'; import UpdateSnippetModal from '../components/editor/modals/update-snippet'; import boundOneWay from 'ghost-admin/utils/bound-one-way'; import classic from 'ember-classic-decorator'; @@ -112,7 +113,6 @@ export default class EditorController extends Controller { /* public properties -----------------------------------------------------*/ shouldFocusTitle = false; - showReAuthenticateModal = false; showSettingsMenu = false; /** @@ -127,7 +127,6 @@ export default class EditorController extends Controller { _leaveConfirmed = false; _previousTagNames = null; // set by setPost and _postSaved, used in hasDirtyAttributes - _reAuthenticateModalToggle = false; /* computed properties ---------------------------------------------------*/ @@ -266,22 +265,6 @@ export default class EditorController extends Controller { } } - @action - toggleReAuthenticateModal() { - this._reAuthenticateModalToggle = true; - - if (this.showReAuthenticateModal) { - // closing, re-attempt save if needed - if (this._reauthSave) { - this.saveTask.perform(this._reauthSaveOptions); - } - - this._reauthSave = false; - this._reauthSaveOptions = null; - } - this.toggleProperty('showReAuthenticateModal'); - } - @action openUpgradeModal(hostLimitError = {}) { this.modals.open(PublishLimitModal, { @@ -467,15 +450,13 @@ export default class EditorController extends Controller { return post; } catch (error) { - if (!this.session.isAuthenticated && !this._reAuthenticateModalToggle) { - this.toggleProperty('showReAuthenticateModal'); - } + if (!this.session.isAuthenticated) { + yield this.modals.open(ReAuthenticateModal); - this._reAuthenticateModalToggle = false; - if (this.showReAuthenticateModal) { - this._reauthSave = true; - this._reauthSaveOptions = options; - return; + if (this.session.isAuthenticated) { + this.saveTask.perform(options); + return; + } } this.set('post.status', prevStatus); diff --git a/ghost/admin/app/controllers/lexical-editor.js b/ghost/admin/app/controllers/lexical-editor.js index b0f03af525..5e9dbaa496 100644 --- a/ghost/admin/app/controllers/lexical-editor.js +++ b/ghost/admin/app/controllers/lexical-editor.js @@ -4,6 +4,7 @@ import DeletePostModal from '../components/modals/delete-post'; import DeleteSnippetModal from '../components/editor/modals/delete-snippet'; import PostModel from 'ghost-admin/models/post'; import PublishLimitModal from '../components/modals/limits/publish-limit'; +import ReAuthenticateModal from '../components/editor/modals/re-authenticate'; import UpdateSnippetModal from '../components/editor/modals/update-snippet'; import boundOneWay from 'ghost-admin/utils/bound-one-way'; import classic from 'ember-classic-decorator'; @@ -112,7 +113,6 @@ export default class LexicalEditorController extends Controller { /* public properties -----------------------------------------------------*/ shouldFocusTitle = false; - showReAuthenticateModal = false; showSettingsMenu = false; /** @@ -127,7 +127,6 @@ export default class LexicalEditorController extends Controller { _leaveConfirmed = false; _previousTagNames = null; // set by setPost and _postSaved, used in hasDirtyAttributes - _reAuthenticateModalToggle = false; /* computed properties ---------------------------------------------------*/ @@ -266,22 +265,6 @@ export default class LexicalEditorController extends Controller { } } - @action - toggleReAuthenticateModal() { - this._reAuthenticateModalToggle = true; - - if (this.showReAuthenticateModal) { - // closing, re-attempt save if needed - if (this._reauthSave) { - this.saveTask.perform(this._reauthSaveOptions); - } - - this._reauthSave = false; - this._reauthSaveOptions = null; - } - this.toggleProperty('showReAuthenticateModal'); - } - @action openUpgradeModal(hostLimitError = {}) { this.modals.open(PublishLimitModal, { @@ -468,15 +451,13 @@ export default class LexicalEditorController extends Controller { return post; } catch (error) { - if (!this.session.isAuthenticated && !this._reAuthenticateModalToggle) { - this.toggleProperty('showReAuthenticateModal'); - } + if (!this.session.isAuthenticated) { + yield this.modals.open(ReAuthenticateModal); - this._reAuthenticateModalToggle = false; - if (this.showReAuthenticateModal) { - this._reauthSave = true; - this._reauthSaveOptions = options; - return; + if (this.session.isAuthenticated) { + this.saveTask.perform(options); + return; + } } this.set('post.status', prevStatus); diff --git a/ghost/admin/app/routes/editor.js b/ghost/admin/app/routes/editor.js index 32eab8bec2..3a515fb897 100644 --- a/ghost/admin/app/routes/editor.js +++ b/ghost/admin/app/routes/editor.js @@ -38,7 +38,8 @@ export default AuthenticatedRoute.extend({ }, authorizationFailed() { - this.controller.send('toggleReAuthenticateModal'); + // noop - re-auth is handled by controller save + return; }, willTransition(transition) { diff --git a/ghost/admin/app/templates/editor.hbs b/ghost/admin/app/templates/editor.hbs index 40fee577ad..853c83c8d1 100644 --- a/ghost/admin/app/templates/editor.hbs +++ b/ghost/admin/app/templates/editor.hbs @@ -120,12 +120,6 @@ {{svg-jar "sidemenu"}} {{/if}} - - {{#if this.showReAuthenticateModal}} - - {{/if}} {{/if}} {{outlet}} diff --git a/ghost/admin/tests/acceptance/authentication-test.js b/ghost/admin/tests/acceptance/authentication-test.js index 918f0a9ba0..f2757a2c85 100644 --- a/ghost/admin/tests/acceptance/authentication-test.js +++ b/ghost/admin/tests/acceptance/authentication-test.js @@ -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 {currentRouteName, currentURL, fillIn, findAll, triggerKeyEvent, visit} from '@ember/test-helpers'; +import {click, currentRouteName, currentURL, fillIn, findAll, triggerKeyEvent, visit, waitFor} from '@ember/test-helpers'; import {expect} from 'chai'; import {run} from '@ember/runloop'; import {setupApplicationTest} from 'ember-mocha'; @@ -115,7 +115,6 @@ describe('Acceptance: Authentication', function () { }); }); - // TODO: re-enable once modal reappears correctly describe('editor', function () { let origDebounce = run.debounce; let origThrottle = run.throttle; @@ -160,20 +159,23 @@ describe('Acceptance: Authentication', function () { }); // we shouldn't have a modal at this point - expect(findAll('.modal-container #login').length, 'modal exists').to.equal(0); + expect(findAll('[data-test-modal="re-authenticate"]').length, 'modal exists').to.equal(0); // we also shouldn't have any alerts expect(findAll('.gh-alert').length, 'no of alerts').to.equal(0); // update the post testOn = 'edit'; await fillIn('.__mobiledoc-editor', 'Edited post body'); - await triggerKeyEvent('.gh-editor-title', 'keydown', 83, { + triggerKeyEvent('.gh-editor-title', 'keydown', 83, { metaKey: ctrlOrCmd === 'command', ctrlKey: ctrlOrCmd === 'ctrl' }); // we should see a re-auth modal - expect(findAll('.fullscreen-modal #login').length, 'modal exists').to.equal(1); + await waitFor('[data-test-modal="re-authenticate"]', {timeout: 100}); + + // close the modal so the modal promise is settled and we can continue + await click('[data-test-modal="re-authenticate"] button[title="Close"]'); }); // don't clobber debounce/throttle for future tests