Skip to content

Commit e166fb5

Browse files
authored
Test: Address ACL error messaging changes (#3170)
- Address "Unify ACL failure error messaging." - PR: redis/redis#11160 - Commit: redis/redis@3193f08 - Refactor AccessControlListCommandsTest
1 parent ec6ad55 commit e166fb5

File tree

1 file changed

+71
-90
lines changed

1 file changed

+71
-90
lines changed

src/test/java/redis/clients/jedis/commands/jedis/AccessControlListCommandsTest.java

Lines changed: 71 additions & 90 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,21 @@
11
package redis.clients.jedis.commands.jedis;
22

33
import static org.hamcrest.CoreMatchers.containsString;
4+
import static org.hamcrest.CoreMatchers.endsWith;
5+
import static org.hamcrest.CoreMatchers.hasItem;
6+
import static org.hamcrest.CoreMatchers.startsWith;
7+
import static org.hamcrest.Matchers.greaterThan;
48
import static org.hamcrest.Matchers.greaterThanOrEqualTo;
5-
import static org.junit.Assert.*;
9+
import static org.junit.Assert.assertArrayEquals;
10+
import static org.junit.Assert.assertEquals;
11+
import static org.junit.Assert.assertFalse;
12+
import static org.junit.Assert.assertNotNull;
13+
import static org.junit.Assert.assertThat;
14+
import static org.junit.Assert.assertTrue;
15+
import static org.junit.Assert.fail;
616

717
import java.util.List;
18+
import org.junit.After;
819
import org.junit.BeforeClass;
920
import org.junit.Test;
1021

@@ -33,6 +44,15 @@ public static void prepare() throws Exception {
3344
RedisVersionUtil.checkRedisMajorVersionNumber(6));
3445
}
3546

47+
@After
48+
@Override
49+
public void tearDown() throws Exception {
50+
try {
51+
jedis.aclDelUser(USER_NAME);
52+
} catch (Exception e) { }
53+
super.tearDown();
54+
}
55+
3656
@Test
3757
public void aclWhoAmI() {
3858
String string = jedis.aclWhoAmI();
@@ -44,8 +64,8 @@ public void aclWhoAmI() {
4464

4565
@Test
4666
public void aclListDefault() {
47-
assertTrue(jedis.aclList().size() > 0);
48-
assertTrue(jedis.aclListBinary().size() > 0);
67+
assertThat(jedis.aclList().size(), greaterThan(0));
68+
assertThat(jedis.aclListBinary().size(), greaterThan(0));
4969
}
5070

5171
@Test
@@ -57,7 +77,8 @@ public void addAndRemoveUser() {
5777
assertEquals(existingUsers + 1, jedis.aclList().size());
5878
assertEquals(existingUsers + 1, jedis.aclListBinary().size()); // test binary
5979

60-
jedis.aclDelUser(USER_NAME);
80+
long count = jedis.aclDelUser(USER_NAME);
81+
assertEquals(1, count);
6182
assertEquals(existingUsers, jedis.aclList().size());
6283
assertEquals(existingUsers, jedis.aclListBinary().size()); // test binary
6384
}
@@ -66,7 +87,7 @@ public void addAndRemoveUser() {
6687
public void aclUsers() {
6788
List<String> users = jedis.aclUsers();
6889
assertEquals(2, users.size());
69-
assertTrue(users.contains("default"));
90+
assertThat(users, hasItem("default"));
7091

7192
assertEquals(2, jedis.aclUsersBinary().size()); // Test binary
7293
}
@@ -82,7 +103,6 @@ public void aclGetUser() {
82103
assertEquals("~*", userInfo.getKeys().get(0));
83104

84105
// create new user
85-
jedis.aclDelUser(USER_NAME);
86106
jedis.aclSetUser(USER_NAME);
87107
userInfo = jedis.aclGetUser(USER_NAME);
88108
assertThat(userInfo.getFlags().size(), greaterThanOrEqualTo(1));
@@ -97,8 +117,6 @@ public void aclGetUser() {
97117
assertThat(userInfo.getCommands(), containsString("+@all"));
98118
assertThat(userInfo.getCommands(), containsString("-@string"));
99119
assertThat(userInfo.getCommands(), containsString("+debug|digest"));
100-
101-
jedis.aclDelUser(USER_NAME);
102120
}
103121

104122
@Test
@@ -108,58 +126,52 @@ public void createUserAndPasswords() {
108126

109127
// create a new client to try to authenticate
110128
Jedis jedis2 = new Jedis();
111-
String authResult = null;
112129

113130
// the user is just created without any permission the authentication should fail
114131
try {
115-
authResult = jedis2.auth(USER_NAME, USER_PASSWORD);
132+
jedis2.auth(USER_NAME, USER_PASSWORD);
116133
fail("Should throw a WRONGPASS exception");
117134
} catch (JedisAccessControlException e) {
118-
assertNull(authResult);
119-
assertTrue(e.getMessage().startsWith("WRONGPASS "));
135+
assertThat(e.getMessage(), startsWith("WRONGPASS "));
120136
}
121137

122138
// now activate the user
123-
authResult = jedis.aclSetUser(USER_NAME, "on", "+acl");
139+
jedis.aclSetUser(USER_NAME, "on", "+acl");
124140
jedis2.auth(USER_NAME, USER_PASSWORD);
125141
assertEquals(USER_NAME, jedis2.aclWhoAmI());
126142

127143
// test invalid password
128144
jedis2.close();
129145

130146
try {
131-
authResult = jedis2.auth(USER_NAME, "wrong-password");
147+
jedis2.auth(USER_NAME, "wrong-password");
132148
fail("Should throw a WRONGPASS exception");
133149
} catch (JedisAccessControlException e) {
134-
assertEquals("OK", authResult);
135-
assertTrue(e.getMessage().startsWith("WRONGPASS "));
150+
assertThat(e.getMessage(), startsWith("WRONGPASS "));
136151
}
137152

138153
// remove password, and try to authenticate
139-
status = jedis.aclSetUser(USER_NAME, "<" + USER_PASSWORD);
154+
jedis.aclSetUser(USER_NAME, "<" + USER_PASSWORD);
140155
try {
141-
authResult = jedis2.auth(USER_NAME, USER_PASSWORD);
156+
jedis2.auth(USER_NAME, USER_PASSWORD);
142157
fail("Should throw a WRONGPASS exception");
143158
} catch (JedisAccessControlException e) {
144-
assertEquals("OK", authResult);
145-
assertTrue(e.getMessage().startsWith("WRONGPASS "));
159+
assertThat(e.getMessage(), startsWith("WRONGPASS "));
146160
}
147161

148162
jedis.aclDelUser(USER_NAME); // delete the user
149163
try {
150-
authResult = jedis2.auth(USER_NAME, "wrong-password");
164+
jedis2.auth(USER_NAME, "wrong-password");
151165
fail("Should throw a WRONGPASS exception");
152166
} catch (JedisAccessControlException e) {
153-
assertEquals("OK", authResult);
154-
assertTrue(e.getMessage().startsWith("WRONGPASS "));
167+
assertThat(e.getMessage(), startsWith("WRONGPASS "));
155168
}
156169

157170
jedis2.close();
158171
}
159172

160173
@Test
161174
public void aclSetUserWithAnyPassword() {
162-
jedis.aclDelUser(USER_NAME);
163175
String status = jedis.aclSetUser(USER_NAME, "nopass");
164176
assertEquals("OK", status);
165177
status = jedis.aclSetUser(USER_NAME, "on", "+acl");
@@ -169,14 +181,11 @@ public void aclSetUserWithAnyPassword() {
169181
Jedis jedis2 = new Jedis();
170182
String authResult = jedis2.auth(USER_NAME, "any password");
171183
assertEquals("OK", authResult);
172-
173184
jedis2.close();
174-
jedis.aclDelUser(USER_NAME);
175185
}
176186

177187
@Test
178188
public void aclExcudeSingleCommand() {
179-
jedis.aclDelUser(USER_NAME);
180189
String status = jedis.aclSetUser(USER_NAME, "nopass");
181190
assertEquals("OK", status);
182191

@@ -196,55 +205,47 @@ public void aclExcudeSingleCommand() {
196205

197206
jedis2.incr("mycounter");
198207

199-
String result = null;
200208
try {
201-
result = jedis2.ping();
209+
jedis2.ping();
202210
fail("Should throw a NOPERM exception");
203211
} catch (JedisAccessControlException e) {
204-
assertNull(result);
205-
assertEquals(
206-
"NOPERM this user has no permissions to run the 'ping' command",
207-
e.getMessage());
212+
assertThat(e.getMessage(), startsWith("NOPERM "));
213+
assertThat(e.getMessage(), endsWith(" has no permissions to run the 'ping' command"));
208214
}
209215

210216
jedis2.close();
211-
jedis.aclDelUser(USER_NAME);
212217
}
213218

214219
@Test
215220
public void aclDryRun() {
216-
jedis.aclDelUser(USER_NAME);
217-
218221
jedis.aclSetUser(USER_NAME, "nopass", "allkeys", "+set", "-get");
219222

220223
assertEquals("OK", jedis.aclDryRun(USER_NAME, "SET", "key", "value"));
221-
assertEquals("This user has no permissions to run the 'get' command",
222-
jedis.aclDryRun(USER_NAME, "GET", "key"));
224+
assertThat(jedis.aclDryRun(USER_NAME, "GET", "key"),
225+
endsWith(" has no permissions to run the 'get' command"));
223226

224227
assertEquals("OK", jedis.aclDryRun(USER_NAME,
225-
new CommandArguments(Protocol.Command.SET).key("key").add("value")));
226-
assertEquals("This user has no permissions to run the 'get' command", jedis.aclDryRun(USER_NAME,
227-
new CommandArguments(Protocol.Command.GET).key("key")));
228-
229-
jedis.aclDelUser(USER_NAME);
228+
new CommandArguments(Protocol.Command.SET).key("ca-key").add("value")));
229+
assertThat(jedis.aclDryRun(USER_NAME, new CommandArguments(Protocol.Command.GET).key("ca-key")),
230+
endsWith(" has no permissions to run the 'get' command"));
230231
}
231232

232233
@Test
233234
public void aclDryRunBinary() {
234235
byte[] username = USER_NAME.getBytes();
235-
jedis.aclDelUser(username);
236236

237237
jedis.aclSetUser(username, "nopass".getBytes(), "allkeys".getBytes(), "+set".getBytes(), "-get".getBytes());
238238

239-
assertArrayEquals("OK".getBytes(), jedis.aclDryRunBinary(username, "SET".getBytes(), "key".getBytes(), "value".getBytes()));
240-
assertArrayEquals("This user has no permissions to run the 'get' command".getBytes(),
241-
jedis.aclDryRunBinary(username, "GET".getBytes(), "key".getBytes()));
239+
assertArrayEquals("OK".getBytes(), jedis.aclDryRunBinary(username,
240+
"SET".getBytes(), "key".getBytes(), "value".getBytes()));
241+
assertThat(new String(jedis.aclDryRunBinary(username, "GET".getBytes(), "key".getBytes())),
242+
endsWith(" has no permissions to run the 'get' command"));
242243

243-
assertArrayEquals("OK".getBytes(), jedis.aclDryRunBinary(username, new CommandArguments(Protocol.Command.SET).key("key").add("value")));
244-
assertArrayEquals("This user has no permissions to run the 'get' command".getBytes(),
245-
jedis.aclDryRunBinary(username, new CommandArguments(Protocol.Command.GET).key("key")));
246-
247-
jedis.aclDelUser(USER_NAME);
244+
assertArrayEquals("OK".getBytes(), jedis.aclDryRunBinary(username,
245+
new CommandArguments(Protocol.Command.SET).key("ca-key").add("value")));
246+
assertThat(new String(jedis.aclDryRunBinary(username,
247+
new CommandArguments(Protocol.Command.GET).key("ca-key"))),
248+
endsWith(" has no permissions to run the 'get' command"));
248249
}
249250

250251
@Test
@@ -259,86 +260,66 @@ public void aclDelUser() {
259260

260261
@Test
261262
public void basicPermissionsTest() {
262-
263263
// create a user with login permissions
264-
265-
jedis.aclDelUser(USER_NAME); // delete the user
264+
jedis.aclSetUser(USER_NAME, ">" + USER_PASSWORD);
266265

267266
// users are not able to access any command
268-
String status = jedis.aclSetUser(USER_NAME, ">" + USER_PASSWORD);
269-
String authResult = jedis.aclSetUser(USER_NAME, "on", "+acl");
267+
jedis.aclSetUser(USER_NAME, "on", "+acl");
270268

271269
// connect with this new user and try to get/set keys
272270
Jedis jedis2 = new Jedis();
273271
jedis2.auth(USER_NAME, USER_PASSWORD);
274272

275-
String result = null;
276273
try {
277-
result = jedis2.set("foo", "bar");
274+
jedis2.set("foo", "bar");
278275
fail("Should throw a NOPERM exception");
279276
} catch (JedisAccessControlException e) {
280-
assertNull(result);
281-
assertEquals(
282-
"NOPERM this user has no permissions to run the 'set' command",
283-
e.getMessage());
277+
assertThat(e.getMessage(), startsWith("NOPERM "));
278+
assertThat(e.getMessage(), endsWith(" has no permissions to run the 'set' command"));
284279
}
285280

286281
// change permissions of the user
287282
// by default users are not able to access any key
288-
status = jedis.aclSetUser(USER_NAME, "+set");
283+
jedis.aclSetUser(USER_NAME, "+set");
289284

290285
jedis2.close();
291286
jedis2.auth(USER_NAME, USER_PASSWORD);
292287

293-
result = null;
294288
try {
295-
result = jedis2.set("foo", "bar");
289+
jedis2.set("foo", "bar");
296290
fail("Should throw a NOPERM exception");
297291
} catch (JedisAccessControlException e) {
298-
assertNull(result);
299-
assertEquals(
300-
"NOPERM this user has no permissions to access one of the keys used as arguments",
301-
e.getMessage());
292+
assertEquals("NOPERM No permissions to access a key", e.getMessage());
302293
}
303294

304295
// allow user to access a subset of the key
305-
result = jedis.aclSetUser(USER_NAME, "allcommands", "~foo:*", "~bar:*"); // TODO : Define a DSL
296+
jedis.aclSetUser(USER_NAME, "allcommands", "~foo:*", "~bar:*"); // TODO : Define a DSL
306297

307298
// create key foo, bar and zap
308-
result = jedis2.set("foo:1", "a");
309-
assertEquals("OK", status);
299+
jedis2.set("foo:1", "a");
310300

311-
result = jedis2.set("bar:2", "b");
312-
assertEquals("OK", status);
301+
jedis2.set("bar:2", "b");
313302

314-
result = null;
315303
try {
316-
result = jedis2.set("zap:3", "c");
304+
jedis2.set("zap:3", "c");
317305
fail("Should throw a NOPERM exception");
318306
} catch (JedisAccessControlException e) {
319-
assertNull(result);
320-
assertEquals(
321-
"NOPERM this user has no permissions to access one of the keys used as arguments",
322-
e.getMessage());
307+
assertEquals("NOPERM No permissions to access a key", e.getMessage());
323308
}
324-
325-
// remove user
326-
jedis.aclDelUser(USER_NAME); // delete the user
327-
328309
}
329310

330311
@Test
331312
public void aclCatTest() {
332313
List<String> categories = jedis.aclCat();
333-
assertTrue(!categories.isEmpty());
314+
assertFalse(categories.isEmpty());
334315

335316
// test binary
336317
List<byte[]> categoriesBinary = jedis.aclCatBinary();
337-
assertTrue(!categories.isEmpty());
318+
assertFalse(categories.isEmpty());
338319
assertEquals(categories.size(), categoriesBinary.size());
339320

340321
// test commands in a category
341-
assertTrue(!jedis.aclCat("scripting").isEmpty());
322+
assertFalse(jedis.aclCat("scripting").isEmpty());
342323

343324
try {
344325
jedis.aclCat("testcategory");
@@ -513,7 +494,7 @@ public void aclLoadTest() {
513494
jedis.aclLoad();
514495
fail("Should throw a JedisDataException: ERR This Redis instance is not configured to use an ACL file...");
515496
} catch (JedisDataException e) {
516-
assertTrue(e.getMessage().contains("ERR This Redis instance is not configured to use an ACL file."));
497+
assertThat(e.getMessage(), startsWith("ERR This Redis instance is not configured to use an ACL file."));
517498
}
518499

519500
// TODO test with ACL file
@@ -525,7 +506,7 @@ public void aclSaveTest() {
525506
jedis.aclSave();
526507
fail("Should throw a JedisDataException: ERR This Redis instance is not configured to use an ACL file...");
527508
} catch (JedisDataException e) {
528-
assertTrue(e.getMessage().contains("ERR This Redis instance is not configured to use an ACL file."));
509+
assertThat(e.getMessage(), startsWith("ERR This Redis instance is not configured to use an ACL file."));
529510
}
530511

531512
// TODO test with ACL file

0 commit comments

Comments
 (0)