From e82500b2119e4754017e8891e0b23f9dd5f3d33f Mon Sep 17 00:00:00 2001 From: Hugh Nimmo-Smith Date: Fri, 26 May 2023 18:54:54 +0100 Subject: [PATCH 1/7] Support for stable MSC3882 get_login_token --- spec/integ/matrix-client-methods.spec.ts | 16 +++++------ spec/unit/rendezvous/rendezvous.spec.ts | 32 ++++++++++----------- src/@types/auth.ts | 7 ++--- src/client.ts | 36 ++++++++++++++++-------- src/rendezvous/MSC3906Rendezvous.ts | 13 +++------ 5 files changed, 54 insertions(+), 50 deletions(-) diff --git a/spec/integ/matrix-client-methods.spec.ts b/spec/integ/matrix-client-methods.spec.ts index 5a026f7ff40..065e0937f3f 100644 --- a/spec/integ/matrix-client-methods.spec.ts +++ b/spec/integ/matrix-client-methods.spec.ts @@ -1206,13 +1206,11 @@ describe("MatrixClient", function () { it("should hit the expected API endpoint with UIA", async () => { httpBackend .when("GET", "/capabilities") - .respond(200, { capabilities: { "org.matrix.msc3882.get_login_token": { enabled: true } } }); + .respond(200, { capabilities: { "m.get_login_token": { enabled: true } } }); const response = {}; const uiaData = {}; const prom = client.requestLoginToken(uiaData); - httpBackend - .when("POST", "/unstable/org.matrix.msc3882/login/get_token", { auth: uiaData }) - .respond(200, response); + httpBackend.when("POST", "/v1/login/get_token", { auth: uiaData }).respond(200, response); await httpBackend.flush(""); expect(await prom).toStrictEqual(response); }); @@ -1220,22 +1218,22 @@ describe("MatrixClient", function () { it("should hit the expected API endpoint without UIA", async () => { httpBackend .when("GET", "/capabilities") - .respond(200, { capabilities: { "org.matrix.msc3882.get_login_token": { enabled: true } } }); + .respond(200, { capabilities: { "m.get_login_token": { enabled: true } } }); const response = { login_token: "xyz", expires_in_ms: 5000 }; const prom = client.requestLoginToken(); - httpBackend.when("POST", "/unstable/org.matrix.msc3882/login/get_token", {}).respond(200, response); + httpBackend.when("POST", "/v1/login/get_token", {}).respond(200, response); await httpBackend.flush(""); // check that expires_in has been populated for compatibility with r0 expect(await prom).toStrictEqual({ ...response, expires_in: 5 }); }); - it("should hit the r1 endpoint when capability is disabled", async () => { + it("should hit the stable endpoint when capability is disabled", async () => { httpBackend .when("GET", "/capabilities") - .respond(200, { capabilities: { "org.matrix.msc3882.get_login_token": { enabled: false } } }); + .respond(200, { capabilities: { "m.get_login_token": { enabled: false } } }); const response = { login_token: "xyz", expires_in_ms: 5000 }; const prom = client.requestLoginToken(); - httpBackend.when("POST", "/unstable/org.matrix.msc3882/login/get_token", {}).respond(200, response); + httpBackend.when("POST", "/v1/login/get_token", {}).respond(200, response); await httpBackend.flush(""); // check that expires_in has been populated for compatibility with r0 expect(await prom).toStrictEqual({ ...response, expires_in: 5 }); diff --git a/spec/unit/rendezvous/rendezvous.spec.ts b/spec/unit/rendezvous/rendezvous.spec.ts index ba36005092b..33ca7090341 100644 --- a/spec/unit/rendezvous/rendezvous.spec.ts +++ b/spec/unit/rendezvous/rendezvous.spec.ts @@ -37,7 +37,7 @@ function makeMockClient(opts: { userId: string; deviceId: string; deviceKey?: string; - msc3882Enabled: boolean; + getLoginTokenEnabled: boolean; msc3882r0Only: boolean; msc3886Enabled: boolean; devices?: Record>; @@ -54,7 +54,7 @@ function makeMockClient(opts: { getVersions() { return { unstable_features: { - "org.matrix.msc3882": opts.msc3882Enabled, + "org.matrix.msc3882": opts.getLoginTokenEnabled, "org.matrix.msc3886": opts.msc3886Enabled, }, }; @@ -64,8 +64,8 @@ function makeMockClient(opts: { ? {} : { capabilities: { - "org.matrix.msc3882.get_login_token": { - enabled: opts.msc3882Enabled, + "m.get_login_token": { + enabled: opts.getLoginTokenEnabled, }, }, }; @@ -122,7 +122,7 @@ describe("Rendezvous", function () { userId: "@alice:example.com", deviceId: "DEVICEID", msc3886Enabled: false, - msc3882Enabled: true, + getLoginTokenEnabled: true, msc3882r0Only: true, }); httpBackend.when("POST", "https://fallbackserver/rz").response = { @@ -180,10 +180,10 @@ describe("Rendezvous", function () { }); async function testNoProtocols({ - msc3882Enabled, + getLoginTokenEnabled, msc3882r0Only, }: { - msc3882Enabled: boolean; + getLoginTokenEnabled: boolean; msc3882r0Only: boolean; }) { const aliceTransport = makeTransport("Alice"); @@ -198,7 +198,7 @@ describe("Rendezvous", function () { userId: "alice", deviceId: "ALICE", msc3886Enabled: false, - msc3882Enabled, + getLoginTokenEnabled, msc3882r0Only, }); const aliceEcdh = new MSC3903ECDHRendezvousChannel(aliceTransport, undefined, aliceOnFailure); @@ -241,11 +241,11 @@ describe("Rendezvous", function () { } it("no protocols - r0", async function () { - await testNoProtocols({ msc3882Enabled: false, msc3882r0Only: true }); + await testNoProtocols({ getLoginTokenEnabled: false, msc3882r0Only: true }); }); - it("no protocols - r1", async function () { - await testNoProtocols({ msc3882Enabled: false, msc3882r0Only: false }); + it("no protocols - stable", async function () { + await testNoProtocols({ getLoginTokenEnabled: false, msc3882r0Only: false }); }); it("new device declines protocol with outcome unsupported", async function () { @@ -260,7 +260,7 @@ describe("Rendezvous", function () { const alice = makeMockClient({ userId: "alice", deviceId: "ALICE", - msc3882Enabled: true, + getLoginTokenEnabled: true, msc3882r0Only: false, msc3886Enabled: false, }); @@ -319,7 +319,7 @@ describe("Rendezvous", function () { const alice = makeMockClient({ userId: "alice", deviceId: "ALICE", - msc3882Enabled: true, + getLoginTokenEnabled: true, msc3882r0Only: false, msc3886Enabled: false, }); @@ -378,7 +378,7 @@ describe("Rendezvous", function () { const alice = makeMockClient({ userId: "alice", deviceId: "ALICE", - msc3882Enabled: true, + getLoginTokenEnabled: true, msc3882r0Only: false, msc3886Enabled: false, }); @@ -439,7 +439,7 @@ describe("Rendezvous", function () { const alice = makeMockClient({ userId: "alice", deviceId: "ALICE", - msc3882Enabled: true, + getLoginTokenEnabled: true, msc3882r0Only: false, msc3886Enabled: false, }); @@ -508,7 +508,7 @@ describe("Rendezvous", function () { const alice = makeMockClient({ userId: "alice", deviceId: "ALICE", - msc3882Enabled: true, + getLoginTokenEnabled: true, msc3882r0Only: false, msc3886Enabled: false, devices, diff --git a/src/@types/auth.ts b/src/@types/auth.ts index ac0f6c9d4af..ea431d93f8c 100644 --- a/src/@types/auth.ts +++ b/src/@types/auth.ts @@ -243,9 +243,8 @@ export interface LoginResponse { } /** - * The result of a successful [MSC3882](https://github.com/matrix-org/matrix-spec-proposals/pull/3882) - * `m.login.token` issuance request. - * Note that this is UNSTABLE and subject to breaking changes without notice. + * The result of a successful `m.login.token` issuance request as per https://spec.matrix.org/v1.7/client-server-api/#post_matrixclientv1loginget_token + * */ export interface LoginTokenPostResponse { /** @@ -255,7 +254,7 @@ export interface LoginTokenPostResponse { /** * Expiration in seconds. * - * @deprecated this is only provided for compatibility with original revision of the MSC. + * @deprecated this is only provided for compatibility with original revision of [MSC3882](https://github.com/matrix-org/matrix-spec-proposals/pull/3882). */ expires_in: number; /** diff --git a/src/client.ts b/src/client.ts index e99be6d7a65..82adf49394a 100644 --- a/src/client.ts +++ b/src/client.ts @@ -201,7 +201,7 @@ import { threadFilterTypeToFilter, } from "./models/thread"; import { M_BEACON_INFO, MBeaconInfoEventContent } from "./@types/beacon"; -import { UnstableValue } from "./NamespacedValue"; +import { NamespacedValue, UnstableValue } from "./NamespacedValue"; import { ToDeviceMessageQueue } from "./ToDeviceMessageQueue"; import { ToDeviceBatch } from "./models/ToDeviceMessage"; import { IgnoredInvites } from "./models/invites-ignorer"; @@ -519,9 +519,12 @@ export interface IChangePasswordCapability extends ICapability {} export interface IThreadsCapability extends ICapability {} -export interface IMSC3882GetLoginTokenCapability extends ICapability {} +export interface IGetLoginTokenCapability extends ICapability {} -export const UNSTABLE_MSC3882_CAPABILITY = new UnstableValue("m.get_login_token", "org.matrix.msc3882.get_login_token"); +export const GET_LOGIN_TOKEN_CAPABILITY = new NamespacedValue( + "m.get_login_token", + "org.matrix.msc3882.get_login_token", +); export const UNSTABLE_MSC2666_SHARED_ROOMS = "uk.half-shot.msc2666"; export const UNSTABLE_MSC2666_MUTUAL_ROOMS = "uk.half-shot.msc2666.mutual_rooms"; @@ -536,8 +539,8 @@ export interface Capabilities { "m.change_password"?: IChangePasswordCapability; "m.room_versions"?: IRoomVersionsCapability; "io.element.thread"?: IThreadsCapability; - [UNSTABLE_MSC3882_CAPABILITY.name]?: IMSC3882GetLoginTokenCapability; - [UNSTABLE_MSC3882_CAPABILITY.altName]?: IMSC3882GetLoginTokenCapability; + "m.get_login_token"?: IGetLoginTokenCapability; + "org.matrix.msc3882.get_login_token"?: IGetLoginTokenCapability; } /** @deprecated prefer {@link CrossSigningKeyInfo}. */ @@ -8002,7 +8005,9 @@ export class MatrixClient extends TypedEventEmitter> { // use capabilities to determine which revision of the MSC is being used const capabilities = await this.getCapabilities(); - // use r1 endpoint if capability is exposed otherwise use old r0 endpoint - const endpoint = UNSTABLE_MSC3882_CAPABILITY.findIn(capabilities) - ? "/org.matrix.msc3882/login/get_token" // r1 endpoint - : "/org.matrix.msc3882/login/token"; // r0 endpoint + + const usesStable = !!capabilities["m.get_login_token"]; + const usesUnstableRevision1 = !usesStable && !!capabilities["org.matrix.msc3882.get_login_token"]; + // use stable endpoint if capability is exposed otherwise use old unstable r0 endpoint + const endpoint = + usesStable || usesUnstableRevision1 + ? "/login/get_token" // endpoint for stable + unstable r1 + : "/login/token"; // unstable r0 endpoint + + // determine whether to use stable or unstable prefix + const prefix = usesStable ? ClientPrefix.V1 : `${ClientPrefix.Unstable}/org.matrix.msc3882`; const body: UIARequest<{}> = { auth }; const res = await this.http.authedRequest>( @@ -8021,10 +8033,10 @@ export class MatrixClient extends TypedEventEmitter(capabilities); + const capability = GET_LOGIN_TOKEN_CAPABILITY.findIn(capabilities); // determine available protocols if (!capability?.enabled && features.get(Feature.LoginTokenRequest) === ServerSupport.Unsupported) { - logger.info("Server doesn't support MSC3882"); + logger.info("Server doesn't support get_login_token"); await this.send({ type: PayloadType.Finish, outcome: Outcome.Unsupported }); await this.cancel(RendezvousFailureReason.HomeserverLacksSupport); return undefined; From c7c0b5fda412e40c259ceecd33668b772ce5073a Mon Sep 17 00:00:00 2001 From: Hugh Nimmo-Smith Date: Wed, 27 Sep 2023 09:50:43 +0100 Subject: [PATCH 2/7] Make changes non-breaking by deprecation --- src/client.ts | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/client.ts b/src/client.ts index 82adf49394a..c94dde82c80 100644 --- a/src/client.ts +++ b/src/client.ts @@ -521,11 +521,21 @@ export interface IThreadsCapability extends ICapability {} export interface IGetLoginTokenCapability extends ICapability {} +/** + * @deprecated use {@link IGetLoginTokenCapability} instead + */ +export type IMSC3882GetLoginTokenCapability = IGetLoginTokenCapability; + export const GET_LOGIN_TOKEN_CAPABILITY = new NamespacedValue( "m.get_login_token", "org.matrix.msc3882.get_login_token", ); +/** + * @deprecated use {@link GET_LOGIN_TOKEN_CAPABILITY} instead + */ +export const UNSTABLE_MSC3882_CAPABILITY = GET_LOGIN_TOKEN_CAPABILITY; + export const UNSTABLE_MSC2666_SHARED_ROOMS = "uk.half-shot.msc2666"; export const UNSTABLE_MSC2666_MUTUAL_ROOMS = "uk.half-shot.msc2666.mutual_rooms"; export const UNSTABLE_MSC2666_QUERY_MUTUAL_ROOMS = "uk.half-shot.msc2666.query_mutual_rooms"; From df2bd039009d5e69f3309574e8bd21a5a91f8bc0 Mon Sep 17 00:00:00 2001 From: Hugh Nimmo-Smith Date: Wed, 27 Sep 2023 17:20:51 +0100 Subject: [PATCH 3/7] Update src/@types/auth.ts Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> --- src/@types/auth.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/@types/auth.ts b/src/@types/auth.ts index ea431d93f8c..61b4f70aa03 100644 --- a/src/@types/auth.ts +++ b/src/@types/auth.ts @@ -244,7 +244,6 @@ export interface LoginResponse { /** * The result of a successful `m.login.token` issuance request as per https://spec.matrix.org/v1.7/client-server-api/#post_matrixclientv1loginget_token - * */ export interface LoginTokenPostResponse { /** From d4a1fc61ba70ff26ea0522fb0bc381b0cc8d7b4a Mon Sep 17 00:00:00 2001 From: Hugh Nimmo-Smith Date: Wed, 27 Sep 2023 17:21:06 +0100 Subject: [PATCH 4/7] Update spec/integ/matrix-client-methods.spec.ts Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> --- spec/integ/matrix-client-methods.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/integ/matrix-client-methods.spec.ts b/spec/integ/matrix-client-methods.spec.ts index 065e0937f3f..e919f67fba9 100644 --- a/spec/integ/matrix-client-methods.spec.ts +++ b/spec/integ/matrix-client-methods.spec.ts @@ -1227,7 +1227,7 @@ describe("MatrixClient", function () { expect(await prom).toStrictEqual({ ...response, expires_in: 5 }); }); - it("should hit the stable endpoint when capability is disabled", async () => { + it("should still hit the stable endpoint when capability is disabled (but present)", async () => { httpBackend .when("GET", "/capabilities") .respond(200, { capabilities: { "m.get_login_token": { enabled: false } } }); From d6d9036c5434527435b15f693a1e4539d6aaa810 Mon Sep 17 00:00:00 2001 From: Hugh Nimmo-Smith Date: Wed, 27 Sep 2023 17:25:34 +0100 Subject: [PATCH 5/7] Suggestions from review --- src/client.ts | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/src/client.ts b/src/client.ts index c94dde82c80..984fe0c6148 100644 --- a/src/client.ts +++ b/src/client.ts @@ -8026,16 +8026,17 @@ export class MatrixClient extends TypedEventEmitter = { auth }; const res = await this.http.authedRequest>( @@ -8043,7 +8044,6 @@ export class MatrixClient extends TypedEventEmitter Date: Thu, 28 Sep 2023 13:48:59 +0100 Subject: [PATCH 6/7] Update src/client.ts Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> --- src/client.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/client.ts b/src/client.ts index 984fe0c6148..816cf3ad0bf 100644 --- a/src/client.ts +++ b/src/client.ts @@ -8030,7 +8030,7 @@ export class MatrixClient extends TypedEventEmitter Date: Thu, 28 Sep 2023 16:18:16 +0100 Subject: [PATCH 7/7] Fix and test prefix behaviour --- spec/integ/matrix-client-methods.spec.ts | 24 ++++++++++++++++++++++++ src/client.ts | 1 + 2 files changed, 25 insertions(+) diff --git a/spec/integ/matrix-client-methods.spec.ts b/spec/integ/matrix-client-methods.spec.ts index e919f67fba9..eb3578b9328 100644 --- a/spec/integ/matrix-client-methods.spec.ts +++ b/spec/integ/matrix-client-methods.spec.ts @@ -1204,6 +1204,7 @@ describe("MatrixClient", function () { describe("requestLoginToken", () => { it("should hit the expected API endpoint with UIA", async () => { + jest.spyOn(client.http, "getUrl"); httpBackend .when("GET", "/capabilities") .respond(200, { capabilities: { "m.get_login_token": { enabled: true } } }); @@ -1213,9 +1214,15 @@ describe("MatrixClient", function () { httpBackend.when("POST", "/v1/login/get_token", { auth: uiaData }).respond(200, response); await httpBackend.flush(""); expect(await prom).toStrictEqual(response); + expect(client.http.getUrl).toHaveLastReturnedWith( + expect.objectContaining({ + href: "http://alice.localhost.test.server/_matrix/client/v1/login/get_token", + }), + ); }); it("should hit the expected API endpoint without UIA", async () => { + jest.spyOn(client.http, "getUrl"); httpBackend .when("GET", "/capabilities") .respond(200, { capabilities: { "m.get_login_token": { enabled: true } } }); @@ -1225,9 +1232,15 @@ describe("MatrixClient", function () { await httpBackend.flush(""); // check that expires_in has been populated for compatibility with r0 expect(await prom).toStrictEqual({ ...response, expires_in: 5 }); + expect(client.http.getUrl).toHaveLastReturnedWith( + expect.objectContaining({ + href: "http://alice.localhost.test.server/_matrix/client/v1/login/get_token", + }), + ); }); it("should still hit the stable endpoint when capability is disabled (but present)", async () => { + jest.spyOn(client.http, "getUrl"); httpBackend .when("GET", "/capabilities") .respond(200, { capabilities: { "m.get_login_token": { enabled: false } } }); @@ -1237,9 +1250,15 @@ describe("MatrixClient", function () { await httpBackend.flush(""); // check that expires_in has been populated for compatibility with r0 expect(await prom).toStrictEqual({ ...response, expires_in: 5 }); + expect(client.http.getUrl).toHaveLastReturnedWith( + expect.objectContaining({ + href: "http://alice.localhost.test.server/_matrix/client/v1/login/get_token", + }), + ); }); it("should hit the r0 endpoint for fallback", async () => { + jest.spyOn(client.http, "getUrl"); httpBackend.when("GET", "/capabilities").respond(200, {}); const response = { login_token: "xyz", expires_in: 5 }; const prom = client.requestLoginToken(); @@ -1247,6 +1266,11 @@ describe("MatrixClient", function () { await httpBackend.flush(""); // check that expires_in has been populated for compatibility with r1 expect(await prom).toStrictEqual({ ...response, expires_in_ms: 5000 }); + expect(client.http.getUrl).toHaveLastReturnedWith( + expect.objectContaining({ + href: "http://alice.localhost.test.server/_matrix/client/unstable/org.matrix.msc3882/login/token", + }), + ); }); }); diff --git a/src/client.ts b/src/client.ts index 816cf3ad0bf..5a5d454dff1 100644 --- a/src/client.ts +++ b/src/client.ts @@ -8044,6 +8044,7 @@ export class MatrixClient extends TypedEventEmitter