You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by ja...@apache.org on 2017/09/18 16:16:18 UTC
[geode] branch develop updated: GEODE-3578: Only DATA:READ
permissions are required for creating/closing a cq from a client
This is an automated email from the ASF dual-hosted git repository.
jasonhuynh pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/geode.git
The following commit(s) were added to refs/heads/develop by this push:
new 7658cea GEODE-3578: Only DATA:READ permissions are required for creating/closing a cq from a client
7658cea is described below
commit 7658cea6a8906a1055eac97ea1fbd003fb81617d
Author: Jason Huynh <hu...@gmail.com>
AuthorDate: Thu Sep 7 15:46:56 2017 -0700
GEODE-3578: Only DATA:READ permissions are required for creating/closing a cq from a client
* As part of this commit GEODE-3576: Moved security check earlier in cq process
---
.../cache/tier/sockets/command/CloseCQ.java | 24 ++++++++++---------
.../cache/tier/sockets/command/ExecuteCQ61.java | 7 +++---
.../cache/tier/sockets/command/CloseCQTest.java | 5 ++--
.../tier/sockets/command/ExecuteCQ61Test.java | 4 ++--
.../geode/security/ClientCQAuthDUnitTest.java | 5 ++--
.../geode/test/dunit/rules/CQUnitTestRule.java | 27 +++++++++++++++++++++-
6 files changed, 49 insertions(+), 23 deletions(-)
diff --git a/geode-cq/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/CloseCQ.java b/geode-cq/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/CloseCQ.java
index 82c4dae..8b34b34 100644
--- a/geode-cq/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/CloseCQ.java
+++ b/geode-cq/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/CloseCQ.java
@@ -76,8 +76,6 @@ public class CloseCQ extends BaseCQCommand {
return;
}
- securityService.authorize(Resource.CLUSTER, Operation.MANAGE, Target.QUERY);
-
// Process CQ close request
try {
// Append Client ID to CQ name
@@ -90,19 +88,23 @@ public class CloseCQ extends BaseCQCommand {
}
InternalCqQuery cqQuery = cqService.getCq(serverCqName);
- AuthorizeRequest authzRequest = serverConnection.getAuthzRequest();
- if (authzRequest != null) {
+ if (cqQuery != null) {
+ securityService.authorize(Resource.DATA, Operation.READ, cqQuery.getRegionName());
+
+ AuthorizeRequest authzRequest = serverConnection.getAuthzRequest();
+ if (authzRequest != null) {
+
+ if (cqQuery != null) {
+ String queryStr = cqQuery.getQueryString();
+ Set cqRegionNames = new HashSet();
+ cqRegionNames.add(cqQuery.getRegionName());
+ authzRequest.closeCQAuthorize(cqName, queryStr, cqRegionNames);
+ }
- if (cqQuery != null) {
- String queryStr = cqQuery.getQueryString();
- Set cqRegionNames = new HashSet();
- cqRegionNames.add(cqQuery.getRegionName());
- authzRequest.closeCQAuthorize(cqName, queryStr, cqRegionNames);
}
+ cqService.closeCq(cqName, id);
}
-
- cqService.closeCq(cqName, id);
if (cqQuery != null)
serverConnection.removeCq(cqName, cqQuery.isDurable());
} catch (CqException cqe) {
diff --git a/geode-cq/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/ExecuteCQ61.java b/geode-cq/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/ExecuteCQ61.java
index 4920694..6e59f67 100755
--- a/geode-cq/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/ExecuteCQ61.java
+++ b/geode-cq/src/main/java/org/apache/geode/internal/cache/tier/sockets/command/ExecuteCQ61.java
@@ -120,9 +120,9 @@ public class ExecuteCQ61 extends BaseCQCommand {
// Authorization check
AuthorizeRequest authzRequest = serverConnection.getAuthzRequest();
+ query = qService.newQuery(cqQueryString);
+ cqRegionNames = ((DefaultQuery) query).getRegionsInQuery(null);
if (authzRequest != null) {
- query = qService.newQuery(cqQueryString);
- cqRegionNames = ((DefaultQuery) query).getRegionsInQuery(null);
executeCQContext = authzRequest.executeCQAuthorize(cqName, cqQueryString, cqRegionNames);
String newCqQueryString = executeCQContext.getQuery();
@@ -137,7 +137,8 @@ public class ExecuteCQ61 extends BaseCQCommand {
}
// auth check to see if user can create CQ or not
- securityService.authorize(Resource.CLUSTER, Operation.MANAGE, Target.QUERY);
+ ((DefaultQuery) query).getRegionsInQuery(null).forEach((regionName) -> securityService
+ .authorize(Resource.DATA, Operation.READ, (String) regionName));
// test hook to trigger vMotion during CQ registration
if (CqServiceProvider.VMOTION_DURING_CQ_REGISTRATION_FLAG) {
diff --git a/geode-cq/src/test/java/org/apache/geode/internal/cache/tier/sockets/command/CloseCQTest.java b/geode-cq/src/test/java/org/apache/geode/internal/cache/tier/sockets/command/CloseCQTest.java
index b242c6e..6cffb31 100644
--- a/geode-cq/src/test/java/org/apache/geode/internal/cache/tier/sockets/command/CloseCQTest.java
+++ b/geode-cq/src/test/java/org/apache/geode/internal/cache/tier/sockets/command/CloseCQTest.java
@@ -25,7 +25,6 @@ import org.junit.experimental.categories.Category;
import org.apache.geode.security.ResourcePermission.Operation;
import org.apache.geode.security.ResourcePermission.Resource;
-import org.apache.geode.security.ResourcePermission.Target;
import org.apache.geode.test.dunit.rules.CQUnitTestRule;
import org.apache.geode.test.junit.categories.UnitTest;
@@ -36,13 +35,13 @@ public class CloseCQTest {
public CQUnitTestRule cqRule = new CQUnitTestRule();
@Test
- public void needClusterManageQueryToStopCQ() throws Exception {
+ public void needDataReadRegionToClose() throws Exception {
CloseCQ closeCQ = mock(CloseCQ.class);
doCallRealMethod().when(closeCQ).cmdExecute(cqRule.message, cqRule.connection,
cqRule.securityService, 0);
closeCQ.cmdExecute(cqRule.message, cqRule.connection, cqRule.securityService, 0);
- verify(cqRule.securityService).authorize(Resource.CLUSTER, Operation.MANAGE, Target.QUERY);
+ verify(cqRule.securityService).authorize(Resource.DATA, Operation.READ, "regionName");
}
}
diff --git a/geode-cq/src/test/java/org/apache/geode/internal/cache/tier/sockets/command/ExecuteCQ61Test.java b/geode-cq/src/test/java/org/apache/geode/internal/cache/tier/sockets/command/ExecuteCQ61Test.java
index 51f7533..d59281f 100644
--- a/geode-cq/src/test/java/org/apache/geode/internal/cache/tier/sockets/command/ExecuteCQ61Test.java
+++ b/geode-cq/src/test/java/org/apache/geode/internal/cache/tier/sockets/command/ExecuteCQ61Test.java
@@ -35,12 +35,12 @@ public class ExecuteCQ61Test {
public CQUnitTestRule cqRule = new CQUnitTestRule();
@Test
- public void needClusterQueryManageToExecute() throws Exception {
+ public void needRegionRegionToExecute() throws Exception {
ExecuteCQ61 executeCQ61 = mock(ExecuteCQ61.class);
doCallRealMethod().when(executeCQ61).cmdExecute(cqRule.message, cqRule.connection,
cqRule.securityService, 0);
executeCQ61.cmdExecute(cqRule.message, cqRule.connection, cqRule.securityService, 0);
- verify(cqRule.securityService).authorize(Resource.CLUSTER, Operation.MANAGE, Target.QUERY);
+ verify(cqRule.securityService).authorize(Resource.DATA, Operation.READ, "regionName");
}
}
diff --git a/geode-cq/src/test/java/org/apache/geode/security/ClientCQAuthDUnitTest.java b/geode-cq/src/test/java/org/apache/geode/security/ClientCQAuthDUnitTest.java
index 5f0894e..c6fcb77 100644
--- a/geode-cq/src/test/java/org/apache/geode/security/ClientCQAuthDUnitTest.java
+++ b/geode-cq/src/test/java/org/apache/geode/security/ClientCQAuthDUnitTest.java
@@ -70,9 +70,8 @@ public class ClientCQAuthDUnitTest {
// Create the CqQuery (this is on the client side)
CqQuery cq = qs.newCq("CQ1", query, cqa);
- assertNotAuthorized(cq::execute, "CLUSTER:MANAGE:QUERY");
- assertNotAuthorized(cq::executeWithInitialResults, "CLUSTER:MANAGE:QUERY");
- assertNotAuthorized(cq::close, "CLUSTER:MANAGE:QUERY");
+ assertNotAuthorized(cq::execute, "DATA:READ:AuthRegion");
+ assertNotAuthorized(cq::executeWithInitialResults, "DATA:READ:AuthRegion");
assertNotAuthorized(qs::getAllDurableCqsFromServer, "CLUSTER:READ");
});
diff --git a/geode-cq/src/test/java/org/apache/geode/test/dunit/rules/CQUnitTestRule.java b/geode-cq/src/test/java/org/apache/geode/test/dunit/rules/CQUnitTestRule.java
index 5a6294b..3fd01e5 100644
--- a/geode-cq/src/test/java/org/apache/geode/test/dunit/rules/CQUnitTestRule.java
+++ b/geode-cq/src/test/java/org/apache/geode/test/dunit/rules/CQUnitTestRule.java
@@ -15,13 +15,22 @@
package org.apache.geode.test.dunit.rules;
+import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyInt;
+import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
+import java.util.HashSet;
+import java.util.Set;
+
import org.junit.rules.ExternalResource;
+import org.apache.geode.cache.query.QueryService;
+import org.apache.geode.cache.query.internal.DefaultQuery;
+import org.apache.geode.cache.query.internal.DefaultQueryService;
import org.apache.geode.cache.query.internal.cq.CqService;
+import org.apache.geode.cache.query.internal.cq.InternalCqQuery;
import org.apache.geode.internal.cache.InternalCache;
import org.apache.geode.internal.cache.tier.CachedRegionHelper;
import org.apache.geode.internal.cache.tier.sockets.AcceptorImpl;
@@ -37,15 +46,26 @@ public class CQUnitTestRule extends ExternalResource {
public Message message;
public ServerConnection connection;
public InternalCache cache;
+ public CqService cqService;
+ public InternalCqQuery internalCqQuery;
protected void before() throws Throwable {
securityService = mock(SecurityService.class);
message = mock(Message.class);
connection = mock(ServerConnection.class);
cache = mock(InternalCache.class);
+ cqService = mock(CqService.class);
+ internalCqQuery = mock(InternalCqQuery.class);
+ String regionName = "regionName";
Part part = mock(Part.class);
CachedRegionHelper crHelper = mock(CachedRegionHelper.class);
+ DefaultQueryService queryService = mock(DefaultQueryService.class);
+ DefaultQuery query = mock(DefaultQuery.class);
+
+ Set<String> regionsInQuery = new HashSet();
+ regionsInQuery.add(regionName);
+
when(connection.getCachedRegionHelper()).thenReturn(crHelper);
when(connection.getCacheServerStats()).thenReturn(mock(CacheServerStats.class));
when(connection.getAcceptor()).thenReturn(mock(AcceptorImpl.class));
@@ -54,7 +74,12 @@ public class CQUnitTestRule extends ExternalResource {
when(part.getString()).thenReturn("CQ");
when(part.getInt()).thenReturn(10);
when(crHelper.getCache()).thenReturn(cache);
- when(cache.getCqService()).thenReturn(mock(CqService.class));
+ when(cache.getCqService()).thenReturn(cqService);
+ when(cache.getLocalQueryService()).thenReturn(queryService);
+ when(queryService.newQuery(anyString())).thenReturn(query);
+ when(query.getRegionsInQuery(null)).thenReturn(regionsInQuery);
+ when(cqService.getCq("CQ")).thenReturn(internalCqQuery);
+ when(internalCqQuery.getRegionName()).thenReturn(regionName);
}
}
--
To stop receiving notification emails like this one, please contact
['"commits@geode.apache.org" <co...@geode.apache.org>'].