Merge branch 'develop' into feat/matrix-wysisyg-integration

2022-10-11 10:47:01 +02:00 · 2022-10-11 10:47:01 +02:00 · 203f75f755
parent 0dd9aba47f 4a98e26c4a
commit 203f75f755
5 changed files with 97 additions and 43 deletions
--- a/src/DeviceListener.ts
+++ b/src/DeviceListener.ts
@ -42,7 +42,10 @@ import { Action } from "./dispatcher/actions";
 import { isLoggedIn } from "./utils/login";
 import SdkConfig from "./SdkConfig";
 import PlatformPeg from "./PlatformPeg";
-import { recordClientInformation } from "./utils/device/clientInformation";
+import {
+    recordClientInformation,
+    removeClientInformation,
+} from "./utils/device/clientInformation";
 import SettingsStore, { CallbackFn } from "./settings/SettingsStore";

 const KEY_BACKUP_POLL_INTERVAL = 5 * 60 * 1000;
@ -90,7 +93,7 @@ export default class DeviceListener {
        );
        this.dispatcherRef = dis.register(this.onAction);
        this.recheck();
-        this.recordClientInformation();
+        this.updateClientInformation();
    }

    public stop() {
@ -216,7 +219,7 @@ export default class DeviceListener {
    private onAction = ({ action }: ActionPayload) => {
        if (action !== Action.OnLoggedIn) return;
        this.recheck();
-        this.recordClientInformation();
+        this.updateClientInformation();
    };

    // The server doesn't tell us when key backup is set up, so we poll
@ -368,25 +371,26 @@ export default class DeviceListener {

        this.shouldRecordClientInformation = !!newValue;

-        if (this.shouldRecordClientInformation && !prevValue) {
-            this.recordClientInformation();
+        if (this.shouldRecordClientInformation !== prevValue) {
+            this.updateClientInformation();
        }
    };

-    private recordClientInformation = async () => {
-        if (!this.shouldRecordClientInformation) {
-            return;
-        }
+    private updateClientInformation = async () => {
        try {
+            if (this.shouldRecordClientInformation) {
                await recordClientInformation(
                    MatrixClientPeg.get(),
                    SdkConfig.get(),
                    PlatformPeg.get(),
                );
+            } else {
+                await removeClientInformation(MatrixClientPeg.get());
+            }
        } catch (error) {
            // this is a best effort operation
            // log the error without rethrowing
-            logger.error('Failed to record client information', error);
+            logger.error('Failed to update client information', error);
        }
    };
 }
--- a/src/utils/device/clientInformation.ts
+++ b/src/utils/device/clientInformation.ts
@ -65,6 +65,24 @@ export const recordClientInformation = async (
    });
 };

+/**
+ * Remove extra client information
+ * @todo(kerrya) revisit after MSC3391: account data deletion is done
+ * (PSBE-12)
+ */
+export const removeClientInformation = async (
+    matrixClient: MatrixClient,
+): Promise<void> => {
+    const deviceId = matrixClient.getDeviceId();
+    const type = getClientInformationEventType(deviceId);
+    const clientInformation = getDeviceClientInformation(matrixClient, deviceId);
+
+    // if a non-empty client info event exists, overwrite to remove the content
+    if (clientInformation.name || clientInformation.version || clientInformation.url) {
+        await matrixClient.setAccountData(type, {});
+    }
+};
+
 const sanitizeContentString = (value: unknown): string | undefined =>
    value && typeof value === 'string' ? value : undefined;

--- a/src/utils/device/parseUserAgent.ts
+++ b/src/utils/device/parseUserAgent.ts
@ -58,15 +58,16 @@ const getDeviceType = (
    return DeviceType.Unknown;
 };

+interface CustomValues {
+    customDeviceModel?: string;
+    customDeviceOS?: string;
+}
 /**
 * Some mobile model and OS strings are not recognised
 * by the UA parsing library
 * check they exist by hand
 */
-const checkForCustomValues = (userAgent: string): {
-    customDeviceModel?: string;
-    customDeviceOS?: string;
-} => {
+const checkForCustomValues = (userAgent: string): CustomValues => {
    if (userAgent.includes(BROWSER_KEYWORD)) {
        return {};
    }
@ -97,13 +98,23 @@ export const parseUserAgent = (userAgent?: string): ExtendedDeviceInformation =>
    const device = parser.getDevice();
    const operatingSystem = parser.getOS();

-    const deviceOperatingSystem = concatenateNameAndVersion(operatingSystem.name, operatingSystem.version);
-    const deviceModel = concatenateNameAndVersion(device.vendor, device.model);
-    const client = concatenateNameAndVersion(browser.name, browser.major || browser.version);
-
-    const { customDeviceModel, customDeviceOS } = checkForCustomValues(userAgent);
    const deviceType = getDeviceType(userAgent, device, browser, operatingSystem);

+    // OSX versions are frozen at 10.15.17 in UA strings https://chromestatus.com/feature/5452592194781184
+    // ignore OS version in browser based sessions
+    const shouldIgnoreOSVersion = deviceType === DeviceType.Web || deviceType === DeviceType.Desktop;
+    const deviceOperatingSystem = concatenateNameAndVersion(
+        operatingSystem.name,
+        shouldIgnoreOSVersion ? undefined : operatingSystem.version,
+    );
+    const deviceModel = concatenateNameAndVersion(device.vendor, device.model);
+    const client = concatenateNameAndVersion(browser.name, browser.version);
+
+    // only try to parse custom model and OS when device type is known
+    const { customDeviceModel, customDeviceOS } = deviceType !== DeviceType.Unknown
+        ? checkForCustomValues(userAgent)
+        : {} as CustomValues;
+
    return {
        deviceType,
        deviceModel: deviceModel || customDeviceModel,
--- a/test/DeviceListener-test.ts
+++ b/test/DeviceListener-test.ts
@ -17,7 +17,7 @@ limitations under the License.

 import { EventEmitter } from "events";
 import { mocked } from "jest-mock";
-import { Room } from "matrix-js-sdk/src/matrix";
+import { MatrixEvent, Room } from "matrix-js-sdk/src/matrix";
 import { logger } from "matrix-js-sdk/src/logger";

 import DeviceListener from "../src/DeviceListener";
@ -66,6 +66,7 @@ class MockClient extends EventEmitter {
    getClientWellKnown = jest.fn();
    getDeviceId = jest.fn().mockReturnValue(deviceId);
    setAccountData = jest.fn();
+    getAccountData = jest.fn();
 }
 const mockDispatcher = mocked(dis);
 const flushPromises = async () => await new Promise(process.nextTick);
@ -138,7 +139,7 @@ describe('DeviceListener', () => {
                await createAndStart();

                expect(errorLogSpy).toHaveBeenCalledWith(
-                    'Failed to record client information',
+                    'Failed to update client information',
                    error,
                );
            });
@ -161,19 +162,39 @@ describe('DeviceListener', () => {
        });

        describe('when device client information feature is disabled', () => {
+            const clientInfoEvent = new MatrixEvent({ type: `io.element.matrix_client_information.${deviceId}`,
+                content: { name: 'hello' },
+            });
+            const emptyClientInfoEvent = new MatrixEvent({ type: `io.element.matrix_client_information.${deviceId}` });
            beforeEach(() => {
                jest.spyOn(SettingsStore, 'getValue').mockReturnValue(false);
+
+                mockClient.getAccountData.mockReturnValue(undefined);
            });

            it('does not save client information on start', async () => {
                await createAndStart();

-                expect(mockClient.setAccountData).not.toHaveBeenCalledWith(
+                expect(mockClient.setAccountData).not.toHaveBeenCalled();
+            });
+
+            it('removes client information on start if it exists', async () => {
+                mockClient.getAccountData.mockReturnValue(clientInfoEvent);
+                await createAndStart();
+
+                expect(mockClient.setAccountData).toHaveBeenCalledWith(
                    `io.element.matrix_client_information.${deviceId}`,
-                    { name: 'Element', url: 'localhost', version: '1.2.3' },
+                    {},
                );
            });

+            it('does not try to remove client info event that are already empty', async () => {
+                mockClient.getAccountData.mockReturnValue(emptyClientInfoEvent);
+                await createAndStart();
+
+                expect(mockClient.setAccountData).not.toHaveBeenCalled();
+            });
+
            it('does not save client information on logged in action', async () => {
                const instance = await createAndStart();

@ -182,10 +203,7 @@ describe('DeviceListener', () => {

                await flushPromises();

-                expect(mockClient.setAccountData).not.toHaveBeenCalledWith(
-                    `io.element.matrix_client_information.${deviceId}`,
-                    { name: 'Element', url: 'localhost', version: '1.2.3' },
-                );
+                expect(mockClient.setAccountData).not.toHaveBeenCalled();
            });

            it('saves client information after setting is enabled', async () => {
--- a/test/utils/device/parseUserAgent-test.ts
+++ b/test/utils/device/parseUserAgent-test.ts
@ -70,8 +70,8 @@ const DESKTOP_UA = [
    "Mozilla/5.0 (Windows NT 10.0) AppleWebKit/537.36 (KHTML, like Gecko) ElementNightly/2022091301 Chrome/104.0.5112.102 Electron/20.1.1 Safari/537.36",
 ];
 const DESKTOP_EXPECTED_RESULT = [
-    makeDeviceExtendedInfo(DeviceType.Desktop, undefined, "Mac OS 10.15.7", "Electron", "20"),
-    makeDeviceExtendedInfo(DeviceType.Desktop, undefined, "Windows 10", "Electron", "20"),
+    makeDeviceExtendedInfo(DeviceType.Desktop, undefined, "Mac OS", "Electron", "20.1.1"),
+    makeDeviceExtendedInfo(DeviceType.Desktop, undefined, "Windows", "Electron", "20.1.1"),
 ];

 const WEB_UA = [
@ -88,16 +88,16 @@ const WEB_UA = [
 ];

 const WEB_EXPECTED_RESULT = [
-    makeDeviceExtendedInfo(DeviceType.Web, undefined, "Mac OS 10.15.7", "Chrome", "104"),
-    makeDeviceExtendedInfo(DeviceType.Web, undefined, "Windows 10", "Chrome", "104"),
-    makeDeviceExtendedInfo(DeviceType.Web, undefined, "Mac OS 10.10", "Firefox", "39"),
-    makeDeviceExtendedInfo(DeviceType.Web, undefined, "Mac OS 10.10.2", "Safari", "8"),
-    makeDeviceExtendedInfo(DeviceType.Web, undefined, "Windows Vista", "Firefox", "40"),
-    makeDeviceExtendedInfo(DeviceType.Web, undefined, "Windows 10", "Edge", "12"),
+    makeDeviceExtendedInfo(DeviceType.Web, undefined, "Mac OS", "Chrome", "104.0.5112.102"),
+    makeDeviceExtendedInfo(DeviceType.Web, undefined, "Windows", "Chrome", "104.0.5112.102"),
+    makeDeviceExtendedInfo(DeviceType.Web, undefined, "Mac OS", "Firefox", "39.0"),
+    makeDeviceExtendedInfo(DeviceType.Web, undefined, "Mac OS", "Safari", "8.0.3"),
+    makeDeviceExtendedInfo(DeviceType.Web, undefined, "Windows", "Firefox", "40.0"),
+    makeDeviceExtendedInfo(DeviceType.Web, undefined, "Windows", "Edge", "12.246"),
    // using mobile browser
-    makeDeviceExtendedInfo(DeviceType.Web, "Apple iPad", "iOS 8.4.1", "Mobile Safari", "8"),
-    makeDeviceExtendedInfo(DeviceType.Web, "Apple iPhone", "iOS 8.4.1", "Mobile Safari", "8"),
-    makeDeviceExtendedInfo(DeviceType.Web, "Samsung SM-G973U", "Android 9", "Chrome", "69"),
+    makeDeviceExtendedInfo(DeviceType.Web, "Apple iPad", "iOS", "Mobile Safari", "8.0"),
+    makeDeviceExtendedInfo(DeviceType.Web, "Apple iPhone", "iOS", "Mobile Safari", "8.0"),
+    makeDeviceExtendedInfo(DeviceType.Web, "Samsung SM-G973U", "Android", "Chrome", "69.0.3497.100"),

 ];

@ -106,6 +106,8 @@ const MISC_UA = [
    "Curl Client/1.0",
    "banana",
    "",
+    // fluffy chat ios
+    "Dart/2.18 (dart:io)",
 ];

 const MISC_EXPECTED_RESULT = [
@ -113,6 +115,7 @@ const MISC_EXPECTED_RESULT = [
    makeDeviceExtendedInfo(DeviceType.Unknown, undefined, undefined, undefined, undefined),
    makeDeviceExtendedInfo(DeviceType.Unknown, undefined, undefined, undefined, undefined),
    makeDeviceExtendedInfo(DeviceType.Unknown, undefined, undefined, undefined, undefined),
+    makeDeviceExtendedInfo(DeviceType.Unknown, undefined, undefined, undefined, undefined),
 ];
 /* eslint-disable max-len */