From 888e69f39ae7f0ff4fa88200f8b89398c24742bd Mon Sep 17 00:00:00 2001 From: Kerry Date: Fri, 9 Dec 2022 10:52:00 +1300 Subject: [PATCH] Device manage - handle sessions that don't support encryption (#9717) * add handling for unverifiable sessions * test * update types for filtervariation * strict fixes * avoid setting up cross signing in device man tests --- .../e2e/settings/device-management.spec.ts | 1 - .../settings/devices/DeviceSecurityCard.tsx | 1 + .../devices/DeviceSecurityLearnMore.tsx | 20 ++++ .../devices/DeviceVerificationStatusCard.tsx | 50 ++++++--- .../settings/devices/FilteredDeviceList.tsx | 29 +++-- .../devices/SecurityRecommendations.tsx | 4 +- .../views/settings/devices/filter.ts | 8 +- .../views/settings/devices/types.ts | 3 + .../settings/tabs/user/SessionManagerTab.tsx | 7 +- src/i18n/strings/en_EN.json | 6 +- .../tabs/user/SessionManagerTab-test.tsx | 73 +++++++++++-- .../SessionManagerTab-test.tsx.snap | 103 +++++++----------- 12 files changed, 200 insertions(+), 105 deletions(-) diff --git a/cypress/e2e/settings/device-management.spec.ts b/cypress/e2e/settings/device-management.spec.ts index ba88db4861..352bbb80bb 100644 --- a/cypress/e2e/settings/device-management.spec.ts +++ b/cypress/e2e/settings/device-management.spec.ts @@ -49,7 +49,6 @@ describe("Device manager", () => { cy.get('[data-testid="current-session-section"]').within(() => { cy.contains('Unverified session').should('exist'); - cy.get('.mx_DeviceSecurityCard_actions [role="button"]').should('exist'); }); // current session details opened diff --git a/src/components/views/settings/devices/DeviceSecurityCard.tsx b/src/components/views/settings/devices/DeviceSecurityCard.tsx index 3e4eadb322..aaf0a5362f 100644 --- a/src/components/views/settings/devices/DeviceSecurityCard.tsx +++ b/src/components/views/settings/devices/DeviceSecurityCard.tsx @@ -32,6 +32,7 @@ const VariationIcon: Record = ({ variation }) => { diff --git a/src/components/views/settings/devices/DeviceSecurityLearnMore.tsx b/src/components/views/settings/devices/DeviceSecurityLearnMore.tsx index 2c10b02ecc..57fe23b086 100644 --- a/src/components/views/settings/devices/DeviceSecurityLearnMore.tsx +++ b/src/components/views/settings/devices/DeviceSecurityLearnMore.tsx @@ -56,6 +56,26 @@ const securityCardContent: Record , }, + // unverifiable uses single-session case + // because it is only ever displayed on a single session detail + [DeviceSecurityVariation.Unverifiable]: { + title: _t('Unverified session'), + description: <> +

{ _t(`This session doesn't support encryption, so it can't be verified.`) } +

+

+ { _t( + `You won't be able to participate in rooms where encryption is enabled when using this session.`, + ) + } +

+ { _t( + `For best security and privacy, it is recommended to use Matrix clients that support encryption.`, + ) + } +

+ , + }, [DeviceSecurityVariation.Inactive]: { title: _t('Inactive sessions'), description: <> diff --git a/src/components/views/settings/devices/DeviceVerificationStatusCard.tsx b/src/components/views/settings/devices/DeviceVerificationStatusCard.tsx index 0ee37c9bc4..6740175ff6 100644 --- a/src/components/views/settings/devices/DeviceVerificationStatusCard.tsx +++ b/src/components/views/settings/devices/DeviceVerificationStatusCard.tsx @@ -30,18 +30,33 @@ interface Props { onVerifyDevice?: () => void; } -export const DeviceVerificationStatusCard: React.FC = ({ - device, - onVerifyDevice, -}) => { - const securityCardProps = device.isVerified ? { - variation: DeviceSecurityVariation.Verified, - heading: _t('Verified session'), - description: <> - { _t('This session is ready for secure messaging.') } - - , - } : { +const getCardProps = (device: ExtendedDevice): { + variation: DeviceSecurityVariation; + heading: string; + description: React.ReactNode; +} => { + if (device.isVerified) { + return { + variation: DeviceSecurityVariation.Verified, + heading: _t('Verified session'), + description: <> + { _t('This session is ready for secure messaging.') } + + , + }; + } + if (device.isVerified === null) { + return { + variation: DeviceSecurityVariation.Unverified, + heading: _t('Unverified session'), + description: <> + { _t(`This session doesn't support encryption and thus can't be verified.`) } + + , + }; + } + + return { variation: DeviceSecurityVariation.Unverified, heading: _t('Unverified session'), description: <> @@ -49,10 +64,19 @@ export const DeviceVerificationStatusCard: React.FC = ({ , }; +}; + +export const DeviceVerificationStatusCard: React.FC = ({ + device, + onVerifyDevice, +}) => { + const securityCardProps = getCardProps(device); + return - { !device.isVerified && !!onVerifyDevice && + { /* check for explicit false to exclude unverifiable devices */ } + { device.isVerified === false && !!onVerifyDevice && void; + filter?: FilterVariation; + onFilterChange: (filter: FilterVariation | undefined) => void; onDeviceExpandToggle: (deviceId: ExtendedDevice['device_id']) => void; onSignOutDevices: (deviceIds: ExtendedDevice['device_id'][]) => void; saveDeviceName: DevicesState['saveDeviceName']; @@ -68,12 +69,12 @@ const sortDevicesByLatestActivityThenDisplayName = (left: ExtendedDevice, right: (right.last_seen_ts || 0) - (left.last_seen_ts || 0) || ((left.display_name || left.device_id).localeCompare(right.display_name || right.device_id)); -const getFilteredSortedDevices = (devices: DevicesDictionary, filter?: DeviceSecurityVariation) => +const getFilteredSortedDevices = (devices: DevicesDictionary, filter?: FilterVariation) => filterDevicesBySecurityRecommendation(Object.values(devices), filter ? [filter] : []) .sort(sortDevicesByLatestActivityThenDisplayName); const ALL_FILTER_ID = 'ALL'; -type DeviceFilterKey = DeviceSecurityVariation | typeof ALL_FILTER_ID; +type DeviceFilterKey = FilterVariation | typeof ALL_FILTER_ID; const securityCardContent: Record - Object.values(DeviceSecurityVariation).includes(filter); +const isSecurityVariation = (filter?: DeviceFilterKey): filter is FilterVariation => + !!filter && ([ + DeviceSecurityVariation.Inactive, + DeviceSecurityVariation.Unverified, + DeviceSecurityVariation.Verified, + ] as string[]).includes(filter); const FilterSecurityCard: React.FC<{ filter?: DeviceFilterKey }> = ({ filter }) => { if (isSecurityVariation(filter)) { @@ -124,7 +135,7 @@ const FilterSecurityCard: React.FC<{ filter?: DeviceFilterKey }> = ({ filter }) return null; }; -const getNoResultsMessage = (filter?: DeviceSecurityVariation): string => { +const getNoResultsMessage = (filter?: FilterVariation): string => { switch (filter) { case DeviceSecurityVariation.Verified: return _t('No verified sessions found.'); @@ -136,7 +147,7 @@ const getNoResultsMessage = (filter?: DeviceSecurityVariation): string => { return _t('No sessions found.'); } }; -interface NoResultsProps { filter?: DeviceSecurityVariation, clearFilter: () => void} +interface NoResultsProps { filter?: FilterVariation, clearFilter: () => void} const NoResults: React.FC = ({ filter, clearFilter }) =>
{ getNoResultsMessage(filter) } @@ -273,7 +284,7 @@ export const FilteredDeviceList = ]; const onFilterOptionChange = (filterId: DeviceFilterKey) => { - onFilterChange(filterId === ALL_FILTER_ID ? undefined : filterId as DeviceSecurityVariation); + onFilterChange(filterId === ALL_FILTER_ID ? undefined : filterId as FilterVariation); }; const isAllSelected = selectedDeviceIds.length >= sortedDevices.length; diff --git a/src/components/views/settings/devices/SecurityRecommendations.tsx b/src/components/views/settings/devices/SecurityRecommendations.tsx index 7b6381306b..33cd566ee7 100644 --- a/src/components/views/settings/devices/SecurityRecommendations.tsx +++ b/src/components/views/settings/devices/SecurityRecommendations.tsx @@ -21,7 +21,7 @@ import AccessibleButton from '../../elements/AccessibleButton'; import SettingsSubsection from '../shared/SettingsSubsection'; import DeviceSecurityCard from './DeviceSecurityCard'; import { DeviceSecurityLearnMore } from './DeviceSecurityLearnMore'; -import { filterDevicesBySecurityRecommendation, INACTIVE_DEVICE_AGE_DAYS } from './filter'; +import { filterDevicesBySecurityRecommendation, FilterVariation, INACTIVE_DEVICE_AGE_DAYS } from './filter'; import { DeviceSecurityVariation, ExtendedDevice, @@ -31,7 +31,7 @@ import { interface Props { devices: DevicesDictionary; currentDeviceId: ExtendedDevice['device_id']; - goToFilteredList: (filter: DeviceSecurityVariation) => void; + goToFilteredList: (filter: FilterVariation) => void; } const SecurityRecommendations: React.FC = ({ diff --git a/src/components/views/settings/devices/filter.ts b/src/components/views/settings/devices/filter.ts index 05ceb9c697..542d079be0 100644 --- a/src/components/views/settings/devices/filter.ts +++ b/src/components/views/settings/devices/filter.ts @@ -22,10 +22,14 @@ const MS_DAY = 24 * 60 * 60 * 1000; export const INACTIVE_DEVICE_AGE_MS = 7.776e+9; // 90 days export const INACTIVE_DEVICE_AGE_DAYS = INACTIVE_DEVICE_AGE_MS / MS_DAY; +export type FilterVariation = DeviceSecurityVariation.Verified + | DeviceSecurityVariation.Inactive + | DeviceSecurityVariation.Unverified; + export const isDeviceInactive: DeviceFilterCondition = device => !!device.last_seen_ts && device.last_seen_ts < Date.now() - INACTIVE_DEVICE_AGE_MS; -const filters: Record = { +const filters: Record = { [DeviceSecurityVariation.Verified]: device => !!device.isVerified, [DeviceSecurityVariation.Unverified]: device => !device.isVerified, [DeviceSecurityVariation.Inactive]: isDeviceInactive, @@ -33,7 +37,7 @@ const filters: Record = { export const filterDevicesBySecurityRecommendation = ( devices: ExtendedDevice[], - securityVariations: DeviceSecurityVariation[], + securityVariations: FilterVariation[], ) => { const activeFilters = securityVariations.map(variation => filters[variation]); if (!activeFilters.length) { diff --git a/src/components/views/settings/devices/types.ts b/src/components/views/settings/devices/types.ts index 3fa125a09f..99afd5c759 100644 --- a/src/components/views/settings/devices/types.ts +++ b/src/components/views/settings/devices/types.ts @@ -32,4 +32,7 @@ export enum DeviceSecurityVariation { Verified = 'Verified', Unverified = 'Unverified', Inactive = 'Inactive', + // sessions that do not support encryption + // eg a session that logged in via api to get an access token + Unverifiable = 'Unverifiable' } diff --git a/src/components/views/settings/tabs/user/SessionManagerTab.tsx b/src/components/views/settings/tabs/user/SessionManagerTab.tsx index a266820189..2d07d855a6 100644 --- a/src/components/views/settings/tabs/user/SessionManagerTab.tsx +++ b/src/components/views/settings/tabs/user/SessionManagerTab.tsx @@ -29,7 +29,7 @@ import { useOwnDevices } from '../../devices/useOwnDevices'; import { FilteredDeviceList } from '../../devices/FilteredDeviceList'; import CurrentDeviceSection from '../../devices/CurrentDeviceSection'; import SecurityRecommendations from '../../devices/SecurityRecommendations'; -import { DeviceSecurityVariation, ExtendedDevice } from '../../devices/types'; +import { ExtendedDevice } from '../../devices/types'; import { deleteDevicesWithInteractiveAuth } from '../../devices/deleteDevices'; import SettingsTab from '../SettingsTab'; import LoginWithQRSection from '../../devices/LoginWithQRSection'; @@ -37,6 +37,7 @@ import LoginWithQR, { Mode } from '../../../auth/LoginWithQR'; import SettingsStore from '../../../../../settings/SettingsStore'; import { useAsyncMemo } from '../../../../../hooks/useAsyncMemo'; import QuestionDialog from '../../../dialogs/QuestionDialog'; +import { FilterVariation } from '../../devices/filter'; const confirmSignOut = async (sessionsToSignOutCount: number): Promise => { const { finished } = Modal.createDialog(QuestionDialog, { @@ -123,7 +124,7 @@ const SessionManagerTab: React.FC = () => { setPushNotifications, supportsMSC3881, } = useOwnDevices(); - const [filter, setFilter] = useState(); + const [filter, setFilter] = useState(); const [expandedDeviceIds, setExpandedDeviceIds] = useState([]); const [selectedDeviceIds, setSelectedDeviceIds] = useState([]); const filteredDeviceListRef = useRef(null); @@ -142,7 +143,7 @@ const SessionManagerTab: React.FC = () => { } }; - const onGoToFilteredList = (filter: DeviceSecurityVariation) => { + const onGoToFilteredList = (filter: FilterVariation) => { setFilter(filter); clearTimeout(scrollIntoViewTimeoutRef.current); // wait a tick for the filtered section to rerender with different height diff --git a/src/i18n/strings/en_EN.json b/src/i18n/strings/en_EN.json index 1ebef5ac97..bc437398ae 100644 --- a/src/i18n/strings/en_EN.json +++ b/src/i18n/strings/en_EN.json @@ -1801,6 +1801,10 @@ "Unverified sessions": "Unverified sessions", "Unverified sessions are sessions that have logged in with your credentials but have not been cross-verified.": "Unverified sessions are sessions that have logged in with your credentials but have not been cross-verified.", "You should make especially certain that you recognise these sessions as they could represent an unauthorised use of your account.": "You should make especially certain that you recognise these sessions as they could represent an unauthorised use of your account.", + "Unverified session": "Unverified session", + "This session doesn't support encryption, so it can't be verified.": "This session doesn't support encryption, so it can't be verified.", + "You won't be able to participate in rooms where encryption is enabled when using this session.": "You won't be able to participate in rooms where encryption is enabled when using this session.", + "For best security and privacy, it is recommended to use Matrix clients that support encryption.": "For best security and privacy, it is recommended to use Matrix clients that support encryption.", "Inactive sessions": "Inactive sessions", "Inactive sessions are sessions you have not used in some time, but they continue to receive encryption keys.": "Inactive sessions are sessions you have not used in some time, but they continue to receive encryption keys.", "Removing inactive sessions improves security and performance, and makes it easier for you to identify if a new session is suspicious.": "Removing inactive sessions improves security and performance, and makes it easier for you to identify if a new session is suspicious.", @@ -1813,7 +1817,7 @@ "Unknown session type": "Unknown session type", "Verified session": "Verified session", "This session is ready for secure messaging.": "This session is ready for secure messaging.", - "Unverified session": "Unverified session", + "This session doesn't support encryption and thus can't be verified.": "This session doesn't support encryption and thus can't be verified.", "Verify or sign out from this session for best security and reliability.": "Verify or sign out from this session for best security and reliability.", "Verify session": "Verify session", "For best security, sign out from any session that you don't recognize or use anymore.": "For best security, sign out from any session that you don't recognize or use anymore.", diff --git a/test/components/views/settings/tabs/user/SessionManagerTab-test.tsx b/test/components/views/settings/tabs/user/SessionManagerTab-test.tsx index 3c4bfd470f..58839d5e96 100644 --- a/test/components/views/settings/tabs/user/SessionManagerTab-test.tsx +++ b/test/components/views/settings/tabs/user/SessionManagerTab-test.tsx @@ -255,12 +255,23 @@ describe('', () => { }); it('sets device verification status correctly', async () => { - mockClient.getDevices.mockResolvedValue({ devices: [alicesDevice, alicesMobileDevice] }); + mockClient.getDevices.mockResolvedValue({ devices: + [alicesDevice, alicesMobileDevice, alicesOlderMobileDevice], + }); + mockClient.getStoredDevice.mockImplementation((_userId, deviceId) => new DeviceInfo(deviceId)); mockCrossSigningInfo.checkDeviceTrust - // alices device is trusted - .mockReturnValueOnce(new DeviceTrustLevel(true, true, false, false)) - // alices mobile device is not - .mockReturnValueOnce(new DeviceTrustLevel(false, false, false, false)); + .mockImplementation((_userId, { deviceId }) => { + // alices device is trusted + if (deviceId === alicesDevice.device_id) { + return new DeviceTrustLevel(true, true, false, false); + } + // alices mobile device is not + if (deviceId === alicesMobileDevice.device_id) { + return new DeviceTrustLevel(false, false, false, false); + } + // alicesOlderMobileDevice does not support encryption + throw new Error('encryption not supported'); + }); const { getByTestId } = render(getComponent()); @@ -268,8 +279,20 @@ describe('', () => { await flushPromises(); }); - expect(mockCrossSigningInfo.checkDeviceTrust).toHaveBeenCalledTimes(2); - expect(getByTestId(`device-tile-${alicesDevice.device_id}`)).toMatchSnapshot(); + expect(mockCrossSigningInfo.checkDeviceTrust).toHaveBeenCalledTimes(3); + expect( + getByTestId(`device-tile-${alicesDevice.device_id}`) + .querySelector('[aria-label="Verified"]'), + ).toBeTruthy(); + expect( + getByTestId(`device-tile-${alicesMobileDevice.device_id}`) + .querySelector('[aria-label="Unverified"]'), + ).toBeTruthy(); + // sessions that dont support encryption use unverified badge + expect( + getByTestId(`device-tile-${alicesOlderMobileDevice.device_id}`) + .querySelector('[aria-label="Unverified"]'), + ).toBeTruthy(); }); it('extends device with client information when available', async () => { @@ -489,7 +512,7 @@ describe('', () => { if (deviceId === alicesDevice.device_id) { return new DeviceTrustLevel(true, true, false, false); } - throw new Error('everything else unverified'); + return new DeviceTrustLevel(false, false, false, false); }); const { getByTestId } = render(getComponent()); @@ -507,6 +530,38 @@ describe('', () => { expect(modalSpy).toHaveBeenCalled(); }); + it('does not allow device verification on session that do not support encryption', async () => { + mockClient.getDevices.mockResolvedValue({ devices: [alicesDevice, alicesMobileDevice] }); + mockClient.getStoredDevice.mockImplementation((_userId, deviceId) => new DeviceInfo(deviceId)); + mockCrossSigningInfo.checkDeviceTrust + .mockImplementation((_userId, { deviceId }) => { + // current session verified = able to verify other sessions + if (deviceId === alicesDevice.device_id) { + return new DeviceTrustLevel(true, true, false, false); + } + // but alicesMobileDevice doesn't support encryption + throw new Error('encryption not supported'); + }); + + const { + getByTestId, + queryByTestId, + } = render(getComponent()); + + await act(async () => { + await flushPromises(); + }); + + toggleDeviceDetails(getByTestId, alicesMobileDevice.device_id); + + // no verify button + expect(queryByTestId(`verification-status-button-${alicesMobileDevice.device_id}`)).toBeFalsy(); + expect( + getByTestId(`device-detail-${alicesMobileDevice.device_id}`) + .getElementsByClassName('mx_DeviceSecurityCard'), + ).toMatchSnapshot(); + }); + it('refreshes devices after verifying other device', async () => { const modalSpy = jest.spyOn(Modal, 'createDialog'); @@ -518,7 +573,7 @@ describe('', () => { if (deviceId === alicesDevice.device_id) { return new DeviceTrustLevel(true, true, false, false); } - throw new Error('everything else unverified'); + return new DeviceTrustLevel(false, false, false, false); }); const { getByTestId } = render(getComponent()); diff --git a/test/components/views/settings/tabs/user/__snapshots__/SessionManagerTab-test.tsx.snap b/test/components/views/settings/tabs/user/__snapshots__/SessionManagerTab-test.tsx.snap index a92f68c756..166f54a32e 100644 --- a/test/components/views/settings/tabs/user/__snapshots__/SessionManagerTab-test.tsx.snap +++ b/test/components/views/settings/tabs/user/__snapshots__/SessionManagerTab-test.tsx.snap @@ -1,5 +1,43 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP +exports[` Device verification does not allow device verification on session that do not support encryption 1`] = ` +HTMLCollection [ +
+
+
+
+
+

+ Unverified session +

+

+ This session doesn't support encryption and thus can't be verified. +

+ Learn more +
+

+
+
, +] +`; + exports[` Sign out Signs out of current device 1`] = `
goes to filtered list from security recommendatio
`; - -exports[` sets device verification status correctly 1`] = ` -
-
-
- - -
-

- Alices device -

-
- - Verified - - ยท - - alices_device - -
-
-
-
-
-
-
-
-`;