fix(jwt): detect the redis client correctly and branch markUsed on it

.changeset/jwt-redis-store-dialect.md

@@ -0,0 +1,18 @@
+---
+'@exortek/jwt': patch
+---
+
+Fix the Redis store's client detection so the blacklist and refresh registry
+work with both supported clients.
+
+`createStore('redis', …)` identified an `ioredis` client by its constructor
+name, which never matches a real instance — so every ioredis client took the
+node-redis code path and `add()` sent the wrong `SET` argument form.
+Separately, `markUsed()` ignored the detected dialect entirely and always used
+the ioredis `eval` form, which node-redis accepts but executes with no keys.
+Detection now probes the client's API surface, `markUsed()` branches like the
+rest of the store, and `deleteAll()` seeds its `SCAN` cursor as a string
+(node-redis requires this from v6, which the declared peer range admits).
+
+If you previously worked around this by passing `dialect` explicitly, that
+option still works and still takes precedence.

packages/jwt/src/internal/redis-store.js

@@ -14,6 +14,8 @@
  * family membership out of band and delete keys explicitly.
  */
 
+import { isFunction, isString } from '@exortek/shared/predicates';
+
 import { JwtError, ErrorCode } from './errors.js';
 
 // Atomically stamp metadata.usedAt if it is currently null/absent.
@@ -148,7 +150,10 @@ export function createRedisStore(options) {
         );
       }
       const pattern = `${keyPrefix}*`;
-      let cursor = dialect === 'ioredis' ? '0' : 0;
+      // Redis cursors are protocol strings. node-redis typed them as numbers
+      // through v5 but requires a string from v6 on, and the declared peer
+      // range admits v6 — so seed a string for both dialects.
+      let cursor = '0';
       let count = 0;
       try {
         do {
@@ -185,7 +190,7 @@ export function createRedisStore(options) {
               count++;
             }
           }
-        } while (dialect === 'ioredis' ? cursor !== '0' : Number(cursor) !== 0);
+        } while (String(cursor) !== '0');
       } catch (err) {
         if (err instanceof JwtError) {
           throw err;
@@ -200,7 +205,14 @@ export function createRedisStore(options) {
     },
     async markUsed(key, nowSec) {
       try {
-        const result = await client.eval(MARK_USED_LUA, 1, build(key), String(nowSec));
+        // ioredis takes positional EVAL args; node-redis takes an options
+        // object. Passing the positional form to node-redis does not throw —
+        // it sends `EVAL <script> 0`, so the script runs with an empty KEYS
+        // and errors inside Redis instead.
+        const result =
+          dialect === 'ioredis'
+            ? await client.eval(MARK_USED_LUA, 1, build(key), String(nowSec))
+            : await client.eval(MARK_USED_LUA, { keys: [build(key)], arguments: [String(nowSec)] });
         if (result == null) {
           return null;
         }
@@ -237,19 +249,28 @@ export function createRedisStore(options) {
 }
 
 /**
- * Detect whether the client is `ioredis`-style or `redis@4`-style.
- * `ioredis`'s `set` takes positional args (`'EX', ttl`); `redis@4`'s
- * takes an options object. We probe the constructor name; users with
- * a wrapped client can force via `options.dialect` if we add that
- * later.
+ * Detect whether the client is `ioredis`-style or `node-redis`-style.
+ * `ioredis`'s `set` takes positional args (`'EX', ttl`) and its `eval`
+ * takes `(script, numKeys, ...keys, ...args)`; node-redis takes an
+ * options object for both.
+ *
+ * A real `ioredis` instance reports `constructor.name === 'EventEmitter'`
+ * — it extends EventEmitter and does not set a class name that survives
+ * to the instance — so a constructor-name probe alone never matches it.
+ * Detect on the API surface instead (`scanStream` / the `status` string),
+ * both of which node-redis lacks, and keep the name check as a fallback
+ * for wrapped clients. Callers with an exotic client can still force the
+ * answer with `options.dialect`.
  *
  * @param {any} client
  * @returns {'ioredis' | 'node-redis'}
  */
 function _detectDialect(client) {
+  if (client && (isFunction(client.scanStream) || isString(client.status))) {
+    return 'ioredis';
+  }
   const name = (client && client.constructor && client.constructor.name) || '';
   if (name === 'Redis' || name === 'Cluster') {
-    // ioredis — top-level classes are `Redis` and `Cluster`.
     return 'ioredis';
   }
   return 'node-redis';

packages/jwt/tests/stores/redis.integration.test.js

@@ -19,24 +19,8 @@ import { createStore } from '../../src/stores.js';
 
 const nowSec = () => Math.floor(Date.now() / 1000);
 
-// Two known defects, scheduled for the follow-up commit. Each case is marked
-// only for the driver it actually breaks, so the rest keep asserting:
-//
-//   ioredis    — the store misdetects the client as node-redis, so every
-//                SET-with-TTL path (`add`) sends the wrong argument form.
-//   node-redis — `markUsed` never consults the detected dialect, so its Lua
-//                call always uses the ioredis positional form.
-const WRONG_SET_FORM = { todo: { ioredis: 'redis dialect detection — fixed in a follow-up' } };
-const WRONG_EVAL_FORM = { todo: { 'node-redis': 'markUsed dialect branch — fixed in a follow-up' } };
-const BOTH = {
-  todo: {
-    ioredis: 'redis dialect detection — fixed in a follow-up',
-    'node-redis': 'markUsed dialect branch — fixed in a follow-up',
-  },
-};
-
 forEachRedisDriver('jwt redis store', ({ test, client, ns }) => {
-  test('add + has + get round trip', WRONG_SET_FORM, async () => {
+  test('add + has + get round trip', async () => {
     const store = createStore('redis', { client: client(), keyPrefix: ns('a') });
     await store.add('jti-1', nowSec() + 60, { userId: 'u1' });
 
@@ -47,7 +31,7 @@ forEachRedisDriver('jwt redis store', ({ test, client, ns }) => {
     assert.equal(record.metadata.userId, 'u1');
   });
 
-  test('markUsed flips the record once and reports reuse on the second call', BOTH, async () => {
+  test('markUsed flips the record once and reports reuse on the second call', async () => {
     const store = createStore('redis', { client: client(), keyPrefix: ns('b') });
     await store.add('jti-2', nowSec() + 60, {});
 
@@ -58,15 +42,26 @@ forEachRedisDriver('jwt redis store', ({ test, client, ns }) => {
     assert.equal(second.swapped, false, 'second markUsed must report the token as already used');
   });
 
-  test('markUsed on an absent key resolves to null', WRONG_EVAL_FORM, async () => {
+  test('markUsed on an absent key resolves to null', async () => {
     const store = createStore('redis', { client: client(), keyPrefix: ns('c') });
     assert.equal(await store.markUsed('never-added', nowSec()), null);
   });
 
-  test('delete removes the entry', WRONG_SET_FORM, async () => {
+  test('delete removes the entry', async () => {
     const store = createStore('redis', { client: client(), keyPrefix: ns('d') });
     await store.add('jti-3', nowSec() + 60, {});
     await store.delete('jti-3');
     assert.equal(await store.has('jti-3'), false);
   });
+
+  test('deleteAll sweeps entries matching a metadata filter', async () => {
+    const store = createStore('redis', { client: client(), keyPrefix: ns('e') });
+    await store.add('keep', nowSec() + 60, { familyId: 'f2' });
+    await store.add('drop-1', nowSec() + 60, { familyId: 'f1' });
+    await store.add('drop-2', nowSec() + 60, { familyId: 'f1' });
+
+    assert.equal(await store.deleteAll({ familyId: 'f1' }), 2);
+    assert.equal(await store.has('drop-1'), false);
+    assert.equal(await store.has('keep'), true, 'a non-matching entry must survive');
+  });
 });