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) {