Skip to content

Commit

Permalink
Merge pull request #701 from nats-io/fix-sub-queue-perm-errors
Browse files Browse the repository at this point in the history
[FIX] fixed an issue where permission errors related queue subscriptions was not properly notified to the client
  • Loading branch information
aricart authored May 30, 2024
2 parents 8b52908 + f86a4a4 commit 7661902
Show file tree
Hide file tree
Showing 4 changed files with 58 additions and 3 deletions.
2 changes: 1 addition & 1 deletion nats-base-client/core.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
8 changes: 7 additions & 1 deletion nats-base-client/protocol.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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") {
Expand Down Expand Up @@ -700,7 +700,13 @@ export class ProtocolHandler implements Dispatcher<ParserEvent> {
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) {
Expand Down
45 changes: 45 additions & 0 deletions tests/auth_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import {
assertArrayIncludes,
assertEquals,
assertRejects,
assertStringIncludes,
fail,
} from "https://deno.land/[email protected]/assert/mod.ts";
import {
Expand Down Expand Up @@ -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<NatsError>();
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);
});
6 changes: 5 additions & 1 deletion tests/helpers/launcher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
}
}
Expand Down

0 comments on commit 7661902

Please sign in to comment.