fix(plugins/acme): username/password is a valid authentication method

Fixed an issue where username and password were not accepted as a valid authentication method. This is already accepted as valid authentication method in other plugins that use the shared Redis library such as the rate-limiting plugin. Depends on this PR of lua-resty-acme: fffonion/lua-resty-acme#121 Fix FTI-6143
Kong · Aug 14, 2024 · 500ca0d · 500ca0d
1 parent 9bc3deb
commit 500ca0d
Show file tree

Hide file tree

Showing 6 changed files with 151 additions and 1 deletion.
diff --git a/.github/workflows/build_and_test.yml b/.github/workflows/build_and_test.yml
@@ -198,6 +198,19 @@ jobs:
         ports:
           - 9411:9411
 
+      redis-auth:
+        image: redis/redis-stack-server
+        # Set health checks to wait until redis has started
+        options: >-
+          --health-cmd "redis-cli ping"
+          --health-interval 10s
+          --health-timeout 5s
+          --health-retries 5
+        ports:
+          - 6381:6379
+        env:
+          REDIS_ARGS: "--requirepass passdefault"
+
     steps:
     - name: Bump max open files
       run: |

diff --git a/changelog/unreleased/kong/fix-acme-username-password-auth.yml b/changelog/unreleased/kong/fix-acme-username-password-auth.yml
@@ -0,0 +1,3 @@
+message: "**ACME**: Fixed an issue where username and password were not accepted as valid authentication methods."
+type: bugfix
+scope: Plugin
diff --git a/kong-3.8.0-0.rockspec b/kong-3.8.0-0.rockspec
@@ -38,7 +38,7 @@ dependencies = {
   "lua-resty-gcp == 0.0.13",
   "lua-resty-counter == 0.2.1",
   "lua-resty-ipmatcher == 0.6.1",
-  "lua-resty-acme == 0.14.0",
+  "lua-resty-acme == 0.15.0",
   "lua-resty-session == 4.0.5",
   "lua-resty-timer-ng == 0.2.7",
   "lpeg == 1.1.0",

diff --git a/kong/plugins/acme/storage/config_adapters/redis.lua b/kong/plugins/acme/storage/config_adapters/redis.lua
@@ -7,6 +7,8 @@ local function redis_config_adapter(conf)
         ssl = conf.ssl,
         ssl_verify = conf.ssl_verify,
         ssl_server_name = conf.server_name,
+        username = conf.username,
+        password = conf.password,
 
         namespace = conf.extra_options.namespace,
         scan_count = conf.extra_options.scan_count,

diff --git a/spec/03-plugins/29-acme/05-redis_storage_spec.lua b/spec/03-plugins/29-acme/05-redis_storage_spec.lua
@@ -555,4 +555,134 @@ describe("Plugin: acme (storage.redis)", function()
     end)
   end)
 
+  describe("redis authentication", function()
+    describe("happy path", function()
+
+      it("should successfully connect to Redis with Auth using username/password", function()
+        local config = {
+          host = helpers.redis_host,
+          port = helpers.redis_auth_port,
+          database = 0,
+          username = "default",
+          password = "passdefault",
+        }
+        local storage, err = redis_storage.new(config)
+        assert.is_nil(err)
+        assert.not_nil(storage)
+        local err = storage:set("foo", "bar", 10)
+        assert.is_nil(err)
+        local value, err = storage:get("foo")
+        assert.is_nil(err)
+        assert.equal("bar", value)
+      end)
+
+      it("should successfully connect to Redis with Auth using legacy auth", function()
+        local config = {
+          host = helpers.redis_host,
+          port = helpers.redis_auth_port,
+          database = 0,
+          auth = "passdefault",
+        }
+        local storage, err = redis_storage.new(config)
+        assert.is_nil(err)
+        assert.not_nil(storage)
+        local err = storage:set("foo", "bar", 10)
+        assert.is_nil(err)
+        local value, err = storage:get("foo")
+        assert.is_nil(err)
+        assert.equal("bar", value)
+      end)
+
+      it("should successfully connect to Redis with Auth using just password", function()
+        local config = {
+          host = helpers.redis_host,
+          port = helpers.redis_auth_port,
+          database = 0,
+          password = "passdefault",
+        }
+        local storage, err = redis_storage.new(config)
+        assert.is_nil(err)
+        assert.not_nil(storage)
+        local err = storage:set("foo", "bar", 10)
+        assert.is_nil(err)
+        local value, err = storage:get("foo")
+        assert.is_nil(err)
+        assert.equal("bar", value)
+      end)
+    end)
+
+    describe("unhappy path", function()
+      it("should not connect to Redis with Auth using just username", function()
+        local config = {
+          host = helpers.redis_host,
+          port = helpers.redis_auth_port,
+          database = 0,
+          username = "default",
+        }
+        local storage, err = redis_storage.new(config)
+        assert.is_nil(err)
+        assert.not_nil(storage)
+        local err = storage:set("foo", "bar", 10)
+        assert.equal("can't select database NOAUTH Authentication required.", err)
+      end)
+
+      it("should not connect to Redis with Auth using wrong username", function()
+        local config = {
+          host = helpers.redis_host,
+          port = helpers.redis_auth_port,
+          database = 0,
+          username = "wrongusername",
+        }
+        local storage, err = redis_storage.new(config)
+        assert.is_nil(err)
+        assert.not_nil(storage)
+        local err = storage:set("foo", "bar", 10)
+        assert.equal("can't select database NOAUTH Authentication required.", err)
+      end)
+
+      it("should not connect to Redis with Auth using wrong password and no username", function()
+        local config = {
+          host = helpers.redis_host,
+          port = helpers.redis_auth_port,
+          database = 0,
+          password = "wrongpassword"
+        }
+        local storage, err = redis_storage.new(config)
+        assert.is_nil(err)
+        assert.not_nil(storage)
+        local err = storage:set("foo", "bar", 10)
+        assert.equal("authentication failed WRONGPASS invalid username-password pair or user is disabled.", err)
+      end)
+
+      it("should not connect to Redis with Auth using wrong password and correct username", function()
+        local config = {
+          host = helpers.redis_host,
+          port = helpers.redis_auth_port,
+          database = 0,
+          username = "default",
+          password = "wrongpassword"
+        }
+        local storage, err = redis_storage.new(config)
+        assert.is_nil(err)
+        assert.not_nil(storage)
+        local err = storage:set("foo", "bar", 10)
+        assert.equal("authentication failed WRONGPASS invalid username-password pair or user is disabled.", err)
+      end)
+
+      it("should not connect to Redis with Auth using correct password and wrong username", function()
+        local config = {
+          host = helpers.redis_host,
+          port = helpers.redis_auth_port,
+          database = 0,
+          username = "kong",
+          password = "passdefault"
+        }
+        local storage, err = redis_storage.new(config)
+        assert.is_nil(err)
+        assert.not_nil(storage)
+        local err = storage:set("foo", "bar", 10)
+        assert.equal("authentication failed WRONGPASS invalid username-password pair or user is disabled.", err)
+      end)
+    end)
+  end)
 end)
diff --git a/spec/helpers.lua b/spec/helpers.lua
@@ -33,6 +33,7 @@ local OTELCOL_FILE_EXPORTER_PATH = os.getenv("KONG_SPEC_TEST_OTELCOL_FILE_EXPORT
 local REDIS_HOST = os.getenv("KONG_SPEC_TEST_REDIS_HOST") or "localhost"
 local REDIS_PORT = tonumber(os.getenv("KONG_SPEC_TEST_REDIS_PORT") or 6379)
 local REDIS_SSL_PORT = tonumber(os.getenv("KONG_SPEC_TEST_REDIS_SSL_PORT") or 6380)
+local REDIS_AUTH_PORT = tonumber(os.getenv("KONG_SPEC_TEST_REDIS_AUTH_PORT") or 6381)
 local REDIS_SSL_SNI = os.getenv("KONG_SPEC_TEST_REDIS_SSL_SNI") or "test-redis.example.com"
 local TEST_COVERAGE_MODE = os.getenv("KONG_COVERAGE")
 local TEST_COVERAGE_TIMEOUT = 30
@@ -4340,6 +4341,7 @@ end
   redis_port     = REDIS_PORT,
   redis_ssl_port = REDIS_SSL_PORT,
   redis_ssl_sni  = REDIS_SSL_SNI,
+  redis_auth_port = REDIS_AUTH_PORT,
 
   blackhole_host = BLACKHOLE_HOST,