Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fast: Avoid unecessary round trips when the token is invalid #1046

Merged
merged 4 commits into from
Jan 7, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions packages/client-core/src/fast/fast.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ export default function fast({ sasl2 }, { saveToken, fetchToken } = {}) {
};

Object.assign(fast, {
mechanism: null,
mechanisms: [],
async saveToken() {
try {
await saveToken();
Expand All @@ -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,
Expand Down Expand Up @@ -79,6 +85,7 @@ export default function fast({ sasl2 }, { saveToken, fetchToken } = {}) {

function reset() {
fast.mechanism = null;
fast.mechanisms = [];
}
reset();

Expand All @@ -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
Expand All @@ -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,
Expand All @@ -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;
}
51 changes: 51 additions & 0 deletions packages/client-core/src/fast/isTokenValid.test.js
Original file line number Diff line number Diff line change
@@ -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"],
),
);
});
19 changes: 16 additions & 3 deletions packages/sasl2/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 });
Expand All @@ -126,7 +137,9 @@ test("with FAST token", async () => {

expect(await promise(entity, "send")).toEqual(
<authenticate xmlns="urn:xmpp:sasl:2" mechanism={mech}>
<initial-response>bnVsbACNMNimsTBnxS04m8x7wgKjBHdDUL/nXPU4J4vqxqjBIg==</initial-response>
<initial-response>
bnVsbACNMNimsTBnxS04m8x7wgKjBHdDUL/nXPU4J4vqxqjBIg==
</initial-response>
{userAgent}
<fast xmlns="urn:xmpp:fast:0" />
</authenticate>,
Expand Down
Loading