denodrivers/redis

Unable to connect to Redis with correct credentials

iuioiua opened this issue · 12 comments

iuioiua commented

I'm attempting to use a newly created Redis user. However, I'm getting the following error:

error: Uncaught (in promise) Error: -WRONGPASS invalid username-password pair

    throw new ErrorReplyError(line);
          ^
    at tryParseErrorReply (https://deno.land/x/redis@v0.25.4/protocol/reply.ts:205:11)
    at readReply (https://deno.land/x/redis@v0.25.4/protocol/reply.ts:55:7)
    at async RedisConnection.connectThunkified (https://deno.land/x/redis@v0.25.4/connection.ts:97:11)
    at async RedisConnection.connect (https://deno.land/x/redis@v0.25.4/connection.ts:126:5)
    at async connect (https://deno.land/x/redis@v0.25.4/redis.ts:2287:3)
    at async file://<location>

I'm essentially connecting to the following client via the following:

export const REDIS = await connect(parseURL(Deno.env.get("REDIS_URL")!));

When using my Redis default user, this code works fine. However, when using the new user, it doesn't. I know the REDIS_URL environmental variable is correct because I've used it to access Redis via redis-cli -u <url>.

Please let me know if there's any more information you need and thank you in advance.

uki00a commented

@isyouaint Thanks for the report! Could you tell us the format of the URL set in the REDIS_URL variable so we can investigate the cause in more detail?

iuioiua commented

@isyouaint Thanks for the report! Could you tell us the format of the URL set in the REDIS_URL variable so we can investigate the cause in more detail?

As well, I tried connecting without using parseURL and got the same error. Format of the REDIS_URL variable is:

redis://<name>:<password>@<hostname>:<port>
iuioiua commented

@uki00a, I added a username parameter to RedisConnection.authenticate and was able to get this working. Reading this gave me the idea. Hopefully, this helps come to a solution.
Before:

private authenticate(password: string): Promise<RedisReply> {
  return sendCommand(this.writer, this.reader, "AUTH", password);
}

After:

private authenticate(username: string, password: string): Promise<RedisReply> {
  return sendCommand(this.writer, this.reader, "AUTH", username, password);
}
  • 👍1
Xirui commented

@isyouaint I guess you are using Redis 6.

The code in your Before block only works with Redis versions prior of 6.

  • 👀1
iuioiua commented

Any issue with me adding this functionality in a PR?

uki00a commented

@isyouaint A PR will be welcome! 👍

OldManFroggy commented

suggest you update RedisConnectionOptions to support username?: string and also update authenticate function to check for username and adjust redis command as needed for password/default auth "AUTH", password and if exists "AUTH", username, password

not sure if this is being progressed, if not I am more than willing push these changes as a PR let me know, as I am using a modified local repo for production currently with redis cloud.

also I am not sure where to add tests for this as there are connection tests in /tests/commands/general and /tests/commands/connection polite suggestions encouraged ;)

@uki00a @keroxp I can push a PR from my local branch anytime as its done and I am using in production. (ps. never done a PR request/push to opensource before, but my local version <branched as #300> is ready to be pushed as a PR, if I am added as a contributor.)

  • 👍2
uki00a commented

@scott-ling

suggest you update RedisConnectionOptions to support username?: string and also update authenticate function to check for username and adjust redis command as needed for password/default auth "AUTH", password and if exists "AUTH", username, password

Sounds good!

not sure if this is being progressed, if not I am more than willing push these changes as a PR let me know, as I am using a modified local repo for production currently with redis cloud.

I haven't worked on this yet, so you are welcome to submit a PR! 👍

also I am not sure where to add tests for this as there are connection tests in /tests/commands/general and /tests/commands/connection polite suggestions encouraged ;)

I think it would be better to add tests to /tests/commands/general

Please let me know on this issue or on Discord If you have any other questions or issues 🙂

OldManFroggy commented

@uki00a all good, I will add a couple tests for the new situations

  1. password passed no username passed
  2. password and username passed

Will let ya know when its ready to roll, I should be able to complete this tonight once other work is done (I am on GMT)

Thanks for the quick turn-around, this was a breaking issue for me so assume its same for a few others.

-Scott

  • 👍2
iuioiua commented

@scott-ling I haven’t yet started work on a PR, so I guess it’s yours 🤙🏾

  • 👍1
OldManFroggy commented
  • 👍1
OldManFroggy commented

Sorry for delay, PR made.