fix(security): harden auth and secret handling (HH-444) (#101)

* fix(security): harden auth and credential handling (HH-444)

* fix(security): address HH-444 review blockers

* fix(security): close remaining HH-444 review blockers

---------

Co-authored-by: Rogee <rogee@ipao.vip>

frontend/app/javascript/dashboard/api/auth.js

@@ -14,29 +14,13 @@ export default {
   },
   logout() {
     const urlData = endPoints('logout');
-    const fetchPromise = new Promise((resolve, reject) => {
-      axios
-        .delete(urlData.url)
-        .then(response => {
-          deleteIndexedDBOnLogout();
-          clearCookiesOnLogout();
-          resolve(response);
-        })
-        .catch(error => {
-          reject(error);
-        });
+    return axios.delete(urlData.url).finally(() => {
+      deleteIndexedDBOnLogout();
+      clearCookiesOnLogout();
     });
-    return fetchPromise;
   },
   hasAuthCookie() {
-    return !!Cookies.get('cw_d_session_info');
-  },
-  getAuthData() {
-    if (this.hasAuthCookie()) {
-      const savedAuthInfo = Cookies.get('cw_d_session_info');
-      return JSON.parse(savedAuthInfo || '{}');
-    }
-    return false;
+    return !!Cookies.get('cw_d_session_state');
   },
   profileUpdate({ displayName, avatar, ...profileAttributes }) {
     const formData = new FormData();

frontend/app/javascript/dashboard/api/auth.spec.js

@@ -0,0 +1,17 @@
+import * as APIUtils from '../store/utils/api';
+import Auth from './auth';
+
+vi.spyOn(APIUtils, 'clearCookiesOnLogout').mockImplementation(() => {});
+vi.spyOn(APIUtils, 'deleteIndexedDBOnLogout').mockResolvedValue();
+
+describe('Auth.logout', () => {
+  it('clears browser credentials even when the server rejects logout', async () => {
+    globalThis.axios = {
+      delete: vi.fn().mockRejectedValue({ response: { status: 401 } }),
+    };
+
+    await expect(Auth.logout()).rejects.toEqual({ response: { status: 401 } });
+    expect(APIUtils.deleteIndexedDBOnLogout).toHaveBeenCalledOnce();
+    expect(APIUtils.clearCookiesOnLogout).toHaveBeenCalledOnce();
+  });
+});

frontend/app/javascript/dashboard/helper/APIHelper.js

@@ -1,31 +1,48 @@
 import Auth from '../api/auth';
 
 const parseErrorCode = error => Promise.reject(error);
+const AUTH_HEADERS = ['access-token', 'token-type', 'client', 'expiry', 'uid'];
 
 export default axios => {
   const { apiHost = '' } = window.chatwootConfig || {};
-  const wootApi = axios.create({ baseURL: `${apiHost}/` });
-  // Add Auth Headers to requests if logged in
-  if (Auth.hasAuthCookie()) {
-    const {
-      'access-token': accessToken,
-      'token-type': tokenType,
-      client,
-      expiry,
-      uid,
-    } = Auth.getAuthData();
-    Object.assign(wootApi.defaults.headers.common, {
-      'access-token': accessToken,
-      'token-type': tokenType,
-      client,
-      expiry,
-      uid,
-    });
-  }
-  // Response parsing interceptor
+  const wootApi = axios.create({
+    baseURL: `${apiHost}/`,
+    withCredentials: true,
+  });
+  const authHeaders = {};
+  let refreshRequest;
+
+  wootApi.interceptors.request.use(config => {
+    Object.assign(config.headers, authHeaders);
+    return config;
+  });
   wootApi.interceptors.response.use(
-    response => response,
-    error => parseErrorCode(error)
+    response => {
+      AUTH_HEADERS.forEach(header => {
+        if (response.headers?.[header])
+          authHeaders[header] = response.headers[header];
+      });
+      return response;
+    },
+    async error => {
+      const request = error.config;
+      if (
+        error.response?.status !== 401 ||
+        request?._retry ||
+        request?._sessionRefresh ||
+        !Auth.hasAuthCookie()
+      ) {
+        return parseErrorCode(error);
+      }
+      request._retry = true;
+      refreshRequest ||= wootApi
+        .get('/auth/validate_token', { _sessionRefresh: true })
+        .finally(() => {
+          refreshRequest = null;
+        });
+      await refreshRequest;
+      return wootApi(request);
+    }
   );
   return wootApi;
 };

frontend/app/javascript/dashboard/helper/APIHelper.spec.js

@@ -0,0 +1,43 @@
+import axios from 'axios';
+import Cookies from 'js-cookie';
+import createAPIClient from './APIHelper';
+
+describe('APIHelper browser session exchange', () => {
+  it('keeps the access token in memory across reload exchange and 401 refresh', async () => {
+    vi.spyOn(Cookies, 'get').mockImplementation(name =>
+      name === 'cw_d_session_state' ? '1' : undefined
+    );
+    const requests = [];
+    let protectedCalls = 0;
+    const client = createAPIClient(axios);
+    client.defaults.adapter = config => {
+      requests.push(config);
+      if (config.url === '/auth/validate_token') {
+        return Promise.resolve({
+          data: {},
+          status: 200,
+          statusText: 'OK',
+          headers: { 'access-token': 'short-lived', client: 'browser' },
+          config,
+        });
+      }
+      protectedCalls += 1;
+      if (protectedCalls === 1) {
+        return Promise.reject({ config, response: { status: 401 } });
+      }
+      return Promise.resolve({
+        data: {},
+        status: 200,
+        statusText: 'OK',
+        headers: {},
+        config,
+      });
+    };
+
+    await client.get('/auth/validate_token');
+    await client.get('/api/v1/protected');
+
+    expect(requests.at(-1).headers['access-token']).toBe('short-lived');
+    expect(localStorage.getItem('access-token')).toBeNull();
+  });
+});

frontend/app/javascript/dashboard/store/modules/specs/auth/actions.spec.js

@@ -7,7 +7,6 @@ import '../../../../routes';
 
 vi.spyOn(APIHelpers, 'setUser');
 vi.spyOn(APIHelpers, 'clearCookiesOnLogout');
-vi.spyOn(APIHelpers, 'getHeaderExpiry');
 vi.spyOn(Cookies, 'get');
 
 const commit = vi.fn();

frontend/app/javascript/dashboard/store/utils/api.js

@@ -1,5 +1,3 @@
-import fromUnixTime from 'date-fns/fromUnixTime';
-import differenceInDays from 'date-fns/differenceInDays';
 import Cookies from 'js-cookie';
 import { LOCAL_STORAGE_KEYS } from 'dashboard/constants/localStorage';
 import { SESSION_STORAGE_KEYS } from 'dashboard/constants/sessionStorage';
@@ -25,21 +23,18 @@ export const setUser = user => {
   emitter.emit(ANALYTICS_IDENTITY, { user });
 };
 
-export const getHeaderExpiry = response =>
-  fromUnixTime(response.headers.expiry);
-
 export const setAuthCredentials = response => {
-  const expiryDate = getHeaderExpiry(response);
-  Cookies.set('cw_d_session_info', JSON.stringify(response.headers), {
-    expires: differenceInDays(expiryDate, new Date()),
-  });
-  setUser(response.data.data, expiryDate);
+  setUser(response.data.data);
 };
 
 export const clearBrowserSessionCookies = () => {
   Cookies.remove('cw_d_session_info');
+  Cookies.remove('cw_d_session_state');
   Cookies.remove('auth_data');
   Cookies.remove('user');
+  ['access-token', 'client', 'uid', 'token-type', 'expiry'].forEach(key =>
+    localStorage.removeItem(key)
+  );
 };
 
 export const clearLocalStorageOnLogout = () => {

frontend/app/javascript/dashboard/store/utils/api.spec.js

@@ -0,0 +1,32 @@
+import Cookies from 'js-cookie';
+import { clearBrowserSessionCookies, setAuthCredentials } from './api';
+
+describe('browser auth storage', () => {
+  it('never persists response auth headers in JavaScript-readable storage', () => {
+    const cookie = vi.spyOn(Cookies, 'set');
+    const storage = vi.spyOn(Storage.prototype, 'setItem');
+
+    setAuthCredentials({
+      data: { data: { id: 1 } },
+      headers: { 'access-token': 'xss-readable-token', client: 'browser' },
+    });
+
+    expect(cookie).not.toHaveBeenCalled();
+    expect(storage).not.toHaveBeenCalledWith(
+      expect.any(String),
+      expect.stringContaining('xss-readable-token')
+    );
+  });
+
+  it('removes legacy auth storage during logout', () => {
+    const removeCookie = vi.spyOn(Cookies, 'remove');
+    const removeStorage = vi.spyOn(Storage.prototype, 'removeItem');
+
+    clearBrowserSessionCookies();
+
+    expect(removeCookie).toHaveBeenCalledWith('cw_d_session_info');
+    expect(removeCookie).toHaveBeenCalledWith('cw_d_session_state');
+    expect(removeStorage).toHaveBeenCalledWith('access-token');
+    expect(removeStorage).toHaveBeenCalledWith('client');
+  });
+});

frontend/app/javascript/entrypoints/dashboardConfig.js

@@ -38,23 +38,6 @@ window.globalConfig = Object.assign(
 window.errorLoggingConfig = '';
 window.browserConfig = { browser_name: navigator.userAgent };
 
-try {
-  const sessionCookie = document.cookie
-    .split('; ')
-    .find(cookie => cookie.startsWith('cw_d_session_info='));
-  if (sessionCookie) {
-    const raw = decodeURIComponent(sessionCookie.split('=').slice(1).join('='));
-    const extract = key =>
-      raw.match(new RegExp(`"${key}":"([^"]+)"`))?.[1] || '';
-    const token = extract('access-token');
-    if (token) {
-      localStorage.setItem('access-token', token);
-      localStorage.setItem('client', extract('client'));
-      localStorage.setItem('uid', extract('uid'));
-      localStorage.setItem('token-type', extract('token-type') || 'Bearer');
-      localStorage.setItem('expiry', extract('expiry'));
-    }
-  }
-} catch {
-  // Ignore malformed legacy session cookies.
-}
+['access-token', 'client', 'uid', 'token-type', 'expiry'].forEach(key =>
+  localStorage.removeItem(key)
+);

frontend/app/javascript/shared/helpers/BaseActionCableConnector.js

@@ -13,34 +13,50 @@ class BaseActionCableConnector {
     websocketHost = '',
     presenceInterval = PRESENCE_INTERVAL
   ) {
-    // Read access-token for WebSocket auth from cookie
-    let accessToken = '';
-    try {
-      const raw = Cookies.get('cw_d_session_info');
-      if (raw) {
-        const parsed = JSON.parse(raw);
-        accessToken = parsed['access-token'] || '';
-      }
-    } catch (e) {
-      // Ignore cookie parse errors
-    }
+    this.consumer = null;
+    this.subscription = null;
+    this.websocketHost = websocketHost;
+    this.pubsubToken = pubsubToken;
+    this.app = app;
+    this.events = {};
+    this.reconnectTimer = null;
+    this.isAValidEvent = () => true;
+    this.connect();
+    this.triggerPresenceInterval = () => {
+      setTimeout(() => {
+        this.subscription?.updatePresence();
+        this.triggerPresenceInterval();
+      }, presenceInterval);
+    };
+    this.triggerPresenceInterval();
+  }
 
-    // Default to the page origin so the URL is never undefined.
-    const wsOrigin = websocketHost || window.location.origin;
+  async connect() {
+    const wsOrigin = this.websocketHost || window.location.origin;
     let websocketURL = `${wsOrigin}/cable`;
-    if (accessToken) {
-      websocketURL += `?access-token=${encodeURIComponent(accessToken)}`;
-    } else if (pubsubToken) {
-      websocketURL += `?pubsub_token=${encodeURIComponent(pubsubToken)}`;
+    if (Cookies.get('cw_d_session_state')) {
+      try {
+        const response = await window.axios.post('/api/v1/auth/ws_ticket');
+        const ticket = response.data?.data?.ticket;
+        if (!ticket) throw new Error('missing websocket ticket');
+        websocketURL += `?ticket=${encodeURIComponent(ticket)}`;
+        this.usesWSTicket = true;
+      } catch (error) {
+        this.initReconnectTimer();
+        return;
+      }
+    } else if (this.pubsubToken) {
+      websocketURL += `?pubsub_token=${encodeURIComponent(this.pubsubToken)}`;
+      this.usesWSTicket = false;
     }
 
     this.consumer = createConsumer(websocketURL);
     this.subscription = this.consumer.subscriptions.create(
       {
         channel: 'RoomChannel',
-        pubsub_token: pubsubToken,
-        account_id: app.$store.getters.getCurrentAccountId,
-        user_id: app.$store.getters.getCurrentUserID,
+        pubsub_token: this.pubsubToken,
+        account_id: this.app.$store.getters.getCurrentAccountId,
+        user_id: this.app.$store.getters.getCurrentUserID,
       },
       {
         updatePresence() {
@@ -49,25 +65,23 @@ class BaseActionCableConnector {
         received: this.onReceived,
         disconnected: () => {
           BaseActionCableConnector.isDisconnected = true;
+          if (this.usesWSTicket) {
+            this.consumer?.disconnect();
+            this.consumer = null;
+            this.subscription = null;
+          }
           this.onDisconnected();
           this.initReconnectTimer();
         },
       }
     );
-    this.app = app;
-    this.events = {};
-    this.reconnectTimer = null;
-    this.isAValidEvent = () => true;
-    this.triggerPresenceInterval = () => {
-      setTimeout(() => {
-        this.subscription.updatePresence();
-        this.triggerPresenceInterval();
-      }, presenceInterval);
-    };
-    this.triggerPresenceInterval();
   }
 
   checkConnection() {
+    if (!this.consumer) {
+      this.connect();
+      return;
+    }
     const isConnectionActive = this.consumer.connection.isOpen();
     const isReconnected =
       BaseActionCableConnector.isDisconnected && isConnectionActive;
@@ -101,7 +115,7 @@ class BaseActionCableConnector {
   onDisconnected = () => {};
 
   disconnect() {
-    this.consumer.disconnect();
+    this.consumer?.disconnect();
   }
 
   onReceived = ({ event, data } = {}) => {

frontend/app/javascript/shared/helpers/specs/BaseActionCableConnector.spec.js

@@ -1,25 +1,47 @@
-import { afterEach, describe, expect, it, vi } from 'vitest';
+import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
 
-const { createConsumer, createSubscription, getCookie, subscriptionState } =
-  vi.hoisted(() => {
-    const subscriptionState = { callbacks: null };
-    return {
-      createConsumer: vi.fn(),
-      createSubscription: vi.fn((_identifier, callbacks) => {
-        subscriptionState.callbacks = callbacks;
-        return { updatePresence: vi.fn() };
-      }),
-      getCookie: vi.fn(),
-      subscriptionState,
-    };
-  });
+const {
+  createConsumer,
+  createSubscription,
+  getCookie,
+  post,
+  subscriptionState,
+} = vi.hoisted(() => {
+  const subscriptionState = { callbacks: null };
+  return {
+    createConsumer: vi.fn(),
+    createSubscription: vi.fn((_identifier, callbacks) => {
+      subscriptionState.callbacks = callbacks;
+      return { updatePresence: vi.fn() };
+    }),
+    getCookie: vi.fn(),
+    post: vi.fn(),
+    subscriptionState,
+  };
+});
 
 vi.mock('@rails/actioncable', () => ({ createConsumer }));
 vi.mock('js-cookie', () => ({ default: { get: getCookie } }));
 
 import BaseActionCableConnector from '../BaseActionCableConnector';
 
+const app = {
+  $store: {
+    getters: { getCurrentAccountId: 3, getCurrentUserID: 7 },
+  },
+};
+
 describe('BaseActionCableConnector', () => {
+  beforeEach(() => {
+    vi.useFakeTimers();
+    createConsumer.mockReturnValue({
+      connection: { isOpen: vi.fn(() => true) },
+      disconnect: vi.fn(),
+      subscriptions: { create: createSubscription },
+    });
+    window.axios = { post };
+  });
+
   afterEach(() => {
     vi.clearAllMocks();
     vi.useRealTimers();
@@ -28,16 +50,9 @@ describe('BaseActionCableConnector', () => {
   });
 
   it('authenticates a widget socket and RoomChannel with its pubsub token', () => {
-    vi.useFakeTimers();
     getCookie.mockReturnValue(undefined);
-    createConsumer.mockReturnValue({
-      subscriptions: { create: createSubscription },
-    });
 
-    new BaseActionCableConnector(
-      { $store: { getters: {} } },
-      'visitor token'
-    );
+    new BaseActionCableConnector(app, 'visitor token');
 
     expect(createConsumer).toHaveBeenCalledWith(
       `${window.location.origin}/cable?pubsub_token=visitor%20token`
@@ -51,18 +66,9 @@ describe('BaseActionCableConnector', () => {
     );
   });
 
-  it('continues consuming widget events after ActionCable reconnects', () => {
-    vi.useFakeTimers();
+  it('continues consuming widget events after ActionCable reconnects', async () => {
     getCookie.mockReturnValue(undefined);
-    const isOpen = vi.fn(() => true);
-    createConsumer.mockReturnValue({
-      connection: { isOpen },
-      subscriptions: { create: createSubscription },
-    });
-    const connector = new BaseActionCableConnector(
-      { $store: { getters: {} } },
-      'visitor-token'
-    );
+    const connector = new BaseActionCableConnector(app, 'visitor-token');
     const onMessage = vi.fn();
     connector.events['message.created'] = onMessage;
     connector.onDisconnected = vi.fn();
@@ -73,7 +79,7 @@ describe('BaseActionCableConnector', () => {
       data: { id: 1, content: 'before reconnect' },
     });
     subscriptionState.callbacks.disconnected();
-    vi.advanceTimersByTime(1000);
+    await vi.advanceTimersByTimeAsync(1000);
     subscriptionState.callbacks.received({
       event: 'message.created',
       data: { id: 2, content: 'after reconnect' },
@@ -90,4 +96,51 @@ describe('BaseActionCableConnector', () => {
       content: 'after reconnect',
     });
   });
+
+  it('exchanges the dashboard session for a ticket without putting JWT in the URL', async () => {
+    getCookie.mockReturnValue('1');
+    post.mockResolvedValue({ data: { data: { ticket: 'one-time' } } });
+
+    const connector = new BaseActionCableConnector(
+      app,
+      'pubsub',
+      'wss://chat.test'
+    );
+    await vi.waitFor(() => expect(createConsumer).toHaveBeenCalledOnce());
+
+    expect(post).toHaveBeenCalledWith('/api/v1/auth/ws_ticket');
+    expect(createConsumer).toHaveBeenCalledWith(
+      'wss://chat.test/cable?ticket=one-time'
+    );
+    expect(createConsumer.mock.calls[0][0]).not.toContain('access-token');
+    connector.disconnect();
+  });
+
+  it('gets a fresh ticket after a dashboard disconnect', async () => {
+    getCookie.mockReturnValue('1');
+    post
+      .mockResolvedValueOnce({ data: { data: { ticket: 'first' } } })
+      .mockResolvedValueOnce({ data: { data: { ticket: 'second' } } });
+
+    const connector = new BaseActionCableConnector(app, 'pubsub');
+    await vi.waitFor(() => expect(createConsumer).toHaveBeenCalledOnce());
+    subscriptionState.callbacks.disconnected();
+    await vi.advanceTimersByTimeAsync(1000);
+    await vi.waitFor(() => expect(createConsumer).toHaveBeenCalledTimes(2));
+
+    expect(createConsumer).toHaveBeenLastCalledWith(
+      `${window.location.origin}/cable?ticket=second`
+    );
+    connector.disconnect();
+  });
+
+  it('does not open a dashboard socket when ticket exchange returns 401', async () => {
+    getCookie.mockReturnValue('1');
+    post.mockRejectedValue({ response: { status: 401 } });
+
+    new BaseActionCableConnector(app, 'pubsub');
+    await vi.waitFor(() => expect(post).toHaveBeenCalledOnce());
+
+    expect(createConsumer).not.toHaveBeenCalled();
+  });
 });

frontend/app/javascript/v3/api/apiClient.js

@@ -1,6 +1,6 @@
 import axios from 'axios';
 
 const { apiHost = '' } = window.chatwootConfig || {};
-const wootAPI = axios.create({ baseURL: `${apiHost}/` });
+const wootAPI = axios.create({ baseURL: `${apiHost}/`, withCredentials: true });
 
 export default wootAPI;

frontend/app/javascript/v3/helpers/AuthHelper.js

@@ -3,7 +3,7 @@ import { DEFAULT_REDIRECT_URL } from 'dashboard/constants/globals';
 import { frontendURL } from 'dashboard/helper/URLHelper';
 
 export const hasAuthCookie = () => {
-  return !!Cookies.get('cw_d_session_info');
+  return !!Cookies.get('cw_d_session_state');
 };
 
 const getSSOAccountPath = ({ ssoAccountId, user }) => {

frontend/super_admin.html

@@ -45,29 +45,6 @@
         );
         window.errorLoggingConfig = '';
         window.browserConfig = { browser_name: navigator.userAgent };
-        
-        // sync auth tokens from cookie to localStorage
-        (function() {
-          try {
-            var cookies = document.cookie.split('; ');
-            var sessionCookie = cookies.find(function(c) { return c.startsWith('cw_d_session_info='); });
-            if (sessionCookie) {
-              var raw = decodeURIComponent(sessionCookie.split('=').slice(1).join('='));
-              var extract = function(key) {
-                var match = raw.match(new RegExp('"' + key + '":"([^"]+)"'));
-                return match ? match[1] : '';
-              };
-              var token = extract('access-token');
-              if (token) {
-                localStorage.setItem('access-token', token);
-                localStorage.setItem('client', extract('client'));
-                localStorage.setItem('uid', extract('uid'));
-                localStorage.setItem('token-type', extract('token-type') || 'Bearer');
-                localStorage.setItem('expiry', extract('expiry'));
-              }
-            }
-          } catch(e) { /* ignore */ }
-        })();
       })();
     </script>
   </head>