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/10/10 00:06:41 UTC

[geode] branch develop updated: GEODE-3787: Do not catch NotAuthorizedExceptions in CompiledIteratorDef

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 42b7658  GEODE-3787: Do not catch NotAuthorizedExceptions in CompiledIteratorDef
42b7658 is described below

commit 42b76583b1ac14e4caa4a1311620c0be2e2fb7de
Author: Jason Huynh <hu...@gmail.com>
AuthorDate: Fri Oct 6 17:44:29 2017 -0700

    GEODE-3787: Do not catch NotAuthorizedExceptions in CompiledIteratorDef
    
      * Fixed test where fail was not being called when needed
      * Queries that do not actually invoke a method but should be restricted
        are now reclassified into a new dunit test
---
 .../cache/query/internal/CompiledIteratorDef.java  |   3 +
 .../geode/cache/query/internal/CompiledSelect.java |   2 +-
 ...rictedButMethodsDoNotExistQueriesDUnitTest.java |  36 ++++++
 .../geode/security/query/QuerySecurityBase.java    |   3 +
 .../QuerySecurityRestrictedQueriesDUnitTest.java   |  98 +++++-----------
 ...rityRetrictedButMethodsDoNotExistDUnitTest.java | 127 +++++++++++++++++++++
 6 files changed, 198 insertions(+), 71 deletions(-)

diff --git a/geode-core/src/main/java/org/apache/geode/cache/query/internal/CompiledIteratorDef.java b/geode-core/src/main/java/org/apache/geode/cache/query/internal/CompiledIteratorDef.java
index 872614d..fda594d 100644
--- a/geode-core/src/main/java/org/apache/geode/cache/query/internal/CompiledIteratorDef.java
+++ b/geode-core/src/main/java/org/apache/geode/cache/query/internal/CompiledIteratorDef.java
@@ -41,6 +41,7 @@ import org.apache.geode.cache.query.types.MapType;
 import org.apache.geode.cache.query.types.ObjectType;
 import org.apache.geode.internal.i18n.LocalizedStrings;
 import org.apache.geode.internal.logging.LogService;
+import org.apache.geode.security.NotAuthorizedException;
 
 public class CompiledIteratorDef extends AbstractCompiledValue {
   private static final Logger logger = LogService.getLogger();
@@ -118,6 +119,8 @@ public class CompiledIteratorDef extends AbstractCompiledValue {
         throw qet;
       } catch (RegionNotFoundException re) {
         throw re;
+      } catch (NotAuthorizedException e) {
+        throw e;
       } catch (Exception e) {
         if (logger.isDebugEnabled()) {
           logger.debug("Exception while getting runtime iterator.", e);
diff --git a/geode-core/src/main/java/org/apache/geode/cache/query/internal/CompiledSelect.java b/geode-core/src/main/java/org/apache/geode/cache/query/internal/CompiledSelect.java
index 3ab90aa..dbaba84 100644
--- a/geode-core/src/main/java/org/apache/geode/cache/query/internal/CompiledSelect.java
+++ b/geode-core/src/main/java/org/apache/geode/cache/query/internal/CompiledSelect.java
@@ -640,7 +640,7 @@ public class CompiledSelect extends AbstractCompiledValue {
 
   /**
    * Returns the size of region iterator for count(*) on a region without whereclause.
-   * 
+   *
    * @since GemFire 6.6.2
    */
   private int getRegionIteratorSize(ExecutionContext context, CompiledValue collExpr)
diff --git a/geode-core/src/test/java/org/apache/geode/security/query/PartitionedQuerySecurityRestrictedButMethodsDoNotExistQueriesDUnitTest.java b/geode-core/src/test/java/org/apache/geode/security/query/PartitionedQuerySecurityRestrictedButMethodsDoNotExistQueriesDUnitTest.java
new file mode 100644
index 0000000..a670111
--- /dev/null
+++ b/geode-core/src/test/java/org/apache/geode/security/query/PartitionedQuerySecurityRestrictedButMethodsDoNotExistQueriesDUnitTest.java
@@ -0,0 +1,36 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
+ * agreements. See the NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package org.apache.geode.security.query;
+
+import org.junit.experimental.categories.Category;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+import org.apache.geode.cache.RegionShortcut;
+import org.apache.geode.test.junit.categories.DistributedTest;
+import org.apache.geode.test.junit.categories.SecurityTest;
+import org.apache.geode.test.junit.runners.CategoryWithParameterizedRunnerFactory;
+
+@Category({DistributedTest.class, SecurityTest.class})
+@RunWith(Parameterized.class)
+@Parameterized.UseParametersRunnerFactory(CategoryWithParameterizedRunnerFactory.class)
+public class PartitionedQuerySecurityRestrictedButMethodsDoNotExistQueriesDUnitTest
+    extends QuerySecurityRetrictedButMethodsDoNotExistDUnitTest {
+
+  public RegionShortcut getRegionType() {
+    return RegionShortcut.PARTITION;
+  }
+
+}
diff --git a/geode-core/src/test/java/org/apache/geode/security/query/QuerySecurityBase.java b/geode-core/src/test/java/org/apache/geode/security/query/QuerySecurityBase.java
index 1e1815e..3e71817 100644
--- a/geode-core/src/test/java/org/apache/geode/security/query/QuerySecurityBase.java
+++ b/geode-core/src/test/java/org/apache/geode/security/query/QuerySecurityBase.java
@@ -119,7 +119,9 @@ public class QuerySecurityBase extends JUnit4DistributedTestCase {
   protected void assertExceptionOccurred(QueryService qs, String query, String authErrorRegexp) {
     try {
       qs.newQuery(query).execute();
+      fail();
     } catch (Exception e) {
+      e.printStackTrace();
       if (!e.getMessage().matches(authErrorRegexp)) {
         Throwable cause = e.getCause();
         while (cause != null) {
@@ -141,6 +143,7 @@ public class QuerySecurityBase extends JUnit4DistributedTestCase {
       qs.newQuery(query).execute(bindParams);
       fail();
     } catch (Exception e) {
+
       if (!e.getMessage().matches(authErrorRegexp)) {
         Throwable cause = e.getCause();
         while (cause != null) {
diff --git a/geode-core/src/test/java/org/apache/geode/security/query/QuerySecurityRestrictedQueriesDUnitTest.java b/geode-core/src/test/java/org/apache/geode/security/query/QuerySecurityRestrictedQueriesDUnitTest.java
index bf39447..aae5532 100644
--- a/geode-core/src/test/java/org/apache/geode/security/query/QuerySecurityRestrictedQueriesDUnitTest.java
+++ b/geode-core/src/test/java/org/apache/geode/security/query/QuerySecurityRestrictedQueriesDUnitTest.java
@@ -14,6 +14,8 @@
  */
 package org.apache.geode.security.query;
 
+import static org.junit.Assert.fail;
+
 import java.io.Serializable;
 import java.text.ParseException;
 import java.text.SimpleDateFormat;
@@ -32,6 +34,8 @@ import org.apache.geode.cache.Region;
 import org.apache.geode.cache.RegionDestroyedException;
 import org.apache.geode.cache.client.Pool;
 import org.apache.geode.cache.client.PoolManager;
+import org.apache.geode.cache.query.QueryService;
+import org.apache.geode.cache.query.TypeMismatchException;
 import org.apache.geode.security.query.data.QueryTestObject;
 import org.apache.geode.test.dunit.VM;
 import org.apache.geode.test.junit.categories.DistributedTest;
@@ -162,30 +166,34 @@ public class QuerySecurityRestrictedQueriesDUnitTest extends QuerySecurityBase {
   }
 
   @Test
-  public void usersWhoCanExecuteQueryShouldNotInvokeRegionCreateForSelectRegionCreateQuery()
-      throws Exception {
-    String query = "select * from /" + regionName + ".create('key2', 15)";
-    executeQueryWithCheckForAccessPermissions(specificUserClient, query, regionName,
-        regexForExpectedExceptions);
-    executeAndConfirmRegionMatches(specificUserClient, regionName, Arrays.asList(values));
-  }
-
-
-  @Test
-  // @Parameters(method = "getAllUsersWhoCanExecuteQuery")
-  public void usersWhoCanExecuteQueryShouldGetResultsForSelectCreateFromRegionQuery()
-      throws Exception {
-    String query = "select r.create('key2', 15) from /" + regionName + " r";
-    executeQueryWithCheckForAccessPermissions(specificUserClient, query, regionName,
-        regexForExpectedExceptions);
-    executeAndConfirmRegionMatches(specificUserClient, regionName, Arrays.asList(values));
+  public void RegionMethodInvocationShouldThrowSecurityExceptionNotTypeMismatch() {
+    String query = "select * from /" + regionName + ".containsValue('value')";
+    specificUserClient.invoke(() -> {
+      QueryService queryService = getClientCache().getQueryService();
+      try {
+        queryService.newQuery(query).execute();
+        fail();
+      } catch (Exception e) {
+        e.printStackTrace();
+        if (!e.getMessage().matches(regexForExpectedExceptions)) {
+          Throwable cause = e.getCause();
+          while (cause != null || cause instanceof TypeMismatchException) {
+            if (cause.getMessage().matches(regexForExpectedExceptions)) {
+              return;
+            }
+            cause = cause.getCause();
+          }
+          e.printStackTrace();
+          fail();
+        }
+      }
+    });
   }
 
   @Test
-  // @Parameters(method = "getAllUsersWhoCanExecuteQuery")
-  public void usersWhoCanExecuteQueryShouldNotInvokeDestroyForSelectRegionDestroyQuery()
+  public void usersWhoCanExecuteQueryShouldNotInvokeRegionCreateForSelectRegionCreateQuery()
       throws Exception {
-    String query = "select * from /" + regionName + ".destroyKey('" + keys[0] + "')";
+    String query = "select * from /" + regionName + ".create('key2', 15)";
     executeQueryWithCheckForAccessPermissions(specificUserClient, query, regionName,
         regexForExpectedExceptions);
     executeAndConfirmRegionMatches(specificUserClient, regionName, Arrays.asList(values));
@@ -205,13 +213,6 @@ public class QuerySecurityRestrictedQueriesDUnitTest extends QuerySecurityBase {
   }
 
   @Test
-  public void checkUserAuthorizationsForSelectRegionGetQuery() {
-    String query = "select * from /" + regionName + ".getKey('" + keys[0] + "')";
-    executeQueryWithCheckForAccessPermissions(specificUserClient, query, regionName,
-        regexForExpectedExceptions);
-  }
-
-  @Test
   public void usersWhoCanExecuteQueryShouldNotInvokeRegionPutForSelectRegionPutQuery()
       throws Exception {
     String query = "select * from /" + regionName + ".put('key-2', 'something')";
@@ -220,18 +221,6 @@ public class QuerySecurityRestrictedQueriesDUnitTest extends QuerySecurityBase {
     executeAndConfirmRegionMatches(specificUserClient, regionName, Arrays.asList(values));
   }
 
-
-
-  @Test
-  public void usersWhoCanExecuteQueryShouldNotInvokePutIfAbsentForSelectRegionPutIfAbsentQuery()
-      throws Exception {
-    String query = "select * from /" + regionName + ".putIfAbsent('key-2', 'something')";
-    executeQueryWithCheckForAccessPermissions(specificUserClient, query, regionName,
-        regexForExpectedExceptions);
-    executeAndConfirmRegionMatches(specificUserClient, regionName, Arrays.asList(values));
-  }
-
-
   @Test
   @Parameters(method = "getAllUsersWhoCanExecuteQuery")
   public void usersWhoCanExecuteQueryShouldNotInvokedRegionRemoveForSelectRegionRemoveQuery()
@@ -243,30 +232,6 @@ public class QuerySecurityRestrictedQueriesDUnitTest extends QuerySecurityBase {
   }
 
   @Test
-  @Parameters(method = "getAllUsersWhoCanExecuteQuery")
-  public void usersWhoCannExecuteQueryShouldReceiveExpectedResultsForSelectRegionReplaceQuery()
-      throws Exception {
-    String query = "select * from /" + regionName + ".replace('key-0', 'something')";
-    executeQueryWithCheckForAccessPermissions(specificUserClient, query, regionName,
-        regexForExpectedExceptions);
-    executeAndConfirmRegionMatches(specificUserClient, regionName, Arrays.asList(values));
-  }
-
-  @Test
-  public void checkUserAuthorizationsForSelectGetInterestListRegexRegionQuery() {
-    String query = "select r.getInterestListRegex from /" + regionName + " r";
-    executeQueryWithCheckForAccessPermissions(specificUserClient, query, regionName,
-        regexForExpectedExceptions);
-  }
-
-  @Test
-  public void checkUserAuthorizationsForSelectGetInterestListRegexParenRegionQuery() {
-    String query = "select r.getInterestListRegex() from /" + regionName + " r";
-    executeQueryWithCheckForAccessPermissions(specificUserClient, query, regionName,
-        regexForExpectedExceptions);
-  }
-
-  @Test
   public void checkUserAuthorizationsForSelectByGetClassQuery() {
     String query = "select * from /" + regionName + " r where r.getClass != '1'";
     executeQueryWithCheckForAccessPermissions(specificUserClient, query, regionName,
@@ -308,11 +273,4 @@ public class QuerySecurityRestrictedQueriesDUnitTest extends QuerySecurityBase {
     executeQueryWithCheckForAccessPermissions(specificUserClient, query, regionName,
         regexForExpectedExceptions);
   }
-
-  @Test
-  public void checkUserAuthorizationsForSelectRegionCloneQuery() {
-    String query = "select * from /" + regionName + ".clone";
-    executeQueryWithCheckForAccessPermissions(specificUserClient, query, regionName,
-        regexForExpectedExceptions);
-  }
 }
diff --git a/geode-core/src/test/java/org/apache/geode/security/query/QuerySecurityRetrictedButMethodsDoNotExistDUnitTest.java b/geode-core/src/test/java/org/apache/geode/security/query/QuerySecurityRetrictedButMethodsDoNotExistDUnitTest.java
new file mode 100644
index 0000000..533b266
--- /dev/null
+++ b/geode-core/src/test/java/org/apache/geode/security/query/QuerySecurityRetrictedButMethodsDoNotExistDUnitTest.java
@@ -0,0 +1,127 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
+ * agreements. See the NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package org.apache.geode.security.query;
+
+import java.util.Arrays;
+import java.util.List;
+
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+
+import org.apache.geode.cache.query.QueryService;
+import org.apache.geode.cache.query.internal.index.IndexManager;
+import org.apache.geode.security.query.data.QueryTestObject;
+import org.apache.geode.test.junit.categories.DistributedTest;
+import org.apache.geode.test.junit.categories.SecurityTest;
+import org.apache.geode.test.junit.runners.CategoryWithParameterizedRunnerFactory;
+
+@Category({DistributedTest.class, SecurityTest.class})
+@RunWith(Parameterized.class)
+@Parameterized.UseParametersRunnerFactory(CategoryWithParameterizedRunnerFactory.class)
+public class QuerySecurityRetrictedButMethodsDoNotExistDUnitTest extends QuerySecurityBase {
+
+  @Parameterized.Parameters
+  public static Object[] usersAllowed() {
+    return new Object[] {"dataReader", "dataReaderRegion", "clusterManagerDataReader",
+        "clusterManagerDataReaderRegion", "super-user"};
+  }
+
+  @Parameterized.Parameter
+  public String user;
+
+  @Before
+  public void configureCache() {
+    createClientCache(specificUserClient, user, userPerms.getUserPassword(user));
+    createProxyRegion(specificUserClient, regionName);
+
+    keys = new Object[] {"key-0", "key-1"};
+    values = new Object[] {new QueryTestObject(1, "John"), new QueryTestObject(3, "Beth")};
+    putIntoRegion(superUserClient, keys, values, regionName);
+  }
+
+  @Test
+  public void executingMethodThatDoesNotExistOnResultsWillReturnUndefinedAsAResult() {
+    String query = "select r.getInterestListRegex() from /" + regionName + " r";
+    List<Object> expectedResults = Arrays.asList(QueryService.UNDEFINED, QueryService.UNDEFINED);
+    executeQueryWithCheckForAccessPermissions(specificUserClient, query, regionName,
+        expectedResults);
+  }
+
+  @Test
+  public void cloneExecutedOnQRegionWillReturnEmptyResults() {
+    String query = "select * from /" + regionName + ".clone";
+    List<Object> expectedResults = Arrays.asList();
+    executeQueryWithCheckForAccessPermissions(specificUserClient, query, regionName,
+        expectedResults);
+  }
+
+  @Test
+  public void executingMethodThatDoesNotExistOnQRegionWillReturnEmptyResult() {
+    String query = "select * from /" + regionName + ".getKey('" + keys[0] + "')";
+    List<Object> expectedResults = Arrays.asList();
+    executeQueryWithCheckForAccessPermissions(specificUserClient, query, regionName,
+        expectedResults);
+  }
+
+  @Test
+  public void executingCreateOnRegionWillResultInUndefinedButNotModifyRegion() throws Exception {
+    String query = "select r.create('key2', 15) from /" + regionName + " r";
+    List<Object> expectedResults = Arrays.asList(QueryService.UNDEFINED, QueryService.UNDEFINED);
+    executeQueryWithCheckForAccessPermissions(specificUserClient, query, regionName,
+        expectedResults);
+    executeAndConfirmRegionMatches(specificUserClient, regionName, Arrays.asList(values));
+  }
+
+  @Test
+  public void executingDestroyKeyOnRegionWillReturnEmptyResultsAndNotModifyRegion()
+      throws Exception {
+    String query = "select * from /" + regionName + ".destroyKey('" + keys[0] + "')";
+    List<Object> expectedResults = Arrays.asList();
+    executeQueryWithCheckForAccessPermissions(specificUserClient, query, regionName,
+        expectedResults);
+    executeAndConfirmRegionMatches(specificUserClient, regionName, Arrays.asList(values));
+  }
+
+  @Test
+  public void executingPutIfAbsentOnRegionWillReturnEmptyResultsAndNotModifyRegion()
+      throws Exception {
+    String query = "select * from /" + regionName + ".putIfAbsent('key-2', 'something')";
+    List<Object> expectedResults = Arrays.asList();
+    executeQueryWithCheckForAccessPermissions(specificUserClient, query, regionName,
+        expectedResults);
+    executeAndConfirmRegionMatches(specificUserClient, regionName, Arrays.asList(values));
+  }
+
+  @Test
+  public void executingReplaceOnRegionWillReturnEmptyResultsAndNotModifyRegion() throws Exception {
+    String query = "select * from /" + regionName + ".replace('key-0', 'something')";
+    List<Object> expectedResults = Arrays.asList();
+    executeQueryWithCheckForAccessPermissions(specificUserClient, query, regionName,
+        expectedResults);
+    executeAndConfirmRegionMatches(specificUserClient, regionName, Arrays.asList(values));
+  }
+
+  @Test
+  public void checkUserAuthorizationsForSelectGetInterestListRegexParenRegionQuery() {
+    String query = "select r.getInterestListRegex() from /" + regionName + " r";
+    List<Object> expectedResults = Arrays.asList(QueryService.UNDEFINED, QueryService.UNDEFINED);
+    executeQueryWithCheckForAccessPermissions(specificUserClient, query, regionName,
+        expectedResults);
+  }
+
+}

-- 
To stop receiving notification emails like this one, please contact
['"commits@geode.apache.org" <co...@geode.apache.org>'].