From e166fb55c62e88a600b57045c6d528d085326af6 Mon Sep 17 00:00:00 2001 From: M Sazzadul Hoque <7600764+sazzad16@users.noreply.github.com> Date: Mon, 17 Oct 2022 13:35:39 +0600 Subject: [PATCH] Test: Address ACL error messaging changes (#3170) - Address "Unify ACL failure error messaging." - PR: https://github.com/redis/redis/pull/11160 - Commit: https://github.com/redis/redis/commit/3193f086ca0e167e89b6c5cf14133c03213b8378 - Refactor AccessControlListCommandsTest --- .../jedis/AccessControlListCommandsTest.java | 161 ++++++++---------- 1 file changed, 71 insertions(+), 90 deletions(-) diff --git a/src/test/java/redis/clients/jedis/commands/jedis/AccessControlListCommandsTest.java b/src/test/java/redis/clients/jedis/commands/jedis/AccessControlListCommandsTest.java index 79a9ec101b..384441bd3b 100644 --- a/src/test/java/redis/clients/jedis/commands/jedis/AccessControlListCommandsTest.java +++ b/src/test/java/redis/clients/jedis/commands/jedis/AccessControlListCommandsTest.java @@ -1,10 +1,21 @@ package redis.clients.jedis.commands.jedis; import static org.hamcrest.CoreMatchers.containsString; +import static org.hamcrest.CoreMatchers.endsWith; +import static org.hamcrest.CoreMatchers.hasItem; +import static org.hamcrest.CoreMatchers.startsWith; +import static org.hamcrest.Matchers.greaterThan; import static org.hamcrest.Matchers.greaterThanOrEqualTo; -import static org.junit.Assert.*; +import static org.junit.Assert.assertArrayEquals; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertThat; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; import java.util.List; +import org.junit.After; import org.junit.BeforeClass; import org.junit.Test; @@ -33,6 +44,15 @@ public static void prepare() throws Exception { RedisVersionUtil.checkRedisMajorVersionNumber(6)); } + @After + @Override + public void tearDown() throws Exception { + try { + jedis.aclDelUser(USER_NAME); + } catch (Exception e) { } + super.tearDown(); + } + @Test public void aclWhoAmI() { String string = jedis.aclWhoAmI(); @@ -44,8 +64,8 @@ public void aclWhoAmI() { @Test public void aclListDefault() { - assertTrue(jedis.aclList().size() > 0); - assertTrue(jedis.aclListBinary().size() > 0); + assertThat(jedis.aclList().size(), greaterThan(0)); + assertThat(jedis.aclListBinary().size(), greaterThan(0)); } @Test @@ -57,7 +77,8 @@ public void addAndRemoveUser() { assertEquals(existingUsers + 1, jedis.aclList().size()); assertEquals(existingUsers + 1, jedis.aclListBinary().size()); // test binary - jedis.aclDelUser(USER_NAME); + long count = jedis.aclDelUser(USER_NAME); + assertEquals(1, count); assertEquals(existingUsers, jedis.aclList().size()); assertEquals(existingUsers, jedis.aclListBinary().size()); // test binary } @@ -66,7 +87,7 @@ public void addAndRemoveUser() { public void aclUsers() { List users = jedis.aclUsers(); assertEquals(2, users.size()); - assertTrue(users.contains("default")); + assertThat(users, hasItem("default")); assertEquals(2, jedis.aclUsersBinary().size()); // Test binary } @@ -82,7 +103,6 @@ public void aclGetUser() { assertEquals("~*", userInfo.getKeys().get(0)); // create new user - jedis.aclDelUser(USER_NAME); jedis.aclSetUser(USER_NAME); userInfo = jedis.aclGetUser(USER_NAME); assertThat(userInfo.getFlags().size(), greaterThanOrEqualTo(1)); @@ -97,8 +117,6 @@ public void aclGetUser() { assertThat(userInfo.getCommands(), containsString("+@all")); assertThat(userInfo.getCommands(), containsString("-@string")); assertThat(userInfo.getCommands(), containsString("+debug|digest")); - - jedis.aclDelUser(USER_NAME); } @Test @@ -108,19 +126,17 @@ public void createUserAndPasswords() { // create a new client to try to authenticate Jedis jedis2 = new Jedis(); - String authResult = null; // the user is just created without any permission the authentication should fail try { - authResult = jedis2.auth(USER_NAME, USER_PASSWORD); + jedis2.auth(USER_NAME, USER_PASSWORD); fail("Should throw a WRONGPASS exception"); } catch (JedisAccessControlException e) { - assertNull(authResult); - assertTrue(e.getMessage().startsWith("WRONGPASS ")); + assertThat(e.getMessage(), startsWith("WRONGPASS ")); } // now activate the user - authResult = jedis.aclSetUser(USER_NAME, "on", "+acl"); + jedis.aclSetUser(USER_NAME, "on", "+acl"); jedis2.auth(USER_NAME, USER_PASSWORD); assertEquals(USER_NAME, jedis2.aclWhoAmI()); @@ -128,30 +144,27 @@ public void createUserAndPasswords() { jedis2.close(); try { - authResult = jedis2.auth(USER_NAME, "wrong-password"); + jedis2.auth(USER_NAME, "wrong-password"); fail("Should throw a WRONGPASS exception"); } catch (JedisAccessControlException e) { - assertEquals("OK", authResult); - assertTrue(e.getMessage().startsWith("WRONGPASS ")); + assertThat(e.getMessage(), startsWith("WRONGPASS ")); } // remove password, and try to authenticate - status = jedis.aclSetUser(USER_NAME, "<" + USER_PASSWORD); + jedis.aclSetUser(USER_NAME, "<" + USER_PASSWORD); try { - authResult = jedis2.auth(USER_NAME, USER_PASSWORD); + jedis2.auth(USER_NAME, USER_PASSWORD); fail("Should throw a WRONGPASS exception"); } catch (JedisAccessControlException e) { - assertEquals("OK", authResult); - assertTrue(e.getMessage().startsWith("WRONGPASS ")); + assertThat(e.getMessage(), startsWith("WRONGPASS ")); } jedis.aclDelUser(USER_NAME); // delete the user try { - authResult = jedis2.auth(USER_NAME, "wrong-password"); + jedis2.auth(USER_NAME, "wrong-password"); fail("Should throw a WRONGPASS exception"); } catch (JedisAccessControlException e) { - assertEquals("OK", authResult); - assertTrue(e.getMessage().startsWith("WRONGPASS ")); + assertThat(e.getMessage(), startsWith("WRONGPASS ")); } jedis2.close(); @@ -159,7 +172,6 @@ public void createUserAndPasswords() { @Test public void aclSetUserWithAnyPassword() { - jedis.aclDelUser(USER_NAME); String status = jedis.aclSetUser(USER_NAME, "nopass"); assertEquals("OK", status); status = jedis.aclSetUser(USER_NAME, "on", "+acl"); @@ -169,14 +181,11 @@ public void aclSetUserWithAnyPassword() { Jedis jedis2 = new Jedis(); String authResult = jedis2.auth(USER_NAME, "any password"); assertEquals("OK", authResult); - jedis2.close(); - jedis.aclDelUser(USER_NAME); } @Test public void aclExcudeSingleCommand() { - jedis.aclDelUser(USER_NAME); String status = jedis.aclSetUser(USER_NAME, "nopass"); assertEquals("OK", status); @@ -196,55 +205,47 @@ public void aclExcudeSingleCommand() { jedis2.incr("mycounter"); - String result = null; try { - result = jedis2.ping(); + jedis2.ping(); fail("Should throw a NOPERM exception"); } catch (JedisAccessControlException e) { - assertNull(result); - assertEquals( - "NOPERM this user has no permissions to run the 'ping' command", - e.getMessage()); + assertThat(e.getMessage(), startsWith("NOPERM ")); + assertThat(e.getMessage(), endsWith(" has no permissions to run the 'ping' command")); } jedis2.close(); - jedis.aclDelUser(USER_NAME); } @Test public void aclDryRun() { - jedis.aclDelUser(USER_NAME); - jedis.aclSetUser(USER_NAME, "nopass", "allkeys", "+set", "-get"); assertEquals("OK", jedis.aclDryRun(USER_NAME, "SET", "key", "value")); - assertEquals("This user has no permissions to run the 'get' command", - jedis.aclDryRun(USER_NAME, "GET", "key")); + assertThat(jedis.aclDryRun(USER_NAME, "GET", "key"), + endsWith(" has no permissions to run the 'get' command")); assertEquals("OK", jedis.aclDryRun(USER_NAME, - new CommandArguments(Protocol.Command.SET).key("key").add("value"))); - assertEquals("This user has no permissions to run the 'get' command", jedis.aclDryRun(USER_NAME, - new CommandArguments(Protocol.Command.GET).key("key"))); - - jedis.aclDelUser(USER_NAME); + new CommandArguments(Protocol.Command.SET).key("ca-key").add("value"))); + assertThat(jedis.aclDryRun(USER_NAME, new CommandArguments(Protocol.Command.GET).key("ca-key")), + endsWith(" has no permissions to run the 'get' command")); } @Test public void aclDryRunBinary() { byte[] username = USER_NAME.getBytes(); - jedis.aclDelUser(username); jedis.aclSetUser(username, "nopass".getBytes(), "allkeys".getBytes(), "+set".getBytes(), "-get".getBytes()); - assertArrayEquals("OK".getBytes(), jedis.aclDryRunBinary(username, "SET".getBytes(), "key".getBytes(), "value".getBytes())); - assertArrayEquals("This user has no permissions to run the 'get' command".getBytes(), - jedis.aclDryRunBinary(username, "GET".getBytes(), "key".getBytes())); + assertArrayEquals("OK".getBytes(), jedis.aclDryRunBinary(username, + "SET".getBytes(), "key".getBytes(), "value".getBytes())); + assertThat(new String(jedis.aclDryRunBinary(username, "GET".getBytes(), "key".getBytes())), + endsWith(" has no permissions to run the 'get' command")); - assertArrayEquals("OK".getBytes(), jedis.aclDryRunBinary(username, new CommandArguments(Protocol.Command.SET).key("key").add("value"))); - assertArrayEquals("This user has no permissions to run the 'get' command".getBytes(), - jedis.aclDryRunBinary(username, new CommandArguments(Protocol.Command.GET).key("key"))); - - jedis.aclDelUser(USER_NAME); + assertArrayEquals("OK".getBytes(), jedis.aclDryRunBinary(username, + new CommandArguments(Protocol.Command.SET).key("ca-key").add("value"))); + assertThat(new String(jedis.aclDryRunBinary(username, + new CommandArguments(Protocol.Command.GET).key("ca-key"))), + endsWith(" has no permissions to run the 'get' command")); } @Test @@ -259,86 +260,66 @@ public void aclDelUser() { @Test public void basicPermissionsTest() { - // create a user with login permissions - - jedis.aclDelUser(USER_NAME); // delete the user + jedis.aclSetUser(USER_NAME, ">" + USER_PASSWORD); // users are not able to access any command - String status = jedis.aclSetUser(USER_NAME, ">" + USER_PASSWORD); - String authResult = jedis.aclSetUser(USER_NAME, "on", "+acl"); + jedis.aclSetUser(USER_NAME, "on", "+acl"); // connect with this new user and try to get/set keys Jedis jedis2 = new Jedis(); jedis2.auth(USER_NAME, USER_PASSWORD); - String result = null; try { - result = jedis2.set("foo", "bar"); + jedis2.set("foo", "bar"); fail("Should throw a NOPERM exception"); } catch (JedisAccessControlException e) { - assertNull(result); - assertEquals( - "NOPERM this user has no permissions to run the 'set' command", - e.getMessage()); + assertThat(e.getMessage(), startsWith("NOPERM ")); + assertThat(e.getMessage(), endsWith(" has no permissions to run the 'set' command")); } // change permissions of the user // by default users are not able to access any key - status = jedis.aclSetUser(USER_NAME, "+set"); + jedis.aclSetUser(USER_NAME, "+set"); jedis2.close(); jedis2.auth(USER_NAME, USER_PASSWORD); - result = null; try { - result = jedis2.set("foo", "bar"); + jedis2.set("foo", "bar"); fail("Should throw a NOPERM exception"); } catch (JedisAccessControlException e) { - assertNull(result); - assertEquals( - "NOPERM this user has no permissions to access one of the keys used as arguments", - e.getMessage()); + assertEquals("NOPERM No permissions to access a key", e.getMessage()); } // allow user to access a subset of the key - result = jedis.aclSetUser(USER_NAME, "allcommands", "~foo:*", "~bar:*"); // TODO : Define a DSL + jedis.aclSetUser(USER_NAME, "allcommands", "~foo:*", "~bar:*"); // TODO : Define a DSL // create key foo, bar and zap - result = jedis2.set("foo:1", "a"); - assertEquals("OK", status); + jedis2.set("foo:1", "a"); - result = jedis2.set("bar:2", "b"); - assertEquals("OK", status); + jedis2.set("bar:2", "b"); - result = null; try { - result = jedis2.set("zap:3", "c"); + jedis2.set("zap:3", "c"); fail("Should throw a NOPERM exception"); } catch (JedisAccessControlException e) { - assertNull(result); - assertEquals( - "NOPERM this user has no permissions to access one of the keys used as arguments", - e.getMessage()); + assertEquals("NOPERM No permissions to access a key", e.getMessage()); } - - // remove user - jedis.aclDelUser(USER_NAME); // delete the user - } @Test public void aclCatTest() { List categories = jedis.aclCat(); - assertTrue(!categories.isEmpty()); + assertFalse(categories.isEmpty()); // test binary List categoriesBinary = jedis.aclCatBinary(); - assertTrue(!categories.isEmpty()); + assertFalse(categories.isEmpty()); assertEquals(categories.size(), categoriesBinary.size()); // test commands in a category - assertTrue(!jedis.aclCat("scripting").isEmpty()); + assertFalse(jedis.aclCat("scripting").isEmpty()); try { jedis.aclCat("testcategory"); @@ -513,7 +494,7 @@ public void aclLoadTest() { jedis.aclLoad(); fail("Should throw a JedisDataException: ERR This Redis instance is not configured to use an ACL file..."); } catch (JedisDataException e) { - assertTrue(e.getMessage().contains("ERR This Redis instance is not configured to use an ACL file.")); + assertThat(e.getMessage(), startsWith("ERR This Redis instance is not configured to use an ACL file.")); } // TODO test with ACL file @@ -525,7 +506,7 @@ public void aclSaveTest() { jedis.aclSave(); fail("Should throw a JedisDataException: ERR This Redis instance is not configured to use an ACL file..."); } catch (JedisDataException e) { - assertTrue(e.getMessage().contains("ERR This Redis instance is not configured to use an ACL file.")); + assertThat(e.getMessage(), startsWith("ERR This Redis instance is not configured to use an ACL file.")); } // TODO test with ACL file