From 69817ca17a19ffeced9ec0ed89f4ef9fa6706757 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Thu, 11 Apr 2024 16:24:54 -0600 Subject: [PATCH 01/21] Typescriptify & use service worker for MSC3916 authentication --- res/sw.js | 1 - src/serviceworker/index.ts | 77 ++++++++++++++++++++++++++++++ src/vector/platform/WebPlatform.ts | 5 +- webpack.config.js | 7 ++- 4 files changed, 86 insertions(+), 4 deletions(-) delete mode 100644 res/sw.js create mode 100644 src/serviceworker/index.ts diff --git a/res/sw.js b/res/sw.js deleted file mode 100644 index 1fdf7324e17..00000000000 --- a/res/sw.js +++ /dev/null @@ -1 +0,0 @@ -self.addEventListener("fetch", () => {}); diff --git a/src/serviceworker/index.ts b/src/serviceworker/index.ts new file mode 100644 index 00000000000..7d7de1da4c4 --- /dev/null +++ b/src/serviceworker/index.ts @@ -0,0 +1,77 @@ +const serverSupportMap: { + [serverUrl: string]: { + supportsMSC3916: boolean, + cacheExpires: number, + }, +} = {}; + +const credentialStore: { + [serverUrl: string]: string, +} = {}; + +// We skipWaiting() to update the service worker more frequently, particularly in development environments. +// @ts-expect-error - service worker types are not available. See 'fetch' event handler. +skipWaiting(); + +self.addEventListener("message", (event) => { + if (event.data?.type !== "credentials") return; // ignore + credentialStore[event.data.homeserverUrl] = event.data.accessToken; + console.log(`[Service Worker] Updated access token for ${event.data.homeserverUrl} (accessToken? ${Boolean(event.data.accessToken)})`); +}); + +// @ts-expect-error - getting types to work for this is difficult, so we anticipate that "addEventListener" doesn't +// have a valid signature. +self.addEventListener("fetch", (event: FetchEvent) => { + // This is the authenticated media (MSC3916) check, proxying what was unauthenticated to the authenticated variants. + + if (event.request.method !== "GET") { + return; // not important to us + } + + // Note: ideally we'd keep the request headers and etc, but in practice we can't even see those details. + // See https://stackoverflow.com/a/59152482 + let url = event.request.url; + + // We only intercept v3 download and thumbnail requests as presumably everything else is deliberate. + // For example, `/_matrix/media/unstable` or `/_matrix/media/v3/preview_url` are something well within + // the control of the application, and appear to be choices made at a higher level than us. + if (url.includes("/_matrix/media/v3/download") || url.includes("/_matrix/media/v3/thumbnail")) { + // We need to call respondWith synchronously, otherwise we may never execute properly. This means + // later on we need to proxy the request through if it turns out the server doesn't support authentication. + event.respondWith((async (): Promise => { + // Figure out which homeserver we're communicating with + const csApi = url.substring(0, url.indexOf("/_matrix/media/v3")); + + // Locate our access token, and populate the fetchConfig with the authentication header. + const accessToken = credentialStore[csApi]; + let fetchConfig: {headers?: {[key: string]: string}} = {}; + if (accessToken) { + fetchConfig = { + headers: { + Authorization: `Bearer ${accessToken}`, + }, + }; + } + + // Update or populate the server support map using a (usually) authenticated `/versions` call. + if (!serverSupportMap[csApi] || serverSupportMap[csApi].cacheExpires <= (new Date()).getTime()) { + const versions = await (await fetch(`${csApi}/_matrix/client/versions`, fetchConfig)).json(); + serverSupportMap[csApi] = { + supportsMSC3916: Boolean(versions?.unstable_features?.["org.matrix.msc3916"]), + cacheExpires: (new Date()).getTime() + (2 * 60 * 60 * 1000), // 2 hours from now + }; + } + + // If we have server support (and a means of authentication), rewrite the URL to use MSC3916 endpoints. + if (serverSupportMap[csApi].supportsMSC3916 && accessToken) { + // Currently unstable only. + url = url.replace(/\/media\/v3\/(.*)\//, "/client/unstable/org.matrix.msc3916/media/$1/"); + } // else by default we make no changes + + // Add authentication and send the request. We add authentication even if MSC3916 endpoints aren't + // being used to ensure patches like this work: + // https://github.com/matrix-org/synapse/commit/2390b66bf0ec3ff5ffb0c7333f3c9b239eeb92bb + return fetch(url, fetchConfig); + })()); + } +}); diff --git a/src/vector/platform/WebPlatform.ts b/src/vector/platform/WebPlatform.ts index 39446158fda..1a734b3c431 100644 --- a/src/vector/platform/WebPlatform.ts +++ b/src/vector/platform/WebPlatform.ts @@ -46,7 +46,10 @@ export default class WebPlatform extends VectorBasePlatform { super(); // Register service worker if available on this platform if ("serviceWorker" in navigator) { - navigator.serviceWorker.register("sw.js"); + // sw.js is exported by webpack, sourced from `/src/serviceworker/index.ts` + navigator.serviceWorker.register("sw.js") + .then(r => r.update()) + .catch(e => console.error("Error registering/updating service worker:", e)); } } diff --git a/webpack.config.js b/webpack.config.js index 3b827c8d95d..9ea4d10317c 100644 --- a/webpack.config.js +++ b/webpack.config.js @@ -153,6 +153,10 @@ module.exports = (env, argv) => { mobileguide: "./src/vector/mobile_guide/index.ts", jitsi: "./src/vector/jitsi/index.ts", usercontent: "./node_modules/matrix-react-sdk/src/usercontent/index.ts", + serviceworker: { + import: "./src/serviceworker/index.ts", + filename: "sw.js", // update WebPlatform if this changes + }, ...(useHMR ? {} : cssThemes), }, @@ -666,7 +670,7 @@ module.exports = (env, argv) => { // HtmlWebpackPlugin will screw up our formatting like the names // of the themes and which chunks we actually care about. inject: false, - excludeChunks: ["mobileguide", "usercontent", "jitsi"], + excludeChunks: ["mobileguide", "usercontent", "jitsi", "serviceworker"], minify: false, templateParameters: { og_image_url: ogImageUrl, @@ -739,7 +743,6 @@ module.exports = (env, argv) => { "res/jitsi_external_api.min.js", "res/jitsi_external_api.min.js.LICENSE.txt", "res/manifest.json", - "res/sw.js", "res/welcome.html", { from: "welcome/**", context: path.resolve(__dirname, "res") }, { from: "themes/**", context: path.resolve(__dirname, "res") }, From 296c82c645b8d822060072654719ab7635c5c591 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Thu, 11 Apr 2024 16:31:00 -0600 Subject: [PATCH 02/21] appease the linter --- src/serviceworker/index.ts | 76 ++++++++++++++++-------------- src/vector/platform/WebPlatform.ts | 7 +-- 2 files changed, 44 insertions(+), 39 deletions(-) diff --git a/src/serviceworker/index.ts b/src/serviceworker/index.ts index 7d7de1da4c4..169e68ede87 100644 --- a/src/serviceworker/index.ts +++ b/src/serviceworker/index.ts @@ -1,12 +1,12 @@ const serverSupportMap: { [serverUrl: string]: { - supportsMSC3916: boolean, - cacheExpires: number, - }, + supportsMSC3916: boolean; + cacheExpires: number; + }; } = {}; const credentialStore: { - [serverUrl: string]: string, + [serverUrl: string]: string; } = {}; // We skipWaiting() to update the service worker more frequently, particularly in development environments. @@ -16,7 +16,9 @@ skipWaiting(); self.addEventListener("message", (event) => { if (event.data?.type !== "credentials") return; // ignore credentialStore[event.data.homeserverUrl] = event.data.accessToken; - console.log(`[Service Worker] Updated access token for ${event.data.homeserverUrl} (accessToken? ${Boolean(event.data.accessToken)})`); + console.log( + `[Service Worker] Updated access token for ${event.data.homeserverUrl} (accessToken? ${Boolean(event.data.accessToken)})`, + ); }); // @ts-expect-error - getting types to work for this is difficult, so we anticipate that "addEventListener" doesn't @@ -38,40 +40,42 @@ self.addEventListener("fetch", (event: FetchEvent) => { if (url.includes("/_matrix/media/v3/download") || url.includes("/_matrix/media/v3/thumbnail")) { // We need to call respondWith synchronously, otherwise we may never execute properly. This means // later on we need to proxy the request through if it turns out the server doesn't support authentication. - event.respondWith((async (): Promise => { - // Figure out which homeserver we're communicating with - const csApi = url.substring(0, url.indexOf("/_matrix/media/v3")); + event.respondWith( + (async (): Promise => { + // Figure out which homeserver we're communicating with + const csApi = url.substring(0, url.indexOf("/_matrix/media/v3")); - // Locate our access token, and populate the fetchConfig with the authentication header. - const accessToken = credentialStore[csApi]; - let fetchConfig: {headers?: {[key: string]: string}} = {}; - if (accessToken) { - fetchConfig = { - headers: { - Authorization: `Bearer ${accessToken}`, - }, - }; - } + // Locate our access token, and populate the fetchConfig with the authentication header. + const accessToken = credentialStore[csApi]; + let fetchConfig: { headers?: { [key: string]: string } } = {}; + if (accessToken) { + fetchConfig = { + headers: { + Authorization: `Bearer ${accessToken}`, + }, + }; + } - // Update or populate the server support map using a (usually) authenticated `/versions` call. - if (!serverSupportMap[csApi] || serverSupportMap[csApi].cacheExpires <= (new Date()).getTime()) { - const versions = await (await fetch(`${csApi}/_matrix/client/versions`, fetchConfig)).json(); - serverSupportMap[csApi] = { - supportsMSC3916: Boolean(versions?.unstable_features?.["org.matrix.msc3916"]), - cacheExpires: (new Date()).getTime() + (2 * 60 * 60 * 1000), // 2 hours from now - }; - } + // Update or populate the server support map using a (usually) authenticated `/versions` call. + if (!serverSupportMap[csApi] || serverSupportMap[csApi].cacheExpires <= new Date().getTime()) { + const versions = await (await fetch(`${csApi}/_matrix/client/versions`, fetchConfig)).json(); + serverSupportMap[csApi] = { + supportsMSC3916: Boolean(versions?.unstable_features?.["org.matrix.msc3916"]), + cacheExpires: new Date().getTime() + 2 * 60 * 60 * 1000, // 2 hours from now + }; + } - // If we have server support (and a means of authentication), rewrite the URL to use MSC3916 endpoints. - if (serverSupportMap[csApi].supportsMSC3916 && accessToken) { - // Currently unstable only. - url = url.replace(/\/media\/v3\/(.*)\//, "/client/unstable/org.matrix.msc3916/media/$1/"); - } // else by default we make no changes + // If we have server support (and a means of authentication), rewrite the URL to use MSC3916 endpoints. + if (serverSupportMap[csApi].supportsMSC3916 && accessToken) { + // Currently unstable only. + url = url.replace(/\/media\/v3\/(.*)\//, "/client/unstable/org.matrix.msc3916/media/$1/"); + } // else by default we make no changes - // Add authentication and send the request. We add authentication even if MSC3916 endpoints aren't - // being used to ensure patches like this work: - // https://github.com/matrix-org/synapse/commit/2390b66bf0ec3ff5ffb0c7333f3c9b239eeb92bb - return fetch(url, fetchConfig); - })()); + // Add authentication and send the request. We add authentication even if MSC3916 endpoints aren't + // being used to ensure patches like this work: + // https://github.com/matrix-org/synapse/commit/2390b66bf0ec3ff5ffb0c7333f3c9b239eeb92bb + return fetch(url, fetchConfig); + })(), + ); } }); diff --git a/src/vector/platform/WebPlatform.ts b/src/vector/platform/WebPlatform.ts index 1a734b3c431..0eb268e0c0a 100644 --- a/src/vector/platform/WebPlatform.ts +++ b/src/vector/platform/WebPlatform.ts @@ -47,9 +47,10 @@ export default class WebPlatform extends VectorBasePlatform { // Register service worker if available on this platform if ("serviceWorker" in navigator) { // sw.js is exported by webpack, sourced from `/src/serviceworker/index.ts` - navigator.serviceWorker.register("sw.js") - .then(r => r.update()) - .catch(e => console.error("Error registering/updating service worker:", e)); + navigator.serviceWorker + .register("sw.js") + .then((r) => r.update()) + .catch((e) => console.error("Error registering/updating service worker:", e)); } } From 8542ce2848c737bd01af5c0c3eaea0bf4acb2d17 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Thu, 11 Apr 2024 16:44:32 -0600 Subject: [PATCH 03/21] appease jest --- src/vector/platform/WebPlatform.ts | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/vector/platform/WebPlatform.ts b/src/vector/platform/WebPlatform.ts index 0eb268e0c0a..e25c90360e1 100644 --- a/src/vector/platform/WebPlatform.ts +++ b/src/vector/platform/WebPlatform.ts @@ -47,10 +47,13 @@ export default class WebPlatform extends VectorBasePlatform { // Register service worker if available on this platform if ("serviceWorker" in navigator) { // sw.js is exported by webpack, sourced from `/src/serviceworker/index.ts` - navigator.serviceWorker - .register("sw.js") - .then((r) => r.update()) - .catch((e) => console.error("Error registering/updating service worker:", e)); + const swPromise = navigator.serviceWorker.register("sw.js"); + + // Jest causes `register()` to return undefined, so swallow that case. + if (swPromise) { + swPromise.then((r) => r.update()) + .catch((e) => console.error("Error registering/updating service worker:", e)); + } } } From b333b297d63dae94a4271bd65e3d4c2e375e4b71 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Thu, 11 Apr 2024 16:50:58 -0600 Subject: [PATCH 04/21] appease linter --- src/vector/platform/WebPlatform.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/vector/platform/WebPlatform.ts b/src/vector/platform/WebPlatform.ts index e25c90360e1..1fc6d5d9c8f 100644 --- a/src/vector/platform/WebPlatform.ts +++ b/src/vector/platform/WebPlatform.ts @@ -51,7 +51,8 @@ export default class WebPlatform extends VectorBasePlatform { // Jest causes `register()` to return undefined, so swallow that case. if (swPromise) { - swPromise.then((r) => r.update()) + swPromise + .then((r) => r.update()) .catch((e) => console.error("Error registering/updating service worker:", e)); } } From af1ba39f20c8df20ddd071a5a86c83993fd815a5 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Thu, 18 Apr 2024 16:20:22 -0600 Subject: [PATCH 05/21] Get the access token directly --- src/serviceworker/index.ts | 164 ++++++++++++++++++++++------- src/vector/platform/WebPlatform.ts | 24 ++++- 2 files changed, 148 insertions(+), 40 deletions(-) diff --git a/src/serviceworker/index.ts b/src/serviceworker/index.ts index 169e68ede87..c8456b0b075 100644 --- a/src/serviceworker/index.ts +++ b/src/serviceworker/index.ts @@ -1,3 +1,23 @@ +/* +Copyright 2024 New Vector Ltd + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +import { idbLoad } from "matrix-react-sdk/src/utils/StorageAccess"; +import { ACCESS_TOKEN_IV, tryDecryptToken } from "matrix-react-sdk/src/utils/tokens/tokens"; +import { encodeUnpaddedBase64 } from "matrix-js-sdk/src/base64"; + const serverSupportMap: { [serverUrl: string]: { supportsMSC3916: boolean; @@ -5,20 +25,16 @@ const serverSupportMap: { }; } = {}; -const credentialStore: { - [serverUrl: string]: string; -} = {}; - -// We skipWaiting() to update the service worker more frequently, particularly in development environments. -// @ts-expect-error - service worker types are not available. See 'fetch' event handler. -skipWaiting(); +self.addEventListener("install", (event) => { + // We skipWaiting() to update the service worker more frequently, particularly in development environments. + // @ts-expect-error - service worker types are not available. See 'fetch' event handler. + event.waitUntil(skipWaiting()); +}); -self.addEventListener("message", (event) => { - if (event.data?.type !== "credentials") return; // ignore - credentialStore[event.data.homeserverUrl] = event.data.accessToken; - console.log( - `[Service Worker] Updated access token for ${event.data.homeserverUrl} (accessToken? ${Boolean(event.data.accessToken)})`, - ); +self.addEventListener("activate", (event) => { + // We force all clients to be under our control, immediately. This could be old tabs. + // @ts-expect-error - service worker types are not available. See 'fetch' event handler. + event.waitUntil(clients.claim()); }); // @ts-expect-error - getting types to work for this is difficult, so we anticipate that "addEventListener" doesn't @@ -42,34 +58,40 @@ self.addEventListener("fetch", (event: FetchEvent) => { // later on we need to proxy the request through if it turns out the server doesn't support authentication. event.respondWith( (async (): Promise => { - // Figure out which homeserver we're communicating with - const csApi = url.substring(0, url.indexOf("/_matrix/media/v3")); - - // Locate our access token, and populate the fetchConfig with the authentication header. - const accessToken = credentialStore[csApi]; let fetchConfig: { headers?: { [key: string]: string } } = {}; - if (accessToken) { - fetchConfig = { - headers: { - Authorization: `Bearer ${accessToken}`, - }, - }; - } + try { + // Figure out which homeserver we're communicating with + const csApi = url.substring(0, url.indexOf("/_matrix/media/v3")); - // Update or populate the server support map using a (usually) authenticated `/versions` call. - if (!serverSupportMap[csApi] || serverSupportMap[csApi].cacheExpires <= new Date().getTime()) { - const versions = await (await fetch(`${csApi}/_matrix/client/versions`, fetchConfig)).json(); - serverSupportMap[csApi] = { - supportsMSC3916: Boolean(versions?.unstable_features?.["org.matrix.msc3916"]), - cacheExpires: new Date().getTime() + 2 * 60 * 60 * 1000, // 2 hours from now - }; - } + // Locate our access token, and populate the fetchConfig with the authentication header. + // @ts-expect-error - service worker types are not available. See 'fetch' event handler. + const client = await self.clients.get(event.clientId); + const accessToken = await getAccessToken(client); + if (accessToken) { + fetchConfig = { + headers: { + Authorization: `Bearer ${accessToken}`, + }, + }; + } + + // Update or populate the server support map using a (usually) authenticated `/versions` call. + if (!serverSupportMap[csApi] || serverSupportMap[csApi].cacheExpires <= new Date().getTime()) { + const versions = await (await fetch(`${csApi}/_matrix/client/versions`, fetchConfig)).json(); + serverSupportMap[csApi] = { + supportsMSC3916: Boolean(versions?.unstable_features?.["org.matrix.msc3916"]), + cacheExpires: new Date().getTime() + 2 * 60 * 60 * 1000, // 2 hours from now + }; + } - // If we have server support (and a means of authentication), rewrite the URL to use MSC3916 endpoints. - if (serverSupportMap[csApi].supportsMSC3916 && accessToken) { - // Currently unstable only. - url = url.replace(/\/media\/v3\/(.*)\//, "/client/unstable/org.matrix.msc3916/media/$1/"); - } // else by default we make no changes + // If we have server support (and a means of authentication), rewrite the URL to use MSC3916 endpoints. + if (serverSupportMap[csApi].supportsMSC3916 && accessToken) { + // Currently unstable only. + url = url.replace(/\/media\/v3\/(.*)\//, "/client/unstable/org.matrix.msc3916/media/$1/"); + } // else by default we make no changes + } catch (err) { + console.error("SW: Error in request rewrite.", err); + } // Add authentication and send the request. We add authentication even if MSC3916 endpoints aren't // being used to ensure patches like this work: @@ -79,3 +101,69 @@ self.addEventListener("fetch", (event: FetchEvent) => { ); } }); + +// Ideally we'd use the `Client` interface for `client`, but since it's not available (see 'fetch' listener), we use +// unknown for now and force-cast it to something close enough later. +async function getAccessToken(client: unknown): Promise { + // Access tokens are encrypted at rest, so while we can grab the "access token", we'll need to do work to get the + // real thing. + const encryptedAccessToken = await idbLoad("account", "mx_access_token"); + + // We need to extract a user ID and device ID from localstorage, which means calling WebPlatform for the + // read operation. Service workers can't access localstorage. + const { userId, deviceId } = await new Promise<{ userId: string; deviceId: string }>((resolve, reject) => { + // Avoid stalling the tab in case something goes wrong. + const timeoutId = setTimeout(() => reject(new Error("timeout in postMessage")), 1000); + + // We don't need particularly good randomness here - we just use this to generate a request ID, so we know + // which postMessage reply is for our active request. + const responseKey = Math.random().toString(36); + + // Add the listener first, just in case the tab is *really* fast. + const listener = (event: MessageEvent): void => { + if (event.data?.responseKey !== responseKey) return; // not for us + clearTimeout(timeoutId); // do this as soon as possible, avoiding a race between resolve and reject. + resolve(event.data); // "unblock" the remainder of the thread, if that were such a thing in JavaScript. + self.removeEventListener("message", listener); // cleanup, since we're not going to do anything else. + }; + self.addEventListener("message", listener); + + // Ask the tab for the information we need. This is handled by WebPlatform. + (client as Window).postMessage({ responseKey, type: "userinfo" }); + }); + + // ... and this is why we need the user ID and device ID: they're index keys for the pickle key table. + const pickleKeyData = await idbLoad("pickleKey", [userId, deviceId]); + if (pickleKeyData && (!pickleKeyData.encrypted || !pickleKeyData.iv || !pickleKeyData.cryptoKey)) { + console.error("SW: Invalid pickle key loaded - ignoring"); + return undefined; + } + + let pickleKey: string | undefined; + + // Extract a useful pickle key out of our queries. + if (pickleKeyData) { + // We also need to generate the additional data needed for the key + const additionalData = new Uint8Array(userId.length + deviceId.length + 1); + for (let i = 0; i < userId.length; i++) { + additionalData[i] = userId.charCodeAt(i); + } + additionalData[userId.length] = 124; // "|" + for (let i = 0; i < deviceId.length; i++) { + additionalData[userId.length + 1 + i] = deviceId.charCodeAt(i); + } + + // Convert pickle key to a base64 key we can use + const pickleKeyBuf = await crypto.subtle.decrypt( + { name: "AES-GCM", iv: pickleKeyData.iv, additionalData }, + pickleKeyData.cryptoKey, + pickleKeyData.encrypted, + ); + if (pickleKeyBuf) { + pickleKey = encodeUnpaddedBase64(pickleKeyBuf); + } + } + + // Finally, try decrypting the thing and return that. This may fail, but that's okay. + return tryDecryptToken(pickleKey, encryptedAccessToken, ACCESS_TOKEN_IV); +} diff --git a/src/vector/platform/WebPlatform.ts b/src/vector/platform/WebPlatform.ts index 1fc6d5d9c8f..bf4fd323a8d 100644 --- a/src/vector/platform/WebPlatform.ts +++ b/src/vector/platform/WebPlatform.ts @@ -1,7 +1,7 @@ /* Copyright 2016 Aviral Dasgupta Copyright 2016 OpenMarket Ltd -Copyright 2017-2020 New Vector Ltd +Copyright 2017-2020, 2024 New Vector Ltd Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -52,7 +52,27 @@ export default class WebPlatform extends VectorBasePlatform { // Jest causes `register()` to return undefined, so swallow that case. if (swPromise) { swPromise - .then((r) => r.update()) + .then(async (r) => { + await r.update(); + return r; + }) + .then((r) => { + navigator.serviceWorker.addEventListener("message", (e) => { + try { + if (e.data?.["type"] === "userinfo" && e.data?.["responseKey"]) { + const userId = localStorage.getItem("mx_user_id"); + const deviceId = localStorage.getItem("mx_device_id"); + r.active!.postMessage({ + responseKey: e.data["responseKey"], + userId, + deviceId, + }); + } + } catch (e) { + console.error("Error responding to service worker: ", e); + } + }); + }) .catch((e) => console.error("Error registering/updating service worker:", e)); } } From 9494257745ffab1cb2057d012fb33202f3d3423d Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Thu, 18 Apr 2024 16:49:38 -0600 Subject: [PATCH 06/21] Add a bit of jitter --- src/serviceworker/index.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/serviceworker/index.ts b/src/serviceworker/index.ts index c8456b0b075..5f01a41fc5e 100644 --- a/src/serviceworker/index.ts +++ b/src/serviceworker/index.ts @@ -63,6 +63,9 @@ self.addEventListener("fetch", (event: FetchEvent) => { // Figure out which homeserver we're communicating with const csApi = url.substring(0, url.indexOf("/_matrix/media/v3")); + // Add jitter to reduce request spam, particularly to `/versions` on initial page load + await new Promise(resolve => setTimeout(() => resolve(), Math.random() * 10)); + // Locate our access token, and populate the fetchConfig with the authentication header. // @ts-expect-error - service worker types are not available. See 'fetch' event handler. const client = await self.clients.get(event.clientId); From 80671970b3926c482ebdc820782efba048aa01b1 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Mon, 22 Apr 2024 12:39:50 -0600 Subject: [PATCH 07/21] Improve legibility, use factored-out functions for pickling --- src/serviceworker/index.ts | 67 ++++++++++++++++---------------------- 1 file changed, 28 insertions(+), 39 deletions(-) diff --git a/src/serviceworker/index.ts b/src/serviceworker/index.ts index 5f01a41fc5e..d43ca05f079 100644 --- a/src/serviceworker/index.ts +++ b/src/serviceworker/index.ts @@ -16,7 +16,7 @@ limitations under the License. import { idbLoad } from "matrix-react-sdk/src/utils/StorageAccess"; import { ACCESS_TOKEN_IV, tryDecryptToken } from "matrix-react-sdk/src/utils/tokens/tokens"; -import { encodeUnpaddedBase64 } from "matrix-js-sdk/src/base64"; +import { buildAndEncodePickleKey } from "matrix-react-sdk/src/utils/tokens/pickling"; const serverSupportMap: { [serverUrl: string]: { @@ -37,8 +37,9 @@ self.addEventListener("activate", (event) => { event.waitUntil(clients.claim()); }); -// @ts-expect-error - getting types to work for this is difficult, so we anticipate that "addEventListener" doesn't -// have a valid signature. +// @ts-expect-error - the service worker types conflict with the DOM types available through TypeScript. Many hours +// have been spent trying to convince the type system that there's no actual conflict, but it has yet to work. Instead +// of trying to make it do the thing, we force-cast to something close enough where we can (and ignore errors otherwise). self.addEventListener("fetch", (event: FetchEvent) => { // This is the authenticated media (MSC3916) check, proxying what was unauthenticated to the authenticated variants. @@ -90,6 +91,7 @@ self.addEventListener("fetch", (event: FetchEvent) => { // If we have server support (and a means of authentication), rewrite the URL to use MSC3916 endpoints. if (serverSupportMap[csApi].supportsMSC3916 && accessToken) { // Currently unstable only. + // TODO: Support stable endpoints when available. url = url.replace(/\/media\/v3\/(.*)\//, "/client/unstable/org.matrix.msc3916/media/$1/"); } // else by default we make no changes } catch (err) { @@ -114,7 +116,29 @@ async function getAccessToken(client: unknown): Promise { // We need to extract a user ID and device ID from localstorage, which means calling WebPlatform for the // read operation. Service workers can't access localstorage. - const { userId, deviceId } = await new Promise<{ userId: string; deviceId: string }>((resolve, reject) => { + const { userId, deviceId } = await askClientForUserIdParams(client); + + // ... and this is why we need the user ID and device ID: they're index keys for the pickle key table. + const pickleKeyData = await idbLoad("pickleKey", [userId, deviceId]); + if (pickleKeyData && (!pickleKeyData.encrypted || !pickleKeyData.iv || !pickleKeyData.cryptoKey)) { + console.error("SW: Invalid pickle key loaded - ignoring"); + return undefined; + } + + // Finally, try decrypting the thing and return that. This may fail, but that's okay. + try { + const pickleKey = await buildAndEncodePickleKey(pickleKeyData, userId, deviceId); + return tryDecryptToken(pickleKey, encryptedAccessToken, ACCESS_TOKEN_IV); + } catch (e) { + console.error("SW: Error decrypting access token.", e); + return undefined; + } +} + +// Ideally we'd use the `Client` interface for `client`, but since it's not available (see 'fetch' listener), we use +// unknown for now and force-cast it to something close enough inside the function. +async function askClientForUserIdParams(client: unknown): Promise<{ userId: string, deviceId: string }> { + return new Promise((resolve, reject) => { // Avoid stalling the tab in case something goes wrong. const timeoutId = setTimeout(() => reject(new Error("timeout in postMessage")), 1000); @@ -134,39 +158,4 @@ async function getAccessToken(client: unknown): Promise { // Ask the tab for the information we need. This is handled by WebPlatform. (client as Window).postMessage({ responseKey, type: "userinfo" }); }); - - // ... and this is why we need the user ID and device ID: they're index keys for the pickle key table. - const pickleKeyData = await idbLoad("pickleKey", [userId, deviceId]); - if (pickleKeyData && (!pickleKeyData.encrypted || !pickleKeyData.iv || !pickleKeyData.cryptoKey)) { - console.error("SW: Invalid pickle key loaded - ignoring"); - return undefined; - } - - let pickleKey: string | undefined; - - // Extract a useful pickle key out of our queries. - if (pickleKeyData) { - // We also need to generate the additional data needed for the key - const additionalData = new Uint8Array(userId.length + deviceId.length + 1); - for (let i = 0; i < userId.length; i++) { - additionalData[i] = userId.charCodeAt(i); - } - additionalData[userId.length] = 124; // "|" - for (let i = 0; i < deviceId.length; i++) { - additionalData[userId.length + 1 + i] = deviceId.charCodeAt(i); - } - - // Convert pickle key to a base64 key we can use - const pickleKeyBuf = await crypto.subtle.decrypt( - { name: "AES-GCM", iv: pickleKeyData.iv, additionalData }, - pickleKeyData.cryptoKey, - pickleKeyData.encrypted, - ); - if (pickleKeyBuf) { - pickleKey = encodeUnpaddedBase64(pickleKeyBuf); - } - } - - // Finally, try decrypting the thing and return that. This may fail, but that's okay. - return tryDecryptToken(pickleKey, encryptedAccessToken, ACCESS_TOKEN_IV); } From ea7e8fb212f563b4f9708e70548396bd77b7a309 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Mon, 22 Apr 2024 14:02:41 -0600 Subject: [PATCH 08/21] Add docs --- src/vector/platform/WebPlatform.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/src/vector/platform/WebPlatform.ts b/src/vector/platform/WebPlatform.ts index bf4fd323a8d..de45a9ac7e5 100644 --- a/src/vector/platform/WebPlatform.ts +++ b/src/vector/platform/WebPlatform.ts @@ -53,6 +53,7 @@ export default class WebPlatform extends VectorBasePlatform { if (swPromise) { swPromise .then(async (r) => { + // always ask the browser to update. The browser might not actually do it, but at least we asked. await r.update(); return r; }) From d0dcf89f94578297d2d61a273c380cafcc782500 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Mon, 22 Apr 2024 14:11:29 -0600 Subject: [PATCH 09/21] Appease the linter --- src/serviceworker/index.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/serviceworker/index.ts b/src/serviceworker/index.ts index d43ca05f079..19df9e3edc8 100644 --- a/src/serviceworker/index.ts +++ b/src/serviceworker/index.ts @@ -65,7 +65,7 @@ self.addEventListener("fetch", (event: FetchEvent) => { const csApi = url.substring(0, url.indexOf("/_matrix/media/v3")); // Add jitter to reduce request spam, particularly to `/versions` on initial page load - await new Promise(resolve => setTimeout(() => resolve(), Math.random() * 10)); + await new Promise((resolve) => setTimeout(() => resolve(), Math.random() * 10)); // Locate our access token, and populate the fetchConfig with the authentication header. // @ts-expect-error - service worker types are not available. See 'fetch' event handler. @@ -137,7 +137,7 @@ async function getAccessToken(client: unknown): Promise { // Ideally we'd use the `Client` interface for `client`, but since it's not available (see 'fetch' listener), we use // unknown for now and force-cast it to something close enough inside the function. -async function askClientForUserIdParams(client: unknown): Promise<{ userId: string, deviceId: string }> { +async function askClientForUserIdParams(client: unknown): Promise<{ userId: string; deviceId: string }> { return new Promise((resolve, reject) => { // Avoid stalling the tab in case something goes wrong. const timeoutId = setTimeout(() => reject(new Error("timeout in postMessage")), 1000); From 80dd41508b51732bfcc20aefbad0f442ad12ed29 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Tue, 23 Apr 2024 13:46:06 -0600 Subject: [PATCH 10/21] Document risks of postMessage --- src/serviceworker/index.ts | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/serviceworker/index.ts b/src/serviceworker/index.ts index 19df9e3edc8..a7d64d3c17c 100644 --- a/src/serviceworker/index.ts +++ b/src/serviceworker/index.ts @@ -139,6 +139,15 @@ async function getAccessToken(client: unknown): Promise { // unknown for now and force-cast it to something close enough inside the function. async function askClientForUserIdParams(client: unknown): Promise<{ userId: string; deviceId: string }> { return new Promise((resolve, reject) => { + // Dev note: this uses postMessage, which is a highly insecure channel. postMessage is typically visible to other + // tabs, windows, browser extensions, etc, making it far from ideal for sharing sensitive information. This is + // why our service worker calculates/decrypts the access token manually: we don't want the user's access token + // to be available to (potentially) malicious listeners. We do require some information for that decryption to + // work though, and request that in the least sensitive way possible. + // + // We could also potentially use some version of TLS to encrypt postMessage, though that feels way more involved + // than just reading IndexedDB ourselves. + // Avoid stalling the tab in case something goes wrong. const timeoutId = setTimeout(() => reject(new Error("timeout in postMessage")), 1000); From 0395ee450e699f41ca122dea9d9a82d29be10793 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Tue, 23 Apr 2024 13:46:20 -0600 Subject: [PATCH 11/21] Split service worker post message handling out to function --- src/vector/platform/WebPlatform.ts | 32 ++++++++++++++++-------------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/src/vector/platform/WebPlatform.ts b/src/vector/platform/WebPlatform.ts index de45a9ac7e5..5e563685c41 100644 --- a/src/vector/platform/WebPlatform.ts +++ b/src/vector/platform/WebPlatform.ts @@ -58,27 +58,29 @@ export default class WebPlatform extends VectorBasePlatform { return r; }) .then((r) => { - navigator.serviceWorker.addEventListener("message", (e) => { - try { - if (e.data?.["type"] === "userinfo" && e.data?.["responseKey"]) { - const userId = localStorage.getItem("mx_user_id"); - const deviceId = localStorage.getItem("mx_device_id"); - r.active!.postMessage({ - responseKey: e.data["responseKey"], - userId, - deviceId, - }); - } - } catch (e) { - console.error("Error responding to service worker: ", e); - } - }); + navigator.serviceWorker.addEventListener("message", this.onServiceWorkerPostMessage.bind(this)); }) .catch((e) => console.error("Error registering/updating service worker:", e)); } } } + private onServiceWorkerPostMessage(event: MessageEvent): void { + try { + if (event.data?.["type"] === "userinfo" && event.data?.["responseKey"]) { + const userId = localStorage.getItem("mx_user_id"); + const deviceId = localStorage.getItem("mx_device_id"); + event.source!.postMessage({ + responseKey: event.data["responseKey"], + userId, + deviceId, + }); + } + } catch (e) { + console.error("Error responding to service worker: ", e); + } + } + public getHumanReadableName(): string { return "Web Platform"; // no translation required: only used for analytics } From 2ad00c03ddb9b927a4278a54985b3e140ef5623e Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Tue, 23 Apr 2024 13:50:33 -0600 Subject: [PATCH 12/21] Move registration to async function --- src/vector/platform/WebPlatform.ts | 44 ++++++++++++++++++------------ 1 file changed, 26 insertions(+), 18 deletions(-) diff --git a/src/vector/platform/WebPlatform.ts b/src/vector/platform/WebPlatform.ts index 5e563685c41..e6cefd20e77 100644 --- a/src/vector/platform/WebPlatform.ts +++ b/src/vector/platform/WebPlatform.ts @@ -27,6 +27,7 @@ import { logger } from "matrix-js-sdk/src/logger"; import VectorBasePlatform from "./VectorBasePlatform"; import { parseQs } from "../url_utils"; import { _t } from "../../languageHandler"; +import regist = jsrsasign.KJUR.crypto.ECParameterDB.regist; const POKE_RATE_MS = 10 * 60 * 1000; // 10 min @@ -44,24 +45,31 @@ export default class WebPlatform extends VectorBasePlatform { public constructor() { super(); - // Register service worker if available on this platform - if ("serviceWorker" in navigator) { - // sw.js is exported by webpack, sourced from `/src/serviceworker/index.ts` - const swPromise = navigator.serviceWorker.register("sw.js"); - - // Jest causes `register()` to return undefined, so swallow that case. - if (swPromise) { - swPromise - .then(async (r) => { - // always ask the browser to update. The browser might not actually do it, but at least we asked. - await r.update(); - return r; - }) - .then((r) => { - navigator.serviceWorker.addEventListener("message", this.onServiceWorkerPostMessage.bind(this)); - }) - .catch((e) => console.error("Error registering/updating service worker:", e)); - } + + // noinspection JSIgnoredPromiseFromCall - can run async + this.tryRegisterServiceWorker(); + } + + private async tryRegisterServiceWorker(): Promise { + if (!("serviceWorker" in navigator)) { + return; // not available on this platform - don't try to register the service worker + } + + // sw.js is exported by webpack, sourced from `/src/serviceworker/index.ts` + const swPromise = navigator.serviceWorker.register("sw.js"); + if (!swPromise) { + // Registration didn't return a promise for some reason - assume failed and ignore. + // This typically happens in Jest. + return; + } + + try { + const registration = await swPromise; + await registration.update(); + navigator.serviceWorker.addEventListener("message", this.onServiceWorkerPostMessage.bind(this)); + } catch (e) { + console.error("Error registering/updating service worker:", e); + // otherwise ignore the error and remain unregistered } } From 0951fe7ef06421e803a88b1a6bf6b04ff6f8d26c Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Tue, 23 Apr 2024 14:02:17 -0600 Subject: [PATCH 13/21] Use more early returns --- src/serviceworker/index.ts | 112 ++++++++++++++++++++----------------- 1 file changed, 62 insertions(+), 50 deletions(-) diff --git a/src/serviceworker/index.ts b/src/serviceworker/index.ts index a7d64d3c17c..cde64c58980 100644 --- a/src/serviceworker/index.ts +++ b/src/serviceworker/index.ts @@ -54,59 +54,71 @@ self.addEventListener("fetch", (event: FetchEvent) => { // We only intercept v3 download and thumbnail requests as presumably everything else is deliberate. // For example, `/_matrix/media/unstable` or `/_matrix/media/v3/preview_url` are something well within // the control of the application, and appear to be choices made at a higher level than us. - if (url.includes("/_matrix/media/v3/download") || url.includes("/_matrix/media/v3/thumbnail")) { - // We need to call respondWith synchronously, otherwise we may never execute properly. This means - // later on we need to proxy the request through if it turns out the server doesn't support authentication. - event.respondWith( - (async (): Promise => { - let fetchConfig: { headers?: { [key: string]: string } } = {}; - try { - // Figure out which homeserver we're communicating with - const csApi = url.substring(0, url.indexOf("/_matrix/media/v3")); - - // Add jitter to reduce request spam, particularly to `/versions` on initial page load - await new Promise((resolve) => setTimeout(() => resolve(), Math.random() * 10)); - - // Locate our access token, and populate the fetchConfig with the authentication header. - // @ts-expect-error - service worker types are not available. See 'fetch' event handler. - const client = await self.clients.get(event.clientId); - const accessToken = await getAccessToken(client); - if (accessToken) { - fetchConfig = { - headers: { - Authorization: `Bearer ${accessToken}`, - }, - }; - } - - // Update or populate the server support map using a (usually) authenticated `/versions` call. - if (!serverSupportMap[csApi] || serverSupportMap[csApi].cacheExpires <= new Date().getTime()) { - const versions = await (await fetch(`${csApi}/_matrix/client/versions`, fetchConfig)).json(); - serverSupportMap[csApi] = { - supportsMSC3916: Boolean(versions?.unstable_features?.["org.matrix.msc3916"]), - cacheExpires: new Date().getTime() + 2 * 60 * 60 * 1000, // 2 hours from now - }; - } - - // If we have server support (and a means of authentication), rewrite the URL to use MSC3916 endpoints. - if (serverSupportMap[csApi].supportsMSC3916 && accessToken) { - // Currently unstable only. - // TODO: Support stable endpoints when available. - url = url.replace(/\/media\/v3\/(.*)\//, "/client/unstable/org.matrix.msc3916/media/$1/"); - } // else by default we make no changes - } catch (err) { - console.error("SW: Error in request rewrite.", err); - } - - // Add authentication and send the request. We add authentication even if MSC3916 endpoints aren't - // being used to ensure patches like this work: - // https://github.com/matrix-org/synapse/commit/2390b66bf0ec3ff5ffb0c7333f3c9b239eeb92bb - return fetch(url, fetchConfig); - })(), - ); + if (!url.includes("/_matrix/media/v3/download") && !url.includes("/_matrix/media/v3/thumbnail")) { + return; // not a URL we care about } + + // We need to call respondWith synchronously, otherwise we may never execute properly. This means + // later on we need to proxy the request through if it turns out the server doesn't support authentication. + event.respondWith( + (async (): Promise => { + let accessToken: string | undefined; + try { + // Figure out which homeserver we're communicating with + const csApi = url.substring(0, url.indexOf("/_matrix/media/v3")); + + // Add jitter to reduce request spam, particularly to `/versions` on initial page load + await new Promise((resolve) => setTimeout(() => resolve(), Math.random() * 10)); + + // Locate our access token, and populate the fetchConfig with the authentication header. + // @ts-expect-error - service worker types are not available. See 'fetch' event handler. + const client = await self.clients.get(event.clientId); + accessToken = await getAccessToken(client); + + // Update or populate the server support map using a (usually) authenticated `/versions` call. + await tryUpdateServerSupportMap(csApi, accessToken); + + // If we have server support (and a means of authentication), rewrite the URL to use MSC3916 endpoints. + if (serverSupportMap[csApi].supportsMSC3916 && accessToken) { + // Currently unstable only. + // TODO: Support stable endpoints when available. + url = url.replace(/\/media\/v3\/(.*)\//, "/client/unstable/org.matrix.msc3916/media/$1/"); + } // else by default we make no changes + } catch (err) { + console.error("SW: Error in request rewrite.", err); + } + + // Add authentication and send the request. We add authentication even if MSC3916 endpoints aren't + // being used to ensure patches like this work: + // https://github.com/matrix-org/synapse/commit/2390b66bf0ec3ff5ffb0c7333f3c9b239eeb92bb + const config = !accessToken ? undefined : { + headers: { + Authorization: `Bearer ${accessToken}`, + }, + }; + return fetch(url, config); + })(), + ); }); +async function tryUpdateServerSupportMap(clientApiUrl: string, accessToken?: string): Promise { + // only update if we don't know about it, or if the data is stale + if (serverSupportMap[clientApiUrl]?.cacheExpires > new Date().getTime()) { + return; // up to date + } + + const versions = await (await fetch(`${clientApiUrl}/_matrix/client/versions`, { + headers: { + Authorization: `Bearer ${accessToken}`, + }, + })).json(); + + serverSupportMap[clientApiUrl] = { + supportsMSC3916: Boolean(versions?.unstable_features?.["org.matrix.msc3916"]), + cacheExpires: new Date().getTime() + 2 * 60 * 60 * 1000, // 2 hours from now + }; +} + // Ideally we'd use the `Client` interface for `client`, but since it's not available (see 'fetch' listener), we use // unknown for now and force-cast it to something close enough later. async function getAccessToken(client: unknown): Promise { From c20d5f1e75a0e9d522cfed2c8f940575ebf16197 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Tue, 23 Apr 2024 14:03:19 -0600 Subject: [PATCH 14/21] Thanks(?), WebStorm --- src/vector/platform/WebPlatform.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/vector/platform/WebPlatform.ts b/src/vector/platform/WebPlatform.ts index e6cefd20e77..86e7082b1d4 100644 --- a/src/vector/platform/WebPlatform.ts +++ b/src/vector/platform/WebPlatform.ts @@ -27,7 +27,6 @@ import { logger } from "matrix-js-sdk/src/logger"; import VectorBasePlatform from "./VectorBasePlatform"; import { parseQs } from "../url_utils"; import { _t } from "../../languageHandler"; -import regist = jsrsasign.KJUR.crypto.ECParameterDB.regist; const POKE_RATE_MS = 10 * 60 * 1000; // 10 min From 3947d903f9813c41dd37424c0781f81796e1d135 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Tue, 23 Apr 2024 14:10:52 -0600 Subject: [PATCH 15/21] Handle case of no access token for /versions --- src/serviceworker/index.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/serviceworker/index.ts b/src/serviceworker/index.ts index cde64c58980..314de4432f3 100644 --- a/src/serviceworker/index.ts +++ b/src/serviceworker/index.ts @@ -107,11 +107,12 @@ async function tryUpdateServerSupportMap(clientApiUrl: string, accessToken?: str return; // up to date } - const versions = await (await fetch(`${clientApiUrl}/_matrix/client/versions`, { + const config = !accessToken ? undefined : { headers: { Authorization: `Bearer ${accessToken}`, }, - })).json(); + }; + const versions = await (await fetch(`${clientApiUrl}/_matrix/client/versions`, config)).json(); serverSupportMap[clientApiUrl] = { supportsMSC3916: Boolean(versions?.unstable_features?.["org.matrix.msc3916"]), From 7d9e7d6c7498452585e744665c58eea6ca365e5d Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Tue, 23 Apr 2024 14:16:41 -0600 Subject: [PATCH 16/21] Appease linter --- src/serviceworker/index.ts | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/src/serviceworker/index.ts b/src/serviceworker/index.ts index 314de4432f3..25942bd3a53 100644 --- a/src/serviceworker/index.ts +++ b/src/serviceworker/index.ts @@ -91,11 +91,13 @@ self.addEventListener("fetch", (event: FetchEvent) => { // Add authentication and send the request. We add authentication even if MSC3916 endpoints aren't // being used to ensure patches like this work: // https://github.com/matrix-org/synapse/commit/2390b66bf0ec3ff5ffb0c7333f3c9b239eeb92bb - const config = !accessToken ? undefined : { - headers: { - Authorization: `Bearer ${accessToken}`, - }, - }; + const config = !accessToken + ? undefined + : { + headers: { + Authorization: `Bearer ${accessToken}`, + }, + }; return fetch(url, config); })(), ); @@ -107,11 +109,13 @@ async function tryUpdateServerSupportMap(clientApiUrl: string, accessToken?: str return; // up to date } - const config = !accessToken ? undefined : { - headers: { - Authorization: `Bearer ${accessToken}`, - }, - }; + const config = !accessToken + ? undefined + : { + headers: { + Authorization: `Bearer ${accessToken}`, + }, + }; const versions = await (await fetch(`${clientApiUrl}/_matrix/client/versions`, config)).json(); serverSupportMap[clientApiUrl] = { From 37e3dfdb36a9e9151fbea16bc5b135309266b8d0 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Wed, 1 May 2024 09:42:36 -0600 Subject: [PATCH 17/21] Apply suggestions from code review Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> --- src/vector/platform/WebPlatform.ts | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/vector/platform/WebPlatform.ts b/src/vector/platform/WebPlatform.ts index 86e7082b1d4..5643488b523 100644 --- a/src/vector/platform/WebPlatform.ts +++ b/src/vector/platform/WebPlatform.ts @@ -45,8 +45,8 @@ export default class WebPlatform extends VectorBasePlatform { public constructor() { super(); - // noinspection JSIgnoredPromiseFromCall - can run async - this.tryRegisterServiceWorker(); + // Register the service worker in the background + this.tryRegisterServiceWorker().catch((e) => console.error("Error registering/updating service worker:", e)); } private async tryRegisterServiceWorker(): Promise { @@ -55,15 +55,14 @@ export default class WebPlatform extends VectorBasePlatform { } // sw.js is exported by webpack, sourced from `/src/serviceworker/index.ts` - const swPromise = navigator.serviceWorker.register("sw.js"); - if (!swPromise) { - // Registration didn't return a promise for some reason - assume failed and ignore. + const registration = await navigator.serviceWorker.register("sw.js"); + if (!registration) { + // Registration didn't work for some reason - assume failed and ignore. // This typically happens in Jest. return; } - + try { - const registration = await swPromise; await registration.update(); navigator.serviceWorker.addEventListener("message", this.onServiceWorkerPostMessage.bind(this)); } catch (e) { From 0d5e2a9beef96910e6aa8d38aabcf3574bd10297 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Wed, 1 May 2024 09:43:40 -0600 Subject: [PATCH 18/21] Remove spurious try/catch --- src/vector/platform/WebPlatform.ts | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/src/vector/platform/WebPlatform.ts b/src/vector/platform/WebPlatform.ts index 5643488b523..da6f61b323e 100644 --- a/src/vector/platform/WebPlatform.ts +++ b/src/vector/platform/WebPlatform.ts @@ -61,14 +61,9 @@ export default class WebPlatform extends VectorBasePlatform { // This typically happens in Jest. return; } - - try { - await registration.update(); - navigator.serviceWorker.addEventListener("message", this.onServiceWorkerPostMessage.bind(this)); - } catch (e) { - console.error("Error registering/updating service worker:", e); - // otherwise ignore the error and remain unregistered - } + + await registration.update(); + navigator.serviceWorker.addEventListener("message", this.onServiceWorkerPostMessage.bind(this)); } private onServiceWorkerPostMessage(event: MessageEvent): void { From ec159a34aab0ada1a72d147b1ea354783778a4a4 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Wed, 1 May 2024 09:46:54 -0600 Subject: [PATCH 19/21] Factor out fetch config stuff --- src/serviceworker/index.ts | 29 ++++++++++++++--------------- 1 file changed, 14 insertions(+), 15 deletions(-) diff --git a/src/serviceworker/index.ts b/src/serviceworker/index.ts index 25942bd3a53..fd674c1e4fb 100644 --- a/src/serviceworker/index.ts +++ b/src/serviceworker/index.ts @@ -91,14 +91,7 @@ self.addEventListener("fetch", (event: FetchEvent) => { // Add authentication and send the request. We add authentication even if MSC3916 endpoints aren't // being used to ensure patches like this work: // https://github.com/matrix-org/synapse/commit/2390b66bf0ec3ff5ffb0c7333f3c9b239eeb92bb - const config = !accessToken - ? undefined - : { - headers: { - Authorization: `Bearer ${accessToken}`, - }, - }; - return fetch(url, config); + return fetch(url, fetchConfigForToken(accessToken)); })(), ); }); @@ -109,13 +102,7 @@ async function tryUpdateServerSupportMap(clientApiUrl: string, accessToken?: str return; // up to date } - const config = !accessToken - ? undefined - : { - headers: { - Authorization: `Bearer ${accessToken}`, - }, - }; + const config = fetchConfigForToken(accessToken); const versions = await (await fetch(`${clientApiUrl}/_matrix/client/versions`, config)).json(); serverSupportMap[clientApiUrl] = { @@ -185,3 +172,15 @@ async function askClientForUserIdParams(client: unknown): Promise<{ userId: stri (client as Window).postMessage({ responseKey, type: "userinfo" }); }); } + +function fetchConfigForToken(accessToken?: string): RequestInit | undefined { + if (!accessToken) { + return undefined; // no headers/config to specify + } + + return { + headers: { + Authorization: `Bearer ${accessToken}`, + }, + }; +} From b80adc59a37982c2bb2b45944eb3e7389ff3e6cf Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Tue, 14 May 2024 13:02:02 -0600 Subject: [PATCH 20/21] Apply suggestions from code review Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> --- src/serviceworker/index.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/serviceworker/index.ts b/src/serviceworker/index.ts index fd674c1e4fb..8963ee1dff6 100644 --- a/src/serviceworker/index.ts +++ b/src/serviceworker/index.ts @@ -21,7 +21,7 @@ import { buildAndEncodePickleKey } from "matrix-react-sdk/src/utils/tokens/pickl const serverSupportMap: { [serverUrl: string]: { supportsMSC3916: boolean; - cacheExpires: number; + cacheExpiryTimeMs: number; }; } = {}; @@ -47,7 +47,7 @@ self.addEventListener("fetch", (event: FetchEvent) => { return; // not important to us } - // Note: ideally we'd keep the request headers and etc, but in practice we can't even see those details. + // Note: ideally we'd keep the request headers etc, but in practice we can't even see those details. // See https://stackoverflow.com/a/59152482 let url = event.request.url; From a57c111d276e992436f1f347afedeb7234fbaeb8 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Tue, 14 May 2024 13:02:52 -0600 Subject: [PATCH 21/21] Finish applying code review suggestions --- src/serviceworker/index.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/serviceworker/index.ts b/src/serviceworker/index.ts index 8963ee1dff6..b06921892f2 100644 --- a/src/serviceworker/index.ts +++ b/src/serviceworker/index.ts @@ -98,7 +98,7 @@ self.addEventListener("fetch", (event: FetchEvent) => { async function tryUpdateServerSupportMap(clientApiUrl: string, accessToken?: string): Promise { // only update if we don't know about it, or if the data is stale - if (serverSupportMap[clientApiUrl]?.cacheExpires > new Date().getTime()) { + if (serverSupportMap[clientApiUrl]?.cacheExpiryTimeMs > new Date().getTime()) { return; // up to date } @@ -107,7 +107,7 @@ async function tryUpdateServerSupportMap(clientApiUrl: string, accessToken?: str serverSupportMap[clientApiUrl] = { supportsMSC3916: Boolean(versions?.unstable_features?.["org.matrix.msc3916"]), - cacheExpires: new Date().getTime() + 2 * 60 * 60 * 1000, // 2 hours from now + cacheExpiryTimeMs: new Date().getTime() + 2 * 60 * 60 * 1000, // 2 hours from now }; }