Skip to content

Commit

Permalink
7538 - Validate cookie when restoring the app from the bfcache (#7575)
Browse files Browse the repository at this point in the history
Ticket: #7538

This commit: 
- When the app is restored from the bfcache, the session is validated once more to make sure the user doesn't get stuck in an impossible state.
  • Loading branch information
m5r authored Apr 19, 2022
1 parent f0c5498 commit 6f3551a
Show file tree
Hide file tree
Showing 12 changed files with 100 additions and 12 deletions.
1 change: 1 addition & 0 deletions admin/src/js/services/db.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ angular.module('inboxServices').factory('DB',
const isOnlineOnly = Session.isOnlineOnly();

const getUsername = remote => {
Session.checkCurrentSession();
const username = Session.userCtx().name;
if (!remote) {
return username;
Expand Down
2 changes: 1 addition & 1 deletion admin/src/js/services/session.js
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ const _ = require('lodash/core');

navigateToLogin: navigateToLogin,

init: checkCurrentSession,
checkCurrentSession: checkCurrentSession,

/**
* Returns true if the logged in user has the db or national admin role.
Expand Down
3 changes: 2 additions & 1 deletion admin/tests/unit/services/db.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,8 @@ describe('DB service', () => {
});
$provide.value('Session', {
userCtx: userCtx,
isOnlineOnly: isOnlineOnly
isOnlineOnly: isOnlineOnly,
checkCurrentSession: sinon.stub(),
} );
$provide.value('Location', Location);
});
Expand Down
10 changes: 5 additions & 5 deletions admin/tests/unit/services/session.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ describe('Session service', function() {
$httpBackend
.expect('DELETE', '/_session')
.respond(200);
service.init();
service.checkCurrentSession();
$httpBackend.flush();
chai.expect(location.href).to.equal('/DB_NAME/login?redirect=CURRENT_URL');
chai.expect(ipCookieRemove.args[0][0]).to.equal('userCtx');
Expand All @@ -82,7 +82,7 @@ describe('Session service', function() {
$httpBackend
.expect('GET', '/_session')
.respond(401);
service.init();
service.checkCurrentSession();
$httpBackend.flush();
chai.expect(ipCookieRemove.args[0][0]).to.equal('userCtx');
done();
Expand All @@ -93,7 +93,7 @@ describe('Session service', function() {
$httpBackend
.expect('GET', '/_session')
.respond(0);
service.init();
service.checkCurrentSession();
$httpBackend.flush();
chai.expect(ipCookieRemove.callCount).to.equal(0);
done();
Expand All @@ -110,7 +110,7 @@ describe('Session service', function() {
$httpBackend
.expect('DELETE', '/_session')
.respond(200);
service.init();
service.checkCurrentSession();
$httpBackend.flush();
chai.expect(location.href).to.equal(`/DB_NAME/login?redirect=CURRENT_URL&username=${expected.name}`);
chai.expect(ipCookieRemove.args[0][0]).to.equal('userCtx');
Expand All @@ -122,7 +122,7 @@ describe('Session service', function() {
$httpBackend
.expect('GET', '/_session')
.respond(200, { userCtx: { name: 'bryan' } });
service.init();
service.checkCurrentSession();
$httpBackend.flush();
chai.expect(ipCookieRemove.callCount).to.equal(0);
done();
Expand Down
1 change: 1 addition & 0 deletions admin/tests/unit/services/telemetry.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ describe('Telemetry service', () => {
userCtx: () => {
return { name: 'greg' };
},
checkCurrentSession: sinon.stub(),
});
});
inject(_Telemetry_ => {
Expand Down
5 changes: 4 additions & 1 deletion admin/tests/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,10 @@ window.KarmaUtils = {
$provide.value('ContactViewModelGenerator', () => {});
$provide.value('ReportViewModelGenerator', () => {});
$provide.value('LiveList', mockLiveList);
$provide.value('Session', { userCtx: sinon.stub().returns({}) });
$provide.value('Session', {
userCtx: sinon.stub().returns({}),
checkCurrentSession: sinon.stub(),
});
});
}
};
Expand Down
6 changes: 6 additions & 0 deletions api/src/public/login/script.js
Original file line number Diff line number Diff line change
Expand Up @@ -213,3 +213,9 @@ document.addEventListener('DOMContentLoaded', function() {
}
}
});

window.addEventListener('pageshow', (event) => {
if (event.persisted) {
checkSession();
}
});
52 changes: 52 additions & 0 deletions tests/e2e/navigation/bfcache.wdio-spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
const commonPage = require('../../page-objects/common/common.wdio.page');
const loginPage = require('../../page-objects/login/login.wdio.page');
const usersAdminPage = require('../../page-objects/admin/user.wdio.page');
const auth = require('../../auth')();

describe('bfcache', async () => {
beforeEach(async () => {
await loginPage.login({
username: auth.username,
password: auth.password,
createUser: true,
});
});

afterEach(async () => {
await browser.deleteCookies();
await browser.url('/');
});

describe('login page', () => {
it('should redirect to the app page when session is valid', async () => {
await commonPage.goToBase();
expect(await browser.getUrl()).to.contain('/messages');
await browser.back();
await browser.waitUntil(async () => (await browser.getUrl()).includes('/messages'));
});
});

describe('webapp', () => {
it('should redirect to login page when session is expired', async () => {
await commonPage.goToPeople();
await browser.deleteCookies('AuthSession');
await commonPage.goToMessages();
const redirectToLoginBtn = await $('#session-expired .btn.submit.btn-primary');
await redirectToLoginBtn.click();
expect(await browser.getUrl()).to.contain('/login?redirect=');
await browser.back();
await browser.waitUntil(async () => (await browser.getUrl()).includes('/login?redirect='));
});
});

describe('admin app', () => {
it('should redirect to login page when session is expired', async () => {
await usersAdminPage.goToAdminUser();
await browser.deleteCookies('AuthSession');
await usersAdminPage.goToAdminUpgrade();
expect(await browser.getUrl()).to.contain('/login?redirect=');
await browser.back();
await browser.waitUntil(async () => (await browser.getUrl()).includes('/login?redirect='));
});
});
});
9 changes: 7 additions & 2 deletions tests/page-objects/admin/user.wdio.page.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,10 @@ const goToAdminUser = async () => {
await browser.url('/admin/#/users');
};

const goToAdminUpgrade = async () => {
await browser.url('/admin/#/upgrade');
};

const openAddUserDialog = async () => {
await (await addUserButton()).waitForDisplayed();
await (await addUserButton()).click();
Expand All @@ -40,7 +44,7 @@ const closeUserDialog = async () => {
await (await addUserDialog()).waitForDisplayed({ reverse: true });
};

const inputAddUserFields = async (username, fullname, role, place, associatedContact, password,
const inputAddUserFields = async (username, fullname, role, place, associatedContact, password,
confirmPassword = password) => {
await (await userName()).addValue(username);
await (await userFullName()).addValue(fullname);
Expand All @@ -53,7 +57,7 @@ const inputAddUserFields = async (username, fullname, role, place, associatedCon
if (!_.isEmpty(associatedContact)) {
await selectContact(associatedContact);
}

await (await userPassword()).addValue(password);
await (await userConfirmPassword()).addValue(confirmPassword);
};
Expand Down Expand Up @@ -117,6 +121,7 @@ const getContactErrorText = async () => {

module.exports = {
goToAdminUser,
goToAdminUpgrade,
openAddUserDialog,
closeUserDialog,
inputAddUserFields,
Expand Down
2 changes: 1 addition & 1 deletion tests/wdio.conf.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ const baseConfig = {
// will be called from there.
//
specs: [
'./tests/e2e/**/*.wdio-spec.js'
'./tests/e2e/**/*.wdio-spec.js',
],
// Patterns to exclude.
exclude: [
Expand Down
7 changes: 7 additions & 0 deletions webapp/src/ts/app.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -675,6 +675,13 @@ export class AppComponent implements OnInit {
this.changesService.killWatchers();
}

@HostListener('window:pageshow', ['$event'])
private pageshow(event) {
if (event.persisted) {
this.sessionService.check();
}
}

private async initAnalyticsModules() {
try {
const modules = await this.analyticsModulesService.get();
Expand Down
14 changes: 13 additions & 1 deletion webapp/src/ts/services/session.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,12 @@ export class SessionService {
*/
userCtx () {
if (!this.userCtxCookieValue) {
this.userCtxCookieValue = JSON.parse(this.cookieService.get(COOKIE_NAME));
try {
this.userCtxCookieValue = JSON.parse(this.cookieService.get(COOKIE_NAME));
} catch(error) {
console.error('Cookie parsing error', error);
this.userCtxCookieValue = null;
}
}

return this.userCtxCookieValue;
Expand All @@ -68,11 +73,18 @@ export class SessionService {
.catch(this.logout);
}

public check() {
if (!this.cookieService.check(COOKIE_NAME)) {
this.navigateToLogin();
}
}

init () {
const userCtx = this.userCtx();
if (!userCtx || !userCtx.name) {
return this.logout();
}

return this.http
.get<{ userCtx: { name:string; roles:string[] } }>('/_session', { responseType: 'json' })
.toPromise()
Expand Down

0 comments on commit 6f3551a

Please sign in to comment.