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 740d5e5675 GEODE-10243: Fail early if old client auth expires (#7603)
740d5e5675 is described below
commit 740d5e56758566146a1034f885197b742832c436
Author: Jinmei Liao <[email protected]>
AuthorDate: Thu Apr 21 12:24:39 2022 -0700
GEODE-10243: Fail early if old client auth expires (#7603)
* change the default "waitForReAuth" time to 60 seconds
---
.../security/AuthExpirationDistributedTest.java | 4 ++-
.../cache/tier/sockets/MessageDispatcher.java | 38 +++++++++++++++-------
.../cache/tier/sockets/MessageDispatcherTest.java | 27 ---------------
.../AuthExpirationBackwardCompatibleDUnitTest.java | 25 --------------
4 files changed, 30 insertions(+), 64 deletions(-)
diff --git
a/geode-core/src/distributedTest/java/org/apache/geode/security/AuthExpirationDistributedTest.java
b/geode-core/src/distributedTest/java/org/apache/geode/security/AuthExpirationDistributedTest.java
index d1bb73e2c9..0c0fcf1439 100644
---
a/geode-core/src/distributedTest/java/org/apache/geode/security/AuthExpirationDistributedTest.java
+++
b/geode-core/src/distributedTest/java/org/apache/geode/security/AuthExpirationDistributedTest.java
@@ -21,6 +21,8 @@ import static
org.apache.geode.cache.query.dunit.SecurityTestUtils.createAndExec
import static
org.apache.geode.distributed.ConfigurationProperties.DURABLE_CLIENT_ID;
import static
org.apache.geode.distributed.ConfigurationProperties.SECURITY_CLIENT_AUTH_INIT;
import static
org.apache.geode.distributed.ConfigurationProperties.SECURITY_LOG_LEVEL;
+import static org.apache.geode.internal.lang.SystemProperty.DEFAULT_PREFIX;
+import static
org.apache.geode.internal.lang.SystemPropertyHelper.RE_AUTHENTICATE_WAIT_TIME;
import static org.apache.geode.test.awaitility.GeodeAwaitility.await;
import static org.assertj.core.api.Assertions.assertThat;
@@ -61,9 +63,9 @@ public class AuthExpirationDistributedTest {
public ServerStarterRule server = new ServerStarterRule()
.withSecurityManager(ExpirableSecurityManager.class)
.withProperty(SECURITY_LOG_LEVEL, "debug")
+ .withSystemProperty(DEFAULT_PREFIX + RE_AUTHENTICATE_WAIT_TIME, "5000")
.withRegion(RegionShortcut.REPLICATE, "region");
-
private ClientVM clientVM;
@Before
diff --git
a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/MessageDispatcher.java
b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/MessageDispatcher.java
index a03ab4bf81..4af2e970c4 100644
---
a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/MessageDispatcher.java
+++
b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/MessageDispatcher.java
@@ -14,7 +14,6 @@
*/
package org.apache.geode.internal.cache.tier.sockets;
-import static org.apache.geode.internal.cache.EntryEventImpl.deserialize;
import static
org.apache.geode.internal.cache.tier.sockets.ClientReAuthenticateMessage.RE_AUTHENTICATION_START_VERSION;
import static
org.apache.geode.internal.lang.SystemPropertyHelper.RE_AUTHENTICATE_WAIT_TIME;
import static org.apache.geode.util.internal.UncheckedUtils.uncheckedCast;
@@ -22,6 +21,7 @@ import static
org.apache.geode.util.internal.UncheckedUtils.uncheckedCast;
import java.io.IOException;
import java.net.Socket;
import java.nio.ByteBuffer;
+import java.time.Duration;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Iterator;
@@ -83,7 +83,7 @@ public class MessageDispatcher extends LoggingThread {
/**
* Default value in milliseconds for waiting for re-authentication
*/
- private static final long DEFAULT_RE_AUTHENTICATE_WAIT_TIME = 5000;
+ private static final long DEFAULT_RE_AUTHENTICATE_WAIT_TIME =
Duration.ofMinutes(1).toMillis();
/**
* The queue of messages to be sent to the client
@@ -531,6 +531,10 @@ public class MessageDispatcher extends LoggingThread {
private boolean
handleAuthenticationExpiredException(AuthenticationExpiredException expired)
throws InterruptedException {
+ if (unregisterUnsupportedClient(expired)) {
+ return true;
+ }
+
long reAuthenticateWaitTime =
getSystemProperty(RE_AUTHENTICATE_WAIT_TIME,
DEFAULT_RE_AUTHENTICATE_WAIT_TIME);
synchronized (reAuthenticationLock) {
@@ -539,15 +543,9 @@ public class MessageDispatcher extends LoggingThread {
// flag for the notification to happen.
waitForReAuthenticationStartTime = System.currentTimeMillis();
subjectUpdated = false;
- // only send the message to clients who can handle the message
- if
(getProxy().getVersion().isNewerThanOrEqualTo(RE_AUTHENTICATION_START_VERSION))
{
- EventID eventId = createEventId();
- sendMessageDirectly(new ClientReAuthenticateMessage(eventId));
- }
+ EventID eventId = createEventId();
+ sendMessageDirectly(new ClientReAuthenticateMessage(eventId));
- // We wait for all versions of clients to re-authenticate. For older
clients we still
- // wait, just in case client will perform some operations to
- // trigger credential refresh on its own.
long waitFinishTime = waitForReAuthenticationStartTime +
reAuthenticateWaitTime;
long remainingWaitTime = waitFinishTime - System.currentTimeMillis();
while (!subjectUpdated && remainingWaitTime > 0) {
@@ -565,12 +563,30 @@ public class MessageDispatcher extends LoggingThread {
"Client did not re-authenticate back successfully in {} ms.
Unregister this client proxy.",
elapsedTime);
pauseOrUnregisterProxy(expired);
- return true;
}
+ return true;
}
return false;
}
+ /**
+ * for old client, don't wait for re-auth but unregister this proxy
completely.
+ */
+ private boolean unregisterUnsupportedClient(AuthenticationExpiredException
expired) {
+ if
(getProxy().getVersion().isNewerThanOrEqualTo(RE_AUTHENTICATION_START_VERSION))
{
+ return false;
+ }
+
+ logger.warn(
+ "Authentication expired for a client with a version less than Geode
1.15. Cannot re-authenticate an older client "
+ + "that has a server to client queue for CQs or interest
registrations. "
+ + "Please upgrade your client to Geode 1.15 or greater to allow
re-authentication.");
+ synchronized (_stopDispatchingLock) {
+ pauseOrUnregisterProxy(expired);
+ }
+ return true;
+ }
+
@VisibleForTesting
void dispatchResidualMessages() {
List<ClientMessage> list = new ArrayList<>();
diff --git
a/geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/MessageDispatcherTest.java
b/geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/MessageDispatcherTest.java
index a01bc7bf1c..e874a3cfdf 100644
---
a/geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/MessageDispatcherTest.java
+++
b/geode-core/src/test/java/org/apache/geode/internal/cache/tier/sockets/MessageDispatcherTest.java
@@ -18,7 +18,6 @@
package org.apache.geode.internal.cache.tier.sockets;
import static
org.apache.geode.internal.lang.SystemPropertyHelper.RE_AUTHENTICATE_WAIT_TIME;
-import static org.apache.geode.test.awaitility.GeodeAwaitility.await;
import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyLong;
@@ -174,32 +173,6 @@ public class MessageDispatcherTest {
verify(dispatcher, never()).dispatchResidualMessages();
}
-
- @Test
- public void oldClientWillContinueToDeliverMessageIfNotified() throws
Exception {
- doReturn(false, false, true).when(dispatcher).isStopped();
- // make sure wait time is short
-
doReturn(10000L).when(dispatcher).getSystemProperty(eq(RE_AUTHENTICATE_WAIT_TIME),
anyLong());
-
doThrow(AuthenticationExpiredException.class).when(dispatcher).dispatchMessage(any());
- when(messageQueue.peek()).thenReturn(message);
- when(proxy.getVersion()).thenReturn(KnownVersion.GEODE_1_14_0);
-
- Thread dispatcherThread = new Thread(() -> dispatcher.runDispatcher());
- Thread notifyThread = new Thread(() ->
dispatcher.notifyReAuthentication());
-
- dispatcherThread.start();
- await().until(() -> dispatcher.isWaitingForReAuthentication());
- notifyThread.start();
-
- dispatcherThread.join();
- notifyThread.join();
-
- verify(dispatcher, never()).sendMessageDirectly(any());
- // dispatcher will dispatch message
- verify(dispatcher,
never()).pauseOrUnregisterProxy(any(AuthenticationExpiredException.class));
- verify(dispatcher).dispatchResidualMessages();
- }
-
@Test
public void
ioExceptionHappenedForDurableClientWillContinueToPeekForNextMessage()
throws Exception {
diff --git
a/geode-core/src/upgradeTest/java/org/apache/geode/security/AuthExpirationBackwardCompatibleDUnitTest.java
b/geode-core/src/upgradeTest/java/org/apache/geode/security/AuthExpirationBackwardCompatibleDUnitTest.java
index fa4d4af818..67bd11fa27 100644
---
a/geode-core/src/upgradeTest/java/org/apache/geode/security/AuthExpirationBackwardCompatibleDUnitTest.java
+++
b/geode-core/src/upgradeTest/java/org/apache/geode/security/AuthExpirationBackwardCompatibleDUnitTest.java
@@ -366,31 +366,6 @@ public class AuthExpirationBackwardCompatibleDUnitTest {
assertThat(unAuthorizedOps.get("user1")).asList().contains("DATA:READ:region:11");
}
- @Test
- public void cqOlderClientWithClientInteractionWillDeliverEventEventually()
throws Exception {
- // this test should only test the older client
- if (TestVersion.compare(clientVersion, feature_start_version) >= 0) {
- return;
- }
- startClientWithCQ();
-
- Region<Object, Object> region = server.getCache().getRegion("/region");
- region.put("1", "value1");
- getSecurityManager().addExpiredUser("user1");
- region.put("2", "value2");
-
- clientVM.invoke(() -> {
- UpdatableUserAuthInitialize.setUser("user2");
- Region<Object, Object> proxyRegion =
- ClusterStartupRule.clientCacheRule.createProxyRegion("region");
- proxyRegion.put("3", "value3");
- await().untilAsserted(
- () -> assertThat(CQLISTENER0.getKeys())
- .containsExactly("1", "2", "3"));
-
- });
- }
-
@Test
public void createCQWillReAuth() throws Exception {
int serverPort = server.getPort();
