Merge branch 'develop' into toger5/new-MembershipManager

Author: toger5

.github/workflows/sonarcloud.yml

@@ -27,7 +27,7 @@ jobs:
         steps:
             # We create the status here and then update it to success/failure in the `report` stage
             # This provides an easy link to this workflow_run from the PR before Sonarcloud is done.
-            - uses: guibranco/github-status-action-v2@7ca807c2ba3401be532d29a876b93262108099fb
+            - uses: guibranco/github-status-action-v2@5ef6e175c333bc629f3718b083c8a2ff6e0bbfbc
               with:
                   authToken: ${{ secrets.GITHUB_TOKEN }}
                   state: pending
@@ -75,7 +75,7 @@ jobs:
 
             - name: "🩻 SonarCloud Scan"
               id: sonarcloud
-              uses: matrix-org/sonarcloud-workflow-action@v3.3
+              uses: matrix-org/sonarcloud-workflow-action@v4.0
               # workflow_run fails report against the develop commit always, we don't want that for PRs
               continue-on-error: ${{ github.event.workflow_run.head_branch != 'develop' }}
               with:
@@ -87,7 +87,7 @@ jobs:
                   revision: ${{ github.event.workflow_run.head_sha }}
                   token: ${{ secrets.SONAR_TOKEN }}
 
-            - uses: guibranco/github-status-action-v2@7ca807c2ba3401be532d29a876b93262108099fb
+            - uses: guibranco/github-status-action-v2@5ef6e175c333bc629f3718b083c8a2ff6e0bbfbc
               if: always()
               with:
                   authToken: ${{ secrets.GITHUB_TOKEN }}

.github/workflows/tests.yml

@@ -116,7 +116,7 @@ jobs:
         steps:
             - name: Skip SonarCloud on merge queues
               if: env.ENABLE_COVERAGE == 'false'
-              uses: guibranco/github-status-action-v2@7ca807c2ba3401be532d29a876b93262108099fb
+              uses: guibranco/github-status-action-v2@5ef6e175c333bc629f3718b083c8a2ff6e0bbfbc
               with:
                   authToken: ${{ secrets.GITHUB_TOKEN }}
                   state: success

babel.config.cjs

@@ -26,6 +26,7 @@ module.exports = {
         ],
     ],
     plugins: [
+        ["@babel/plugin-proposal-decorators", { version: "2023-11" }],
         "@babel/plugin-transform-numeric-separator",
         "@babel/plugin-transform-class-properties",
         "@babel/plugin-transform-object-rest-spread",

package.json

@@ -72,6 +72,7 @@
         "@babel/core": "^7.12.10",
         "@babel/eslint-parser": "^7.12.10",
         "@babel/eslint-plugin": "^7.12.10",
+        "@babel/plugin-proposal-decorators": "^7.25.9",
         "@babel/plugin-syntax-dynamic-import": "^7.8.3",
         "@babel/plugin-transform-class-properties": "^7.12.1",
         "@babel/plugin-transform-numeric-separator": "^7.12.7",
@@ -81,7 +82,7 @@
         "@babel/preset-typescript": "^7.12.7",
         "@casualbot/jest-sonar-reporter": "2.2.7",
         "@peculiar/webcrypto": "^1.4.5",
-        "@stylistic/eslint-plugin": "^3.0.0",
+        "@stylistic/eslint-plugin": "^4.0.0",
         "@types/content-type": "^1.1.5",
         "@types/debug": "^4.1.7",
         "@types/jest": "^29.0.0",
@@ -121,7 +122,7 @@
         "ts-node": "^10.9.2",
         "typedoc": "^0.27.0",
         "typedoc-plugin-coverage": "^3.0.0",
-        "typedoc-plugin-mdn-links": "^4.0.0",
+        "typedoc-plugin-mdn-links": "^5.0.0",
         "typedoc-plugin-missing-exports": "^3.0.0",
         "typescript": "^5.4.2"
     },

spec/unit/http-api/fetch.spec.ts

@@ -20,6 +20,7 @@ import { FetchHttpApi } from "../../../src/http-api/fetch";
 import { TypedEventEmitter } from "../../../src/models/typed-event-emitter";
 import {
     ClientPrefix,
+    ConnectionError,
     HttpApiEvent,
     type HttpApiEventHandlerMap,
     IdentityPrefix,
@@ -125,7 +126,7 @@ describe("FetchHttpApi", () => {
         ).resolves.toBe(text);
     });
 
-    it("should send token via query params if useAuthorizationHeader=false", () => {
+    it("should send token via query params if useAuthorizationHeader=false", async () => {
         const fetchFn = jest.fn().mockResolvedValue({ ok: true });
         const api = new FetchHttpApi(new TypedEventEmitter<any, any>(), {
             baseUrl,
@@ -134,19 +135,19 @@ describe("FetchHttpApi", () => {
             accessToken: "token",
             useAuthorizationHeader: false,
         });
-        api.authedRequest(Method.Get, "/path");
+        await api.authedRequest(Method.Get, "/path");
         expect(fetchFn.mock.calls[0][0].searchParams.get("access_token")).toBe("token");
     });
 
-    it("should send token via headers by default", () => {
+    it("should send token via headers by default", async () => {
         const fetchFn = jest.fn().mockResolvedValue({ ok: true });
         const api = new FetchHttpApi(new TypedEventEmitter<any, any>(), {
             baseUrl,
             prefix,
             fetchFn,
             accessToken: "token",
         });
-        api.authedRequest(Method.Get, "/path");
+        await api.authedRequest(Method.Get, "/path");
         expect(fetchFn.mock.calls[0][1].headers["Authorization"]).toBe("Bearer token");
     });
 
@@ -163,7 +164,7 @@ describe("FetchHttpApi", () => {
         expect(fetchFn.mock.calls[0][1].headers["Authorization"]).toBeFalsy();
     });
 
-    it("should ensure no token is leaked out via query params if sending via headers", () => {
+    it("should ensure no token is leaked out via query params if sending via headers", async () => {
         const fetchFn = jest.fn().mockResolvedValue({ ok: true });
         const api = new FetchHttpApi(new TypedEventEmitter<any, any>(), {
             baseUrl,
@@ -172,12 +173,12 @@ describe("FetchHttpApi", () => {
             accessToken: "token",
             useAuthorizationHeader: true,
         });
-        api.authedRequest(Method.Get, "/path", { access_token: "123" });
+        await api.authedRequest(Method.Get, "/path", { access_token: "123" });
         expect(fetchFn.mock.calls[0][0].searchParams.get("access_token")).toBeFalsy();
         expect(fetchFn.mock.calls[0][1].headers["Authorization"]).toBe("Bearer token");
     });
 
-    it("should not override manually specified access token via query params", () => {
+    it("should not override manually specified access token via query params", async () => {
         const fetchFn = jest.fn().mockResolvedValue({ ok: true });
         const api = new FetchHttpApi(new TypedEventEmitter<any, any>(), {
             baseUrl,
@@ -186,11 +187,11 @@ describe("FetchHttpApi", () => {
             accessToken: "token",
             useAuthorizationHeader: false,
         });
-        api.authedRequest(Method.Get, "/path", { access_token: "RealToken" });
+        await api.authedRequest(Method.Get, "/path", { access_token: "RealToken" });
         expect(fetchFn.mock.calls[0][0].searchParams.get("access_token")).toBe("RealToken");
     });
 
-    it("should not override manually specified access token via header", () => {
+    it("should not override manually specified access token via header", async () => {
         const fetchFn = jest.fn().mockResolvedValue({ ok: true });
         const api = new FetchHttpApi(new TypedEventEmitter<any, any>(), {
             baseUrl,
@@ -199,16 +200,16 @@ describe("FetchHttpApi", () => {
             accessToken: "token",
             useAuthorizationHeader: true,
         });
-        api.authedRequest(Method.Get, "/path", undefined, undefined, {
+        await api.authedRequest(Method.Get, "/path", undefined, undefined, {
             headers: { Authorization: "Bearer RealToken" },
         });
         expect(fetchFn.mock.calls[0][1].headers["Authorization"]).toBe("Bearer RealToken");
     });
 
-    it("should not override Accept header", () => {
+    it("should not override Accept header", async () => {
         const fetchFn = jest.fn().mockResolvedValue({ ok: true });
         const api = new FetchHttpApi(new TypedEventEmitter<any, any>(), { baseUrl, prefix, fetchFn });
-        api.authedRequest(Method.Get, "/path", undefined, undefined, {
+        await api.authedRequest(Method.Get, "/path", undefined, undefined, {
             headers: { Accept: "text/html" },
         });
         expect(fetchFn.mock.calls[0][1].headers["Accept"]).toBe("text/html");
@@ -288,7 +289,7 @@ describe("FetchHttpApi", () => {
 
                 describe("with a tokenRefreshFunction", () => {
                     it("should emit logout and throw when token refresh fails", async () => {
-                        const error = new Error("uh oh");
+                        const error = new MatrixError();
                         const tokenRefreshFunction = jest.fn().mockRejectedValue(error);
                         const fetchFn = jest.fn().mockResolvedValue(unknownTokenResponse);
                         const emitter = new TypedEventEmitter<HttpApiEvent, HttpApiEventHandlerMap>();
@@ -308,6 +309,27 @@ describe("FetchHttpApi", () => {
                         expect(emitter.emit).toHaveBeenCalledWith(HttpApiEvent.SessionLoggedOut, unknownTokenErr);
                     });
 
+                    it("should not emit logout but still throw when token refresh fails due to transitive fault", async () => {
+                        const error = new ConnectionError("transitive fault");
+                        const tokenRefreshFunction = jest.fn().mockRejectedValue(error);
+                        const fetchFn = jest.fn().mockResolvedValue(unknownTokenResponse);
+                        const emitter = new TypedEventEmitter<HttpApiEvent, HttpApiEventHandlerMap>();
+                        jest.spyOn(emitter, "emit");
+                        const api = new FetchHttpApi(emitter, {
+                            baseUrl,
+                            prefix,
+                            fetchFn,
+                            tokenRefreshFunction,
+                            accessToken,
+                            refreshToken,
+                        });
+                        await expect(api.authedRequest(Method.Post, "/account/password")).rejects.toThrow(
+                            unknownTokenErr,
+                        );
+                        expect(tokenRefreshFunction).toHaveBeenCalledWith(refreshToken);
+                        expect(emitter.emit).not.toHaveBeenCalledWith(HttpApiEvent.SessionLoggedOut, unknownTokenErr);
+                    });
+
                     it("should refresh token and retry request", async () => {
                         const newAccessToken = "new-access-token";
                         const newRefreshToken = "new-refresh-token";
@@ -468,4 +490,61 @@ describe("FetchHttpApi", () => {
             ]
         `);
     });
+
+    it("should not make multiple concurrent refresh token requests", async () => {
+        const tokenInactiveError = new MatrixError({ errcode: "M_UNKNOWN_TOKEN", error: "Token is not active" }, 401);
+
+        const deferredTokenRefresh = defer<{ accessToken: string; refreshToken: string }>();
+        const fetchFn = jest.fn().mockResolvedValue({
+            ok: false,
+            status: tokenInactiveError.httpStatus,
+            async text() {
+                return JSON.stringify(tokenInactiveError.data);
+            },
+            async json() {
+                return tokenInactiveError.data;
+            },
+            headers: {
+                get: jest.fn().mockReturnValue("application/json"),
+            },
+        });
+        const tokenRefreshFunction = jest.fn().mockReturnValue(deferredTokenRefresh.promise);
+
+        const api = new FetchHttpApi(new TypedEventEmitter<any, any>(), {
+            baseUrl,
+            prefix,
+            fetchFn,
+            doNotAttemptTokenRefresh: false,
+            tokenRefreshFunction,
+            accessToken: "ACCESS_TOKEN",
+            refreshToken: "REFRESH_TOKEN",
+        });
+
+        const prom1 = api.authedRequest(Method.Get, "/path1");
+        const prom2 = api.authedRequest(Method.Get, "/path2");
+
+        await jest.advanceTimersByTimeAsync(10); // wait for requests to fire
+        expect(fetchFn).toHaveBeenCalledTimes(2);
+        fetchFn.mockResolvedValue({
+            ok: true,
+            status: 200,
+            async text() {
+                return "{}";
+            },
+            async json() {
+                return {};
+            },
+            headers: {
+                get: jest.fn().mockReturnValue("application/json"),
+            },
+        });
+        deferredTokenRefresh.resolve({ accessToken: "NEW_ACCESS_TOKEN", refreshToken: "NEW_REFRESH_TOKEN" });
+
+        await prom1;
+        await prom2;
+        expect(fetchFn).toHaveBeenCalledTimes(4); // 2 original calls + 2 retries
+        expect(tokenRefreshFunction).toHaveBeenCalledTimes(1);
+        expect(api.opts.accessToken).toBe("NEW_ACCESS_TOKEN");
+        expect(api.opts.refreshToken).toBe("NEW_REFRESH_TOKEN");
+    });
 });

spec/unit/http-api/index.spec.ts

@@ -59,11 +59,12 @@ describe("MatrixHttpApi", () => {
         xhr.onreadystatechange?.(new Event("test"));
     });
 
-    it("should fall back to `fetch` where xhr is unavailable", () => {
+    it("should fall back to `fetch` where xhr is unavailable", async () => {
         globalThis.XMLHttpRequest = undefined!;
         const fetchFn = jest.fn().mockResolvedValue({ ok: true, json: jest.fn().mockResolvedValue({}) });
         const api = new MatrixHttpApi(new TypedEventEmitter<any, any>(), { baseUrl, prefix, fetchFn });
         upload = api.uploadContent({} as File);
+        await upload;
         expect(fetchFn).toHaveBeenCalled();
     });
 

spec/unit/matrix-client.spec.ts

@@ -2753,12 +2753,13 @@ describe("MatrixClient", function () {
             // WHEN we call `setAccountData` ...
             const setProm = client.setAccountData(eventType, content);
 
+            await jest.advanceTimersByTimeAsync(10);
             // THEN, the REST call should have happened, and had the correct content
             const lastCall = fetchMock.lastCall("put-account-data");
             expect(lastCall).toBeDefined();
             expect(lastCall?.[1]?.body).toEqual(JSON.stringify(content));
 
-            // Even after waiting a bit, the method should not yet have returned
+            // Even after waiting a bit more, the method should not yet have returned
             await jest.advanceTimersByTimeAsync(10);
             let finished = false;
             setProm.finally(() => (finished = true));

spec/unit/rust-crypto/rust-crypto.spec.ts

@@ -2318,6 +2318,43 @@ describe("RustCrypto", () => {
             expect(dehydratedDeviceIsDeleted).toBeTruthy();
         });
     });
+
+    describe("disableKeyStorage", () => {
+        it("should disable key storage", async () => {
+            const secretStorage = {
+                getDefaultKeyId: jest.fn().mockResolvedValue("bloop"),
+                setDefaultKeyId: jest.fn(),
+                store: jest.fn(),
+            } as unknown as ServerSideSecretStorage;
+
+            fetchMock.get("path:/_matrix/client/v3/room_keys/version", testData.SIGNED_BACKUP_DATA);
+
+            let backupIsDeleted = false;
+            fetchMock.delete("path:/_matrix/client/v3/room_keys/version/1", () => {
+                backupIsDeleted = true;
+                return {};
+            });
+
+            let dehydratedDeviceIsDeleted = false;
+            fetchMock.delete("path:/_matrix/client/unstable/org.matrix.msc3814.v1/dehydrated_device", () => {
+                dehydratedDeviceIsDeleted = true;
+                return { device_id: "ADEVICEID" };
+            });
+
+            const rustCrypto = await makeTestRustCrypto(makeMatrixHttpApi(), undefined, undefined, secretStorage);
+            await rustCrypto.disableKeyStorage();
+
+            expect(secretStorage.store).toHaveBeenCalledWith("m.cross_signing.master", null);
+            expect(secretStorage.store).toHaveBeenCalledWith("m.cross_signing.self_signing", null);
+            expect(secretStorage.store).toHaveBeenCalledWith("m.cross_signing.user_signing", null);
+            expect(secretStorage.store).toHaveBeenCalledWith("m.megolm_backup.v1", null);
+            expect(secretStorage.store).toHaveBeenCalledWith("m.secret_storage.key.bloop", null);
+            expect(secretStorage.setDefaultKeyId).toHaveBeenCalledWith(null);
+
+            expect(backupIsDeleted).toBeTruthy();
+            expect(dehydratedDeviceIsDeleted).toBeTruthy();
+        });
+    });
 });
 
 /** Build a MatrixHttpApi instance */

src/crypto-api/index.ts

@@ -629,6 +629,16 @@ export interface CryptoApi {
      */
     resetKeyBackup(): Promise<void>;
 
+    /**
+     * Disables server-side key storage and deletes server-side backups.
+     *  * Deletes the current key backup version, if any (but not any previous versions).
+     *  * Disables 4S, deleting the info for the default key, the default key pointer itself and any
+     *    known 4S data (cross-signing keys and the megolm key backup key).
+     *  * Deletes any dehydrated devices.
+     *  * Sets the "m.org.matrix.custom.backup_disabled" account data flag to indicate that the user has disabled backups.
+     */
+    disableKeyStorage(): Promise<void>;
+
     /**
      * Deletes the given key backup.
      *

src/http-api/errors.ts

@@ -197,3 +197,18 @@ export class ConnectionError extends Error {
         return "ConnectionError";
     }
 }
+
+/**
+ * Construct a TokenRefreshError. This indicates that a request failed due to the token being expired,
+ * and attempting to refresh said token also failed but in a way which was not indicative of token invalidation.
+ * Assumed to be a temporary failure.
+ */
+export class TokenRefreshError extends Error {
+    public constructor(cause?: Error) {
+        super(cause?.message ?? "");
+    }
+
+    public get name(): string {
+        return "TokenRefreshError";
+    }
+}