Skip to content

Commit

Permalink
Add support for Redis 6 auth pass [user] (#1508)
Browse files Browse the repository at this point in the history
* Add support for `auth pass user`

* fix lint issues

* fix typo

* fix more lint issues

* more lints fixes

* reverse password user order

* update redis-commands

* Update individualCommands.js

Clean code

* Update individualCommands.js

* Update auth.spec.js

* Update index.js

Co-authored-by: Leibale Eidelman <leibale1998@gmail.com>
  • Loading branch information
gkorland and leibale committed Mar 17, 2021
1 parent bb208d0 commit 428e1c8
Show file tree
Hide file tree
Showing 4 changed files with 26 additions and 9 deletions.
3 changes: 2 additions & 1 deletion index.js
Expand Up @@ -109,6 +109,7 @@ function RedisClient (options, stream) {
this.closing = false;
this.server_info = {};
this.auth_pass = options.auth_pass || options.password;
this.auth_user = options.auth_user || options.user;
this.selected_db = options.db; // Save the selected db here, used when reconnecting
this.fire_strings = true; // Determine if strings or buffers should be written to the stream
this.pipeline = false;
Expand Down Expand Up @@ -240,7 +241,7 @@ RedisClient.prototype.create_stream = function () {
if (this.auth_pass !== undefined) {
this.ready = true;
// Fail silently as we might not be able to connect
this.auth(this.auth_pass, function (err) {
this.auth(this.auth_pass, this.auth_user, function (err) {
if (err && err.code !== 'UNCERTAIN_STATE') {
self.emit('error', err);
}
Expand Down
6 changes: 5 additions & 1 deletion lib/createClient.js
Expand Up @@ -29,7 +29,11 @@ module.exports = function createClient (port_arg, host_arg, options) {
// [redis:]//[[user][:password]@][host][:port][/db-number][?db=db-number[&password=bar[&option=value]]]
if (parsed.slashes) { // We require slashes
if (parsed.auth) {
options.password = parsed.auth.slice(parsed.auth.indexOf(':') + 1);
var columnIndex = parsed.auth.indexOf(':');
options.password = parsed.auth.slice(columnIndex + 1);
if (columnIndex > 0) {
options.user = parsed.auth.slice(0, columnIndex);
}
}
if (parsed.protocol) {
if (parsed.protocol === 'rediss:') {
Expand Down
24 changes: 18 additions & 6 deletions lib/individualCommands.js
Expand Up @@ -180,7 +180,7 @@ Multi.prototype.info = Multi.prototype.INFO = function info (section, callback)
return this;
};

function auth_callback (self, pass, callback) {
function auth_callback (self, pass, user, callback) {
return function (err, res) {
if (err) {
if (no_password_is_set.test(err.message)) {
Expand All @@ -191,7 +191,7 @@ function auth_callback (self, pass, callback) {
// If redis is still loading the db, it will not authenticate and everything else will fail
debug('Redis still loading, trying to authenticate later');
setTimeout(function () {
self.auth(pass, callback);
self.auth(user, pass, callback);

This comment has been minimized.

Copy link
@guyroyse

guyroyse Mar 31, 2021

Contributor

Is this in the correct order? Everything else is pass, user.

}, 100);
return;
}
Expand All @@ -200,25 +200,37 @@ function auth_callback (self, pass, callback) {
};
}

RedisClient.prototype.auth = RedisClient.prototype.AUTH = function auth (pass, callback) {
RedisClient.prototype.auth = RedisClient.prototype.AUTH = function auth (pass, user, callback) {
debug('Sending auth to ' + this.address + ' id ' + this.connection_id);

// Backward compatibility support for auth with password only

This comment has been minimized.

Copy link
@guyroyse

guyroyse Mar 31, 2021

Contributor

I wonder if this might be cleaner if you used arguments instead.

if (user instanceof Function) {
callback = user;
user = null;
}
// Stash auth for connect and reconnect.
this.auth_pass = pass;
this.auth_user = user;
var ready = this.ready;
this.ready = ready || this.offline_queue.length === 0;
var tmp = this.internal_send_command(new Command('auth', [pass], auth_callback(this, pass, callback)));
var tmp = this.internal_send_command(new Command('auth', user ? [user, pass] : [pass], auth_callback(this, pass, user, callback)));
this.ready = ready;
return tmp;
};

// Only works with batch, not in a transaction
Multi.prototype.auth = Multi.prototype.AUTH = function auth (pass, callback) {
Multi.prototype.auth = Multi.prototype.AUTH = function auth (pass, user, callback) {
debug('Sending auth to ' + this.address + ' id ' + this.connection_id);

// Backward compatibility support for auth with password only

This comment has been minimized.

Copy link
@guyroyse

guyroyse Mar 31, 2021

Contributor

Same comment here. Arguments?

if (user instanceof Function) {
callback = user;
user = null;
}
// Stash auth for connect and reconnect.
this.auth_pass = pass;
this.queue.push(new Command('auth', [pass], auth_callback(this._client, callback)));
this.auth_user = user;
this.queue.push(new Command('auth', user ? [user, pass] : [pass], auth_callback(this._client, pass, user, callback)));
return this;
};

Expand Down
2 changes: 1 addition & 1 deletion test/auth.spec.js
Expand Up @@ -61,7 +61,7 @@ describe('client authentication', function () {
});
var tmp = client.command_queue.get(0).callback;
client.command_queue.get(0).callback = function (err, res) {
client.auth = function (pass, callback) {
client.auth = function (pass, user, callback) {
callback(null, 'retry worked');
};
tmp(new Error('ERR redis is still LOADING'));
Expand Down

0 comments on commit 428e1c8

Please sign in to comment.