Skip to content

Commit

Permalink
🐛 Fixed redirect to signin modal not shown when logged out (#15522)
Browse files Browse the repository at this point in the history
fixes:  #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`
  • Loading branch information
h2akim committed Oct 21, 2022
1 parent dbad621 commit b88a54f
Show file tree
Hide file tree
Showing 5 changed files with 85 additions and 10 deletions.
11 changes: 11 additions & 0 deletions ghost/admin/app/controllers/editor.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 ---------------------------------------------------*/

Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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);
}
Expand All @@ -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;
Expand Down
11 changes: 11 additions & 0 deletions ghost/admin/app/controllers/lexical-editor.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 ---------------------------------------------------*/

Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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);
}
Expand All @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion ghost/admin/app/routes/authenticated.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
}
}
23 changes: 23 additions & 0 deletions ghost/admin/app/services/session.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
48 changes: 39 additions & 9 deletions ghost/admin/tests/acceptance/authentication-test.js
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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'}
]
Expand All @@ -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();

Expand All @@ -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;

Expand All @@ -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'}
Expand All @@ -129,18 +152,25 @@ 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);
// 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 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);
Expand Down

0 comments on commit b88a54f

Please sign in to comment.