From 2d117568e2d35aed4203bd66b74871d86c2b5536 Mon Sep 17 00:00:00 2001 From: M Sazzadul Hoque <7600764+sazzad16@users.noreply.github.com> Date: Thu, 26 Dec 2024 16:20:32 +0600 Subject: [PATCH 1/5] Test HASH module ACL support --- ...sModulesAccessControlListCommandsTest.java | 71 +++++++++++++++++++ 1 file changed, 71 insertions(+) create mode 100644 src/test/java/redis/clients/jedis/modules/RedisModulesAccessControlListCommandsTest.java diff --git a/src/test/java/redis/clients/jedis/modules/RedisModulesAccessControlListCommandsTest.java b/src/test/java/redis/clients/jedis/modules/RedisModulesAccessControlListCommandsTest.java new file mode 100644 index 0000000000..5d9ea14b60 --- /dev/null +++ b/src/test/java/redis/clients/jedis/modules/RedisModulesAccessControlListCommandsTest.java @@ -0,0 +1,71 @@ +package redis.clients.jedis.modules; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.endsWith; +import static org.hamcrest.Matchers.startsWith; +import static org.junit.Assert.assertThrows; + +import org.junit.After; +import org.junit.BeforeClass; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; + +import redis.clients.jedis.DefaultJedisClientConfig; +import redis.clients.jedis.RedisProtocol; +import redis.clients.jedis.UnifiedJedis; +import redis.clients.jedis.commands.jedis.JedisCommandsTestBase; +import redis.clients.jedis.exceptions.JedisAccessControlException; +import redis.clients.jedis.util.RedisVersionUtil; + +@RunWith(Parameterized.class) +public class RedisModulesAccessControlListCommandsTest extends JedisCommandsTestBase { + + public static final String USER_NAME = "newuser"; + public static final String USER_PASSWORD = "secret"; + + @BeforeClass + public static void prepare() throws Exception { + // Use to check if the ACL test should be ran. ACL are available only in 6.0 and later + org.junit.Assume.assumeTrue("Not running ACL test on this version of Redis", + RedisVersionUtil.checkRedisMajorVersionNumber(8, endpoint)); + } + + public RedisModulesAccessControlListCommandsTest(RedisProtocol protocol) { + super(protocol); + } + + @After + @Override + public void tearDown() throws Exception { + try { + jedis.aclDelUser(USER_NAME); + } catch (Exception e) { } + super.tearDown(); + } + + @Test + public void aclHashesCommandsTest() { + // create and enable an user with permission to all keys but no commands + jedis.aclSetUser(USER_NAME, ">" + USER_PASSWORD, "on", "~*"); + + // client object with new user + try (UnifiedJedis client = new UnifiedJedis(endpoint.getHostAndPort(), + DefaultJedisClientConfig.builder().user(USER_NAME).password(USER_PASSWORD).build())) { + + // user can't execute hash commands + JedisAccessControlException ace = assertThrows("Should throw a NOPERM exception", + JedisAccessControlException.class, + () -> client.hgetAll("foo")); + assertThat(ace.getMessage(), startsWith("NOPERM ")); + assertThat(ace.getMessage(), endsWith(" has no permissions to run the 'hgetall' command")); + + // permit user to hash commands + jedis.aclSetUser(USER_NAME, "+@hash"); + + // user can now execute hash commands + client.hgetAll("foo"); + } + } + +} From f5c57a7ba332c47e8cdc551cdc2ebc16bef8fc8d Mon Sep 17 00:00:00 2001 From: M Sazzadul Hoque <7600764+sazzad16@users.noreply.github.com> Date: Wed, 22 Jan 2025 19:18:47 +0600 Subject: [PATCH 2/5] Add version rule --- ...solidatedAccessControlListCommandsTest.java} | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) rename src/test/java/redis/clients/jedis/modules/{RedisModulesAccessControlListCommandsTest.java => ConsolidatedAccessControlListCommandsTest.java} (76%) diff --git a/src/test/java/redis/clients/jedis/modules/RedisModulesAccessControlListCommandsTest.java b/src/test/java/redis/clients/jedis/modules/ConsolidatedAccessControlListCommandsTest.java similarity index 76% rename from src/test/java/redis/clients/jedis/modules/RedisModulesAccessControlListCommandsTest.java rename to src/test/java/redis/clients/jedis/modules/ConsolidatedAccessControlListCommandsTest.java index 5d9ea14b60..7efd225b9e 100644 --- a/src/test/java/redis/clients/jedis/modules/RedisModulesAccessControlListCommandsTest.java +++ b/src/test/java/redis/clients/jedis/modules/ConsolidatedAccessControlListCommandsTest.java @@ -5,8 +5,8 @@ import static org.hamcrest.Matchers.startsWith; import static org.junit.Assert.assertThrows; +import io.redis.test.annotations.SinceRedisVersion; import org.junit.After; -import org.junit.BeforeClass; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.Parameterized; @@ -16,22 +16,15 @@ import redis.clients.jedis.UnifiedJedis; import redis.clients.jedis.commands.jedis.JedisCommandsTestBase; import redis.clients.jedis.exceptions.JedisAccessControlException; -import redis.clients.jedis.util.RedisVersionUtil; +@SinceRedisVersion(value = "8.0.0") @RunWith(Parameterized.class) -public class RedisModulesAccessControlListCommandsTest extends JedisCommandsTestBase { +public class ConsolidatedAccessControlListCommandsTest extends JedisCommandsTestBase { - public static final String USER_NAME = "newuser"; + public static final String USER_NAME = "moduser"; public static final String USER_PASSWORD = "secret"; - @BeforeClass - public static void prepare() throws Exception { - // Use to check if the ACL test should be ran. ACL are available only in 6.0 and later - org.junit.Assume.assumeTrue("Not running ACL test on this version of Redis", - RedisVersionUtil.checkRedisMajorVersionNumber(8, endpoint)); - } - - public RedisModulesAccessControlListCommandsTest(RedisProtocol protocol) { + public ConsolidatedAccessControlListCommandsTest(RedisProtocol protocol) { super(protocol); } From 3ac0b894f7a6cd3468c6d3508e93e3d3d11f72a3 Mon Sep 17 00:00:00 2001 From: M Sazzadul Hoque <7600764+sazzad16@users.noreply.github.com> Date: Wed, 22 Jan 2025 21:35:33 +0600 Subject: [PATCH 3/5] Test modules acl categories --- ...olidatedAccessControlListCommandsTest.java | 87 +++++++++++++++---- 1 file changed, 72 insertions(+), 15 deletions(-) diff --git a/src/test/java/redis/clients/jedis/modules/ConsolidatedAccessControlListCommandsTest.java b/src/test/java/redis/clients/jedis/modules/ConsolidatedAccessControlListCommandsTest.java index 7efd225b9e..7f0ba11730 100644 --- a/src/test/java/redis/clients/jedis/modules/ConsolidatedAccessControlListCommandsTest.java +++ b/src/test/java/redis/clients/jedis/modules/ConsolidatedAccessControlListCommandsTest.java @@ -1,11 +1,15 @@ package redis.clients.jedis.modules; import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.endsWith; -import static org.hamcrest.Matchers.startsWith; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertThrows; import io.redis.test.annotations.SinceRedisVersion; +import java.util.Collections; +import java.util.Locale; +import java.util.function.Consumer; + +import org.hamcrest.Matchers; import org.junit.After; import org.junit.Test; import org.junit.runner.RunWith; @@ -14,10 +18,16 @@ import redis.clients.jedis.DefaultJedisClientConfig; import redis.clients.jedis.RedisProtocol; import redis.clients.jedis.UnifiedJedis; +import redis.clients.jedis.bloom.RedisBloomProtocol.*; +import redis.clients.jedis.commands.ProtocolCommand; import redis.clients.jedis.commands.jedis.JedisCommandsTestBase; import redis.clients.jedis.exceptions.JedisAccessControlException; +import redis.clients.jedis.json.JsonProtocol.JsonCommand; +import redis.clients.jedis.search.SearchProtocol.SearchCommand; +import redis.clients.jedis.timeseries.TimeSeriesProtocol.TimeSeriesCommand; +import redis.clients.jedis.util.SafeEncoder; -@SinceRedisVersion(value = "8.0.0") +@SinceRedisVersion(value = "7.9.0") @RunWith(Parameterized.class) public class ConsolidatedAccessControlListCommandsTest extends JedisCommandsTestBase { @@ -38,7 +48,55 @@ public void tearDown() throws Exception { } @Test - public void aclHashesCommandsTest() { + public void listACLCategoriesTest() { + assertThat(jedis.aclCat(), + Matchers.hasItems("bloom", "cuckoo", "cms", "topk", "tdigest", + "search", "timeseries", "json")); + } + + @Test + public void grantBloomCommandCatTest() { + grantModuleCommandCatTest("bloom", BloomFilterCommand.RESERVE, client -> client.bfReserve("foo", 0.01, 10_000)); + } + + @Test + public void grantCuckooCommandCatTest() { + grantModuleCommandCatTest("cuckoo", CuckooFilterCommand.RESERVE, client -> client.cfReserve("foo", 10_000)); + } + + @Test + public void grantCmsCommandCatTest() { + grantModuleCommandCatTest("cms", CountMinSketchCommand.INITBYDIM, client -> client.cmsInitByDim("foo", 16, 4)); + } + + @Test + public void grantTopkModuleCommandCatTest() { + grantModuleCommandCatTest("topk", TopKCommand.RESERVE, client -> client.topkReserve("foo", 1000)); + } + + @Test + public void grantTdigestCommandCatTest() { + grantModuleCommandCatTest("tdigest", TDigestCommand.CREATE, client -> client.tdigestCreate("foo")); + } + + @org.junit.Ignore + @Test + public void grantSearchCommandCatTest() { + grantModuleCommandCatTest("search", SearchCommand.CREATE, + client -> client.ftCreate("index", Collections.emptySet())); + } + + @Test + public void grantTimeseriesCommandCatTest() { + grantModuleCommandCatTest("timeseries", TimeSeriesCommand.CREATE, client -> client.tsCreate("foo")); + } + + @Test + public void grantJsonCommandCatTest() { + grantModuleCommandCatTest("json", JsonCommand.GET, client -> client.jsonGet("foo")); + } + + private void grantModuleCommandCatTest(String category, ProtocolCommand command, Consumer operation) { // create and enable an user with permission to all keys but no commands jedis.aclSetUser(USER_NAME, ">" + USER_PASSWORD, "on", "~*"); @@ -46,19 +104,18 @@ public void aclHashesCommandsTest() { try (UnifiedJedis client = new UnifiedJedis(endpoint.getHostAndPort(), DefaultJedisClientConfig.builder().user(USER_NAME).password(USER_PASSWORD).build())) { - // user can't execute hash commands - JedisAccessControlException ace = assertThrows("Should throw a NOPERM exception", - JedisAccessControlException.class, - () -> client.hgetAll("foo")); - assertThat(ace.getMessage(), startsWith("NOPERM ")); - assertThat(ace.getMessage(), endsWith(" has no permissions to run the 'hgetall' command")); + // user can't execute category commands + JedisAccessControlException noperm = assertThrows("Should throw a NOPERM exception", + JedisAccessControlException.class, () -> operation.accept(client)); + assertEquals(String.format("NOPERM User %s has no permissions to run the '%s' command", + USER_NAME, SafeEncoder.encode(command.getRaw()).toLowerCase(Locale.ENGLISH)), + noperm.getMessage()); - // permit user to hash commands - jedis.aclSetUser(USER_NAME, "+@hash"); + // permit user to category commands + jedis.aclSetUser(USER_NAME, "+@" + category); - // user can now execute hash commands - client.hgetAll("foo"); + // user can now execute category commands + operation.accept(client); } } - } From 22724301361444a98cb376873de0d6bb6e4dd916 Mon Sep 17 00:00:00 2001 From: M Sazzadul Hoque <7600764+sazzad16@users.noreply.github.com> Date: Wed, 22 Jan 2025 22:10:09 +0600 Subject: [PATCH 4/5] Test according to design doc with Redis 8.0-M03 --- ...olidatedAccessControlListCommandsTest.java | 83 +++++++++++++++++-- 1 file changed, 74 insertions(+), 9 deletions(-) diff --git a/src/test/java/redis/clients/jedis/modules/ConsolidatedAccessControlListCommandsTest.java b/src/test/java/redis/clients/jedis/modules/ConsolidatedAccessControlListCommandsTest.java index 7f0ba11730..cfff91f974 100644 --- a/src/test/java/redis/clients/jedis/modules/ConsolidatedAccessControlListCommandsTest.java +++ b/src/test/java/redis/clients/jedis/modules/ConsolidatedAccessControlListCommandsTest.java @@ -1,14 +1,11 @@ package redis.clients.jedis.modules; import static org.hamcrest.MatcherAssert.assertThat; -import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertThrows; import io.redis.test.annotations.SinceRedisVersion; -import java.util.Collections; import java.util.Locale; import java.util.function.Consumer; - import org.hamcrest.Matchers; import org.junit.After; import org.junit.Test; @@ -24,6 +21,7 @@ import redis.clients.jedis.exceptions.JedisAccessControlException; import redis.clients.jedis.json.JsonProtocol.JsonCommand; import redis.clients.jedis.search.SearchProtocol.SearchCommand; +import redis.clients.jedis.search.schemafields.TextField; import redis.clients.jedis.timeseries.TimeSeriesProtocol.TimeSeriesCommand; import redis.clients.jedis.util.SafeEncoder; @@ -54,36 +52,71 @@ public void listACLCategoriesTest() { "search", "timeseries", "json")); } + @Test + public void grantBloomCommandTest() { + grantModuleCommandTest(BloomFilterCommand.RESERVE, client -> client.bfReserve("foo", 0.01, 10_000)); + } + @Test public void grantBloomCommandCatTest() { grantModuleCommandCatTest("bloom", BloomFilterCommand.RESERVE, client -> client.bfReserve("foo", 0.01, 10_000)); } + @Test + public void grantCuckooCommandTest() { + grantModuleCommandTest(CuckooFilterCommand.RESERVE, client -> client.cfReserve("foo", 10_000)); + } + @Test public void grantCuckooCommandCatTest() { grantModuleCommandCatTest("cuckoo", CuckooFilterCommand.RESERVE, client -> client.cfReserve("foo", 10_000)); } + @Test + public void grantCmsCommandTest() { + grantModuleCommandTest(CountMinSketchCommand.INITBYDIM, client -> client.cmsInitByDim("foo", 16, 4)); + } + @Test public void grantCmsCommandCatTest() { grantModuleCommandCatTest("cms", CountMinSketchCommand.INITBYDIM, client -> client.cmsInitByDim("foo", 16, 4)); } @Test - public void grantTopkModuleCommandCatTest() { + public void grantTopkCommandTest() { + grantModuleCommandTest(TopKCommand.RESERVE, client -> client.topkReserve("foo", 1000)); + } + + @Test + public void grantTopkCommandCatTest() { grantModuleCommandCatTest("topk", TopKCommand.RESERVE, client -> client.topkReserve("foo", 1000)); } + @Test + public void grantTdigestCommandTest() { + grantModuleCommandTest(TDigestCommand.CREATE, client -> client.tdigestCreate("foo")); + } + @Test public void grantTdigestCommandCatTest() { grantModuleCommandCatTest("tdigest", TDigestCommand.CREATE, client -> client.tdigestCreate("foo")); } - @org.junit.Ignore + @Test + public void grantSearchCommandTest() { + grantModuleCommandTest(SearchCommand.CREATE, + client -> client.ftCreate("foo", TextField.of("bar"))); + } + @Test public void grantSearchCommandCatTest() { grantModuleCommandCatTest("search", SearchCommand.CREATE, - client -> client.ftCreate("index", Collections.emptySet())); + client -> client.ftCreate("foo", TextField.of("bar"))); + } + + @Test + public void grantTimeseriesCommandTest() { + grantModuleCommandTest(TimeSeriesCommand.CREATE, client -> client.tsCreate("foo")); } @Test @@ -91,11 +124,38 @@ public void grantTimeseriesCommandCatTest() { grantModuleCommandCatTest("timeseries", TimeSeriesCommand.CREATE, client -> client.tsCreate("foo")); } + @Test + public void grantJsonCommandTest() { + grantModuleCommandTest(JsonCommand.GET, client -> client.jsonGet("foo")); + } + @Test public void grantJsonCommandCatTest() { grantModuleCommandCatTest("json", JsonCommand.GET, client -> client.jsonGet("foo")); } + private void grantModuleCommandTest(ProtocolCommand command, Consumer operation) { + // create and enable an user with permission to all keys but no commands + jedis.aclSetUser(USER_NAME, ">" + USER_PASSWORD, "on", "~*"); + + // client object with new user + try (UnifiedJedis client = new UnifiedJedis(endpoint.getHostAndPort(), + DefaultJedisClientConfig.builder().user(USER_NAME).password(USER_PASSWORD).build())) { + + // user can't execute commands + JedisAccessControlException noperm = assertThrows("Should throw a NOPERM exception", + JedisAccessControlException.class, () -> operation.accept(client)); + assertThat(noperm.getMessage(), + Matchers.oneOf(getNopermErrorMessage(false, command), getNopermErrorMessage(true, command))); + + // permit user to commands + jedis.aclSetUser(USER_NAME, "+" + SafeEncoder.encode(command.getRaw())); + + // user can now execute commands + operation.accept(client); + } + } + private void grantModuleCommandCatTest(String category, ProtocolCommand command, Consumer operation) { // create and enable an user with permission to all keys but no commands jedis.aclSetUser(USER_NAME, ">" + USER_PASSWORD, "on", "~*"); @@ -107,9 +167,8 @@ private void grantModuleCommandCatTest(String category, ProtocolCommand command, // user can't execute category commands JedisAccessControlException noperm = assertThrows("Should throw a NOPERM exception", JedisAccessControlException.class, () -> operation.accept(client)); - assertEquals(String.format("NOPERM User %s has no permissions to run the '%s' command", - USER_NAME, SafeEncoder.encode(command.getRaw()).toLowerCase(Locale.ENGLISH)), - noperm.getMessage()); + assertThat(noperm.getMessage(), + Matchers.oneOf(getNopermErrorMessage(false, command), getNopermErrorMessage(true, command))); // permit user to category commands jedis.aclSetUser(USER_NAME, "+@" + category); @@ -118,4 +177,10 @@ private void grantModuleCommandCatTest(String category, ProtocolCommand command, operation.accept(client); } } + + private static String getNopermErrorMessage(boolean commandNameUpperCase, ProtocolCommand protocolCommand) { + String command = SafeEncoder.encode(protocolCommand.getRaw()); + return String.format("NOPERM User %s has no permissions to run the '%s' command", + USER_NAME, commandNameUpperCase ? command.toUpperCase(Locale.ENGLISH) : command.toLowerCase(Locale.ENGLISH)); + } } From 08476a40b020af54127c00afb5816d1c17c6c02b Mon Sep 17 00:00:00 2001 From: M Sazzadul Hoque <7600764+sazzad16@users.noreply.github.com> Date: Wed, 29 Jan 2025 17:13:30 +0600 Subject: [PATCH 5/5] Based on RedisModuleCommandsTestBase --- .../jedis/commands/jedis/JedisCommandsTestBase.java | 6 ++---- .../ConsolidatedAccessControlListCommandsTest.java | 7 +++---- .../jedis/modules/RedisModuleCommandsTestBase.java | 9 +++++---- 3 files changed, 10 insertions(+), 12 deletions(-) diff --git a/src/test/java/redis/clients/jedis/commands/jedis/JedisCommandsTestBase.java b/src/test/java/redis/clients/jedis/commands/jedis/JedisCommandsTestBase.java index d3951f0638..6613d6646e 100644 --- a/src/test/java/redis/clients/jedis/commands/jedis/JedisCommandsTestBase.java +++ b/src/test/java/redis/clients/jedis/commands/jedis/JedisCommandsTestBase.java @@ -15,11 +15,9 @@ public abstract class JedisCommandsTestBase { @Rule - public RedisVersionRule versionRule = new RedisVersionRule( - HostAndPorts.getRedisEndpoint("standalone0")); + public RedisVersionRule versionRule = new RedisVersionRule(endpoint); @Rule - public EnabledOnCommandRule enabledOnCommandRule = new EnabledOnCommandRule( - HostAndPorts.getRedisEndpoint("standalone0")); + public EnabledOnCommandRule enabledOnCommandRule = new EnabledOnCommandRule(endpoint); /** * Input data for parameterized tests. In principle all subclasses of this diff --git a/src/test/java/redis/clients/jedis/modules/ConsolidatedAccessControlListCommandsTest.java b/src/test/java/redis/clients/jedis/modules/ConsolidatedAccessControlListCommandsTest.java index cfff91f974..2ebacda539 100644 --- a/src/test/java/redis/clients/jedis/modules/ConsolidatedAccessControlListCommandsTest.java +++ b/src/test/java/redis/clients/jedis/modules/ConsolidatedAccessControlListCommandsTest.java @@ -17,7 +17,6 @@ import redis.clients.jedis.UnifiedJedis; import redis.clients.jedis.bloom.RedisBloomProtocol.*; import redis.clients.jedis.commands.ProtocolCommand; -import redis.clients.jedis.commands.jedis.JedisCommandsTestBase; import redis.clients.jedis.exceptions.JedisAccessControlException; import redis.clients.jedis.json.JsonProtocol.JsonCommand; import redis.clients.jedis.search.SearchProtocol.SearchCommand; @@ -27,7 +26,7 @@ @SinceRedisVersion(value = "7.9.0") @RunWith(Parameterized.class) -public class ConsolidatedAccessControlListCommandsTest extends JedisCommandsTestBase { +public class ConsolidatedAccessControlListCommandsTest extends RedisModuleCommandsTestBase { public static final String USER_NAME = "moduser"; public static final String USER_PASSWORD = "secret"; @@ -139,7 +138,7 @@ private void grantModuleCommandTest(ProtocolCommand command, Consumer" + USER_PASSWORD, "on", "~*"); // client object with new user - try (UnifiedJedis client = new UnifiedJedis(endpoint.getHostAndPort(), + try (UnifiedJedis client = new UnifiedJedis(hnp, DefaultJedisClientConfig.builder().user(USER_NAME).password(USER_PASSWORD).build())) { // user can't execute commands @@ -161,7 +160,7 @@ private void grantModuleCommandCatTest(String category, ProtocolCommand command, jedis.aclSetUser(USER_NAME, ">" + USER_PASSWORD, "on", "~*"); // client object with new user - try (UnifiedJedis client = new UnifiedJedis(endpoint.getHostAndPort(), + try (UnifiedJedis client = new UnifiedJedis(hnp, DefaultJedisClientConfig.builder().user(USER_NAME).password(USER_PASSWORD).build())) { // user can't execute category commands diff --git a/src/test/java/redis/clients/jedis/modules/RedisModuleCommandsTestBase.java b/src/test/java/redis/clients/jedis/modules/RedisModuleCommandsTestBase.java index 7e0e085c17..cfe2f7910b 100644 --- a/src/test/java/redis/clients/jedis/modules/RedisModuleCommandsTestBase.java +++ b/src/test/java/redis/clients/jedis/modules/RedisModuleCommandsTestBase.java @@ -22,7 +22,7 @@ public abstract class RedisModuleCommandsTestBase { @Rule - public RedisVersionRule versionRule = new RedisVersionRule(hnp,DefaultJedisClientConfig.builder().build() ); + public RedisVersionRule versionRule = new RedisVersionRule(hnp, DefaultJedisClientConfig.builder().build()); /** * Input data for parameterized tests. In principle all subclasses of this @@ -39,6 +39,7 @@ public static Collection data() { protected static final HostAndPort hnp = HostAndPort.from(address); protected final RedisProtocol protocol; + protected Jedis jedis; protected UnifiedJedis client; /** @@ -65,15 +66,15 @@ public static void prepare() { @Before public void setUp() { - try (Jedis jedis = new Jedis(hnp)) { - jedis.flushAll(); - } + jedis = new Jedis(hnp, DefaultJedisClientConfig.builder().protocol(protocol).build()); + jedis.flushAll(); client = new UnifiedJedis(hnp, DefaultJedisClientConfig.builder().protocol(protocol).build()); } @After public void tearDown() throws Exception { client.close(); + jedis.close(); } }