feat(auth): add support for function for password

[simong]

Builds on #1985 and fixes the test failures.

PRing mainly to get feedback.

[jit-ci]

Hi, I’m Jit, a friendly security platform designed to help developers build secure applications from day zero with an MVS (Minimal viable security) mindset.

In case there are security findings, they will be communicated to you as a comment inside the PR.

Hope you’ll enjoy using Jit.

Questions? Comments? Want to learn more? Get in touch with us.

[simong]

This is a big change from @slukes work. Rather than use an async promise here, the resolvePassword logic tries to keep everything synchronous when it can. It's only when this.options.password returns a promise that this callback will be invoked in a separate tick.

If this is changed into async logic, there are a few tests that fail because there seems to be an expectation that the connect logic runs the connector which sets this.connecting = true before any other code runs.
e.g., the following test would fail when making resolvePassword async:

  it("connects successfully immediately after end", (done) => {
    const redis = new Redis();
    redis.once("end", async () => {
      await redis.connect();
      done();
    });

    redis.quit();
  });

The connect function (which is called from the constructor) calls into the connector and sets connecting to true on the connector. If that variable is no longer true in the next tick, it rejects the connect request and will trigger a transition into the end state.

However, if connect needs to resolve something else asynchronously, by the time redis.quit runs, the connector may not have run yet. The net effect of this is that the client will first disconnect and then re-connect, because the end-user has no control over the connect.

All of that said, this test could be changed to wait until the client is ready. Would that be breaking behaviour?

  it("connects successfully immediately after end", (done) => {
    const redis = new Redis();
    redis.once("end", async () => {
      await redis.connect();
      done();
    });

    redis.on("ready", () => redis.quit());
  });

[jit-ci]

❌ Jit has detected 1 important finding in this PR that you should review.
The finding is detailed below as a comment.
It’s highly recommended that you fix this security issue before merge.

Repository Risks:

• Database Integration: Connects to a database, often involving sensitive data that must be securely managed.
• Critical Severity Findings: Indicates that the resource has critical severity security findings that need immediate action.
• Internally Accessible: Accessible only within the internal network, reducing exposure to external threats but still requiring proper controls.

Repository Context:

graph LR
    GitHub$Repository_U23_redis/ioredis["GitHub Repository<br/>redis/ioredis"]:::GitHub$Repository
    Team_U23_client_U2D_developers["Team<br/>client-developers"]:::Team
    DBIntegration_U23_redis["DBIntegration<br/>redis"]:::DBIntegration
    Team_U23_client_U2D_developers -- "Owns" --> GitHub$Repository_U23_redis/ioredis
    GitHub$Repository_U23_redis/ioredis -- "Is accessible to" --> DBIntegration_U23_redis

Loading

[jit-ci]

Security control: Secret Detection Trufflehog

Redis

Redis (Unverified)

Severity: HIGH

---

Jit Bot commands and options (e.g., ignore issue)

You can trigger Jit actions by commenting on this PR review:

• #jit_ignore_fp Ignore and mark this specific single instance of finding as “False Positive”
• #jit_ignore_accept Ignore and mark this specific single instance of finding as “Accept Risk”
• #jit_ignore_type_in_file Ignore any finding of type "Redis" in test/functional/auth.ts; future occurrences will also be ignored.
• #jit_undo_ignore Undo ignore command

[simong]

This is another change from @slukes work. This allows pipelining and the offline queue to function.