From 1a8a2c7e6b48131971c18b653d409f65206d695e Mon Sep 17 00:00:00 2001 From: Sonny Date: Tue, 7 Jan 2025 09:10:02 +0100 Subject: [PATCH] fast: Avoid unecessary round trips when the token is invalid (#1046) --- packages/client-core/src/fast/fast.js | 22 ++++++++ .../client-core/src/fast/isTokenValid.test.js | 51 +++++++++++++++++++ packages/sasl2/test.js | 19 +++++-- 3 files changed, 89 insertions(+), 3 deletions(-) create mode 100644 packages/client-core/src/fast/isTokenValid.test.js diff --git a/packages/client-core/src/fast/fast.js b/packages/client-core/src/fast/fast.js index 2b3adbbc..5af90be5 100644 --- a/packages/client-core/src/fast/fast.js +++ b/packages/client-core/src/fast/fast.js @@ -19,6 +19,8 @@ export default function fast({ sasl2 }, { saveToken, fetchToken } = {}) { }; Object.assign(fast, { + mechanism: null, + mechanisms: [], async saveToken() { try { await saveToken(); @@ -43,6 +45,10 @@ export default function fast({ sasl2 }, { saveToken, fetchToken } = {}) { streamFeatures, features, }) { + if (!isTokenValid(token, fast.mechanisms)) { + return false; + } + try { await authenticate({ saslFactory: fast.saslFactory, @@ -79,6 +85,7 @@ export default function fast({ sasl2 }, { saveToken, fetchToken } = {}) { function reset() { fast.mechanism = null; + fast.mechanisms = []; } reset(); @@ -93,6 +100,7 @@ export default function fast({ sasl2 }, { saveToken, fetchToken } = {}) { const mechanism = mechanisms[0]; if (!mechanism) return reset(); + fast.mechanisms = mechanisms; fast.mechanism = mechanism; // The rest is handled by @xmpp/sasl2 @@ -101,6 +109,8 @@ export default function fast({ sasl2 }, { saveToken, fetchToken } = {}) { if (element.is("token", NS)) { try { await saveToken({ + // The token is bound by the mechanism + // > Servers MUST bind tokens to the mechanism selected by the client in its original request, and reject attempts to use them with other mechanisms. mechanism: fast.mechanism, token: element.attrs.token, expiry: element.attrs.expiry, @@ -114,3 +124,15 @@ export default function fast({ sasl2 }, { saveToken, fetchToken } = {}) { return fast; } + +export function isTokenValid(token, mechanisms) { + // Avoid an error round trip if the server does not support the token mechanism anymore + if (!mechanisms.includes(token.mechanism)) { + return false; + } + // Avoid an error round trip if the token is already expired + if (new Date(token.expiry) <= new Date()) { + return false; + } + return true; +} diff --git a/packages/client-core/src/fast/isTokenValid.test.js b/packages/client-core/src/fast/isTokenValid.test.js new file mode 100644 index 00000000..dc20d811 --- /dev/null +++ b/packages/client-core/src/fast/isTokenValid.test.js @@ -0,0 +1,51 @@ +import { isTokenValid } from "./fast.js"; +// eslint-disable-next-line n/no-extraneous-import +import { datetime } from "@xmpp/time"; + +const tomorrow = new Date(); +tomorrow.setDate(tomorrow.getDate() + 1); + +const yesterday = new Date(); +yesterday.setDate(yesterday.getDate() - 1); + +it("returns false if the token.mechanism is not available", async () => { + expect( + isTokenValid( + { + expires: datetime(tomorrow), + mechanism: "bar", + }, + ["foo"], + ), + ); +}); + +it("returns true if the token.mechanism is available", async () => { + expect( + isTokenValid({ expires: datetime(tomorrow), mechanism: "foo" }, ["foo"]), + ); +}); + +it("returns false if the token is expired", async () => { + expect( + isTokenValid( + { + expires: datetime(yesterday), + mechanism: "foo", + }, + ["foo"], + ), + ); +}); + +it("returns true if the token is not expired", async () => { + expect( + isTokenValid( + { + expires: datetime(tomorrow), + mechanism: "foo", + }, + ["foo"], + ), + ); +}); diff --git a/packages/sasl2/test.js b/packages/sasl2/test.js index e311e264..07c4ce80 100644 --- a/packages/sasl2/test.js +++ b/packages/sasl2/test.js @@ -102,12 +102,23 @@ test("with function credentials", async () => { expect(entity.jid.toString()).toBe(jid); }); -test("with FAST token", async () => { +// https://github.com/xmppjs/xmpp.js/pull/1045#discussion_r1904611099 +test("with FAST token only", async () => { const mech = "HT-SHA-256-NONE"; + function onAuthenticate(authenticate, mechanisms, fast) { expect(mechanisms).toEqual([]); expect(fast.mechanism).toEqual(mech); - return authenticate({ token: { token: "hai", mechanism: fast.mechanism } }, null, userAgent); + return authenticate( + { + token: { + token: "hai", + mechanism: fast.mechanism, + }, + }, + null, + userAgent, + ); } const { entity } = mockClient({ credentials: onAuthenticate }); @@ -126,7 +137,9 @@ test("with FAST token", async () => { expect(await promise(entity, "send")).toEqual( - bnVsbACNMNimsTBnxS04m8x7wgKjBHdDUL/nXPU4J4vqxqjBIg== + + bnVsbACNMNimsTBnxS04m8x7wgKjBHdDUL/nXPU4J4vqxqjBIg== + {userAgent} ,