Repository: geode
Updated Branches:
  refs/heads/feature/GEODE-2456 4c82d156e -> 7d23a89d7 (forced update)


GEODE-2247: GFSH connect over HTTP without credentials should fail earlier


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

Branch: refs/heads/feature/GEODE-2456
Commit: 30341ec154b0d073dbca497cae110508f9480be1
Parents: 253a490
Author: Kevin J. Duling <kdul...@pivotal.io>
Authored: Wed Feb 8 10:58:23 2017 -0800
Committer: Kevin J. Duling <kdul...@pivotal.io>
Committed: Thu Feb 9 12:47:53 2017 -0800

----------------------------------------------------------------------
 .../security/IntegratedSecurityService.java     | 15 --------
 .../internal/security/SecurityService.java      |  2 --
 .../support/LoginHandlerInterceptor.java        | 28 ++++++++-------
 .../auth/AbstractGMSAuthenticatorTestCase.java  | 15 ++++----
 ...atedSecurityServiceWithIniFileJUnitTest.java | 36 ++++++++++++--------
 .../security/GeodeAuthenticationProvider.java   | 13 +++++--
 6 files changed, 53 insertions(+), 56 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/geode/blob/30341ec1/geode-core/src/main/java/org/apache/geode/internal/security/IntegratedSecurityService.java
----------------------------------------------------------------------
diff --git 
a/geode-core/src/main/java/org/apache/geode/internal/security/IntegratedSecurityService.java
 
b/geode-core/src/main/java/org/apache/geode/internal/security/IntegratedSecurityService.java
index 6507295..8366930 100644
--- 
a/geode-core/src/main/java/org/apache/geode/internal/security/IntegratedSecurityService.java
+++ 
b/geode-core/src/main/java/org/apache/geode/internal/security/IntegratedSecurityService.java
@@ -25,12 +25,10 @@ import org.apache.commons.lang.StringUtils;
 import org.apache.geode.GemFireIOException;
 import org.apache.geode.internal.cache.EntryEventImpl;
 import org.apache.geode.internal.logging.LogService;
-import org.apache.geode.internal.logging.log4j.LogMarker;
 import org.apache.geode.internal.security.shiro.CustomAuthRealm;
 import org.apache.geode.internal.security.shiro.GeodeAuthenticationToken;
 import org.apache.geode.internal.security.shiro.ShiroPrincipal;
 import org.apache.geode.internal.util.BlobHelper;
-import org.apache.geode.management.internal.security.ResourceConstants;
 import org.apache.geode.management.internal.security.ResourceOperation;
 import org.apache.geode.security.AuthenticationFailedException;
 import org.apache.geode.security.GemFireSecurityException;
@@ -121,19 +119,6 @@ public class IntegratedSecurityService implements 
SecurityService {
   }
 
   /**
-   * convenient method for testing
-   */
-  public Subject login(String username, String password) {
-    if (StringUtils.isBlank(username) || StringUtils.isBlank(password))
-      return null;
-
-    Properties credentials = new Properties();
-    credentials.setProperty(ResourceConstants.USER_NAME, username);
-    credentials.setProperty(ResourceConstants.PASSWORD, password);
-    return login(credentials);
-  }
-
-  /**
    * @return null if security is not enabled, otherwise return a shiro subject
    */
   public Subject login(Properties credentials) {

http://git-wip-us.apache.org/repos/asf/geode/blob/30341ec1/geode-core/src/main/java/org/apache/geode/internal/security/SecurityService.java
----------------------------------------------------------------------
diff --git 
a/geode-core/src/main/java/org/apache/geode/internal/security/SecurityService.java
 
b/geode-core/src/main/java/org/apache/geode/internal/security/SecurityService.java
index 727a1ce..14784c3 100644
--- 
a/geode-core/src/main/java/org/apache/geode/internal/security/SecurityService.java
+++ 
b/geode-core/src/main/java/org/apache/geode/internal/security/SecurityService.java
@@ -36,8 +36,6 @@ public interface SecurityService {
 
   Subject login(Properties credentials);
 
-  Subject login(String username, String password);
-
   void logout();
 
   Callable associateWith(Callable callable);

http://git-wip-us.apache.org/repos/asf/geode/blob/30341ec1/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 79c8c27..9e05174 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
@@ -14,17 +14,6 @@
  */
 package org.apache.geode.management.internal.web.controllers.support;
 
-import java.util.Collections;
-import java.util.Enumeration;
-import java.util.HashMap;
-import java.util.Map;
-
-import javax.servlet.http.HttpServletRequest;
-import javax.servlet.http.HttpServletResponse;
-
-import org.apache.logging.log4j.Logger;
-import org.springframework.web.servlet.handler.HandlerInterceptorAdapter;
-
 import org.apache.geode.cache.Cache;
 import org.apache.geode.distributed.internal.DistributionConfig;
 import org.apache.geode.internal.logging.LogService;
@@ -34,6 +23,16 @@ 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;
+
+import java.util.Collections;
+import java.util.Enumeration;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Properties;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
 
 /**
  * The GetEnvironmentHandlerInterceptor class handles extracting Gfsh 
environment variables encoded
@@ -110,7 +109,12 @@ public class LoginHandlerInterceptor extends 
HandlerInterceptorAdapter {
 
     String username = requestParameterValues.get(ResourceConstants.USER_NAME);
     String password = requestParameterValues.get(ResourceConstants.PASSWORD);
-    this.securityService.login(username, password);
+    Properties credentials = new Properties();
+    if (username != null)
+      credentials.put(ResourceConstants.USER_NAME, username);
+    if (password != null)
+      credentials.put(ResourceConstants.PASSWORD, password);
+    this.securityService.login(credentials);
 
     ENV.set(requestParameterValues);
 

http://git-wip-us.apache.org/repos/asf/geode/blob/30341ec1/geode-core/src/test/java/org/apache/geode/distributed/internal/membership/gms/auth/AbstractGMSAuthenticatorTestCase.java
----------------------------------------------------------------------
diff --git 
a/geode-core/src/test/java/org/apache/geode/distributed/internal/membership/gms/auth/AbstractGMSAuthenticatorTestCase.java
 
b/geode-core/src/test/java/org/apache/geode/distributed/internal/membership/gms/auth/AbstractGMSAuthenticatorTestCase.java
index 65c38ad..3cdfd01 100644
--- 
a/geode-core/src/test/java/org/apache/geode/distributed/internal/membership/gms/auth/AbstractGMSAuthenticatorTestCase.java
+++ 
b/geode-core/src/test/java/org/apache/geode/distributed/internal/membership/gms/auth/AbstractGMSAuthenticatorTestCase.java
@@ -14,12 +14,8 @@
  */
 package org.apache.geode.distributed.internal.membership.gms.auth;
 
-import static org.assertj.core.api.Assertions.assertThat;
-import static org.mockito.Matchers.*;
-import static org.mockito.Mockito.*;
-
-import java.security.Principal;
-import java.util.Properties;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
 
 import org.apache.geode.LogWriter;
 import org.apache.geode.distributed.DistributedMember;
@@ -31,14 +27,15 @@ import org.apache.geode.internal.security.SecurityService;
 import org.apache.geode.security.AuthInitialize;
 import org.apache.geode.security.AuthenticationFailedException;
 import org.apache.geode.security.Authenticator;
-
 import org.apache.shiro.subject.Subject;
 import org.junit.Before;
-import org.junit.BeforeClass;
 import org.mockito.InjectMocks;
 import org.mockito.Mock;
 import org.mockito.MockitoAnnotations;
 
+import java.security.Principal;
+import java.util.Properties;
+
 public abstract class AbstractGMSAuthenticatorTestCase {
 
   @Mock
@@ -73,7 +70,7 @@ public abstract class AbstractGMSAuthenticatorTestCase {
 
     
when(securityService.isIntegratedSecurity()).thenReturn(isIntegratedSecurity());
     when(securityService.isPeerSecurityRequired()).thenReturn(true);
-    when(securityService.login(anyString(), anyString())).thenReturn(subject);
+    when(securityService.login(securityProps)).thenReturn(subject);
     when(distributionConfig.getSecurityProps()).thenReturn(securityProps);
     when(serviceConfig.getDistributionConfig()).thenReturn(distributionConfig);
     
when(services.getSecurityLogWriter()).thenReturn(mock(InternalLogWriter.class));

http://git-wip-us.apache.org/repos/asf/geode/blob/30341ec1/geode-core/src/test/java/org/apache/geode/management/internal/security/IntegratedSecurityServiceWithIniFileJUnitTest.java
----------------------------------------------------------------------
diff --git 
a/geode-core/src/test/java/org/apache/geode/management/internal/security/IntegratedSecurityServiceWithIniFileJUnitTest.java
 
b/geode-core/src/test/java/org/apache/geode/management/internal/security/IntegratedSecurityServiceWithIniFileJUnitTest.java
index a300880..8561f18 100644
--- 
a/geode-core/src/test/java/org/apache/geode/management/internal/security/IntegratedSecurityServiceWithIniFileJUnitTest.java
+++ 
b/geode-core/src/test/java/org/apache/geode/management/internal/security/IntegratedSecurityServiceWithIniFileJUnitTest.java
@@ -14,22 +14,21 @@
  */
 package org.apache.geode.management.internal.security;
 
-import static org.apache.geode.distributed.ConfigurationProperties.*;
-import static org.assertj.core.api.Assertions.*;
-
-import java.util.Properties;
+import static 
org.apache.geode.distributed.ConfigurationProperties.SECURITY_SHIRO_INIT;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
 
+import org.apache.geode.internal.security.IntegratedSecurityService;
+import org.apache.geode.internal.security.SecurityService;
+import org.apache.geode.security.GemFireSecurityException;
 import org.apache.geode.security.ResourcePermission;
+import org.apache.geode.test.junit.categories.IntegrationTest;
+import org.apache.geode.test.junit.categories.SecurityTest;
 import org.junit.Before;
 import org.junit.BeforeClass;
 import org.junit.Test;
 import org.junit.experimental.categories.Category;
 
-import org.apache.geode.internal.security.IntegratedSecurityService;
-import org.apache.geode.internal.security.SecurityService;
-import org.apache.geode.security.GemFireSecurityException;
-import org.apache.geode.test.junit.categories.IntegrationTest;
-import org.apache.geode.test.junit.categories.SecurityTest;
+import java.util.Properties;
 
 /**
  * Integration tests for {@link IntegratedSecurityService} using shiro.ini
@@ -53,7 +52,7 @@ public class IntegratedSecurityServiceWithIniFileJUnitTest {
 
   @Test
   public void testRoot() {
-    this.securityService.login("root", "secret");
+    this.securityService.login(loginCredentials("root", "secret"));
     this.securityService.authorize(TestCommand.none);
     this.securityService.authorize(TestCommand.everyOneAllowed);
     this.securityService.authorize(TestCommand.dataRead);
@@ -66,7 +65,7 @@ public class IntegratedSecurityServiceWithIniFileJUnitTest {
 
   @Test
   public void testGuest() {
-    this.securityService.login("guest", "guest");
+    this.securityService.login(loginCredentials("guest", "guest"));
     this.securityService.authorize(TestCommand.none);
     this.securityService.authorize(TestCommand.everyOneAllowed);
 
@@ -81,7 +80,7 @@ public class IntegratedSecurityServiceWithIniFileJUnitTest {
 
   @Test
   public void testRegionAReader() {
-    this.securityService.login("regionAReader", "password");
+    this.securityService.login(loginCredentials("regionAReader", "password"));
     this.securityService.authorize(TestCommand.none);
     this.securityService.authorize(TestCommand.everyOneAllowed);
     this.securityService.authorize(TestCommand.regionARead);
@@ -96,7 +95,7 @@ public class IntegratedSecurityServiceWithIniFileJUnitTest {
 
   @Test
   public void testRegionAUser() {
-    this.securityService.login("regionAUser", "password");
+    this.securityService.login(loginCredentials("regionAUser", "password"));
     this.securityService.authorize(TestCommand.none);
     this.securityService.authorize(TestCommand.everyOneAllowed);
     this.securityService.authorize(TestCommand.regionAWrite);
@@ -111,7 +110,7 @@ public class IntegratedSecurityServiceWithIniFileJUnitTest {
 
   @Test
   public void testDataReader() {
-    this.securityService.login("dataReader", "12345");
+    this.securityService.login(loginCredentials("dataReader", "12345"));
     this.securityService.authorize(TestCommand.none);
     this.securityService.authorize(TestCommand.everyOneAllowed);
     this.securityService.authorize(TestCommand.regionARead);
@@ -126,7 +125,7 @@ public class IntegratedSecurityServiceWithIniFileJUnitTest {
 
   @Test
   public void testReader() {
-    this.securityService.login("reader", "12345");
+    this.securityService.login(loginCredentials("reader", "12345"));
     this.securityService.authorize(TestCommand.none);
     this.securityService.authorize(TestCommand.everyOneAllowed);
     this.securityService.authorize(TestCommand.regionARead);
@@ -143,4 +142,11 @@ public class IntegratedSecurityServiceWithIniFileJUnitTest 
{
     assertThatThrownBy(() -> this.securityService.authorize(context))
         
.isInstanceOf(GemFireSecurityException.class).hasMessageContaining(context.toString());
   }
+
+  private Properties loginCredentials(String username, String password) {
+    Properties credentials = new Properties();
+    credentials.put(ResourceConstants.USER_NAME, username);
+    credentials.put(ResourceConstants.PASSWORD, password);
+    return credentials;
+  }
 }

http://git-wip-us.apache.org/repos/asf/geode/blob/30341ec1/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/security/GeodeAuthenticationProvider.java
----------------------------------------------------------------------
diff --git 
a/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/security/GeodeAuthenticationProvider.java
 
b/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/security/GeodeAuthenticationProvider.java
index c8b5cf0..06c0fb1 100644
--- 
a/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/security/GeodeAuthenticationProvider.java
+++ 
b/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/security/GeodeAuthenticationProvider.java
@@ -16,6 +16,9 @@
 
 package org.apache.geode.rest.internal.web.security;
 
+import org.apache.geode.internal.security.SecurityService;
+import org.apache.geode.management.internal.security.ResourceConstants;
+import org.apache.geode.security.GemFireSecurityException;
 import org.springframework.security.authentication.AuthenticationProvider;
 import org.springframework.security.authentication.BadCredentialsException;
 import 
org.springframework.security.authentication.UsernamePasswordAuthenticationToken;
@@ -24,8 +27,7 @@ import 
org.springframework.security.core.AuthenticationException;
 import org.springframework.security.core.authority.AuthorityUtils;
 import org.springframework.stereotype.Component;
 
-import org.apache.geode.internal.security.SecurityService;
-import org.apache.geode.security.GemFireSecurityException;
+import java.util.Properties;
 
 
 @Component
@@ -36,9 +38,14 @@ public class GeodeAuthenticationProvider implements 
AuthenticationProvider {
   public Authentication authenticate(Authentication authentication) throws 
AuthenticationException {
     String username = authentication.getName();
     String password = authentication.getCredentials().toString();
+    Properties credentials = new Properties();
+    if (username != null)
+      credentials.put(ResourceConstants.USER_NAME, username);
+    if (password != null)
+      credentials.put(ResourceConstants.PASSWORD, password);
 
     try {
-      securityService.login(username, password);
+      securityService.login(credentials);
       return new UsernamePasswordAuthenticationToken(username, password,
           AuthorityUtils.NO_AUTHORITIES);
     } catch (GemFireSecurityException e) {

Reply via email to