diff --git a/.eslintrc.json b/.eslintrc.json index c5f1f087..135ffa52 100644 --- a/.eslintrc.json +++ b/.eslintrc.json @@ -48,9 +48,9 @@ "position": "after" }, { - "pattern": "(app|common|main|renderer){,/**}", - "group": "external", - "position": "after" + "pattern": "@(app|common|main|renderer){,/**}", + "group": "internal", + "position": "before" }, { "pattern": "types{,/**}", diff --git a/api-types/index.ts b/api-types/index.ts index d96b710a..8fde7159 100644 --- a/api-types/index.ts +++ b/api-types/index.ts @@ -26,6 +26,8 @@ export type DesktopAPI = { idleTime: number, isSystemEvent: boolean, ) => void) => () => void; + onLogin: () => void; + onLogout: () => void; // Unreads/mentions/notifications sendNotification: (title: string, body: string, channelId: string, teamId: string, url: string, silent: boolean, soundName: string) => Promise<{result: string; reason?: string; data?: string}>; diff --git a/api-types/lib/index.d.ts b/api-types/lib/index.d.ts index f3e671b0..c42e0c9b 100644 --- a/api-types/lib/index.d.ts +++ b/api-types/lib/index.d.ts @@ -20,6 +20,8 @@ export type DesktopAPI = { reactAppInitialized: () => void; setSessionExpired: (isExpired: boolean) => void; onUserActivityUpdate: (listener: (userIsActive: boolean, idleTime: number, isSystemEvent: boolean) => void) => () => void; + onLogin: () => void; + onLogout: () => void; sendNotification: (title: string, body: string, channelId: string, teamId: string, url: string, silent: boolean, soundName: string) => Promise<{ result: string; reason?: string; diff --git a/api-types/package.json b/api-types/package.json index 4ea5101c..1344141a 100644 --- a/api-types/package.json +++ b/api-types/package.json @@ -1,6 +1,6 @@ { "name": "@mattermost/desktop-api", - "version": "5.8.0-3", + "version": "5.8.0-4", "description": "Shared types for the Desktop App API provided to the Web App", "keywords": [ "mattermost" @@ -28,6 +28,7 @@ "scripts": { "build": "tsc --build --verbose", "run": "tsc --watch --preserveWatchOutput", - "clean": "rm -rf tsconfig.tsbuildinfo ./lib" + "clean": "rm -rf tsconfig.tsbuildinfo ./lib", + "prepublish": "npm run build && rm -rf tsconfig.tsbuildinfo" } } diff --git a/e2e/specs/menu_bar/view_menu.test.js b/e2e/specs/menu_bar/view_menu.test.js index ecb60498..5afebf47 100644 --- a/e2e/specs/menu_bar/view_menu.test.js +++ b/e2e/specs/menu_bar/view_menu.test.js @@ -201,7 +201,7 @@ describe('menu/view', function desc() { }); }); - it('MM-T820 should open Developer Tools For Application Wrapper for main window', async () => { + it('MM-T820 should open Developer Tools For Application Wrapper for main window', async () => { const mainWindow = this.app.windows().find((window) => window.url().includes('index.html')); const browserWindow = await this.app.browserWindow(mainWindow); const loadingScreen = this.app.windows().find((window) => window.url().includes('loadingScreen')); diff --git a/e2e/utils/analyze-flaky-test.js b/e2e/utils/analyze-flaky-test.js index ff6ed24a..68bf7ca0 100644 --- a/e2e/utils/analyze-flaky-test.js +++ b/e2e/utils/analyze-flaky-test.js @@ -35,6 +35,7 @@ function analyzeFlakyTests() { return {commentBody, newFailedTests}; } catch (error) { console.error('Error analyzing failures:', error); + return {}; } } diff --git a/package-lock.json b/package-lock.json index b4611230..cca40b92 100644 --- a/package-lock.json +++ b/package-lock.json @@ -43,7 +43,7 @@ "@babel/preset-typescript": "7.23.3", "@electron/fuses": "1.6.0", "@electron/rebuild": "3.6.0", - "@mattermost/desktop-api": "*", + "@mattermost/desktop-api": "file:api-types", "@mattermost/eslint-plugin": "1.1.0-0", "@types/auto-launch": "5.0.5", "@types/jest": "29.4.1", @@ -100,6 +100,20 @@ "node": ">=18.0.0" } }, + "api-types": { + "name": "@mattermost/desktop-api", + "version": "5.8.0-4", + "dev": true, + "license": "MIT", + "peerDependencies": { + "typescript": "^4.3.0 || ^5.0.0" + }, + "peerDependenciesMeta": { + "typescript": { + "optional": true + } + } + }, "node_modules/@aashutoshrathi/word-wrap": { "version": "1.2.6", "resolved": "https://registry.npmjs.org/@aashutoshrathi/word-wrap/-/word-wrap-1.2.6.tgz", @@ -5038,18 +5052,8 @@ "integrity": "sha512-vQThJ4SAynnS2u94lQtZ9xANsStpVh8uTpsJascHJOWcavLuL2aDmMLgvg9EAx8Z1qRmTdP6hF5+IU5+9E9+Jg==" }, "node_modules/@mattermost/desktop-api": { - "version": "5.8.0-1", - "resolved": "https://registry.npmjs.org/@mattermost/desktop-api/-/desktop-api-5.8.0-1.tgz", - "integrity": "sha512-uS/vBY9ehaMolR2j7EAQSSZJl0TO5VHxDFraxy75QHpq9hD504lLlNKIjGUNgPeZSMWde+ah29xX2lBq12gt7A==", - "dev": true, - "peerDependencies": { - "typescript": "5.3.3" - }, - "peerDependenciesMeta": { - "typescript": { - "optional": true - } - } + "resolved": "api-types", + "link": true }, "node_modules/@mattermost/eslint-plugin": { "version": "1.1.0-0", @@ -9913,17 +9917,14 @@ } }, "node_modules/electron-dl": { - "version": "3.5.2", - "resolved": "https://registry.npmjs.org/electron-dl/-/electron-dl-3.5.2.tgz", - "integrity": "sha512-i104cl+u8yJ0lhpRAtUWfeGuWuL1PL6TBiw2gLf0MMIBjfgE485Ags2mcySx4uWU9P9uj/vsD3jd7X+w1lzZxw==", + "version": "3.5.0", + "resolved": "https://registry.npmjs.org/electron-dl/-/electron-dl-3.5.0.tgz", + "integrity": "sha512-Oj+VSuScVx8hEKM2HEvTQswTX6G3MLh7UoAz/oZuvKyNDfudNi1zY6PK/UnFoK1nCl9DF6k+3PFwElKbtZlDig==", "dependencies": { "ext-name": "^5.0.0", "pupa": "^2.0.1", "unused-filename": "^2.1.0" }, - "engines": { - "node": ">=12" - }, "funding": { "url": "https://github.com/sponsors/sindresorhus" } diff --git a/package.json b/package.json index 201d7d4f..f69be94e 100644 --- a/package.json +++ b/package.json @@ -135,7 +135,7 @@ "@babel/preset-typescript": "7.23.3", "@electron/fuses": "1.6.0", "@electron/rebuild": "3.6.0", - "@mattermost/desktop-api": "*", + "@mattermost/desktop-api": "file:api-types", "@mattermost/eslint-plugin": "1.1.0-0", "@types/auto-launch": "5.0.5", "@types/jest": "29.4.1", diff --git a/src/app/serverViewState.test.js b/src/app/serverViewState.test.js index 23c72d0e..b4989315 100644 --- a/src/app/serverViewState.test.js +++ b/src/app/serverViewState.test.js @@ -417,6 +417,26 @@ describe('app/serverViewState', () => { expect(result.validatedURL).toBe('http://server.com/'); }); + it('should be able to recognize localhost with a port and add the appropriate prefix', async () => { + ServerInfo.mockImplementation(({url}) => ({ + fetchConfigData: jest.fn().mockImplementation(() => { + if (url.startsWith('https:')) { + return undefined; + } + + return { + serverVersion: '7.8.0', + siteName: 'Mattermost', + siteURL: url, + }; + }), + })); + + const result = await serverViewState.handleServerURLValidation({}, 'localhost:8065'); + expect(result.status).toBe(URLValidationStatus.Insecure); + expect(result.validatedURL).toBe('http://localhost:8065/'); + }); + it('should show a warning when the ping request times out', async () => { ServerInfo.mockImplementation(() => ({ fetchConfigData: jest.fn().mockImplementation(() => { diff --git a/src/app/serverViewState.ts b/src/app/serverViewState.ts index c6d1c985..1817e683 100644 --- a/src/app/serverViewState.ts +++ b/src/app/serverViewState.ts @@ -233,7 +233,7 @@ export class ServerViewState { if (!isValidURL(url)) { // If it already includes the protocol, tell them it's invalid if (isValidURI(url)) { - httpUrl = url.replace(/^(.+):/, 'https:'); + httpUrl = url.replace(/^((.+):\/\/)?/, 'https://'); } else { // Otherwise add HTTPS for them httpUrl = `https://${url}`; @@ -260,11 +260,13 @@ export class ServerViewState { // Try and get remote info from the most secure URL, otherwise use the insecure one let remoteURL = secureURL; + const insecureURL = parseURL(secureURL.toString().replace(/^https:/, 'http:')); let remoteInfo = await this.testRemoteServer(secureURL); - if (!remoteInfo) { - if (secureURL.toString() !== parsedURL.toString()) { - remoteURL = parsedURL; - remoteInfo = await this.testRemoteServer(parsedURL); + if (!remoteInfo && insecureURL) { + // Try to fall back to HTTP + remoteInfo = await this.testRemoteServer(insecureURL); + if (remoteInfo) { + remoteURL = insecureURL; } } @@ -274,9 +276,11 @@ export class ServerViewState { return {status: URLValidationStatus.NotMattermost, validatedURL: parsedURL.toString()}; } + const remoteServerName = remoteInfo.siteName === 'Mattermost' ? remoteURL.host.split('.')[0] : remoteInfo.siteName; + // If we were only able to connect via HTTP, warn the user that the connection is not secure if (remoteURL.protocol === 'http:') { - return {status: URLValidationStatus.Insecure, serverVersion: remoteInfo.serverVersion, validatedURL: remoteURL.toString()}; + return {status: URLValidationStatus.Insecure, serverVersion: remoteInfo.serverVersion, serverName: remoteServerName, validatedURL: remoteURL.toString()}; } // If the URL doesn't match the Site URL, set the URL to the correct one @@ -292,15 +296,15 @@ export class ServerViewState { // If we can't reach the remote Site URL, there's probably a configuration issue const remoteSiteURLInfo = await this.testRemoteServer(parsedSiteURL); if (!remoteSiteURLInfo) { - return {status: URLValidationStatus.URLNotMatched, serverVersion: remoteInfo.serverVersion, serverName: remoteInfo.siteName, validatedURL: remoteURL.toString()}; + return {status: URLValidationStatus.URLNotMatched, serverVersion: remoteInfo.serverVersion, serverName: remoteServerName, validatedURL: remoteURL.toString()}; } } // Otherwise fix it for them and return - return {status: URLValidationStatus.URLUpdated, serverVersion: remoteInfo.serverVersion, serverName: remoteInfo.siteName, validatedURL: remoteInfo.siteURL}; + return {status: URLValidationStatus.URLUpdated, serverVersion: remoteInfo.serverVersion, serverName: remoteServerName, validatedURL: remoteInfo.siteURL}; } - return {status: URLValidationStatus.OK, serverVersion: remoteInfo.serverVersion, serverName: remoteInfo.siteName, validatedURL: remoteURL.toString()}; + return {status: URLValidationStatus.OK, serverVersion: remoteInfo.serverVersion, serverName: remoteServerName, validatedURL: remoteURL.toString()}; }; private handleCloseView = (event: IpcMainEvent, viewId: string) => { diff --git a/src/common/appState.ts b/src/common/appState.ts index d58740ef..6ce66a46 100644 --- a/src/common/appState.ts +++ b/src/common/appState.ts @@ -49,6 +49,8 @@ export class AppState extends EventEmitter { this.expired.delete(viewId); this.mentions.delete(viewId); this.unreads.delete(viewId); + + this.emitStatusForView(viewId); }; emitStatus = () => { diff --git a/src/main/app/initialize.ts b/src/main/app/initialize.ts index 96716604..45fb7fa7 100644 --- a/src/main/app/initialize.ts +++ b/src/main/app/initialize.ts @@ -87,9 +87,9 @@ import { shouldShowTrayIcon, updateSpellCheckerLocales, wasUpdated, - initCookieManager, migrateMacAppStore, updateServerInfos, + flushCookiesStore, } from './utils'; import { handleClose, @@ -200,6 +200,12 @@ function initializeAppEventListeners() { app.on('child-process-gone', handleChildProcessGone); app.on('login', AuthManager.handleAppLogin); app.on('will-finish-launching', handleAppWillFinishLaunching); + + // Somehow cookies are not immediately saved to disk. + // So manually flush cookie store to disk on closing the app. + // https://github.com/electron/electron/issues/8416 + // TODO: We can remove this once every server supported will flush on login/logout + app.on('before-quit', flushCookiesStore); } function initializeBeforeAppReady() { @@ -347,7 +353,6 @@ async function initializeAfterAppReady() { catch((err) => log.error('An error occurred: ', err)); } - initCookieManager(defaultSession); MainWindow.show(); let deeplinkingURL; diff --git a/src/main/app/utils.ts b/src/main/app/utils.ts index f655bd02..12200e68 100644 --- a/src/main/app/utils.ts +++ b/src/main/app/utils.ts @@ -4,7 +4,7 @@ import fs from 'fs'; import path from 'path'; -import type {BrowserWindow, Rectangle, Session} from 'electron'; +import type {BrowserWindow, Rectangle} from 'electron'; import {app, Menu, session, dialog, nativeImage, screen} from 'electron'; import isDev from 'electron-is-dev'; @@ -166,26 +166,13 @@ export function resizeScreen(browserWindow: BrowserWindow) { handle(); } -export function flushCookiesStore(session: Session) { +export function flushCookiesStore() { log.debug('flushCookiesStore'); - session.cookies.flushStore().catch((err) => { + session.defaultSession.cookies.flushStore().catch((err) => { log.error(`There was a problem flushing cookies:\n${err}`); }); } -export function initCookieManager(session: Session) { - // Somehow cookies are not immediately saved to disk. - // So manually flush cookie store to disk on closing the app. - // https://github.com/electron/electron/issues/8416 - app.on('before-quit', () => { - flushCookiesStore(session); - }); - - app.on('browser-window-blur', () => { - flushCookiesStore(session); - }); -} - export function migrateMacAppStore() { const migrationPrefs = new JsonFileManager(migrationInfoPath); const oldPath = path.join(app.getPath('userData'), '../../../../../../../Library/Application Support/Mattermost'); diff --git a/src/main/preload/externalAPI.ts b/src/main/preload/externalAPI.ts index 5d7e69de..e529c311 100644 --- a/src/main/preload/externalAPI.ts +++ b/src/main/preload/externalAPI.ts @@ -70,6 +70,9 @@ const desktopAPI: DesktopAPI = { setSessionExpired: (isExpired) => ipcRenderer.send(SESSION_EXPIRED, isExpired), onUserActivityUpdate: (listener) => createListener(USER_ACTIVITY_UPDATE, listener), + onLogin: () => ipcRenderer.send(APP_LOGGED_IN), + onLogout: () => ipcRenderer.send(APP_LOGGED_OUT), + // Unreads/mentions/notifications sendNotification: (title, body, channelId, teamId, url, silent, soundName) => ipcRenderer.invoke(NOTIFY_MENTION, title, body, channelId, teamId, url, silent, soundName), @@ -178,11 +181,11 @@ setInterval(() => { const onLoad = () => { if (document.getElementById('root') === null) { - console.log('The guest is not assumed as mattermost-webapp'); + console.warn('The guest is not assumed as mattermost-webapp'); return; } watchReactAppUntilInitialized(() => { - console.log('Legacy preload initialized'); + console.warn('Legacy preload initialized'); ipcRenderer.send(REACT_APP_INITIALIZED); ipcRenderer.invoke(REQUEST_BROWSER_HISTORY_STATUS).then(sendHistoryButtonReturn); }); diff --git a/src/main/views/viewManager.test.js b/src/main/views/viewManager.test.js index f220f542..c83788ae 100644 --- a/src/main/views/viewManager.test.js +++ b/src/main/views/viewManager.test.js @@ -56,6 +56,10 @@ jest.mock('common/utils/url', () => ({ equalUrlsIgnoringSubpath: jest.fn(), })); +jest.mock('main/app/utils', () => ({ + flushCookiesStore: jest.fn(), +})); + jest.mock('main/i18nManager', () => ({ localizeMessage: jest.fn(), })); diff --git a/src/main/views/viewManager.ts b/src/main/views/viewManager.ts index 818045e1..2fc5e387 100644 --- a/src/main/views/viewManager.ts +++ b/src/main/views/viewManager.ts @@ -43,6 +43,7 @@ import {getFormattedPathName, parseURL} from 'common/utils/url'; import Utils from 'common/utils/util'; import type {MattermostView} from 'common/views/View'; import {TAB_MESSAGING} from 'common/views/View'; +import {flushCookiesStore} from 'main/app/utils'; import {localizeMessage} from 'main/i18nManager'; import MainWindow from 'main/windows/mainWindow'; @@ -471,11 +472,24 @@ export class ViewManager { }; private handleAppLoggedIn = (event: IpcMainEvent) => { - this.getViewByWebContentsId(event.sender.id)?.onLogin(true); + log.debug('handleAppLoggedIn', event.sender.id); + const view = this.getViewByWebContentsId(event.sender.id); + if (!view) { + return; + } + view.onLogin(true); + flushCookiesStore(); }; private handleAppLoggedOut = (event: IpcMainEvent) => { - this.getViewByWebContentsId(event.sender.id)?.onLogin(false); + log.debug('handleAppLoggedOut', event.sender.id); + const view = this.getViewByWebContentsId(event.sender.id); + if (!view) { + return; + } + view.onLogin(false); + AppState.clear(view.id); + flushCookiesStore(); }; private handleBrowserHistoryPush = (e: IpcMainEvent, pathName: string) => { diff --git a/src/main/views/webContentEvents.test.js b/src/main/views/webContentEvents.test.js index c139189e..7f4ae741 100644 --- a/src/main/views/webContentEvents.test.js +++ b/src/main/views/webContentEvents.test.js @@ -245,8 +245,7 @@ describe('main/views/webContentsEvents', () => { const logObject = { error: jest.fn(), warn: jest.fn(), - info: jest.fn(), - verbose: jest.fn(), + debug: jest.fn(), withPrefix: jest.fn().mockReturnThis(), }; webContentsEventManager.log = jest.fn().mockReturnValue(logObject); @@ -258,10 +257,10 @@ describe('main/views/webContentsEvents', () => { it('should respect logging levels', () => { consoleMessage({}, 0, 'test0', 0, ''); - expect(logObject.verbose).toHaveBeenCalledWith('test0'); + expect(logObject.debug).toHaveBeenCalledWith('test0'); consoleMessage({}, 1, 'test1', 0, ''); - expect(logObject.info).toHaveBeenCalledWith('test1'); + expect(logObject.debug).toHaveBeenCalledWith('test1'); consoleMessage({}, 2, 'test2', 0, ''); expect(logObject.warn).toHaveBeenCalledWith('test2'); @@ -273,11 +272,11 @@ describe('main/views/webContentsEvents', () => { it('should only add line numbers for debug and silly', () => { getLevel.mockReturnValue('debug'); consoleMessage({}, 0, 'test1', 42, 'meaning_of_life.js'); - expect(logObject.verbose).toHaveBeenCalledWith('test1', '(meaning_of_life.js:42)'); + expect(logObject.debug).toHaveBeenCalledWith('test1', '(meaning_of_life.js:42)'); - getLevel.mockReturnValue('info'); + getLevel.mockReturnValue('warn'); consoleMessage({}, 0, 'test2', 42, 'meaning_of_life.js'); - expect(logObject.verbose).not.toHaveBeenCalledWith('test2', '(meaning_of_life.js:42)'); + expect(logObject.warn).not.toHaveBeenCalledWith('test2', '(meaning_of_life.js:42)'); }); }); }); diff --git a/src/main/views/webContentEvents.ts b/src/main/views/webContentEvents.ts index 4915c909..ab9830d1 100644 --- a/src/main/views/webContentEvents.ts +++ b/src/main/views/webContentEvents.ts @@ -4,7 +4,7 @@ import path from 'path'; import type {WebContents, Event} from 'electron'; -import {BrowserWindow, session, shell} from 'electron'; +import {BrowserWindow, shell} from 'electron'; import Config from 'common/config'; import {Logger, getLevel} from 'common/log'; @@ -116,7 +116,7 @@ export class WebContentsEventManager { return; } if (this.customLogins[webContentsId]?.inProgress) { - flushCookiesStore(session.defaultSession); + flushCookiesStore(); return; } @@ -298,7 +298,7 @@ export class WebContentsEventManager { private generateHandleConsoleMessage = (webContentsId: number) => (_: Event, level: number, message: string, line: number, sourceId: string) => { const wcLog = this.log(webContentsId).withPrefix('renderer'); - let logFn = wcLog.verbose; + let logFn = wcLog.debug; switch (level) { case ConsoleMessageLevel.Error: logFn = wcLog.error; @@ -306,9 +306,6 @@ export class WebContentsEventManager { case ConsoleMessageLevel.Warning: logFn = wcLog.warn; break; - case ConsoleMessageLevel.Info: - logFn = wcLog.info; - break; } // Only include line entries if we're debugging diff --git a/src/renderer/components/MainPage.tsx b/src/renderer/components/MainPage.tsx index 21cdbca2..2bafaa70 100644 --- a/src/renderer/components/MainPage.tsx +++ b/src/renderer/components/MainPage.tsx @@ -162,7 +162,7 @@ class MainPage extends React.PureComponent { // set page on retry window.desktop.onLoadRetry((viewId, retry, err, loadUrl) => { - console.log(`${viewId}: failed to load ${err}, but retrying`); + console.error(`${viewId}: failed to load ${err}, but retrying`); const statusValue = { status: Status.RETRY, extra: { @@ -179,7 +179,7 @@ class MainPage extends React.PureComponent { }); window.desktop.onLoadFailed((viewId, err, loadUrl) => { - console.log(`${viewId}: failed to load ${err}`); + console.error(`${viewId}: failed to load ${err}`); const statusValue = { status: Status.FAILED, extra: { diff --git a/src/renderer/index.tsx b/src/renderer/index.tsx index 6a1ef65b..5ca646af 100644 --- a/src/renderer/index.tsx +++ b/src/renderer/index.tsx @@ -56,7 +56,7 @@ class Root extends React.PureComponent, State> { const configRequest = await window.desktop.getConfiguration() as CombinedConfig; return configRequest; } catch (err: any) { - console.log(`there was an error with the config: ${err}`); + console.error(`there was an error with the config: ${err}`); if (exitOnError) { window.desktop.quit(`unable to load configuration: ${err}`, err.stack); } diff --git a/webpack.config.base.js b/webpack.config.base.js index 8af456db..5ffa01ef 100644 --- a/webpack.config.base.js +++ b/webpack.config.base.js @@ -7,7 +7,7 @@ const path = require('path'); const webpack = require('webpack'); -const VERSION = childProcess.execSync('git rev-parse --short HEAD').toString(); +const VERSION = childProcess.execSync('git rev-parse --short HEAD', {cwd: __dirname}).toString(); const isProduction = process.env.NODE_ENV === 'production'; const isTest = process.env.NODE_ENV === 'test'; const isRelease = process.env.GITHUB_WORKFLOW && process.env.GITHUB_WORKFLOW.startsWith('release');