Merge pull request #967 from supercaracal/fix-auth-for-acl

byroot · web-flow · commit c449816bd9f4 · 2021-01-06T14:57:20.000+01:00
Support AUTH command for ACL
diff --git a/README.md b/README.md
@@ -54,6 +54,12 @@ To connect to a password protected Redis instance, use:
 redis = Redis.new(password: "mysecret")
 ```
 
+To connect a Redis instance using [ACL](https://redis.io/topics/acl), use:
+
+```ruby
+redis = Redis.new(username: 'myname', password: 'mysecret')
+```
+
 The Redis class exports methods that are named identical to the commands
 they execute. The arguments these methods accept are often identical to
 the arguments specified on the [Redis website][redis-commands]. For
diff --git a/lib/redis.rb b/lib/redis.rb
@@ -39,6 +39,7 @@ def self.current
   # @option options [String] :path path to server socket (overrides host and port)
   # @option options [Float] :timeout (5.0) timeout in seconds
   # @option options [Float] :connect_timeout (same as timeout) timeout for initial connect in seconds
+  # @option options [String] :username Username to authenticate against server
   # @option options [String] :password Password to authenticate against server
   # @option options [Integer] :db (0) Database to select after initial connect
   # @option options [Symbol] :driver Driver to use, currently supported: `:ruby`, `:hiredis`, `:synchrony`
@@ -143,12 +144,13 @@ def _client
 
   # Authenticate to the server.
   #
-  # @param [String] password must match the password specified in the
-  #   `requirepass` directive in the configuration file
+  # @param [Array<String>] args includes both username and password
+  #   or only password
   # @return [String] `OK`
-  def auth(password)
+  # @see https://redis.io/commands/auth AUTH command
+  def auth(*args)
     synchronize do |client|
-      client.call([:auth, password])
+      client.call([:auth, *args])
     end
   end
 
diff --git a/lib/redis/client.rb b/lib/redis/client.rb
@@ -17,6 +17,7 @@ class Client
       write_timeout: nil,
       connect_timeout: nil,
       timeout: 5.0,
+      username: nil,
       password: nil,
       db: 0,
       driver: nil,
@@ -61,6 +62,10 @@ def timeout
       @options[:read_timeout]
     end
 
+    def username
+      @options[:username]
+    end
+
     def password
       @options[:password]
     end
@@ -110,7 +115,7 @@ def connect
       # Don't try to reconnect when the connection is fresh
       with_reconnect(false) do
         establish_connection
-        call [:auth, password] if password
+        call [:auth, username, password].compact if username || password
         call [:select, db] if db != 0
         call [:client, :setname, @options[:id]] if @options[:id]
         @connector.check(self)
@@ -434,7 +439,8 @@ def _parse_options(options)
           defaults[:scheme]   = uri.scheme
           defaults[:host]     = uri.host if uri.host
           defaults[:port]     = uri.port if uri.port
-          defaults[:password] = CGI.unescape(uri.password) if uri.password
+          defaults[:username] = CGI.unescape(uri.user) if uri.user && !uri.user.empty?
+          defaults[:password] = CGI.unescape(uri.password) if uri.password && !uri.password.empty?
           defaults[:db]       = uri.path[1..-1].to_i if uri.path
           defaults[:role] = :master
         else
@@ -579,6 +585,7 @@ def sentinel_detect
             client = Client.new(@options.merge({
                                                  host: sentinel[:host] || sentinel["host"],
                                                  port: sentinel[:port] || sentinel["port"],
+                                                 username: sentinel[:username] || sentinel["username"],
                                                  password: sentinel[:password] || sentinel["password"],
                                                  reconnect_attempts: 0
                                                }))
diff --git a/lib/redis/cluster.rb b/lib/redis/cluster.rb
@@ -128,7 +128,7 @@ def fetch_command_details(nodes)
     def send_command(command, &block)
       cmd = command.first.to_s.downcase
       case cmd
-      when 'auth', 'bgrewriteaof', 'bgsave', 'quit', 'save'
+      when 'acl', 'auth', 'bgrewriteaof', 'bgsave', 'quit', 'save'
         @node.call_all(command, &block).first
       when 'flushall', 'flushdb'
         @node.call_master(command, &block).first
diff --git a/lib/redis/cluster/option.rb b/lib/redis/cluster/option.rb
@@ -18,6 +18,7 @@ def initialize(options)
         @node_opts = build_node_options(node_addrs)
         @replica = options.delete(:replica) == true
         add_common_node_option_if_needed(options, @node_opts, :scheme)
+        add_common_node_option_if_needed(options, @node_opts, :username)
         add_common_node_option_if_needed(options, @node_opts, :password)
         @options = options
       end
@@ -63,7 +64,9 @@ def parse_node_url(addr)
         raise InvalidClientOptionError, "Invalid uri scheme #{addr}" unless VALID_SCHEMES.include?(uri.scheme)
 
         db = uri.path.split('/')[1]&.to_i
-        { scheme: uri.scheme, password: uri.password, host: uri.host, port: uri.port, db: db }.reject { |_, v| v.nil? }
+
+        { scheme: uri.scheme, username: uri.user, password: uri.password, host: uri.host, port: uri.port, db: db }
+          .reject { |_, v| v.nil? || v == '' }
       rescue URI::InvalidURIError => err
         raise InvalidClientOptionError, err.message
       end
@@ -79,7 +82,7 @@ def parse_node_option(addr)
 
       # Redis cluster node returns only host and port information.
       # So we should complement additional information such as:
-      #   scheme, password and so on.
+      #   scheme, username, password and so on.
       def add_common_node_option_if_needed(options, node_opts, key)
         return options if options[key].nil? && node_opts.first[key].nil?
 
diff --git a/test/cluster_client_internals_test.rb b/test/cluster_client_internals_test.rb
@@ -74,4 +74,23 @@ def test_connection
 
     assert_equal expected, redis.connection
   end
+
+  def test_acl_auth_success
+    target_version "6.0.0" do
+      with_acl do |username, password|
+        r = _new_client(cluster: DEFAULT_PORTS.map { |port| "redis://#{username}:#{password}@#{DEFAULT_HOST}:#{port}" })
+        assert_equal('PONG', r.ping)
+      end
+    end
+  end
+
+  def test_acl_auth_failure
+    target_version "6.0.0" do
+      with_acl do |username, _|
+        assert_raises(Redis::CannotConnectError) do
+          _new_client(cluster: DEFAULT_PORTS.map { |port| "redis://#{username}:wrongpassword@#{DEFAULT_HOST}:#{port}" })
+        end
+      end
+    end
+  end
 end
diff --git a/test/cluster_client_options_test.rb b/test/cluster_client_options_test.rb
@@ -20,11 +20,14 @@ def test_option_class
     assert_equal false, option.use_replica?
 
     option = Redis::Cluster::Option.new(cluster: %w[rediss://johndoe:foobar@127.0.0.1:7000/1/namespace])
-    assert_equal({ '127.0.0.1:7000' => { scheme: 'rediss', password: 'foobar', host: '127.0.0.1', port: 7000, db: 1 } }, option.per_node_key)
+    assert_equal({ '127.0.0.1:7000' => { scheme: 'rediss', username: 'johndoe', password: 'foobar', host: '127.0.0.1', port: 7000, db: 1 } }, option.per_node_key)
 
     option = Redis::Cluster::Option.new(cluster: %w[rediss://127.0.0.1:7000], scheme: 'redis')
     assert_equal({ '127.0.0.1:7000' => { scheme: 'rediss', host: '127.0.0.1', port: 7000 } }, option.per_node_key)
 
+    option = Redis::Cluster::Option.new(cluster: %w[redis://bazzap:@127.0.0.1:7000], username: 'foobar')
+    assert_equal({ '127.0.0.1:7000' => { scheme: 'redis', username: 'bazzap', host: '127.0.0.1', port: 7000 } }, option.per_node_key)
+
     option = Redis::Cluster::Option.new(cluster: %w[redis://:bazzap@127.0.0.1:7000], password: 'foobar')
     assert_equal({ '127.0.0.1:7000' => { scheme: 'redis', password: 'bazzap', host: '127.0.0.1', port: 7000 } }, option.per_node_key)
 
diff --git a/test/cluster_commands_on_connection_test.rb b/test/cluster_commands_on_connection_test.rb
@@ -1,17 +1,13 @@
 # frozen_string_literal: true
 
 require_relative 'helper'
+require 'lint/authentication'
 
 # ruby -w -Itest test/cluster_commands_on_connection_test.rb
 # @see https://redis.io/commands#connection
 class TestClusterCommandsOnConnection < Minitest::Test
   include Helper::Cluster
-
-  def test_auth
-    redis_cluster_mock(auth: ->(*_) { '+OK' }) do |redis|
-      assert_equal 'OK', redis.auth('my-password-123')
-    end
-  end
+  include Lint::Authentication
 
   def test_echo
     assert_equal 'hogehoge', redis.echo('hogehoge')
@@ -37,4 +33,8 @@ def test_swapdb
       redis.swapdb(1, 2)
     end
   end
+
+  def mock(*args, &block)
+    redis_cluster_mock(*args, &block)
+  end
 end
diff --git a/test/connection_handling_test.rb b/test/connection_handling_test.rb
@@ -1,20 +1,11 @@
 # frozen_string_literal: true
 
 require_relative "helper"
+require 'lint/authentication'
 
 class TestConnectionHandling < Minitest::Test
   include Helper::Client
-
-  def test_auth
-    commands = {
-      auth: ->(password) { @auth = password; "+OK" },
-      get: ->(_key) { @auth == "secret" ? "$3\r\nbar" : "$-1" }
-    }
-
-    redis_mock(commands, password: "secret") do |redis|
-      assert_equal "bar", redis.get("foo")
-    end
-  end
+  include Lint::Authentication
 
   def test_id
     commands = {
diff --git a/test/helper.rb b/test/helper.rb
@@ -165,6 +165,16 @@ def omit_version(min_ver)
     def version
       Version.new(redis.info['redis_version'])
     end
+
+    def with_acl
+      admin = _new_client
+      admin.acl('SETUSER', 'johndoe', 'on',
+                '+ping', '+select', '+command', '+cluster|slots', '+cluster|nodes',
+                '>mysecret')
+      yield('johndoe', 'mysecret')
+      admin.acl('DELUSER', 'johndoe')
+      admin.close
+    end
   end
 
   module Client
diff --git a/test/lint/authentication.rb b/test/lint/authentication.rb
@@ -0,0 +1,30 @@
+# frozen_string_literal: true
+
+module Lint
+  module Authentication
+    def test_auth_with_password
+      mock(auth: ->(*_) { '+OK' }) do |r|
+        assert_equal 'OK', r.auth('mysecret')
+      end
+
+      mock(auth: ->(*_) { '-ERR some error' }) do |r|
+        assert_raises(Redis::BaseError) { r.auth('mysecret') }
+      end
+    end
+
+    def test_auth_for_acl
+      target_version "6.0.0" do
+        with_acl do |username, password|
+          assert_raises(Redis::BaseError) { redis.auth(username, 'wrongpassword') }
+          assert_equal 'OK', redis.auth(username, password)
+          assert_equal 'PONG', redis.ping
+          assert_raises(Redis::BaseError) { redis.echo('foo') }
+        end
+      end
+    end
+
+    def mock(*args, &block)
+      redis_mock(*args, &block)
+    end
+  end
+end
diff --git a/test/sentinel_test.rb b/test/sentinel_test.rb
@@ -257,6 +257,49 @@ def test_authentication_for_sentinel_and_redis
     assert_equal [%w[auth bar], %w[role]], commands[:m1]
   end
 
+  def test_authentication_with_acl
+    commands = { s1: [], m1: [] }
+
+    sentinel = lambda do |port|
+      {
+        auth: lambda do |user, pass|
+          commands[:s1] << ['auth', user, pass]
+          '+OK'
+        end,
+        select: lambda do |db|
+          commands[:s1] << ['select', db]
+          '-ERR unknown command `select`'
+        end,
+        sentinel: lambda do |command, *args|
+          commands[:s1] << [command, *args]
+          ['127.0.0.1', port.to_s]
+        end
+      }
+    end
+
+    master = {
+      auth: lambda do |user, pass|
+        commands[:m1] << ['auth', user, pass]
+        '+OK'
+      end,
+      role: lambda do
+        commands[:m1] << ['role']
+        ['master']
+      end
+    }
+
+    RedisMock.start(master) do |master_port|
+      RedisMock.start(sentinel.call(master_port)) do |sen_port|
+        s = [{ host: '127.0.0.1', port: sen_port, username: 'bob', password: 'foo' }]
+        r = Redis.new(host: 'master1', sentinels: s, role: :master, username: 'alice', password: 'bar')
+        assert r.ping
+      end
+    end
+
+    assert_equal [%w[auth bob foo], %w[get-master-addr-by-name master1]], commands[:s1]
+    assert_equal [%w[auth alice bar], %w[role]], commands[:m1]
+  end
+
   def test_sentinel_role_mismatch
     sentinels = [{ host: "127.0.0.1", port: 26_381 }]
 
diff --git a/test/url_param_test.rb b/test/url_param_test.rb
@@ -134,4 +134,15 @@ def test_defaults_to_localhost
 
     assert_equal "127.0.0.1", redis._client.host
   end
+
+  def test_user_and_password
+    redis = Redis.new(url: 'redis://johndoe:mysecret@foo.com:999/2')
+
+    assert_equal('redis', redis._client.scheme)
+    assert_equal('johndoe', redis._client.username)
+    assert_equal('mysecret', redis._client.password)
+    assert_equal('foo.com', redis._client.host)
+    assert_equal(999, redis._client.port)
+    assert_equal(2, redis._client.db)
+  end
 end
