PTV-1913 force cookie resetting, including backup cookie

eslint.config.mjs

@@ -41,8 +41,6 @@ export default [{
     },
 
     rules: {
-        strict: ["error", "function"],
-
         "no-console": ["error", {
             allow: ["warn", "error"],
         }],

kbase-extension/static/kbase/js/api/auth.js

@@ -24,7 +24,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
@@ -33,10 +33,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.
@@ -50,7 +50,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',
             };
@@ -66,14 +66,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]}`;
@@ -85,7 +79,7 @@ define(['bluebird', 'jquery', 'narrativeConfig'], (Promise, $, Config) => {
 
             const propStr = fields.join(';');
 
-            const newCookie = `${name}=${value}; ${propStr}`;
+            const newCookie = `${name}=${value};${propStr}`;
             document.cookie = newCookie;
         }
 
@@ -161,22 +155,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;

kbase-extension/static/kbase/js/narrativeLogin.js

@@ -15,6 +15,7 @@ define([
     '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;
@@ -98,10 +99,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',
@@ -304,10 +303,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;

test/integration/specs/cookie_tests.js

@@ -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();
+        }
+    });
+});

test/unit/spec/api/authSpec.js

@@ -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',
@@ -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}`;
@@ -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) => {
@@ -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 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 newToken = 'someRandomToken';
-            authClient.setAuthToken(newToken);
+            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
@@ -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();
@@ -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') {
@@ -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
@@ -414,8 +463,9 @@ define(['api/auth', 'narrativeConfig', 'uuid', 'testUtil'], (Auth, Config, Uuid,
             expect(authClient.getCookie(backupCookieName)).toEqual(backupCookieValue);
         });
 
-        it('Should set and clear backup cookie in prod', () => {
+        it('Should set and clear backup cookie in prod', async () => {
             const env = Config.get('environment');
+            const tokenCookie = 'kbase_session';
             const backupCookieName = 'kbase_session_backup';
             if (env !== 'prod') {
                 pending('This test is only valid for a prod config');
@@ -427,14 +477,16 @@ define(['api/auth', 'narrativeConfig', 'uuid', 'testUtil'], (Auth, Config, Uuid,
 
             // Setting an arbitrary token should work.
             const cookieValue = new Uuid(4).format();
-            authClient.setAuthToken(cookieValue);
+            mockAuthRequest('token', makeTokenInfo(), 200);
+            await authClient.setAuthToken(cookieValue);
             expect(authClient.getAuthToken()).toEqual(cookieValue);
+            expect(authClient.getCookie(tokenCookie)).toEqual(cookieValue);
             expect(authClient.getCookie(backupCookieName)).toEqual(cookieValue);
 
             // Clearing an auth token should also work.
             authClient.clearAuthToken();
             expect(authClient.getAuthToken()).toBeNull();
-            expect(authClient.getCookie('kbase_session')).toBeNull();
+            expect(authClient.getCookie(tokenCookie)).toBeNull();
             expect(authClient.getCookie(backupCookieName)).toBeNull();
         });
 
@@ -448,7 +500,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.