From 6b54bb31586e03309cf652e6d4dd3f10008da517 Mon Sep 17 00:00:00 2001 From: Jean Brito Date: Wed, 8 Jul 2026 12:03:13 -0300 Subject: [PATCH] security(CORE-1130): clear session data on logout transition MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit WEBVIEW_USER_LOGGED_IN was dispatched by the webview preload bridge on every login/logout, but only the reducer consumed it (updating server.userLoggedIn in Redux state). No main-process side effect cleared the logged-out server's session storage/cookies/cache, so login data for a server the user explicitly logged out of remained on disk and in memory. Add handleUserLoggedOutDataClearing() in servers/cache.ts, wired in main.ts next to the existing handleClearCacheDialog(). It listens on WEBVIEW_USER_LOGGED_IN and reuses the existing clearWebviewStorageDeletingLoginData() primitive (previously only reachable from the manual "Clear Cache" dialog). Transition guard: the action fires userLoggedIn=false on webview attach/startup before the user has ever logged in, not just on an actual logout. Reacting to every false payload would wipe storage and reload the webview on every app launch — a regression, not a fix. A per-server-URL Map tracks the previously observed userLoggedIn value (independent of the Redux reducer, which already overwrites server.userLoggedIn by the time listeners run) so the clear only fires on a genuine logged-in(true) -> logged-out(false) transition. Initial false, false->true, and true->true are all no-ops. Scope: this only clears webview session storage/cookies/cache for the server that logged out - it intentionally does not touch app-level data (download history, persisted preferences) or other servers' sessions, since a multi-server client can have one server logged out while staying logged into another. Test: src/servers/main/cache.spec.ts (named to match jest's main-process testMatch, which only discovers src/*/main/**/*.spec.ts or src/**/main.spec.ts - a flat src/servers/cache.main.spec.ts is not picked up). Covers: logged-in->logged-out triggers the clear; initial false at startup does not; logged-out->logged-in does not; missing webContents is a safe no-op. --- src/main.ts | 6 +- src/servers/cache.ts | 26 +++++++++ src/servers/main/cache.spec.ts | 101 +++++++++++++++++++++++++++++++++ 3 files changed, 132 insertions(+), 1 deletion(-) create mode 100644 src/servers/main/cache.spec.ts diff --git a/src/main.ts b/src/main.ts index f62058104e..54fdf107a5 100644 --- a/src/main.ts +++ b/src/main.ts @@ -38,7 +38,10 @@ import { setupOutlookLogger } from './outlookCalendar/logger'; import { handleDesktopCapturerGetSources } from './screenSharing/desktopCapturerCache'; import { setupScreenSharing } from './screenSharing/main'; import { startServerViewScreenSharingHandler } from './screenSharing/serverViewScreenSharing'; -import { handleClearCacheDialog } from './servers/cache'; +import { + handleClearCacheDialog, + handleUserLoggedOutDataClearing, +} from './servers/cache'; import { setupServers } from './servers/main'; import { checkSupportedVersionServers } from './servers/supportedVersions/main'; import { setupSpellChecking } from './spellChecking/main'; @@ -155,6 +158,7 @@ const start = async (): Promise => { handleJitsiDesktopCapturerGetSources(); handleDesktopCapturerGetSources(); handleClearCacheDialog(); + handleUserLoggedOutDataClearing(); startDocumentViewerHandler(); startBrowserHandler(); checkSupportedVersionServers(); diff --git a/src/servers/cache.ts b/src/servers/cache.ts index c59506ca57..6252bb733c 100644 --- a/src/servers/cache.ts +++ b/src/servers/cache.ts @@ -4,7 +4,10 @@ import { listen } from '../store'; import { CLEAR_CACHE_DIALOG_DELETE_LOGIN_DATA_CLICKED, CLEAR_CACHE_DIALOG_KEEP_LOGIN_DATA_CLICKED, + WEBVIEW_USER_LOGGED_IN, } from '../ui/actions'; +import { getWebContentsByServerUrl } from '../ui/main/serverView'; +import type { Server } from './common'; export const clearWebviewStorageKeepingLoginData = async ( guestWebContents: WebContents @@ -51,3 +54,26 @@ export const handleClearCacheDialog = () => { await clearWebviewStorageDeletingLoginData(guestWebContents); }); }; + +const previousUserLoggedInByUrl = new Map< + Server['url'], + Server['userLoggedIn'] +>(); + +export const handleUserLoggedOutDataClearing = (): void => { + listen(WEBVIEW_USER_LOGGED_IN, async (action) => { + const { url, userLoggedIn } = action.payload; + const wasLoggedIn = previousUserLoggedInByUrl.get(url); + previousUserLoggedInByUrl.set(url, userLoggedIn); + + if (wasLoggedIn !== true || userLoggedIn !== false) { + return; + } + + const guestWebContents = getWebContentsByServerUrl(url); + if (!guestWebContents) { + return; + } + await clearWebviewStorageDeletingLoginData(guestWebContents); + }); +}; diff --git a/src/servers/main/cache.spec.ts b/src/servers/main/cache.spec.ts new file mode 100644 index 0000000000..c1d8f98ef5 --- /dev/null +++ b/src/servers/main/cache.spec.ts @@ -0,0 +1,101 @@ +import type { WebContents } from 'electron'; + +import { listen } from '../../store'; +import type { RootAction } from '../../store/actions'; +import { WEBVIEW_USER_LOGGED_IN } from '../../ui/actions'; +import { getWebContentsByServerUrl } from '../../ui/main/serverView'; +import { handleUserLoggedOutDataClearing } from '../cache'; + +jest.mock('electron', () => ({ + webContents: { + fromId: jest.fn(), + }, +})); + +jest.mock('../../store', () => ({ + listen: jest.fn(), +})); + +jest.mock('../../ui/main/serverView', () => ({ + getWebContentsByServerUrl: jest.fn(), +})); + +describe('servers/cache handleUserLoggedOutDataClearing', () => { + const mockListen = listen as unknown as jest.Mock< + () => void, + [string, (action: RootAction) => void] + >; + const mockGetWebContentsByServerUrl = + getWebContentsByServerUrl as jest.MockedFunction< + typeof getWebContentsByServerUrl + >; + + const url = 'https://open.rocket.chat/'; + + const createMockWebContents = (): WebContents => + ({ + session: { + clearCache: jest.fn().mockResolvedValue(undefined), + clearStorageData: jest.fn().mockResolvedValue(undefined), + }, + reloadIgnoringCache: jest.fn(), + }) as unknown as WebContents; + + const dispatchUserLoggedIn = async (userLoggedIn: boolean) => { + const [, listener] = mockListen.mock.calls.find( + ([type]) => type === WEBVIEW_USER_LOGGED_IN + )!; + await listener({ + type: WEBVIEW_USER_LOGGED_IN, + payload: { url, userLoggedIn }, + } as any); + }; + + beforeEach(() => { + jest.clearAllMocks(); + handleUserLoggedOutDataClearing(); + }); + + it('clears webview storage on a logged-in -> logged-out transition', async () => { + const mockWebContents = createMockWebContents(); + mockGetWebContentsByServerUrl.mockReturnValue(mockWebContents); + + await dispatchUserLoggedIn(true); + await dispatchUserLoggedIn(false); + + expect(mockGetWebContentsByServerUrl).toHaveBeenCalledWith(url); + expect(mockWebContents.session.clearCache).toHaveBeenCalled(); + expect(mockWebContents.session.clearStorageData).toHaveBeenCalledWith(); + expect(mockWebContents.reloadIgnoringCache).toHaveBeenCalled(); + }); + + it('does not clear on the initial logged-out state at startup/attach', async () => { + const mockWebContents = createMockWebContents(); + mockGetWebContentsByServerUrl.mockReturnValue(mockWebContents); + + await dispatchUserLoggedIn(false); + + expect(mockGetWebContentsByServerUrl).not.toHaveBeenCalled(); + expect(mockWebContents.session.clearStorageData).not.toHaveBeenCalled(); + }); + + it('does not clear on a logged-out -> logged-in transition', async () => { + const mockWebContents = createMockWebContents(); + mockGetWebContentsByServerUrl.mockReturnValue(mockWebContents); + + await dispatchUserLoggedIn(false); + await dispatchUserLoggedIn(true); + + expect(mockGetWebContentsByServerUrl).not.toHaveBeenCalled(); + expect(mockWebContents.session.clearStorageData).not.toHaveBeenCalled(); + }); + + it('is a safe no-op when no webContents is found for the url', async () => { + mockGetWebContentsByServerUrl.mockReturnValue(undefined); + + await dispatchUserLoggedIn(true); + await expect(dispatchUserLoggedIn(false)).resolves.not.toThrow(); + + expect(mockGetWebContentsByServerUrl).toHaveBeenCalledWith(url); + }); +});