You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by wi...@apache.org on 2018/03/09 21:40:16 UTC
[geode] branch develop updated: GEODE-4750: Allow region level
authorization for OQL queries (#1570)
This is an automated email from the ASF dual-hosted git repository.
wirebaron 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 17602ce GEODE-4750: Allow region level authorization for OQL queries (#1570)
17602ce is described below
commit 17602ce01f82188b807cae63c65770e33006a26f
Author: Brian Rowe <br...@pivotal.io>
AuthorDate: Fri Mar 9 13:40:13 2018 -0800
GEODE-4750: Allow region level authorization for OQL queries (#1570)
---
.../geode/cache/query/internal/DefaultQuery.java | 4 +--
.../AbstractFunctionRequestOperationHandler.java | 7 ++--
.../OqlQueryRequestOperationHandler.java | 32 ++++++++++++++++++-
.../registry/ProtobufOperationContextRegistry.java | 4 ++-
.../protobuf/v1/AuthorizationIntegrationTest.java | 37 ++++++++++++++++++++++
...ionOnGroupRequestOperationHandlerJUnitTest.java | 15 +++++----
...onOnMemberRequestOperationHandlerJUnitTest.java | 16 +++++-----
...onOnRegionRequestOperationHandlerJUnitTest.java | 17 +++++-----
8 files changed, 101 insertions(+), 31 deletions(-)
diff --git a/geode-core/src/main/java/org/apache/geode/cache/query/internal/DefaultQuery.java b/geode-core/src/main/java/org/apache/geode/cache/query/internal/DefaultQuery.java
index 4df0bbf..ab0f898 100644
--- a/geode-core/src/main/java/org/apache/geode/cache/query/internal/DefaultQuery.java
+++ b/geode-core/src/main/java/org/apache/geode/cache/query/internal/DefaultQuery.java
@@ -772,8 +772,8 @@ public class DefaultQuery implements Query {
* @param parameters the parameters to be passed in to the query when executed
* @return Unmodifiable List containing the region names.
*/
- public Set getRegionsInQuery(Object[] parameters) {
- Set regions = new HashSet();
+ public Set<String> getRegionsInQuery(Object[] parameters) {
+ Set<String> regions = new HashSet<>();
this.compiledQuery.getRegionsInQuery(regions, parameters);
return Collections.unmodifiableSet(regions);
}
diff --git a/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/operations/AbstractFunctionRequestOperationHandler.java b/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/operations/AbstractFunctionRequestOperationHandler.java
index c1a475c..afba64f 100644
--- a/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/operations/AbstractFunctionRequestOperationHandler.java
+++ b/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/operations/AbstractFunctionRequestOperationHandler.java
@@ -37,6 +37,7 @@ import org.apache.geode.internal.protocol.protobuf.v1.Result;
import org.apache.geode.internal.protocol.protobuf.v1.serialization.exception.DecodingException;
import org.apache.geode.internal.protocol.protobuf.v1.serialization.exception.EncodingException;
import org.apache.geode.internal.protocol.protobuf.v1.state.ProtobufConnectionAuthorizingStateProcessor;
+import org.apache.geode.internal.protocol.protobuf.v1.state.exception.OperationNotAuthorizedException;
import org.apache.geode.internal.security.SecurityService;
import org.apache.geode.security.NotAuthorizedException;
@@ -71,9 +72,9 @@ public abstract class AbstractFunctionRequestOperationHandler<Req, Resp>
// check security for function.
function.getRequiredPermissions(regionName).forEach(securityService::authorize);
} catch (NotAuthorizedException ex) {
- final String message = "Authorization failed for function \"" + functionID + "\"";
- logger.warn(message, ex);
- return Failure.of(BasicTypes.ErrorCode.AUTHORIZATION_FAILED, message);
+ messageExecutionContext.getStatistics().incAuthorizationViolations();
+ throw new OperationNotAuthorizedException(
+ "The user is not authorized to complete this operation");
} finally {
if (threadState != null) {
((ProtobufConnectionAuthorizingStateProcessor) messageExecutionContext
diff --git a/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/operations/OqlQueryRequestOperationHandler.java b/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/operations/OqlQueryRequestOperationHandler.java
index 448fcea..faabb91 100644
--- a/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/operations/OqlQueryRequestOperationHandler.java
+++ b/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/operations/OqlQueryRequestOperationHandler.java
@@ -16,13 +16,16 @@ package org.apache.geode.internal.protocol.protobuf.v1.operations;
import java.util.Arrays;
import java.util.List;
+import java.util.Set;
import org.apache.logging.log4j.Logger;
+import org.apache.shiro.util.ThreadState;
import org.apache.geode.cache.query.Query;
import org.apache.geode.cache.query.QueryException;
import org.apache.geode.cache.query.SelectResults;
import org.apache.geode.cache.query.Struct;
+import org.apache.geode.cache.query.internal.DefaultQuery;
import org.apache.geode.cache.query.internal.InternalQueryService;
import org.apache.geode.cache.query.types.StructType;
import org.apache.geode.internal.exception.InvalidExecutionContextException;
@@ -40,7 +43,12 @@ import org.apache.geode.internal.protocol.protobuf.v1.Result;
import org.apache.geode.internal.protocol.protobuf.v1.Success;
import org.apache.geode.internal.protocol.protobuf.v1.serialization.exception.DecodingException;
import org.apache.geode.internal.protocol.protobuf.v1.serialization.exception.EncodingException;
+import org.apache.geode.internal.protocol.protobuf.v1.state.ProtobufConnectionAuthorizingStateProcessor;
import org.apache.geode.internal.protocol.protobuf.v1.state.exception.ConnectionStateException;
+import org.apache.geode.internal.protocol.protobuf.v1.state.exception.OperationNotAuthorizedException;
+import org.apache.geode.internal.security.SecurityService;
+import org.apache.geode.security.NotAuthorizedException;
+import org.apache.geode.security.ResourcePermission;
public class OqlQueryRequestOperationHandler
implements ProtobufOperationHandler<OQLQueryRequest, OQLQueryResponse> {
@@ -57,8 +65,30 @@ public class OqlQueryRequestOperationHandler
InternalQueryService queryService = messageExecutionContext.getCache().getQueryService();
Query query = queryService.newQuery(queryString);
-
Object[] bindParameters = decodeBindParameters(serializationService, encodedParameters);
+
+ if (messageExecutionContext
+ .getConnectionStateProcessor() instanceof ProtobufConnectionAuthorizingStateProcessor) {
+ final SecurityService securityService =
+ messageExecutionContext.getCache().getSecurityService();
+ ThreadState threadState =
+ ((ProtobufConnectionAuthorizingStateProcessor) messageExecutionContext
+ .getConnectionStateProcessor()).prepareThreadForAuthorization();
+ try {
+ for (String regionName : ((DefaultQuery) query).getRegionsInQuery(bindParameters)) {
+ securityService.authorize(ResourcePermission.Resource.DATA,
+ ResourcePermission.Operation.READ, regionName);
+ }
+ } catch (NotAuthorizedException ex) {
+ messageExecutionContext.getStatistics().incAuthorizationViolations();
+ throw new OperationNotAuthorizedException(
+ "The user is not authorized to complete this operation");
+ } finally {
+ ((ProtobufConnectionAuthorizingStateProcessor) messageExecutionContext
+ .getConnectionStateProcessor()).restoreThreadState(threadState);
+ }
+ }
+
try {
Object results = query.execute(bindParameters);
return Success.of(encodeResults(serializationService, results));
diff --git a/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/registry/ProtobufOperationContextRegistry.java b/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/registry/ProtobufOperationContextRegistry.java
index 40c3cf8..552f71e 100644
--- a/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/registry/ProtobufOperationContextRegistry.java
+++ b/geode-protobuf/src/main/java/org/apache/geode/internal/protocol/protobuf/v1/registry/ProtobufOperationContextRegistry.java
@@ -158,7 +158,9 @@ public class ProtobufOperationContextRegistry {
new ProtobufOperationContext<>(ClientProtocol.Message::getOqlQueryRequest,
new OqlQueryRequestOperationHandler(),
opsResp -> ClientProtocol.Message.newBuilder().setOqlQueryResponse(opsResp),
- new ResourcePermission(Resource.DATA, Operation.READ)));
+ // Perform authorization inside handler to avoid having to compile the OQL multiple
+ // times
+ this::skipAuthorizationCheck));
operationContexts.put(MessageTypeCase.KEYSETREQUEST,
new ProtobufOperationContext<>(ClientProtocol.Message::getKeySetRequest,
diff --git a/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/AuthorizationIntegrationTest.java b/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/AuthorizationIntegrationTest.java
index 2eaed1e..5fdd2c5 100644
--- a/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/AuthorizationIntegrationTest.java
+++ b/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/AuthorizationIntegrationTest.java
@@ -63,6 +63,9 @@ public class AuthorizationIntegrationTest {
private static final String TEST_KEY1 = "testKey1";
private static final String TEST_KEY2 = "testKey2";
+ private final String OQLTwoRegionTestQuery =
+ "select * from /" + TEST_REGION1 + " one, /" + TEST_REGION2 + " two where one.id = two.id";
+
@Rule
public final RestoreSystemProperties restoreSystemProperties = new RestoreSystemProperties();
@@ -259,6 +262,40 @@ public class AuthorizationIntegrationTest {
verifyBatchOperation(false, TEST_REGION2, false, false);
}
+ @Test
+ public void testOQLAuthorization() throws Exception {
+ securityManager.addAllowedPermission(new ResourcePermission(ResourcePermission.Resource.DATA,
+ ResourcePermission.Operation.READ, TEST_REGION1));
+ securityManager.addAllowedPermission(new ResourcePermission(ResourcePermission.Resource.DATA,
+ ResourcePermission.Operation.READ, TEST_REGION2));
+
+ ClientProtocol.Message request = ClientProtocol.Message.newBuilder()
+ .setOqlQueryRequest(RegionAPI.OQLQueryRequest.newBuilder().setQuery(OQLTwoRegionTestQuery))
+ .build();
+ protobufProtocolSerializer.serialize(request, outputStream);
+
+ ClientProtocol.Message response = protobufProtocolSerializer.deserialize(inputStream);
+ assertEquals(ClientProtocol.Message.MessageTypeCase.OQLQUERYRESPONSE,
+ response.getMessageTypeCase());
+ }
+
+ @Test
+ public void testOQLRequiresAuthorizationForMultipleRegions() throws Exception {
+ securityManager.addAllowedPermission(new ResourcePermission(ResourcePermission.Resource.DATA,
+ ResourcePermission.Operation.READ, TEST_REGION1));
+
+ ClientProtocol.Message request = ClientProtocol.Message.newBuilder()
+ .setOqlQueryRequest(RegionAPI.OQLQueryRequest.newBuilder().setQuery(OQLTwoRegionTestQuery))
+ .build();
+ protobufProtocolSerializer.serialize(request, outputStream);
+
+ ClientProtocol.Message response = protobufProtocolSerializer.deserialize(inputStream);
+ assertEquals(ClientProtocol.Message.MessageTypeCase.ERRORRESPONSE,
+ response.getMessageTypeCase());
+ assertEquals(BasicTypes.ErrorCode.AUTHORIZATION_FAILED,
+ response.getErrorResponse().getError().getErrorCode());
+ }
+
private void verifyBatchOperation(boolean testRead, String region, boolean expectedKey1Success,
boolean expectedKey2Success) throws Exception {
ClientProtocol.Message request;
diff --git a/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/operations/ExecuteFunctionOnGroupRequestOperationHandlerJUnitTest.java b/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/operations/ExecuteFunctionOnGroupRequestOperationHandlerJUnitTest.java
index cbf373b..385d9b5 100644
--- a/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/operations/ExecuteFunctionOnGroupRequestOperationHandlerJUnitTest.java
+++ b/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/operations/ExecuteFunctionOnGroupRequestOperationHandlerJUnitTest.java
@@ -19,6 +19,7 @@ import static org.apache.geode.internal.protocol.protobuf.v1.BasicTypes.ErrorCod
import static org.apache.geode.internal.protocol.protobuf.v1.BasicTypes.ErrorCode.NO_AVAILABLE_SERVER;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.mock;
@@ -49,6 +50,7 @@ import org.apache.geode.internal.protocol.protobuf.v1.FunctionAPI;
import org.apache.geode.internal.protocol.protobuf.v1.ProtobufSerializationService;
import org.apache.geode.internal.protocol.protobuf.v1.Result;
import org.apache.geode.internal.protocol.protobuf.v1.ServerMessageExecutionContext;
+import org.apache.geode.internal.protocol.protobuf.v1.state.exception.OperationNotAuthorizedException;
import org.apache.geode.internal.security.SecurityService;
import org.apache.geode.management.internal.security.ResourcePermissions;
import org.apache.geode.security.NotAuthorizedException;
@@ -172,13 +174,12 @@ public class ExecuteFunctionOnGroupRequestOperationHandlerJUnitTest {
FunctionAPI.ExecuteFunctionOnGroupRequest.newBuilder().setFunctionID(TEST_FUNCTION_ID)
.addGroupName(TEST_GROUP1).build();
- final Result<FunctionAPI.ExecuteFunctionOnGroupResponse> result =
- operationHandler.process(serializationService, request, mockedMessageExecutionContext());
-
- assertTrue(result instanceof Failure);
-
- assertEquals(AUTHORIZATION_FAILED, result.getErrorMessage().getError().getErrorCode());
-
+ try {
+ operationHandler.process(serializationService, request, mockedMessageExecutionContext());
+ fail("Should not have been authorized.");
+ } catch (OperationNotAuthorizedException ex) {
+ // Expected failure
+ }
}
@Test
diff --git a/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/operations/ExecuteFunctionOnMemberRequestOperationHandlerJUnitTest.java b/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/operations/ExecuteFunctionOnMemberRequestOperationHandlerJUnitTest.java
index c39a9b1..e296303 100644
--- a/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/operations/ExecuteFunctionOnMemberRequestOperationHandlerJUnitTest.java
+++ b/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/operations/ExecuteFunctionOnMemberRequestOperationHandlerJUnitTest.java
@@ -19,6 +19,7 @@ import static org.apache.geode.internal.protocol.protobuf.v1.BasicTypes.ErrorCod
import static org.apache.geode.internal.protocol.protobuf.v1.BasicTypes.ErrorCode.NO_AVAILABLE_SERVER;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.mock;
@@ -45,6 +46,7 @@ import org.apache.geode.internal.protocol.protobuf.v1.FunctionAPI;
import org.apache.geode.internal.protocol.protobuf.v1.ProtobufSerializationService;
import org.apache.geode.internal.protocol.protobuf.v1.Result;
import org.apache.geode.internal.protocol.protobuf.v1.ServerMessageExecutionContext;
+import org.apache.geode.internal.protocol.protobuf.v1.state.exception.OperationNotAuthorizedException;
import org.apache.geode.internal.security.SecurityService;
import org.apache.geode.management.internal.security.ResourcePermissions;
import org.apache.geode.security.NotAuthorizedException;
@@ -159,14 +161,12 @@ public class ExecuteFunctionOnMemberRequestOperationHandlerJUnitTest {
final FunctionAPI.ExecuteFunctionOnMemberRequest request =
FunctionAPI.ExecuteFunctionOnMemberRequest.newBuilder().setFunctionID(TEST_FUNCTION_ID)
.addMemberName(TEST_MEMBER1).build();
-
- final Result<FunctionAPI.ExecuteFunctionOnMemberResponse> result =
- operationHandler.process(serializationService, request, mockedMessageExecutionContext());
-
- assertTrue(result instanceof Failure);
-
- assertEquals(AUTHORIZATION_FAILED, result.getErrorMessage().getError().getErrorCode());
-
+ try {
+ operationHandler.process(serializationService, request, mockedMessageExecutionContext());
+ fail("Should not have been authorized.");
+ } catch (OperationNotAuthorizedException ex) {
+ // Expected failure
+ }
}
@Test
diff --git a/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/operations/ExecuteFunctionOnRegionRequestOperationHandlerJUnitTest.java b/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/operations/ExecuteFunctionOnRegionRequestOperationHandlerJUnitTest.java
index 07dce3a..2fa98d2 100644
--- a/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/operations/ExecuteFunctionOnRegionRequestOperationHandlerJUnitTest.java
+++ b/geode-protobuf/src/test/java/org/apache/geode/internal/protocol/protobuf/v1/operations/ExecuteFunctionOnRegionRequestOperationHandlerJUnitTest.java
@@ -16,6 +16,7 @@ package org.apache.geode.internal.protocol.protobuf.v1.operations;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.verify;
@@ -42,6 +43,7 @@ import org.apache.geode.internal.protocol.protobuf.v1.FunctionAPI;
import org.apache.geode.internal.protocol.protobuf.v1.ProtobufSerializationService;
import org.apache.geode.internal.protocol.protobuf.v1.Result;
import org.apache.geode.internal.protocol.protobuf.v1.ServerMessageExecutionContext;
+import org.apache.geode.internal.protocol.protobuf.v1.state.exception.OperationNotAuthorizedException;
import org.apache.geode.internal.security.SecurityService;
import org.apache.geode.management.internal.security.ResourcePermissions;
import org.apache.geode.security.NotAuthorizedException;
@@ -128,15 +130,12 @@ public class ExecuteFunctionOnRegionRequestOperationHandlerJUnitTest {
final FunctionAPI.ExecuteFunctionOnRegionRequest request =
FunctionAPI.ExecuteFunctionOnRegionRequest.newBuilder().setFunctionID(TEST_FUNCTION_ID)
.setRegion(TEST_REGION).build();
-
- final Result<FunctionAPI.ExecuteFunctionOnRegionResponse> result =
- operationHandler.process(serializationService, request, mockedMessageExecutionContext());
-
- assertTrue(result instanceof Failure);
-
- assertEquals(BasicTypes.ErrorCode.AUTHORIZATION_FAILED,
- result.getErrorMessage().getError().getErrorCode());
-
+ try {
+ operationHandler.process(serializationService, request, mockedMessageExecutionContext());
+ fail("Should not have been authorized.");
+ } catch (OperationNotAuthorizedException ex) {
+ // Expected failure
+ }
}
@Test
--
To stop receiving notification emails like this one, please contact
wirebaron@apache.org.