0
Fork 0
mirror of https://github.com/TryGhost/Ghost.git synced 2025-03-18 02:21:47 -05:00

🐛 Fixed redirect to signin modal not shown when logged out (#15522)

fixes:  https://github.com/TryGhost/Ghost/issues/15291

- An attempt to improve re-authenticate modal toggle - show re-authenticate modal every time user save (ctrl/cmd + s)
- An attempt to fix redirection when user re-login on different tab. Prevent redirection to sign-in page since the user already logged in on another tab.
- Re-enable `editor` test on `authentication-test.js`
This commit is contained in:
Hakim Razalan 2022-10-22 04:03:12 +08:00 committed by GitHub
parent dbad621b91
commit b88a54fb71
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 85 additions and 10 deletions

View file

@ -124,6 +124,7 @@ export default class EditorController extends Controller {
_leaveConfirmed = false;
_previousTagNames = null; // set by setPost and _postSaved, used in hasDirtyAttributes
_reAuthenticateModalToggle = false;
/* computed properties ---------------------------------------------------*/
@ -264,6 +265,8 @@ export default class EditorController extends Controller {
@action
toggleReAuthenticateModal() {
this._reAuthenticateModalToggle = true;
if (this.showReAuthenticateModal) {
// closing, re-attempt save if needed
if (this._reauthSave) {
@ -476,6 +479,9 @@ export default class EditorController extends Controller {
post.set('statusScratch', null);
// Clear any error notification (if any)
this.notifications.clearAll();
if (!options.silent) {
this._showSaveNotification(prevStatus, post.get('status'), isNew ? true : false);
}
@ -490,6 +496,11 @@ export default class EditorController extends Controller {
return post;
} catch (error) {
if (!this.session.isAuthenticated && !this._reAuthenticateModalToggle) {
this.toggleProperty('showReAuthenticateModal');
}
this._reAuthenticateModalToggle = false;
if (this.showReAuthenticateModal) {
this._reauthSave = true;
this._reauthSaveOptions = options;

View file

@ -120,6 +120,7 @@ export default class LexicalEditorController extends Controller {
_leaveConfirmed = false;
_previousTagNames = null; // set by setPost and _postSaved, used in hasDirtyAttributes
_reAuthenticateModalToggle = false;
/* computed properties ---------------------------------------------------*/
@ -260,6 +261,8 @@ export default class LexicalEditorController extends Controller {
@action
toggleReAuthenticateModal() {
this._reAuthenticateModalToggle = true;
if (this.showReAuthenticateModal) {
// closing, re-attempt save if needed
if (this._reauthSave) {
@ -473,6 +476,9 @@ export default class LexicalEditorController extends Controller {
post.set('statusScratch', null);
// Clear any error notification (if any)
this.notifications.clearAll();
if (!options.silent) {
this._showSaveNotification(prevStatus, post.get('status'), isNew ? true : false);
}
@ -487,6 +493,11 @@ export default class LexicalEditorController extends Controller {
return post;
} catch (error) {
if (!this.session.isAuthenticated && !this._reAuthenticateModalToggle) {
this.toggleProperty('showReAuthenticateModal');
}
this._reAuthenticateModalToggle = false;
if (this.showReAuthenticateModal) {
this._reauthSave = true;
this._reauthSaveOptions = options;

View file

@ -4,7 +4,7 @@ import {inject as service} from '@ember/service';
export default class AuthenticatedRoute extends Route {
@service session;
beforeModel(transition) {
async beforeModel(transition) {
this.session.requireAuthentication(transition, 'signin');
}
}

View file

@ -75,6 +75,29 @@ export default class SessionService extends ESASessionService {
});
}
/**
* Always try to re-setup session & retry the original transition
* if user data is still available in session store although the
* ember-session is unauthenticated.
*
* If success, it will retry the original transition.
* If failed, it will be handled by the redirect to sign in.
*/
async requireAuthentication(transition, route) {
// Only when ember session invalidated
if (!this.isAuthenticated) {
transition.abort();
if (this.user) {
await this.setup();
this.notifications.clearAll();
transition.retry();
}
}
super.requireAuthentication(transition, route);
}
handleInvalidation() {
let transition = this.appLoadTransition;

View file

@ -1,8 +1,9 @@
import ctrlOrCmd from 'ghost-admin/utils/ctrl-or-cmd';
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, findAll, visit} from '@ember/test-helpers';
import {currentRouteName, currentURL, fillIn, findAll, triggerKeyEvent, visit} from '@ember/test-helpers';
import {expect} from 'chai';
import {run} from '@ember/runloop';
import {setupApplicationTest} from 'ember-mocha';
@ -49,7 +50,7 @@ describe('Acceptance: Authentication', function () {
it('invalidates session on 401 API response', async function () {
// return a 401 when attempting to retrieve users
this.server.get('/users/', () => new Response(401, {}, {
this.server.get('/users/me', () => new Response(401, {}, {
errors: [
{message: 'Access denied.', type: 'UnauthorizedError'}
]
@ -68,6 +69,27 @@ describe('Acceptance: Authentication', function () {
expect(currentURL(), 'url after 401').to.equal('/signin');
});
it('invalidates session on 403 API response', async function () {
// return a 401 when attempting to retrieve users
this.server.get('/users/me', () => new Response(403, {}, {
errors: [
{message: 'Authorization failed', type: 'NoPermissionError'}
]
}));
await authenticateSession();
await visit('/settings/staff');
// running `visit(url)` inside windowProxy.replaceLocation breaks
// the async behaviour so we need to run `visit` here to simulate
// the browser visiting the new page
if (newLocation) {
await visit(newLocation);
}
expect(currentURL(), 'url after 403').to.equal('/signin');
});
it('doesn\'t show navigation menu on invalid url when not authenticated', async function () {
await invalidateSession();
@ -94,7 +116,7 @@ describe('Acceptance: Authentication', function () {
});
// TODO: re-enable once modal reappears correctly
describe.skip('editor', function () {
describe('editor', function () {
let origDebounce = run.debounce;
let origThrottle = run.throttle;
@ -107,13 +129,14 @@ describe('Acceptance: Authentication', function () {
it('displays re-auth modal attempting to save with invalid session', async function () {
let role = this.server.create('role', {name: 'Administrator'});
this.server.create('user', {roles: [role]});
let testOn = 'save'; // use marker for different type of server.put result
// simulate an invalid session when saving the edited post
this.server.put('/posts/:id/', function ({posts}, {params}) {
this.server.put('/posts/:id/', function ({posts, db}, {params}) {
let post = posts.find(params.id);
let attrs = this.normalizedRequestAttrs();
let attrs = db.posts.find(params.id); // use attribute from db.posts to avoid hasInverseFor error
if (attrs.mobiledoc.cards[0][1].markdown === 'Edited post body') {
if (testOn === 'edit') {
return new Response(401, {}, {
errors: [
{message: 'Access denied.', type: 'UnauthorizedError'}
@ -129,9 +152,12 @@ describe('Acceptance: Authentication', function () {
await visit('/editor');
// create the post
await fillIn('#entry-title', 'Test Post');
await fillIn('.gh-editor-title', 'Test Post');
await fillIn('.__mobiledoc-editor', 'Test post body');
await click('.js-publish-button');
await triggerKeyEvent('.gh-editor-title', 'keydown', 83, {
metaKey: ctrlOrCmd === 'command',
ctrlKey: ctrlOrCmd === 'ctrl'
});
// we shouldn't have a modal at this point
expect(findAll('.modal-container #login').length, 'modal exists').to.equal(0);
@ -139,8 +165,12 @@ describe('Acceptance: Authentication', function () {
expect(findAll('.gh-alert').length, 'no of alerts').to.equal(0);
// update the post
testOn = 'edit';
await fillIn('.__mobiledoc-editor', 'Edited post body');
await click('.js-publish-button');
await 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);