Merge pull request 'fix(acl): hard-reject a lapsed token binding instead of prompting (#36)' (#38) from issue-36-expired-binding-hard-reject into dev

Reviewed-on: #38

src/daemon/lib/acl/index.ts

@@ -94,15 +94,6 @@ export async function checkIfPubkeyAllowed(
     // policy has a matching PolicyRule. The live filter is what closes #24:
     // an expired or revoked token simply stops matching here, every request,
     // with no photocopy to outlive it.
-    if (method === 'connect') {
-        const liveToken = await prisma.token.findFirst({
-            where: { keyUserId: keyUser.id, ...live },
-        });
-
-        if (liveToken) {
-            return true;
-        }
-    } else {
     // PolicyRule.kind matching:
     //   - exact match against the stringified payload kind (matches the
     //     create_new_policy.ts storage format `rule.kind.toString()`)
@@ -120,6 +111,24 @@ export async function checkIfPubkeyAllowed(
         kindMatchers.push({ kind: payloadKindString });
     }
 
+    // A token "grants" this request when its policy carries a matching rule —
+    // except `connect`, which any bound token grants (it IS the pairing). This
+    // predicate is reused below to tell a lapsed binding from a never-granted
+    // one (#36).
+    const policyMatch =
+        method === 'connect'
+            ? {}
+            : { policy: { rules: { some: { method, OR: kindMatchers } } } };
+
+    if (method === 'connect') {
+        const liveToken = await prisma.token.findFirst({
+            where: { keyUserId: keyUser.id, ...live },
+        });
+
+        if (liveToken) {
+            return true;
+        }
+    } else {
         // Find live tokens bound to this KeyUser whose policy has at least one
         // rule matching (method, kind), and pull the rules so usage caps can be
         // enforced live off the SigningLog (#28).
@@ -127,7 +136,7 @@ export async function checkIfPubkeyAllowed(
             where: {
                 keyUserId: keyUser.id,
                 ...live,
-                policy: { rules: { some: { method, OR: kindMatchers } } },
+                ...policyMatch,
             },
             include: { policy: { include: { rules: true } } },
         });
@@ -157,7 +166,11 @@ export async function checkIfPubkeyAllowed(
                     },
                 });
                 if (used >= rule.maxUsageCount) {
-                    // Cap exhausted — deny (step 5 / caller may still prompt).
+                    // Cap exhausted. Left as `undefined` deliberately: a windowed
+                    // cap is a temporary rate-limit (it refills as the window
+                    // rolls), not a permanent lapse, so it must NOT be reclassed
+                    // as the re-pair signal below. A dedicated rate-limit reply
+                    // is a separate follow-up to #36.
                     return undefined;
                 }
             }
@@ -165,8 +178,26 @@ export async function checkIfPubkeyAllowed(
         }
     }
 
-    // Step 5: no live override and no live token grant matched. Caller's
-    // requestPermission flow may still prompt the admin out-of-band.
+    // Step 5: no LIVE grant. Distinguish a LAPSED binding — a token that would
+    // have granted this request but is now expired or token-revoked — from a
+    // request that was never granted at all. A lapsed binding hard-rejects
+    // (`false`) so an unattended client (e.g. a spire) re-pairs immediately
+    // instead of hanging on the admin-prompt path; a never-granted request
+    // stays `undefined` so the caller's requestPermission flow can still prompt
+    // an admin to approve a genuinely new permission. See aiolabs/nsecbunkerd#36.
+    const lapsedGrant = await prisma.token.findFirst({
+        where: {
+            keyUserId: keyUser.id,
+            ...policyMatch,
+            OR: [{ revokedAt: { not: null } }, { expiresAt: { lte: now } }],
+        },
+    });
+
+    if (lapsedGrant) {
+        return false;
+    }
+
+    // Step 6: genuinely no grant for this (method, kind).
     return undefined;
 }
 

tests/acl.integration.test.ts

@@ -149,18 +149,25 @@ test('live token + matching policy rule -> sign_event allowed', async () => {
     assert.equal(await checkIfPubkeyAllowed(KEY, PUB, 'sign_event', signEvt), true);
 });
 
-test('expired token -> sign_event denied [#24 regression guard]', async () => {
+// A token bound to the KeyUser that has lapsed (expiry or token-revoke) means
+// the pairing WAS granted and is now spent. It must hard-reject with `false` so
+// an unattended client re-pairs immediately, NOT `undefined` (which routes to
+// the admin-prompt path and hangs an ATM until a BunkerTimeoutError). The smoke
+// on the Sintra proved the divergence: revoke -> clean reject, expiry -> hang.
+// See aiolabs/nsecbunkerd#36 (and #24, which made the expired token stop
+// granting in the first place).
+test('expired token -> sign_event hard-rejected (false) [#24 + #36]', async () => {
     const pid = await seedPolicy('sign_event', '1');
     const ku = await seedKeyUser();
     await seedToken(ku, pid, { expiresAt: past() });
-    assert.equal(await checkIfPubkeyAllowed(KEY, PUB, 'sign_event', signEvt), undefined);
+    assert.equal(await checkIfPubkeyAllowed(KEY, PUB, 'sign_event', signEvt), false);
 });
 
-test('revoked token -> sign_event denied', async () => {
+test('token-revoked -> sign_event hard-rejected (false) [#36]', async () => {
     const pid = await seedPolicy('sign_event', '1');
     const ku = await seedKeyUser();
     await seedToken(ku, pid, { revokedAt: past() });
-    assert.equal(await checkIfPubkeyAllowed(KEY, PUB, 'sign_event', signEvt), undefined);
+    assert.equal(await checkIfPubkeyAllowed(KEY, PUB, 'sign_event', signEvt), false);
 });
 
 test('live token -> connect allowed (pairing)', async () => {
@@ -170,11 +177,31 @@ test('live token -> connect allowed (pairing)', async () => {
     assert.equal(await checkIfPubkeyAllowed(KEY, PUB, 'connect'), true);
 });
 
-test('expired token -> connect denied', async () => {
+test('expired token -> connect hard-rejected (false) [#36]', async () => {
     const pid = await seedPolicy('sign_event', '1');
     const ku = await seedKeyUser();
     await seedToken(ku, pid, { expiresAt: past() });
-    assert.equal(await checkIfPubkeyAllowed(KEY, PUB, 'connect'), undefined);
+    assert.equal(await checkIfPubkeyAllowed(KEY, PUB, 'connect'), false);
+});
+
+// The reject above is reserved for a binding that LAPSED. A request whose method
+// was never in the (still-live) token's policy is genuinely new permission and
+// must stay `undefined` so an admin could approve it out-of-band — it must NOT
+// be swept up by the #36 re-pair signal.
+test('live token, method outside its policy -> undefined (never granted, not lapsed) [#36]', async () => {
+    const pid = await seedPolicy('sign_event', '1');
+    const ku = await seedKeyUser();
+    await seedToken(ku, pid, {});
+    assert.equal(await checkIfPubkeyAllowed(KEY, PUB, 'nip44_encrypt'), undefined);
+});
+
+// And a lapsed token only rejects the method IT covered: a different method has
+// no lapsed grant of its own, so it stays a never-granted `undefined`.
+test('expired token, request a method it never covered -> undefined [#36]', async () => {
+    const pid = await seedPolicy('sign_event', '1');
+    const ku = await seedKeyUser();
+    await seedToken(ku, pid, { expiresAt: past() });
+    assert.equal(await checkIfPubkeyAllowed(KEY, PUB, 'nip44_encrypt'), undefined);
 });
 
 test('KeyUser.revokedAt denies (false) and beats a live token', async () => {