fast: Delete token when it is invalid (#1055)

eslint.config.js

@@ -58,8 +58,6 @@ export default [
       // node
       // https://github.com/eslint-community/eslint-plugin-n/
       "n/no-unpublished-require": "off", // doesn't play nice with monorepo
-      "n/no-extraneous-require": ["error", { allowModules: ["@xmpp/test"] }],
-      "n/no-extraneous-import": ["error", { allowModules: ["@xmpp/test"] }],
       "n/hashbang": "off",
 
       // promise
@@ -109,6 +107,21 @@ export default [
       //   },
       // ],
       "promise/no-callback-in-promise": "off",
+      // "n/no-extraneous-require": ["error", { allowModules: ["@xmpp/test"] }],
+      "n/no-extraneous-import": [
+        "error",
+        {
+          allowModules: [
+            "@xmpp/test",
+            "@xmpp/time",
+            "@xmpp/xml",
+            "@xmpp/connection",
+            "@xmpp/websocket",
+            "selfsigned",
+            "@xmpp/events",
+          ],
+        },
+      ],
     },
   },
 ];

packages/client-core/src/fast/README.md

@@ -6,7 +6,7 @@ By default `@xmpp/fast` stores the token in memory and as such fast authenticati
 
 You can supply your own functions to store and retrieve the token from a persistent database.
 
-If fast authentication fails, regular authentication with `credentials` will happen.
+If fast authentication fails, regular authentication will happen.
 
 ## Usage
 
@@ -26,10 +26,9 @@ client.fast.saveToken = async (token) => {
   await secureStorage.set("token", JSON.stringify(token));
 }
 
-// Debugging only
-client.fast.on("error", (error) => {
-  console.log("fast error", error);
-})
+client.fast.removeToken = async () => {
+  await secureStorage.del("token");
+}
 ```
 
 ## References

packages/client-core/src/fast/fast.js

@@ -1,5 +1,6 @@
 import { EventEmitter } from "@xmpp/events";
 import { getAvailableMechanisms } from "@xmpp/sasl";
+import SASLError from "@xmpp/sasl/lib/SASLError.js";
 import xml from "@xmpp/xml";
 import SASLFactory from "saslmechanisms";
 
@@ -20,6 +21,9 @@ export default function fast({ sasl2, entity }) {
     async fetchToken() {
       return token;
     },
+    async deleteToken() {
+      token = null;
+    },
     async save(token) {
       try {
         await this.saveToken(token);
@@ -34,17 +38,31 @@ export default function fast({ sasl2, entity }) {
         entity.emit("error", err);
       }
     },
+    async delete() {
+      try {
+        await this.deleteToken();
+      } catch (err) {
+        entity.emit("error", err);
+      }
+    },
     saslFactory,
     async auth({
       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;
       }
 
@@ -68,20 +86,29 @@ export default function fast({ sasl2, entity }) {
         });
         return true;
       } catch (err) {
+        if (
+          err instanceof SASLError &&
+          ["not-authorized", "credentials-expired"].includes(err.condition)
+        ) {
+          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 = [];
@@ -121,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;
 }

packages/client-core/src/fast/isTokenValid.test.js

@@ -1,5 +1,4 @@
 import { isTokenValid } from "./fast.js";
-// eslint-disable-next-line n/no-extraneous-import
 import { datetime } from "@xmpp/time";
 
 const tomorrow = new Date();
@@ -12,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);
 });

packages/client-core/test/fast.js

@@ -0,0 +1,118 @@
+import { tick } from "@xmpp/events";
+import { mockClient } from "@xmpp/test";
+import { datetime } from "@xmpp/time";
+import { Element } from "@xmpp/xml";
+
+const mechanism = "HT-SHA-256-NONE";
+
+test("requests and saves token if server advertises fast", async () => {
+  const { entity, fast } = mockClient();
+
+  const spy_saveToken = jest.spyOn(fast, "saveToken");
+
+  entity.mockInput(
+    <features xmlns="http://etherx.jabber.org/streams">
+      <authentication xmlns="urn:xmpp:sasl:2">
+        <mechanism>PLAIN</mechanism>
+        <inline>
+          <fast xmlns="urn:xmpp:fast:0">
+            <mechanism>{mechanism}</mechanism>
+          </fast>
+        </inline>
+      </authentication>
+    </features>,
+  );
+
+  const authenticate = await entity.catchOutgoing();
+  expect(authenticate.is("authenticate", "urn:xmpp:sasl:2")).toBe(true);
+  const request_token = authenticate.getChild(
+    "request-token",
+    "urn:xmpp:fast:0",
+  );
+  expect(request_token.attrs.mechanism).toBe(mechanism);
+
+  const token = "secret-token:fast-HZzFpFwHTy4nc3C8Y1NVNZqYef_7Q3YjMLu2";
+  const expiry = "2025-02-06T09:58:40.774329Z";
+
+  expect(spy_saveToken).not.toHaveBeenCalled();
+
+  entity.mockInput(
+    <success xmlns="urn:xmpp:sasl:2">
+      <token expiry={expiry} xmlns="urn:xmpp:fast:0" token={token} />
+      <authorization-identifier>
+        username@localhost/rOYwkWIywtnF
+      </authorization-identifier>
+    </success>,
+  );
+
+  expect(spy_saveToken).toHaveBeenCalledWith({ token, expiry, mechanism });
+});
+
+async function setupFast() {
+  const { entity, fast } = mockClient();
+
+  const d = new Date();
+  d.setFullYear(d.getFullYear() + 1);
+  const expiry = datetime(d);
+
+  fast.fetchToken = async () => {
+    return {
+      mechanism,
+      expiry,
+      token: "foobar",
+    };
+  };
+
+  entity.mockInput(
+    <features xmlns="http://etherx.jabber.org/streams">
+      <authentication xmlns="urn:xmpp:sasl:2">
+        <mechanism>PLAIN</mechanism>
+        <inline>
+          <fast xmlns="urn:xmpp:fast:0">
+            <mechanism>{mechanism}</mechanism>
+          </fast>
+        </inline>
+      </authentication>
+    </features>,
+  );
+
+  expect(fast.mechanism).toBe(mechanism);
+
+  const authenticate = await entity.catchOutgoing();
+  expect(authenticate.is("authenticate", "urn:xmpp:sasl:2"));
+  expect(authenticate.attrs.mechanism).toBe(mechanism);
+  expect(authenticate.getChild("fast", "urn:xmpp:fast:0")).toBeInstanceOf(
+    Element,
+  );
+
+  return entity;
+}
+
+test("deletes the token if server replies with not-authorized", async () => {
+  const entity = await setupFast();
+  const spy_deleteToken = jest.spyOn(entity.fast, "deleteToken");
+
+  expect(spy_deleteToken).not.toHaveBeenCalled();
+  entity.mockInput(
+    <failure xmlns="urn:xmpp:sasl:2">
+      <not-authorized xmlns="urn:ietf:params:xml:ns:xmpp-sasl" />
+    </failure>,
+  );
+  await tick();
+  expect(spy_deleteToken).toHaveBeenCalled();
+});
+
+test("deletes the token if server replies with credentials-expired", async () => {
+  const entity = await setupFast();
+  const spy_deleteToken = jest.spyOn(entity.fast, "deleteToken");
+
+  // credentials-expired
+  expect(spy_deleteToken).not.toHaveBeenCalled();
+  entity.mockInput(
+    <failure xmlns="urn:xmpp:sasl:2">
+      <credentials-expired xmlns="urn:ietf:params:xml:ns:xmpp-sasl" />
+    </failure>,
+  );
+  await tick();
+  expect(spy_deleteToken).toHaveBeenCalled();
+});

packages/error/test.js

@@ -1,5 +1,4 @@
 import XMPPError from "./index.js";
-// eslint-disable-next-line n/no-extraneous-import
 import parse from "@xmpp/xml/lib/parse.js";
 
 test("fromElement", () => {

packages/events/index.js

@@ -9,6 +9,12 @@ import procedure from "./lib/procedure.js";
 import listeners from "./lib/listeners.js";
 import onoff from "./lib/onoff.js";
 
+function tick() {
+  return new Promise((resolve) => {
+    process.nextTick(resolve);
+  });
+}
+
 export {
   EventEmitter,
   timeout,
@@ -19,4 +25,5 @@ export {
   procedure,
   listeners,
   onoff,
+  tick,
 };

packages/reconnect/test.js

@@ -1,5 +1,4 @@
 import _reconnect from "./index.js";
-// eslint-disable-next-line n/no-extraneous-import
 import Connection from "@xmpp/connection";
 
 test("schedules a reconnect when disconnect is emitted", () => {