[GitHub] [geode] nonbinaryprogrammer commented on a change in pull request #7029: GEODE-9740: require DATA:WRITE permission for WRITE redis ops and PUBLISH

[GitBox <gi...@apache.org>]

nonbinaryprogrammer commented on a change in pull request #7029:
URL: https://github.com/apache/geode/pull/7029#discussion_r736816240



##########
File path: geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/connection/AuthIntegrationTest.java
##########
@@ -137,16 +142,45 @@ public void givenNoSecurity_accessWithAuthAndUsernamePassword_fails() throws Exc
 
   @Test
   public void givenSecurity_accessWithCorrectAuthorization_passes() throws Exception {
-    setupCacheWithSecurity();
+    setupCacheWithSecurity(false);
 
     jedis.auth("dataWrite", "dataWrite");
 
     assertThat(jedis.set("foo", "bar")).isEqualTo("OK");
   }
 
+  @Test
+  public void givenSecurity_readOpWithReadAuthorization_passes() throws Exception {
+    setupCacheWithSecurity(false);
+
+    jedis.auth("dataRead", "dataRead");
+
+    assertThat(jedis.get("foo")).isNull();
+  }
+
+  @Test
+  public void givenSecurity_readOpWithWriteAuthorization_fails() throws Exception {
+    setupCacheWithSecurity(false);
+
+    jedis.auth("dataWrite", "dataWrite");
+
+    assertThatThrownBy(() -> jedis.get("foo"))
+        .hasMessageContaining(RedisConstants.ERROR_NOT_AUTHORIZED);
+  }
+
+  @Test
+  public void givenSecurity_writeOpWithReadAuthorization_fails() throws Exception {
+    setupCacheWithSecurity(false);
+
+    jedis.auth("dataRead", "dataRead");
+
+    assertThatThrownBy(() -> jedis.set("foo", "bar"))
+        .hasMessageContaining(RedisConstants.ERROR_NOT_AUTHORIZED);
+  }
+

Review comment:
       I think it would be good to add a couple of tests for when security is enabled and the user has write permissions. I'm especially interested in the case where the user with write permissions tries to read. I think it's worth documenting that behavior in tests.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@geode.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org