Extended Composite Auth in #217 is broken

[isnifer]

Prerequisites

• [X] I have written a descriptive issue title
• [X] I have searched existing issues to ensure the bug has not already been reported

Fastify version

4.26.1

Plugin version

4.5.0

Node.js version

20.11.0

Operating system

macOS

Operating system version (i.e. 20.04, 11.3, 10)

14.3.1 (23D60)

Description

217 introduced new rules for auth.

The main problem is that now both paths (in or relation) going in parallel instead of waiting until the first path will fail. I commented all tests except one that clearly showing a problem (and my own production code solution too).

Steps to Reproduce

1. Clone repo https://github.com/isnifer/fastify-auth and go inside (cd fastify-auth)
2. Install deps (npm i)
3. Run tests (npm t)
4. You will see a message from a second auth path in or relation "THIS IS A SECOND AUTH PATH SHOULD NOT BE CALLED" but you SHOULDN'T.

THE ONLY ROUTE

fastify.route({
  method: "POST",
  url: "/check-two-sub-arrays-or",
  preHandler: fastify.auth(
    [[fastify.verifyBigAsync], [fastify.verifyOddAsync]],
    { relation: "or" }
  ),
  handler: (req, reply) => {
    req.log.info("Auth route");
    reply.send({ hello: "world" });
  },
});

TWO AUTH HANDLERS UPDATED WITH "CONSOLE.LOG"

function verifyBigAsync(request, reply) {
  console.log("If you see it — IT IS OK");
  return new Promise((resolve, reject) => {
    verifyBig(request, reply, (err) => {
      if (err) {
        console.log("FIRST_ERROR", err);
        reject(err);
      }
      resolve();
    });
  });
}

function verifyOddAsync(request, reply) {
  console.log("THIS IS A SECOND AUTH PATH SHOULD NOT BE CALLED");
  return new Promise((resolve, reject) => {
    verifyOdd(request, reply, (err) => {
      if (err) reject(err);
      resolve();
    });
  });
}

// A function behind "verifyBigAsync"
function verifyBig(request, reply, done) {
  const n = request.body.n;

  if (typeof n !== "number" || n < 100) {
    request.big = false;
    return done(new Error("`n` is not big"));
  }

  request.big = true;
  return done();
}

THE ONLY TEST WHERE "verifyBigAsync" is passing, BUT "verifyOddAsync" called to

test("Two sub-arrays Or Relation success", (t) => {
  t.plan(2);

  fastify.inject(
    {
      method: "POST",
      url: "/check-two-sub-arrays-or",
      payload: {
        n: 110, // previously 11 which less than 100
      },
    },
    (err, res) => {
      t.error(err);
      const payload = JSON.parse(res.payload);
      t.same(payload, { hello: "world" });
    }
  );
});

Logs from the terminal

npm t

> @fastify/auth@4.5.0 test
> npm run test:unit && npm run test:typescript

> @fastify/auth@4.5.0 test:unit
> tap

 PASS ​ ./test/auth.test.js 29 OK 37.593ms
 PASS ​ ./test/example-async.test.js 18 OK 50.854ms
./test/example-composited.test.js 1> If you see it — IT IS OK
./test/example-composited.test.js 1> THIS IS A SECOND AUTH PATH SHOULD NOT BE CALLED
 PASS ​ ./test/example-composited.test.js 2 OK 17.401ms
 PASS ​ ./test/example.test.js 20 OK 46.829ms

Expected Behavior

No response

[mcollina]

If I understand correctly, the reported breakage is about the "or" function being called in parallel, instead of sequentially?

[isnifer]

@mcollina you're totally right

[mcollina]

How is that a problem?

[isnifer]

@mcollina I want to check second path only then first path is failed.

[isnifer]

@mcollina imagine:

[isnifer]

@mcollina so, the problem is — if it fails in User path, it doesn't care about Admin case, btw User path should not be called at all.

[mcollina]

Can you please craft an example to reproduce that case? Something that would pass before and fail now? Thx

[isnifer]

@mcollina yes, sure. I'll do it on Monday

[yakovenkodenis]

@isnifer @mcollina Here's the PR with a fix and an additional test: https://github.com/fastify/fastify-auth/pull/224

@isnifer Could you please verify that this PR resolves the issue you're having?

[isnifer]

@mcollina repro is here https://github.com/isnifer/fastify-issue-217-example

I tested PR from @yakovenkodenis — it works!