From d7b33ee959ed74238f679b043ff52be87a4ddd70 Mon Sep 17 00:00:00 2001 From: David Baker Date: Mon, 9 Oct 2023 17:43:50 +0100 Subject: [PATCH 1/5] Always store room passwords with the right room ID Take the room ID from the URL rather than just assuming it's still the one that was in URL params before: if only the hash changes, the app won't reload. Fixes https://github.com/vector-im/element-call/issues/1708 --- src/e2ee/sharedKeyManagement.ts | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/src/e2ee/sharedKeyManagement.ts b/src/e2ee/sharedKeyManagement.ts index 06183878..61f26373 100644 --- a/src/e2ee/sharedKeyManagement.ts +++ b/src/e2ee/sharedKeyManagement.ts @@ -17,7 +17,7 @@ limitations under the License. import { useEffect, useMemo } from "react"; import { useEnableE2EE } from "../settings/useSetting"; -import { useLocalStorage } from "../useLocalStorage"; +import { setLocalStorageItem, useLocalStorage } from "../useLocalStorage"; import { useClient } from "../ClientContext"; import { useUrlParams } from "../UrlParams"; import { widget } from "../widget"; @@ -25,27 +25,29 @@ import { widget } from "../widget"; export const getRoomSharedKeyLocalStorageKey = (roomId: string): string => `room-shared-key-${roomId}`; -const useInternalRoomSharedKey = ( - roomId: string -): [string | null, (value: string) => void] => { +const useInternalRoomSharedKey = (roomId: string): string | null => { const key = useMemo(() => getRoomSharedKeyLocalStorageKey(roomId), [roomId]); const [e2eeEnabled] = useEnableE2EE(); - const [roomSharedKey, setRoomSharedKey] = useLocalStorage(key); + const roomSharedKey = useLocalStorage(key)[0]; - return [e2eeEnabled ? roomSharedKey : null, setRoomSharedKey]; + return e2eeEnabled ? roomSharedKey : null; }; const useKeyFromUrl = (roomId: string): string | null => { const urlParams = useUrlParams(); - const [e2eeSharedKey, setE2EESharedKey] = useInternalRoomSharedKey(roomId); + const e2eeSharedKey = useInternalRoomSharedKey(roomId); useEffect(() => { if (!urlParams.password) return; if (urlParams.password === "") return; if (urlParams.password === e2eeSharedKey) return; + if (!urlParams.roomId) return; - setE2EESharedKey(urlParams.password); - }, [urlParams, e2eeSharedKey, setE2EESharedKey]); + setLocalStorageItem( + getRoomSharedKeyLocalStorageKey(urlParams.roomId), + urlParams.password + ); + }, [urlParams, e2eeSharedKey]); return urlParams.password ?? null; }; @@ -57,7 +59,7 @@ export const useRoomSharedKey = (roomId: string): string | null => { // time for us to read it out again). const passwordFormUrl = useKeyFromUrl(roomId); - return useInternalRoomSharedKey(roomId)[0] ?? passwordFormUrl; + return useInternalRoomSharedKey(roomId) ?? passwordFormUrl; }; export const useIsRoomE2EE = (roomId: string): boolean | null => { From 51f87fa42ab8b1e5b8dca0b575de169b9c4e9bc5 Mon Sep 17 00:00:00 2001 From: David Baker Date: Tue, 10 Oct 2023 17:06:49 +0100 Subject: [PATCH 2/5] Add comment Co-authored-by: Timo <16718859+toger5@users.noreply.github.com> --- src/e2ee/sharedKeyManagement.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/e2ee/sharedKeyManagement.ts b/src/e2ee/sharedKeyManagement.ts index 61f26373..ce0845dc 100644 --- a/src/e2ee/sharedKeyManagement.ts +++ b/src/e2ee/sharedKeyManagement.ts @@ -44,6 +44,9 @@ const useKeyFromUrl = (roomId: string): string | null => { if (!urlParams.roomId) return; setLocalStorageItem( + // We set the Item by only using data from the url. This way we + // make sure, we always have matching pairs in the LocalStorage, + // as they occur in the call links. getRoomSharedKeyLocalStorageKey(urlParams.roomId), urlParams.password ); From 6039253a3228c03bfb5377df67698a7e9056f796 Mon Sep 17 00:00:00 2001 From: David Baker Date: Wed, 11 Oct 2023 12:53:33 +0100 Subject: [PATCH 3/5] Reafctor a bit --- src/e2ee/sharedKeyManagement.ts | 28 +++++++++++++++++++--------- 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/src/e2ee/sharedKeyManagement.ts b/src/e2ee/sharedKeyManagement.ts index ce0845dc..1e06dffc 100644 --- a/src/e2ee/sharedKeyManagement.ts +++ b/src/e2ee/sharedKeyManagement.ts @@ -26,21 +26,25 @@ export const getRoomSharedKeyLocalStorageKey = (roomId: string): string => `room-shared-key-${roomId}`; const useInternalRoomSharedKey = (roomId: string): string | null => { - const key = useMemo(() => getRoomSharedKeyLocalStorageKey(roomId), [roomId]); + const key = getRoomSharedKeyLocalStorageKey(roomId); const [e2eeEnabled] = useEnableE2EE(); const roomSharedKey = useLocalStorage(key)[0]; return e2eeEnabled ? roomSharedKey : null; }; -const useKeyFromUrl = (roomId: string): string | null => { +/** + * Extracts the room password from the URL if one is present, saving it in localstorage + * and returning it in a tuple with the corresponding room ID from the URL. + * @returns A tuple of the roomId and password from the URL if the URL has both, + * otherwise [undefined, undefined] + */ +const useKeyFromUrl = (): [string, string] | [undefined, undefined] => { const urlParams = useUrlParams(); - const e2eeSharedKey = useInternalRoomSharedKey(roomId); useEffect(() => { if (!urlParams.password) return; if (urlParams.password === "") return; - if (urlParams.password === e2eeSharedKey) return; if (!urlParams.roomId) return; setLocalStorageItem( @@ -50,19 +54,25 @@ const useKeyFromUrl = (roomId: string): string | null => { getRoomSharedKeyLocalStorageKey(urlParams.roomId), urlParams.password ); - }, [urlParams, e2eeSharedKey]); + }, [urlParams]); - return urlParams.password ?? null; + return urlParams.roomId && urlParams.password + ? [urlParams.roomId, urlParams.password] + : [undefined, undefined]; }; -export const useRoomSharedKey = (roomId: string): string | null => { +export const useRoomSharedKey = (roomId: string): string | undefined => { // make sure we've extracted the key from the URL first // (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); + const [urlRoomId, passwordFormUrl] = useKeyFromUrl(); - return useInternalRoomSharedKey(roomId) ?? passwordFormUrl; + const storedPassword = useInternalRoomSharedKey(roomId); + + if (storedPassword) return storedPassword; + if (urlRoomId === roomId) return passwordFormUrl; + return undefined; }; export const useIsRoomE2EE = (roomId: string): boolean | null => { From 9d4ade97b0bc65cfe01297dccc70b846ac1a76a8 Mon Sep 17 00:00:00 2001 From: David Baker Date: Wed, 11 Oct 2023 16:10:03 +0100 Subject: [PATCH 4/5] Remove redundant check Co-authored-by: Timo <16718859+toger5@users.noreply.github.com> --- src/e2ee/sharedKeyManagement.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/e2ee/sharedKeyManagement.ts b/src/e2ee/sharedKeyManagement.ts index 1e06dffc..e8e75b55 100644 --- a/src/e2ee/sharedKeyManagement.ts +++ b/src/e2ee/sharedKeyManagement.ts @@ -43,8 +43,7 @@ const useKeyFromUrl = (): [string, string] | [undefined, undefined] => { const urlParams = useUrlParams(); useEffect(() => { - if (!urlParams.password) return; - if (urlParams.password === "") return; + if (!urlParams.password || !urlParams.roomId) return; if (!urlParams.roomId) return; setLocalStorageItem( From d058f08c4795462689112c47e0518ea2c095a1bc Mon Sep 17 00:00:00 2001 From: David Baker Date: Wed, 11 Oct 2023 16:25:47 +0100 Subject: [PATCH 5/5] Prettier --- 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 e8e75b55..8d4dc83e 100644 --- a/src/e2ee/sharedKeyManagement.ts +++ b/src/e2ee/sharedKeyManagement.ts @@ -51,7 +51,7 @@ const useKeyFromUrl = (): [string, string] | [undefined, undefined] => { // make sure, we always have matching pairs in the LocalStorage, // as they occur in the call links. getRoomSharedKeyLocalStorageKey(urlParams.roomId), - urlParams.password + urlParams.password, ); }, [urlParams]); @@ -82,6 +82,6 @@ export const useIsRoomE2EE = (roomId: string): boolean | null => { // should inspect the e2eEnabled URL parameter here? return useMemo( () => widget === null && (room === null || !room.getCanonicalAlias()), - [room] + [room], ); };