Repository: incubator-geode
Updated Branches:
  refs/heads/develop 584337b32 -> e15657e9d


GEODE-1979: refactor SecurityClusterConfig to remove the flakiness.

* add more test cases to cover more scenarios


Project: http://git-wip-us.apache.org/repos/asf/incubator-geode/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-geode/commit/e15657e9
Tree: http://git-wip-us.apache.org/repos/asf/incubator-geode/tree/e15657e9
Diff: http://git-wip-us.apache.org/repos/asf/incubator-geode/diff/e15657e9

Branch: refs/heads/develop
Commit: e15657e9de8bffdc8fa25249fcee86c46ce79928
Parents: 584337b
Author: Jinmei Liao <jil...@pivotal.io>
Authored: Tue Oct 11 09:43:33 2016 -0700
Committer: Jinmei Liao <jil...@pivotal.io>
Committed: Wed Oct 12 09:42:04 2016 -0700

----------------------------------------------------------------------
 .../ClusterConfigWithoutSecurityDUnitTest.java  | 102 ++++++++++++++++++
 .../SecurityClusterConfigDUnitTest.java         | 107 +++++--------------
 .../SecurityWithoutClusterConfigDUnitTest.java  |  92 ++++------------
 .../security/StartServerAuthorizationTest.java  |  73 +++++--------
 .../rules/LocatorServerConfigurationRule.java   |  79 +++++++-------
 .../LuceneClusterConfigurationDUnitTest.java    |  24 +----
 6 files changed, 223 insertions(+), 254 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/e15657e9/geode-core/src/test/java/org/apache/geode/security/ClusterConfigWithoutSecurityDUnitTest.java
----------------------------------------------------------------------
diff --git 
a/geode-core/src/test/java/org/apache/geode/security/ClusterConfigWithoutSecurityDUnitTest.java
 
b/geode-core/src/test/java/org/apache/geode/security/ClusterConfigWithoutSecurityDUnitTest.java
new file mode 100644
index 0000000..3854bb1
--- /dev/null
+++ 
b/geode-core/src/test/java/org/apache/geode/security/ClusterConfigWithoutSecurityDUnitTest.java
@@ -0,0 +1,102 @@
+/*
+ * 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;
+
+import static org.apache.geode.distributed.ConfigurationProperties.*;
+import static org.assertj.core.api.Java6Assertions.*;
+import static org.junit.Assert.*;
+
+import java.util.Properties;
+
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import org.apache.geode.GemFireConfigException;
+import org.apache.geode.cache.CacheFactory;
+import org.apache.geode.distributed.internal.InternalDistributedSystem;
+import org.apache.geode.internal.i18n.LocalizedStrings;
+import org.apache.geode.security.templates.SimpleSecurityManager;
+import org.apache.geode.test.dunit.IgnoredException;
+import org.apache.geode.test.dunit.internal.JUnit4DistributedTestCase;
+import org.apache.geode.test.dunit.rules.LocatorServerConfigurationRule;
+import org.apache.geode.test.junit.categories.DistributedTest;
+import org.apache.geode.test.junit.categories.SecurityTest;
+
+@Category({ DistributedTest.class, SecurityTest.class })
+
+public class ClusterConfigWithoutSecurityDUnitTest extends 
JUnit4DistributedTestCase {
+
+  @Rule
+  public LocatorServerConfigurationRule lsRule = new 
LocatorServerConfigurationRule(this);
+
+  @Before
+  public void before() throws Exception {
+    
IgnoredException.addIgnoredException(LocalizedStrings.GEMFIRE_CACHE_SECURITY_MISCONFIGURATION.toString());
+    
IgnoredException.addIgnoredException(LocalizedStrings.GEMFIRE_CACHE_SECURITY_MISCONFIGURATION_2.toString());
+    lsRule.getLocatorVM(new Properties());
+  }
+
+  @After
+  public void after() {
+    IgnoredException.removeAllExpectedExceptions();
+  }
+
+  // when locator is not secured, a secured server should be allowed to start 
with its own security manager
+  // if use-cluster-config is false
+  @Test
+  public void 
serverShouldBeAllowedToStartWithSecurityIfNotUsingClusterConfig() throws 
Exception {
+    Properties props = new Properties();
+    props.setProperty(SECURITY_MANAGER, SimpleSecurityManager.class.getName());
+    props.setProperty(SECURITY_POST_PROCESSOR, 
PDXPostProcessor.class.getName());
+
+    props.setProperty("use-cluster-configuration", "false");
+
+    // initial security properties should only contain initial set of values
+    InternalDistributedSystem ds = lsRule.getSystem(props);
+    assertEquals(2, ds.getSecurityProperties().size());
+
+    CacheFactory.create(ds);
+
+    // after cache is created, the configuration won't chagne
+    Properties secProps = ds.getSecurityProperties();
+    assertEquals(2, secProps.size());
+    assertEquals(SimpleSecurityManager.class.getName(), 
secProps.getProperty("security-manager"));
+    assertEquals(PDXPostProcessor.class.getName(), 
secProps.getProperty("security-post-processor"));
+  }
+
+
+  @Test
+  // when locator is not secured, server should not be secured if 
use-cluster-config is true
+  public void 
serverShouldNotBeAllowedToStartWithSecurityIfUsingClusterConfig() {
+    Properties props = new Properties();
+
+    props.setProperty("security-manager", "mySecurityManager");
+    props.setProperty("use-cluster-configuration", "true");
+
+    InternalDistributedSystem ds = lsRule.getSystem(props);
+
+    assertThatThrownBy(() -> 
CacheFactory.create(ds)).isInstanceOf(GemFireConfigException.class)
+                                                     
.hasMessage(LocalizedStrings.GEMFIRE_CACHE_SECURITY_MISCONFIGURATION
+                                                       .toLocalizedString());
+
+  }
+
+}

http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/e15657e9/geode-core/src/test/java/org/apache/geode/security/SecurityClusterConfigDUnitTest.java
----------------------------------------------------------------------
diff --git 
a/geode-core/src/test/java/org/apache/geode/security/SecurityClusterConfigDUnitTest.java
 
b/geode-core/src/test/java/org/apache/geode/security/SecurityClusterConfigDUnitTest.java
index ff7f028..54c02f7 100644
--- 
a/geode-core/src/test/java/org/apache/geode/security/SecurityClusterConfigDUnitTest.java
+++ 
b/geode-core/src/test/java/org/apache/geode/security/SecurityClusterConfigDUnitTest.java
@@ -21,83 +21,53 @@ import static 
org.apache.geode.distributed.ConfigurationProperties.*;
 import static org.assertj.core.api.Java6Assertions.*;
 import static org.junit.Assert.*;
 
-import java.io.File;
 import java.util.Properties;
 
-import org.junit.After;
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.experimental.categories.Category;
-import org.junit.rules.TemporaryFolder;
 
 import org.apache.geode.GemFireConfigException;
 import org.apache.geode.cache.CacheFactory;
-import org.apache.geode.distributed.Locator;
 import org.apache.geode.distributed.internal.InternalDistributedSystem;
 import org.apache.geode.internal.i18n.LocalizedStrings;
-import org.apache.geode.security.templates.SampleSecurityManager;
-import org.apache.geode.test.dunit.Host;
+import org.apache.geode.security.templates.SimpleSecurityManager;
 import org.apache.geode.test.dunit.IgnoredException;
-import org.apache.geode.test.dunit.VM;
 import org.apache.geode.test.dunit.internal.JUnit4DistributedTestCase;
+import org.apache.geode.test.dunit.rules.LocatorServerConfigurationRule;
 import org.apache.geode.test.junit.categories.DistributedTest;
-import org.apache.geode.test.junit.categories.FlakyTest;
 import org.apache.geode.test.junit.categories.SecurityTest;
 
 @Category({ DistributedTest.class, SecurityTest.class })
-
 public class SecurityClusterConfigDUnitTest extends JUnit4DistributedTestCase {
 
-  private int locatorPort = 0;
-
   @Rule
-  public transient TemporaryFolder temporaryFolder = new TemporaryFolder();
+  public LocatorServerConfigurationRule lsRule = new 
LocatorServerConfigurationRule(this);
 
   @Before
   public void before() throws Exception {
     
IgnoredException.addIgnoredException(LocalizedStrings.GEMFIRE_CACHE_SECURITY_MISCONFIGURATION.toString());
     
IgnoredException.addIgnoredException(LocalizedStrings.GEMFIRE_CACHE_SECURITY_MISCONFIGURATION_2.toString());
-    File locatorFile = temporaryFolder.newFile("locator.log");
-    final Host host = Host.getHost(0);
-    VM locator = host.getVM(0);
-    // set up locator with security
-    this.locatorPort = locator.invoke(() -> {
-      Properties props = new Properties();
-      props.setProperty(SampleSecurityManager.SECURITY_JSON, 
"org/apache/geode/management/internal/security/cacheServer.json");
-      props.setProperty(SECURITY_MANAGER, 
SampleSecurityManager.class.getName());
-      props.setProperty(MCAST_PORT, "0");
-      props.put(JMX_MANAGER, "false");
-      props.put(JMX_MANAGER_START, "false");
-      props.put(JMX_MANAGER_PORT, 0);
-      props.setProperty(SECURITY_POST_PROCESSOR, 
PDXPostProcessor.class.getName());
-      Locator lc = Locator.startLocatorAndDS(locatorPort, locatorFile, props);
-      return lc.getPort();
-    });
-  }
-
-  @After
-  public void after() {
-    IgnoredException.removeAllExpectedExceptions();
+    Properties props = new Properties();
+    props.setProperty(SECURITY_MANAGER, SimpleSecurityManager.class.getName());
+    props.put(JMX_MANAGER, "false");
+    props.put(JMX_MANAGER_START, "false");
+    props.put(JMX_MANAGER_PORT, 0);
+    props.setProperty(SECURITY_POST_PROCESSOR, 
PDXPostProcessor.class.getName());
+    lsRule.getLocatorVM(props);
   }
 
-  @Category(FlakyTest.class) // GEODE-1977
   @Test
   public void testStartServerWithClusterConfig() throws Exception {
-    // set up server with security
-    String locators = "localhost[" + locatorPort + "]";
-
     Properties props = new Properties();
-    props.setProperty(MCAST_PORT, "0");
-    props.setProperty(LOCATORS, locators);
-
     // the following are needed for peer-to-peer authentication
-    props.setProperty("security-username", "cluster-manager");
-    props.setProperty("security-password", "1234567");
+    props.setProperty("security-username", "cluster");
+    props.setProperty("security-password", "cluster");
     props.setProperty("use-cluster-configuration", "true");
 
     // initial security properties should only contain initial set of values
-    InternalDistributedSystem ds = getSystem(props);
+    InternalDistributedSystem ds = lsRule.getSystem(props);
     assertEquals(2, ds.getSecurityProperties().size());
 
     CacheFactory.create(ds);
@@ -111,22 +81,16 @@ public class SecurityClusterConfigDUnitTest extends 
JUnit4DistributedTestCase {
 
   @Test
   public void testStartServerWithSameSecurityManager() throws Exception {
-    // set up server with security
-    String locators = "localhost[" + locatorPort + "]";
-
     Properties props = new Properties();
-    props.setProperty(MCAST_PORT, "0");
-    props.setProperty(LOCATORS, locators);
 
     // the following are needed for peer-to-peer authentication
-    props.setProperty("security-username", "cluster-manager");
-    props.setProperty("security-password", "1234567");
+    props.setProperty("security-username", "cluster");
+    props.setProperty("security-password", "cluster");
     props.setProperty("use-cluster-configuration", "true");
-    props.setProperty(SampleSecurityManager.SECURITY_JSON, 
"org/apache/geode/management/internal/security/cacheServer.json");
-    props.setProperty(SECURITY_MANAGER, SampleSecurityManager.class.getName());
+    props.setProperty(SECURITY_MANAGER, SimpleSecurityManager.class.getName());
 
     // initial security properties should only contain initial set of values
-    InternalDistributedSystem ds = getSystem(props);
+    InternalDistributedSystem ds = lsRule.getSystem(props);
 
     CacheFactory.create(ds);
 
@@ -136,24 +100,18 @@ public class SecurityClusterConfigDUnitTest extends 
JUnit4DistributedTestCase {
     assertTrue(secProps.containsKey("security-post-processor"));
   }
 
-  @Category(FlakyTest.class) // GEODE-1975
   @Test
   public void serverWithDifferentSecurityManagerShouldThrowException() {
-    // set up server with security
-    String locators = "localhost[" + locatorPort + "]";
-
     Properties props = new Properties();
-    props.setProperty(MCAST_PORT, "0");
-    props.setProperty(LOCATORS, locators);
 
     // the following are needed for peer-to-peer authentication
-    props.setProperty("security-username", "cluster-manager");
-    props.setProperty("security-password", "1234567");
+    props.setProperty("security-username", "cluster");
+    props.setProperty("security-password", "cluster");
     props.setProperty("security-manager", "mySecurityManager");
     props.setProperty("use-cluster-configuration", "true");
 
     // initial security properties should only contain initial set of values
-    InternalDistributedSystem ds = getSystem(props);
+    InternalDistributedSystem ds = lsRule.getSystem(props);
 
     assertThatThrownBy(() -> 
CacheFactory.create(ds)).isInstanceOf(GemFireConfigException.class)
                                                      
.hasMessage(LocalizedStrings.GEMFIRE_CACHE_SECURITY_MISCONFIGURATION
@@ -163,22 +121,16 @@ public class SecurityClusterConfigDUnitTest extends 
JUnit4DistributedTestCase {
 
   @Test
   public void serverWithDifferentPostProcessorShouldThrowException() {
-    // set up server with security
-    String locators = "localhost[" + locatorPort + "]";
-
     Properties props = new Properties();
-    props.setProperty(MCAST_PORT, "0");
-    props.setProperty(LOCATORS, locators);
 
     // the following are needed for peer-to-peer authentication
-    props.setProperty("security-username", "cluster-manager");
-    props.setProperty("security-password", "1234567");
+    props.setProperty("security-username", "cluster");
+    props.setProperty("security-password", "cluster");
     props.setProperty(SECURITY_POST_PROCESSOR, "this-is-not-ok");
     props.setProperty("use-cluster-configuration", "true");
 
-
     // initial security properties should only contain initial set of values
-    InternalDistributedSystem ds = getSystem(props);
+    InternalDistributedSystem ds = lsRule.getSystem(props);
 
     assertThatThrownBy(() -> 
CacheFactory.create(ds)).isInstanceOf(GemFireConfigException.class)
                                                      
.hasMessage(LocalizedStrings.GEMFIRE_CACHE_SECURITY_MISCONFIGURATION
@@ -186,24 +138,17 @@ public class SecurityClusterConfigDUnitTest extends 
JUnit4DistributedTestCase {
 
   }
 
-  @Category(FlakyTest.class) // GEODE-1974
   @Test
   public void serverConnectingToSecuredLocatorMustUseClusterConfig() {
-    // set up server with security
-    String locators = "localhost[" + locatorPort + "]";
-
     Properties props = new Properties();
-    props.setProperty(MCAST_PORT, "0");
-    props.setProperty(LOCATORS, locators);
 
     // the following are needed for peer-to-peer authentication
-    props.setProperty("security-username", "cluster-manager");
-    props.setProperty("security-password", "1234567");
+    props.setProperty("security-username", "cluster");
+    props.setProperty("security-password", "cluster");
     props.setProperty("security-manager", "mySecurityManager");
     props.setProperty("use-cluster-configuration", "false");
 
-    // initial security properties should only contain initial set of values
-    InternalDistributedSystem ds = getSystem(props);
+    InternalDistributedSystem ds = lsRule.getSystem(props);
 
     assertThatThrownBy(() -> 
CacheFactory.create(ds)).isInstanceOf(GemFireConfigException.class)
                                                      
.hasMessage(LocalizedStrings.GEMFIRE_CACHE_SECURITY_MISCONFIGURATION_2

http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/e15657e9/geode-core/src/test/java/org/apache/geode/security/SecurityWithoutClusterConfigDUnitTest.java
----------------------------------------------------------------------
diff --git 
a/geode-core/src/test/java/org/apache/geode/security/SecurityWithoutClusterConfigDUnitTest.java
 
b/geode-core/src/test/java/org/apache/geode/security/SecurityWithoutClusterConfigDUnitTest.java
index 877f057..d5f8686 100644
--- 
a/geode-core/src/test/java/org/apache/geode/security/SecurityWithoutClusterConfigDUnitTest.java
+++ 
b/geode-core/src/test/java/org/apache/geode/security/SecurityWithoutClusterConfigDUnitTest.java
@@ -18,29 +18,23 @@
 package org.apache.geode.security;
 
 import static org.apache.geode.distributed.ConfigurationProperties.*;
-import static org.assertj.core.api.Java6Assertions.*;
 import static org.junit.Assert.*;
 
-import java.io.File;
 import java.util.Properties;
 
-import org.junit.After;
 import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.experimental.categories.Category;
-import org.junit.rules.TemporaryFolder;
 
-import org.apache.geode.GemFireConfigException;
 import org.apache.geode.cache.CacheFactory;
-import org.apache.geode.distributed.Locator;
 import org.apache.geode.distributed.internal.InternalDistributedSystem;
 import org.apache.geode.internal.i18n.LocalizedStrings;
 import org.apache.geode.security.templates.SampleSecurityManager;
-import org.apache.geode.test.dunit.Host;
+import org.apache.geode.security.templates.SimpleSecurityManager;
 import org.apache.geode.test.dunit.IgnoredException;
-import org.apache.geode.test.dunit.VM;
 import org.apache.geode.test.dunit.internal.JUnit4DistributedTestCase;
+import org.apache.geode.test.dunit.rules.LocatorServerConfigurationRule;
 import org.apache.geode.test.junit.categories.DistributedTest;
 import org.apache.geode.test.junit.categories.SecurityTest;
 
@@ -48,90 +42,42 @@ import org.apache.geode.test.junit.categories.SecurityTest;
 
 public class SecurityWithoutClusterConfigDUnitTest extends 
JUnit4DistributedTestCase {
 
-  private int locatorPort = 0;
-
   @Rule
-  public transient TemporaryFolder temporaryFolder = new TemporaryFolder();
+  public LocatorServerConfigurationRule lsRule = new 
LocatorServerConfigurationRule(this);
 
   @Before
   public void before() throws Exception {
     
IgnoredException.addIgnoredException(LocalizedStrings.GEMFIRE_CACHE_SECURITY_MISCONFIGURATION.toString());
     
IgnoredException.addIgnoredException(LocalizedStrings.GEMFIRE_CACHE_SECURITY_MISCONFIGURATION_2.toString());
-    File locatorFile = temporaryFolder.newFile("locator.log");
-    final Host host = Host.getHost(0);
-    VM locator = host.getVM(0);
-    // set up locator with security
-    this.locatorPort = locator.invoke(() -> {
-      Properties props = new Properties();
-      props.setProperty(MCAST_PORT, "0");
-      props.put(JMX_MANAGER, "false");
-      props.put(JMX_MANAGER_START, "false");
-      props.put(JMX_MANAGER_PORT, 0);
-      ;
-      Locator lc = Locator.startLocatorAndDS(locatorPort, locatorFile, props);
-      return lc.getPort();
-    });
-  }
-
-  @After
-  public void after() {
-    IgnoredException.removeAllExpectedExceptions();
+    Properties props = new Properties();
+    props.setProperty(SECURITY_MANAGER, SimpleSecurityManager.class.getName());
+    props.setProperty(SECURITY_POST_PROCESSOR, 
PDXPostProcessor.class.getName());
+    props.setProperty(ENABLE_CLUSTER_CONFIGURATION, "false");
+    lsRule.getLocatorVM(props);
   }
 
   @Test
-  public void 
serverShouldBeAllowedToStartWithSecurityIfNotUsingClusterConfig() throws 
Exception {
-    // set up server with security
-    String locators = "localhost[" + locatorPort + "]";
+  // if a secured locator is started without cluster config service, the 
server is free to use any security manager
+  // or no security manager at all. Currently this is valid, but not 
recommended.
+  public void testStartServerWithClusterConfig() throws Exception {
 
     Properties props = new Properties();
-    props.setProperty(MCAST_PORT, "0");
-    props.setProperty(LOCATORS, locators);
-
     // the following are needed for peer-to-peer authentication
-    props.setProperty("security-username", "cluster-manager");
-    props.setProperty("security-password", "1234567");
-    props.setProperty(SampleSecurityManager.SECURITY_JSON, 
"org/apache/geode/management/internal/security/cacheServer.json");
-    props.setProperty(SECURITY_MANAGER, SampleSecurityManager.class.getName());
-    props.setProperty(SECURITY_POST_PROCESSOR, 
PDXPostProcessor.class.getName());
-
-    props.setProperty("use-cluster-configuration", "false");
+    props.setProperty("security-username", "cluster");
+    props.setProperty("security-password", "cluster");
+    props.setProperty("security-manager", 
SampleSecurityManager.class.getName());
+    props.setProperty("use-cluster-configuration", "true");
 
     // initial security properties should only contain initial set of values
-    InternalDistributedSystem ds = getSystem(props);
-    assertEquals(5, ds.getSecurityProperties().size());
+    InternalDistributedSystem ds = lsRule.getSystem(props);
+    assertEquals(3, ds.getSecurityProperties().size());
 
     CacheFactory.create(ds);
 
     // after cache is created, we got the security props passed in by cluster 
config
     Properties secProps = ds.getSecurityProperties();
-    assertEquals(5, secProps.size());
+    assertEquals(3, secProps.size());
     assertEquals(SampleSecurityManager.class.getName(), 
secProps.getProperty("security-manager"));
-    assertEquals(PDXPostProcessor.class.getName(), 
secProps.getProperty("security-post-processor"));
+    assertFalse(secProps.containsKey("security-post-processor"));
   }
-
-
-  @Test
-  public void 
serverShouldNotBeAllowedToStartWithSecurityIfUsingClusterConfig() {
-    // set up server with security
-    String locators = "localhost[" + locatorPort + "]";
-
-    Properties props = new Properties();
-    props.setProperty(MCAST_PORT, "0");
-    props.setProperty(LOCATORS, locators);
-
-    // the following are needed for peer-to-peer authentication
-    props.setProperty("security-username", "cluster-manager");
-    props.setProperty("security-password", "1234567");
-    props.setProperty("security-manager", "mySecurityManager");
-    props.setProperty("use-cluster-configuration", "true");
-
-    // initial security properties should only contain initial set of values
-    InternalDistributedSystem ds = getSystem(props);
-
-    assertThatThrownBy(() -> 
CacheFactory.create(ds)).isInstanceOf(GemFireConfigException.class)
-                                                     
.hasMessage(LocalizedStrings.GEMFIRE_CACHE_SECURITY_MISCONFIGURATION
-                                                       .toLocalizedString());
-
-  }
-
 }

http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/e15657e9/geode-core/src/test/java/org/apache/geode/security/StartServerAuthorizationTest.java
----------------------------------------------------------------------
diff --git 
a/geode-core/src/test/java/org/apache/geode/security/StartServerAuthorizationTest.java
 
b/geode-core/src/test/java/org/apache/geode/security/StartServerAuthorizationTest.java
index dae062a..953cdb7 100644
--- 
a/geode-core/src/test/java/org/apache/geode/security/StartServerAuthorizationTest.java
+++ 
b/geode-core/src/test/java/org/apache/geode/security/StartServerAuthorizationTest.java
@@ -20,92 +20,75 @@ package org.apache.geode.security;
 import static org.apache.geode.distributed.ConfigurationProperties.*;
 import static org.assertj.core.api.Assertions.*;
 
-import java.io.File;
 import java.util.Properties;
 
 import org.junit.Before;
+import org.junit.Rule;
 import org.junit.Test;
 import org.junit.experimental.categories.Category;
 
-import org.apache.geode.distributed.Locator;
-import org.apache.geode.security.templates.SampleSecurityManager;
-import org.apache.geode.test.dunit.Host;
+import org.apache.geode.security.templates.SimpleSecurityManager;
 import org.apache.geode.test.dunit.VM;
 import org.apache.geode.test.dunit.internal.JUnit4DistributedTestCase;
+import org.apache.geode.test.dunit.rules.LocatorServerConfigurationRule;
 import org.apache.geode.test.junit.categories.DistributedTest;
 import org.apache.geode.test.junit.categories.SecurityTest;
 
 @Category({ DistributedTest.class, SecurityTest.class })
 public class StartServerAuthorizationTest extends JUnit4DistributedTestCase {
-  private int locatorPort = 0;
+
+  @Rule
+  public LocatorServerConfigurationRule lsRule = new 
LocatorServerConfigurationRule(this);
 
   @Before
   public void before() throws Exception {
-    final Host host = Host.getHost(0);
-    VM locator = host.getVM(0);
-    // set up locator with security
-    this.locatorPort = locator.invoke(()->{
-      Properties props = new Properties();
-      props.setProperty(SampleSecurityManager.SECURITY_JSON, 
"org/apache/geode/management/internal/security/cacheServer.json");
-      props.setProperty(SECURITY_MANAGER, 
SampleSecurityManager.class.getName());
-      props.setProperty(MCAST_PORT, "0");
-      props.put(JMX_MANAGER, "true");
-      props.put(JMX_MANAGER_START, "true");
-      props.put(JMX_MANAGER_PORT, 0);
-      props.setProperty(SECURITY_POST_PROCESSOR, 
PDXPostProcessor.class.getName());
-      Locator lc = Locator.startLocatorAndDS(locatorPort, new 
File("locator.log"), props);
-      return lc.getPort();
-    });
+    Properties props = new Properties();
+    props.setProperty(SECURITY_MANAGER, SimpleSecurityManager.class.getName());
+    props.put(JMX_MANAGER, "true");
+    props.put(JMX_MANAGER_START, "true");
+    props.put(JMX_MANAGER_PORT, 0);
+    props.setProperty(SECURITY_POST_PROCESSOR, 
PDXPostProcessor.class.getName());
+    lsRule.getLocatorVM(props);
   }
 
   @Test
   public void testStartServerWithInvalidCredential() throws Exception{
-    // set up server with security
-    String locators = "localhost[" + locatorPort + "]";
-
     Properties props = new Properties();
-    props.setProperty(MCAST_PORT, "0");
-    props.setProperty(LOCATORS, locators);
-
     // the following are needed for peer-to-peer authentication
-    props.setProperty("security-username", "stranger");
+    props.setProperty("security-username", "user");
     props.setProperty("security-password", "wrongPswd");
 
-    
assertThatThrownBy(()->getSystem(props)).isInstanceOf(GemFireSecurityException.class).hasMessageContaining("Authentication
 error. Please check your credentials.");
+    VM server = lsRule.getNodeVM(1);
+    server.invoke(()->{
+      
assertThatThrownBy(()->lsRule.getSystem(props)).isInstanceOf(GemFireSecurityException.class).hasMessageContaining("Security
 check failed. Authentication error. Please check your credentials");
+    });
   }
 
   @Test
   public void testStartServerWithInsufficientPrevilage() throws Exception{
-    // set up server with security
-    String locators = "localhost[" + locatorPort + "]";
-
     Properties props = new Properties();
-    props.setProperty(MCAST_PORT, "0");
-    props.setProperty(LOCATORS, locators);
 
     // the following are needed for peer-to-peer authentication
-    props.setProperty("security-username", "stranger");
-    props.setProperty("security-password", "1234567");
+    props.setProperty("security-username", "user");
+    props.setProperty("security-password", "user");
+
+    VM server = lsRule.getNodeVM(1);
+    server.invoke(()->{
+      
assertThatThrownBy(()->lsRule.getSystem(props)).isInstanceOf(GemFireSecurityException.class).hasMessageContaining("user
 not authorized for CLUSTER:MANAGE");
+    });
 
 
-    
assertThatThrownBy(()->getSystem(props)).isInstanceOf(GemFireSecurityException.class).hasMessageContaining("stranger
 not authorized for CLUSTER:MANAGE");
   }
 
   @Test
   public void testStartServerWithSufficientPrevilage() throws Exception{
-    // set up server with security
-    String locators = "localhost[" + locatorPort + "]";
-
     Properties props = new Properties();
-    props.setProperty(MCAST_PORT, "0");
-    props.setProperty(LOCATORS, locators);
 
     // the following are needed for peer-to-peer authentication
-    props.setProperty("security-username", "cluster-manager");
-    props.setProperty("security-password", "1234567");
+    props.setProperty("security-username", "cluster");
+    props.setProperty("security-password", "cluster");
 
-    // No exception should be thrown
-    getSystem(props);
+    lsRule.getServerVM(1, props);
   }
 
 }

http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/e15657e9/geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorServerConfigurationRule.java
----------------------------------------------------------------------
diff --git 
a/geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorServerConfigurationRule.java
 
b/geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorServerConfigurationRule.java
index 2fa262c..7f52ce1 100644
--- 
a/geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorServerConfigurationRule.java
+++ 
b/geode-core/src/test/java/org/apache/geode/test/dunit/rules/LocatorServerConfigurationRule.java
@@ -18,7 +18,6 @@
 package org.apache.geode.test.dunit.rules;
 
 import static org.apache.geode.distributed.ConfigurationProperties.*;
-import static org.apache.geode.internal.AvailablePortHelper.*;
 import static org.apache.geode.test.dunit.Host.*;
 import static org.apache.geode.test.dunit.internal.JUnit4DistributedTestCase.*;
 import static org.junit.Assert.*;
@@ -34,10 +33,12 @@ import com.jayway.awaitility.Awaitility;
 import org.junit.rules.ExternalResource;
 
 import org.apache.geode.distributed.Locator;
+import org.apache.geode.distributed.internal.InternalDistributedSystem;
 import org.apache.geode.distributed.internal.InternalLocator;
 import org.apache.geode.test.dunit.Host;
 import org.apache.geode.test.dunit.VM;
 import org.apache.geode.test.dunit.cache.internal.JUnit4CacheTestCase;
+import org.apache.geode.test.dunit.internal.JUnit4DistributedTestCase;
 
 
 public class LocatorServerConfigurationRule extends ExternalResource 
implements Serializable {
@@ -46,9 +47,9 @@ public class LocatorServerConfigurationRule extends 
ExternalResource implements
 
   private boolean locatorInitialized = false;
 
-  private JUnit4CacheTestCase testCase;
+  private JUnit4DistributedTestCase testCase;
 
-  public LocatorServerConfigurationRule(JUnit4CacheTestCase testCase) {
+  public LocatorServerConfigurationRule(JUnit4DistributedTestCase testCase) {
     this.testCase = testCase;
   }
 
@@ -66,7 +67,6 @@ public class LocatorServerConfigurationRule extends 
ExternalResource implements
     disconnectAllFromDS();
   }
 
-
   /**
    * Returns getHost(0).getVM(0) as a locator instance with the given
    * configuration properties.
@@ -77,7 +77,20 @@ public class LocatorServerConfigurationRule extends 
ExternalResource implements
    * @throws IOException
    */
   public VM getLocatorVM(Properties locatorProperties) throws IOException {
-    initLocator(locatorProperties);
+    if (!locatorProperties.containsKey(MCAST_PORT)) {
+      locatorProperties.setProperty(MCAST_PORT, "0");
+    }
+
+    locatorPort = locator.invoke(() -> {
+      InternalLocator locator = (InternalLocator) Locator.startLocatorAndDS(0, 
null, locatorProperties);
+      locator.resetInternalLocatorFileNamesWithCorrectPortNumber(locatorPort);
+
+      if (locator.getConfig().getEnableClusterConfiguration()) {
+        Awaitility.await().atMost(65, TimeUnit.SECONDS).until(() -> 
assertTrue(locator.isSharedConfigurationRunning()));
+      }
+      return locator.getPort();
+    });
+
     this.locatorInitialized = true;
     return locator;
   }
@@ -85,49 +98,43 @@ public class LocatorServerConfigurationRule extends 
ExternalResource implements
   /**
    * Returns a node VM with given configuration properties.
    * @param index valid 1 to 3 (returns getHist(0).getVM(index)
-   * @param nodeProperties
+   * @param properties
    *
    * @return VM node vm
    */
-  public VM getNodeVM(int index, Properties nodeProperties) {
+  public VM getServerVM(int index, Properties properties) {
     assertTrue("Locator not initialized. Initialize locator by calling 
getLocatorVM()", this.locatorInitialized);
     assertTrue("VM with index 0 is used for locator service.", (index != 0));
-    VM nodeVM = host.getVM(index);
-    initNode(nodeVM, nodeProperties);
+    VM nodeVM = getNodeVM(index);
+    nodeVM.invoke(() -> {
+      getSystem(properties);
+    });
     return nodeVM;
   }
 
-  private void initLocator(Properties locatorProperties) throws IOException {
-    final int[] ports = getRandomAvailableTCPPorts(1);
-    locatorPort = ports[0];
-
-    if (!locatorProperties.containsKey(MCAST_PORT)) {
-      locatorProperties.setProperty(MCAST_PORT, "0");
-    }
-
-    locatorPort = locator.invoke(() -> {
-      InternalLocator locator = (InternalLocator) Locator.startLocatorAndDS(0, 
null, locatorProperties);
-      locatorPort = locator.getPort();
-      locator.resetInternalLocatorFileNamesWithCorrectPortNumber(locatorPort);
-
-      if (locatorProperties.containsKey(ENABLE_CLUSTER_CONFIGURATION)) {
-        Awaitility.await().atMost(65, TimeUnit.SECONDS).until(() -> 
assertTrue(locator.isSharedConfigurationRunning()));
-      }
-      return locatorPort;
-    });
+  /**
+   * this will simply returns the node
+   * @param index
+   * @return
+   */
+  public VM getNodeVM(int index){
+    return host.getVM(index);
   }
 
-  private void initNode(VM nodeVM, Properties props) {
-    if (!props.containsKey(MCAST_PORT)) {
-      props.setProperty(MCAST_PORT, "0");
+  public InternalDistributedSystem getSystem(Properties properties){
+    if (!properties.containsKey(MCAST_PORT)) {
+      properties.setProperty(MCAST_PORT, "0");
     }
+    properties.setProperty(LOCATORS, getHostName() + "[" + locatorPort + "]");
+    InternalDistributedSystem ds = testCase.getSystem(properties);
+    if(testCase instanceof JUnit4CacheTestCase){
+      ((JUnit4CacheTestCase)testCase).getCache();
+    }
+    return ds;
+  }
 
-    props.setProperty(LOCATORS, getHostName() + "[" + locatorPort + "]");
-
-    nodeVM.invoke(() -> {
-      testCase.getSystem(props);
-      assertNotNull(testCase.getCache());
-    });
+  public int getLocatorPort(){
+    return locatorPort;
   }
 
   private String getHostName() {

http://git-wip-us.apache.org/repos/asf/incubator-geode/blob/e15657e9/geode-lucene/src/test/java/org/apache/geode/cache/lucene/internal/configuration/LuceneClusterConfigurationDUnitTest.java
----------------------------------------------------------------------
diff --git 
a/geode-lucene/src/test/java/org/apache/geode/cache/lucene/internal/configuration/LuceneClusterConfigurationDUnitTest.java
 
b/geode-lucene/src/test/java/org/apache/geode/cache/lucene/internal/configuration/LuceneClusterConfigurationDUnitTest.java
index f837530..30532b9 100755
--- 
a/geode-lucene/src/test/java/org/apache/geode/cache/lucene/internal/configuration/LuceneClusterConfigurationDUnitTest.java
+++ 
b/geode-lucene/src/test/java/org/apache/geode/cache/lucene/internal/configuration/LuceneClusterConfigurationDUnitTest.java
@@ -16,24 +16,10 @@
  */
 package org.apache.geode.cache.lucene.internal.configuration;
 
-import static 
org.apache.geode.cache.lucene.test.LuceneTestUtilities.INDEX_NAME;
-import static 
org.apache.geode.cache.lucene.test.LuceneTestUtilities.REGION_NAME;
-import static 
org.apache.geode.distributed.ConfigurationProperties.CLUSTER_CONFIGURATION_DIR;
-import static 
org.apache.geode.distributed.ConfigurationProperties.DEPLOY_WORKING_DIR;
-import static 
org.apache.geode.distributed.ConfigurationProperties.ENABLE_CLUSTER_CONFIGURATION;
-import static org.apache.geode.distributed.ConfigurationProperties.GROUPS;
-import static 
org.apache.geode.distributed.ConfigurationProperties.HTTP_SERVICE_PORT;
-import static org.apache.geode.distributed.ConfigurationProperties.JMX_MANAGER;
-import static 
org.apache.geode.distributed.ConfigurationProperties.JMX_MANAGER_BIND_ADDRESS;
-import static 
org.apache.geode.distributed.ConfigurationProperties.JMX_MANAGER_PORT;
-import static 
org.apache.geode.distributed.ConfigurationProperties.JMX_MANAGER_START;
-import static 
org.apache.geode.distributed.ConfigurationProperties.USE_CLUSTER_CONFIGURATION;
-import static 
org.apache.geode.internal.AvailablePortHelper.getRandomAvailableTCPPorts;
-import static org.junit.Assert.assertArrayEquals;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertNull;
+import static org.apache.geode.cache.lucene.test.LuceneTestUtilities.*;
+import static org.apache.geode.distributed.ConfigurationProperties.*;
+import static org.apache.geode.internal.AvailablePortHelper.*;
+import static org.junit.Assert.*;
 
 import java.io.File;
 import java.net.InetAddress;
@@ -256,7 +242,7 @@ public class LuceneClusterConfigurationDUnitTest extends 
CliCommandTestBase {
     if (addGroup) {
       nodeProperties.setProperty(GROUPS, groupName);
     }
-    return ls.getNodeVM(vmIndex, nodeProperties);
+    return ls.getServerVM(vmIndex, nodeProperties);
   }
 
   private VM startLocatorWithClusterConfigurationEnabled() throws Exception {


Reply via email to