Use correct /v3 prefix for /refresh (#3016)

* Add tests to ensure /v3/refresh is called + automatic /v1 retry

* Request /refresh with v3 prefix, and quietly fall back to v1

* Add tests checking re-raising errors

* Update spec/unit/login.spec.ts

* Update comment

Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>

---------

Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>

spec/unit/login.spec.ts

@@ -1,6 +1,15 @@
+import fetchMock from "fetch-mock-jest";
+
+import { ClientPrefix, MatrixClient } from "../../src";
 import { SSOAction } from "../../src/@types/auth";
 import { TestClient } from "../TestClient";
 
+function createExampleMatrixClient(): MatrixClient {
+    return new MatrixClient({
+        baseUrl: "https://example.com",
+    });
+}
+
 describe("Login request", function () {
     let client: TestClient;
 
@@ -57,3 +66,84 @@ describe("SSO login URL", function () {
         });
     });
 });
+
+describe("refreshToken", () => {
+    afterEach(() => {
+        fetchMock.mockReset();
+    });
+
+    it("requests the correctly-prefixed /refresh endpoint when server correctly accepts /v3", async () => {
+        const client = createExampleMatrixClient();
+
+        const response = {
+            access_token: "access_token",
+            refresh_token: "refresh_token",
+            expires_in_ms: 30000,
+        };
+
+        fetchMock.postOnce(client.http.getUrl("/refresh", undefined, ClientPrefix.V3).toString(), response);
+        fetchMock.postOnce(client.http.getUrl("/refresh", undefined, ClientPrefix.V1).toString(), () => {
+            throw new Error("/v1/refresh unexpectedly called");
+        });
+
+        const refreshResult = await client.refreshToken("initial_refresh_token");
+        expect(refreshResult).toEqual(response);
+    });
+
+    it("falls back to /v1 when server does not recognized /v3 refresh", async () => {
+        const client = createExampleMatrixClient();
+
+        const response = {
+            access_token: "access_token",
+            refresh_token: "refresh_token",
+            expires_in_ms: 30000,
+        };
+
+        fetchMock.postOnce(client.http.getUrl("/refresh", undefined, ClientPrefix.V3).toString(), {
+            status: 400,
+            body: { errcode: "M_UNRECOGNIZED" },
+        });
+        fetchMock.postOnce(client.http.getUrl("/refresh", undefined, ClientPrefix.V1).toString(), response);
+
+        const refreshResult = await client.refreshToken("initial_refresh_token");
+        expect(refreshResult).toEqual(response);
+    });
+
+    it("re-raises M_UNRECOGNIZED exceptions from /v1", async () => {
+        const client = createExampleMatrixClient();
+
+        fetchMock.postOnce(client.http.getUrl("/refresh", undefined, ClientPrefix.V3).toString(), {
+            status: 400,
+            body: { errcode: "M_UNRECOGNIZED" },
+        });
+        fetchMock.postOnce(client.http.getUrl("/refresh", undefined, ClientPrefix.V1).toString(), {
+            status: 400,
+            body: { errcode: "M_UNRECOGNIZED" },
+        });
+
+        expect(client.refreshToken("initial_refresh_token")).rejects.toMatchObject({ errcode: "M_UNRECOGNIZED" });
+    });
+
+    it("re-raises non-M_UNRECOGNIZED exceptions from /v3", async () => {
+        const client = createExampleMatrixClient();
+
+        fetchMock.postOnce(client.http.getUrl("/refresh", undefined, ClientPrefix.V3).toString(), 429);
+        fetchMock.postOnce(client.http.getUrl("/refresh", undefined, ClientPrefix.V1).toString(), () => {
+            throw new Error("/v1/refresh unexpectedly called");
+        });
+
+        expect(client.refreshToken("initial_refresh_token")).rejects.toMatchObject({ httpStatus: 429 });
+    });
+
+    it("re-raises non-M_UNRECOGNIZED exceptions from /v1", async () => {
+        const client = createExampleMatrixClient();
+
+        fetchMock.postOnce(client.http.getUrl("/refresh", undefined, ClientPrefix.V3).toString(), {
+            status: 400,
+            body: { errcode: "M_UNRECOGNIZED" },
+        });
+        fetchMock.postOnce(client.http.getUrl("/refresh", undefined, ClientPrefix.V1).toString(), 429);
+
+        expect(client.refreshToken("initial_refresh_token")).rejects.toMatchObject({ httpStatus: 429 });
+    });
+});

src/client.ts

@@ -7689,16 +7689,27 @@ export class MatrixClient extends TypedEventEmitter<EmittedEvents, ClientEventHa
      * @returns Rejects with an error response.
      */
     public refreshToken(refreshToken: string): Promise<IRefreshTokenResponse> {
-        return this.http.authedRequest(
+        const performRefreshRequestWithPrefix = (prefix: ClientPrefix): Promise<IRefreshTokenResponse> =>
-            Method.Post,
+            this.http.authedRequest(
-            "/refresh",
+                Method.Post,
-            undefined,
+                "/refresh",
-            { refresh_token: refreshToken },
+                undefined,
-            {
+                { refresh_token: refreshToken },
-                prefix: ClientPrefix.V1,
+                {
-                inhibitLogoutEmit: true, // we don't want to cause logout loops
+                    prefix,
-            },
+                    inhibitLogoutEmit: true, // we don't want to cause logout loops
-        );
+                },
+            );
+
+        // First try with the (specced) /v3/ prefix.
+        // However, before Synapse 1.72.0, Synapse incorrectly required a /v1/ prefix, so we fall
+        // back to that if the request fails, for backwards compatibility.
+        return performRefreshRequestWithPrefix(ClientPrefix.V3).catch((e) => {
+            if (e.errcode === "M_UNRECOGNIZED") {
+                return performRefreshRequestWithPrefix(ClientPrefix.V1);
+            }
+            throw e;
+        });
     }
 
     /**