From ad5b45587b2e378c15fa879cc72580c391c3c18d Mon Sep 17 00:00:00 2001 From: Asyrique Thevendran Date: Sat, 16 May 2020 14:23:34 +0800 Subject: [PATCH] feat: add auth support for Redis 6 (#1130) --- examples/basic_operations.js | 1 + lib/cluster/ClusterSubscriber.ts | 1 + lib/cluster/util.ts | 1 + lib/connectors/SentinelConnector/index.ts | 2 + lib/redis/RedisOptions.ts | 2 + lib/redis/event_handler.ts | 24 +- lib/redis/index.ts | 6 +- lib/utils/index.ts | 6 +- package.json | 1 + test/functional/auth.ts | 267 ++++++++++++++++++---- test/unit/utils.ts | 2 + 11 files changed, 260 insertions(+), 53 deletions(-) diff --git a/examples/basic_operations.js b/examples/basic_operations.js index 939d64d6..f348c175 100644 --- a/examples/basic_operations.js +++ b/examples/basic_operations.js @@ -2,6 +2,7 @@ const Redis = require("ioredis"); const redis = new Redis({ port: process.env.redisPort, host: process.env.redisEndpoint, + username: process.env.redisUsername, password: process.env.redisPW, }); diff --git a/lib/cluster/ClusterSubscriber.ts b/lib/cluster/ClusterSubscriber.ts index bb5ea48f..1a459013 100644 --- a/lib/cluster/ClusterSubscriber.ts +++ b/lib/cluster/ClusterSubscriber.ts @@ -74,6 +74,7 @@ export default class ClusterSubscriber { this.subscriber = new Redis({ port: options.port, host: options.host, + username: options.username, password: options.password, enableReadyCheck: true, connectionName: SUBSCRIBER_CONNECTION_NAME, diff --git a/lib/cluster/util.ts b/lib/cluster/util.ts index f5e64cff..d6e82037 100644 --- a/lib/cluster/util.ts +++ b/lib/cluster/util.ts @@ -7,6 +7,7 @@ export type NodeRole = "master" | "slave" | "all"; export interface IRedisOptions { port: number; host: string; + username?: string; password?: string; [key: string]: any; } diff --git a/lib/connectors/SentinelConnector/index.ts b/lib/connectors/SentinelConnector/index.ts index b968cd83..bcde400d 100644 --- a/lib/connectors/SentinelConnector/index.ts +++ b/lib/connectors/SentinelConnector/index.ts @@ -35,6 +35,7 @@ export { ISentinelAddress, SentinelIterator }; export interface ISentinelConnectionOptions extends ITcpConnectionOptions { role: "master" | "slave"; name: string; + sentinelUsername?: string; sentinelPassword?: string; sentinels: Array>; sentinelRetryStrategy?: (retryAttempts: number) => number | void | null; @@ -278,6 +279,7 @@ export default class SentinelConnector extends AbstractConnector { const client = new Redis({ port: endpoint.port || 26379, host: endpoint.host, + username: this.options.sentinelUsername || null, password: this.options.sentinelPassword || null, family: endpoint.family || diff --git a/lib/redis/RedisOptions.ts b/lib/redis/RedisOptions.ts index da59aa21..78b6e6fe 100644 --- a/lib/redis/RedisOptions.ts +++ b/lib/redis/RedisOptions.ts @@ -14,6 +14,7 @@ export interface IRedisOptions keepAlive?: number; noDelay?: boolean; connectionName?: string; + username?: string; password?: string; db?: number; dropBufferSupport?: boolean; @@ -50,6 +51,7 @@ export const DEFAULT_REDIS_OPTIONS: IRedisOptions = { enableTLSForSentinelMode: false, updateSentinels: true, // Status + username: null, password: null, db: 0, // Others diff --git a/lib/redis/event_handler.ts b/lib/redis/event_handler.ts index dcda3d00..9932d1eb 100644 --- a/lib/redis/event_handler.ts +++ b/lib/redis/event_handler.ts @@ -25,13 +25,29 @@ export function connectHandler(self) { return; } if (err) { - if (err.message.indexOf("no password is set") === -1) { - flushed = true; - self.recoverFromFatalError(err, err); - } else { + if (err.message.indexOf("no password is set") !== -1) { console.warn( "[WARN] Redis server does not require a password, but a password was supplied." ); + } else if ( + err.message.indexOf( + "without any password configured for the default user" + ) !== -1 + ) { + console.warn( + "[WARN] This Redis server's `default` user does not require a password, but a password was supplied" + ); + } else if ( + err.message.indexOf( + "wrong number of arguments for 'auth' command" + ) !== -1 + ) { + console.warn( + `[ERROR] The server returned "wrong number of arguments for 'auth' command". You are probably passing both username and password to Redis version 5 or below. You should only pass the 'password' option for Redis version 5 and under.` + ); + } else { + flushed = true; + self.recoverFromFatalError(err, err); } } }); diff --git a/lib/redis/index.ts b/lib/redis/index.ts index 0775c81b..8ec832e6 100644 --- a/lib/redis/index.ts +++ b/lib/redis/index.ts @@ -42,6 +42,7 @@ const debug = Debug("redis"); * it to reduce the latency. * @param {string} [options.connectionName=null] - Connection name. * @param {number} [options.db=0] - Database index to use. + * @param {string} [options.username=null] - If set, client will send AUTH command with this user and password when connected. * @param {string} [options.password=null] - If set, client will send AUTH command * with the value of this option when connected. * @param {boolean} [options.dropBufferSupport=false] - Drop the buffer support for better performance. @@ -277,7 +278,10 @@ Redis.prototype.connect = function (callback) { this.condition = { select: options.db, - auth: options.password, + auth: + options.username + ? [options.username, options.password] + : options.password, subscriber: false, }; diff --git a/lib/utils/index.ts b/lib/utils/index.ts index 039c9032..3f9cd434 100644 --- a/lib/utils/index.ts +++ b/lib/utils/index.ts @@ -257,7 +257,11 @@ export function parseURL(url) { const result: any = {}; if (parsed.auth) { - result.password = parsed.auth.split(":")[1]; + const parsedAuth = parsed.auth.split(":"); + if (parsedAuth[0]) { + result.username = parsedAuth[0]; + } + result.password = parsedAuth[1]; } if (parsed.pathname) { if (parsed.protocol === "redis:" || parsed.protocol === "rediss:") { diff --git a/package.json b/package.json index 58d06c53..130403d2 100644 --- a/package.json +++ b/package.json @@ -8,6 +8,7 @@ ], "scripts": { "test": "TS_NODE_TRANSPILE_ONLY=true TS_NODE_LOG_ERROR=true NODE_ENV=test mocha \"test/**/*.ts\"", + "test-single": "TS_NODE_TRANSPILE_ONLY=true TS_NODE_LOG_ERROR=true NODE_ENV=test mocha $1", "lint": "eslint --ext .js,.ts .", "format": "prettier --write \"{,!(node_modules)/**/}*.{js,ts}\"", "format-check": "prettier --check \"{,!(node_modules)/**/}*.{js,ts}\"", diff --git a/test/functional/auth.ts b/test/functional/auth.ts index 92c7d065..7438831e 100644 --- a/test/functional/auth.ts +++ b/test/functional/auth.ts @@ -4,6 +4,7 @@ import Redis from "../../lib/redis"; import * as sinon from "sinon"; describe("auth", function () { + /* General non-Redis-version specific tests */ it("should send auth before other commands", function (done) { let authed = false; new MockServer(17379, (argv) => { @@ -42,67 +43,239 @@ describe("auth", function () { }); }); - it('should not emit "error" when the server doesn\'t need auth', function (done) { - new MockServer(17379, function (argv) { - if (argv[0] === "auth" && argv[1] === "pass") { - return new Error("ERR Client sent AUTH, but no password is set"); - } + describe("auth:redis5-specific", function () { + it("should handle auth with Redis URL string (redis://:foo@bar.com/) correctly", function (done) { + const password = "pass"; + let redis; + new MockServer(17379, function (argv) { + if (argv[0] === "auth" && argv[1] === password) { + redis.disconnect(); + done(); + } + }); + redis = new Redis(`redis://:${password}@localhost:17379/`); }); - let errorEmited = false; - const redis = new Redis({ port: 17379, password: "pass" }); - redis.on("error", function () { - errorEmited = true; + + it('should not emit "error" when the server doesn\'t need auth', function (done) { + new MockServer(17379, function (argv) { + if (argv[0] === "auth" && argv[1] === "pass") { + return new Error("ERR Client sent AUTH, but no password is set"); + } + }); + let errorEmited = false; + const redis = new Redis({ port: 17379, password: "pass" }); + redis.on("error", function () { + errorEmited = true; + }); + const stub = sinon.stub(console, "warn").callsFake((warn) => { + if (warn.indexOf("but a password was supplied") !== -1) { + stub.restore(); + setTimeout(function () { + expect(errorEmited).to.eql(false); + redis.disconnect(); + done(); + }, 0); + } + }); }); - const stub = sinon.stub(console, "warn").callsFake((warn) => { - if (warn.indexOf("but a password was supplied") !== -1) { - stub.restore(); - setTimeout(function () { - expect(errorEmited).to.eql(false); + + it('should emit "error" when the password is wrong', function (done) { + new MockServer(17379, function (argv) { + if (argv[0] === "auth" && argv[1] === "pass") { + return new Error("ERR invalid password"); + } + }); + const redis = new Redis({ port: 17379, password: "pass" }); + let pending = 2; + function check() { + if (!--pending) { redis.disconnect(); done(); - }, 0); + } } + redis.on("error", function (error) { + expect(error).to.have.property("message", "ERR invalid password"); + check(); + }); + redis.get("foo", function (err, res) { + expect(err.message).to.eql("ERR invalid password"); + check(); + }); }); - }); - it('should emit "error" when the password is wrong', function (done) { - new MockServer(17379, function (argv) { - if (argv[0] === "auth" && argv[1] === "pass") { - return new Error("ERR invalid password"); - } - }); - const redis = new Redis({ port: 17379, password: "pass" }); - let pending = 2; - function check() { - if (!--pending) { + it('should emit "error" when password is not provided', function (done) { + new MockServer(17379, function (argv) { + if (argv[0] === "info") { + return new Error("NOAUTH Authentication required."); + } + }); + const redis = new Redis({ port: 17379 }); + redis.on("error", function (error) { + expect(error).to.have.property( + "message", + "NOAUTH Authentication required." + ); redis.disconnect(); done(); - } - } - redis.on("error", function (error) { - expect(error).to.have.property("message", "ERR invalid password"); - check(); + }); }); - redis.get("foo", function (err, res) { - expect(err.message).to.eql("ERR invalid password"); - check(); + + it('should emit "error" when username and password are set for a Redis 5 server', function (done) { + let username = "user"; + let password = "password"; + + new MockServer(17379, function (argv) { + if ( + argv[0] === "auth" && + argv[1] === username && + argv[2] === password + ) { + return new Error("ERR wrong number of arguments for 'auth' command"); + } + }); + + const redis = new Redis({ port: 17379, username, password }); + const stub = sinon.stub(console, "warn").callsFake((warn) => { + if ( + warn.indexOf( + "You are probably passing both username and password to Redis version 5 or below" + ) !== -1 + ) { + stub.restore(); + setTimeout(function () { + redis.disconnect(); + done(); + }, 0); + } + }); }); }); - it('should emit "error" when password is not provided', function (done) { - new MockServer(17379, function (argv) { - if (argv[0] === "info") { - return new Error("NOAUTH Authentication required."); - } + describe("auth:redis6-specific", function () { + /*Redis 6 specific tests */ + it("should handle username and password auth (Redis >=6) correctly", function (done) { + let username = "user"; + let password = "pass"; + let redis; + new MockServer(17379, function (argv) { + if ( + argv[0] === "auth" && + argv[1] === username && + argv[2] === password + ) { + redis.disconnect(); + done(); + } + }); + redis = new Redis({ port: 17379, username, password }); + }); + + it("should handle auth with Redis URL string with username and password (Redis >=6) (redis://foo:bar@baz.com/) correctly", function (done) { + let username = "user"; + let password = "pass"; + let redis; + new MockServer(17379, function (argv) { + if ( + argv[0] === "auth" && + argv[1] === username && + argv[2] === password + ) { + redis.disconnect(); + done(); + } + }); + redis = new Redis(`redis://user:pass@localhost:17379/`); + }); + + it('should not emit "error" when the Redis >=6 server doesn\'t need auth', function (done) { + new MockServer(17379, function (argv) { + if (argv[0] === "auth" && argv[1] === "pass") { + return new Error( + "ERR AUTH called without any password configured for the default user. Are you sure your configuration is correct?" + ); + } + }); + let errorEmited = false; + const redis = new Redis({ port: 17379, password: "pass" }); + redis.on("error", function () { + console.log("boop"); + errorEmited = true; + }); + const stub = sinon.stub(console, "warn").callsFake((warn) => { + if (warn.indexOf("`default` user does not require a password") !== -1) { + stub.restore(); + setTimeout(function () { + expect(errorEmited).to.eql(false); + redis.disconnect(); + done(); + }, 0); + } + }); + }); + + it('should emit "error" when passing username but not password to Redis >=6 instance', function (done) { + let username = "user"; + let password = "pass"; + let redis; + new MockServer(17379, function (argv) { + if (argv[0] === "auth") { + if (argv[1] === username && argv[2] === password) { + return "OK"; + } else { + return new Error("WRONGPASS invalid username-password pair"); + } + } + }); + redis = new Redis({ port: 17379, username }); + redis.on("error", function (error) { + expect(error).to.have.property( + "message", + "WRONGPASS invalid username-password pair" + ); + redis.disconnect(); + done(); + }); }); - const redis = new Redis({ port: 17379 }); - redis.on("error", function (error) { - expect(error).to.have.property( - "message", - "NOAUTH Authentication required." - ); - redis.disconnect(); - done(); + + it('should emit "error" when the password is wrong', function (done) { + let username = "user"; + let password = "pass"; + let redis; + new MockServer(17379, function (argv) { + if (argv[0] === "auth") { + if (argv[1] === username && argv[2] === password) { + return "OK"; + } else { + return new Error("WRONGPASS invalid username-password pair"); + } + } + }); + redis = new Redis({ port: 17379, username, password: "notpass" }); + redis.on("error", function (error) { + expect(error).to.have.property( + "message", + "WRONGPASS invalid username-password pair" + ); + redis.disconnect(); + done(); + }); + }); + + it('should emit "error" when password is required but not provided', function (done) { + new MockServer(17379, function (argv) { + if (argv[0] === "info") { + return new Error("NOAUTH Authentication required."); + } + }); + const redis = new Redis({ port: 17379 }); + redis.on("error", function (error) { + expect(error).to.have.property( + "message", + "NOAUTH Authentication required." + ); + redis.disconnect(); + done(); + }); }); }); }); diff --git a/test/unit/utils.ts b/test/unit/utils.ts index c8acbad0..fda1ba4d 100644 --- a/test/unit/utils.ts +++ b/test/unit/utils.ts @@ -193,6 +193,7 @@ describe("utils", function () { port: "6380", db: "4", password: "pass", + username: "user", key: "value", }); expect(utils.parseURL("redis://127.0.0.1/")).to.eql({ @@ -205,6 +206,7 @@ describe("utils", function () { port: "6380", db: "4", password: "pass", + username: "user", key: "value", }); });