This is an automated email from the ASF dual-hosted git repository.
jinmeiliao 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 ccccc5e GEODE-9663: throw and handle AuthenticationExpiredException
at login time (#6927)
ccccc5e is described below
commit ccccc5e8031f314c3d58df0ba7bbdf0d68704bdb
Author: Jinmei Liao <[email protected]>
AuthorDate: Tue Oct 5 17:41:59 2021 -0700
GEODE-9663: throw and handle AuthenticationExpiredException at login time
(#6927)
* GEODE-9663: throw and handle AuthenticationExpiredException at login time
---
.../internal/executor/connection/AuthExecutor.java | 28 +++++----
.../executor/connection/AuthExecutorTest.java | 70 +++++++++++++++++++++
.../ClusterConfigurationSecurityDUnitTest.java | 2 +-
.../IntegratedSecurityPeerAuthDUnitTest.java | 9 +--
.../PeerSecurityWithEmbeddedLocatorDUnitTest.java | 2 +-
.../security/StartServerAuthorizationTest.java | 2 +-
.../SecurityWithExpirationIntegrationTest.java | 3 +-
.../cache/client/internal/AuthenticateUserOp.java | 27 +++++++-
.../internal/cache/tier/sockets/Handshake.java | 3 +-
.../cache/tier/sockets/ServerConnection.java | 4 +-
.../security/IntegratedSecurityService.java | 12 +++-
.../org/apache/geode/security/SecurityManager.java | 7 ++-
.../client/internal/AuthenticateUserOpTest.java | 73 +++++++++++++++++++++-
.../security/IntegratedSecurityServiceTest.java | 57 +++++++++++++++++
.../AuthExpirationMultiServerDUnitTest.java | 56 +++++++++++++++++
.../geode/security/ExpirableSecurityManager.java | 2 +-
.../gemstone/gemfire/OldClientSupportProvider.java | 11 ++--
...usterManagementSecurityRestIntegrationTest.java | 2 +-
18 files changed, 334 insertions(+), 36 deletions(-)
diff --git
a/geode-apis-compatible-with-redis/src/main/java/org/apache/geode/redis/internal/executor/connection/AuthExecutor.java
b/geode-apis-compatible-with-redis/src/main/java/org/apache/geode/redis/internal/executor/connection/AuthExecutor.java
index 1edc56f..e5e8682 100644
---
a/geode-apis-compatible-with-redis/src/main/java/org/apache/geode/redis/internal/executor/connection/AuthExecutor.java
+++
b/geode-apis-compatible-with-redis/src/main/java/org/apache/geode/redis/internal/executor/connection/AuthExecutor.java
@@ -29,6 +29,7 @@ import org.apache.geode.redis.internal.executor.Executor;
import org.apache.geode.redis.internal.executor.RedisResponse;
import org.apache.geode.redis.internal.netty.Command;
import org.apache.geode.redis.internal.netty.ExecutionHandlerContext;
+import org.apache.geode.security.AuthenticationExpiredException;
import org.apache.geode.security.AuthenticationFailedException;
import org.apache.geode.security.SecurityManager;
@@ -36,8 +37,6 @@ public class AuthExecutor implements Executor {
@Override
public RedisResponse executeCommand(Command command, ExecutionHandlerContext
context) {
- List<byte[]> commandElems = command.getProcessedCommand();
-
SecurityService securityService = context.getSecurityService();
// We're deviating from Redis here in that any AUTH requests, without
security explicitly
@@ -46,23 +45,28 @@ public class AuthExecutor implements Executor {
return
RedisResponse.error(ERROR_AUTH_CALLED_WITHOUT_SECURITY_CONFIGURED);
}
- Properties props = new Properties();
- if (commandElems.size() == 2) {
- props.setProperty(SecurityManager.USER_NAME, context.getRedisUsername());
- props.setProperty(SecurityManager.PASSWORD,
bytesToString(commandElems.get(1)));
- } else {
- props.setProperty(SecurityManager.USER_NAME,
bytesToString(commandElems.get(1)));
- props.setProperty(SecurityManager.PASSWORD,
bytesToString(commandElems.get(2)));
- }
-
+ Properties props = getSecurityProperties(command, context);
try {
Subject subject = securityService.login(props);
context.setSubject(subject);
- } catch (AuthenticationFailedException ex) {
+ } catch (AuthenticationFailedException | AuthenticationExpiredException
ex) {
return RedisResponse.wrongpass(ERROR_INVALID_USERNAME_OR_PASSWORD);
}
return RedisResponse.ok();
}
+ Properties getSecurityProperties(Command command, ExecutionHandlerContext
context) {
+ Properties properties = new Properties();
+ List<byte[]> commandElems = command.getProcessedCommand();
+ if (commandElems.size() == 2) {
+ properties.setProperty(SecurityManager.USER_NAME,
context.getRedisUsername());
+ properties.setProperty(SecurityManager.PASSWORD,
bytesToString(commandElems.get(1)));
+ } else {
+ properties.setProperty(SecurityManager.USER_NAME,
bytesToString(commandElems.get(1)));
+ properties.setProperty(SecurityManager.PASSWORD,
bytesToString(commandElems.get(2)));
+ }
+ return properties;
+ }
+
}
diff --git
a/geode-apis-compatible-with-redis/src/test/java/org/apache/geode/redis/internal/executor/connection/AuthExecutorTest.java
b/geode-apis-compatible-with-redis/src/test/java/org/apache/geode/redis/internal/executor/connection/AuthExecutorTest.java
new file mode 100644
index 0000000..13be66d
--- /dev/null
+++
b/geode-apis-compatible-with-redis/src/test/java/org/apache/geode/redis/internal/executor/connection/AuthExecutorTest.java
@@ -0,0 +1,70 @@
+/*
+ *
+ * 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.redis.internal.executor.connection;
+
+import static
org.apache.geode.redis.internal.RedisConstants.ERROR_AUTH_CALLED_WITHOUT_SECURITY_CONFIGURED;
+import static
org.apache.geode.redis.internal.RedisConstants.ERROR_INVALID_USERNAME_OR_PASSWORD;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.when;
+
+import java.util.Properties;
+
+import org.junit.Before;
+import org.junit.Test;
+
+import org.apache.geode.internal.security.SecurityService;
+import org.apache.geode.redis.internal.executor.RedisResponse;
+import org.apache.geode.redis.internal.netty.Command;
+import org.apache.geode.redis.internal.netty.ExecutionHandlerContext;
+import org.apache.geode.security.AuthenticationExpiredException;
+
+public class AuthExecutorTest {
+ private AuthExecutor executor;
+ private Command command;
+ private ExecutionHandlerContext context;
+ private SecurityService securityService;
+
+ @Before
+ public void before() {
+ executor = spy(AuthExecutor.class);
+ command = mock(Command.class);
+ context = mock(ExecutionHandlerContext.class);
+ securityService = mock(SecurityService.class);
+ when(context.getSecurityService()).thenReturn(securityService);
+ }
+
+ @Test
+ public void notIntegratedService() {
+ when(securityService.isIntegratedSecurity()).thenReturn(false);
+ RedisResponse redisResponse = executor.executeCommand(command, context);
+
assertThat(redisResponse.toString()).contains(ERROR_AUTH_CALLED_WITHOUT_SECURITY_CONFIGURED);
+ }
+
+ @Test
+ public void handleAuthExpirationException() {
+ when(securityService.isIntegratedSecurity()).thenReturn(true);
+ doReturn(new Properties()).when(executor).getSecurityProperties(command,
context);
+ when(securityService.login(any())).thenThrow(new
AuthenticationExpiredException("expired"));
+ RedisResponse redisResponse = executor.executeCommand(command, context);
+
assertThat(redisResponse.toString()).contains(ERROR_INVALID_USERNAME_OR_PASSWORD);
+ }
+}
diff --git
a/geode-core/src/distributedTest/java/org/apache/geode/security/ClusterConfigurationSecurityDUnitTest.java
b/geode-core/src/distributedTest/java/org/apache/geode/security/ClusterConfigurationSecurityDUnitTest.java
index 2426592..ae20b74 100644
---
a/geode-core/src/distributedTest/java/org/apache/geode/security/ClusterConfigurationSecurityDUnitTest.java
+++
b/geode-core/src/distributedTest/java/org/apache/geode/security/ClusterConfigurationSecurityDUnitTest.java
@@ -65,7 +65,7 @@ public class ClusterConfigurationSecurityDUnitTest {
properties.put("security-password", "invalidPassword");
assertThatThrownBy(() -> serverStarter.startServer(properties,
locator.getPort()))
.isInstanceOf(GemFireSecurityException.class)
- .hasMessageContaining("Security check failed. Authentication error.");
+ .hasMessageContaining("Security check failed. invalid
username/password");
}
@Test
diff --git
a/geode-core/src/distributedTest/java/org/apache/geode/security/IntegratedSecurityPeerAuthDUnitTest.java
b/geode-core/src/distributedTest/java/org/apache/geode/security/IntegratedSecurityPeerAuthDUnitTest.java
index b147d67..a6de76e 100644
---
a/geode-core/src/distributedTest/java/org/apache/geode/security/IntegratedSecurityPeerAuthDUnitTest.java
+++
b/geode-core/src/distributedTest/java/org/apache/geode/security/IntegratedSecurityPeerAuthDUnitTest.java
@@ -44,6 +44,7 @@ import org.apache.geode.test.junit.rules.ServerStarterRule;
@Category({SecurityTest.class})
public class IntegratedSecurityPeerAuthDUnitTest {
+ private static final String SERVER_3_IS_NOT_AUTHENTICATED = "server-3 is not
authenticated";
@ClassRule
public static ClusterStartupRule cluster = new ClusterStartupRule();
@@ -73,7 +74,7 @@ public class IntegratedSecurityPeerAuthDUnitTest {
cluster.getVM(2).invoke(() -> {
ServerStarterRule server = new ServerStarterRule();
server.withProperties(props).withConnectionToLocator(locatorPort).withAutoStart();
- assertThatThrownBy(() ->
server.before()).isInstanceOf(GemFireSecurityException.class)
+
assertThatThrownBy(server::before).isInstanceOf(GemFireSecurityException.class)
.hasMessageContaining("server-2 not authorized for CLUSTER:MANAGE");
});
}
@@ -87,8 +88,8 @@ public class IntegratedSecurityPeerAuthDUnitTest {
cluster.getVM(3).invoke(() -> {
ServerStarterRule server = new ServerStarterRule();
server.withProperties(props).withConnectionToLocator(locatorPort).withAutoStart();
- assertThatThrownBy(() ->
server.before()).isInstanceOf(GemFireSecurityException.class)
- .hasMessageContaining("Authentication error");
+
assertThatThrownBy(server::before).isInstanceOf(GemFireSecurityException.class)
+ .hasMessageContaining(SERVER_3_IS_NOT_AUTHENTICATED);
});
}
@@ -108,7 +109,7 @@ public class IntegratedSecurityPeerAuthDUnitTest {
// server1 and server2 are authenticated, server3 is not
String name = credentials.getProperty("name");
if ("server-3".equals(name)) {
- throw new AuthenticationFailedException("server-3 is not
authenticated");
+ throw new AuthenticationFailedException(SERVER_3_IS_NOT_AUTHENTICATED);
}
return name;
}
diff --git
a/geode-core/src/distributedTest/java/org/apache/geode/security/PeerSecurityWithEmbeddedLocatorDUnitTest.java
b/geode-core/src/distributedTest/java/org/apache/geode/security/PeerSecurityWithEmbeddedLocatorDUnitTest.java
index b805e91..c236942 100644
---
a/geode-core/src/distributedTest/java/org/apache/geode/security/PeerSecurityWithEmbeddedLocatorDUnitTest.java
+++
b/geode-core/src/distributedTest/java/org/apache/geode/security/PeerSecurityWithEmbeddedLocatorDUnitTest.java
@@ -66,7 +66,7 @@ public class PeerSecurityWithEmbeddedLocatorDUnitTest {
ClusterStartupRule.memberStarter = serverStarter;
assertThatThrownBy(() -> serverStarter.startServer(server2Props,
locatorPort))
.isInstanceOf(GemFireSecurityException.class)
- .hasMessageContaining("Security check failed. Authentication error");
+ .hasMessageContaining("Security check failed. invalid
username/password");
});
}
diff --git
a/geode-core/src/distributedTest/java/org/apache/geode/security/StartServerAuthorizationTest.java
b/geode-core/src/distributedTest/java/org/apache/geode/security/StartServerAuthorizationTest.java
index 79fd095..91d182c 100644
---
a/geode-core/src/distributedTest/java/org/apache/geode/security/StartServerAuthorizationTest.java
+++
b/geode-core/src/distributedTest/java/org/apache/geode/security/StartServerAuthorizationTest.java
@@ -58,7 +58,7 @@ public class StartServerAuthorizationTest {
assertThatThrownBy(() -> serverStarter.startServer(props,
locator.getPort()))
.isInstanceOf(GemFireSecurityException.class).hasMessageContaining(
- "Security check failed. Authentication error. Please check your
credentials");
+ "Security check failed. invalid username/password");
}
@Test
diff --git
a/geode-core/src/integrationTest/java/org/apache/geode/management/internal/security/SecurityWithExpirationIntegrationTest.java
b/geode-core/src/integrationTest/java/org/apache/geode/management/internal/security/SecurityWithExpirationIntegrationTest.java
index 598533a..cdab082 100644
---
a/geode-core/src/integrationTest/java/org/apache/geode/management/internal/security/SecurityWithExpirationIntegrationTest.java
+++
b/geode-core/src/integrationTest/java/org/apache/geode/management/internal/security/SecurityWithExpirationIntegrationTest.java
@@ -26,7 +26,6 @@ import org.junit.experimental.categories.Category;
import org.apache.geode.internal.security.SecurityService;
import org.apache.geode.internal.security.SecurityServiceFactory;
import org.apache.geode.security.AuthenticationExpiredException;
-import org.apache.geode.security.AuthenticationFailedException;
import org.apache.geode.security.ExpirableSecurityManager;
import org.apache.geode.test.junit.categories.SecurityTest;
@@ -47,7 +46,7 @@ public class SecurityWithExpirationIntegrationTest {
public void testAuthenticationWhenUserExpired() {
getSecurityManager().addExpiredUser("data");
assertThatThrownBy(() ->
this.securityService.login(loginCredentials("data", "data")))
- .isInstanceOf(AuthenticationFailedException.class);
+ .isInstanceOf(AuthenticationExpiredException.class);
}
@Test
diff --git
a/geode-core/src/main/java/org/apache/geode/cache/client/internal/AuthenticateUserOp.java
b/geode-core/src/main/java/org/apache/geode/cache/client/internal/AuthenticateUserOp.java
index a312e2e..43bafab 100644
---
a/geode-core/src/main/java/org/apache/geode/cache/client/internal/AuthenticateUserOp.java
+++
b/geode-core/src/main/java/org/apache/geode/cache/client/internal/AuthenticateUserOp.java
@@ -21,6 +21,7 @@ import java.util.Properties;
import org.apache.geode.DataSerializer;
import org.apache.geode.InternalGemFireError;
+import org.apache.geode.annotations.VisibleForTesting;
import org.apache.geode.cache.client.ServerOperationException;
import org.apache.geode.distributed.DistributedMember;
import org.apache.geode.distributed.internal.InternalDistributedSystem;
@@ -34,6 +35,7 @@ import org.apache.geode.internal.cache.tier.sockets.Part;
import org.apache.geode.internal.cache.tier.sockets.command.PutUserCredentials;
import org.apache.geode.internal.serialization.ByteArrayDataInput;
import org.apache.geode.internal.serialization.KnownVersion;
+import org.apache.geode.security.AuthenticationExpiredException;
import org.apache.geode.security.AuthenticationFailedException;
import org.apache.geode.security.AuthenticationRequiredException;
import org.apache.geode.security.NotAuthorizedException;
@@ -176,13 +178,30 @@ public class AuthenticateUserOp {
@Override
public Object attempt(Connection connection) throws Exception {
- if (connection.getServer().getRequiresCredentials()) {
- return super.attempt(connection);
- } else {
+ if (!connection.getServer().getRequiresCredentials()) {
return null;
}
+
+ try {
+ return parentAttempt(connection);
+ }
+ // if login failed for auth expired reason, try again once more
+ catch (AuthenticationExpiredException first) {
+ getMessage().clear();
+ try {
+ return parentAttempt(connection);
+ } catch (AuthenticationExpiredException second) {
+ throw new AuthenticationFailedException(second.getMessage(), second);
+ }
+ }
+ }
+
+ @VisibleForTesting
+ Object parentAttempt(Connection connection) throws Exception {
+ return super.attempt(connection);
}
+
@Override
protected Object processResponse(Message msg, Connection connection)
throws Exception {
byte[] bytes;
@@ -215,6 +234,8 @@ public class AuthenticateUserOp {
} else {
throw new AuthenticationFailedException(s, afe);
}
+ } else if (result instanceof AuthenticationExpiredException) {
+ throw (AuthenticationExpiredException) result;
} else if (result instanceof AuthenticationRequiredException) {
throw new AuthenticationRequiredException(s,
(AuthenticationRequiredException) result);
} else if (result instanceof NotAuthorizedException) {
diff --git
a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/Handshake.java
b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/Handshake.java
index 6a3d221..7c7ce0c 100644
---
a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/Handshake.java
+++
b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/Handshake.java
@@ -44,6 +44,7 @@ import org.apache.geode.internal.security.SecurityService;
import org.apache.geode.internal.serialization.KnownVersion;
import org.apache.geode.logging.internal.log4j.api.LogService;
import org.apache.geode.security.AuthInitialize;
+import org.apache.geode.security.AuthenticationExpiredException;
import org.apache.geode.security.AuthenticationFailedException;
import org.apache.geode.security.AuthenticationRequiredException;
import org.apache.geode.security.Authenticator;
@@ -492,7 +493,7 @@ public abstract class Handshake {
auth.init(securityProperties, logWriter, securityLogWriter);
return auth.authenticate(credentials, member);
}
- } catch (AuthenticationFailedException ex) {
+ } catch (AuthenticationFailedException | AuthenticationExpiredException
ex) {
throw ex;
} catch (Exception ex) {
throw new AuthenticationFailedException(ex.getMessage(), ex);
diff --git
a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ServerConnection.java
b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ServerConnection.java
index 06c8c6b..eae9384 100644
---
a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ServerConnection.java
+++
b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/ServerConnection.java
@@ -76,6 +76,7 @@ import
org.apache.geode.internal.serialization.ByteArrayDataInput;
import org.apache.geode.internal.serialization.KnownVersion;
import org.apache.geode.internal.util.Breadcrumbs;
import org.apache.geode.logging.internal.log4j.api.LogService;
+import org.apache.geode.security.AuthenticationExpiredException;
import org.apache.geode.security.AuthenticationFailedException;
import org.apache.geode.security.AuthenticationRequiredException;
import org.apache.geode.security.GemFireSecurityException;
@@ -1179,7 +1180,8 @@ public class ServerConnection implements Runnable {
// create secure part which will be send in response
return encryptId(uniqueId);
- } catch (AuthenticationFailedException | AuthenticationRequiredException
exception) {
+ } catch (AuthenticationFailedException | AuthenticationRequiredException
+ | AuthenticationExpiredException exception) {
throw exception;
} catch (Exception exception) {
throw new AuthenticationFailedException("REPLY_REFUSED", exception);
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 e6cf99f..c7425dd 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
@@ -42,6 +42,7 @@ import
org.apache.geode.internal.security.shiro.SecurityManagerProvider;
import org.apache.geode.internal.security.shiro.ShiroPrincipal;
import org.apache.geode.internal.util.BlobHelper;
import org.apache.geode.logging.internal.log4j.api.LogService;
+import org.apache.geode.security.AuthenticationExpiredException;
import org.apache.geode.security.AuthenticationFailedException;
import org.apache.geode.security.AuthenticationRequiredException;
import org.apache.geode.security.GemFireSecurityException;
@@ -159,8 +160,17 @@ public class IntegratedSecurityService implements
SecurityService {
currentUser.login(token);
} catch (ShiroException e) {
logger.info("error logging in: " + token.getPrincipal());
+ Throwable cause = e.getCause();
+ if (cause == null) {
+ throw new AuthenticationFailedException(
+ "Authentication error. Please check your credentials.", e);
+ }
+ if (cause instanceof AuthenticationFailedException
+ || cause instanceof AuthenticationExpiredException) {
+ throw (GemFireSecurityException) cause;
+ }
throw new AuthenticationFailedException(
- "Authentication error. Please check your credentials.", e);
+ "Authentication error. Please check your credentials.", cause);
}
Session currentSession = currentUser.getSession();
diff --git
a/geode-core/src/main/java/org/apache/geode/security/SecurityManager.java
b/geode-core/src/main/java/org/apache/geode/security/SecurityManager.java
index 0b11f4f..16e0e4e 100644
--- a/geode-core/src/main/java/org/apache/geode/security/SecurityManager.java
+++ b/geode-core/src/main/java/org/apache/geode/security/SecurityManager.java
@@ -65,8 +65,13 @@ public interface SecurityManager {
* as keys of the properties, also the properties generated by your
AuthInitialize
* interface
* @return a serializable principal object
+ * @throw AuthenticationFailedException if the credentials are invalid, this
exception will be
+ * seen by the client.
+ * @throw AuthenticationExpiredException if credentials have expired, this
will give the
+ * client a second chance to gather new credentials and try login
again once more.
*/
- Object authenticate(Properties credentials) throws
AuthenticationFailedException;
+ Object authenticate(Properties credentials)
+ throws AuthenticationFailedException, AuthenticationExpiredException;
/**
* Authorize the ResourcePermission for a given Principal
diff --git
a/geode-core/src/test/java/org/apache/geode/cache/client/internal/AuthenticateUserOpTest.java
b/geode-core/src/test/java/org/apache/geode/cache/client/internal/AuthenticateUserOpTest.java
index 568ca2b..f170bf2 100644
---
a/geode-core/src/test/java/org/apache/geode/cache/client/internal/AuthenticateUserOpTest.java
+++
b/geode-core/src/test/java/org/apache/geode/cache/client/internal/AuthenticateUserOpTest.java
@@ -16,11 +16,15 @@ package org.apache.geode.cache.client.internal;
import static
org.apache.geode.cache.client.internal.AuthenticateUserOp.NOT_A_USER_ID;
import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.doReturn;
+import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
@@ -32,7 +36,10 @@ import org.junit.Test;
import org.mockito.ArgumentCaptor;
import org.apache.geode.distributed.internal.InternalDistributedSystem;
+import org.apache.geode.distributed.internal.ServerLocation;
import org.apache.geode.internal.cache.tier.sockets.Message;
+import org.apache.geode.security.AuthenticationExpiredException;
+import org.apache.geode.security.AuthenticationFailedException;
public class AuthenticateUserOpTest {
@@ -41,6 +48,7 @@ public class AuthenticateUserOpTest {
private InternalDistributedSystem system;
private byte[] credentialBytes;
private Message message;
+ private ServerLocation server;
@Before
public void before() throws Exception {
@@ -48,11 +56,13 @@ public class AuthenticateUserOpTest {
system = mock(InternalDistributedSystem.class);
credentialBytes = "test".getBytes(StandardCharsets.UTF_8);
message = mock(Message.class);
+ server = mock(ServerLocation.class);
+ when(connection.getServer()).thenReturn(server);
+ impl = spy(new AuthenticateUserOp.AuthenticateUserOpImpl());
}
@Test
public void constructorWithNoArg() throws Exception {
- impl = spy(new AuthenticateUserOp.AuthenticateUserOpImpl());
doReturn(system).when(impl).getConnectedSystem();
Properties properties = new Properties();
when(system.getSecurityProperties()).thenReturn(properties);
@@ -82,4 +92,65 @@ public class AuthenticateUserOpTest {
verify(impl).getCredentialBytes(eq(connection), captor.capture());
assertThat(captor.getValue()).isEqualTo(properties);
}
+
+ @Test
+ public void noAttempt_if_NotRequireCredentials() throws Exception {
+ when(server.getRequiresCredentials()).thenReturn(false);
+ impl.attempt(connection);
+ verify(impl, never()).parentAttempt(connection);
+ }
+
+ @Test
+ public void callParentAttempt_IfRequireCredentials() throws Exception {
+ when(server.getRequiresCredentials()).thenReturn(true);
+ doReturn(null).when(impl).parentAttempt(connection);
+ impl.attempt(connection);
+ verify(impl).parentAttempt(connection);
+ }
+
+ @Test
+ public void whenParentAttemptThrowAuthenticationExpiredException() throws
Exception {
+ when(server.getRequiresCredentials()).thenReturn(true);
+ doThrow(new
AuthenticationExpiredException("expired")).when(impl).parentAttempt(connection);
+ assertThatThrownBy(() -> impl.attempt(connection))
+ .isInstanceOf(AuthenticationFailedException.class)
+ .hasCauseInstanceOf(AuthenticationExpiredException.class)
+ .hasMessageContaining("expired");
+ verify(impl, times(2)).parentAttempt(connection);
+ }
+
+ @Test
+ public void whenParentAttemptThrowAuthenticationFailedException() throws
Exception {
+ when(server.getRequiresCredentials()).thenReturn(true);
+ doThrow(new
AuthenticationFailedException("failed")).when(impl).parentAttempt(connection);
+ assertThatThrownBy(() -> impl.attempt(connection))
+ .isInstanceOf(AuthenticationFailedException.class)
+ .hasNoCause()
+ .hasMessageContaining("failed");
+ verify(impl).parentAttempt(connection);
+ }
+
+ @Test
+ public void
whenParentAttemptThrowAuthenticationExpiredException_ThenAuthenticationFailedException()
+ throws Exception {
+ when(server.getRequiresCredentials()).thenReturn(true);
+ doThrow(new AuthenticationExpiredException("expired"),
+ new
AuthenticationFailedException("failed")).when(impl).parentAttempt(connection);
+ assertThatThrownBy(() -> impl.attempt(connection))
+ .isInstanceOf(AuthenticationFailedException.class)
+ .hasNoCause()
+ .hasMessageContaining("failed");
+ verify(impl, times(2)).parentAttempt(connection);
+ }
+
+ @Test
+ public void
whenParentAttemptThrowAuthenticationExpiredException_ThenSucceed()
+ throws Exception {
+ when(server.getRequiresCredentials()).thenReturn(true);
+ doThrow(new AuthenticationExpiredException("expired"))
+ .doReturn(null)
+ .when(impl).parentAttempt(connection);
+ impl.attempt(connection);
+ verify(impl, times(2)).parentAttempt(connection);
+ }
}
diff --git
a/geode-core/src/test/java/org/apache/geode/internal/security/IntegratedSecurityServiceTest.java
b/geode-core/src/test/java/org/apache/geode/internal/security/IntegratedSecurityServiceTest.java
index f1aa2eb..899f7ab 100644
---
a/geode-core/src/test/java/org/apache/geode/internal/security/IntegratedSecurityServiceTest.java
+++
b/geode-core/src/test/java/org/apache/geode/internal/security/IntegratedSecurityServiceTest.java
@@ -17,11 +17,13 @@ package org.apache.geode.internal.security;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
import java.util.Properties;
+import org.apache.shiro.ShiroException;
import org.apache.shiro.session.Session;
import org.apache.shiro.subject.Subject;
import org.apache.shiro.subject.SubjectContext;
@@ -32,7 +34,10 @@ import org.junit.After;
import org.junit.Before;
import org.junit.Test;
+import org.apache.geode.internal.security.shiro.GeodeAuthenticationToken;
import org.apache.geode.internal.security.shiro.SecurityManagerProvider;
+import org.apache.geode.security.AuthenticationExpiredException;
+import org.apache.geode.security.AuthenticationFailedException;
import org.apache.geode.security.AuthenticationRequiredException;
import org.apache.geode.security.PostProcessor;
import org.apache.geode.security.SecurityManager;
@@ -45,6 +50,8 @@ public class IntegratedSecurityServiceTest {
private org.apache.shiro.mgt.SecurityManager shiroManager;
private IntegratedSecurityService securityService;
+ private ShiroException shiroException;
+ private Properties properties;
@Before
public void before() throws Exception {
@@ -58,6 +65,9 @@ public class IntegratedSecurityServiceTest {
when(mockSubject.getPrincipal()).thenReturn("principal");
when(mockSubject.getSession()).thenReturn(mock(Session.class));
+ shiroException = mock(ShiroException.class);
+ properties = new Properties();
+
this.securityService = new IntegratedSecurityService(provider, null);
}
@@ -162,4 +172,51 @@ public class IntegratedSecurityServiceTest {
PostProcessor postProcessor = this.securityService.getPostProcessor();
assertThat(postProcessor).isNull();
}
+
+ @Test
+ public void login_when_credential_isNull() throws Exception {
+ assertThatThrownBy(() -> securityService.login(null))
+ .isInstanceOf(AuthenticationRequiredException.class)
+ .hasNoCause()
+ .hasMessageContaining("credentials are null");
+ }
+
+ @Test
+ public void login_when_ShiroException_hasNoCause() throws Exception {
+
doThrow(shiroException).when(mockSubject).login(any(GeodeAuthenticationToken.class));
+ assertThatThrownBy(() -> securityService.login(properties))
+ .isInstanceOf(AuthenticationFailedException.class)
+ .hasCauseInstanceOf(ShiroException.class)
+ .hasMessageContaining("Authentication error. Please check your
credentials");
+ }
+
+ @Test
+ public void login_when_ShiroException_causedBy_AuthenticationFailed() throws
Exception {
+
doThrow(shiroException).when(mockSubject).login(any(GeodeAuthenticationToken.class));
+ when(shiroException.getCause()).thenReturn(new
AuthenticationFailedException("failed"));
+ assertThatThrownBy(() -> securityService.login(properties))
+ .isInstanceOf(AuthenticationFailedException.class)
+ .hasNoCause()
+ .hasMessageContaining("failed");
+ }
+
+ @Test
+ public void login_when_ShiroException_causedBy_AuthenticationExpired()
throws Exception {
+
doThrow(shiroException).when(mockSubject).login(any(GeodeAuthenticationToken.class));
+ when(shiroException.getCause()).thenReturn(new
AuthenticationExpiredException("expired"));
+ assertThatThrownBy(() -> securityService.login(properties))
+ .isInstanceOf(AuthenticationExpiredException.class)
+ .hasNoCause()
+ .hasMessageContaining("expired");
+ }
+
+ @Test
+ public void login_when_ShiroException_causedBy_OtherExceptions() throws
Exception {
+
doThrow(shiroException).when(mockSubject).login(any(GeodeAuthenticationToken.class));
+ when(shiroException.getCause()).thenReturn(new RuntimeException("other
reasons"));
+ assertThatThrownBy(() -> securityService.login(properties))
+ .isInstanceOf(AuthenticationFailedException.class)
+ .hasCauseInstanceOf(RuntimeException.class)
+ .hasMessageContaining("Authentication error. Please check your
credentials.");
+ }
}
diff --git
a/geode-core/src/upgradeTest/java/org/apache/geode/security/AuthExpirationMultiServerDUnitTest.java
b/geode-core/src/upgradeTest/java/org/apache/geode/security/AuthExpirationMultiServerDUnitTest.java
index 7397c03..f010f8f 100644
---
a/geode-core/src/upgradeTest/java/org/apache/geode/security/AuthExpirationMultiServerDUnitTest.java
+++
b/geode-core/src/upgradeTest/java/org/apache/geode/security/AuthExpirationMultiServerDUnitTest.java
@@ -39,6 +39,8 @@ import org.apache.geode.cache.client.ClientRegionShortcut;
import org.apache.geode.cache.client.ServerOperationException;
import org.apache.geode.internal.cache.InternalCache;
import org.apache.geode.security.AuthExpirationDUnitTest.EventsCqListner;
+import org.apache.geode.test.concurrent.FileBasedCountDownLatch;
+import org.apache.geode.test.dunit.AsyncInvocation;
import org.apache.geode.test.dunit.rules.ClientVM;
import org.apache.geode.test.dunit.rules.ClusterStartupRule;
import org.apache.geode.test.dunit.rules.MemberVM;
@@ -253,6 +255,60 @@ public class AuthExpirationMultiServerDUnitTest {
.containsExactly("DATA:READ:partitionRegion:key0");
}
+ @Test
+ public void consecutivePut() throws Exception {
+ FileBasedCountDownLatch latch = new FileBasedCountDownLatch(1);
+ int locatorPort = locator.getPort();
+ // do consecutive puts using a client
+ ClientVM client = cluster.startClientVM(3,
+ c -> c.withProperty(SECURITY_CLIENT_AUTH_INIT,
UpdatableUserAuthInitialize.class.getName())
+ .withCacheSetup(ccf -> ccf.setPoolMaxConnections(2))
+ .withLocatorConnection(locatorPort));
+ AsyncInvocation<Void> invokePut = client.invokeAsync(() -> {
+ UpdatableUserAuthInitialize.setUser("user1");
+ Region<Object, Object> proxyRegion =
+ ClusterStartupRule.getClientCache()
+ .createClientRegionFactory(ClientRegionShortcut.CACHING_PROXY)
+ .create(PARTITION_REGION);
+ for (int i = 0; i < 1000; i++) {
+ // make sure at least some data is put by user2
+ if (i == 900) {
+ latch.await();
+ }
+ proxyRegion.put("key" + i, "value" + i);
+ }
+ });
+
+ client.invoke(() -> {
+ // wait till at least one of the data is in the region to expire the user
+ await().untilAsserted(() -> assertThat(getProxyRegion()).isNotNull());
+ await().untilAsserted(() ->
assertThat(getProxyRegion().size()).isGreaterThan(1));
+ UpdatableUserAuthInitialize.setUser("user2");
+ });
+ expireUserOnAllVms("user1");
+ latch.countDown();
+ invokePut.await();
+
+ ExpirableSecurityManager securityManager =
collectSecurityManagers(server1, server2);
+ Map<String, List<String>> authorizedOps =
securityManager.getAuthorizedOps();
+
+ assertThat(authorizedOps).hasSize(2);
+ assertThat(authorizedOps.get("user1").size() +
authorizedOps.get("user2").size())
+ .as(String.format("Combined sizes of user1 %s and user2 %s",
+ authorizedOps.get("user1").size(),
+ authorizedOps.get("user2").size()))
+ .isEqualTo(1000);
+ Map<String, List<String>> unAuthorizedOps =
securityManager.getUnAuthorizedOps();
+ assertThat(unAuthorizedOps).hasSize(1);
+ // user1 may not be unauthorized for just 1 operations, puts maybe done by
different
+ // connections
+ assertThat(unAuthorizedOps.get("user1")).isNotEmpty();
+ }
+
+ private static Region<Object, Object> getProxyRegion() {
+ return ClusterStartupRule.getClientCache().getRegion(PARTITION_REGION);
+ }
+
private void expireUserOnAllVms(String user) {
MemberVM.invokeInEveryMember(() -> {
getSecurityManager().addExpiredUser(user);
diff --git
a/geode-junit/src/main/java/org/apache/geode/security/ExpirableSecurityManager.java
b/geode-junit/src/main/java/org/apache/geode/security/ExpirableSecurityManager.java
index 9ad5069..cf98dd8 100644
---
a/geode-junit/src/main/java/org/apache/geode/security/ExpirableSecurityManager.java
+++
b/geode-junit/src/main/java/org/apache/geode/security/ExpirableSecurityManager.java
@@ -46,7 +46,7 @@ public class ExpirableSecurityManager extends
SimpleSecurityManager implements S
public Object authenticate(final Properties credentials) throws
AuthenticationFailedException {
Object user = super.authenticate(credentials);
if (expired_users.contains((String) user)) {
- throw new AuthenticationFailedException("User already expired.");
+ throw new AuthenticationExpiredException("User already expired.");
}
return user;
}
diff --git
a/geode-old-client-support/src/main/java/com/gemstone/gemfire/OldClientSupportProvider.java
b/geode-old-client-support/src/main/java/com/gemstone/gemfire/OldClientSupportProvider.java
index f8fb848..4730119 100644
---
a/geode-old-client-support/src/main/java/com/gemstone/gemfire/OldClientSupportProvider.java
+++
b/geode-old-client-support/src/main/java/com/gemstone/gemfire/OldClientSupportProvider.java
@@ -127,11 +127,13 @@ public class OldClientSupportProvider implements
OldClientSupportService {
return theThrowable;
}
- String className = theThrowable.getClass().getName();
-
// backward compatibility for authentication expiration
if
(clientVersion.isOlderThan(ClientReAuthenticateMessage.RE_AUTHENTICATION_START_VERSION))
{
- if (className.equals(AuthenticationExpiredException.class.getName())) {
+ if (theThrowable instanceof AuthenticationExpiredException) {
+ return new AuthenticationRequiredException(USER_NOT_FOUND);
+ }
+ Throwable cause = theThrowable.getCause();
+ if (cause instanceof AuthenticationExpiredException) {
return new AuthenticationRequiredException(USER_NOT_FOUND);
}
}
@@ -140,8 +142,7 @@ public class OldClientSupportProvider implements
OldClientSupportService {
return theThrowable;
}
-
-
+ String className = theThrowable.getClass().getName();
// this class has been renamed, so it cannot be automatically translated
// during java deserialization
if
(className.equals("org.apache.geode.cache.execute.EmptyRegionFunctionException"))
{
diff --git
a/geode-web-management/src/integrationTest/java/org/apache/geode/management/internal/rest/ClusterManagementSecurityRestIntegrationTest.java
b/geode-web-management/src/integrationTest/java/org/apache/geode/management/internal/rest/ClusterManagementSecurityRestIntegrationTest.java
index 29e5892..e2bd836 100644
---
a/geode-web-management/src/integrationTest/java/org/apache/geode/management/internal/rest/ClusterManagementSecurityRestIntegrationTest.java
+++
b/geode-web-management/src/integrationTest/java/org/apache/geode/management/internal/rest/ClusterManagementSecurityRestIntegrationTest.java
@@ -159,7 +159,7 @@ public class ClusterManagementSecurityRestIntegrationTest {
.andExpect(status().isUnauthorized())
.andExpect(jsonPath("$.statusCode", is("UNAUTHENTICATED")))
.andExpect(jsonPath("$.statusMessage",
- is("Authentication error. Please check your credentials.")));
+ is("Invalid username/password.")));
}
@Test
