From e1f5c4210faa236f2a38196f13d9a30136732722 Mon Sep 17 00:00:00 2001 From: Sonny Piers Date: Thu, 16 Jan 2025 17:45:25 +0100 Subject: [PATCH] Fix isTokenValid --- packages/client-core/src/fast/fast.js | 33 +++++++++++++------ .../client-core/src/fast/isTokenValid.test.js | 21 +++++++----- packages/sasl2/index.js | 32 ++++++++---------- 3 files changed, 49 insertions(+), 37 deletions(-) diff --git a/packages/client-core/src/fast/fast.js b/packages/client-core/src/fast/fast.js index f48c9364..1599b948 100644 --- a/packages/client-core/src/fast/fast.js +++ b/packages/client-core/src/fast/fast.js @@ -50,12 +50,19 @@ export default function fast({ sasl2, entity }) { authenticate, entity, userAgent, - token, credentials, streamFeatures, features, }) { + // Unavailable + if (!fast.mechanism) { + return false; + } + + const { token } = credentials; + // Invalid or unavailable token if (!isTokenValid(token, fast.mechanisms)) { + requestToken(streamFeatures); return false; } @@ -83,23 +90,25 @@ export default function fast({ sasl2, entity }) { err instanceof SASLError && ["not-authorized", "credentials-expired"].includes(err.condition) ) { - this.delete(); + await this.delete(); + requestToken(streamFeatures); return false; } entity.emit("error", err); return false; } }, - _requestToken(streamFeatures) { - streamFeatures.push( - xml("request-token", { - xmlns: NS, - mechanism: fast.mechanism, - }), - ); - }, }); + function requestToken(streamFeatures) { + streamFeatures.push( + xml("request-token", { + xmlns: NS, + mechanism: fast.mechanism, + }), + ); + } + function reset() { fast.mechanism = null; fast.mechanisms = []; @@ -139,13 +148,17 @@ export default function fast({ sasl2, entity }) { } export function isTokenValid(token, mechanisms) { + if (!token) return false; + // 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 index 61220040..94cf91ff 100644 --- a/packages/client-core/src/fast/isTokenValid.test.js +++ b/packages/client-core/src/fast/isTokenValid.test.js @@ -11,40 +11,45 @@ it("returns false if the token.mechanism is not available", async () => { expect( isTokenValid( { - expires: datetime(tomorrow), + expiry: datetime(tomorrow), mechanism: "bar", }, ["foo"], ), - ); + ).toBe(false); }); it("returns true if the token.mechanism is available", async () => { expect( - isTokenValid({ expires: datetime(tomorrow), mechanism: "foo" }, ["foo"]), - ); + isTokenValid({ expiry: datetime(tomorrow), mechanism: "foo" }, ["foo"]), + ).toBe(true); }); it("returns false if the token is expired", async () => { expect( isTokenValid( { - expires: datetime(yesterday), + expiry: datetime(yesterday), mechanism: "foo", }, ["foo"], ), - ); + ).toBe(false); }); it("returns true if the token is not expired", async () => { expect( isTokenValid( { - expires: datetime(tomorrow), + expiry: datetime(tomorrow), mechanism: "foo", }, ["foo"], ), - ); + ).toBe(true); +}); + +it("returns false if the token is nullish", async () => { + expect(isTokenValid(null)).toBe(false); + expect(isTokenValid(undefined)).toBe(false); }); diff --git a/packages/sasl2/index.js b/packages/sasl2/index.js index f6d2c86a..7cb81975 100644 --- a/packages/sasl2/index.js +++ b/packages/sasl2/index.js @@ -114,26 +114,20 @@ export default function sasl2({ streamFeatures, saslFactory }, onAuthenticate) { ); async function done(credentials, mechanism, userAgent) { - if (fast_available) { - const { token } = credentials; - // eslint-disable-next-line unicorn/no-negated-condition - if (!token) { - fast._requestToken(streamFeatures); - } else { - const success = await fast.auth({ - authenticate, - entity, - userAgent, - token, - streamFeatures, - features, - credentials, - }); - if (success) return; - // If fast authentication fails, continue and try with sasl - } - } + // Try fast + const success = await fast.auth({ + authenticate, + entity, + userAgent, + streamFeatures, + features, + credentials, + }); + if (success) return; + + // fast.auth may mutate streamFeatures to request a token + // If fast authentication fails, continue and try without await authenticate({ entity, userAgent,