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

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

dschneider-pivotal commented on a change in pull request #7029:
URL: https://github.com/apache/geode/pull/7029#discussion_r736829024



##########
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:
       those test methods already exist (look for calls of "setupCacheWithSecurity(true)"). I'll add a test that calls this and does a read op but that will fail because the test is using SimpleSecurityManager. All the product is doing is saying what permission the current op requires. But is up to the SecurityManager to decide if the current principal has that permission.




-- 
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