From f86a4a435b6c81794107c760c559e19c7ff1a74b Mon Sep 17 00:00:00 2001 From: Alberto Ricart Date: Thu, 30 May 2024 13:29:29 -0500 Subject: [PATCH] [FIX] fixed an issue where permission errors related queue subscriptions was not properly notified to the client --- nats-base-client/core.ts | 2 +- nats-base-client/protocol.ts | 8 ++++++- tests/auth_test.ts | 45 ++++++++++++++++++++++++++++++++++++ tests/helpers/launcher.ts | 6 ++++- 4 files changed, 58 insertions(+), 3 deletions(-) diff --git a/nats-base-client/core.ts b/nats-base-client/core.ts index 988e86f5..9228439b 100644 --- a/nats-base-client/core.ts +++ b/nats-base-client/core.ts @@ -143,7 +143,7 @@ export class NatsError extends Error { message: string; // TODO: on major version this should change to a number/enum code: string; - permissionContext?: { operation: string; subject: string }; + permissionContext?: { operation: string; subject: string; queue?: string }; chainedError?: Error; // these are for supporting jetstream api_error?: ApiError; diff --git a/nats-base-client/protocol.ts b/nats-base-client/protocol.ts index 58bd4761..eb1ea3a0 100644 --- a/nats-base-client/protocol.ts +++ b/nats-base-client/protocol.ts @@ -342,7 +342,7 @@ export class Subscriptions { let sub; if (ctx.operation === "subscription") { sub = subs.find((s) => { - return s.subject === ctx.subject; + return s.subject === ctx.subject && s.queue === ctx.queue; }); } if (ctx.operation === "publish") { @@ -700,7 +700,13 @@ export class ProtocolHandler implements Dispatcher { err.permissionContext = { operation: m[1].toLowerCase(), subject: m[2], + queue: undefined, }; + + const qm = s.match(/using queue "(\S+)"/); + if (qm) { + err.permissionContext.queue = qm[1]; + } } return err; } else if (t.indexOf("authorization violation") !== -1) { diff --git a/tests/auth_test.ts b/tests/auth_test.ts index ae3e3b44..79b60d89 100644 --- a/tests/auth_test.ts +++ b/tests/auth_test.ts @@ -17,6 +17,7 @@ import { assertArrayIncludes, assertEquals, assertRejects, + assertStringIncludes, fail, } from "https://deno.land/std@0.221.0/assert/mod.ts"; import { @@ -1254,3 +1255,47 @@ Deno.test("auth - request context", async () => { await cleanup(ns, nc, a); }); + +Deno.test("auth - sub permission queue", async () => { + const conf = { + authorization: { + users: [{ + user: "a", + password: "a", + permissions: { subscribe: ["q A"] }, + }], + }, + }; + + const { ns, nc } = await setup(conf, { user: "a", pass: "a" }); + + const qA = deferred(); + nc.subscribe("q", { + queue: "A", + callback: (err, msg) => { + if (err) { + qA.reject(err); + } + }, + }); + + const qBad = deferred(); + nc.subscribe("q", { + queue: "bad", + callback: (err, msg) => { + if (err) { + qBad.resolve(err); + } + }, + }); + await nc.flush(); + + const err = await qBad; + qA.resolve(); + + await qA; + + assertEquals(err.code, ErrorCode.PermissionsViolation); + assertStringIncludes(err.message, 'using queue "bad"'); + await cleanup(ns, nc); +}); diff --git a/tests/helpers/launcher.ts b/tests/helpers/launcher.ts index 8e0c646f..095d6d1a 100644 --- a/tests/helpers/launcher.ts +++ b/tests/helpers/launcher.ts @@ -792,7 +792,11 @@ export function toConf(o: any, indent?: string): string { buf.push(`${pad}${k}: ${v}`); } } else { - buf.push(pad + v); + if (v.includes(" ")) { + buf.push(`${pad}"${v}"`); + } else { + buf.push(pad + v); + } } } }