Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 15 commits into from
Oct 21, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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