You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by ji...@apache.org on 2017/04/06 21:17:26 UTC

geode git commit: GEODE-2756: Do not put security-* properties in the env.

Repository: geode
Updated Branches:
  refs/heads/develop 5430c91f6 -> 19376d306


GEODE-2756: Do not put security-* properties in the env.


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

Branch: refs/heads/develop
Commit: 19376d3069a7808481b2ed572ea49e3610dc9df1
Parents: 5430c91
Author: Jinmei Liao <ji...@pivotal.io>
Authored: Thu Apr 6 09:50:57 2017 -0700
Committer: Jinmei Liao <ji...@pivotal.io>
Committed: Thu Apr 6 14:16:27 2017 -0700

----------------------------------------------------------------------
 .../support/LoginHandlerInterceptor.java        | 27 +++--------
 .../LoginHandlerInterceptorJUnitTest.java       | 51 +++++++++++++-------
 ...andlerInterceptorRequestHeaderJUnitTest.java | 28 ++++++++---
 3 files changed, 60 insertions(+), 46 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/geode/blob/19376d30/geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/support/LoginHandlerInterceptor.java
----------------------------------------------------------------------
diff --git a/geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/support/LoginHandlerInterceptor.java b/geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/support/LoginHandlerInterceptor.java
index 9e05174..56d9b9e 100644
--- a/geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/support/LoginHandlerInterceptor.java
+++ b/geode-core/src/main/java/org/apache/geode/management/internal/web/controllers/support/LoginHandlerInterceptor.java
@@ -22,7 +22,6 @@ import org.apache.geode.internal.security.SecurityService;
 import org.apache.geode.management.internal.cli.multistep.CLIMultiStepHelper;
 import org.apache.geode.management.internal.security.ResourceConstants;
 import org.apache.geode.management.internal.web.util.UriUtils;
-import org.apache.geode.security.Authenticator;
 import org.apache.logging.log4j.Logger;
 import org.springframework.web.servlet.handler.HandlerInterceptorAdapter;
 
@@ -51,8 +50,6 @@ public class LoginHandlerInterceptor extends HandlerInterceptorAdapter {
 
   private Cache cache;
 
-  private Authenticator auth = null;
-
   private SecurityService securityService = IntegratedSecurityService.getSecurityService();
 
   private static final ThreadLocal<Map<String, String>> ENV =
@@ -93,22 +90,10 @@ public class LoginHandlerInterceptor extends HandlerInterceptorAdapter {
       }
     }
 
+    ENV.set(requestParameterValues);
 
-
-    for (Enumeration<String> requestHeaders = request.getHeaderNames(); requestHeaders
-        .hasMoreElements();) {
-
-      // since http request headers are case-insensitive and all our security-* properties
-      // are in lower case, it's safe to do toLowerCase here.
-      final String requestHeader = requestHeaders.nextElement().toLowerCase();
-
-      if (requestHeader.startsWith(SECURITY_VARIABLE_REQUEST_HEADER_PREFIX)) {
-        requestParameterValues.put(requestHeader, request.getHeader(requestHeader));
-      }
-    }
-
-    String username = requestParameterValues.get(ResourceConstants.USER_NAME);
-    String password = requestParameterValues.get(ResourceConstants.PASSWORD);
+    String username = request.getHeader(ResourceConstants.USER_NAME);
+    String password = request.getHeader(ResourceConstants.PASSWORD);
     Properties credentials = new Properties();
     if (username != null)
       credentials.put(ResourceConstants.USER_NAME, username);
@@ -116,11 +101,13 @@ public class LoginHandlerInterceptor extends HandlerInterceptorAdapter {
       credentials.put(ResourceConstants.PASSWORD, password);
     this.securityService.login(credentials);
 
-    ENV.set(requestParameterValues);
-
     return true;
   }
 
+  public void setSecurityService(SecurityService securityService) {
+    this.securityService = securityService;
+  }
+
 
   @Override
   public void afterCompletion(final HttpServletRequest request, final HttpServletResponse response,

http://git-wip-us.apache.org/repos/asf/geode/blob/19376d30/geode-web/src/test/java/org/apache/geode/management/internal/web/controllers/support/LoginHandlerInterceptorJUnitTest.java
----------------------------------------------------------------------
diff --git a/geode-web/src/test/java/org/apache/geode/management/internal/web/controllers/support/LoginHandlerInterceptorJUnitTest.java b/geode-web/src/test/java/org/apache/geode/management/internal/web/controllers/support/LoginHandlerInterceptorJUnitTest.java
index 63d410f..80e26fd 100644
--- a/geode-web/src/test/java/org/apache/geode/management/internal/web/controllers/support/LoginHandlerInterceptorJUnitTest.java
+++ b/geode-web/src/test/java/org/apache/geode/management/internal/web/controllers/support/LoginHandlerInterceptorJUnitTest.java
@@ -14,16 +14,13 @@
  */
 package org.apache.geode.management.internal.web.controllers.support;
 
-import static org.junit.Assert.*;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNotSame;
+import static org.junit.Assert.assertTrue;
 
-import java.util.Enumeration;
-import java.util.HashMap;
-import java.util.Iterator;
-import java.util.Map;
-import javax.servlet.http.HttpServletRequest;
-
-import edu.umd.cs.mtc.MultithreadedTestCase;
-import edu.umd.cs.mtc.TestFramework;
+import org.apache.geode.management.internal.security.ResourceConstants;
+import org.apache.geode.test.junit.categories.UnitTest;
 import org.jmock.Expectations;
 import org.jmock.Mockery;
 import org.jmock.lib.concurrent.Synchroniser;
@@ -33,7 +30,13 @@ import org.junit.Before;
 import org.junit.Test;
 import org.junit.experimental.categories.Category;
 
-import org.apache.geode.test.junit.categories.UnitTest;
+import java.util.Enumeration;
+import java.util.HashMap;
+import java.util.Iterator;
+import java.util.Map;
+import javax.servlet.http.HttpServletRequest;
+import edu.umd.cs.mtc.MultithreadedTestCase;
+import edu.umd.cs.mtc.TestFramework;
 
 /**
  * The LoginHandlerInterceptorJUnitTest class is a test suite of test cases to test the contract and
@@ -83,7 +86,6 @@ public class LoginHandlerInterceptorJUnitTest {
   @Test
   public void testPreHandleAfterCompletion() throws Exception {
     final Map<String, String> requestParameters = new HashMap<>(2);
-    final Map<String, String> requestHeaders = new HashMap<>();
 
     requestParameters.put("parameter", "one");
     requestParameters.put(createEnvironmentVariable("variable"), "two");
@@ -95,8 +97,10 @@ public class LoginHandlerInterceptorJUnitTest {
       {
         oneOf(mockHttpRequest).getParameterNames();
         will(returnValue(enumeration(requestParameters.keySet().iterator())));
-        oneOf(mockHttpRequest).getHeaderNames();
-        will(returnValue(enumeration(requestHeaders.keySet().iterator())));
+        oneOf(mockHttpRequest).getHeader(ResourceConstants.USER_NAME);
+        will(returnValue("admin"));
+        oneOf(mockHttpRequest).getHeader(ResourceConstants.PASSWORD);
+        will(returnValue("password"));
         oneOf(mockHttpRequest).getParameter(with(equal(createEnvironmentVariable("variable"))));
         will(returnValue(requestParameters.get(createEnvironmentVariable("variable"))));
       }
@@ -144,7 +148,6 @@ public class LoginHandlerInterceptorJUnitTest {
       super.initialize();
 
       final Map<String, String> requestParametersOne = new HashMap<>(3);
-      final Map<String, String> requestHeaders = new HashMap<>();
 
       requestParametersOne.put("param", "one");
       requestParametersOne.put(createEnvironmentVariable("STAGE"), "test");
@@ -157,8 +160,10 @@ public class LoginHandlerInterceptorJUnitTest {
         {
           oneOf(mockHttpRequestOne).getParameterNames();
           will(returnValue(enumeration(requestParametersOne.keySet().iterator())));
-          oneOf(mockHttpRequestOne).getHeaderNames();
-          will(returnValue(enumeration(requestHeaders.keySet().iterator())));
+          oneOf(mockHttpRequestOne).getHeader(ResourceConstants.USER_NAME);
+          will(returnValue("admin"));
+          oneOf(mockHttpRequestOne).getHeader(ResourceConstants.PASSWORD);
+          will(returnValue("password"));
           oneOf(mockHttpRequestOne).getParameter(with(equal(createEnvironmentVariable("STAGE"))));
           will(returnValue(requestParametersOne.get(createEnvironmentVariable("STAGE"))));
           oneOf(mockHttpRequestOne)
@@ -180,8 +185,10 @@ public class LoginHandlerInterceptorJUnitTest {
         {
           oneOf(mockHttpRequestTwo).getParameterNames();
           will(returnValue(enumeration(requestParametersTwo.keySet().iterator())));
-          oneOf(mockHttpRequestTwo).getHeaderNames();
-          will(returnValue(enumeration(requestHeaders.keySet().iterator())));
+          oneOf(mockHttpRequestTwo).getHeader(ResourceConstants.USER_NAME);
+          will(returnValue("admin"));
+          oneOf(mockHttpRequestTwo).getHeader(ResourceConstants.PASSWORD);
+          will(returnValue("password"));
           oneOf(mockHttpRequestTwo).getParameter(with(equal(createEnvironmentVariable("HOST"))));
           will(returnValue(requestParametersTwo.get(createEnvironmentVariable("HOST"))));
           oneOf(mockHttpRequestTwo)
@@ -210,6 +217,8 @@ public class LoginHandlerInterceptorJUnitTest {
       assertFalse(env.containsKey("param"));
       assertFalse(env.containsKey("parameter"));
       assertFalse(env.containsKey("HOST"));
+      assertFalse(env.containsKey("security-username"));
+      assertFalse(env.containsKey("security-password"));
       assertEquals("test", env.get("STAGE"));
       assertEquals("/path/to/gemfire/700", env.get("GEODE_HOME"));
 
@@ -222,6 +231,8 @@ public class LoginHandlerInterceptorJUnitTest {
       assertFalse(env.containsKey("param"));
       assertFalse(env.containsKey("parameter"));
       assertFalse(env.containsKey("HOST"));
+      assertFalse(env.containsKey("security-username"));
+      assertFalse(env.containsKey("security-password"));
       assertEquals("test", env.get("STAGE"));
       assertEquals("/path/to/gemfire/700", env.get("GEODE_HOME"));
 
@@ -234,6 +245,8 @@ public class LoginHandlerInterceptorJUnitTest {
       assertFalse(env.containsKey("param"));
       assertFalse(env.containsKey("parameter"));
       assertFalse(env.containsKey("HOST"));
+      assertFalse(env.containsKey("security-username"));
+      assertFalse(env.containsKey("security-password"));
       assertEquals("test", env.get("STAGE"));
       assertEquals("/path/to/gemfire/700", env.get("GEODE_HOME"));
 
@@ -263,6 +276,8 @@ public class LoginHandlerInterceptorJUnitTest {
       assertFalse(env.containsKey("parameter"));
       assertFalse(env.containsKey("param"));
       assertFalse(env.containsKey("STAGE"));
+      assertFalse(env.containsKey("security-username"));
+      assertFalse(env.containsKey("security-password"));
       assertEquals("localhost", env.get("HOST"));
       assertEquals("/path/to/gemfire/75", env.get("GEODE_HOME"));
 

http://git-wip-us.apache.org/repos/asf/geode/blob/19376d30/geode-web/src/test/java/org/apache/geode/management/internal/web/controllers/support/LoginHandlerInterceptorRequestHeaderJUnitTest.java
----------------------------------------------------------------------
diff --git a/geode-web/src/test/java/org/apache/geode/management/internal/web/controllers/support/LoginHandlerInterceptorRequestHeaderJUnitTest.java b/geode-web/src/test/java/org/apache/geode/management/internal/web/controllers/support/LoginHandlerInterceptorRequestHeaderJUnitTest.java
index 118e385..00156cd 100644
--- a/geode-web/src/test/java/org/apache/geode/management/internal/web/controllers/support/LoginHandlerInterceptorRequestHeaderJUnitTest.java
+++ b/geode-web/src/test/java/org/apache/geode/management/internal/web/controllers/support/LoginHandlerInterceptorRequestHeaderJUnitTest.java
@@ -16,23 +16,33 @@ package org.apache.geode.management.internal.web.controllers.support;
 
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.Mockito.verify;
 
+import org.apache.geode.internal.security.SecurityService;
 import org.apache.geode.test.junit.categories.UnitTest;
-
 import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
 import org.junit.experimental.categories.Category;
+import org.mockito.ArgumentCaptor;
+import org.mockito.Mockito;
 import org.springframework.mock.web.MockHttpServletRequest;
 
 import java.util.Map;
+import java.util.Properties;
 
 @Category(UnitTest.class)
 public class LoginHandlerInterceptorRequestHeaderJUnitTest {
 
+  SecurityService securityService;
+  LoginHandlerInterceptor interceptor;
+
   @Before
   public void before() {
     LoginHandlerInterceptor.getEnvironment().clear();
+    securityService = Mockito.mock(SecurityService.class);
+    interceptor = new LoginHandlerInterceptor();
+    interceptor.setSecurityService(securityService);
   }
 
   @After
@@ -42,21 +52,23 @@ public class LoginHandlerInterceptorRequestHeaderJUnitTest {
 
   @Test
   public void testCaseInsensitive() throws Exception {
-    LoginHandlerInterceptor interceptor = new LoginHandlerInterceptor();
     MockHttpServletRequest mockRequest = new MockHttpServletRequest();
     mockRequest.addHeader("Security-Username", "John");
     mockRequest.addHeader("Security-Password", "Password");
     mockRequest.addHeader("security-something", "anything");
     mockRequest.addHeader("Content-Type", "application/json");
 
+
     interceptor.preHandle(mockRequest, null, null);
-    Map<String, String> env = interceptor.getEnvironment();
 
-    // make sure only security-* are put in the environment variable
-    assertThat(env).hasSize(3);
-    assertThat(env.get("security-username")).isEqualTo("John");
-    assertThat(env.get("security-password")).isEqualTo("Password");
-    assertThat(env.get("security-something")).isEqualTo("anything");
+    ArgumentCaptor<Properties> props = ArgumentCaptor.forClass(Properties.class);
+    verify(securityService).login(props.capture());
+    assertThat(props.getValue().getProperty("security-username")).isEqualTo("John");
+    assertThat(props.getValue().getProperty("security-password")).isEqualTo("Password");
+
+    Map<String, String> env = interceptor.getEnvironment();
+    // make sure security-* are not put in the environment variable
+    assertThat(env).hasSize(0);
   }
 
 }