Device manager - display client information in device details (PSG-682) (#9315)

* record device client inforamtion events on app start

* matrix-client-information -> matrix_client_information

* fix types

* remove another unused export

* add docs link

* display device client information in device details

* update snapshots

* integration-ish test client information in metadata

* tests

* fix tests

* export helper

* DeviceClientInformation type

src/components/views/settings/devices/DeviceDetails.tsx

@@ -26,10 +26,10 @@ import Spinner from '../../elements/Spinner';
 import ToggleSwitch from '../../elements/ToggleSwitch';
 import { DeviceDetailHeading } from './DeviceDetailHeading';
 import { DeviceVerificationStatusCard } from './DeviceVerificationStatusCard';
-import { DeviceWithVerification } from './types';
+import { ExtendedDevice } from './types';
 
 interface Props {
-    device: DeviceWithVerification;
+    device: ExtendedDevice;
     pusher?: IPusher | undefined;
     localNotificationSettings?: LocalNotificationSettings | undefined;
     isSigningOut: boolean;
@@ -41,6 +41,7 @@ interface Props {
 }
 
 interface MetadataTable {
+    id: string;
     heading?: string;
     values: { label: string, value?: string | React.ReactNode }[];
 }
@@ -58,6 +59,7 @@ const DeviceDetails: React.FC<Props> = ({
 }) => {
     const metadata: MetadataTable[] = [
         {
+            id: 'session',
             values: [
                 { label: _t('Session ID'), value: device.device_id },
                 {
@@ -67,12 +69,28 @@ const DeviceDetails: React.FC<Props> = ({
             ],
         },
         {
+            id: 'application',
+            heading: _t('Application'),
+            values: [
+                { label: _t('Name'), value: device.clientName },
+                { label: _t('Version'), value: device.clientVersion },
+                { label: _t('URL'), value: device.url },
+            ],
+        },
+        {
+            id: 'device',
             heading: _t('Device'),
             values: [
                 { label: _t('IP address'), value: device.last_seen_ip },
             ],
         },
-    ];
+    ].map(section =>
+        // filter out falsy values
+        ({ ...section, values: section.values.filter(row => !!row.value) }))
+        .filter(section =>
+        // then filter out sections with no values
+            section.values.length,
+        );
 
     const showPushNotificationSection = !!pusher || !!localNotificationSettings;
 
@@ -101,9 +119,10 @@ const DeviceDetails: React.FC<Props> = ({
         </section>
         <section className='mx_DeviceDetails_section'>
             <p className='mx_DeviceDetails_sectionHeading'>{ _t('Session details') }</p>
-            { metadata.map(({ heading, values }, index) => <table
+            { metadata.map(({ heading, values, id }, index) => <table
                 className='mx_DeviceDetails_metadataTable'
                 key={index}
+                data-testid={`device-detail-metadata-${id}`}
             >
                 { heading &&
                     <thead>

src/components/views/settings/devices/types.ts

@@ -17,7 +17,13 @@ limitations under the License.
 import { IMyDevice } from "matrix-js-sdk/src/matrix";
 
 export type DeviceWithVerification = IMyDevice & { isVerified: boolean | null };
-export type DevicesDictionary = Record<DeviceWithVerification['device_id'], DeviceWithVerification>;
+export type ExtendedDeviceInfo = {
+    clientName?: string;
+    clientVersion?: string;
+    url?: string;
+};
+export type ExtendedDevice = DeviceWithVerification & ExtendedDeviceInfo;
+export type DevicesDictionary = Record<DeviceWithVerification['device_id'], ExtendedDevice>;
 
 export enum DeviceSecurityVariation {
     Verified = 'Verified',

src/components/views/settings/devices/useOwnDevices.ts

@@ -33,7 +33,8 @@ import { LocalNotificationSettings } from "matrix-js-sdk/src/@types/local_notifi
 
 import MatrixClientContext from "../../../../contexts/MatrixClientContext";
 import { _t } from "../../../../languageHandler";
-import { DevicesDictionary, DeviceWithVerification } from "./types";
+import { getDeviceClientInformation } from "../../../../utils/device/clientInformation";
+import { DevicesDictionary, DeviceWithVerification, ExtendedDeviceInfo } from "./types";
 import { useEventEmitter } from "../../../../hooks/useEventEmitter";
 
 const isDeviceVerified = (
@@ -62,6 +63,16 @@ const isDeviceVerified = (
     }
 };
 
+const parseDeviceExtendedInformation = (matrixClient: MatrixClient, device: IMyDevice): ExtendedDeviceInfo => {
+    const { name, version, url } = getDeviceClientInformation(matrixClient, device.device_id);
+
+    return {
+        clientName: name,
+        clientVersion: version,
+        url,
+    };
+};
+
 const fetchDevicesWithVerification = async (
     matrixClient: MatrixClient,
     userId: string,
@@ -75,6 +86,7 @@ const fetchDevicesWithVerification = async (
         [device.device_id]: {
             ...device,
             isVerified: isDeviceVerified(matrixClient, crossSigningInfo, device),
+            ...parseDeviceExtendedInformation(matrixClient, device),
         },
     }), {});
 

src/i18n/strings/en_EN.json

@@ -1717,6 +1717,9 @@
     "Please be aware that session names are also visible to people you communicate with": "Please be aware that session names are also visible to people you communicate with",
     "Session ID": "Session ID",
     "Last activity": "Last activity",
+    "Application": "Application",
+    "Version": "Version",
+    "URL": "URL",
     "Device": "Device",
     "IP address": "IP address",
     "Session details": "Session details",

src/utils/device/clientInformation.ts

@@ -19,6 +19,12 @@ import { MatrixClient } from "matrix-js-sdk/src/client";
 import BasePlatform from "../../BasePlatform";
 import { IConfigOptions } from "../../IConfigOptions";
 
+export type DeviceClientInformation = {
+    name?: string;
+    version?: string;
+    url?: string;
+};
+
 const formatUrl = (): string | undefined => {
     // don't record url for electron clients
     if (window.electron) {
@@ -34,7 +40,7 @@ const formatUrl = (): string | undefined => {
     ].join("");
 };
 
-const getClientInformationEventType = (deviceId: string): string =>
+export const getClientInformationEventType = (deviceId: string): string =>
     `io.element.matrix_client_information.${deviceId}`;
 
 /**
@@ -58,3 +64,23 @@ export const recordClientInformation = async (
         url,
     });
 };
+
+const sanitizeContentString = (value: unknown): string | undefined =>
+    value && typeof value === 'string' ? value : undefined;
+
+export const getDeviceClientInformation = (matrixClient: MatrixClient, deviceId: string): DeviceClientInformation => {
+    const event = matrixClient.getAccountData(getClientInformationEventType(deviceId));
+
+    if (!event) {
+        return {};
+    }
+
+    const { name, version, url } = event.getContent();
+
+    return {
+        name: sanitizeContentString(name),
+        version: sanitizeContentString(version),
+        url: sanitizeContentString(url),
+    };
+};
+

test/DeviceListener-test.ts

@@ -29,8 +29,8 @@ import { isSecretStorageBeingAccessed } from "../src/SecurityManager";
 import dis from "../src/dispatcher/dispatcher";
 import { Action } from "../src/dispatcher/actions";
 import SettingsStore from "../src/settings/SettingsStore";
-import { mockPlatformPeg } from "./test-utils";
 import { SettingLevel } from "../src/settings/SettingLevel";
+import { mockPlatformPeg } from "./test-utils";
 
 // don't litter test console with logs
 jest.mock("matrix-js-sdk/src/logger");

test/components/views/settings/devices/DeviceDetails-test.tsx

@@ -58,6 +58,7 @@ describe('<DeviceDetails />', () => {
             display_name: 'My Device',
             last_seen_ip: '123.456.789',
             last_seen_ts: now - 60000000,
+            clientName: 'Element Web',
         };
         const { container } = render(getComponent({ device }));
         expect(container).toMatchSnapshot();

test/components/views/settings/devices/__snapshots__/CurrentDeviceSection-test.tsx.snap

@@ -76,6 +76,7 @@ HTMLCollection [
       </p>
       <table
         class="mx_DeviceDetails_metadataTable"
+        data-testid="device-detail-metadata-session"
       >
         <tbody>
           <tr>
@@ -90,39 +91,6 @@ HTMLCollection [
               alices_device
             </td>
           </tr>
-          <tr>
-            <td
-              class="mxDeviceDetails_metadataLabel"
-            >
-              Last activity
-            </td>
-            <td
-              class="mxDeviceDetails_metadataValue"
-            />
-          </tr>
-        </tbody>
-      </table>
-      <table
-        class="mx_DeviceDetails_metadataTable"
-      >
-        <thead>
-          <tr>
-            <th>
-              Device
-            </th>
-          </tr>
-        </thead>
-        <tbody>
-          <tr>
-            <td
-              class="mxDeviceDetails_metadataLabel"
-            >
-              IP address
-            </td>
-            <td
-              class="mxDeviceDetails_metadataValue"
-            />
-          </tr>
         </tbody>
       </table>
     </section>

test/components/views/settings/devices/__snapshots__/DeviceDetails-test.tsx.snap

@@ -64,6 +64,7 @@ exports[`<DeviceDetails /> renders a verified device 1`] = `
       </p>
       <table
         class="mx_DeviceDetails_metadataTable"
+        data-testid="device-detail-metadata-session"
       >
         <tbody>
           <tr>
@@ -78,39 +79,6 @@ exports[`<DeviceDetails /> renders a verified device 1`] = `
               my-device
             </td>
           </tr>
-          <tr>
-            <td
-              class="mxDeviceDetails_metadataLabel"
-            >
-              Last activity
-            </td>
-            <td
-              class="mxDeviceDetails_metadataValue"
-            />
-          </tr>
-        </tbody>
-      </table>
-      <table
-        class="mx_DeviceDetails_metadataTable"
-      >
-        <thead>
-          <tr>
-            <th>
-              Device
-            </th>
-          </tr>
-        </thead>
-        <tbody>
-          <tr>
-            <td
-              class="mxDeviceDetails_metadataLabel"
-            >
-              IP address
-            </td>
-            <td
-              class="mxDeviceDetails_metadataValue"
-            />
-          </tr>
         </tbody>
       </table>
     </section>
@@ -198,6 +166,7 @@ exports[`<DeviceDetails /> renders device with metadata 1`] = `
       </p>
       <table
         class="mx_DeviceDetails_metadataTable"
+        data-testid="device-detail-metadata-session"
       >
         <tbody>
           <tr>
@@ -228,6 +197,33 @@ exports[`<DeviceDetails /> renders device with metadata 1`] = `
       </table>
       <table
         class="mx_DeviceDetails_metadataTable"
+        data-testid="device-detail-metadata-application"
+      >
+        <thead>
+          <tr>
+            <th>
+              Application
+            </th>
+          </tr>
+        </thead>
+        <tbody>
+          <tr>
+            <td
+              class="mxDeviceDetails_metadataLabel"
+            >
+              Name
+            </td>
+            <td
+              class="mxDeviceDetails_metadataValue"
+            >
+              Element Web
+            </td>
+          </tr>
+        </tbody>
+      </table>
+      <table
+        class="mx_DeviceDetails_metadataTable"
+        data-testid="device-detail-metadata-device"
       >
         <thead>
           <tr>
@@ -336,6 +332,7 @@ exports[`<DeviceDetails /> renders device without metadata 1`] = `
       </p>
       <table
         class="mx_DeviceDetails_metadataTable"
+        data-testid="device-detail-metadata-session"
       >
         <tbody>
           <tr>
@@ -350,39 +347,6 @@ exports[`<DeviceDetails /> renders device without metadata 1`] = `
               my-device
             </td>
           </tr>
-          <tr>
-            <td
-              class="mxDeviceDetails_metadataLabel"
-            >
-              Last activity
-            </td>
-            <td
-              class="mxDeviceDetails_metadataValue"
-            />
-          </tr>
-        </tbody>
-      </table>
-      <table
-        class="mx_DeviceDetails_metadataTable"
-      >
-        <thead>
-          <tr>
-            <th>
-              Device
-            </th>
-          </tr>
-        </thead>
-        <tbody>
-          <tr>
-            <td
-              class="mxDeviceDetails_metadataLabel"
-            >
-              IP address
-            </td>
-            <td
-              class="mxDeviceDetails_metadataValue"
-            />
-          </tr>
         </tbody>
       </table>
     </section>

test/components/views/settings/tabs/user/SessionManagerTab-test.tsx

@@ -87,10 +87,10 @@ describe('<SessionManagerTab />', () => {
         deleteMultipleDevices: jest.fn(),
         generateClientSecret: jest.fn(),
         setDeviceDetails: jest.fn(),
+        getAccountData: jest.fn(),
         doesServerSupportUnstableFeature: jest.fn().mockResolvedValue(true),
         getPushers: jest.fn(),
         setPusher: jest.fn(),
-        getAccountData: jest.fn(),
         setLocalNotificationSettings: jest.fn(),
     });
 
@@ -243,6 +243,48 @@ describe('<SessionManagerTab />', () => {
         expect(getByTestId(`device-tile-${alicesDevice.device_id}`)).toMatchSnapshot();
     });
 
+    it('extends device with client information when available', async () => {
+        mockClient.getDevices.mockResolvedValue({ devices: [alicesDevice, alicesMobileDevice] });
+        mockClient.getAccountData.mockImplementation((eventType: string) => {
+            const content = {
+                name: 'Element Web',
+                version: '1.2.3',
+                url: 'test.com',
+            };
+            return new MatrixEvent({
+                type: eventType,
+                content,
+            });
+        });
+
+        const { getByTestId } = render(getComponent());
+
+        await act(async () => {
+            await flushPromisesWithFakeTimers();
+        });
+
+        // twice for each device
+        expect(mockClient.getAccountData).toHaveBeenCalledTimes(4);
+
+        toggleDeviceDetails(getByTestId, alicesDevice.device_id);
+        // application metadata section rendered
+        expect(getByTestId('device-detail-metadata-application')).toBeTruthy();
+    });
+
+    it('renders devices without available client information without error', async () => {
+        mockClient.getDevices.mockResolvedValue({ devices: [alicesDevice, alicesMobileDevice] });
+
+        const { getByTestId, queryByTestId } = render(getComponent());
+
+        await act(async () => {
+            await flushPromisesWithFakeTimers();
+        });
+
+        toggleDeviceDetails(getByTestId, alicesDevice.device_id);
+        // application metadata section not rendered
+        expect(queryByTestId('device-detail-metadata-application')).toBeFalsy();
+    });
+
     it('renders current session section with an unverified session', async () => {
         mockClient.getDevices.mockResolvedValue({ devices: [alicesDevice, alicesMobileDevice] });
         const { getByTestId } = render(getComponent());

test/utils/device/clientInformation-test.ts

@@ -14,9 +14,14 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
+import { MatrixEvent } from "matrix-js-sdk/src/matrix";
+
 import BasePlatform from "../../../src/BasePlatform";
 import { IConfigOptions } from "../../../src/IConfigOptions";
-import { recordClientInformation } from "../../../src/utils/device/clientInformation";
+import {
+    getDeviceClientInformation,
+    recordClientInformation,
+} from "../../../src/utils/device/clientInformation";
 import { getMockClientWithEventEmitter } from "../../test-utils";
 
 describe('recordClientInformation()', () => {
@@ -84,3 +89,58 @@ describe('recordClientInformation()', () => {
         );
     });
 });
+
+describe('getDeviceClientInformation()', () => {
+    const deviceId = 'my-device-id';
+
+    const mockClient = getMockClientWithEventEmitter({
+        getAccountData: jest.fn(),
+    });
+
+    beforeEach(() => {
+        jest.resetAllMocks();
+    });
+
+    it('returns an empty object when no event exists for the device', () => {
+        expect(getDeviceClientInformation(mockClient, deviceId)).toEqual({});
+
+        expect(mockClient.getAccountData).toHaveBeenCalledWith(
+            `io.element.matrix_client_information.${deviceId}`,
+        );
+    });
+
+    it('returns client information for the device', () => {
+        const eventContent = {
+            name: 'Element Web',
+            version: '1.2.3',
+            url: 'test.com',
+        };
+        const event = new MatrixEvent({
+            type: `io.element.matrix_client_information.${deviceId}`,
+            content: eventContent,
+        });
+        mockClient.getAccountData.mockReturnValue(event);
+        expect(getDeviceClientInformation(mockClient, deviceId)).toEqual(eventContent);
+    });
+
+    it('excludes values with incorrect types', () => {
+        const eventContent = {
+            extraField: 'hello',
+            name: 'Element Web',
+            // wrong format
+            version: { value: '1.2.3' },
+            url: 'test.com',
+        };
+        const event = new MatrixEvent({
+            type: `io.element.matrix_client_information.${deviceId}`,
+            content: eventContent,
+        });
+        mockClient.getAccountData.mockReturnValue(event);
+        // invalid fields excluded
+        expect(getDeviceClientInformation(mockClient, deviceId)).toEqual({
+            name: eventContent.name,
+            url: eventContent.url,
+        });
+    });
+});
+