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

PTV-1913 force cookie resetting, including backup cookie #3657

Merged
merged 12 commits into from
Sep 27, 2024
50 changes: 27 additions & 23 deletions kbase-extension/static/kbase/js/api/auth.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
define(['bluebird', 'jquery', 'narrativeConfig'], (Promise, $, Config) => {
'use strict';

function factory(config) {
const url = config.url;
const secureCookies =
Expand All @@ -24,7 +22,7 @@ define(['bluebird', 'jquery', 'narrativeConfig'], (Promise, $, Config) => {
},
};

const TOKEN_AGE = 14; // days
const DEFAULT_TOKEN_LIFE = 14 * 24 * 60 * 60 * 1000; // millis

/**
* Meant for managing auth or session cookies (mainly auth cookies as set by
Expand All @@ -33,10 +31,10 @@ define(['bluebird', 'jquery', 'narrativeConfig'], (Promise, $, Config) => {
* If it's missing name or value, does nothing.
* Default expiration time is 14 days.
* domain, expires, and max-age are optional
* expires is expected to be in days
* auto set fields are:
* expires is expected to be the timestamp (ms since epoch) it will expire
* default fields are:
* - path = '/'
* - expires = TOKEN_AGE (default 14) days
* - expires = now + 14 days
* @param {object} cookie
* - has the cookie keys: name, value, path, expires, max-age, domain
* - adds secure=true, samesite=Lax for KBase use.
Expand All @@ -50,7 +48,7 @@ define(['bluebird', 'jquery', 'narrativeConfig'], (Promise, $, Config) => {
const name = encodeURIComponent(cookie.name);
const value = encodeURIComponent(cookie.value || '');
const props = {
expires: TOKEN_AGE, // gets translated to GMT string
expires: Date.now() + DEFAULT_TOKEN_LIFE, // gets translated to GMT string
path: '/',
samesite: 'Lax',
};
Expand All @@ -66,14 +64,8 @@ define(['bluebird', 'jquery', 'narrativeConfig'], (Promise, $, Config) => {
if (cookie.domain) {
props.domain = cookie.domain;
}
props['max-age'] = 86400 * props.expires;
if (props.expires === 0) {
props.expires = new Date(0).toUTCString();
} else {
props.expires = new Date(
new Date().getTime() + 86400000 * props.expires
).toUTCString();
}
props['max-age'] = parseInt((props.expires - Date.now()) / 1000);
props.expires = new Date(props.expires).toUTCString();

const fields = Object.keys(props).map((key) => {
return `${key}=${props[key]}`;
Expand All @@ -85,7 +77,7 @@ define(['bluebird', 'jquery', 'narrativeConfig'], (Promise, $, Config) => {

const propStr = fields.join(';');

const newCookie = `${name}=${value}; ${propStr}`;
const newCookie = `${name}=${value};${propStr}`;
document.cookie = newCookie;
}

Expand Down Expand Up @@ -161,22 +153,34 @@ define(['bluebird', 'jquery', 'narrativeConfig'], (Promise, $, Config) => {
return getCookie(cookieConfig.auth.name);
}

/* Sets the given auth token into the browser's cookie.
* Does nothing if the token is null.
/**
* Returns a Promise that ets the given auth token into the
* browser's cookie, as configured. The cookie has the
* same lifespan as the token.
* If the token is null or expired, this does nothing.
* If there's an error in looking up the token, this throws
* an error.
* @param {string} token
* @returns
*/
function setAuthToken(token) {
async function setAuthToken(token) {
const tokenInfo = await getTokenInfo(token);
// if it's expired, don't set (actually should've thrown
// here, and get caught by the caller, but check anyway)
if (tokenInfo.expires - Date.now() <= 0) {
return;
}
const deployEnv = Config.get('environment');

function setToken(config) {
// Honor cookie host whitelist if present.
if (config.enableIn) {
if (config.enableIn.indexOf(deployEnv) === -1) {
return;
}
if (config.enableIn && !config.enableIn.includes(deployEnv)) {
return;
}
const cookieField = {
name: config.name,
value: token,
expires: tokenInfo.expires,
};
if (config.domain) {
cookieField.domain = config.domain;
Expand Down
19 changes: 10 additions & 9 deletions kbase-extension/static/kbase/js/narrativeLogin.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ define([
'userMenu',
'util/bootstrapDialog',
], ($, Promise, kbapi, JupyterUtils, Config, Auth, UserMenu, BootstrapDialog) => {
'use strict';
const baseUrl = JupyterUtils.get_body_data('baseUrl');
const authClient = Auth.make({ url: Config.url('auth') });
let sessionInfo = null;
Expand Down Expand Up @@ -98,10 +97,8 @@ define([
function showNotLoggedInDialog() {
const message = `
<p>You are logged out (or your session has expired).</p>
<p>You will be redirected to the sign in page after closing this, or ${
AUTO_LOGOUT_DELAY / 1000
} seconds,
whichever comes first.</p>
<p>You will be redirected to the sign in page after closing this, or
${AUTO_LOGOUT_DELAY / 1000} seconds, whichever comes first.</p>
`;
const dialog = new BootstrapDialog({
title: 'Logged Out',
Expand Down Expand Up @@ -304,10 +301,14 @@ define([
*/
clearTokenCheckTimers();
const sessionToken = authClient.getAuthToken();
return Promise.all([
authClient.getTokenInfo(sessionToken),
authClient.getUserProfile(sessionToken),
])
return authClient
.setAuthToken(sessionToken)
.then(() =>
Promise.all([
authClient.getTokenInfo(sessionToken),
authClient.getUserProfile(sessionToken),
])
)
.then(([tokenInfo, accountInfo]) => {
sessionInfo = tokenInfo;
this.sessionInfo = tokenInfo;
Expand Down
57 changes: 57 additions & 0 deletions test/integration/specs/cookie_tests.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
/* global browser, $ */
const { login } = require('../wdioUtils.js');
const TIMEOUT = 30000;
const env = process.env.ENV || 'ci';
const testData = require('./narrative_basic_data.json');
const testCases = testData.envs[env];
const AUTH_COOKIE = 'kbase_session';
const AUTH_BACKUP_COOKIE = 'kbase_session_backup';

describe('Auth cookie tests', () => {
beforeEach(async () => {
await browser.setTimeout({ pageLoad: TIMEOUT });
await browser.reloadSession();
await login();
});

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

it('opens a narrative and has a backup cookie, if expected by the env', async () => {
await browser.url(`/narrative/${testCases.CASE_1.narrativeId}`);
const loadingBlocker = await $('#kb-loading-blocker');
await loadingBlocker.waitForDisplayed({
timeout: TIMEOUT,
timeoutMsg: `Timeout after waiting ${TIMEOUT}ms for loading blocker to appear`,
});

// And then the loading blocker should disappear!
await loadingBlocker.waitForDisplayed({
timeout: TIMEOUT,
timeoutMsg: `Timeout after waiting ${TIMEOUT}ms for loading blocker to disappear`,
reverse: true,
});

const KBASE_ENV = browser.options.testParams.ENV;
const cookies = await browser.getCookies();
const expectedNames = [AUTH_COOKIE, AUTH_BACKUP_COOKIE];
const authCookies = cookies.reduce((cookies, cookie) => {
if (expectedNames.includes(cookie.name)) {
cookies[cookie.name] = cookie;
}
return cookies;
}, {});
if (KBASE_ENV === 'narrative') {
expect(authCookies.length).toBe(2);
expectedNames.forEach((cookieName) => {
expect(cookieName in authCookies).toBeTruthy();
});
expect(authCookies[AUTH_COOKIE].value).toEqual(authCookies[AUTH_BACKUP_COOKIE].value);
} else {
expect(Object.keys(authCookies).length).toBe(1);
expect(AUTH_COOKIE in authCookies).toBeTruthy();
expect(AUTH_BACKUP_COOKIE in authCookies).toBeFalsy();
}
});
});
93 changes: 71 additions & 22 deletions test/unit/spec/api/authSpec.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
define(['api/auth', 'narrativeConfig', 'uuid', 'testUtil'], (Auth, Config, Uuid, TestUtil) => {
'use strict';

let authClient;
const FAKE_TOKEN = 'a_fake_auth_token',
FAKE_USER = 'some_user',
Expand Down Expand Up @@ -44,6 +42,28 @@ define(['api/auth', 'narrativeConfig', 'uuid', 'testUtil'], (Auth, Config, Uuid,
idents: [],
};

/**
* Makes a dummy token that expires either 100 seconds from now or 100 seconds ago,
* depending on the isValid variable.
* @param {int} lifespan in ms from Date.now(), default 100000.
* Make 0 or negative for an expired token.
* @param {string} tokenType default 'Login'
* @returns {object} a tokenInfo object from the auth service.
*/
function makeTokenInfo(lifespan, tokenType) {
tokenType = tokenType || 'Login';
lifespan = lifespan || 100000;
return {
expires: Date.now() + lifespan,
created: Date.now(),
name: 'some_token',
id: 'some_uuid',
type: tokenType,
user: FAKE_USER,
cachefor: 500000,
};
}

function setToken(token) {
cookieKeys.forEach((key) => {
document.cookie = `${key}=${token}`;
Expand Down Expand Up @@ -229,15 +249,7 @@ define(['api/auth', 'narrativeConfig', 'uuid', 'testUtil'], (Auth, Config, Uuid,
});

it('Should get auth token info', () => {
const tokenInfo = {
expires: Date.now() + 10 * 60 * 60 * 24 * 1000,
created: Date.now(),
name: 'some_token',
id: 'some_uuid',
type: 'Login',
user: FAKE_USER,
cachefor: 500000,
};
const tokenInfo = makeTokenInfo(10 * 1000 * 60 * 60 * 24);
mockAuthRequest('token', tokenInfo, 200);
return authClient.getTokenInfo(FAKE_TOKEN).then((response) => {
Object.keys(tokenInfo).forEach((tokenKey) => {
Expand Down Expand Up @@ -300,11 +312,45 @@ define(['api/auth', 'narrativeConfig', 'uuid', 'testUtil'], (Auth, Config, Uuid,
});
});

it('Should set an auth token cookie', () => {
it('Should set a valid auth token cookie', async () => {
clearToken();
const newToken = 'someRandomToken';
authClient.setAuthToken(newToken);
const tokenInfo = makeTokenInfo();
mockAuthRequest('token', tokenInfo, 200);
const newToken = 'someNewToken';
await authClient.setAuthToken(newToken);
expect(authClient.getAuthToken()).toEqual(newToken);
expect(authClient.getCookie('kbase_session')).toEqual(newToken);
});

/* Leaving this in here though it's not working as expected.
* some mish-mash of Jasmine + the test runner + the headless browser really doesn't
* want to expire old cookies. Testing in a real browser by hand works, but not unit
* tests. Argh!
* Things tried:
* 1. TestUtil.wait for time out
* 2. setTimeout timeouts
* 3. jasmine.clock() mock the clock skipping ahead
* 4. time skips in ranges of seconds to (mocked) days
*/
xit('Should set a valid auth token cookie that expires after 2 seconds', async () => {
clearToken();
const timeoutTime = 2000;
const newToken = 'someTimeoutToken';
const tokenInfo = makeTokenInfo(timeoutTime);
mockAuthRequest('token', tokenInfo, 200);
await authClient.setAuthToken(newToken);
expect(authClient.getAuthToken()).toEqual(newToken);
expect(authClient.getCookie('kbase_session')).toEqual(newToken);
await TestUtil.wait(timeoutTime + 1000);
expect(authClient.getCookie('kbase_session')).toBeNull();
});

it('Should not set an expired token cookie', async () => {
clearToken();
mockAuthRequest('token', makeTokenInfo(-1000));
await authClient.setAuthToken('unsetToken');
expect(authClient.getAuthToken()).toBeNull();
expect(authClient.getCookie('kbase_session')).toBeNull();
});

// test validateToken with either good or invalid tokens
Expand Down Expand Up @@ -354,14 +400,16 @@ define(['api/auth', 'narrativeConfig', 'uuid', 'testUtil'], (Auth, Config, Uuid,
});
});

it('Should clear auth token cookie on request', () => {
it('Should clear auth token cookie on request', async () => {
// Ensure that the token automagically set is removed first.
clearToken();

const fakeToken = 'fakeAuthToken';
const tokenInfo = makeTokenInfo();
mockAuthRequest('token', tokenInfo, 200);
// Setting an arbitrary token should work.
const cookieValue = new Uuid(4).format();
authClient.setAuthToken(cookieValue);
expect(authClient.getAuthToken()).toEqual(cookieValue);
await authClient.setAuthToken(fakeToken);
expect(authClient.getAuthToken()).toEqual(fakeToken);

// Clearing an auth token should also work.
authClient.clearAuthToken();
Expand All @@ -372,7 +420,7 @@ define(['api/auth', 'narrativeConfig', 'uuid', 'testUtil'], (Auth, Config, Uuid,
});

// FIXME: this fails on successive test runs
it('Should properly handle backup cookie in non-prod environment', () => {
it('Should properly handle backup cookie in non-prod environment', async () => {
const env = Config.get('environment');
const backupCookieName = 'kbase_session_backup';
if (env === 'prod') {
Expand All @@ -386,8 +434,9 @@ define(['api/auth', 'narrativeConfig', 'uuid', 'testUtil'], (Auth, Config, Uuid,
// Get a unique fake token, to ensure we don't conflict with
// another cookie value.
const cookieValue = new Uuid(4).format();

authClient.setAuthToken(cookieValue);
const tokenInfo = makeTokenInfo();
mockAuthRequest('token', tokenInfo, 200);
await authClient.setAuthToken(cookieValue);
expect(authClient.getAuthToken()).toEqual(cookieValue);

// There should not be a backup cookie set yet
Expand Down Expand Up @@ -448,7 +497,7 @@ define(['api/auth', 'narrativeConfig', 'uuid', 'testUtil'], (Auth, Config, Uuid,
authClient.setCookie({
name: 'narrative_session',
value: cookieValue,
expires: 14,
expires: Date.now() + 14 * 1000 * 60 * 60 * 24,
});

// Okay, it should be set.
Expand Down
Loading