From b76884dcfcdf4f3c7bbda35ab082e800521a01b1 Mon Sep 17 00:00:00 2001 From: Kiran Niranjan Date: Wed, 20 Nov 2024 09:09:33 +0100 Subject: [PATCH] SDA-4714 - Use plist parser to validate if a field exists (#2227) * SDA-4714 - Use plist parser to validate if a field exists * SDA-4714 - Fix uts --- package-lock.json | 46 +++++++++++++++++++++++++++----- package.json | 1 + spec/appMenu.spec.ts | 4 +-- spec/plistHandler.spec.ts | 39 +++++++++++++++++++++++++++ src/app/config-handler.ts | 7 ++++- src/app/plist-handler.ts | 55 +++++++++++++++++++++++++++++++++++++-- 6 files changed, 140 insertions(+), 12 deletions(-) diff --git a/package-lock.json b/package-lock.json index 4624b12e1..92737d3ba 100644 --- a/package-lock.json +++ b/package-lock.json @@ -12,6 +12,7 @@ "dependencies": { "@types/lazy-brush": "^1.0.0", "adm-zip": "^0.5.10", + "bplist-parser": "^0.3.2", "classnames": "2.2.6", "electron-dl": "3.5.0", "electron-fetch": "1.9.1", @@ -2950,6 +2951,15 @@ "@types/node": "*" } }, + "node_modules/@xmldom/xmldom": { + "version": "0.8.10", + "resolved": "https://repo.symphony.com/artifactory/api/npm/npm-virtual-dev/@xmldom/xmldom/-/xmldom-0.8.10.tgz", + "integrity": "sha512-2WALfTl4xo2SkGCYRt6rDTFfk9R1czmBvUQy12gK2KuRKIpWEhcbbzy8EZXtz/jkRqHX8bFEc6FC1HjX4TUWYw==", + "dev": true, + "engines": { + "node": ">=10.0.0" + } + }, "node_modules/7zip-bin": { "version": "5.2.0", "resolved": "https://repo.symphony.com/artifactory/api/npm/npm-virtual-dev/7zip-bin/-/7zip-bin-5.2.0.tgz", @@ -4135,6 +4145,14 @@ "tweetnacl": "^0.14.3" } }, + "node_modules/big-integer": { + "version": "1.6.52", + "resolved": "https://repo.symphony.com/artifactory/api/npm/npm-virtual-dev/big-integer/-/big-integer-1.6.52.tgz", + "integrity": "sha512-QxD8cf2eVqJOOz63z6JIN9BzvVs/dlySa5HGSBH5xtR8dPteIRQnBxxKqkNTiT6jbDTF6jAfrd4oMcND9RGbQg==", + "engines": { + "node": ">=0.6" + } + }, "node_modules/binaryextensions": { "version": "2.3.0", "dev": true, @@ -4194,6 +4212,17 @@ "license": "MIT", "optional": true }, + "node_modules/bplist-parser": { + "version": "0.3.2", + "resolved": "https://repo.symphony.com/artifactory/api/npm/npm-virtual-dev/bplist-parser/-/bplist-parser-0.3.2.tgz", + "integrity": "sha512-apC2+fspHGI3mMKj+dGevkGo/tCqVB8jMb6i+OX+E29p0Iposz07fABkRIfVUPNd5A5VbuOz1bZbnmkKLYF+wQ==", + "dependencies": { + "big-integer": "1.6.x" + }, + "engines": { + "node": ">= 5.10.0" + } + }, "node_modules/brace-expansion": { "version": "1.1.11", "dev": true, @@ -13599,23 +13628,26 @@ } }, "node_modules/plist": { - "version": "3.0.5", + "version": "3.1.0", + "resolved": "https://repo.symphony.com/artifactory/api/npm/npm-virtual-dev/plist/-/plist-3.1.0.tgz", + "integrity": "sha512-uysumyrvkUX0rX/dEVqt8gC3sTBzd4zoWfLeS29nb53imdaXVvLINYXTI2GNqzaMuvacNx4uJQ8+b3zXR0pkgQ==", "dev": true, - "license": "MIT", "dependencies": { + "@xmldom/xmldom": "^0.8.8", "base64-js": "^1.5.1", - "xmlbuilder": "^9.0.7" + "xmlbuilder": "^15.1.1" }, "engines": { - "node": ">=6" + "node": ">=10.4.0" } }, "node_modules/plist/node_modules/xmlbuilder": { - "version": "9.0.7", + "version": "15.1.1", + "resolved": "https://repo.symphony.com/artifactory/api/npm/npm-virtual-dev/xmlbuilder/-/xmlbuilder-15.1.1.tgz", + "integrity": "sha512-yMqGBqtXyeN1e3TGYvgNgDVZ3j84W4cwkOXQswghol6APgZWaff9lnbvN7MHYJOiXsvGPXtjTYJEiC9J2wv9Eg==", "dev": true, - "license": "MIT", "engines": { - "node": ">=4.0" + "node": ">=8.0" } }, "node_modules/plugin-error": { diff --git a/package.json b/package.json index 66839beb6..46da27de4 100644 --- a/package.json +++ b/package.json @@ -218,6 +218,7 @@ "dependencies": { "@types/lazy-brush": "^1.0.0", "adm-zip": "^0.5.10", + "bplist-parser": "^0.3.2", "classnames": "2.2.6", "electron-dl": "3.5.0", "electron-fetch": "1.9.1", diff --git a/spec/appMenu.spec.ts b/spec/appMenu.spec.ts index eaa391bf5..393a39784 100644 --- a/spec/appMenu.spec.ts +++ b/spec/appMenu.spec.ts @@ -3,12 +3,12 @@ import { autoLaunchInstance } from '../src/app/auto-launch-controller'; import { config } from '../src/app/config-handler'; import { exportCrashDumps, exportLogs } from '../src/app/reports-handler'; import { updateAlwaysOnTop } from '../src/app/window-actions'; +import { windowHandler } from '../src/app/window-handler'; import { zoomIn, zoomOut } from '../src/app/window-utils'; +import { apiName } from '../src/common/api-interface'; import * as envMock from '../src/common/env'; import { logger } from '../src/common/logger'; import { BrowserWindow, dialog, session, shell } from './__mocks__/electron'; -import { windowHandler } from '../src/app/window-handler'; -import { apiName } from '../src/common/api-interface'; jest.mock('../src/app/stores', () => { const mock = new Map(); diff --git a/spec/plistHandler.spec.ts b/spec/plistHandler.spec.ts index d6d966ae9..6d7f1427b 100644 --- a/spec/plistHandler.spec.ts +++ b/spec/plistHandler.spec.ts @@ -1,5 +1,44 @@ import { getAllUserDefaults } from '../src/app/plist-handler'; +jest.mock('../src/app/config-handler', () => { + return { + ConfigFieldsDefaultValues: {}, + CloudConfigDataTypes: { + NOT_SET: 'NOT_SET', + ENABLED: 'ENABLED', + DISABLED: 'DISABLED', + }, + config: { + getConfigFields: jest.fn(() => { + return { + minimizeOnClose: 'ENABLED', + launchOnStartup: 'ENABLED', + alwaysOnTop: 'ENABLED', + isAlwaysOnTop: 'ENABLED', + bringToFront: 'ENABLED', + devToolsEnabled: true, + }; + }), + getGlobalConfigFields: jest.fn(() => { + return { + devToolsEnabled: true, + }; + }), + getFilteredCloudConfigFields: jest.fn(() => { + return { + devToolsEnabled: true, + }; + }), + getCloudConfigFields: jest.fn(() => { + return { + devToolsEnabled: true, + }; + }), + updateUserConfig: jest.fn(), + }, + }; +}); + describe('Plist Handler', () => { it('should return config object', () => { expect(getAllUserDefaults()).toStrictEqual({ diff --git a/src/app/config-handler.ts b/src/app/config-handler.ts index 3d93fd088..256a847cb 100644 --- a/src/app/config-handler.ts +++ b/src/app/config-handler.ts @@ -19,6 +19,7 @@ import { terminateC9Shell } from './c9-shell-handler'; import { getAllUserDefaults, initializePlistFile, + readPlistFile, setPlistFromPreviousSettings, } from './plist-handler'; import { appStats } from './stats'; @@ -840,7 +841,7 @@ class Config { /** * Reads a stores the global config file */ - private readGlobalConfig() { + private async readGlobalConfig() { if (isMac) { if (fs.existsSync(this.tempGlobalConfigFilePath)) { this.globalConfig = this.parseConfigData( @@ -862,6 +863,8 @@ class Config { this.globalConfig as IConfig, appGlobalConfigData, ); + // Validate user config before starting the application + await readPlistFile(); // After everything is set from previous SDA version this.globalConfig = getAllUserDefaults(); return; @@ -877,6 +880,8 @@ class Config { 'installVariant', 'string', ); + // Validate user config before starting the application + await readPlistFile(); this.globalConfig = getAllUserDefaults(); logger.info( `config-handler: Global configuration from plist: `, diff --git a/src/app/plist-handler.ts b/src/app/plist-handler.ts index ba60796fa..0e62d3f11 100644 --- a/src/app/plist-handler.ts +++ b/src/app/plist-handler.ts @@ -1,8 +1,13 @@ +import * as bplistParser from 'bplist-parser'; import { execSync } from 'child_process'; -import { systemPreferences } from 'electron'; +import { app, systemPreferences } from 'electron'; +import * as fs from 'fs'; + import { logger } from '../common/logger'; import { getGuid } from '../common/utils'; -import { IConfig } from './config-handler'; +import { ConfigFieldsDefaultValues, IConfig } from './config-handler'; + +let plistData = {}; const GENERAL_SETTINGS = { url: 'string', @@ -62,6 +67,18 @@ export const getAllUserDefaults = (): IConfig => { const settings: any = {}; Object.keys(GENERAL_SETTINGS).map((key) => { + // Validate plist file only for keys that exist in ConfigFieldsDefaultValues + if (ConfigFieldsDefaultValues.hasOwnProperty(key)) { + // If plistData has entries but the key is missing, set it to the default value + if (Object.keys(plistData).length && !hasField(key)) { + logger.info( + 'plist-handler: field does not exists in plist file using default value', + key, + ); + settings[key] = ConfigFieldsDefaultValues[key]; + return; + } + } settings[key] = systemPreferences.getUserDefault( key, GENERAL_SETTINGS[key], @@ -158,3 +175,37 @@ export const initializePlistFile = (path: string) => { logger.error('plist-handler: initialize exception', error?.message); } }; + +/** + * Reads and parses a plist file from the user's home directory. + * The plist file is located at `~/Library/Preferences/com.symphony.electron-desktop.plist`. + * If the file exists and is successfully parsed, its data is stored in the `plistData` variable. + * + * @returns {Promise} A promise that resolves once the file has been read and processed. + * @throws {Error} Throws an error if the plist file cannot be read or parsed, which is logged using the logger. + */ +export const readPlistFile = async () => { + const userPath = app.getPath('home'); + const plistPath = `${userPath}/Library/Preferences/com.symphony.electron-desktop.plist`; + try { + if (fs.existsSync(plistPath)) { + const data = fs.readFileSync(plistPath); + const parsedData = bplistParser.parseBuffer(data); + if (parsedData && parsedData.length > 0) { + plistData = parsedData[0]; + } + } + } catch (error) { + logger.error('plist-handler: failed to read plist', error); + } +}; + +/** + * Checks if a given field name exists in the `plistData` object. + * + * @param {string} fieldName - The name of the field to check for in the `plistData` object. + * @returns {boolean} Returns `true` if the field exists, `false` otherwise. + */ +export const hasField = (fieldName: string): boolean => { + return plistData.hasOwnProperty(fieldName); +};