Device manager - prune client information events after remote sign out (#9874)
* prune client infromation events from old devices * test client data pruning * remove debug * strict * Update src/components/views/settings/devices/useOwnDevices.ts Co-authored-by: Michael Weimann <michaelw@matrix.org> * improvements from review Co-authored-by: Michael Weimann <michaelw@matrix.org>
This commit is contained in:
Kerry
GitHub
parent
4f0a5d1eb4
commit
b1c32995c6
3 changed files with 80 additions and 15 deletions
|
|
@ -35,7 +35,7 @@ import { CryptoEvent } from "matrix-js-sdk/src/crypto";
|
|||
|
||||
import MatrixClientContext from "../../../../contexts/MatrixClientContext";
|
||||
import { _t } from "../../../../languageHandler";
|
||||
import { getDeviceClientInformation } from "../../../../utils/device/clientInformation";
|
||||
import { getDeviceClientInformation, pruneClientInformation } from "../../../../utils/device/clientInformation";
|
||||
import { DevicesDictionary, ExtendedDevice, ExtendedDeviceAppInfo } from "./types";
|
||||
import { useEventEmitter } from "../../../../hooks/useEventEmitter";
|
||||
import { parseUserAgent } from "../../../../utils/device/parseUserAgent";
|
||||
|
|
@ -116,8 +116,8 @@ export type DevicesState = {
|
|||
export const useOwnDevices = (): DevicesState => {
|
||||
const matrixClient = useContext(MatrixClientContext);
|
||||
|
||||
const currentDeviceId = matrixClient.getDeviceId();
|
||||
const userId = matrixClient.getUserId();
|
||||
const currentDeviceId = matrixClient.getDeviceId()!;
|
||||
const userId = matrixClient.getSafeUserId();
|
||||
|
||||
const [devices, setDevices] = useState<DevicesState["devices"]>({});
|
||||
const [pushers, setPushers] = useState<DevicesState["pushers"]>([]);
|
||||
|
|
@ -138,11 +138,6 @@ export const useOwnDevices = (): DevicesState => {
|
|||
const refreshDevices = useCallback(async () => {
|
||||
setIsLoadingDeviceList(true);
|
||||
try {
|
||||
// realistically we should never hit this
|
||||
// but it satisfies types
|
||||
if (!userId) {
|
||||
throw new Error("Cannot fetch devices without user id");
|
||||
}
|
||||
const devices = await fetchDevicesWithVerification(matrixClient, userId);
|
||||
setDevices(devices);
|
||||
|
||||
|
|
@ -176,6 +171,15 @@ export const useOwnDevices = (): DevicesState => {
|
|||
refreshDevices();
|
||||
}, [refreshDevices]);
|
||||
|
||||
useEffect(() => {
|
||||
const deviceIds = Object.keys(devices);
|
||||
// empty devices means devices have not been fetched yet
|
||||
// as there is always at least the current device
|
||||
if (deviceIds.length) {
|
||||
pruneClientInformation(deviceIds, matrixClient);
|
||||
}
|
||||
}, [devices, matrixClient]);
|
||||
|
||||
useEventEmitter(matrixClient, CryptoEvent.DevicesUpdated, (users: string[]): void => {
|
||||
if (users.includes(userId)) {
|
||||
refreshDevices();
|
||||
|
|
|
|||
|
|
@ -40,8 +40,8 @@ const formatUrl = (): string | undefined => {
|
|||
].join("");
|
||||
};
|
||||
|
||||
export const getClientInformationEventType = (deviceId: string): string =>
|
||||
`io.element.matrix_client_information.${deviceId}`;
|
||||
const clientInformationEventPrefix = "io.element.matrix_client_information.";
|
||||
export const getClientInformationEventType = (deviceId: string): string => `${clientInformationEventPrefix}${deviceId}`;
|
||||
|
||||
/**
|
||||
* Record extra client information for the current device
|
||||
|
|
@ -52,7 +52,7 @@ export const recordClientInformation = async (
|
|||
sdkConfig: IConfigOptions,
|
||||
platform: BasePlatform,
|
||||
): Promise<void> => {
|
||||
const deviceId = matrixClient.getDeviceId();
|
||||
const deviceId = matrixClient.getDeviceId()!;
|
||||
const { brand } = sdkConfig;
|
||||
const version = await platform.getAppVersion();
|
||||
const type = getClientInformationEventType(deviceId);
|
||||
|
|
@ -66,12 +66,27 @@ export const recordClientInformation = async (
|
|||
};
|
||||
|
||||
/**
|
||||
* Remove extra client information
|
||||
* @todo(kerrya) revisit after MSC3391: account data deletion is done
|
||||
* (PSBE-12)
|
||||
* Remove client information events for devices that no longer exist
|
||||
* @param validDeviceIds - ids of current devices,
|
||||
* client information for devices NOT in this list will be removed
|
||||
*/
|
||||
export const pruneClientInformation = (validDeviceIds: string[], matrixClient: MatrixClient): void => {
|
||||
Object.values(matrixClient.store.accountData).forEach((event) => {
|
||||
if (!event.getType().startsWith(clientInformationEventPrefix)) {
|
||||
return;
|
||||
}
|
||||
const [, deviceId] = event.getType().split(clientInformationEventPrefix);
|
||||
if (deviceId && !validDeviceIds.includes(deviceId)) {
|
||||
matrixClient.deleteAccountData(event.getType());
|
||||
}
|
||||
});
|
||||
};
|
||||
|
||||
/**
|
||||
* Remove extra client information for current device
|
||||
*/
|
||||
export const removeClientInformation = async (matrixClient: MatrixClient): Promise<void> => {
|
||||
const deviceId = matrixClient.getDeviceId();
|
||||
const deviceId = matrixClient.getDeviceId()!;
|
||||
const type = getClientInformationEventType(deviceId);
|
||||
const clientInformation = getDeviceClientInformation(matrixClient, deviceId);
|
||||
|
||||
|
|
|
|||
|
|
@ -46,6 +46,7 @@ import LogoutDialog from "../../../../../../src/components/views/dialogs/LogoutD
|
|||
import { DeviceSecurityVariation, ExtendedDevice } from "../../../../../../src/components/views/settings/devices/types";
|
||||
import { INACTIVE_DEVICE_AGE_MS } from "../../../../../../src/components/views/settings/devices/filter";
|
||||
import SettingsStore from "../../../../../../src/settings/SettingsStore";
|
||||
import { getClientInformationEventType } from "../../../../../../src/utils/device/clientInformation";
|
||||
|
||||
mockPlatformPeg();
|
||||
|
||||
|
|
@ -87,6 +88,7 @@ describe("<SessionManagerTab />", () => {
|
|||
generateClientSecret: jest.fn(),
|
||||
setDeviceDetails: jest.fn(),
|
||||
getAccountData: jest.fn(),
|
||||
deleteAccountData: jest.fn(),
|
||||
doesServerSupportUnstableFeature: jest.fn().mockResolvedValue(true),
|
||||
getPushers: jest.fn(),
|
||||
setPusher: jest.fn(),
|
||||
|
|
@ -182,6 +184,9 @@ describe("<SessionManagerTab />", () => {
|
|||
],
|
||||
});
|
||||
|
||||
// @ts-ignore mock
|
||||
mockClient.store = { accountData: {} };
|
||||
|
||||
mockClient.getAccountData.mockReset().mockImplementation((eventType) => {
|
||||
if (eventType.startsWith(LOCAL_NOTIFICATION_SETTINGS_PREFIX.name)) {
|
||||
return new MatrixEvent({
|
||||
|
|
@ -667,6 +672,47 @@ describe("<SessionManagerTab />", () => {
|
|||
);
|
||||
});
|
||||
|
||||
it("removes account data events for devices after sign out", async () => {
|
||||
const mobileDeviceClientInfo = new MatrixEvent({
|
||||
type: getClientInformationEventType(alicesMobileDevice.device_id),
|
||||
content: {
|
||||
name: "test",
|
||||
},
|
||||
});
|
||||
// @ts-ignore setup mock
|
||||
mockClient.store = {
|
||||
// @ts-ignore setup mock
|
||||
accountData: {
|
||||
[mobileDeviceClientInfo.getType()]: mobileDeviceClientInfo,
|
||||
},
|
||||
};
|
||||
|
||||
mockClient.getDevices
|
||||
.mockResolvedValueOnce({
|
||||
devices: [alicesDevice, alicesMobileDevice, alicesOlderMobileDevice],
|
||||
})
|
||||
.mockResolvedValueOnce({
|
||||
// refreshed devices after sign out
|
||||
devices: [alicesDevice],
|
||||
});
|
||||
|
||||
const { getByTestId, getByLabelText } = render(getComponent());
|
||||
|
||||
await act(async () => {
|
||||
await flushPromises();
|
||||
});
|
||||
|
||||
expect(mockClient.deleteAccountData).not.toHaveBeenCalled();
|
||||
|
||||
fireEvent.click(getByTestId("current-session-menu"));
|
||||
fireEvent.click(getByLabelText("Sign out of all other sessions (2)"));
|
||||
await confirmSignout(getByTestId);
|
||||
|
||||
// only called once for signed out device with account data event
|
||||
expect(mockClient.deleteAccountData).toHaveBeenCalledTimes(1);
|
||||
expect(mockClient.deleteAccountData).toHaveBeenCalledWith(mobileDeviceClientInfo.getType());
|
||||
});
|
||||
|
||||
describe("other devices", () => {
|
||||
const interactiveAuthError = { httpStatus: 401, data: { flows: [{ stages: ["m.login.password"] }] } };
|
||||
|
||||
|
|
|
|||
Loading…
Reference in a new issue