From 96c6217a83e4ba18017fc51a0e663bccb06e692b Mon Sep 17 00:00:00 2001 From: David Baker Date: Wed, 20 Sep 2023 16:25:02 +0100 Subject: [PATCH 1/4] Fix race where app would be opened with no e2ee key The key hadn't been extractwed from the URL at the point the modal was mounted, so it just didn't get the key. --- src/e2ee/sharedKeyManagement.ts | 33 ++++++++++++++++++++++----------- 1 file changed, 22 insertions(+), 11 deletions(-) diff --git a/src/e2ee/sharedKeyManagement.ts b/src/e2ee/sharedKeyManagement.ts index 2af0b463..030cb0cb 100644 --- a/src/e2ee/sharedKeyManagement.ts +++ b/src/e2ee/sharedKeyManagement.ts @@ -25,7 +25,7 @@ import { widget } from "../widget"; export const getRoomSharedKeyLocalStorageKey = (roomId: string): string => `room-shared-key-${roomId}`; -export const useInternalRoomSharedKey = ( +const useInternalRoomSharedKey = ( roomId: string ): [string | null, (value: string) => void] => { const key = useMemo(() => getRoomSharedKeyLocalStorageKey(roomId), [roomId]); @@ -35,34 +35,45 @@ export const useInternalRoomSharedKey = ( return [e2eeEnabled ? roomSharedKey : null, setRoomSharedKey]; }; +const useKeyFromUrl = (roomId: string) => { + const urlParams = useUrlParams(); + const [e2eeSharedKey, setE2EESharedKey] = useInternalRoomSharedKey(roomId); + + useEffect(() => { + if (!urlParams.password) return; + if (urlParams.password === "") return; + if (urlParams.password === e2eeSharedKey) return; + + setE2EESharedKey(urlParams.password); + }, [urlParams, e2eeSharedKey, setE2EESharedKey]); +}; + export const useRoomSharedKey = (roomId: string): string | null => { + // make sure we've extracted the key from the URL first + useKeyFromUrl(roomId); + return useInternalRoomSharedKey(roomId)[0]; }; export const useManageRoomSharedKey = (roomId: string): string | null => { - const { password } = useUrlParams(); - const [e2eeSharedKey, setE2EESharedKey] = useInternalRoomSharedKey(roomId); + const urlParams = useUrlParams(); - useEffect(() => { - if (!password) return; - if (password === "") return; - if (password === e2eeSharedKey) return; + useKeyFromUrl(roomId); - setE2EESharedKey(password); - }, [password, e2eeSharedKey, setE2EESharedKey]); + const [e2eeSharedKey] = useInternalRoomSharedKey(roomId); useEffect(() => { const hash = location.hash; if (!hash.includes("?")) return; if (!hash.includes(PASSWORD_STRING)) return; - if (password !== e2eeSharedKey) return; + if (urlParams.password !== e2eeSharedKey) return; const [hashStart, passwordStart] = hash.split(PASSWORD_STRING); const hashEnd = passwordStart.split("&")[1]; location.replace((hashStart ?? "") + (hashEnd ?? "")); - }, [password, e2eeSharedKey]); + }, [urlParams, e2eeSharedKey]); return e2eeSharedKey; }; From 4f4340229966ecadd8ab193a1de7cbb482fd0183 Mon Sep 17 00:00:00 2001 From: David Baker Date: Wed, 20 Sep 2023 16:29:46 +0100 Subject: [PATCH 2/4] Log an error if we don't have the key when generating a url for en e2ee room --- src/room/AppSelectionModal.tsx | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/room/AppSelectionModal.tsx b/src/room/AppSelectionModal.tsx index 80e2871b..10081189 100644 --- a/src/room/AppSelectionModal.tsx +++ b/src/room/AppSelectionModal.tsx @@ -18,9 +18,10 @@ import { FC, MouseEvent, useCallback, useMemo, useState } from "react"; import { useTranslation } from "react-i18next"; import { Button, Text } from "@vector-im/compound-web"; import { ReactComponent as PopOutIcon } from "@vector-im/compound-design-tokens/icons/pop-out.svg"; +import { logger } from "matrix-js-sdk/src/logger"; import { Modal } from "../Modal"; -import { useRoomSharedKey } from "../e2ee/sharedKeyManagement"; +import { useIsRoomE2EE, useRoomSharedKey } from "../e2ee/sharedKeyManagement"; import { getAbsoluteRoomUrl } from "../matrix-utils"; import styles from "./AppSelectionModal.module.css"; import { editFragmentQuery } from "../UrlParams"; @@ -43,6 +44,13 @@ export const AppSelectionModal: FC = ({ roomId }) => { ); const roomSharedKey = useRoomSharedKey(roomId ?? ""); + const roomIsEncrypted = useIsRoomE2EE(roomId ?? ""); + if (roomIsEncrypted && roomSharedKey === undefined) { + logger.error( + "Generating app redirect URL for encrypted room but don't have key available!" + ); + } + const appUrl = useMemo(() => { // If the room ID is not known, fall back to the URL of the current page // Also, we don't really know the room name at this stage as we haven't From ed0059c898ef429b048736d2fbdcb9b881ed1b96 Mon Sep 17 00:00:00 2001 From: David Baker Date: Wed, 20 Sep 2023 17:05:10 +0100 Subject: [PATCH 3/4] Hopefully actually fix password bug --- src/e2ee/sharedKeyManagement.ts | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/e2ee/sharedKeyManagement.ts b/src/e2ee/sharedKeyManagement.ts index 030cb0cb..08d86e30 100644 --- a/src/e2ee/sharedKeyManagement.ts +++ b/src/e2ee/sharedKeyManagement.ts @@ -35,7 +35,7 @@ const useInternalRoomSharedKey = ( return [e2eeEnabled ? roomSharedKey : null, setRoomSharedKey]; }; -const useKeyFromUrl = (roomId: string) => { +const useKeyFromUrl = (roomId: string): string | null => { const urlParams = useUrlParams(); const [e2eeSharedKey, setE2EESharedKey] = useInternalRoomSharedKey(roomId); @@ -46,13 +46,18 @@ const useKeyFromUrl = (roomId: string) => { setE2EESharedKey(urlParams.password); }, [urlParams, e2eeSharedKey, setE2EESharedKey]); + + return urlParams.password ?? null; }; export const useRoomSharedKey = (roomId: string): string | null => { // make sure we've extracted the key from the URL first - useKeyFromUrl(roomId); + // (and we still need to take the value it returns because + // the effect won't run in time for it to save to localstorage in + // time for us to read it out again). + const passwordFormUrl = useKeyFromUrl(roomId); - return useInternalRoomSharedKey(roomId)[0]; + return useInternalRoomSharedKey(roomId)[0] ?? passwordFormUrl; }; export const useManageRoomSharedKey = (roomId: string): string | null => { From cbf27fc88779406c537f5e73dfec5e7a6ed3a1e1 Mon Sep 17 00:00:00 2001 From: David Baker Date: Wed, 20 Sep 2023 17:40:37 +0100 Subject: [PATCH 4/4] Also use return value password in the other hook --- src/e2ee/sharedKeyManagement.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/e2ee/sharedKeyManagement.ts b/src/e2ee/sharedKeyManagement.ts index 08d86e30..1c7cd4b6 100644 --- a/src/e2ee/sharedKeyManagement.ts +++ b/src/e2ee/sharedKeyManagement.ts @@ -63,7 +63,7 @@ export const useRoomSharedKey = (roomId: string): string | null => { export const useManageRoomSharedKey = (roomId: string): string | null => { const urlParams = useUrlParams(); - useKeyFromUrl(roomId); + const urlPassword = useKeyFromUrl(roomId); const [e2eeSharedKey] = useInternalRoomSharedKey(roomId); @@ -80,7 +80,7 @@ export const useManageRoomSharedKey = (roomId: string): string | null => { location.replace((hashStart ?? "") + (hashEnd ?? "")); }, [urlParams, e2eeSharedKey]); - return e2eeSharedKey; + return e2eeSharedKey ?? urlPassword; }; export const useIsRoomE2EE = (roomId: string): boolean | null => {