Merge pull request #1557 from vector-im/dbkr/fix_password_race

Fix race where app would be opened with no e2ee key

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,36 +35,52 @@ export const useInternalRoomSharedKey = (
   return [e2eeEnabled ? roomSharedKey : null, setRoomSharedKey];
 };
 
-export const useRoomSharedKey = (roomId: string): string | null => {
+const useKeyFromUrl = (roomId: string): string | null => {
-  return useInternalRoomSharedKey(roomId)[0];
+  const urlParams = useUrlParams();
-};
-
-export const useManageRoomSharedKey = (roomId: string): string | null => {
-  const { password } = useUrlParams();
   const [e2eeSharedKey, setE2EESharedKey] = useInternalRoomSharedKey(roomId);
 
   useEffect(() => {
-    if (!password) return;
+    if (!urlParams.password) return;
-    if (password === "") return;
+    if (urlParams.password === "") return;
-    if (password === e2eeSharedKey) return;
+    if (urlParams.password === e2eeSharedKey) return;
 
-    setE2EESharedKey(password);
+    setE2EESharedKey(urlParams.password);
-  }, [password, e2eeSharedKey, setE2EESharedKey]);
+  }, [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
+  // (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] ?? passwordFormUrl;
+};
+
+export const useManageRoomSharedKey = (roomId: string): string | null => {
+  const urlParams = useUrlParams();
+
+  const urlPassword = useKeyFromUrl(roomId);
+
+  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("&").slice(1).join("&");
 
     location.replace((hashStart ?? "") + (hashEnd ?? ""));
-  }, [password, e2eeSharedKey]);
+  }, [urlParams, e2eeSharedKey]);
 
-  return e2eeSharedKey;
+  return e2eeSharedKey ?? urlPassword;
 };
 
 export const useIsRoomE2EE = (roomId: string): boolean | null => {

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<Props> = ({ 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